From f364475e44d0c6bb1486c7e0e0fe8e0ff71dfa8a Mon Sep 17 00:00:00 2001 From: Mike Hamer Date: Wed, 15 Jun 2016 15:30:31 +0200 Subject: [PATCH] Fixed syslink unhandled overrun interrupt This commit fixes a bug, whereby an overrun of the syslink UART buffer was not handled in the interrupt routine, causing constant retriggering and reentry, stalling the system. Overruns are caused when high CPU/interrupt load causes the syslink UART data buffer to not be processed fast enough. This bug was fixed by: 1. handling unhandled conditions in the ISR, and 2. reordering/simplifying instructions in the ISR to reduce latency before data read. As it is currently implemented, every received byte causes the CPU to be interrupted, and the byte to be sent to the syslink for processing (this is due to a variable packet size and hence unknown transaction length). A nicer implementation would be to fix the packet length at its maximum (32 bytes), and use DMA for the entire RX transaction. This would entirely avert the problem of an overrun causing packet loss. I assert (without evidence) that the time required to interrupt the CPU every byte, is less than the overhead due to fixing the packet size at its maximum length. Signed-off-by: Mike Hamer --- src/drivers/src/uart_syslink.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/drivers/src/uart_syslink.c b/src/drivers/src/uart_syslink.c index b187f1d5a6..43495ade2b 100644 --- a/src/drivers/src/uart_syslink.c +++ b/src/drivers/src/uart_syslink.c @@ -23,6 +23,7 @@ * * uart_syslink.c - Uart syslink to nRF51 and raw access functions */ +#include #include /*ST includes */ @@ -308,9 +309,17 @@ void uartslkDmaIsr(void) void uartslkIsr(void) { portBASE_TYPE xHigherPriorityTaskWoken = pdFALSE; - uint8_t rxDataInterrupt; - if (USART_GetITStatus(UARTSLK_TYPE, USART_IT_TXE)) + // the following if statement replaces: + // if (USART_GetITStatus(UARTSLK_TYPE, USART_IT_RXNE) == SET) + // we do this check as fast as possible to minimize the chance of an overrun, + // which occasionally cause problems and cause packet loss at high CPU usage + if ((UARTSLK_TYPE->SR & (1<<5)) != 0) // if the RXNE interrupt has occurred + { + uint8_t rxDataInterrupt = (uint8_t)(UARTSLK_TYPE->DR & 0xFF); + xQueueSendFromISR(uartslkDataDelivery, &rxDataInterrupt, &xHigherPriorityTaskWoken); + } + else if (USART_GetITStatus(UARTSLK_TYPE, USART_IT_TXE) == SET) { if (outDataIsr && (dataIndexIsr < dataSizeIsr)) { @@ -324,11 +333,15 @@ void uartslkIsr(void) xSemaphoreGiveFromISR(waitUntilSendDone, &xHigherPriorityTaskWoken); } } - USART_ClearITPendingBit(UARTSLK_TYPE, USART_IT_TXE); - if (USART_GetITStatus(UARTSLK_TYPE, USART_IT_RXNE)) + else { - rxDataInterrupt = USART_ReceiveData(UARTSLK_TYPE) & 0x00FF; - xQueueSendFromISR(uartslkDataDelivery, &rxDataInterrupt, &xHigherPriorityTaskWoken); + /** if we get here, the error is most likely caused by an overrun! + * - PE (Parity error), FE (Framing error), NE (Noise error), ORE (OverRun error) + * - and IDLE (Idle line detected) pending bits are cleared by software sequence: + * - reading USART_SR register followed reading the USART_DR register. + */ + asm volatile ("" : "=m" (UARTSLK_TYPE->SR) : "r" (UARTSLK_TYPE->SR)); // force non-optimizable reads + asm volatile ("" : "=m" (UARTSLK_TYPE->DR) : "r" (UARTSLK_TYPE->DR)); // of these two registers } }