Semaphore intermittently not taken in task context when given in ISR context on nRF51822

ahedin25 wrote on Wednesday, March 07, 2018:

A semaphore that is given when the UART has completed transmission is not always taken by the task when it should be. The IDLE task is running when it should not be. If a timer expires, then the task will take the semaphore. This anomaly most often occurs when the transmit packet length is small (2-4 bytes).

Am I using the semaphore incorrectly? Is it possible there is an issue in the port specific implementation that was provided by Nordic?

ahedin25 wrote on Wednesday, March 07, 2018:

Details:
FreeRTOS Version 10.0.0

#define configUSE_PREEMPTION     1
#define configIDLE_SHOULD_YIELD  1
#define configUSE_TIME_SLICING   1 
  • IAR 8.20 doesn’t indicate that any stacks are overflowing and the overflow callback isn’t occurring. “configASSERT” is enabled.
  • The task blocking on the UART transmit complete semaphore is the highest priority task after TmrSrv and soft device task.
  • The code requires the nRF51 development kit to run. However, the firmware currently isn’t in a state that I can share it.
  • I have included a Saleae logic capture and a Precipio Tracealyzer log of the issue.
  • The semaphore/mutex macros determine whether or not to call the interrupt safe version.
void WriteUart(...)
{
  TAKE_MUTEX(pUart->tx.mutex);
  ErrorTimerStart(); // 50 ms
  
  ret_code_t status = 
    nrf_drv_uart_tx(pUart->pUartInstance, pData, Length);
  ASSERT_REBOOT( status == NRF_SUCCESS );
  TAKE_SEMAPHORE(pUart->tx.idle);  

  TP_LOW(3, true);
  ErrorTimerStop();
  if( pUart->errorTimeout )
  {
    pUart->errorTimeout = false;
  }

  GIVE_MUTEX(pUart->tx.mutex);
}

static void ErrorTimerCallback(TimerHandle_t Handle)
{  
  Uart_t *pUart = (Uart_t*)pvTimerGetTimerID(Handle);
  TP_HIGH(4, true);
  PRINT_VALUE(true, pUart->tx.busy);
  PRINT_VALUE(true, pUart->tx.confirmedBusy);
  pUart->errorTimeout = true;
  TP_LOW(4, true);
}

static void UartEventIsr(nrf_drv_uart_event_t * pEvent, void * pContext)
{
  Uart_t *pUart = (Uart_t *)pContext;
  
  switch( pEvent->type )
  {
  case NRF_DRV_UART_EVT_TX_DONE:
    TP_HIGH(1, true);
    GIVE_SEMAPHORE(pUart->tx.idle);
    uartObject.tx.busy = false;
    TP_HIGH(3, true);
    TP_LOW(1, true);
    break;

// other cases not shown
  
  }

}

ahedin25 wrote on Wednesday, March 07, 2018:

Precipio Tracealyzer Log (line 525).

ahedin25 wrote on Wednesday, March 07, 2018:

Saleae

rtel wrote on Wednesday, March 07, 2018:

Does GIVE_SEMAPHORE() also call portYIELD_FROM_ISR()?

ahedin25 wrote on Wednesday, March 07, 2018:

Yes.


#define GIVE_SEMAPHORE_FROM_ISR(s) do {         \
  BaseType_t macroYieldRequest = pdFALSE;         \
  BaseType_t macroStatus = xSemaphoreGiveFromISR((s), &macroYieldRequest); \
  ASSERT_WARNING( macroStatus == pdTRUE );        \
  portYIELD_FROM_ISR(macroYieldRequest);          \
} while(0)

#define GIVE_SEMAPHORE_THREAD(s) do {            \
  BaseType_t macroStatus = xSemaphoreGive(s);    \
  ASSERT_WARNING( macroStatus == pdTRUE );       \
} while(0)

#define GIVE_SEMAPHORE(s) do {  \
  if( INTERRUPT_CONTEXT() )     \
  {                             \
     GIVE_SEMAPHORE_FROM_ISR(s);  \
  }                             \
  else                          \
  {                             \
     GIVE_SEMAPHORE_THREAD(s);    \
  }                             \
} while(0)


#define TAKE_SEMAPHORE_FROM_ISR(s) do { \
  BaseType_t macroYieldRequest = pdFALSE;         \
  BaseType_t macroStatus = xSemaphoreTakeFromISR((s), &macroYieldRequest); \
  ASSERT_WARNING( macroStatus == pdTRUE );        \
  portYIELD_FROM_ISR(macroYieldRequest);          \
} while(0)

#define TAKE_SEMAPHORE_THREAD(s) do { \
  BaseType_t macroStatus = xSemaphoreTake((s), portMAX_DELAY);    \
  ASSERT_WARNING( macroStatus == pdTRUE ); \
} while(0)


