stm32/fdcan: Fix FIFO1 usage and handling of error interrupts.

The original code used a independent state with regards to the interrupt.
During heavy bus error conditions the internal state could become
out-of-sync with the interrupts.

Further explanation: during the development of an application using CAN
communication, a interrupt-run-away was found in some situations.  It was
found that the error interrupt triggered (Warning, Passive or Bus-Off, all
triggered it) the run-away.  The only recovery was a reset.

Two problems were found:
- the error interrupt is enabled but not cleared in the interrupt routine;
- an internal variable 'State' that was used to track the message received
  state (empty, new, full, overflow) that was not directly related to
  interrupt that indicated the state.

In this commit these issues are fixed by adding more values for the
interrupt reason (warning, passive, bus off) and clearing the error
interrupts, and making the internal state directly dependent on the
interrupt state for received messages.

Furthermore, introducing the FIFO1 in the CAN receive stage, another issue
existed.  Even if the messages are received into the FIFO1 (by selecting
message filtering for FIFO0 and FIFO1), the interrupt firing was indicating
FIFO0 Rx.  The configuration of the interrupts for this is now also fixed.
The CAN peripheral has 2 interrupt lines going into the NVIC controller.
The assignment of the interrupt reasons to these 2 interrupt lines was
missing.  Now the reception of FIFO1 messages triggers the second interrupt
line.  Other interrupts (Rx FIFO0 and bus error) are assigned to the first
interrupt line.

Tested on a Nucleo-G474, and also checked the HAL function to work with the
H7 family.

ports/stm32/fdcan.c

@@ -44,6 +44,13 @@
 #define FDCAN_ELEMENT_MASK_FIDX  (0x7f000000) // Filter Index
 #define FDCAN_ELEMENT_MASK_ANMF  (0x80000000) // Accepted Non-matching Frame
 
