[PATCH v3 0/9] misc: introduce STATUS LED activity function

This series expand the STATUS LED framework with a new color and a big new feature. One thing that many device need is a way to communicate to the user that the device is actually doing something.
This is especially useful for recovery steps where an user (for example) insert an USB drive, keep a button pressed and the device autorecover.
There is currently no way to signal the user externally that the bootloader is processing/recoverying aside from setting a LED on.
A solid LED on is not enough and won't actually signal any kind of progress. Solution is the good old blinking LED but uboot doesn't suggest (and support) interrupts and almost all the LED are usually GPIO LED that doesn't support HW blink.
To fix this and handle the problem of device not supporting HW blink, expand the STATUS LED framework with new API.
We introduce a new config LED_STATUS_ACTIVITY, that similar to the RED, GREEN and others, takes the LED ID set in the LED_STATUS config and is used as the global LED for activity operations.
We add status_led_activity_start/stop() used to turn the activity LED on and register a cyclic to make it blink. LED will continue to blink until _stop() is called.
Function that will start some kind of action will first call _start() and then at the end will call stop(). If (on the broken implementation) a _start() is called before calling a _stop(), the Cyclic is first unregister and is registered again.
Support for this is initially added to the tftp, mtd and ubi command.
This also contains a big fixup for the gpio_led driver that currently deviates from the Documentation and make the coloured status led feature unusable.
Changes v3: - Add log on Status LED error conditions Changes v2: - Add Documentation conversion - Rework to Cyclic and simplify implementation
Christian Marangi (9): misc: gpio_led: fix broken coloured LED status functions led: status_led: add support for white LED colour led: status_led: add function to toggle a status LED led: status_led: add warning when an invalid Status LED ID is used led: status_led: add new activity LED config and functions tftp: implement support for LED status activity mtd: implement support for LED status activity ubi: implement support for LED status activity doc: convert README.LED to .rst documentation
cmd/legacy_led.c | 6 + cmd/mtd.c | 19 ++++ cmd/ubi.c | 15 ++- common/board_f.c | 2 + doc/README.LED | 77 ------------- doc/api/index.rst | 1 + doc/api/status_led.rst | 35 ++++++ drivers/led/Kconfig | 30 +++++ drivers/misc/gpio_led.c | 41 +++++-- drivers/misc/status_led.c | 75 ++++++++++++- include/status_led.h | 231 +++++++++++++++++++++++++++++++++++++- net/tftp.c | 7 ++ 12 files changed, 440 insertions(+), 99 deletions(-) delete mode 100644 doc/README.LED create mode 100644 doc/api/status_led.rst

The GPIO LED driver is a backend to provide LED status functions via the GPIO common functions.
The coloured LED functions are currently broken and deviates from what is written in README.LED
Quoting the README.LED:
CONFIG_STATUS_LED_RED is the red LED. It is used to signal errors. This must be a valid LED number (0-5). Other similar color LED's macros are CONFIG_STATUS_LED_GREEN, CONFIG_STATUS_LED_YELLOW and CONFIG_STATUS_LED_BLUE.
Hence the value of the config must refer to the ID.
Currently this is not the case and the driver expect the GPIO ID to be put in these config. On top of this to actually have these functions, a never define in Kconfig config must be declared to actually compile them. (CONFIG_GPIO_LED_STUBS)
To fix this and the GPIO problem, introduce some define that reference the LED_STATUS_BIT<n> config to have the ID->BIT connection (as described in Docs) and drop the never defined config.
The gpio_led already provide a wrapper to the functions and should not be enabled if the board provide their custom function hence it's ok to also provide the wrapper for the colour functions.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- drivers/misc/gpio_led.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/misc/gpio_led.c b/drivers/misc/gpio_led.c index 30679f80cf1..0f3682e1465 100644 --- a/drivers/misc/gpio_led.c +++ b/drivers/misc/gpio_led.c @@ -52,56 +52,63 @@ void __led_toggle(led_id_t mask) gpio_set_value(mask, !gpio_get_value(mask)); }
-#ifdef CONFIG_GPIO_LED_STUBS - /* 'generic' override of colored LED stubs, to use GPIO functions instead */
+/* We support up to 6 LEDs, LED 0 STATUS BIT doesn't have the number suffix */ +#define GPIO_BIT0 CONFIG_LED_STATUS_BIT +#define GPIO_BIT1 CONFIG_LED_STATUS_BIT1 +#define GPIO_BIT2 CONFIG_LED_STATUS_BIT2 +#define GPIO_BIT3 CONFIG_LED_STATUS_BIT3 +#define GPIO_BIT4 CONFIG_LED_STATUS_BIT4 +#define GPIO_BIT5 CONFIG_LED_STATUS_BIT5 +/* C preprocessor magic way to generate a GPIO_LED<id> reference */ +#define GPIO_BIT(id) ___PASTE(GPIO_BIT, id) + #ifdef CONFIG_LED_STATUS_RED + void red_led_on(void) { - __led_set(CONFIG_LED_STATUS_RED, CONFIG_LED_STATUS_ON); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_RED), CONFIG_LED_STATUS_ON); }
void red_led_off(void) { - __led_set(CONFIG_LED_STATUS_RED, CONFIG_LED_STATUS_OFF); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_RED), CONFIG_LED_STATUS_OFF); } #endif
#ifdef CONFIG_LED_STATUS_GREEN void green_led_on(void) { - __led_set(CONFIG_LED_STATUS_GREEN, CONFIG_LED_STATUS_ON); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_GREEN), CONFIG_LED_STATUS_ON); }
void green_led_off(void) { - __led_set(CONFIG_LED_STATUS_GREEN, CONFIG_LED_STATUS_OFF); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_GREEN), CONFIG_LED_STATUS_OFF); } #endif
#ifdef CONFIG_LED_STATUS_YELLOW void yellow_led_on(void) { - __led_set(CONFIG_LED_STATUS_YELLOW, CONFIG_LED_STATUS_ON); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_YELLOW), CONFIG_LED_STATUS_ON); }
void yellow_led_off(void) { - __led_set(CONFIG_LED_STATUS_YELLOW, CONFIG_LED_STATUS_OFF); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_YELLOW), CONFIG_LED_STATUS_OFF); } #endif
#ifdef CONFIG_LED_STATUS_BLUE void blue_led_on(void) { - __led_set(CONFIG_LED_STATUS_BLUE, CONFIG_LED_STATUS_ON); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_BLUE), CONFIG_LED_STATUS_ON); }
void blue_led_off(void) { - __led_set(CONFIG_LED_STATUS_BLUE, CONFIG_LED_STATUS_OFF); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_BLUE), CONFIG_LED_STATUS_OFF); } #endif - -#endif /* CONFIG_GPIO_LED_STUBS */

