Skip to content

Commit

Permalink
Merge pull request #7369 from marcuschangarm/fix-nrf52-serial
Browse files Browse the repository at this point in the history 
Fix race condition in serial_api.c for NRF52 series
  • Loading branch information
@0xc0170
0xc0170 committed Jul 2, 2018
2 parents 2da597e + 2d71866 commit 44acaf5
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 86 deletions.
6 changes: 3 additions & 3 deletions targets/TARGET_NORDIC/TARGET_NRF5x/README.md
View file
Expand Up @@ -108,15 +108,15 @@ All buffers can be resized to fit the application:
```
"name": "nordic",
"config": {
"uart-dma-size": {
"uart_dma_size": {
"help": "UART DMA buffer. 2 buffers per instance. DMA buffer is filled by UARTE",
"value": 8
},
"uart-0-fifo-size": {
"uart_0_fifo_size": {
"help": "UART0 FIFO buffer. FIFO buffer is filled from DMA buffer.",
"value": 32
},
"uart-1-fifo-size": {
"uart_1_fifo_size": {
"help": "UART1 FIFO buffer. FIFO buffer is filled from DMA buffer.",
"value": 32
}
Expand Down
106 changes: 23 additions & 83 deletions targets/TARGET_NORDIC/TARGET_NRF5x/TARGET_NRF52/serial_api.c
View file
Expand Up @@ -110,11 +110,6 @@
#define DMA_BUFFER_SIZE MBED_CONF_NORDIC_UART_DMA_SIZE
#define NUMBER_OF_BANKS 2

/**
* Default timer delay for callbacks.
*/
#define CALLBACK_DELAY_US 100

/**
* Use RTC2 for idle timeouts.
* Each channel is dedicated to one particular task.
Expand All @@ -130,10 +125,10 @@
/**
* SWI IRQ numbers
*/
#define UARTE0_SWI_TX_0_IRQ SWI2_EGU2_IRQn
#define UARTE0_SWI_RX_0_IRQ SWI3_EGU3_IRQn
#define UARTE1_SWI_TX_0_IRQ SWI4_EGU4_IRQn
#define UARTE1_SWI_RX_0_IRQ SWI5_EGU5_IRQn
#define UARTE0_SWI_TX_IRQ SWI2_EGU2_IRQn
#define UARTE0_SWI_RX_IRQ SWI3_EGU3_IRQn
#define UARTE1_SWI_TX_IRQ SWI4_EGU4_IRQn
#define UARTE1_SWI_RX_IRQ SWI5_EGU5_IRQn

/***
* _______ _ __
Expand Down Expand Up @@ -433,8 +428,8 @@ static void nordic_nrf5_uart_swi_rx_1(void)
*/
static void nordic_nrf5_uart_event_handler_endtx(int instance)
{
/* Disable TXDRDY event again. */
nordic_nrf5_uart_register[instance]->INTEN &= ~NRF_UARTE_INT_TXDRDY_MASK;
/* Disable ENDTX event again. */
nordic_nrf5_uart_register[instance]->INTEN &= ~NRF_UARTE_INT_ENDTX_MASK;

/* Release mutex. As the owner this call is safe. */
nordic_nrf5_uart_state[instance].tx_in_progress = 0;
Expand Down Expand Up @@ -519,12 +514,12 @@ static void nordic_swi_tx_trigger(int instance)
{
if (instance == 0) {

NVIC_SetPendingIRQ(UARTE0_SWI_TX_0_IRQ);
NVIC_SetPendingIRQ(UARTE0_SWI_TX_IRQ);
}
#if UART1_ENABLED
else if (instance == 1) {

NVIC_SetPendingIRQ(UARTE1_SWI_TX_0_IRQ);
NVIC_SetPendingIRQ(UARTE1_SWI_TX_IRQ);
}
#endif
}
Expand All @@ -538,11 +533,11 @@ static void nordic_swi_rx_trigger(int instance)
{
if (instance == 0) {

NVIC_SetPendingIRQ(UARTE0_SWI_RX_0_IRQ);
NVIC_SetPendingIRQ(UARTE0_SWI_RX_IRQ);
}
else if (instance == 1) {

NVIC_SetPendingIRQ(UARTE1_SWI_RX_0_IRQ);
NVIC_SetPendingIRQ(UARTE1_SWI_RX_IRQ);
}
}

Expand Down Expand Up @@ -716,33 +711,14 @@ static void nordic_nrf5_uart_event_handler(int instance)
nordic_nrf5_uart_event_handler_rxdrdy(instance);
}

/* Tx single character has been sent. */
if (nrf_uarte_event_check(nordic_nrf5_uart_register[instance], NRF_UARTE_EVENT_TXDRDY)) {

nrf_uarte_event_clear(nordic_nrf5_uart_register[instance], NRF_UARTE_EVENT_TXDRDY);

/* In non-async transfers this will generate an interrupt if callback and mask is set. */
if (!nordic_nrf5_uart_state[instance].tx_asynch) {

/* Use SWI to de-prioritize callback. */
nordic_swi_tx_trigger(instance);
}
}

#if DEVICE_SERIAL_ASYNCH
/* Tx DMA buffer has been sent. */
if (nrf_uarte_event_check(nordic_nrf5_uart_register[instance], NRF_UARTE_EVENT_ENDTX))
{
nrf_uarte_event_clear(nordic_nrf5_uart_register[instance], NRF_UARTE_EVENT_ENDTX);

/* Call async event handler in async mode. */
if (nordic_nrf5_uart_state[instance].tx_asynch) {

/* Use SWI to de-prioritize callback. */
nordic_swi_tx_trigger(instance);
}
/* Use SWI to de-prioritize callback. */
nordic_swi_tx_trigger(instance);
}
#endif
}

/**
Expand Down Expand Up @@ -1029,24 +1005,24 @@ void serial_init(serial_t *obj, PinName tx, PinName rx)
NRF_RTC_INT_COMPARE1_MASK);

/* Enable RTC2 IRQ. Priority is set to lowest so that the UARTE ISR can interrupt it. */
nrf_drv_common_irq_enable(RTC2_IRQn, APP_IRQ_PRIORITY_LOWEST);
nrf_drv_common_irq_enable(RTC2_IRQn, APP_IRQ_PRIORITY_HIGHEST);

/* Start RTC2. According to the datasheet the added power consumption is neglible so
* the RTC2 will run forever.
*/
nrf_rtc_task_trigger(NRF_RTC2, NRF_RTC_TASK_START);

/* Enable interrupts for SWI. */
NVIC_SetVector(UARTE0_SWI_TX_0_IRQ, (uint32_t) nordic_nrf5_uart_swi_tx_0);
NVIC_SetVector(UARTE0_SWI_RX_0_IRQ, (uint32_t) nordic_nrf5_uart_swi_rx_0);
nrf_drv_common_irq_enable(UARTE0_SWI_TX_0_IRQ, APP_IRQ_PRIORITY_LOWEST);
nrf_drv_common_irq_enable(UARTE0_SWI_RX_0_IRQ, APP_IRQ_PRIORITY_LOWEST);
NVIC_SetVector(UARTE0_SWI_TX_IRQ, (uint32_t) nordic_nrf5_uart_swi_tx_0);
NVIC_SetVector(UARTE0_SWI_RX_IRQ, (uint32_t) nordic_nrf5_uart_swi_rx_0);
nrf_drv_common_irq_enable(UARTE0_SWI_TX_IRQ, APP_IRQ_PRIORITY_LOWEST);
nrf_drv_common_irq_enable(UARTE0_SWI_RX_IRQ, APP_IRQ_PRIORITY_LOWEST);

#if UART1_ENABLED
NVIC_SetVector(UARTE1_SWI_TX_0_IRQ, (uint32_t) nordic_nrf5_uart_swi_tx_1);
NVIC_SetVector(UARTE1_SWI_RX_0_IRQ, (uint32_t) nordic_nrf5_uart_swi_rx_1);
nrf_drv_common_irq_enable(UARTE1_SWI_TX_0_IRQ, APP_IRQ_PRIORITY_LOWEST);
nrf_drv_common_irq_enable(UARTE1_SWI_RX_0_IRQ, APP_IRQ_PRIORITY_LOWEST);
NVIC_SetVector(UARTE1_SWI_TX_IRQ, (uint32_t) nordic_nrf5_uart_swi_tx_1);
NVIC_SetVector(UARTE1_SWI_RX_IRQ, (uint32_t) nordic_nrf5_uart_swi_rx_1);
nrf_drv_common_irq_enable(UARTE1_SWI_TX_IRQ, APP_IRQ_PRIORITY_LOWEST);
nrf_drv_common_irq_enable(UARTE1_SWI_RX_IRQ, APP_IRQ_PRIORITY_LOWEST);
#endif

/* Initialize FIFO buffer for UARTE0. */
Expand Down Expand Up @@ -1411,21 +1387,6 @@ void serial_irq_set(serial_t *obj, SerialIrq irq, uint32_t enable)

uart_object->mask &= ~type;
}

/* Enable TXDRDY event. */
if ((type == NORDIC_TX_IRQ) && enable) {

/* Clear Tx ready event and enable Tx ready interrupts. */
nrf_uarte_event_clear(nordic_nrf5_uart_register[uart_object->instance], NRF_UARTE_EVENT_TXDRDY);
nordic_nrf5_uart_register[uart_object->instance]->INTEN |= NRF_UARTE_INT_TXDRDY_MASK;

/* Disable TXDRDY event. */
} else if ((type == NORDIC_TX_IRQ) && !enable) {

/* Disable Tx ready interrupts and clear Tx ready event. */
nordic_nrf5_uart_register[uart_object->instance]->INTEN &= ~NRF_UARTE_INT_TXDRDY_MASK;
nrf_uarte_event_clear(nordic_nrf5_uart_register[uart_object->instance], NRF_UARTE_EVENT_TXDRDY);
}
}

/** Get character. This is a blocking call, waiting for a character
Expand Down Expand Up @@ -1503,26 +1464,15 @@ void serial_putc(serial_t *obj, int character)
/* Take ownership and configure UART if necessary. */
nordic_nrf5_serial_configure(obj);

/**
* The UARTE module can generate two different Tx events: TXDRDY when each character has
* been transmitted and ENDTX when the entire buffer has been sent.
*
* For the blocking serial_putc, TXDRDY interrupts are enabled and only used for the
* single character TX IRQ callback handler.
*/

/* Arm Tx DMA buffer. */
nordic_nrf5_uart_state[instance].tx_data = character;
nrf_uarte_tx_buffer_set(nordic_nrf5_uart_register[instance],
&nordic_nrf5_uart_state[instance].tx_data,
1);

/* Clear TXDRDY event and enable TXDRDY interrupts. */
nrf_uarte_event_clear(nordic_nrf5_uart_register[instance], NRF_UARTE_EVENT_TXDRDY);
nordic_nrf5_uart_register[instance]->INTEN |= NRF_UARTE_INT_TXDRDY_MASK;

/* Clear ENDTX event. */
/* Clear ENDTX event and enable interrupts. */
nrf_uarte_event_clear(nordic_nrf5_uart_register[instance], NRF_UARTE_EVENT_ENDTX);
nordic_nrf5_uart_register[instance]->INTEN |= NRF_UARTE_INT_ENDTX_MASK;

/* Trigger DMA transfer. */
nrf_uarte_task_trigger(nordic_nrf5_uart_register[instance],
Expand Down Expand Up @@ -1680,16 +1630,6 @@ int serial_tx_asynch(serial_t *obj, const void *tx, size_t tx_length, uint8_t tx
nordic_nrf5_uart_state[instance].tx_asynch = true;
nordic_nrf5_serial_configure(obj);

/**
* The UARTE module can generate two different Tx events: TXDRDY when each
* character has been transmitted and ENDTX when the entire buffer has been sent.
*
* For the async serial_tx_async, TXDRDY interrupts are disabled completely. ENDTX
* interrupts are enabled and used to signal the completion of the async transfer.
*
* The ENDTX interrupt is diabled immediately after it is fired in the ISR.
*/

/* Clear Tx event and enable Tx interrupts. */
nrf_uarte_event_clear(nordic_nrf5_uart_register[instance], NRF_UARTE_EVENT_ENDTX);
nordic_nrf5_uart_register[instance]->INTEN |= NRF_UARTE_INT_ENDTX_MASK;
Expand Down

0 comments on commit 44acaf5

Please sign in to comment.