#define TAKE_SEMAPHORE(s) do {  \
  if( INTERRUPT_CONTEXT() )     \
  {                             \
     ASSERT_REBOOT(FORCED);       \
  }                             \
  else                          \
  {                             \
     TAKE_SEMAPHORE_THREAD(s);    \
  }                             \
} while(0)


rtel wrote on Wednesday, March 07, 2018:

What type of semaphore is it? More specifically, is it a Mutex type, as
they cannot be used form an ISR.

ahedin25 wrote on Wednesday, March 07, 2018:

It is a binary semaphore.

The mutex isn’t needed, but I added it when testing to ensure only one thing could be in the write function at a time.

void Uart_Initialize(void)
{
  memset(&uartObject, 0, sizeof(uartObject));    

  uartObject.tx.idle = 
    xSemaphoreCreateBinary();
  ASSERT_WARNING( uartObject.tx.idle != NULL );
  
  uartObject.tx.mutex = 
    xSemaphoreCreateMutex();
  ASSERT_WARNING( uartObject.tx.mutex != NULL );
  
  uartObject.pUartInstance = &UART_INSTANCE;
  
  uartObject.errorTimerHandle =       
    xTimerCreate("error timer", 
                 MS_TO_TICKS(50),
                 ONE_SHOT_TIMER, 
                 &uartObject,  
                 ErrorTimerCallback);

  InitializeUartDriver(&uartObject, NRF_UART_BAUDRATE_115200);  
  
  PRINT(true, "Uart Initialized\r\n");
}

rtel wrote on Wednesday, March 07, 2018:

Normally this sort of problem is because the interrupt priorities are
set wrong - but you say you are using FreeRTOS V10 with configASSERT()
defined - V10 will catch nearly all (if not all) interrupt priority
issues, so I’m discounting that as a cause in this case. All the same,
can you try setting the priority of the UART interrupt to the lowest
possible to see if it makes any difference.

ahedin25 wrote on Wednesday, March 07, 2018:

I set it to APP_IRQ_PRIORITY_THREAD and the anomaly still occurs. I am baffled about what I did wrong.

This is a M0 part. I don’t have anything defined for portASSERT_IF_INTERRUPT_PRIORITY_INVALID(). Does that seem correct?

#if __CORTEX_M == (0x00U)
#define _PRIO_SD_HIGH       0
#define _PRIO_APP_HIGH      1
#define _PRIO_APP_MID       1
#define _PRIO_SD_LOW        2
#define _PRIO_APP_LOW       3
#define _PRIO_APP_LOWEST    3
#define _PRIO_THREAD        4
#elif __CORTEX_M == (0x04U)
#define _PRIO_SD_HIGH       0
#define _PRIO_SD_MID        1
#define _PRIO_APP_HIGH      2
#define _PRIO_APP_MID       3
#define _PRIO_SD_LOW        4
#define _PRIO_SD_LOWEST     5
#define _PRIO_APP_LOW       6
#define _PRIO_APP_LOWEST    7
#define _PRIO_THREAD        15
#else
    #error "No platform defined"
#endif


//lint -save -e113 -e452
/**@brief The interrupt priorities available to the application while the SoftDevice is active. */
typedef enum
{
#ifndef SOFTDEVICE_PRESENT
    APP_IRQ_PRIORITY_HIGHEST = _PRIO_SD_HIGH,
#else
    APP_IRQ_PRIORITY_HIGHEST = _PRIO_APP_HIGH,
#endif
    APP_IRQ_PRIORITY_HIGH    = _PRIO_APP_HIGH,
#ifndef SOFTDEVICE_PRESENT
    APP_IRQ_PRIORITY_MID     = _PRIO_SD_LOW,
#else
    APP_IRQ_PRIORITY_MID     = _PRIO_APP_MID,
#endif
    APP_IRQ_PRIORITY_LOW     = _PRIO_APP_LOW,
    APP_IRQ_PRIORITY_LOWEST  = _PRIO_APP_LOWEST,
    APP_IRQ_PRIORITY_THREAD  = _PRIO_THREAD     /**< "Interrupt level" when running in Thread Mode. */
} app_irq_priority_t;
//lint -restore

rtel wrote on Wednesday, March 07, 2018:

Ah - ok sorry - I thought it was an M4 part so you can ignore my
comments about interrupt priorities.

Can you please send me the port.c file you are using so I can compare it
to the ‘official’ one so I can see if there are any changes. You can
send it to r dot barry at freertos dot org. Thanks.

ahedin25 wrote on Wednesday, March 07, 2018:

The Nordic SoftDevice can block the tick interrupt from occurring. This can result in lost ticks (the RTC getting out of sync with the tick counter). The original port is in the folder nrf51_sdk12.3.0.

Nordic tech support provided me with changes to handle lost ticks.
My design doesn’t use tickless idle because I couldn’t get it to work properly.
The anomaly occurs with either version of the systick file.

ahedin25 wrote on Thursday, March 08, 2018:

last post retracted