Add support for white LED colour present on many devices.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- cmd/legacy_led.c | 6 ++++++ common/board_f.c | 2 ++ drivers/led/Kconfig | 14 ++++++++++++++ drivers/misc/gpio_led.c | 12 ++++++++++++ include/status_led.h | 4 ++++ 5 files changed, 38 insertions(+)
diff --git a/cmd/legacy_led.c b/cmd/legacy_led.c index 5256255f052..40dbc05a651 100644 --- a/cmd/legacy_led.c +++ b/cmd/legacy_led.c @@ -57,6 +57,9 @@ static const led_tbl_t led_commands[] = { #endif #ifdef CONFIG_LED_STATUS_BLUE { "blue", CONFIG_LED_STATUS_BLUE, blue_led_off, blue_led_on, NULL }, +#endif +#ifdef CONFIG_LED_STATUS_WHITE + { "white", CONFIG_LED_STATUS_WHITE, white_led_off, white_led_on, NULL }, #endif { NULL, 0, NULL, NULL, NULL } }; @@ -180,6 +183,9 @@ U_BOOT_CMD( #endif #ifdef CONFIG_LED_STATUS_BLUE "blue|" +#endif +#ifdef CONFIG_LED_STATUS_WHITE + "white|" #endif "all] [on|off|toggle|blink] [blink-freq in ms]", "[led_name] [on|off|toggle|blink] sets or clears led(s)" diff --git a/common/board_f.c b/common/board_f.c index 039d6d712d0..54e2009339e 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -72,6 +72,8 @@ __weak void yellow_led_on(void) {} __weak void yellow_led_off(void) {} __weak void blue_led_on(void) {} __weak void blue_led_off(void) {} +__weak void white_led_on(void) {} +__weak void white_led_off(void) {}
/* * Why is gd allocated a register? Prior to reloc it might be better to diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig index 9837960198d..6c4f02d71f2 100644 --- a/drivers/led/Kconfig +++ b/drivers/led/Kconfig @@ -415,6 +415,20 @@ config LED_STATUS_GREEN
endif # LED_STATUS_GREEN_ENABLE
+config LED_STATUS_WHITE_ENABLE + bool "Enable white LED" + help + Enable white status LED. + +if LED_STATUS_WHITE_ENABLE + +config LED_STATUS_WHITE + int "White LED identification" + help + Valid enabled LED device number (0-5). + +endif # LED_STATUS_WHITE_ENABLE + config LED_STATUS_CMD bool "Enable status LED commands"
diff --git a/drivers/misc/gpio_led.c b/drivers/misc/gpio_led.c index 0f3682e1465..de84c206b6b 100644 --- a/drivers/misc/gpio_led.c +++ b/drivers/misc/gpio_led.c @@ -112,3 +112,15 @@ void blue_led_off(void) __led_set(GPIO_BIT(CONFIG_LED_STATUS_BLUE), CONFIG_LED_STATUS_OFF); } #endif + +#ifdef CONFIG_LED_STATUS_WHITE +void white_led_on(void) +{ + __led_set(GPIO_BIT(CONFIG_LED_STATUS_WHITE), CONFIG_LED_STATUS_ON); +} + +void white_led_off(void) +{ + __led_set(GPIO_BIT(CONFIG_LED_STATUS_WHITE), CONFIG_LED_STATUS_OFF); +} +#endif diff --git a/include/status_led.h b/include/status_led.h index 6707ab1d29d..5ce4522b029 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -87,6 +87,8 @@ void yellow_led_on(void); void yellow_led_off(void); void blue_led_on(void); void blue_led_off(void); +void white_led_on(void); +void white_led_off(void); #else .extern LED_init .extern red_led_on @@ -97,6 +99,8 @@ void blue_led_off(void); .extern green_led_off .extern blue_led_on .extern blue_led_off + .extern white_led_on + .extern white_led_off #endif
#endif /* _STATUS_LED_H_ */

Add function to toggle a status LED by using the status LED ID reference configs.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- drivers/misc/status_led.c | 28 +++++++++++++++++++++++----- include/status_led.h | 1 + 2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c index a6e9c03a02e..93bfb410662 100644 --- a/drivers/misc/status_led.c +++ b/drivers/misc/status_led.c @@ -103,17 +103,24 @@ void status_led_tick(ulong timestamp) } }
-void status_led_set(int led, int state) +static led_dev_t *status_get_led_dev(int led) { - led_dev_t *ld; - if (led < 0 || led >= MAX_LED_DEV) - return; + return NULL;
if (!status_led_init_done) status_led_init();
- ld = &led_dev[led]; + return &led_dev[led]; +} + +void status_led_set(int led, int state) +{ + led_dev_t *ld; + + ld = status_get_led_dev(led); + if (!ld) + return;
ld->state = state; if (state == CONFIG_LED_STATUS_BLINKING) { @@ -122,3 +129,14 @@ void status_led_set(int led, int state) } __led_set (ld->mask, state); } + +void status_led_toggle(int led) +{ + led_dev_t *ld; + + ld = status_get_led_dev(led); + if (!ld) + return; + + __led_toggle(ld->mask); +} diff --git a/include/status_led.h b/include/status_led.h index 5ce4522b029..fe0c84fb4b4 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -38,6 +38,7 @@ void status_led_init(void); void status_led_tick(unsigned long timestamp); void status_led_set(int led, int state); +void status_led_toggle(int led);
/***** MVS v1 **********************************************************/ #if (defined(CONFIG_MVS) && CONFIG_MVS < 2)

