Merge branch 'bugfix/twai_esp32_errata_workarounds' into 'master'

TWAI: Add FIFO overrun handling and ESP32 hardware errata workarounds

Closes IDFGH-3307, IDFGH-393, and IDFGH-2114

See merge request espressif/esp-idf!10041

components/app_trace/linker.lf

@@ -9,7 +9,7 @@ entries:
       SEGGER_SYSVIEW_Config_FreeRTOS (noflash)
       SEGGER_SYSVIEW_FreeRTOS (noflash)
 
-[mapping:driver]
+[mapping:app_trace_driver]
 archive: libdriver.a
 entries:
     if SYSVIEW_TS_SOURCE_TIMER_00 = y || SYSVIEW_TS_SOURCE_TIMER_01 = y

components/driver/CMakeLists.txt

@@ -76,7 +76,8 @@ idf_component_register(SRCS "${srcs}"
                     INCLUDE_DIRS ${includes}
                     PRIV_INCLUDE_DIRS "include/driver"
                     PRIV_REQUIRES efuse esp_timer esp_ipc
-                    REQUIRES esp_pm esp_ringbuf freertos soc hal esp_hw_support)
+                    REQUIRES esp_pm esp_ringbuf freertos soc hal esp_hw_support
+                    LDFRAGMENTS linker.lf)
 # (REQUIRES cannot hide soc headers, since many arguments in the driver headers are chip-dependent)
 
 # uses C11 atomic feature

components/driver/Kconfig

@@ -87,6 +87,48 @@ menu "Driver configurations"
                 - Alert logging (i.e., setting of the TWAI_ALERT_AND_LOG flag)
                 will have no effect.
 
+        config TWAI_ERRATA_FIX_BUS_OFF_REC
+            bool "Add SW workaround for REC change during bus-off"
+            depends on IDF_TARGET_ESP32
+            default n
+            help
+                When the bus-off condition is reached, the REC should be reset to 0 and frozen (via LOM) by the
+                driver's ISR. However on the ESP32, there is an edge case where the REC will increase before the
+                driver's ISR can respond in time (e.g., due to the rapid occurrence of bus errors), thus causing the
+                REC to be non-zero after bus-off. A non-zero REC can prevent bus-off recovery as the bus-off recovery
+                condition is that both TEC and REC become 0. Enabling this option will add a workaround in the driver
+                to forcibly reset REC to zero on reaching bus-off.
+
+        config TWAI_ERRATA_FIX_TX_INTR_LOST
+            bool "Add SW workaround for TX interrupt lost errata"
+            depends on IDF_TARGET_ESP32
+            default n
+            help
+                On the ESP32, when a transmit interrupt occurs, and interrupt register is read on the same APB clock
+                cycle, the transmit interrupt could be lost. Enabling this option will add a workaround that checks the
+                transmit buffer status bit to recover any lost transmit interrupt.
+
+        config TWAI_ERRATA_FIX_RX_FRAME_INVALID
+            bool "Add SW workaround for invalid RX frame errata"
+            depends on IDF_TARGET_ESP32
+            default n
+            help
+                On the ESP32, when receiving a data or remote frame, if a bus error occurs in the data or CRC field,
+                the data of the next received frame could be invalid. Enabling this option will add a workaround that
+                will reset the peripheral on detection of this errata condition. Note that if a frame is transmitted on
+                the bus whilst the reset is ongoing, the message will not be receive by the peripheral sent on the bus
+                during the reset, the message will be lost.
+
+        config TWAI_ERRATA_FIX_RX_FIFO_CORRUPT
+            bool "Add SW workaround for RX FIFO corruption errata"
+            depends on IDF_TARGET_ESP32
+            default n
+            help
+                On the ESP32, when the RX FIFO overruns and the RX message counter maxes out at 64 messages, the entire
+                RX FIFO is no longer recoverable. Enabling this option will add a workaround that resets the peripheral
+                on detection of this errata condition. Note that if a frame is being sent on the bus during the reset
+                bus during the reset, the message will be lost.
+
     endmenu # TWAI Configuration
 
     menu "UART configuration"

components/driver/include/driver/twai.h