+#define FDCAN_RX_FIFO0_MASK (FDCAN_FLAG_RX_FIFO0_MESSAGE_LOST | FDCAN_FLAG_RX_FIFO0_FULL | FDCAN_FLAG_RX_FIFO0_NEW_MESSAGE)
+#define FDCAN_RX_FIFO1_MASK (FDCAN_FLAG_RX_FIFO1_MESSAGE_LOST | FDCAN_FLAG_RX_FIFO1_FULL | FDCAN_FLAG_RX_FIFO1_NEW_MESSAGE)
+#define FDCAN_ERROR_STATUS_MASK (FDCAN_FLAG_ERROR_PASSIVE | FDCAN_FLAG_ERROR_WARNING | FDCAN_FLAG_BUS_OFF)
+
+// also defined in <PROC>_hal_fdcan.c, but not able to declare extern and reach the variable
+static const uint8_t DLCtoBytes[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 12, 16, 20, 24, 32, 48, 64};
+
 bool can_init(pyb_can_obj_t *can_obj, uint32_t mode, uint32_t prescaler, uint32_t sjw, uint32_t bs1, uint32_t bs2, bool auto_restart) {
     (void)auto_restart;
 
@@ -60,6 +67,16 @@ bool can_init(pyb_can_obj_t *can_obj, uint32_t mode, uint32_t prescaler, uint32_
     init->TransmitPause = DISABLE;
     init->ProtocolException = ENABLE;
 
+    #if defined(STM32G4)
+    init->ClockDivider = FDCAN_CLOCK_DIV1;
+    init->DataPrescaler = 1;
+    init->DataSyncJumpWidth = 1;
+    init->DataTimeSeg1 = 1;
+    init->DataTimeSeg2 = 1;
+    #endif
+
+    #if defined(STM32H7)
+    // variable used to specify RAM address in HAL, only for H7, G4 uses defined offset address in HAL
     // The Message RAM is shared between CAN1 and CAN2. Setting the offset to half
     // the Message RAM for the second CAN and using half the resources for each CAN.
     if (can_obj->can_id == PYB_CAN_1) {
@@ -67,6 +84,14 @@ bool can_init(pyb_can_obj_t *can_obj, uint32_t mode, uint32_t prescaler, uint32_
     } else {
         init->MessageRAMOffset = 2560 / 2;
     }
+    #endif
+
+    #if defined(STM32G4)
+
+    init->StdFiltersNbr = 28; // /2  ? if FDCAN2 is used !!?
+    init->ExtFiltersNbr = 0; // Not used
+
+    #elif defined(STM32H7)
 
     init->StdFiltersNbr = 64; // 128 / 2
     init->ExtFiltersNbr = 0; // Not used
@@ -83,6 +108,9 @@ bool can_init(pyb_can_obj_t *can_obj, uint32_t mode, uint32_t prescaler, uint32_
 
     init->TxFifoQueueElmtsNbr = 16; // Tx fifo elements
     init->TxElmtSize = FDCAN_DATA_BYTES_8;
+
+    #endif
+
     init->TxFifoQueueMode = FDCAN_TX_FIFO_OPERATION;
 
     FDCAN_GlobalTypeDef *CANx = NULL;
@@ -148,21 +176,27 @@ bool can_init(pyb_can_obj_t *can_obj, uint32_t mode, uint32_t prescaler, uint32_
             NVIC_SetPriority(FDCAN1_IT1_IRQn, IRQ_PRI_CAN);
             HAL_NVIC_EnableIRQ(FDCAN1_IT1_IRQn);
             break;
+        #if defined(MICROPY_HW_CAN2_TX)
         case PYB_CAN_2:
             NVIC_SetPriority(FDCAN2_IT0_IRQn, IRQ_PRI_CAN);
             HAL_NVIC_EnableIRQ(FDCAN2_IT0_IRQn);
             NVIC_SetPriority(FDCAN2_IT1_IRQn, IRQ_PRI_CAN);
             HAL_NVIC_EnableIRQ(FDCAN2_IT1_IRQn);
             break;
+        #endif
         default:
             return false;
     }
+    // FDCAN IT 0
+    HAL_FDCAN_ConfigInterruptLines(&can_obj->can, FDCAN_IT_GROUP_RX_FIFO0 | FDCAN_IT_GROUP_BIT_LINE_ERROR | FDCAN_IT_GROUP_PROTOCOL_ERROR, FDCAN_INTERRUPT_LINE0);
+    // FDCAN IT 1
+    HAL_FDCAN_ConfigInterruptLines(&can_obj->can, FDCAN_IT_GROUP_RX_FIFO1, FDCAN_INTERRUPT_LINE1);
 
-    __HAL_FDCAN_ENABLE_IT(&can_obj->can, FDCAN_IT_BUS_OFF | FDCAN_IT_ERROR_WARNING | FDCAN_IT_ERROR_PASSIVE);
+    uint32_t ActiveITs = FDCAN_IT_BUS_OFF | FDCAN_IT_ERROR_WARNING | FDCAN_IT_ERROR_PASSIVE;
-    __HAL_FDCAN_ENABLE_IT(&can_obj->can, FDCAN_IT_RX_FIFO0_NEW_MESSAGE | FDCAN_IT_RX_FIFO1_NEW_MESSAGE);
+    ActiveITs |= FDCAN_IT_RX_FIFO0_NEW_MESSAGE | FDCAN_IT_RX_FIFO1_NEW_MESSAGE;
-    __HAL_FDCAN_ENABLE_IT(&can_obj->can, FDCAN_IT_RX_FIFO0_MESSAGE_LOST | FDCAN_IT_RX_FIFO1_MESSAGE_LOST);
+    ActiveITs |= FDCAN_IT_RX_FIFO0_MESSAGE_LOST | FDCAN_IT_RX_FIFO1_MESSAGE_LOST;
-    __HAL_FDCAN_ENABLE_IT(&can_obj->can, FDCAN_IT_RX_FIFO0_FULL | FDCAN_IT_RX_FIFO1_FULL);
+    ActiveITs |= FDCAN_IT_RX_FIFO0_FULL | FDCAN_IT_RX_FIFO1_FULL;
-
+    HAL_FDCAN_ActivateNotification(&can_obj->can, ActiveITs, 0);
     return true;
 }
 
@@ -227,10 +261,19 @@ int can_receive(FDCAN_HandleTypeDef *can, int fifo, FDCAN_RxHeaderTypeDef *hdr,
     uint32_t index, *address;
     if (fifo == FDCAN_RX_FIFO0) {
         index = (*rxf & FDCAN_RXF0S_F0GI) >> FDCAN_RXF0S_F0GI_Pos;
+        #if defined(STM32G4)
+        address = (uint32_t *)(can->msgRam.RxFIFO0SA + (index * (18U * 4U)));  // SRAMCAN_RF0_SIZE bytes, size not configurable
+        #else
         address = (uint32_t *)(can->msgRam.RxFIFO0SA + (index * can->Init.RxFifo0ElmtSize * 4));
+        #endif
     } else {
         index = (*rxf & FDCAN_RXF1S_F1GI) >> FDCAN_RXF1S_F1GI_Pos;
+        #if defined(STM32G4)
+        // ToDo: test FIFO1, FIFO 0 is ok
+        address = (uint32_t *)(can->msgRam.RxFIFO1SA + (index * (18U * 4U)));  // SRAMCAN_RF1_SIZE bytes, size not configurable
+        #else
         address = (uint32_t *)(can->msgRam.RxFIFO1SA + (index * can->Init.RxFifo1ElmtSize * 4));
+        #endif
     }
 
     // Parse header of message
@@ -251,7 +294,7 @@ int can_receive(FDCAN_HandleTypeDef *can, int fifo, FDCAN_RxHeaderTypeDef *hdr,
 
     // Copy data
     uint8_t *pdata = (uint8_t *)address;
-    for (uint32_t i = 0; i < 8; ++i) { // TODO use DLCtoBytes[hdr->DataLength] for length > 8
+    for (uint32_t i = 0; i < DLCtoBytes[hdr->DataLength]; ++i) {
         *data++ = *pdata++;
     }
 
@@ -269,41 +312,97 @@ STATIC void can_rx_irq_handler(uint can_id, uint fifo_id) {
 
     self = MP_STATE_PORT(pyb_can_obj_all)[can_id - 1];
 
+    CAN_TypeDef *can = self->can.Instance;
+
+    uint32_t RxFifo0ITs;
+    uint32_t RxFifo1ITs;
+    // uint32_t Errors;
+    uint32_t ErrorStatusITs;
+    uint32_t Psr;
+
+    RxFifo0ITs = can->IR & FDCAN_RX_FIFO0_MASK;
+    RxFifo0ITs &= can->IE;
+    RxFifo1ITs = can->IR & FDCAN_RX_FIFO1_MASK;
+    RxFifo1ITs &= can->IE;
+    // Errors = (&self->can)->Instance->IR & FDCAN_ERROR_MASK;
+    // Errors &= (&self->can)->Instance->IE;
+    ErrorStatusITs = can->IR & FDCAN_ERROR_STATUS_MASK;
+    ErrorStatusITs &= can->IE;
+    Psr = can->PSR;
+
     if (fifo_id == FDCAN_RX_FIFO0) {
         callback = self->rxcallback0;
         state = &self->rx_state0;
+        if (RxFifo0ITs & FDCAN_FLAG_RX_FIFO0_NEW_MESSAGE) {
+            __HAL_FDCAN_DISABLE_IT(&self->can, FDCAN_IT_RX_FIFO0_NEW_MESSAGE);
+            __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_RX_FIFO0_NEW_MESSAGE);
+            irq_reason = MP_OBJ_NEW_SMALL_INT(0);
+            *state = RX_STATE_MESSAGE_PENDING;
+
+        }
+        if (RxFifo0ITs & FDCAN_FLAG_RX_FIFO0_FULL) {
+            __HAL_FDCAN_DISABLE_IT(&self->can, FDCAN_IT_RX_FIFO0_FULL);
+            __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_RX_FIFO0_FULL);
+            irq_reason = MP_OBJ_NEW_SMALL_INT(1);
+            *state = RX_STATE_FIFO_FULL;
+
+        }
+        if (RxFifo0ITs & FDCAN_FLAG_RX_FIFO0_MESSAGE_LOST) {
+            __HAL_FDCAN_DISABLE_IT(&self->can, FDCAN_IT_RX_FIFO0_MESSAGE_LOST);
+            __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_RX_FIFO0_MESSAGE_LOST);
+            irq_reason = MP_OBJ_NEW_SMALL_INT(2);
+            *state = RX_STATE_FIFO_OVERFLOW;
+        }
+
     } else {
         callback = self->rxcallback1;
         state = &self->rx_state1;
-    }
+        if (RxFifo1ITs & FDCAN_FLAG_RX_FIFO1_NEW_MESSAGE) {
-
+            __HAL_FDCAN_DISABLE_IT(&self->can, FDCAN_IT_RX_FIFO1_NEW_MESSAGE);
-    switch (*state) {
+            __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_RX_FIFO1_NEW_MESSAGE);
-        case RX_STATE_FIFO_EMPTY:
-            __HAL_FDCAN_DISABLE_IT(&self->can,  (fifo_id == FDCAN_RX_FIFO0) ?
-                FDCAN_IT_RX_FIFO0_NEW_MESSAGE : FDCAN_IT_RX_FIFO1_NEW_MESSAGE);
             irq_reason = MP_OBJ_NEW_SMALL_INT(0);
             *state = RX_STATE_MESSAGE_PENDING;
-            break;
+
-        case RX_STATE_MESSAGE_PENDING:
+        }
-            __HAL_FDCAN_DISABLE_IT(&self->can, (fifo_id == FDCAN_RX_FIFO0) ? FDCAN_IT_RX_FIFO0_FULL : FDCAN_IT_RX_FIFO1_FULL);
+        if (RxFifo1ITs & FDCAN_FLAG_RX_FIFO1_FULL) {
-            __HAL_FDCAN_CLEAR_FLAG(&self->can, (fifo_id == FDCAN_RX_FIFO0) ? FDCAN_FLAG_RX_FIFO0_FULL : FDCAN_FLAG_RX_FIFO1_FULL);
+            __HAL_FDCAN_DISABLE_IT(&self->can, FDCAN_IT_RX_FIFO1_FULL);
+            __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_RX_FIFO1_FULL);
             irq_reason = MP_OBJ_NEW_SMALL_INT(1);
             *state = RX_STATE_FIFO_FULL;
-            break;
+
-        case RX_STATE_FIFO_FULL:
+        }
-            __HAL_FDCAN_DISABLE_IT(&self->can, (fifo_id == FDCAN_RX_FIFO0) ?
+        if (RxFifo1ITs & FDCAN_FLAG_RX_FIFO1_MESSAGE_LOST) {
-                FDCAN_IT_RX_FIFO0_MESSAGE_LOST : FDCAN_IT_RX_FIFO1_MESSAGE_LOST);
+            __HAL_FDCAN_DISABLE_IT(&self->can, FDCAN_IT_RX_FIFO1_MESSAGE_LOST);
-            __HAL_FDCAN_CLEAR_FLAG(&self->can, (fifo_id == FDCAN_RX_FIFO0) ?
+            __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_RX_FIFO1_MESSAGE_LOST);
-                FDCAN_FLAG_RX_FIFO0_MESSAGE_LOST : FDCAN_FLAG_RX_FIFO1_MESSAGE_LOST);
             irq_reason = MP_OBJ_NEW_SMALL_INT(2);
             *state = RX_STATE_FIFO_OVERFLOW;
-            break;
+        }
-        case RX_STATE_FIFO_OVERFLOW:
+    }
-            // This should never happen
+
-            break;
+    if (ErrorStatusITs & FDCAN_FLAG_ERROR_WARNING) {
+        __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_ERROR_WARNING);
+        if (Psr & FDCAN_PSR_EW) {
+            irq_reason = MP_OBJ_NEW_SMALL_INT(3);
+            // mp_printf(MICROPY_ERROR_PRINTER, "clear warning %08x\n", (can->IR & FDCAN_ERROR_STATUS_MASK));
+        }
+    }
+    if (ErrorStatusITs & FDCAN_FLAG_ERROR_PASSIVE) {
+        __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_ERROR_PASSIVE);
+        if (Psr & FDCAN_PSR_EP) {
+            irq_reason = MP_OBJ_NEW_SMALL_INT(4);
+            // mp_printf(MICROPY_ERROR_PRINTER, "clear passive %08x\n", (can->IR & FDCAN_ERROR_STATUS_MASK));
+        }
+    }
+    if (ErrorStatusITs & FDCAN_FLAG_BUS_OFF) {
+        __HAL_FDCAN_CLEAR_FLAG(&self->can, FDCAN_FLAG_BUS_OFF);
+        if (Psr & FDCAN_PSR_BO) {
+            irq_reason = MP_OBJ_NEW_SMALL_INT(5);
+            // mp_printf(MICROPY_ERROR_PRINTER, "bus off %08x\n", (can->IR & FDCAN_ERROR_STATUS_MASK));
+        }
     }
 
     pyb_can_handle_callback(self, fifo_id, callback, irq_reason);