Add a warning when an invalid Status LED ID is used to make the user aware of bad configurations.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- drivers/misc/status_led.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c index 93bfb410662..882273cc322 100644 --- a/drivers/misc/status_led.c +++ b/drivers/misc/status_led.c @@ -105,8 +105,10 @@ void status_led_tick(ulong timestamp)
static led_dev_t *status_get_led_dev(int led) { - if (led < 0 || led >= MAX_LED_DEV) + if (led < 0 || led >= MAX_LED_DEV) { + printf("Invalid Status LED ID %d.", led) return NULL; + }
if (!status_led_init_done) status_led_init();

Add a new activity LED config and additional functions to implement a simple software blink feature to signal activity of any kind.
Usual activity might be a file transfer with TFTP, a flash write...
User of this API will call status_led_activity_start/stop() on each activity and LED will be toggled based on the defined FREQ config value.
With this enabled, cyclic API are also enabled as this makes use of cyclic API to handle LED blink.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- drivers/led/Kconfig | 16 +++++++++++++++ drivers/misc/status_led.c | 43 +++++++++++++++++++++++++++++++++++++++ include/status_led.h | 2 ++ 3 files changed, 61 insertions(+)
diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig index 6c4f02d71f2..869ed78e87f 100644 --- a/drivers/led/Kconfig +++ b/drivers/led/Kconfig @@ -359,6 +359,22 @@ config LED_STATUS_BOOT
endif # LED_STATUS_BOOT_ENABLE
+config LED_STATUS_ACTIVITY_ENABLE + bool "Enable BOOT LED" + select CYCLIC + help + Enable to turn an LED on when the board is doing some + activity (flash write, file download). + +if LED_STATUS_ACTIVITY_ENABLE + +config LED_STATUS_ACTIVITY + int "LED to light when the board is doing some activity" + help + Valid enabled LED device number. + +endif # LED_STATUS_ACTIVITY_ENABLE + config LED_STATUS_RED_ENABLE bool "Enable red LED" help diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c index 882273cc322..6d5207508d8 100644 --- a/drivers/misc/status_led.c +++ b/drivers/misc/status_led.c @@ -5,6 +5,7 @@ */
#include <common.h> +#include <cyclic.h> #include <status_led.h>
/* @@ -23,6 +24,7 @@ typedef struct { int state; int period; int cnt; + struct cyclic_info *cyclic; } led_dev_t;
led_dev_t led_dev[] = { @@ -142,3 +144,44 @@ void status_led_toggle(int led)
__led_toggle(ld->mask); } + +static void status_led_activity_toggle(void *ctx) +{ + __led_toggle(*(led_id_t *)ctx); +} + +void status_led_activity_start(int led) +{ + led_dev_t *ld; + + ld = status_get_led_dev(led); + if (!ld) + return; + + if (ld->cyclic) { + printf("Cyclic for activity status LED %d already registered. THIS IS AN ERROR.\n", + led); + cyclic_unregister(ld->cyclic); + } + + status_led_set(led, CONFIG_LED_STATUS_BLINKING); + + ld->cyclic = cyclic_register(status_led_activity_toggle, + ld->period * 500, "activity", &ld->mask); + if (!ld->cyclic) + printf("Registering of cyclic function for activity status LED %d failed\n", + led); +} + +void status_led_activity_stop(int led) +{ + led_dev_t *ld; + + ld = status_get_led_dev(led); + if (!ld) + return; + + cyclic_unregister(ld->cyclic); + ld->cyclic = NULL + status_led_set(led, CONFIG_LED_STATUS_OFF); +} diff --git a/include/status_led.h b/include/status_led.h index fe0c84fb4b4..7de40551621 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -39,6 +39,8 @@ void status_led_init(void); void status_led_tick(unsigned long timestamp); void status_led_set(int led, int state); void status_led_toggle(int led); +void status_led_activity_start(int led); +void status_led_activity_stop(int led);
/***** MVS v1 **********************************************************/ #if (defined(CONFIG_MVS) && CONFIG_MVS < 2)

Implement support for LED status activity. If the feature is enabled, make the defined ACTIVITY LED to signal traffic.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- net/tftp.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/net/tftp.c b/net/tftp.c index 2e335413492..0515dc34d4b 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -19,6 +19,7 @@ #include <asm/global_data.h> #include <net/tftp.h> #include "bootp.h" +#include <status_led.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -193,6 +194,9 @@ static void new_transfer(void) #ifdef CONFIG_CMD_TFTPPUT tftp_put_final_block_sent = 0; #endif +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_start(CONFIG_LED_STATUS_ACTIVITY); +#endif }
#ifdef CONFIG_CMD_TFTPPUT @@ -302,6 +306,9 @@ static void tftp_complete(void) time_start * 1000, "/s"); } puts("\ndone\n"); +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_stop(CONFIG_LED_STATUS_ACTIVITY); +#endif if (!tftp_put_active) efi_set_bootdev("Net", "", tftp_filename, map_sysmem(tftp_load_addr, 0),

Implement support for LED status activity. If the feature is enabled, make the defined ACTIVITY LED to signal mtd write or erase operations.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- cmd/mtd.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 9189f45cabd..a10805d05a5 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -11,6 +11,7 @@ #include <command.h> #include <common.h> #include <console.h> +#include <status_led.h> #if CONFIG_IS_ENABLED(CMD_MTD_OTP) #include <hexdump.h> #endif @@ -559,6 +560,11 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc, while (mtd_block_isbad(mtd, off)) off += mtd->erasesize;
+#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + if (!read) + status_led_activity_start(CONFIG_LED_STATUS_ACTIVITY); +#endif + /* Loop over the pages to do the actual read/write */ while (remaining) { /* Skip the block if it is bad */ @@ -586,6 +592,11 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc, io_op.oobbuf += io_op.oobretlen; }
+#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + if (!read) + status_led_activity_stop(CONFIG_LED_STATUS_ACTIVITY); +#endif + if (!ret && dump) mtd_dump_device_buf(mtd, start_off, buf, len, woob);
@@ -653,6 +664,10 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc, erase_op.addr = off; erase_op.len = mtd->erasesize;
+#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_start(CONFIG_LED_STATUS_ACTIVITY); +#endif + while (len) { if (!scrub) { ret = mtd_block_isbad(mtd, erase_op.addr); @@ -681,6 +696,10 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc, erase_op.addr += mtd->erasesize; }
+#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_stop(CONFIG_LED_STATUS_ACTIVITY); +#endif + if (ret && ret != -EIO) ret = CMD_RET_FAILURE; else

Implement support for LED status activity. If the feature is enabled, make the defined ACTIVITY LED to signal ubi write operation.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- cmd/ubi.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/cmd/ubi.c b/cmd/ubi.c index 0a6a80bdd10..37c16048b19 100644 --- a/cmd/ubi.c +++ b/cmd/ubi.c @@ -20,6 +20,7 @@ #include <mtd.h> #include <nand.h> #include <onenand_uboot.h> +#include <status_led.h> #include <dm/devres.h> #include <linux/mtd/mtd.h> #include <linux/mtd/partitions.h> @@ -417,7 +418,19 @@ int ubi_volume_begin_write(char *volume, void *buf, size_t size,
int ubi_volume_write(char *volume, void *buf, size_t size) { - return ubi_volume_begin_write(volume, buf, size, size); + int ret; + +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_start(CONFIG_LED_STATUS_ACTIVITY); +#endif + + ret = ubi_volume_begin_write(volume, buf, size, size); + +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_stop(CONFIG_LED_STATUS_ACTIVITY); +#endif + + return ret; }
int ubi_volume_read(char *volume, char *buf, size_t size)

Convert README.LED to .rst documentation and include all the relevant documentation in the status_led.h.
Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- doc/README.LED | 77 -------------- doc/api/index.rst | 1 + doc/api/status_led.rst | 35 +++++++ include/status_led.h | 224 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 256 insertions(+), 81 deletions(-) delete mode 100644 doc/README.LED create mode 100644 doc/api/status_led.rst
diff --git a/doc/README.LED b/doc/README.LED deleted file mode 100644 index c21c9d53ec3..00000000000 --- a/doc/README.LED +++ /dev/null @@ -1,77 +0,0 @@ -Status LED -======================================== - -This README describes the status LED API. - -The API is defined by the include file include/status_led.h - -The first step is to enable CONFIG_LED_STATUS in menuconfig: -> Device Drivers > LED Support. - -If the LED support is only for specific board, enable -CONFIG_LED_STATUS_BOARD_SPECIFIC in the menuconfig. - -Status LEDS 0 to 5 are enabled by the following configurations at menuconfig: -CONFIG_STATUS_LED0, CONFIG_STATUS_LED1, ... CONFIG_STATUS_LED5 - -The following should be configured for each of the enabled LEDs: -CONFIG_STATUS_LED_BIT<n> -CONFIG_STATUS_LED_STATE<n> -CONFIG_STATUS_LED_FREQ<n> -Where <n> is an integer 1 through 5 (empty for 0). - -CONFIG_STATUS_LED_BIT is passed into the __led_* functions to identify which LED -is being acted on. As such, the value choose must be unique with with respect to -the other CONFIG_STATUS_LED_BIT's. Mapping the value to a physical LED is the -reponsiblity of the __led_* function. - -CONFIG_STATUS_LED_STATE is the initial state of the LED. It should be set to one -of these values: CONFIG_LED_STATUS_OFF or CONFIG_LED_STATUS_ON. - -CONFIG_STATUS_LED_FREQ determines the LED blink frequency. -Values range from 2 to 10. - -Some other LED macros ---------------------- - -CONFIG_STATUS_LED_BOOT is the LED to light when the board is booting. -This must be a valid LED number (0-5). - -CONFIG_STATUS_LED_RED is the red LED. It is used to signal errors. This must be -a valid LED number (0-5). Other similar color LED's macros are -CONFIG_STATUS_LED_GREEN, CONFIG_STATUS_LED_YELLOW and CONFIG_STATUS_LED_BLUE. - -General LED functions ---------------------- -The following functions should be defined: - -__led_init is called once to initialize the LED to CONFIG_STATUS_LED_STATE. -One time start up code should be placed here. - -__led_set is called to change the state of the LED. - -__led_toggle is called to toggle the current state of the LED. - -Colour LED -======================================== - -Colour LED's are at present only used by ARM. - -The functions names explain their purpose. - -coloured_LED_init -red_LED_on -red_LED_off -green_LED_on -green_LED_off -yellow_LED_on -yellow_LED_off -blue_LED_on -blue_LED_off - -These are weakly defined in arch/arm/lib/board.c to noops. Where applicable, define -these functions in the board specific source. - -TBD : Describe older board dependent macros similar to what is done for - -TBD : Describe general support via asm/status_led.h diff --git a/doc/api/index.rst b/doc/api/index.rst index 51b2013af36..d40e90801d1 100644 --- a/doc/api/index.rst +++ b/doc/api/index.rst @@ -22,6 +22,7 @@ U-Boot API documentation rng sandbox serial + status_led sysreset timer unicode diff --git a/doc/api/status_led.rst b/doc/api/status_led.rst new file mode 100644 index 00000000000..44bbea47157 --- /dev/null +++ b/doc/api/status_led.rst @@ -0,0 +1,35 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Status LED +========== + +.. kernel-doc:: include/status_led.h + :doc: Overview + +CONFIG_STATUS_LED Description +----------------------------- + +.. kernel-doc:: include/status_led.h + :doc: CONFIG Description + +Special Status LED Configs +-------------------------- +.. kernel-doc:: include/status_led.h + :doc: LED Status Config + +Colour Status LED Configs +------------------------- +.. kernel-doc:: include/status_led.h + :doc: LED Colour Config + +Required API +------------ + +.. kernel-doc:: include/status_led.h + :doc: Required API + +Status LED API +-------------- + +.. kernel-doc:: include/status_led.h + :internal: diff --git a/include/status_led.h b/include/status_led.h index 7de40551621..6893d1d68e0 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -4,18 +4,102 @@ * Wolfgang Denk, DENX Software Engineering, wd@denx.de. */
-/* - * The purpose of this code is to signal the operational status of a +/** + * DOC: Overview + * + * Status LED is a Low-Level way to handle LEDs to signal state of the + * bootloader, for example boot progress, file transfer fail, activity + * of some sort like tftp transfer, mtd write/erase. + * + * The original usage these API were to signal the operational status of a * target which usually boots over the network; while running in * PCBoot, a status LED is blinking. As soon as a valid BOOTP reply * message has been received, the LED is turned off. The Linux * kernel, once it is running, will start blinking the LED again, * with another frequency. + * + * Status LED require support Low Level and the board to implement + * the specific functions to correctly works. */
#ifndef _STATUS_LED_H_ #define _STATUS_LED_H_
+/** + * DOC: CONFIG Description + * + * Enable `CONFIG_LED_STATUS` to support the Status LED under + * > Device Drivers > LED Support. + * + * The Status LED can be defined in various ways, but most of the time, + * specific functions will need to be defined in the board file. + * If this is the case, enable `CONFIG_LED_STATUS_BOARD_SPECIFIC`. + * + * If the LEDs are GPIO-driven, you can use the GPIO wrapper driver + * instead of defining specific board functions. + * If this is the case, enable `CONFIG_LED_STATUS_GPIO`. + * (Note that `CONFIG_LED_STATUS_BOARD_SPECIFIC` is also required.) + * + * The Status LED allows defining up to six different LEDs, from 0 to 5, + * with the following configurations: + * `CONFIG_STATUS_LED`, `CONFIG_STATUS_LED1`, ..., `CONFIG_STATUS_LED5`. + * + * For each LED, the following options are required: + * * `CONFIG_STATUS_LED_BIT<n>` + * * `CONFIG_STATUS_LED_STATE<n>` + * * `CONFIG_STATUS_LED_FREQ<n>` + * + * Where `<n>` is an integer from 1 through 5. (Note that LED 0 doesn't have the + * integer suffix.) + * + * `CONFIG_STATUS_LED_BIT` is passed into the `__led_*` functions to identify + * which LED is being acted on. The value is opaque, meaning it depends on how + * the low-level API handles this value. It can be an ID to reference the LED + * internally, or in the case of the GPIO wrapper, it's the GPIO number of the LED. + * Mapping the value to a physical LED is the responsibility of the `__led_*` function. + * + * `CONFIG_STATUS_LED_STATE` sets the initial state of the LED. It should be set + * to one of these values: `CONFIG_LED_STATUS_OFF` or `CONFIG_LED_STATUS_ON`. + * + * `CONFIG_STATUS_LED_FREQ` determines the LED blink frequency. + * Values range from 2 to 10. + */ +/** + * DOC: LED Status Config + * + * The Status LED uses two special configurations for common operations: + * + * * CONFIG_STATUS_LED_BOOT is the LED that lights up when the board is booting. + * * CONFIG_STATUS_LED_ACTIVITY is the LED that lights and blinks during activity + * (e.g., file transfer, flash write). + * + * The values set in these configurations refer to the LED ID previously set. + * + * - ``CONFIG_STATUS_LED_RED=0`` will refer to the option ``CONFIG_STATUS_LED_BIT``. + * - ``CONFIG_STATUS_LED_RED=1`` will refer to the option ``CONFIG_STATUS_LED_BIT1``. + * - ``CONFIG_STATUS_LED_RED=2`` will refer to the option ``CONFIG_STATUS_LED_BIT2``. + * - ... + */ +/** + * DOC: LED Colour Config + * + * The Status LED exposes specific configurations for LEDs of different colors. + * + * The values set in these configurations refer to the LED ID previously set. + * + * - ``CONFIG_STATUS_LED_RED=0`` will refer to the option ``CONFIG_STATUS_LED_BIT``. + * - ``CONFIG_STATUS_LED_RED=1`` will refer to the option ``CONFIG_STATUS_LED_BIT1``. + * - ``CONFIG_STATUS_LED_RED=2`` will refer to the option ``CONFIG_STATUS_LED_BIT2``. + * - ... + * + * Supported colors are: + * * red + * * green + * * yellow + * * blue + * * white + */ + #ifdef CONFIG_LED_STATUS
#define LED_STATUS_PERIOD (CONFIG_SYS_HZ / CONFIG_LED_STATUS_FREQ) @@ -35,11 +119,49 @@ #define LED_STATUS_PERIOD5 (CONFIG_SYS_HZ / CONFIG_LED_STATUS_FREQ5) #endif /* CONFIG_LED_STATUS5 */
+/** + * void status_led_init - Init the Status LED with all the required structs. + */ void status_led_init(void); +/** + * void status_led_tick - Blink each LED that is currently set in blinking + * mode + * @timestamp: currently unused + */ void status_led_tick(unsigned long timestamp); +/** + * void status_led_set - Set the LED ID passed as first arg to the mode set + * @led: reference to the Status LED ID + * @state: state to set the LED to + * + * Modes for state arE: + * * CONFIG_LED_STATUS_OFF (LED off) + * * CONFIG_LED_STATUS_ON (LED on) + * * CONFIG_LED_STATUS_BLINK (LED initially on, expected to blink) + */ void status_led_set(int led, int state); +/** + * void status_led_toggle - toggle the LED ID + * @led: reference to the Status LED ID + * + * Toggle the LED ID passed as first arg. If it's ON became OFF, if it's + * OFF became ON. + */ void status_led_toggle(int led); +/** + * void status_led_activity_start - start a LED activity + * @led: reference to the Status LED ID + * + * Set the Status LED ON and start the Cyclic to make the LED blink at + * the configured freq. + */ void status_led_activity_start(int led); +/** + * void status_led_activity_stop - stop a LED activity + * @led: reference to the Status LED ID + * + * Stop and free the Cyclic and turn the LED OFF. + */ void status_led_activity_stop(int led);
/***** MVS v1 **********************************************************/ @@ -62,9 +184,61 @@ void status_led_activity_stop(int led); /* led_id_t is unsigned long mask */ typedef unsigned long led_id_t;
+/** + * DOC: Required API + * + * The Status LED requires the following API functions to operate correctly + * and compile: + * + * - ``__led_toggle``: Low-level function to toggle the LED at the specified + * mask. + * - ``__led_init``: Initializes the Status LED, sets up required tables, and + * configures registers. + * - ``__led_set``: Low-level function to set the state of the LED at the + * specified mask. + * - ``__led_blink``: Low-level function to set the LED to blink at the + * specified frequency. + * + * The Status LED also provides optional functions to control colored LEDs. + * Each supported LED color has corresponding ``_on`` and ``_off`` functions. + * + * There is also support for ``coloured_LED_init`` for LEDs that provide + * multiple colors. (Currently, this is only used by ARM.) + * + * Each function is weakly defined and should be implemented in the + * board-specific source file. (This does not apply to the GPIO LED wrapper.) + */ +/** + * void __led_toggle - toggle LED at @mask + * @mask: opaque value to reference the LED + * + * Low-Level function to toggle the LED at mask. + */ extern void __led_toggle (led_id_t mask); +/** + * void __led_init - Init the LED at @mask + * @mask: opaque value to reference the LED + * @state: starting state of the LED + * + * Init the Status LED, init required tables, setup regs... + */ extern void __led_init (led_id_t mask, int state); +/** + * void __led_set - Set the LED at @mask + * @mask: opaque value to reference the LED + * @state: state to set the LED to + * + * Init the Status LED, init required tables, setup regs... + */ extern void __led_set (led_id_t mask, int state); +/** + * void __led_blink - Set the LED at @mask to HW blink + * @mask: opaque value to reference the LED + * @freq: freq to make the LED blink at + * + * Low-Level function to set the LED at HW blink by the + * passed freq. + */ void __led_blink(led_id_t mask, int freq); #else # error Status LED configuration missing @@ -77,20 +251,62 @@ void __led_blink(led_id_t mask, int freq);
#endif /* CONFIG_LED_STATUS */
-/* - * Coloured LEDs API +/** + * DOC: Coloured LED API + * + * Status LED expose optional functions to control coloured LED. + * Each LED color supported expose _on and _off function. + * + * There is also support for coloured_LED_init for LED that provide + * multiple colours. (currently only used by ARM) + * + * Each function is weakly defined and should be defined in the board + * specific source. (doesn't apply for GPIO LED wrapper) */ #ifndef __ASSEMBLY__ +/** + * void coloured_LED_init - Init multi colour LED + */ void coloured_LED_init(void); +/** + * void red_led_on - Turn LED Red on + */ void red_led_on(void); +/** + * void red_led_off - Turn LED Red off + */ void red_led_off(void); +/** + * void green_led_on - Turn LED Green on + */ void green_led_on(void); +/** + * void green_led_off - Turn LED Green off + */ void green_led_off(void); +/** + * void yellow_led_on - Turn LED Yellow on + */ void yellow_led_on(void); +/** + * void yellow_led_off - Turn LED Yellow off + */ void yellow_led_off(void); +/** + * void blue_led_on - Turn LED Blue on + */ void blue_led_on(void); +/** + * void blue_led_off - Turn LED Blue off + */ void blue_led_off(void); +/** + * void white_led_on - Turn LED White on + */ void white_led_on(void); +/** + * void white_led_off - Turn LED White off + */ void white_led_off(void); #else .extern LED_init

On Wed, Jun 12, 2024 at 01:14:15AM +0200, Christian Marangi wrote:
This series expand the STATUS LED framework with a new color and a big new feature. One thing that many device need is a way to communicate to the user that the device is actually doing something.
This is especially useful for recovery steps where an user (for example) insert an USB drive, keep a button pressed and the device autorecover.
There is currently no way to signal the user externally that the bootloader is processing/recoverying aside from setting a LED on.
A solid LED on is not enough and won't actually signal any kind of progress. Solution is the good old blinking LED but uboot doesn't suggest (and support) interrupts and almost all the LED are usually GPIO LED that doesn't support HW blink.
To fix this and handle the problem of device not supporting HW blink, expand the STATUS LED framework with new API.
We introduce a new config LED_STATUS_ACTIVITY, that similar to the RED, GREEN and others, takes the LED ID set in the LED_STATUS config and is used as the global LED for activity operations.
We add status_led_activity_start/stop() used to turn the activity LED on and register a cyclic to make it blink. LED will continue to blink until _stop() is called.
Function that will start some kind of action will first call _start() and then at the end will call stop(). If (on the broken implementation) a _start() is called before calling a _stop(), the Cyclic is first unregister and is registered again.
Support for this is initially added to the tftp, mtd and ubi command.
This also contains a big fixup for the gpio_led driver that currently deviates from the Documentation and make the coloured status led feature unusable.
A number of platforms now fail to build: https://source.denx.de/u-boot/u-boot/-/jobs/854228#L1455 and please rebase on top of current next as well. To make world build testing easier you may wish to read over https://docs.u-boot.org/en/latest/develop/ci_testing.html but please note that there are sometimes spurious failures due to the cyclic framework and applying https://patchwork.ozlabs.org/project/uboot/patch/20240524210817.1953298-1-ra... temporarily is likely the quickest path forward at this time.

On Wed, Jun 19, 2024 at 08:44:21AM -0600, Tom Rini wrote:
On Wed, Jun 12, 2024 at 01:14:15AM +0200, Christian Marangi wrote:
This series expand the STATUS LED framework with a new color and a big new feature. One thing that many device need is a way to communicate to the user that the device is actually doing something.
This is especially useful for recovery steps where an user (for example) insert an USB drive, keep a button pressed and the device autorecover.
There is currently no way to signal the user externally that the bootloader is processing/recoverying aside from setting a LED on.
A solid LED on is not enough and won't actually signal any kind of progress. Solution is the good old blinking LED but uboot doesn't suggest (and support) interrupts and almost all the LED are usually GPIO LED that doesn't support HW blink.
To fix this and handle the problem of device not supporting HW blink, expand the STATUS LED framework with new API.
We introduce a new config LED_STATUS_ACTIVITY, that similar to the RED, GREEN and others, takes the LED ID set in the LED_STATUS config and is used as the global LED for activity operations.
We add status_led_activity_start/stop() used to turn the activity LED on and register a cyclic to make it blink. LED will continue to blink until _stop() is called.
Function that will start some kind of action will first call _start() and then at the end will call stop(). If (on the broken implementation) a _start() is called before calling a _stop(), the Cyclic is first unregister and is registered again.
Support for this is initially added to the tftp, mtd and ubi command.
This also contains a big fixup for the gpio_led driver that currently deviates from the Documentation and make the coloured status led feature unusable.
A number of platforms now fail to build: https://source.denx.de/u-boot/u-boot/-/jobs/854228#L1455 and please rebase on top of current next as well. To make world build testing easier you may wish to read over https://docs.u-boot.org/en/latest/develop/ci_testing.html but please note that there are sometimes spurious failures due to the cyclic framework and applying https://patchwork.ozlabs.org/project/uboot/patch/20240524210817.1953298-1-ra... temporarily is likely the quickest path forward at this time.
Hi Tom, no idea I had to target next.
I just sent v4 (require moderation approval).
I rebased the thing and fixed the errors catched by the azure pipeline. Hope the gitlab tests doesn't have additional target that I might have missed.
I also had to use the suggested testing patch for the tests.

On Wed, 12 Jun 2024 at 03:33, Christian Marangi ansuelsmth@gmail.com wrote:
This series expand the STATUS LED framework with a new color and a big new feature. One thing that many device need is a way to communicate to the user that the device is actually doing something.
This is especially useful for recovery steps where an user (for example) insert an USB drive, keep a button pressed and the device autorecover.
There is currently no way to signal the user externally that the bootloader is processing/recoverying aside from setting a LED on.
A solid LED on is not enough and won't actually signal any kind of progress. Solution is the good old blinking LED but uboot doesn't suggest (and support) interrupts and almost all the LED are usually GPIO LED that doesn't support HW blink.
To fix this and handle the problem of device not supporting HW blink, expand the STATUS LED framework with new API.
We introduce a new config LED_STATUS_ACTIVITY, that similar to the RED, GREEN and others, takes the LED ID set in the LED_STATUS config and is used as the global LED for activity operations.
We add status_led_activity_start/stop() used to turn the activity LED on and register a cyclic to make it blink. LED will continue to blink until _stop() is called.
Function that will start some kind of action will first call _start() and then at the end will call stop(). If (on the broken implementation) a _start() is called before calling a _stop(), the Cyclic is first unregister and is registered again.
Support for this is initially added to the tftp, mtd and ubi command.
This also contains a big fixup for the gpio_led driver that currently deviates from the Documentation and make the coloured status led feature unusable.
Changes v3:
- Add log on Status LED error conditions
Changes v2:
- Add Documentation conversion
- Rework to Cyclic and simplify implementation
Christian Marangi (9): misc: gpio_led: fix broken coloured LED status functions led: status_led: add support for white LED colour led: status_led: add function to toggle a status LED led: status_led: add warning when an invalid Status LED ID is used led: status_led: add new activity LED config and functions tftp: implement support for LED status activity mtd: implement support for LED status activity ubi: implement support for LED status activity doc: convert README.LED to .rst documentation
cmd/legacy_led.c | 6 + cmd/mtd.c | 19 ++++ cmd/ubi.c | 15 ++- common/board_f.c | 2 + doc/README.LED | 77 ------------- doc/api/index.rst | 1 + doc/api/status_led.rst | 35 ++++++ drivers/led/Kconfig | 30 +++++ drivers/misc/gpio_led.c | 41 +++++--
Does it make sense to move misc/gpio_led.c to drivers/led as that's where the Kconfig is?
drivers/misc/status_led.c | 75 ++++++++++++- include/status_led.h | 231 +++++++++++++++++++++++++++++++++++++- net/tftp.c | 7 ++ 12 files changed, 440 insertions(+), 99 deletions(-) delete mode 100644 doc/README.LED create mode 100644 doc/api/status_led.rst
-- 2.43.0
participants (3)
-
Christian Marangi
-
Peter Robinson
-
Tom Rini