@@ -53,22 +53,25 @@ extern "C" {
  * @note    The TWAI_ALERT_AND_LOG flag is not an actual alert, but will configure
  *          the TWAI driver to log to UART when an enabled alert occurs.
  */
-#define TWAI_ALERT_TX_IDLE                  0x0001  /**< Alert(1): No more messages to transmit */
-#define TWAI_ALERT_TX_SUCCESS               0x0002  /**< Alert(2): The previous transmission was successful */
-#define TWAI_ALERT_BELOW_ERR_WARN           0x0004  /**< Alert(4): Both error counters have dropped below error warning limit */
-#define TWAI_ALERT_ERR_ACTIVE               0x0008  /**< Alert(8): TWAI controller has become error active */
-#define TWAI_ALERT_RECOVERY_IN_PROGRESS     0x0010  /**< Alert(16): TWAI controller is undergoing bus recovery */
-#define TWAI_ALERT_BUS_RECOVERED            0x0020  /**< Alert(32): TWAI controller has successfully completed bus recovery */
-#define TWAI_ALERT_ARB_LOST                 0x0040  /**< Alert(64): The previous transmission lost arbitration */
-#define TWAI_ALERT_ABOVE_ERR_WARN           0x0080  /**< Alert(128): One of the error counters have exceeded the error warning limit */
-#define TWAI_ALERT_BUS_ERROR                0x0100  /**< Alert(256): A (Bit, Stuff, CRC, Form, ACK) error has occurred on the bus */
-#define TWAI_ALERT_TX_FAILED                0x0200  /**< Alert(512): The previous transmission has failed (for single shot transmission) */
-#define TWAI_ALERT_RX_QUEUE_FULL            0x0400  /**< Alert(1024): The RX queue is full causing a frame to be lost */
-#define TWAI_ALERT_ERR_PASS                 0x0800  /**< Alert(2048): TWAI controller has become error passive */
-#define TWAI_ALERT_BUS_OFF                  0x1000  /**< Alert(4096): Bus-off condition occurred. TWAI controller can no longer influence bus */
-#define TWAI_ALERT_ALL                      0x1FFF  /**< Bit mask to enable all alerts during configuration */
-#define TWAI_ALERT_NONE                     0x0000  /**< Bit mask to disable all alerts during configuration */
-#define TWAI_ALERT_AND_LOG                  0x2000  /**< Bit mask to enable alerts to also be logged when they occur. Note that logging from the ISR is disabled if CONFIG_TWAI_ISR_IN_IRAM is enabled (see docs). */
+#define TWAI_ALERT_TX_IDLE                  0x00000001  /**< Alert(1): No more messages to transmit */
+#define TWAI_ALERT_TX_SUCCESS               0x00000002  /**< Alert(2): The previous transmission was successful */
+#define TWAI_ALERT_BELOW_ERR_WARN           0x00000004  /**< Alert(4): Both error counters have dropped below error warning limit */
+#define TWAI_ALERT_ERR_ACTIVE               0x00000008  /**< Alert(8): TWAI controller has become error active */
+#define TWAI_ALERT_RECOVERY_IN_PROGRESS     0x00000010  /**< Alert(16): TWAI controller is undergoing bus recovery */
+#define TWAI_ALERT_BUS_RECOVERED            0x00000020  /**< Alert(32): TWAI controller has successfully completed bus recovery */
+#define TWAI_ALERT_ARB_LOST                 0x00000040  /**< Alert(64): The previous transmission lost arbitration */
+#define TWAI_ALERT_ABOVE_ERR_WARN           0x00000080  /**< Alert(128): One of the error counters have exceeded the error warning limit */
+#define TWAI_ALERT_BUS_ERROR                0x00000100  /**< Alert(256): A (Bit, Stuff, CRC, Form, ACK) error has occurred on the bus */
+#define TWAI_ALERT_TX_FAILED                0x00000200  /**< Alert(512): The previous transmission has failed (for single shot transmission) */
+#define TWAI_ALERT_RX_QUEUE_FULL            0x00000400  /**< Alert(1024): The RX queue is full causing a frame to be lost */
+#define TWAI_ALERT_ERR_PASS                 0x00000800  /**< Alert(2048): TWAI controller has become error passive */
+#define TWAI_ALERT_BUS_OFF                  0x00001000  /**< Alert(4096): Bus-off condition occurred. TWAI controller can no longer influence bus */
+#define TWAI_ALERT_RX_FIFO_OVERRUN          0x00002000  /**< Alert(8192): An RX FIFO overrun has occurred */
+#define TWAI_ALERT_TX_RETRIED               0x00004000  /**< Alert(16384): An message transmission was cancelled and retried due to an errata workaround */
+#define TWAI_ALERT_PERIPH_RESET             0x00008000  /**< Alert(32768): The TWAI controller was reset */
+#define TWAI_ALERT_ALL                      0x0000FFFF  /**< Bit mask to enable all alerts during configuration */
+#define TWAI_ALERT_NONE                     0x00000000  /**< Bit mask to disable all alerts during configuration */
+#define TWAI_ALERT_AND_LOG                  0x00010000  /**< Bit mask to enable alerts to also be logged when they occur. Note that logging from the ISR is disabled if CONFIG_TWAI_ISR_IN_IRAM is enabled (see docs). */
 
 /** @endcond */
 
@@ -114,7 +117,8 @@ typedef struct {
     uint32_t tx_error_counter;      /**< Current value of Transmit Error Counter */
     uint32_t rx_error_counter;      /**< Current value of Receive Error Counter */
     uint32_t tx_failed_count;       /**< Number of messages that failed transmissions */
-    uint32_t rx_missed_count;       /**< Number of messages that were lost due to a full RX queue */
+    uint32_t rx_missed_count;       /**< Number of messages that were lost due to a full RX queue (or errata workaround if enabled) */
+    uint32_t rx_overrun_count;      /**< Number of messages that were lost due to a RX FIFO overrun */
     uint32_t arb_lost_count;        /**< Number of instances arbitration was lost */
     uint32_t bus_error_count;       /**< Number of instances a bus error has occurred */
 } twai_status_info_t;

components/driver/linker.lf

@@ -0,0 +1,9 @@
+
+[mapping:driver]
+archive: libdriver.a
+entries:
+    # TWAI workarounds that require periph_module_reset() won't work if cache is disabled due to the use of switch jump
+    # tables in periph_module_reset(). We prevent any part of periph_module_reset() (either text or RO data) from being
+    # placed in flash.
+    if TWAI_ISR_IN_IRAM = y  && (TWAI_ERRATA_FIX_RX_FRAME_INVALID = y || TWAI_ERRATA_FIX_RX_FIFO_CORRUPT = y):
+        periph_ctrl: periph_module_reset (noflash)

components/driver/periph_ctrl.c

@@ -13,6 +13,7 @@
 // limitations under the License.
 #include "freertos/FreeRTOS.h"
 #include "hal/clk_gate_ll.h"
+#include "esp_attr.h"
 #include "driver/periph_ctrl.h"
 
 static portMUX_TYPE periph_spinlock = portMUX_INITIALIZER_UNLOCKED;

components/driver/twai.c

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "soc/soc_caps.h"
 
 #include "sdkconfig.h"
 #include "freertos/FreeRTOS.h"
@@ -28,6 +27,7 @@
 #include "driver/gpio.h"
 #include "driver/periph_ctrl.h"
 #include "driver/twai.h"
+#include "soc/soc_caps.h"
 #include "soc/twai_periph.h"
 #include "hal/twai_hal.h"
 #include "esp_rom_gpio.h"
@@ -69,6 +69,7 @@ typedef struct {
     twai_state_t state;
     twai_mode_t mode;
     uint32_t rx_missed_count;
+    uint32_t rx_overrun_count;
     uint32_t tx_failed_count;
     uint32_t arb_lost_count;
     uint32_t bus_error_count;
@@ -128,19 +129,49 @@ TWAI_ISR_ATTR static void twai_alert_handler(uint32_t alert_code, int *alert_req
 
 static inline void twai_handle_rx_buffer_frames(BaseType_t *task_woken, int *alert_req)
 {
+#ifdef SOC_TWAI_SUPPORTS_RX_STATUS
     uint32_t msg_count = twai_hal_get_rx_msg_count(&twai_context);
 
     for (uint32_t i = 0; i < msg_count; i++) {
         twai_hal_frame_t frame;
-        twai_hal_read_rx_buffer_and_clear(&twai_context, &frame);
-        //Copy frame into RX Queue
-        if (xQueueSendFromISR(p_twai_obj->rx_queue, &frame, task_woken) == pdTRUE) {
-            p_twai_obj->rx_msg_count++;
+        if (twai_hal_read_rx_buffer_and_clear(&twai_context, &frame)) {
+            //Valid frame copied from RX buffer
+            if (xQueueSendFromISR(p_twai_obj->rx_queue, &frame, task_woken) == pdTRUE) {
+                p_twai_obj->rx_msg_count++;
+            } else {    //Failed to send to queue
+                p_twai_obj->rx_missed_count++;
+                twai_alert_handler(TWAI_ALERT_RX_QUEUE_FULL, alert_req);
+            }
+        } else {    //Failed to read from RX buffer because message is overrun
+            p_twai_obj->rx_overrun_count++;
+            twai_alert_handler(TWAI_ALERT_RX_FIFO_OVERRUN, alert_req);
+        }
+    }
+#else   //SOC_TWAI_SUPPORTS_RX_STATUS
+    uint32_t msg_count = twai_hal_get_rx_msg_count(&twai_context);
+    bool overrun = false;
+    //Clear all valid RX frames
+    for (int i = 0; i < msg_count; i++) {
+        twai_hal_frame_t frame;
+        if (twai_hal_read_rx_buffer_and_clear(&twai_context, &frame)) {
+            //Valid frame copied from RX buffer
+            if (xQueueSendFromISR(p_twai_obj->rx_queue, &frame, task_woken) == pdTRUE) {
+                p_twai_obj->rx_msg_count++;
+            } else {
+                p_twai_obj->rx_missed_count++;
+                twai_alert_handler(TWAI_ALERT_RX_QUEUE_FULL, alert_req);
+            }
         } else {
-            p_twai_obj->rx_missed_count++;
-            twai_alert_handler(TWAI_ALERT_RX_QUEUE_FULL, alert_req);
+            overrun = true;
+            break;
         }
     }
+    //All remaining frames are treated as overrun. Clear them all
+    if (overrun) {
+        p_twai_obj->rx_overrun_count += twai_hal_clear_rx_fifo_overrun(&twai_context);
+        twai_alert_handler(TWAI_ALERT_RX_FIFO_OVERRUN, alert_req);
+    }
+#endif  //SOC_TWAI_SUPPORTS_RX_STATUS
 }
 
 static inline void twai_handle_tx_buffer_frame(BaseType_t *task_woken, int *alert_req)
@@ -176,56 +207,65 @@ TWAI_ISR_ATTR static void twai_intr_handler_main(void *arg)
 {
     BaseType_t task_woken = pdFALSE;
     int alert_req = 0;
-    uint32_t event;
+    uint32_t events;
     TWAI_ENTER_CRITICAL_ISR();
-    if (p_twai_obj == NULL) {    //Incase intr occurs whilst driver is being uninstalled
+    if (p_twai_obj == NULL) {    //In case intr occurs whilst driver is being uninstalled
         TWAI_EXIT_CRITICAL_ISR();
         return;
     }
-    event = twai_hal_decode_interrupt_events(&twai_context);
-    if (event & TWAI_HAL_EVENT_RX_BUFF_FRAME) {
+    events = twai_hal_get_events(&twai_context);    //Get the events that triggered the interrupt
+
+#if defined(CONFIG_TWAI_ERRATA_FIX_RX_FRAME_INVALID) || defined(CONFIG_TWAI_ERRATA_FIX_RX_FIFO_CORRUPT)
+    if (events & TWAI_HAL_EVENT_NEED_PERIPH_RESET) {
+        twai_hal_prepare_for_reset(&twai_context);
+        periph_module_reset(PERIPH_TWAI_MODULE);
+        twai_hal_recover_from_reset(&twai_context);
+        p_twai_obj->rx_missed_count += twai_hal_get_reset_lost_rx_cnt(&twai_context);
+        twai_alert_handler(TWAI_ALERT_PERIPH_RESET, &alert_req);
+    }
+#endif
+    if (events & TWAI_HAL_EVENT_RX_BUFF_FRAME) {
+        //Note: This event will never occur if there is a periph reset event
         twai_handle_rx_buffer_frames(&task_woken, &alert_req);
     }
-    //TX command should be the last command related handler to be called, so that
-    //other command register bits do not overwrite the TX command bit.
-    if (event & TWAI_HAL_EVENT_TX_BUFF_FREE) {
+    if (events & TWAI_HAL_EVENT_TX_BUFF_FREE) {
         twai_handle_tx_buffer_frame(&task_woken, &alert_req);
     }
 
     //Handle events that only require alerting (i.e. no handler)
-    if (event & TWAI_HAL_EVENT_BUS_OFF) {
+    if (events & TWAI_HAL_EVENT_BUS_OFF) {
         p_twai_obj->state = TWAI_STATE_BUS_OFF;
         twai_alert_handler(TWAI_ALERT_BUS_OFF, &alert_req);
     }
-    if (event & TWAI_HAL_EVENT_BUS_RECOV_CPLT) {
+    if (events & TWAI_HAL_EVENT_BUS_RECOV_CPLT) {
         p_twai_obj->state = TWAI_STATE_STOPPED;
         twai_alert_handler(TWAI_ALERT_BUS_RECOVERED, &alert_req);
     }
-    if (event & TWAI_HAL_EVENT_BUS_ERR) {
+    if (events & TWAI_HAL_EVENT_BUS_ERR) {
         p_twai_obj->bus_error_count++;
         twai_alert_handler(TWAI_ALERT_BUS_ERROR, &alert_req);
     }
-    if (event & TWAI_HAL_EVENT_ARB_LOST) {
+    if (events & TWAI_HAL_EVENT_ARB_LOST) {
         p_twai_obj->arb_lost_count++;
         twai_alert_handler(TWAI_ALERT_ARB_LOST, &alert_req);
     }
-    if (event & TWAI_HAL_EVENT_BUS_RECOV_PROGRESS) {
+    if (events & TWAI_HAL_EVENT_BUS_RECOV_PROGRESS) {
         //Bus-recovery in progress. TEC has dropped below error warning limit
         twai_alert_handler(TWAI_ALERT_RECOVERY_IN_PROGRESS, &alert_req);
     }
-    if (event & TWAI_HAL_EVENT_ERROR_PASSIVE) {
+    if (events & TWAI_HAL_EVENT_ERROR_PASSIVE) {
         //Entered error passive
         twai_alert_handler(TWAI_ALERT_ERR_PASS, &alert_req);
     }
-    if (event & TWAI_HAL_EVENT_ERROR_ACTIVE) {
+    if (events & TWAI_HAL_EVENT_ERROR_ACTIVE) {
         //Returned to error active
         twai_alert_handler(TWAI_ALERT_ERR_ACTIVE, &alert_req);
     }
-    if (event & TWAI_HAL_EVENT_ABOVE_EWL) {
+    if (events & TWAI_HAL_EVENT_ABOVE_EWL) {
         //TEC or REC surpassed error warning limit
         twai_alert_handler(TWAI_ALERT_ABOVE_ERR_WARN, &alert_req);
     }
-    if (event & TWAI_HAL_EVENT_BELOW_EWL) {
+    if (events & TWAI_HAL_EVENT_BELOW_EWL) {
         //TEC and REC are both below error warning
         twai_alert_handler(TWAI_ALERT_BELOW_ERR_WARN, &alert_req);
     }
@@ -646,6 +686,7 @@ esp_err_t twai_get_status_info(twai_status_info_t *status_info)
     status_info->msgs_to_rx = p_twai_obj->rx_msg_count;
     status_info->tx_failed_count = p_twai_obj->tx_failed_count;
     status_info->rx_missed_count = p_twai_obj->rx_missed_count;
+    status_info->rx_overrun_count = p_twai_obj->rx_overrun_count;
     status_info->arb_lost_count = p_twai_obj->arb_lost_count;
     status_info->bus_error_count = p_twai_obj->bus_error_count;
     status_info->state = p_twai_obj->state;

components/hal/esp32/include/hal/can_hal.h

@@ -109,7 +109,7 @@ static inline bool can_hal_check_state_flags(can_hal_context_t *hal_ctx, uint32_
 /* ----------------------------- Event Handling ----------------------------- */
 
 static inline uint32_t can_hal_decode_interrupt_events(can_hal_context_t *hal_ctx) {
-    return twai_hal_decode_interrupt_events(hal_ctx);
+    return twai_hal_decode_interrupt(hal_ctx);
 }
 
 /* ------------------------------- TX and RX -------------------------------- */