+    // mp_printf(MICROPY_ERROR_PRINTER, "Ints: %08x, %08x, %08x\n", RxFifo0ITs, RxFifo1ITs, ErrorStatusITs);
 }
 
 #if defined(MICROPY_HW_CAN1_TX)

ports/stm32/make-stmconst.py

@@ -64,7 +64,7 @@ class Lexer:
         (
             "#define typedef",
             re.compile(
-                r"#define +(?P<id>[A-Z0-9_]+(ext)?) +\(\([A-Za-z0-9_]+_TypeDef \*\) (?P<id2>[A-Za-z0-9_]+)\)($| +/\*)"
+                r"#define +(?P<id>[A-Z0-9_]+(ext)?) +\(\([A-Za-z0-9_]+_(Global)?TypeDef \*\) (?P<id2>[A-Za-z0-9_]+)\)($| +/\*)"
             ),
         ),
         ("typedef struct", re.compile(r"typedef struct$")),
@@ -281,6 +281,7 @@ def main():
         #'CAN_FIFOMailBox',
         #'CAN_FilterRegister',
         #'CAN',
+        "FDCAN",
         "CRC",
         "DAC",
         "DBGMCU",

ports/stm32/pyb_can.c

@@ -47,8 +47,14 @@
 #define CAN_FIFO1                   FDCAN_RX_FIFO1
 #define CAN_FILTER_FIFO0            (0)
 
