From ccf13b7afdb454429aa8539e69f642ccedb6f264 Mon Sep 17 00:00:00 2001 From: robbederks Date: Mon, 13 Apr 2020 20:32:53 -0700 Subject: [PATCH] No more infinite while loops in CAN init (#499) --- board/drivers/can.h | 34 +++++----- board/drivers/llcan.h | 146 ++++++++++++++++++++++++++---------------- board/main.c | 3 +- board/pedal/main.c | 3 +- 4 files changed, 113 insertions(+), 73 deletions(-) diff --git a/board/drivers/can.h b/board/drivers/can.h index 10aa1208f6057f4..5a63246b7511f5a 100644 --- a/board/drivers/can.h +++ b/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))) { - puts("CAN init FAILED!!!!!\n"); - puth(can_number); puts(" "); - puth(BUS_NUM_FROM_CAN_NUM(can_number)); puts("\n"); - } + ret &= llcan_set_speed(CAN, can_speed[bus_number], can_loopback, (unsigned int)(can_silent) & (1U << can_number)); + return ret; } 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); - - llcan_init(CAN); - + ret &= can_set_speed(can_number); + ret &= llcan_init(CAN); // in case there are queued up messages process_can(can_number); } + return ret; } diff --git a/board/drivers/llcan.h b/board/drivers/llcan.h index 8acb0fc19723083..0efc134085598df 100644 --- a/board/drivers/llcan.h +++ b/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); - - // 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); - - // 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); + uint32_t 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 (1)!\n"); + ret = false; + } } - // reset - register_set(&(CAN_obj->MCR), CAN_MCR_TTCM | CAN_MCR_ABOM, 0x180FFU); - - #define CAN_TIMEOUT 1000000 - int tmp = 0; - bool ret = false; - while(((CAN_obj->MSR & CAN_MSR_INAK) == CAN_MSR_INAK) && (tmp < CAN_TIMEOUT)) tmp++; - if (tmp < CAN_TIMEOUT) { - ret = true; + if(ret){ + // 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); + + // 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 + 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)) {} - - // 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"); + uint32_t 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(" initialization timed out!\n"); + ret = false; + } } + + 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) { diff --git a/board/main.c b/board/main.c index 4144d80fd6c9404..79cccec3d31a030 100644 --- a/board/main.c +++ b/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 diff --git a/board/pedal/main.c b/board/pedal/main.c index 093f4c9357936d7..3d7bd03a2714462 100644 --- a/board/pedal/main.c +++ b/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);