No more infinite while loops in CAN init (#499)

board/drivers/can.h

@@ -26,7 +26,7 @@ extern uint32_t can_speed[4];
 
 void can_set_forwarding(int from, int to);
 
-void can_init(uint8_t can_number);
+bool can_init(uint8_t can_number);
 void can_init_all(void);
 bool can_tx_check_min_slots_free(uint32_t min);
 void can_send(CAN_FIFOMailBox_TypeDef *to_push, uint8_t bus_number, bool skip_tx_hook);
@@ -148,27 +148,26 @@ uint32_t can_speed[] = {5000, 5000, 5000, 333};
 
 #define CANIF_FROM_CAN_NUM(num) (cans[num])
 #define CAN_NUM_FROM_CANIF(CAN) ((CAN)==CAN1 ? 0 : ((CAN) == CAN2 ? 1 : 2))
-#define CAN_NAME_FROM_CANIF(CAN) ((CAN)==CAN1 ? "CAN1" : ((CAN) == CAN2 ? "CAN2" : "CAN3"))
 #define BUS_NUM_FROM_CAN_NUM(num) (bus_lookup[num])
 #define CAN_NUM_FROM_BUS_NUM(num) (can_num_lookup[num])
 
 void process_can(uint8_t can_number);
 
-void can_set_speed(uint8_t can_number) {
+bool can_set_speed(uint8_t can_number) {
+  bool ret = true;
   CAN_TypeDef *CAN = CANIF_FROM_CAN_NUM(can_number);
   uint8_t bus_number = BUS_NUM_FROM_CAN_NUM(can_number);
 
-  if (!llcan_set_speed(CAN, can_speed[bus_number], can_loopback, (unsigned int)(can_silent) & (1U << can_number))) {
+  ret &= llcan_set_speed(CAN, can_speed[bus_number], can_loopback, (unsigned int)(can_silent) & (1U << can_number));
-    puts("CAN init FAILED!!!!!\n");
+  return ret;
-    puth(can_number); puts(" ");
-    puth(BUS_NUM_FROM_CAN_NUM(can_number)); puts("\n");
-  }
 }
 
 void can_init_all(void) {
+  bool ret = true;
   for (uint8_t i=0U; i < CAN_MAX; i++) {
-    can_init(i);
+    ret &= can_init(i);
   }
+  UNUSED(ret);
 }
 
 void can_flip_buses(uint8_t bus1, uint8_t bus2){
@@ -194,7 +193,8 @@ void can_set_gmlan(uint8_t bus) {
           bus_lookup[prev_bus] = prev_bus;
           can_num_lookup[prev_bus] = prev_bus;
           can_num_lookup[3] = -1;
-          can_init(prev_bus);
+          bool ret = can_init(prev_bus);
+          UNUSED(ret);
           break;
         default:
           // GMLAN was not set on either BUS 1 or 2
@@ -213,7 +213,8 @@ void can_set_gmlan(uint8_t bus) {
         bus_lookup[bus] = 3;
         can_num_lookup[bus] = -1;
         can_num_lookup[3] = bus;
-        can_init(bus);
+        bool ret = can_init(bus);
+        UNUSED(ret);
         break;
       case 0xFF:  //-1 unsigned
         break;
@@ -447,7 +448,9 @@ void can_set_forwarding(int from, int to) {
   can_forwarding[from] = to;
 }
 
-void can_init(uint8_t can_number) {
+bool can_init(uint8_t can_number) {
+  bool ret = false;
+
   REGISTER_INTERRUPT(CAN1_TX_IRQn, CAN1_TX_IRQ_Handler, CAN_INTERRUPT_RATE, FAULT_INTERRUPT_RATE_CAN_1)
   REGISTER_INTERRUPT(CAN1_RX0_IRQn, CAN1_RX0_IRQ_Handler, CAN_INTERRUPT_RATE, FAULT_INTERRUPT_RATE_CAN_1)
   REGISTER_INTERRUPT(CAN1_SCE_IRQn, CAN1_SCE_IRQ_Handler, CAN_INTERRUPT_RATE, FAULT_INTERRUPT_RATE_CAN_1)
@@ -460,12 +463,11 @@ void can_init(uint8_t can_number) {
 
   if (can_number != 0xffU) {
     CAN_TypeDef *CAN = CANIF_FROM_CAN_NUM(can_number);
-    can_set_speed(can_number);
+    ret &= can_set_speed(can_number);
-
+    ret &= llcan_init(CAN);
-    llcan_init(CAN);
-
     // in case there are queued up messages
     process_can(can_number);
   }
+  return ret;
 }
 

board/drivers/llcan.h

@@ -15,78 +15,114 @@
 #define GET_BYTES_04(msg) ((msg)->RDLR)
 #define GET_BYTES_48(msg) ((msg)->RDHR)
 
+#define CAN_INIT_TIMEOUT_MS 500U
+#define CAN_NAME_FROM_CANIF(CAN_DEV) (((CAN_DEV)==CAN1) ? "CAN1" : (((CAN_DEV) == CAN2) ? "CAN2" : "CAN3"))
+
 void puts(const char *a);
 
 bool llcan_set_speed(CAN_TypeDef *CAN_obj, uint32_t speed, bool loopback, bool silent) {
+  bool ret = true;
+
   // initialization mode
   register_set(&(CAN_obj->MCR), CAN_MCR_TTCM | CAN_MCR_INRQ, 0x180FFU);
-  while((CAN_obj->MSR & CAN_MSR_INAK) != CAN_MSR_INAK);
+  uint32_t timeout_counter = 0U;
+  while((CAN_obj->MSR & CAN_MSR_INAK) != CAN_MSR_INAK){
+    // Delay for about 1ms
+    delay(10000);
+    timeout_counter++;
 
-  // set time quanta from defines
+    if(timeout_counter >= CAN_INIT_TIMEOUT_MS){
-  register_set(&(CAN_obj->BTR), ((CAN_BTR_TS1_0 * (CAN_SEQ1-1)) |
+      puts(CAN_NAME_FROM_CANIF(CAN_obj)); puts(" set_speed timed out (1)!\n");
-            (CAN_BTR_TS2_0 * (CAN_SEQ2-1)) |
+      ret = false;
-            (can_speed_to_prescaler(speed) - 1U)), 0xC37F03FFU);
+    }
-
-  // silent loopback mode for debugging
-  if (loopback) {
-    register_set_bits(&(CAN_obj->BTR), CAN_BTR_SILM | CAN_BTR_LBKM);
-  }
-  if (silent) {
-    register_set_bits(&(CAN_obj->BTR), CAN_BTR_SILM);
   }
 
-  // reset
+  if(ret){
-  register_set(&(CAN_obj->MCR), CAN_MCR_TTCM | CAN_MCR_ABOM, 0x180FFU);
+    // set time quanta from defines
+    register_set(&(CAN_obj->BTR), ((CAN_BTR_TS1_0 * (CAN_SEQ1-1)) |
+              (CAN_BTR_TS2_0 * (CAN_SEQ2-1)) |
+              (can_speed_to_prescaler(speed) - 1U)), 0xC37F03FFU);
 
-  #define CAN_TIMEOUT 1000000
+    // silent loopback mode for debugging
-  int tmp = 0;
+    if (loopback) {
-  bool ret = false;
+      register_set_bits(&(CAN_obj->BTR), CAN_BTR_SILM | CAN_BTR_LBKM);
-  while(((CAN_obj->MSR & CAN_MSR_INAK) == CAN_MSR_INAK) && (tmp < CAN_TIMEOUT)) tmp++;
+    }
-  if (tmp < CAN_TIMEOUT) {
+    if (silent) {
-    ret = true;
+      register_set_bits(&(CAN_obj->BTR), CAN_BTR_SILM);
+    }
+
+    // reset
+    register_set(&(CAN_obj->MCR), CAN_MCR_TTCM | CAN_MCR_ABOM, 0x180FFU);
+
+    timeout_counter = 0U;
+    while(((CAN_obj->MSR & CAN_MSR_INAK) == CAN_MSR_INAK)) {
+      // Delay for about 1ms
+      delay(10000);
+      timeout_counter++;
+
+      if(timeout_counter >= CAN_INIT_TIMEOUT_MS){
+        puts(CAN_NAME_FROM_CANIF(CAN_obj)); puts(" set_speed timed out (2)!\n");
+        ret = false;
+      }
+    }
   }
 
   return ret;
 }
 
-void llcan_init(CAN_TypeDef *CAN_obj) {
+bool llcan_init(CAN_TypeDef *CAN_obj) {
+  bool ret = true;
+
   // Enter init mode
   register_set_bits(&(CAN_obj->FMR), CAN_FMR_FINIT);
 
   // Wait for INAK bit to be set
-  while(((CAN_obj->MSR & CAN_MSR_INAK) == CAN_MSR_INAK)) {}
+  uint32_t timeout_counter = 0U;
+  while(((CAN_obj->MSR & CAN_MSR_INAK) == CAN_MSR_INAK)) {
+    // Delay for about 1ms
+    delay(10000);
+    timeout_counter++;
 
-  // no mask
+    if(timeout_counter >= CAN_INIT_TIMEOUT_MS){
-  // For some weird reason some of these registers do not want to set properly on CAN2 and CAN3. Probably something to do with the single/dual mode and their different filters.
+      puts(CAN_NAME_FROM_CANIF(CAN_obj)); puts(" initialization timed out!\n");
-  CAN_obj->sFilterRegister[0].FR1 = 0U;
+      ret = false;
-  CAN_obj->sFilterRegister[0].FR2 = 0U;
+    }
-  CAN_obj->sFilterRegister[14].FR1 = 0U;
-  CAN_obj->sFilterRegister[14].FR2 = 0U;
-  CAN_obj->FA1R |= 1U | (1U << 14);
-
-  // Exit init mode, do not wait
-  register_clear_bits(&(CAN_obj->FMR), CAN_FMR_FINIT);
-
-  // enable certain CAN interrupts
-  register_set_bits(&(CAN_obj->IER), CAN_IER_TMEIE | CAN_IER_FMPIE0 |  CAN_IER_WKUIE);
-
-  if (CAN_obj == CAN1) {
-    NVIC_EnableIRQ(CAN1_TX_IRQn);
-    NVIC_EnableIRQ(CAN1_RX0_IRQn);
-    NVIC_EnableIRQ(CAN1_SCE_IRQn);
-  } else if (CAN_obj == CAN2) {
-    NVIC_EnableIRQ(CAN2_TX_IRQn);
-    NVIC_EnableIRQ(CAN2_RX0_IRQn);
-    NVIC_EnableIRQ(CAN2_SCE_IRQn);
-#ifdef CAN3
-  } else if (CAN_obj == CAN3) {
-    NVIC_EnableIRQ(CAN3_TX_IRQn);
-    NVIC_EnableIRQ(CAN3_RX0_IRQn);
-    NVIC_EnableIRQ(CAN3_SCE_IRQn);
-#endif
-  } else {
-    puts("Invalid CAN: initialization failed\n");
   }
+  
+  if(ret){
+    // no mask
+    // For some weird reason some of these registers do not want to set properly on CAN2 and CAN3. Probably something to do with the single/dual mode and their different filters.
+    CAN_obj->sFilterRegister[0].FR1 = 0U;
+    CAN_obj->sFilterRegister[0].FR2 = 0U;
+    CAN_obj->sFilterRegister[14].FR1 = 0U;
+    CAN_obj->sFilterRegister[14].FR2 = 0U;
+    CAN_obj->FA1R |= 1U | (1U << 14);
+
+    // Exit init mode, do not wait
+    register_clear_bits(&(CAN_obj->FMR), CAN_FMR_FINIT);
+
+    // enable certain CAN interrupts
+    register_set_bits(&(CAN_obj->IER), CAN_IER_TMEIE | CAN_IER_FMPIE0 |  CAN_IER_WKUIE);
+
+    if (CAN_obj == CAN1) {
+      NVIC_EnableIRQ(CAN1_TX_IRQn);
+      NVIC_EnableIRQ(CAN1_RX0_IRQn);
+      NVIC_EnableIRQ(CAN1_SCE_IRQn);
+    } else if (CAN_obj == CAN2) {
+      NVIC_EnableIRQ(CAN2_TX_IRQn);
+      NVIC_EnableIRQ(CAN2_RX0_IRQn);
+      NVIC_EnableIRQ(CAN2_SCE_IRQn);
+    #ifdef CAN3
+      } else if (CAN_obj == CAN3) {
+        NVIC_EnableIRQ(CAN3_TX_IRQn);
+        NVIC_EnableIRQ(CAN3_RX0_IRQn);
+        NVIC_EnableIRQ(CAN3_SCE_IRQn);
+    #endif
+    } else {
+      puts("Invalid CAN: initialization failed\n");
+    }
+  }
+  return ret;
 }
 
 void llcan_clear_send(CAN_TypeDef *CAN_obj) {

board/main.c

@@ -472,7 +472,8 @@ int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired)
     case 0xde:
       if (setup->b.wValue.w < BUS_MAX) {
         can_speed[setup->b.wValue.w] = setup->b.wIndex.w;
-        can_init(CAN_NUM_FROM_BUS_NUM(setup->b.wValue.w));
+        bool ret = can_init(CAN_NUM_FROM_BUS_NUM(setup->b.wValue.w));
+        UNUSED(ret);
       }
       break;
     // **** 0xdf: set unsafe mode

board/pedal/main.c

@@ -316,7 +316,8 @@ int main(void) {
     puts("Failed to set llcan speed");
   }
 
-  llcan_init(CAN1);
+  bool ret = llcan_init(CAN1);
+  UNUSED(ret);
 
   // 48mhz / 65536 ~= 732
   timer_init(TIM3, 15);