-// Default timings; 125Kbps assuming 48MHz clock
+// Default timings; 125Kbps
+#if defined(STM32G4)
+// assuming 24MHz clock
+#define CAN_DEFAULT_PRESCALER       (16)
+#else
+// assuming 48MHz clock
 #define CAN_DEFAULT_PRESCALER       (32)
+#endif
 #define CAN_DEFAULT_SJW             (1)
 #define CAN_DEFAULT_BS1             (8)
 #define CAN_DEFAULT_BS2             (3)
@@ -60,8 +66,10 @@
 
 #define CAN1_RX0_IRQn               FDCAN1_IT0_IRQn
 #define CAN1_RX1_IRQn               FDCAN1_IT1_IRQn
+#if defined(CAN2)
 #define CAN2_RX0_IRQn               FDCAN2_IT0_IRQn
 #define CAN2_RX1_IRQn               FDCAN2_IT1_IRQn
+#endif
 
 #define CAN_IT_FIFO0_FULL           FDCAN_IT_RX_FIFO0_FULL
 #define CAN_IT_FIFO1_FULL           FDCAN_IT_RX_FIFO1_FULL
@@ -326,6 +334,9 @@ STATIC mp_obj_t pyb_can_restart(mp_obj_t self_in) {
     can->CCCR |= FDCAN_CCCR_INIT;
     while ((can->CCCR & FDCAN_CCCR_INIT) == 0) {
     }
+    can->CCCR |= FDCAN_CCCR_CCE;
+    while ((can->CCCR & FDCAN_CCCR_CCE) == 0) {
+    }
     can->CCCR &= ~FDCAN_CCCR_INIT;
     while ((can->CCCR & FDCAN_CCCR_INIT)) {
     }
@@ -348,11 +359,12 @@ STATIC mp_obj_t pyb_can_state(mp_obj_t self_in) {
     if (self->is_enabled) {
         CAN_TypeDef *can = self->can.Instance;
         #if MICROPY_HW_ENABLE_FDCAN
-        if (can->PSR & FDCAN_PSR_BO) {
+        uint32_t psr = can->PSR;
+        if (psr & FDCAN_PSR_BO) {
             state = CAN_STATE_BUS_OFF;
-        } else if (can->PSR & FDCAN_PSR_EP) {
+        } else if (psr & FDCAN_PSR_EP) {
             state = CAN_STATE_ERROR_PASSIVE;
-        } else if (can->PSR & FDCAN_PSR_EW) {
+        } else if (psr & FDCAN_PSR_EW) {
             state = CAN_STATE_ERROR_WARNING;
         } else {
             state = CAN_STATE_ERROR_ACTIVE;
@@ -375,10 +387,6 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(pyb_can_state_obj, pyb_can_state);
 
 // Get info about error states and TX/RX buffers
 STATIC mp_obj_t pyb_can_info(size_t n_args, const mp_obj_t *args) {
-    #if MICROPY_HW_ENABLE_FDCAN
-    // TODO implement for FDCAN
-    return mp_const_none;
-    #else
     pyb_can_obj_t *self = MP_OBJ_TO_PTR(args[0]);
     mp_obj_list_t *list;
     if (n_args == 1) {
@@ -392,6 +400,20 @@ STATIC mp_obj_t pyb_can_info(size_t n_args, const mp_obj_t *args) {
             mp_raise_ValueError(NULL);
         }
     }
+
+    #if MICROPY_HW_ENABLE_FDCAN
+    FDCAN_GlobalTypeDef *can = self->can.Instance;
+    uint32_t esr = can->ECR;
+    list->items[0] = MP_OBJ_NEW_SMALL_INT((esr & FDCAN_ECR_TEC_Msk) >> FDCAN_ECR_TEC_Pos);
+    list->items[1] = MP_OBJ_NEW_SMALL_INT((esr & FDCAN_ECR_REC_Msk) >> FDCAN_ECR_REC_Pos);
+    list->items[2] = MP_OBJ_NEW_SMALL_INT(self->num_error_warning);
+    list->items[3] = MP_OBJ_NEW_SMALL_INT(self->num_error_passive);
+    list->items[4] = MP_OBJ_NEW_SMALL_INT(self->num_bus_off);
+    uint32_t TXEFS = can->TXEFS;
+    list->items[5] = MP_OBJ_NEW_SMALL_INT(TXEFS & 0x7);
+    list->items[6] = MP_OBJ_NEW_SMALL_INT((can->RXF0S & FDCAN_RXF0S_F0FL_Msk) >> FDCAN_RXF0S_F0FL_Pos);
+    list->items[7] = MP_OBJ_NEW_SMALL_INT((can->RXF1S & FDCAN_RXF1S_F1FL_Msk) >> FDCAN_RXF1S_F1FL_Pos);
+    #else
     CAN_TypeDef *can = self->can.Instance;
     uint32_t esr = can->ESR;
     list->items[0] = MP_OBJ_NEW_SMALL_INT(esr >> CAN_ESR_TEC_Pos & 0xff);
@@ -403,8 +425,9 @@ STATIC mp_obj_t pyb_can_info(size_t n_args, const mp_obj_t *args) {
     list->items[5] = MP_OBJ_NEW_SMALL_INT(n_tx_pending);
     list->items[6] = MP_OBJ_NEW_SMALL_INT(can->RF0R >> CAN_RF0R_FMP0_Pos & 3);
     list->items[7] = MP_OBJ_NEW_SMALL_INT(can->RF1R >> CAN_RF1R_FMP1_Pos & 3);
-    return MP_OBJ_FROM_PTR(list);
     #endif
+
+    return MP_OBJ_FROM_PTR(list);
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(pyb_can_info_obj, 1, 2, pyb_can_info);