Init cleanup: global LED driver (#2170)

robbederks · Comma Device · web-flow · commit 998ac9d5d82f · 2025-03-14T18:01:45.000+01:00
* wip

* fix init

* fix libpanda

* cleanup

---------

Co-authored-by: Comma Device &lt;device@comma.ai&gt;
diff --git a/board/boards/black.h b/board/boards/black.h
@@ -26,22 +26,6 @@ static void black_enable_can_transceiver(uint8_t transceiver, bool enabled) {
   }
 }
 
-static void black_set_led(uint8_t color, bool enabled) {
-  switch (color){
-    case LED_RED:
-      set_gpio_output(GPIOC, 9, !enabled);
-      break;
-     case LED_GREEN:
-      set_gpio_output(GPIOC, 7, !enabled);
-      break;
-    case LED_BLUE:
-      set_gpio_output(GPIOC, 6, !enabled);
-      break;
-    default:
-      break;
-  }
-}
-
 static void black_set_usb_load_switch(bool enabled) {
   set_gpio_output(GPIOB, 1, !enabled);
 }
@@ -132,7 +116,8 @@ board board_black = {
   .init = black_init,
   .init_bootloader = black_init_bootloader,
   .enable_can_transceiver = black_enable_can_transceiver,
-  .set_led = black_set_led,
+  .led_GPIO = {GPIOC, GPIOC, GPIOC},
+  .led_pin = {9, 7, 6},
   .set_can_mode = black_set_can_mode,
   .check_ignition = black_check_ignition,
   .read_voltage_mV = white_read_voltage_mV,
diff --git a/board/boards/board_declarations.h b/board/boards/board_declarations.h
@@ -13,7 +13,6 @@ typedef enum {
 typedef void (*board_init)(void);
 typedef void (*board_init_bootloader)(void);
 typedef void (*board_enable_can_transceiver)(uint8_t transceiver, bool enabled);
-typedef void (*board_set_led)(uint8_t color, bool enabled);
 typedef void (*board_set_can_mode)(uint8_t mode);
 typedef bool (*board_check_ignition)(void);
 typedef uint32_t (*board_read_voltage_mV)(void);
@@ -27,6 +26,8 @@ typedef void (*board_set_amp_enabled)(bool enabled);
 
 struct board {
   harness_configuration *harness_config;
+  GPIO_TypeDef * const led_GPIO[3];
+  const uint8_t led_pin[3];
   const bool has_spi;
   const bool has_canfd;
   const uint16_t fan_max_rpm;
@@ -37,7 +38,6 @@ struct board {
   board_init init;
   board_init_bootloader init_bootloader;
   board_enable_can_transceiver enable_can_transceiver;
-  board_set_led set_led;
   board_set_can_mode set_can_mode;
   board_check_ignition check_ignition;
   board_read_voltage_mV read_voltage_mV;
@@ -64,11 +64,6 @@ struct board {
 #define HW_TYPE_TRES 9U
 #define HW_TYPE_CUATRO 10U
 
-// LED colors
-#define LED_RED 0U
-#define LED_GREEN 1U
-#define LED_BLUE 2U
-
 // USB power modes (from cereal.log.health)
 #define USB_POWER_NONE 0U
 #define USB_POWER_CLIENT 1U
diff --git a/board/boards/cuatro.h b/board/boards/cuatro.h
@@ -6,22 +6,6 @@
 // Cuatro (STM32H7) + Harness //
 // ////////////////////////// //
 
-static void cuatro_set_led(uint8_t color, bool enabled) {
-  switch (color) {
-    case LED_RED:
-      set_gpio_output(GPIOC, 6, !enabled);
-      break;
-    case LED_GREEN:
-      set_gpio_output(GPIOC, 7, !enabled);
-      break;
-    case LED_BLUE:
-      set_gpio_output(GPIOC, 9, !enabled);
-      break;
-    default:
-      break;
-  }
-}
-
 static void cuatro_enable_can_transceiver(uint8_t transceiver, bool enabled) {
   switch (transceiver) {
     case 1U:
@@ -66,12 +50,7 @@ static void cuatro_set_amp_enabled(bool enabled){
 static void cuatro_init(void) {
   common_init_gpio();
 
-  // init LEDs as open drain
-  set_gpio_output_type(GPIOC, 6, OUTPUT_TYPE_OPEN_DRAIN);
-  set_gpio_output_type(GPIOC, 7, OUTPUT_TYPE_OPEN_DRAIN);
-  set_gpio_output_type(GPIOC, 9, OUTPUT_TYPE_OPEN_DRAIN);
-
-  // more open drain
+  // open drain
   set_gpio_output_type(GPIOD, 3, OUTPUT_TYPE_OPEN_DRAIN); // FAN_EN
   set_gpio_output_type(GPIOC, 12, OUTPUT_TYPE_OPEN_DRAIN); // VBAT_EN
 
@@ -138,7 +117,8 @@ board board_cuatro = {
   .init = cuatro_init,
   .init_bootloader = unused_init_bootloader,
   .enable_can_transceiver = cuatro_enable_can_transceiver,
-  .set_led = cuatro_set_led,
+  .led_GPIO = {GPIOC, GPIOC, GPIOC},
+  .led_pin = {6, 7, 9},
   .set_can_mode = tres_set_can_mode,
   .check_ignition = red_check_ignition,
   .read_voltage_mV = cuatro_read_voltage_mV,
diff --git a/board/boards/dos.h b/board/boards/dos.h
@@ -26,22 +26,6 @@ static void dos_enable_can_transceiver(uint8_t transceiver, bool enabled) {
   }
 }
 
-static void dos_set_led(uint8_t color, bool enabled) {
-  switch (color){
-    case LED_RED:
-      set_gpio_output(GPIOC, 9, !enabled);
-      break;
-     case LED_GREEN:
-      set_gpio_output(GPIOC, 7, !enabled);
-      break;
-    case LED_BLUE:
-      set_gpio_output(GPIOC, 6, !enabled);
-      break;
-    default:
-      break;
-  }
-}
-
 static void dos_set_bootkick(BootState state) {
   set_gpio_output(GPIOC, 4, state != BOOT_BOOTKICK);
 }
@@ -155,7 +139,8 @@ board board_dos = {
   .init = dos_init,
   .init_bootloader = unused_init_bootloader,
   .enable_can_transceiver = dos_enable_can_transceiver,
-  .set_led = dos_set_led,
+  .led_GPIO = {GPIOC, GPIOC, GPIOC},
+  .led_pin = {9, 7, 6},
   .set_can_mode = dos_set_can_mode,
   .check_ignition = dos_check_ignition,
   .read_voltage_mV = white_read_voltage_mV,
diff --git a/board/boards/grey.h b/board/boards/grey.h
@@ -21,7 +21,8 @@ board board_grey = {
   .init = white_grey_init,
   .init_bootloader = white_grey_init_bootloader,
   .enable_can_transceiver = white_enable_can_transceiver,
-  .set_led = white_set_led,
+  .led_GPIO = {GPIOC, GPIOC, GPIOC},
+  .led_pin = {9, 7, 6},
   .set_can_mode = white_set_can_mode,
   .check_ignition = white_check_ignition,
   .read_voltage_mV = white_read_voltage_mV,
diff --git a/board/boards/red.h b/board/boards/red.h
@@ -25,22 +25,6 @@ static void red_enable_can_transceiver(uint8_t transceiver, bool enabled) {
   }
 }
 
-static void red_set_led(uint8_t color, bool enabled) {
-  switch (color) {
-    case LED_RED:
-      set_gpio_output(GPIOE, 4, !enabled);
-      break;
-     case LED_GREEN:
-      set_gpio_output(GPIOE, 3, !enabled);
-      break;
-    case LED_BLUE:
-      set_gpio_output(GPIOE, 2, !enabled);
-      break;
-    default:
-      break;
-  }
-}
-
 static void red_set_can_mode(uint8_t mode) {
   red_enable_can_transceiver(2U, false);
   red_enable_can_transceiver(4U, false);
@@ -146,7 +130,8 @@ board board_red = {
   .init = red_init,
   .init_bootloader = unused_init_bootloader,
   .enable_can_transceiver = red_enable_can_transceiver,
-  .set_led = red_set_led,
+  .led_GPIO = {GPIOE, GPIOE, GPIOE},
+  .led_pin = {4, 3, 2},
   .set_can_mode = red_set_can_mode,
   .check_ignition = red_check_ignition,
   .read_voltage_mV = red_read_voltage_mV,
diff --git a/board/boards/tres.h b/board/boards/tres.h
@@ -158,7 +158,8 @@ board board_tres = {
   .init = tres_init,
   .init_bootloader = unused_init_bootloader,
   .enable_can_transceiver = tres_enable_can_transceiver,
-  .set_led = red_set_led,
+  .led_GPIO = {GPIOE, GPIOE, GPIOE},
+  .led_pin = {4, 3, 2},
   .set_can_mode = tres_set_can_mode,
   .check_ignition = red_check_ignition,
   .read_voltage_mV = red_read_voltage_mV,
diff --git a/board/boards/uno.h b/board/boards/uno.h
@@ -26,22 +26,6 @@ static void uno_enable_can_transceiver(uint8_t transceiver, bool enabled) {
   }
 }
 
-static void uno_set_led(uint8_t color, bool enabled) {
-  switch (color){
-    case LED_RED:
-      set_gpio_output(GPIOC, 9, !enabled);
-      break;
-     case LED_GREEN:
-      set_gpio_output(GPIOC, 7, !enabled);
-      break;
-    case LED_BLUE:
-      set_gpio_output(GPIOC, 6, !enabled);
-      break;
-    default:
-      break;
-  }
-}
-
 static void uno_set_bootkick(BootState state) {
   if (state == BOOT_BOOTKICK) {
     set_gpio_output(GPIOB, 14, false);
@@ -167,7 +151,8 @@ board board_uno = {
   .init = uno_init,
   .init_bootloader = uno_init_bootloader,
   .enable_can_transceiver = uno_enable_can_transceiver,
-  .set_led = uno_set_led,
+  .led_GPIO = {GPIOC, GPIOC, GPIOC},
+  .led_pin = {9, 7, 6},
   .set_can_mode = uno_set_can_mode,
   .check_ignition = uno_check_ignition,
   .read_voltage_mV = white_read_voltage_mV,
diff --git a/board/boards/white.h b/board/boards/white.h
@@ -22,22 +22,6 @@ static void white_enable_can_transceiver(uint8_t transceiver, bool enabled) {
   }
 }
 
-static void white_set_led(uint8_t color, bool enabled) {
-  switch (color){
-    case LED_RED:
-      set_gpio_output(GPIOC, 9, !enabled);
-      break;
-     case LED_GREEN:
-      set_gpio_output(GPIOC, 7, !enabled);
-      break;
-    case LED_BLUE:
-      set_gpio_output(GPIOC, 6, !enabled);
-      break;
-    default:
-      break;
-  }
-}
-
 static void white_set_usb_power_mode(uint8_t mode){
   switch (mode) {
     case USB_POWER_CLIENT:
@@ -181,7 +165,8 @@ board board_white = {
   .init = white_grey_init,
   .init_bootloader = white_grey_init_bootloader,
   .enable_can_transceiver = white_enable_can_transceiver,
-  .set_led = white_set_led,
+  .led_GPIO = {GPIOC, GPIOC, GPIOC},
+  .led_pin = {9, 7, 6},
   .set_can_mode = white_set_can_mode,
   .check_ignition = white_check_ignition,
   .read_voltage_mV = white_read_voltage_mV,
diff --git a/board/bootstub.c b/board/bootstub.c
@@ -6,6 +6,7 @@
 // ********************* Includes *********************
 #include "config.h"
 
+#include "drivers/led.h"
 #include "drivers/pwm.h"
 #include "drivers/usb.h"
 
diff --git a/board/drivers/bxcan.h b/board/drivers/bxcan.h
@@ -178,7 +178,7 @@ void can_rx(uint8_t can_number) {
     safety_rx_invalid += safety_rx_hook(&to_push) ? 0U : 1U;
     ignition_can_hook(&to_push);
 
-    current_board->set_led(LED_BLUE, true);
+    led_set(LED_BLUE, true);
     rx_buffer_overflow += can_push(&can_rx_q, &to_push) ? 0U : 1U;
 
     // next
diff --git a/board/drivers/fdcan.h b/board/drivers/fdcan.h
@@ -223,7 +223,7 @@ void can_rx(uint8_t can_number) {
     safety_rx_invalid += safety_rx_hook(&to_push) ? 0U : 1U;
     ignition_can_hook(&to_push);
 
-    current_board->set_led(LED_BLUE, true);
+    led_set(LED_BLUE, true);
     rx_buffer_overflow += can_push(&can_rx_q, &to_push) ? 0U : 1U;
 
     // Enable CAN FD and BRS if CAN FD message was received
diff --git a/board/drivers/led.h b/board/drivers/led.h
@@ -0,0 +1,20 @@
+
+#define LED_RED 0U
+#define LED_GREEN 1U
+#define LED_BLUE 2U
+
+void led_set(uint8_t color, bool enabled) {
+  if (color < 3U) {
+    set_gpio_output(current_board->led_GPIO[color], current_board->led_pin[color], !enabled);
+  }
+}
+
+void led_init(void) {
+  for (uint8_t i = 0U; i<3U; i++){
+    set_gpio_pullup(current_board->led_GPIO[i], current_board->led_pin[i], PULL_NONE);
+    set_gpio_mode(current_board->led_GPIO[i], current_board->led_pin[i], MODE_OUTPUT);
+    set_gpio_output_type(current_board->led_GPIO[i], current_board->led_pin[i], OUTPUT_TYPE_OPEN_DRAIN);
+
+    led_set(i, false);
+  }
+}
diff --git a/board/early_init.h b/board/early_init.h
@@ -56,10 +56,11 @@ void early_initialization(void) {
   detect_board_type();
 
   if (enter_bootloader_mode == ENTER_BOOTLOADER_MAGIC) {
+    led_init();
     #ifdef PANDA
     current_board->init_bootloader();
     #endif
-    current_board->set_led(LED_GREEN, 1);
+    led_set(LED_GREEN, 1);
     jump_to_bootloader();
   }
 }
diff --git a/board/fake_stm.h b/board/fake_stm.h
@@ -31,3 +31,5 @@ uint32_t microsecond_timer_get(void);
 uint32_t microsecond_timer_get(void) {
   return MICROSECOND_TIMER->CNT;
 }
+
+typedef uint32_t GPIO_TypeDef;
diff --git a/board/flasher.h b/board/flasher.h
@@ -35,7 +35,7 @@ int comms_control_handler(ControlPacket_t *req, uint8_t *resp) {
         flash_unlock();
         resp[1] = 0xff;
       }
-      current_board->set_led(LED_GREEN, 1);
+      led_set(LED_GREEN, 1);
       unlocked = true;
       prog_ptr = (uint32_t *)APP_START_ADDRESS;
       break;
@@ -112,14 +112,14 @@ int comms_can_read(uint8_t *data, uint32_t max_len) {
 void refresh_can_tx_slots_available(void) {}
 
 void comms_endpoint2_write(const uint8_t *data, uint32_t len) {
-  current_board->set_led(LED_RED, 0);
+  led_set(LED_RED, 0);
   for (uint32_t i = 0; i < len/4; i++) {
     flash_write_word(prog_ptr, *(uint32_t*)(data+(i*4)));
 
     //*(uint64_t*)(&spi_tx_buf[0x30+(i*4)]) = *prog_ptr;
     prog_ptr++;
   }
-  current_board->set_led(LED_RED, 1);
+  led_set(LED_RED, 1);
 }
 
 
@@ -132,6 +132,7 @@ void soft_flasher_start(void) {
 
   gpio_usart2_init();
   gpio_usb_init();
+  led_init();
 
   // enable USB
   usb_init();
@@ -143,15 +144,15 @@ void soft_flasher_start(void) {
   }
 
   // green LED on for flashing
-  current_board->set_led(LED_GREEN, 1);
+  led_set(LED_GREEN, 1);
 
   enable_interrupts();
 
   for (;;) {
     // blink the green LED fast
-    current_board->set_led(LED_GREEN, 0);
+    led_set(LED_GREEN, 0);
     delay(500000);
-    current_board->set_led(LED_GREEN, 1);
+    led_set(LED_GREEN, 1);
     delay(500000);
   }
 }
diff --git a/board/jungle/boards/board_declarations.h b/board/jungle/boards/board_declarations.h
diff --git a/board/jungle/boards/board_v1.h b/board/jungle/boards/board_v1.h
diff --git a/board/jungle/boards/board_v2.h b/board/jungle/boards/board_v2.h
diff --git a/board/jungle/main.c b/board/jungle/main.c
diff --git a/board/main.c b/board/main.c
diff --git a/board/stm32h7/peripherals.h b/board/stm32h7/peripherals.h
