Issue with my port? Thread priority broken?

I have a set-up where two threads are sending on the same UART port in about the same time. The port is protected by a mutex which is released when a tx has ended. The problem is that the thread with the lowest priority always ends up in the vListInsert() loop when looking in its message box. This does not seem to happen if the threads has the same priority. (Or at least, I have not seen it). At first, I thought it was interrupt stack related, but I have now dismissed that, since it is over hundred bytes unused in each thread stack. Then I thought it was related to the portEND_SWITCHING_ISR(), which I found a bug in. (see my previous forum thread containing info about interrupt priorities and levels: https://sourceforge.net/projects/freertos/forums/forum/382005/topic/5563352 ) Then I thought the mutex protection did not work as it should, but now I have done many tests on that, so I pretty confident it works. So now I’m leaning towards something is wrong with my port (very similar to the reference Coldfire 52259 demo). Maybe a set-up thingy or a misunderstanding on my side.
I have tried to disable configUSE_PREEMPTION, but issue remains.
I have tried to disable configUSE_RECURSIVE_MUTEXES, but issue remains.
How can I boil it down further? Code fragments:
void uart_send( int uartch, unsigned char *tx_buff, unsigned short len )
{
    if( xSemaphoreTake( uart_dev[uartch].mutex, 5000 ))
    {
        vTaskDelay(40);
        uart_dev[uartch].tx_buff = tx_buff;
        uart_dev[uartch].len = len;
        MCF_UART_UCR(uartch) = MCF_UART_UCR_TX_ENABLED;
    }
    else
    {
        DEBUG("uart_send: Fail, could not aquire mutex!");
    }
}
// Note: The delay above triggers the problem almost instantly, 
// without it, days can pass without bug appearing.
// Uart 0 @ interrupt source 13
void __attribute__ ((interrupt)) __cs3_isr_interrupt_77( void )
{
    unsigned char usr = MCF_UART0_USR;
    unsigned char uisr = MCF_UART0_UISR;
    int task_woken = FALSE;
    if( usr & MCF_UART_USR_RXRDY )   // rx interrupt ..
    {
        [..]
    }
    if( usr & MCF_UART_USR_TXRDY && uart_dev[0].dma == FALSE ) // tx interrupt
    {
        if( 0 <= --uart_dev[0].len )
            MCF_UART0_UTB = *uart_dev[0].tx_buff++;
        else
        {
            MCF_UART0_UCR = MCF_UART_UCR_TX_DISABLED;
            xSemaphoreGiveFromISR(uart_dev[0].mutex, &task_woken );
        }
    }
    portEND_SWITCHING_ISR( task_woken );
}
FreeRTOSConfig.h:
#define configUSE_PREEMPTION            1
#define configUSE_IDLE_HOOK             0
#define configUSE_TICK_HOOK             0
#define configCPU_CLOCK_HZ              ( ( unsigned portLONG ) 64000000 )
#define configTICK_RATE_HZ              ( ( portTickType ) 1000 )
#define configMINIMAL_STACK_SIZE        ( ( unsigned portSHORT ) 200 )
#define configTOTAL_HEAP_SIZE           ( ( size_t ) ( 15 * 1024))
#define configMAX_TASK_NAME_LEN         ( 5 )
#define configUSE_TRACE_FACILITY        1
#define configGENERATE_RUN_TIME_STATS   1
#define configUSE_16_BIT_TICKS          0
#define configIDLE_SHOULD_YIELD         0 
#define configUSE_CO_ROUTINES           0
#define configUSE_MUTEXES               1
#define configCHECK_FOR_STACK_OVERFLOW  2
#define configUSE_RECURSIVE_MUTEXES     1
#define configQUEUE_REGISTRY_SIZE       0
#define configUSE_COUNTING_SEMAPHORES   0
#define configMAX_PRIORITIES        ( ( unsigned portBASE_TYPE ) 5 )
#define configMAX_CO_ROUTINE_PRIORITIES ( 2 )
/* Set the following definitions to 1 to include the API function, or zero
to exclude the API function. */
#define INCLUDE_vTaskPrioritySet            0
#define INCLUDE_uxTaskPriorityGet           0
#define INCLUDE_vTaskDelete                 1
#define INCLUDE_vTaskCleanUpResources       0
#define INCLUDE_vTaskSuspend                0
#define INCLUDE_vTaskDelayUntil             1
#define INCLUDE_vTaskDelay                  1
#define INCLUDE_uxTaskGetStackHighWaterMark 1
#define    portCONFIGURE_TIMER_FOR_RUN_TIME_STATS() hr_timer_init()
#define    portGET_RUN_TIME_COUNTER_VALUE() hr_timer_get_value()
#define configYIELD_INTERRUPT_VECTOR            16UL
#define configKERNEL_INTERRUPT_PRIORITY         1
#define configMAX_SYSCALL_INTERRUPT_PRIORITY    7

Issue with my port? Thread priority broken?

One idea having looked at your code. Currently the implementation does not expect mutexes to be given from an interrupt, and there is a potential issue with doing this.  As you are not the first person I have noted using mutexes in this way (in fact, I do to, but know what to watch for), I will ensure future versions allow for this usage scenario too. This is perhaps a long shot, as the symptom you describe is not really what I would expect, but it is possible that giving a mutex in an interrupt while the queue is locked, and immediately before the task trying to take the mutex updates the mutex holder variable, that the mutex holder variable can be null.  . To allow interrupts to use mutexes in this way you would have to make a small change in tasks.c.  Currently you will see a function:
    void vTaskPriorityInherit( xTaskHandle * const pxMutexHolder )
    {
    tskTCB * const pxTCB = ( tskTCB * ) pxMutexHolder;
        configASSERT( pxMutexHolder );
        if( pxTCB->uxPriority < pxCurrentTCB->uxPriority )
        {
            /* Adjust the mutex holder state to account for its new priority. */
            listSET_LIST_ITEM_VALUE( &( pxTCB->xEventListItem ), configMAX_PRIORITIES - ( portTickType ) pxCurrentTCB->uxPriority );
            /* If the task being modified is in the ready state it will need to
            be moved in to a new list. */
            if( listIS_CONTAINED_WITHIN( &( pxReadyTasksLists[ pxTCB->uxPriority ] ), &( pxTCB->xGenericListItem ) ) != pdFALSE )
            {
                vListRemove( &( pxTCB->xGenericListItem ) );
                /* Inherit the priority before being moved into the new list. */
                pxTCB->uxPriority = pxCurrentTCB->uxPriority;
                prvAddTaskToReadyQueue( pxTCB );
            }
            else
            {
                /* Just inherit the priority. */
                pxTCB->uxPriority = pxCurrentTCB->uxPriority;
            }
            traceTASK_PRIORITY_INHERIT( pxTCB, pxCurrentTCB->uxPriority );
        }
    }
That can be changed to:
    void vTaskPriorityInherit( xTaskHandle * const pxMutexHolder )
    {
    tskTCB * const pxTCB = ( tskTCB * ) pxMutexHolder;
        /* If the mutex was given back by an interrupt while the queue was
        locked then the mutex holder might now be NULL. */
        if( pxMutexHolder != NULL )  // <<<<<<<<<<<< ADDED if() statement
        {
            if( pxTCB->uxPriority < pxCurrentTCB->uxPriority )
            {
                /* Adjust the mutex holder state to account for its new priority. */
                listSET_LIST_ITEM_VALUE( &( pxTCB->xEventListItem ), configMAX_PRIORITIES - ( portTickType ) pxCurrentTCB->uxPriority );
                /* If the task being modified is in the ready state it will need to
                be moved in to a new list. */
                if( listIS_CONTAINED_WITHIN( &( pxReadyTasksLists[ pxTCB->uxPriority ] ), &( pxTCB->xGenericListItem ) ) != pdFALSE )
                {
                    vListRemove( &( pxTCB->xGenericListItem ) );
                    /* Inherit the priority before being moved into the new list. */
                    pxTCB->uxPriority = pxCurrentTCB->uxPriority;
                    prvAddTaskToReadyQueue( pxTCB );
                }
                else
                {
                    /* Just inherit the priority. */
                    pxTCB->uxPriority = pxCurrentTCB->uxPriority;
                }
                traceTASK_PRIORITY_INHERIT( pxTCB, pxCurrentTCB->uxPriority );
            }
        }
    }
Alternatively, and breaking the FreeRTOS coding standard, you could just add:
if( pxMutexHolder == NULL ) return;
to the top of the function. Regards.

Issue with my port? Thread priority broken?

I was very exited to test this little change, but sadly it does not seam to help with my problem. I wish I had some hardware trace because interrupts are complicated to follow to say the least. I’m on version 7.01, should I upgrade? In you opinion, am I misusing xSemaphoreGiveFromISR()?
I use it in a few more places, and it is quite handy when interfacing slow IO devices.
As far as I can tell, it *seams* to work when threads are on the same priority level.
Is the intended use to have only one thread take the mutex? Thanks,
  Micael

Issue with my port? Thread priority broken?

For future reference (people searching the forum), my solution: I decided to re-write my code, so only one thread could allocate the mutex (and freed from interrupt), as i was never certain about the boundaries of xSemaphoreGiveFromISR, and did not want to stretch it without understanding the implications.

Issue with my port? Thread priority broken?

One point, as Richard alludes to, is that Mutexes, unlike Semaphores, are meant to be taken by a tasks, and then later given BY THE SAME TASK to release it. Then another task can take the Mutex. By definition, an ISR isn’t part of a task, so can not give a mutex. This “ownership” of the mutex is one part of the reason that it can due priority inheritance to avoid priority inversion. In contrast, a Semaphore isn’t “owned” by anyone  and can naturally be set and cleared by different entities. In your case, you are guarding a mailbox holding a message pointer and character count, but once a task gets the mailbox, and starts up the message, it does’t really act like it “owns” that mailbox, the mailbox is just busy. This sounds more like a semaphore, not a mutex. In fact, if the same task comes around and tries to send a second message before the first is done, if the mutex was defined as recursive, it would still be the owner of it, and proceed on and effectively abort the previous message. If the mutex isn’t recursive, then the code will check if the task should be bumped up in priority by inheriting from itself??? As I said, this is much more a semaphore activity, since once you start the action, the task no longer really has any control over it. One way that a mutex might make sense is that when a task want to start sending a message, it grabs the mutex for that channel, and within this mutex protected region, it might make multiple calls to send pieces of the message, and at the end gie the mutex so another task can send a message, This allows you to keep the message “atomic” even though it isn’t all sent with one call. This mutex arbitrates task to task relationships. The actually sending of each block, might use a semaphore to let the task know that it is ok to start another piece of the message.

Issue with my port? Thread priority broken?

Richard, Thank you very much for this excellent explanation! It all makes sense, and I re-wrote my code to use binary semaphores instead, as I already had my test bench up.
And:- Binary semaphore works just fine – should probably have used binary semaphore from the start. Not sure why I created using mutex rather than binary semaphores (This was two year or so ago), but reading the API documentation again, I should have tested this myself. I guess I was too focused in trying to find a problem with my port and/or my own code… Many thanks,
  Micael