From 0bd08fd3d4f06fc8587b85b064011ddb1d0059de Mon Sep 17 00:00:00 2001 From: Jukka Laitinen Date: Thu, 26 Sep 2024 13:19:04 +0300 Subject: [PATCH 1/2] imx9/px4io: Fix crashing for eDMA DEBUGASSERT - This occurs due to nxsem_tickwait returning also other error values than -ETIMEDOUT. Fix the issue by treating any error value as a timeout. - There is no need to clear the uart errors in _stop_dma, errors are cleared in uart irq Signed-off-by: Jukka Laitinen --- .../src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/platforms/nuttx/src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp b/platforms/nuttx/src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp index 5d3403bef045..57d814e17342 100644 --- a/platforms/nuttx/src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp +++ b/platforms/nuttx/src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp @@ -261,14 +261,14 @@ ArchPX4IOSerial::_bus_exchange(IOPacket *_packet) ret = nxsem_tickwait_uninterruptible(&_recv_sem, MSEC2TICK(10)); - if (ret == -ETIMEDOUT) { + if (ret != OK) { _stop_dma(); + nxsem_reset(&_recv_sem, 0); perf_count(_pc_timeouts); perf_cancel(_pc_txns); /* don't count this as a transaction */ - } - if (ret == OK) { + } else { /* Check packet CRC */ uint8_t crc = _current_packet->crc; @@ -361,8 +361,6 @@ ArchPX4IOSerial::_do_interrupt() void ArchPX4IOSerial::_stop_dma() { - /* Mark that we don't care about any DMA data any more */ - _waiting_for_dma = false; /* Stop the DMA channels */ @@ -375,8 +373,4 @@ ArchPX4IOSerial::_stop_dma() while (getreg32(PX4IO_SERIAL_BASE + IMX9_LPUART_STAT_OFFSET) & LPUART_STAT_RDRF) { getreg32(PX4IO_SERIAL_BASE + IMX9_LPUART_DATA_OFFSET); } - - /* Clear any error status flags */ - - modreg32(ERR_FLAGS_MASK, ERR_FLAGS_MASK, PX4IO_SERIAL_BASE + IMX9_LPUART_STAT_OFFSET); } From b4052688dac9f115073348cd8e2bed78c0df7684 Mon Sep 17 00:00:00 2001 From: Jukka Laitinen Date: Mon, 30 Sep 2024 12:48:19 +0300 Subject: [PATCH 2/2] imx9/px4io: Add error checking for all the DMA operations Check the return values from DMA operations and add a performance counter to detect if there are any DMA related errors. Signed-off-by: Jukka Laitinen --- .../nxp/imx9/include/px4_arch/px4io_serial.h | 4 ++ .../nxp/imx9/px4io_serial/px4io_serial.cpp | 41 ++++++++++++++----- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/platforms/nuttx/src/px4/nxp/imx9/include/px4_arch/px4io_serial.h b/platforms/nuttx/src/px4/nxp/imx9/include/px4_arch/px4io_serial.h index 119125b3689c..3938ac7279e3 100644 --- a/platforms/nuttx/src/px4/nxp/imx9/include/px4_arch/px4io_serial.h +++ b/platforms/nuttx/src/px4/nxp/imx9/include/px4_arch/px4io_serial.h @@ -134,6 +134,10 @@ class ArchPX4IOSerial : public PX4IO_serial px4_sem_t _recv_sem; + /** dma error perf counter */ + + perf_counter_t _pc_dmaerrs; + /** * DMA completion handler. */ diff --git a/platforms/nuttx/src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp b/platforms/nuttx/src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp index 57d814e17342..791a99d2cdd8 100644 --- a/platforms/nuttx/src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp +++ b/platforms/nuttx/src/px4/nxp/imx9/px4io_serial/px4io_serial.cpp @@ -38,6 +38,7 @@ */ #include +#include #include @@ -63,7 +64,8 @@ static const struct uart_config_s g_px4io_config = { ArchPX4IOSerial::ArchPX4IOSerial() : _current_packet(nullptr), - _recv_sem(SEM_INITIALIZER(0)) + _recv_sem(SEM_INITIALIZER(0)), + _pc_dmaerrs(perf_alloc(PC_COUNT, "px4io: dmaerrs")) { px4_sem_setprotocol(&_recv_sem, SEM_PRIO_NONE); @@ -88,6 +90,8 @@ ArchPX4IOSerial::~ArchPX4IOSerial() imx9_dmach_free(_rx_dma); imx9_dmach_free(_tx_dma); + + perf_free(_pc_dmaerrs); } int @@ -161,6 +165,7 @@ ArchPX4IOSerial::ioctl(unsigned operation, unsigned &arg) perf_print_counter(_pc_txns); perf_print_counter(_pc_retries); perf_print_counter(_pc_timeouts); + perf_print_counter(_pc_dmaerrs); perf_print_counter(_pc_crcerrs); perf_print_counter(_pc_protoerrs); perf_print_counter(_pc_uerrs); @@ -216,9 +221,7 @@ ArchPX4IOSerial::_bus_exchange(IOPacket *_packet) (uintptr_t)_current_packet + DMA_ALIGN_UP(sizeof(IOPacket))); #endif - /* Start the RX DMA */ - - _waiting_for_dma = true; + /* Configure the RX and TX DMA */ struct imx9_edma_xfrconfig_s rx_config; rx_config.saddr = PX4IO_SERIAL_BASE + IMX9_LPUART_DATA_OFFSET; @@ -234,11 +237,6 @@ ArchPX4IOSerial::_bus_exchange(IOPacket *_packet) rx_config.linkch = NULL; #endif - imx9_dmach_xfrsetup(_rx_dma, &rx_config); - imx9_dmach_start(_rx_dma, _dma_callback, (void *)this); - - /* Start the TX DMA */ - struct imx9_edma_xfrconfig_s tx_config; tx_config.saddr = reinterpret_cast(_current_packet); @@ -254,8 +252,29 @@ ArchPX4IOSerial::_bus_exchange(IOPacket *_packet) tx_config.linkch = NULL; #endif - imx9_dmach_xfrsetup(_tx_dma, &tx_config); - imx9_dmach_start(_tx_dma, nullptr, nullptr); + /* If setup fails, just stop and exit */ + + if (imx9_dmach_xfrsetup(_rx_dma, &rx_config) != OK || + imx9_dmach_xfrsetup(_tx_dma, &tx_config) != OK) { + _stop_dma(); + perf_count(_pc_dmaerrs); + perf_cancel(_pc_txns); + return -EIO; + } + + /* Start DMA. imx9_dmach_start can only return OK at the time of writing this, + * but keep the error checking to look proper + */ + + _waiting_for_dma = true; + + if (imx9_dmach_start(_rx_dma, _dma_callback, (void *)this) != OK || + imx9_dmach_start(_tx_dma, nullptr, nullptr) != OK) { + _stop_dma(); + perf_count(_pc_dmaerrs); + perf_cancel(_pc_txns); + return -EIO; + } /* Wait for response, max 10 ms */