diff mbox series

[v3,4/9] common: board_r: rework BOOT LED handling

Message ID 20240812103254.26972-5-ansuelsmth@gmail.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series led: introduce LED boot and activity function | expand

Commit Message

Christian Marangi Aug. 12, 2024, 10:32 a.m. UTC
Rework BOOT LED handling. There is currently one legacy implementation
for BOOT LED from Status Led API.

This work on ancient implementation wused by BOOTP by setting the LED
to Blink on boot and to turn it OFF when the firmware was correctly
received by network.

Now that we new LED implementation have support for LED boot, rework
this by also set the new BOOT LED to blink and also set it to ON before
entering main loop to confirm successful boot.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 common/board_r.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

Comments

Simon Glass Sept. 19, 2024, 2:13 p.m. UTC | #1
Hi Christian,

On Mon, 12 Aug 2024 at 12:33, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> Rework BOOT LED handling. There is currently one legacy implementation
> for BOOT LED from Status Led API.
>
> This work on ancient implementation wused by BOOTP by setting the LED

used

> to Blink on boot and to turn it OFF when the firmware was correctly
> received by network.
>
> Now that we new LED implementation have support for LED boot, rework
> this by also set the new BOOT LED to blink and also set it to ON before
> entering main loop to confirm successful boot.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  common/board_r.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index d4ba245ac69..57957b4e99b 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -39,6 +39,7 @@
>  #include <initcall.h>
>  #include <kgdb.h>
>  #include <irq_func.h>
> +#include <led.h>
>  #include <malloc.h>
>  #include <mapmem.h>
>  #include <miiphy.h>
> @@ -462,14 +463,30 @@ static int initr_malloc_bootparams(void)
>  #if defined(CONFIG_LED_STATUS)
>  static int initr_status_led(void)
>  {
> -#if defined(CONFIG_LED_STATUS_BOOT)
> -       status_led_set(CONFIG_LED_STATUS_BOOT, CONFIG_LED_STATUS_BLINKING);
> -#else
>         status_led_init();
> +

Please can you rework this to avoid the #ifdefs in this C file? You
can make empty static inlines if needed... probably here you can move
this code to led.h and have the #ifdef there.

> +       return 0;
> +}
> +#endif
> +
> +static int initr_boot_led_blink(void)
> +{
> +#ifdef CONFIG_LED_STATUS_BOOT
> +       status_led_set(CONFIG_LED_STATUS_BOOT, CONFIG_LED_STATUS_BLINKING);
> +#endif
> +#ifdef CONFIG_LED_BOOT_ENABLE
> +       led_boot_blink();
>  #endif

Same here. Note it is OK to use if (IS_ENABLED(...))  - just not the #ifdef

>         return 0;
>  }
> +
> +static int initr_boot_led_on(void)
> +{
> +#ifdef CONFIG_LED_BOOT_ENABLE
> +       led_boot_on();
>  #endif
> +       return 0;
> +}
>
>  #ifdef CONFIG_CMD_NET
>  static int initr_net(void)
> @@ -716,6 +733,7 @@ static init_fnc_t init_sequence_r[] = {
>  #if defined(CONFIG_LED_STATUS)
>         initr_status_led,
>  #endif
> +       initr_boot_led_blink,
>         /* PPC has a udelay(20) here dating from 2002. Why? */
>  #ifdef CONFIG_BOARD_LATE_INIT
>         board_late_init,
> @@ -738,6 +756,7 @@ static init_fnc_t init_sequence_r[] = {
>  #if defined(CFG_PRAM)
>         initr_mem,
>  #endif
> +       initr_boot_led_on,
>         run_main_loop,
>  };
>
> --
> 2.45.2
>

Regards,
Simon
diff mbox series

Patch

diff --git a/common/board_r.c b/common/board_r.c
index d4ba245ac69..57957b4e99b 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -39,6 +39,7 @@ 
 #include <initcall.h>
 #include <kgdb.h>
 #include <irq_func.h>
+#include <led.h>
 #include <malloc.h>
 #include <mapmem.h>
 #include <miiphy.h>
@@ -462,14 +463,30 @@  static int initr_malloc_bootparams(void)
 #if defined(CONFIG_LED_STATUS)
 static int initr_status_led(void)
 {
-#if defined(CONFIG_LED_STATUS_BOOT)
-	status_led_set(CONFIG_LED_STATUS_BOOT, CONFIG_LED_STATUS_BLINKING);
-#else
 	status_led_init();
+
+	return 0;
+}
+#endif
+
+static int initr_boot_led_blink(void)
+{
+#ifdef CONFIG_LED_STATUS_BOOT
+	status_led_set(CONFIG_LED_STATUS_BOOT, CONFIG_LED_STATUS_BLINKING);
+#endif
+#ifdef CONFIG_LED_BOOT_ENABLE
+	led_boot_blink();
 #endif
 	return 0;
 }
+
+static int initr_boot_led_on(void)
+{
+#ifdef CONFIG_LED_BOOT_ENABLE
+	led_boot_on();
 #endif
+	return 0;
+}
 
 #ifdef CONFIG_CMD_NET
 static int initr_net(void)
@@ -716,6 +733,7 @@  static init_fnc_t init_sequence_r[] = {
 #if defined(CONFIG_LED_STATUS)
 	initr_status_led,
 #endif
+	initr_boot_led_blink,
 	/* PPC has a udelay(20) here dating from 2002. Why? */
 #ifdef CONFIG_BOARD_LATE_INIT
 	board_late_init,
@@ -738,6 +756,7 @@  static init_fnc_t init_sequence_r[] = {
 #if defined(CFG_PRAM)
 	initr_mem,
 #endif
+	initr_boot_led_on,
 	run_main_loop,
 };