From 5300e2e28d3ec16c59b77e3612004315460e4ad4 Mon Sep 17 00:00:00 2001 From: Zhanglong Xia Date: Wed, 28 Oct 2020 00:26:06 +0800 Subject: [PATCH] [nrf528xx] fix SPI issue (#5703) When the SPI is transmitting a frame, the ncp_spi sets the SPI to transmit the second frame. After the current frame is transmitted, the SPI generates the END and ACQUIRED event at the same time. The SPI driver processes the ACQUIRED event first and then the END event. The ACQUIRED event notifies the up layer to set the SPI GPIO interrupt pin. The END event notifies the up layer that the transmission has completed. Because the SPI tx/rx buffer variables in the `spi_slave.c` has been updated to the second frame buffer. The ncp_spi thinks that the second frame has been transmitted in the END event. Then the second frame is lost. This CL processes the END event first then ACQUIRED event. To avoid the cached frame buffer variables are passed to the ncp_spi, the SPI driver gets the frame buffers directly from the SPI buffer registers. --- .../nrf528xx/src/transport/spi-slave.c | 12 +++- .../nrfx/drivers/include/nrfx_spis.h | 10 +++- .../nrfx/drivers/src/nrfx_spis.c | 59 ++++++++++++------- .../NordicSemiconductor/nrfx/hal/nrf_spis.h | 46 +++++++++++++++ 4 files changed, 101 insertions(+), 26 deletions(-) diff --git a/examples/platforms/nrf528xx/src/transport/spi-slave.c b/examples/platforms/nrf528xx/src/transport/spi-slave.c index 4e1a4636b..fe4adc997 100644 --- a/examples/platforms/nrf528xx/src/transport/spi-slave.c +++ b/examples/platforms/nrf528xx/src/transport/spi-slave.c @@ -78,7 +78,8 @@ static void spisEventHandler(nrfx_spis_evt_t const *aEvent, void *aContext) nrf_gpio_pin_set(SPIS_PIN_HOST_IRQ); // Execute application callback. - if (sCompleteCallback(sContext, sOutputBuf, sOutputBufLen, sInputBuf, sInputBufLen, aEvent->rx_amount)) + if (sCompleteCallback(sContext, aEvent->tx_buffer, aEvent->tx_buffer_size, aEvent->rx_buffer, + aEvent->rx_buffer_size, aEvent->rx_amount)) { // Further processing is required. sFurtherProcessingFlag = true; @@ -196,7 +197,14 @@ otError otPlatSpiSlavePrepareTransaction(uint8_t *aOutputBuf, sRequestTransactionFlag = aRequestTransactionFlag; error = nrfx_spis_buffers_set(&sSpiSlaveInstance, sOutputBuf, sOutputBufLen, sInputBuf, sInputBufLen); - assert(error == NRFX_SUCCESS); + if (error == NRFX_ERROR_INVALID_STATE) + { + result = OT_ERROR_BUSY; + } + else + { + assert(error == NRFX_SUCCESS); + } exit: return result; diff --git a/third_party/NordicSemiconductor/nrfx/drivers/include/nrfx_spis.h b/third_party/NordicSemiconductor/nrfx/drivers/include/nrfx_spis.h index aa68ce42d..4d67419e6 100644 --- a/third_party/NordicSemiconductor/nrfx/drivers/include/nrfx_spis.h +++ b/third_party/NordicSemiconductor/nrfx/drivers/include/nrfx_spis.h @@ -102,9 +102,13 @@ typedef enum /** @brief SPI slave driver event structure. */ typedef struct { - nrfx_spis_evt_type_t evt_type; //!< Type of the event. - size_t rx_amount; //!< Number of bytes received in the last transaction. This parameter is only valid for @ref NRFX_SPIS_XFER_DONE events. - size_t tx_amount; //!< Number of bytes transmitted in the last transaction. This parameter is only valid for @ref NRFX_SPIS_XFER_DONE events. + nrfx_spis_evt_type_t evt_type; //!< Type of the event. + uint8_t * tx_buffer; //!< SPI slave TX buffer. + uint8_t * rx_buffer; //!< SPI slave RX buffer. + size_t tx_buffer_size; //!< SPI slave TX buffer size in bytes. + size_t rx_buffer_size; //!< SPI slave RX buffer size in bytes. + size_t rx_amount; //!< Number of bytes received in the last transaction. This parameter is only valid for @ref NRFX_SPIS_XFER_DONE events. + size_t tx_amount; //!< Number of bytes transmitted in the last transaction. This parameter is only valid for @ref NRFX_SPIS_XFER_DONE events. } nrfx_spis_evt_t; /** @brief The default configuration of the SPI slave instance. */ diff --git a/third_party/NordicSemiconductor/nrfx/drivers/src/nrfx_spis.c b/third_party/NordicSemiconductor/nrfx/drivers/src/nrfx_spis.c index 88963cee2..7e6eb3ec5 100644 --- a/third_party/NordicSemiconductor/nrfx/drivers/src/nrfx_spis.c +++ b/third_party/NordicSemiconductor/nrfx/drivers/src/nrfx_spis.c @@ -335,12 +335,14 @@ static void spis_state_entry_action_execute(NRF_SPIS_Type * p_spis, case SPIS_XFER_COMPLETED: event.evt_type = NRFX_SPIS_XFER_DONE; + event.tx_buffer = nrf_spis_tx_buffer_get(p_spis, &event.tx_buffer_size); + event.rx_buffer = nrf_spis_rx_buffer_get(p_spis, &event.rx_buffer_size); event.rx_amount = nrf_spis_rx_amount_get(p_spis); event.tx_amount = nrf_spis_tx_amount_get(p_spis); NRFX_LOG_INFO("Transfer rx_len:%d.", event.rx_amount); NRFX_LOG_DEBUG("Rx data:"); - NRFX_LOG_HEXDUMP_DEBUG((uint8_t const *)p_cb->rx_buffer, - event.rx_amount * sizeof(p_cb->rx_buffer[0])); + NRFX_LOG_HEXDUMP_DEBUG((uint8_t const *)event.rx_buffer, + event.rx_amount * sizeof(event.rx_buffer[0])); NRFX_ASSERT(p_cb->handler != NULL); p_cb->handler(&event, p_cb->p_context); break; @@ -428,8 +430,41 @@ static void spis_irq_handler(NRF_SPIS_Type * p_spis, spis_cb_t * p_cb) { // @note: as multiple events can be pending for processing, the correct event processing order // is as follows: - // - SPI semaphore acquired event. // - SPI transaction complete event. + // - SPI semaphore acquired event. + + // Check for SPI transaction complete event. + if (nrf_spis_event_check(p_spis, NRF_SPIS_EVENT_END)) + { + nrfx_spis_evt_t event; + nrf_spis_event_clear(p_spis, NRF_SPIS_EVENT_END); + NRFX_LOG_DEBUG("SPIS: Event: %s.", EVT_TO_STR(NRF_SPIS_EVENT_END)); + + switch (p_cb->spi_state) + { + case SPIS_BUFFER_RESOURCE_CONFIGURED: + spis_state_change(p_spis, p_cb, SPIS_XFER_COMPLETED); + break; + + case SPIS_BUFFER_RESOURCE_REQUESTED: + event.evt_type = NRFX_SPIS_XFER_DONE; + event.tx_buffer = nrf_spis_tx_buffer_get(p_spis, &event.tx_buffer_size); + event.rx_buffer = nrf_spis_rx_buffer_get(p_spis, &event.rx_buffer_size); + event.rx_amount = nrf_spis_rx_amount_get(p_spis); + event.tx_amount = nrf_spis_tx_amount_get(p_spis); + NRFX_LOG_INFO("Transfer rx_len:%d.", event.rx_amount); + NRFX_LOG_DEBUG("Rx data:"); + NRFX_LOG_HEXDUMP_DEBUG((uint8_t const *)event.rx_buffer, + event.rx_amount * sizeof(event.rx_buffer[0])); + NRFX_ASSERT(p_cb->handler != NULL); + p_cb->handler(&event, p_cb->p_context); + break; + + default: + // No implementation required. + break; + } + } // Check for SPI semaphore acquired event. if (nrf_spis_event_check(p_spis, NRF_SPIS_EVENT_ACQUIRED)) @@ -453,24 +488,6 @@ static void spis_irq_handler(NRF_SPIS_Type * p_spis, spis_cb_t * p_cb) break; } } - - // Check for SPI transaction complete event. - if (nrf_spis_event_check(p_spis, NRF_SPIS_EVENT_END)) - { - nrf_spis_event_clear(p_spis, NRF_SPIS_EVENT_END); - NRFX_LOG_DEBUG("SPIS: Event: %s.", EVT_TO_STR(NRF_SPIS_EVENT_END)); - - switch (p_cb->spi_state) - { - case SPIS_BUFFER_RESOURCE_CONFIGURED: - spis_state_change(p_spis, p_cb, SPIS_XFER_COMPLETED); - break; - - default: - // No implementation required. - break; - } - } } #if NRFX_CHECK(NRFX_SPIS0_ENABLED) diff --git a/third_party/NordicSemiconductor/nrfx/hal/nrf_spis.h b/third_party/NordicSemiconductor/nrfx/hal/nrf_spis.h index c9578a6f6..279388e72 100644 --- a/third_party/NordicSemiconductor/nrfx/hal/nrf_spis.h +++ b/third_party/NordicSemiconductor/nrfx/hal/nrf_spis.h @@ -331,6 +331,28 @@ __STATIC_INLINE void nrf_spis_rx_buffer_set(NRF_SPIS_Type * p_reg, uint8_t * p_buffer, size_t length); +/** + * @brief Function for getting the transmit buffer. + * + * @param[in] p_reg Pointer to the structure of registers of the peripheral. + * @param[out] p_length Pointer to the maximum number of data bytes in transmit buffer. + * + * @returns The transmit buffer pointer. + */ +__STATIC_INLINE uint8_t * nrf_spis_tx_buffer_get(NRF_SPIS_Type * p_reg, + size_t * p_length); + +/** + * @brief Function for getting the receive buffer. + * + * @param[in] p_reg Pointer to the structure of registers of the peripheral. + * @param[out] p_length Pointer to the maximum number of data bytes in receive buffer. + * + * @returns The receive buffer pointer. + */ +__STATIC_INLINE uint8_t * nrf_spis_rx_buffer_get(NRF_SPIS_Type * p_reg, + size_t * p_length); + /** * @brief Function for getting the number of bytes transmitted * in the last granted transaction. @@ -545,6 +567,30 @@ __STATIC_INLINE void nrf_spis_rx_buffer_set(NRF_SPIS_Type * p_reg, #endif } +__STATIC_INLINE uint8_t * nrf_spis_tx_buffer_get(NRF_SPIS_Type * p_reg, + size_t * p_length) +{ +#if defined (NRF51) + *p_length = p_reg->MAXTX; + return (uint8_t *)p_reg->TXDPTR; +#else + *p_length = p_reg->TXD.MAXCNT; + return (uint8_t *)p_reg->TXD.PTR; +#endif +} + +__STATIC_INLINE uint8_t * nrf_spis_rx_buffer_get(NRF_SPIS_Type * p_reg, + size_t * p_length) +{ +#if defined (NRF51) + *p_length = p_reg->MAXRX; + return (uint8_t *)p_reg->RXDPTR; +#else + *p_length = p_reg->RXD.MAXCNT; + return (uint8_t *)p_reg->RXD.PTR; +#endif +} + __STATIC_INLINE size_t nrf_spis_tx_amount_get(NRF_SPIS_Type const * p_reg) { #if defined (NRF51)