We currently init the LED OFF when SW blink is triggered when on_state_change() is called. This can be problematic for very short period as the ON/OFF blink might never trigger.

Toggle the LED (ON if OFF, OFF if ON) on initial SW blink to handle this corner case and better display a LED blink from the user.

Signed-off-by: Christian Marangi ansuelsmth@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- drivers/led/led_sw_blink.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/led/led_sw_blink.c b/drivers/led/led_sw_blink.c index 9e36edbee47..9d9820720c6 100644 --- a/drivers/led/led_sw_blink.c +++ b/drivers/led/led_sw_blink.c @@ -103,8 +103,16 @@ bool led_sw_on_state_change(struct udevice *dev, enum led_state_t state) return false;

if (state == LEDST_BLINK) { + struct led_ops *ops = led_get_ops(dev); + enum led_state_t curr_state = led_get_state(dev); + + curr_state = ops->get_state(dev); + /* toggle led initially */ + ops->set_state(dev, curr_state == LEDST_ON ? LEDST_OFF : + LEDST_ON); /* start blinking on next led_sw_blink() call */ - sw_blink->state = LED_SW_BLINK_ST_OFF; + sw_blink->state = curr_state == LEDST_ON ? LED_SW_BLINK_ST_OFF : + LED_SW_BLINK_ST_ON; return true; }

Rework BOOT LED handling. There is currently one legacy implementation for BOOT LED from Status Led API.

This work on ancient implementation used 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 | 28 ++++++++++++++++++++-------- include/status_led.h | 13 +++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c index d4ba245ac69..c3f8dd5d4ee 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> @@ -459,17 +460,28 @@ static int initr_malloc_bootparams(void) } #endif

-#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(); -#endif + + return 0; +} + +static int initr_boot_led_blink(void) +{ + status_led_boot_blink(); + + led_boot_blink(); + + return 0; +} + +static int initr_boot_led_on(void) +{ + led_boot_on(); + return 0; } -#endif

#ifdef CONFIG_CMD_NET static int initr_net(void) @@ -713,9 +725,8 @@ static init_fnc_t init_sequence_r[] = { #if defined(CONFIG_MICROBLAZE) || defined(CONFIG_M68K) timer_init, /* initialize timer */ #endif -#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 +749,7 @@ static init_fnc_t init_sequence_r[] = { #if defined(CFG_PRAM) initr_mem, #endif + initr_boot_led_on, run_main_loop, };

diff --git a/include/status_led.h b/include/status_led.h index 6707ab1d29d..1282022253e 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -39,6 +39,13 @@ void status_led_init(void); void status_led_tick(unsigned long timestamp); void status_led_set(int led, int state);

+static inline void status_led_boot_blink(void) +{ +#ifdef CONFIG_LED_STATUS_BOOT_ENABLE + status_led_set(CONFIG_LED_STATUS_BOOT, CONFIG_LED_STATUS_BLINKING); +#endif +} + /***** MVS v1 **********************************************************/ #if (defined(CONFIG_MVS) && CONFIG_MVS < 2) # define STATUS_LED_PAR im_ioport.iop_pdpar @@ -72,6 +79,12 @@ void __led_blink(led_id_t mask, int freq); # include <asm/status_led.h> #endif

+#else + +static inline void status_led_init(void) { } +static inline void status_led_set(int led, int state) { } +static inline void status_led_boot_blink(void) { } + #endif /* CONFIG_LED_STATUS */

/*

Implement LED activity API similar to BOOT LED API.

Usual activity might be a file transfer with TFTP, a flash write...

User of this API will call led_activity_on/off/blink() to signal these kind of activity.

New Kconfig is implemented similar to BOOT LED, LED_ACTIVITY to enable support for it.

It's introduced a new /options/u-boot property "activity-led" and "activity-led-period" to define the activity LED label and the default period when the activity LED is set to blink mode.

If "activity-led-period" is not defined, the value of 250 (ms) is used by default.

If CONFIG_LED_BLINK or CONFIG_LED_SW_BLINK is not enabled, led_boot_blink call will fallback to simple LED ON.

Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- drivers/led/Kconfig | 8 ++++ drivers/led/led-uclass.c | 94 ++++++++++++++++++++++++++++++++++++++-- include/led.h | 52 ++++++++++++++++++++++ 3 files changed, 151 insertions(+), 3 deletions(-)

diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig index 6149cfa02b8..f0434f247a4 100644 --- a/drivers/led/Kconfig +++ b/drivers/led/Kconfig @@ -16,6 +16,14 @@ config LED_BOOT

LED boot is a specific LED assigned to signal boot operation status.

+config LED_ACTIVITY + bool "Enable LED activity support" + help + Enable LED activity support. + + LED activity is a specific LED assigned to signal activity operation + like file trasnfer, flash write/erase... + config LED_BCM6328 bool "LED Support for BCM6328" depends on LED && ARCH_BMIPS diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c index c5b560982b0..c87fc7540c7 100644 --- a/drivers/led/led-uclass.c +++ b/drivers/led/led-uclass.c @@ -168,12 +168,86 @@ int led_boot_blink(void) #endif #endif

+#ifdef CONFIG_LED_ACTIVITY +int led_activity_on(void) +{ + struct uclass *uc = uclass_find(UCLASS_LED); + struct led_uc_priv *priv; + struct udevice *dev; + + if (!uc) + return -ENOENT; + + priv = uclass_get_priv(uc); + if (!priv->activity_led_dev || + uclass_get_device_tail(priv->activity_led_dev, 0, &dev)) { + printf("Failed to get activity LED %s\n", + priv->activity_led_label); + return -EINVAL; + } + + return led_set_state(dev, LEDST_ON); +} + +int led_activity_off(void) +{ + struct uclass *uc = uclass_find(UCLASS_LED); + struct led_uc_priv *priv; + struct udevice *dev; + + if (!uc) + return -ENOENT; + + priv = uclass_get_priv(uc); + if (!priv->activity_led_dev || + uclass_get_device_tail(priv->activity_led_dev, 0, &dev)) { + printf("Failed to get activity LED %s\n", + priv->activity_led_label); + return -EINVAL; + } + + return led_set_state(dev, LEDST_OFF); +} + +#if defined(CONFIG_LED_BLINK) || defined(CONFIG_LED_SW_BLINK) +int led_activity_blink(void) +{ + struct uclass *uc = uclass_find(UCLASS_LED); + struct led_uc_priv *priv; + struct udevice *dev; + int ret; + + if (!uc) + return -ENOENT; + + priv = uclass_get_priv(uc); + if (!priv->activity_led_dev || + uclass_get_device_tail(priv->activity_led_dev, 0, &dev)) { + printf("Failed to get activity LED %s\n", + priv->activity_led_label); + return -EINVAL; + } + + ret = led_set_period(dev, priv->activity_led_period); + if (ret) { + if (ret != -ENOSYS) + return ret; + + /* fallback to ON with no set_period and no SW_BLINK */ + return led_set_state(dev, LEDST_ON); + } + + return led_set_state(dev, LEDST_BLINK); +} +#endif +#endif + static int led_post_bind(struct udevice *dev) { struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev); const char *default_state;

-#ifdef CONFIG_LED_BOOT +#if defined(CONFIG_LED_BOOT) || defined(CONFIG_LED_ACTIVITY) struct led_uc_priv *priv = uclass_get_priv(dev->uclass); #endif

@@ -189,6 +263,12 @@ static int led_post_bind(struct udevice *dev) priv->boot_led_dev = dev; #endif

+#ifdef CONFIG_LED_ACTIVITY + /* check if we are binding activity LED and assign it */ + if (!strcmp(priv->activity_led_label, uc_plat->label)) + priv->activity_led_dev = dev; +#endif + uc_plat->default_state = LEDST_COUNT;

default_state = dev_read_string(dev, "default-state"); @@ -242,13 +322,21 @@ static int led_post_probe(struct udevice *dev) return ret; }

-#ifdef CONFIG_LED_BOOT +#if defined(CONFIG_LED_BOOT) || defined(CONFIG_LED_ACTIVITY) static int led_init(struct uclass *uc) { struct led_uc_priv *priv = uclass_get_priv(uc);

+#ifdef CONFIG_LED_BOOT priv->boot_led_label = ofnode_options_read_str("boot-led"); priv->boot_led_period = ofnode_options_read_int("boot-led-period", 250); +#endif + +#ifdef CONFIG_LED_ACTIVITY + priv->activity_led_label = ofnode_options_read_str("activity-led"); + priv->activity_led_period = ofnode_options_read_int("activity-led-period", + 250); +#endif

return 0; } @@ -260,7 +348,7 @@ UCLASS_DRIVER(led) = { .per_device_plat_auto = sizeof(struct led_uc_plat), .post_bind = led_post_bind, .post_probe = led_post_probe, -#ifdef CONFIG_LED_BOOT +#if defined(CONFIG_LED_BOOT) || defined(CONFIG_LED_ACTIVITY) .init = led_init, .priv_auto = sizeof(struct led_uc_priv), #endif diff --git a/include/led.h b/include/led.h index ca495217777..bba8c0009ca 100644 --- a/include/led.h +++ b/include/led.h @@ -54,8 +54,11 @@ struct led_uc_plat { * struct led_uc_priv - Private data the uclass stores about each device * * @boot_led_label: Boot LED label + * @activity_led_label: Activity LED label * @boot_led_dev: Boot LED dev + * @activity_led_dev: Activity LED dev * @boot_led_period: Boot LED blink period + * @activity_led_period: Activity LED blink period */ struct led_uc_priv { #ifdef CONFIG_LED_BOOT @@ -63,6 +66,11 @@ struct led_uc_priv { struct udevice *boot_led_dev; int boot_led_period; #endif +#ifdef CONFIG_LED_ACTIVITY + const char *activity_led_label; + struct udevice *activity_led_dev; + int activity_led_period; +#endif };

struct led_ops { @@ -193,4 +201,48 @@ static inline int led_boot_blink(void) } #endif

+#ifdef CONFIG_LED_ACTIVITY + +/** + * led_activity_on() - turn ON the designated LED for activity + * + * Return: 0 if OK, -ve on error + */ +int led_activity_on(void); + +/** + * led_activity_off() - turn OFF the designated LED for activity + * + * Return: 0 if OK, -ve on error + */ +int led_activity_off(void); + +#if defined(CONFIG_LED_BLINK) || defined(CONFIG_LED_SW_BLINK) +/** + * led_activity_blink() - turn ON the designated LED for activity + * + * Return: 0 if OK, -ve on error + */ +int led_activity_blink(void); +#else +/* If LED BLINK is not supported/enabled, fallback to LED ON */ +#define led_activity_blink led_activity_on +#endif +#else +static inline int led_activity_on(void) +{ + return -ENOSYS; +} + +static inline int led_activity_off(void) +{ + return -ENOSYS; +} + +static inline int led_activity_blink(void) +{ + return -ENOSYS; +} +#endif + #endif

Implement support for LED 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 | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c index 0e62e449327..56d7da82629 100644 --- a/cmd/ubi.c +++ b/cmd/ubi.c @@ -14,6 +14,7 @@ #include <command.h> #include <env.h> #include <exports.h> +#include <led.h> #include <malloc.h> #include <memalign.h> #include <mtd.h> @@ -488,10 +489,18 @@ exit:

int ubi_volume_write(char *volume, void *buf, loff_t offset, size_t size) { + int ret; + + led_activity_blink(); + if (!offset) - return ubi_volume_begin_write(volume, buf, size, size); + ret = ubi_volume_begin_write(volume, buf, size, size); + else + ret = ubi_volume_offset_write(volume, buf, offset, size);

- return ubi_volume_offset_write(volume, buf, offset, size); + led_activity_off(); + + return ret; }

int ubi_volume_read(char *volume, char *buf, loff_t offset, size_t size)

Introduce simple led.rst documentation to document all the additional Kconfig and the current limitation of LED_BLINK and GPIO software blink.

Also add missing definition for sw_blink in led_uc_plat struct.

Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- doc/api/index.rst | 1 + doc/api/led.rst | 10 ++++++++++ include/led.h | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 doc/api/led.rst

diff --git a/doc/api/index.rst b/doc/api/index.rst index ec0b8adb2cf..9f7f23f868f 100644 --- a/doc/api/index.rst +++ b/doc/api/index.rst @@ -14,6 +14,7 @@ U-Boot API documentation event getopt interrupt + led linker_lists lmb logging diff --git a/doc/api/led.rst b/doc/api/led.rst new file mode 100644 index 00000000000..e52e350d1bb --- /dev/null +++ b/doc/api/led.rst @@ -0,0 +1,10 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +LED +=== + +.. kernel-doc:: include/led.h + :doc: Overview + +.. kernel-doc:: include/led.h + :internal: \ No newline at end of file diff --git a/include/led.h b/include/led.h index bba8c0009ca..a9dd55efd1f 100644 --- a/include/led.h +++ b/include/led.h @@ -11,6 +11,46 @@ #include <cyclic.h> #include <dm/ofnode.h>

+/** + * DOC: Overview + * + * Generic LED API provided when a supported compatible is defined in DeviceTree. + * + * To enable support for LEDs, enable the `CONFIG_LED` Kconfig option. + * + * The most common implementation is for GPIO-connected LEDs. If using GPIO-connected LEDs, + * enable the `LED_GPIO` Kconfig option. + * + * `LED_BLINK` support requires LED driver support and is therefore optional. If LED blink + * functionality is needed, enable the `LED_BLINK` Kconfig option. If LED driver doesn't + * support HW Blink, SW Blink can be used with the Cyclic framework by enabling the + * CONFIG_LED_SW_BLINK. + * + * Boot and Activity LEDs are also supported. These LEDs can signal various system operations + * during runtime, such as boot initialization, file transfers, and flash write/erase operations. + * + * To enable a Boot LED, enable `CONFIG_LED_BOOT` and define in `/options/u-boot` root node the + * property `boot-led`. This will enable the specified LED to blink and turn ON when + * the bootloader initializes correctly. + * + * To enable an Activity LED, enable `CONFIG_LED_ACTIVITY` and define in `/options/u-boot` root + * node the property `activity-led`. + * This will enable the specified LED to blink and turn ON during file transfers or flash + * write/erase operations. + * + * Both Boot and Activity LEDs provide a simple API to turn the LED ON or OFF: + * `led_boot_on()`, `led_boot_off()`, `led_activity_on()`, and `led_activity_off()`. + * + * Both configurations can optionally define a `boot/activity-led-period` property + * if `CONFIG_LED_BLINK` or `CONFIG_LED_SW_BLINK` is enabled for LED blink operations, which + * is usually used by the Activity LED. If not defined the default value of 250 (ms) is used. + * + * When `CONFIG_LED_BLINK` or `CONFIG_LED_SW_BLINK` is enabled, additional APIs are exposed: + * `led_boot_blink()` and `led_activity_blink()`. Note that if `CONFIG_LED_BLINK` or + * `CONFIG_LED_SW_BLINK` is disabled, these APIs will behave like the `led_boot_on()` and + * `led_activity_on()` APIs, respectively. + */ + struct udevice;

enum led_state_t { @@ -41,6 +81,7 @@ struct led_sw_blink { * * @label: LED label * @default_state: LED default state + * @sw_blink: LED software blink struct */ struct led_uc_plat { const char *label;

Add tests for LED boot and activity feature and add required property in sandbox test DTS.

Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- arch/sandbox/dts/test.dts | 2 ++ test/dm/led.c | 72 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 5fb5eac862e..25859ad852d 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -101,6 +101,8 @@ bootscr-ram-offset = /bits/ 64 <0x12345678>; bootscr-flash-offset = /bits/ 64 <0>; bootscr-flash-size = /bits/ 64 <0x2000>; + boot-led = "sandbox:green"; + activity-led = "sandbox:red"; }; };

diff --git a/test/dm/led.c b/test/dm/led.c index c28fa044f45..4b019c71f3a 100644 --- a/test/dm/led.c +++ b/test/dm/led.c @@ -137,3 +137,75 @@ static int dm_test_led_blink(struct unit_test_state *uts) } DM_TEST(dm_test_led_blink, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); #endif + +/* Test LED boot */ +#ifdef CONFIG_LED_BOOT +static int dm_test_led_boot(struct unit_test_state *uts) +{ + struct udevice *dev + + /* options/u-boot/boot-led is set to "sandbox:green" */ + ut_assertok(led_get_by_label("sandbox:green", &dev)); + ut_asserteq(LEDST_OFF, led_get_state(dev)); + ut_assertok(led_boot_on()); + ut_asserteq(LEDST_ON, led_get_state(dev)); + ut_assertok(led_boot_off()); + ut_asserteq(LEDST_OFF, led_get_state(dev)); + + return 0; +} + +/* Test LED boot blink fallback */ +#ifndef CONFIG_LED_BLINK +static int dm_test_led_boot(struct unit_test_state *uts) +{ + struct udevice *dev + + /* options/u-boot/boot-led is set to "sandbox:green" */ + ut_assertok(led_get_by_label("sandbox:green", &dev)); + ut_asserteq(LEDST_OFF, led_get_state(dev)); + ut_assertok(led_boot_blink()); + ut_asserteq(LEDST_ON, led_get_state(dev)); + ut_assertok(led_boot_off()); + ut_asserteq(LEDST_OFF, led_get_state(dev)); + + return 0; +} +#endif +#endif + +/* Test LED activity */ +#ifdef CONFIG_LED_ACTIVITY +static int dm_test_led_boot(struct unit_test_state *uts) +{ + struct udevice *dev + + /* options/u-boot/activity-led is set to "sandbox:red" */ + ut_assertok(led_get_by_label("sandbox:red", &dev)); + ut_asserteq(LEDST_OFF, led_get_state(dev)); + ut_assertok(led_activity_on()); + ut_asserteq(LEDST_ON, led_get_state(dev)); + ut_assertok(led_activity_off()); + ut_asserteq(LEDST_OFF, led_get_state(dev)); + + return 0; +} + +/* Test LED activity blink fallback */ +#ifndef CONFIG_LED_BLINK +static int dm_test_led_boot(struct unit_test_state *uts) +{ + struct udevice *dev + + /* options/u-boot/activity-led is set to "sandbox:red" */ + ut_assertok(led_get_by_label("sandbox:red", &dev)); + ut_asserteq(LEDST_OFF, led_get_state(dev)); + ut_assertok(led_activity_blink()); + ut_asserteq(LEDST_ON, led_get_state(dev)); + ut_assertok(led_activity_off()); + ut_asserteq(LEDST_OFF, led_get_state(dev)); + + return 0; +} +#endif +#endif

Expand ofnode options test with new generic helper for bool, int and string.

Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- arch/sandbox/dts/test.dts | 3 +++ test/dm/ofnode.c | 9 +++++++++ 2 files changed, 12 insertions(+)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 25859ad852d..e5381b56da4 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -103,6 +103,9 @@ bootscr-flash-size = /bits/ 64 <0x2000>; boot-led = "sandbox:green"; activity-led = "sandbox:red"; + testing-bool; + testing-int = <123>; + testing-str = "testing"; }; };

diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c index 39191d7f52b..7c0adcd596b 100644 --- a/test/dm/ofnode.c +++ b/test/dm/ofnode.c @@ -614,6 +614,15 @@ static int dm_test_ofnode_options(struct unit_test_state *uts) u64 bootscr_address, bootscr_offset; u64 bootscr_flash_offset, bootscr_flash_size;

+ ut_assert(!ofnode_options_read_bool("missing")); + ut_assert(ofnode_options_read_bool("testing-bool")); + + ut_asserteq(123, ofnode_options_read_int("testing-int", 0)); + ut_asserteq(6, ofnode_options_read_int("missing", 6)); + + ut_assertnull(ofnode_options_read_str("missing")); + ut_asserteq_str("testing", ofnode_options_read_str("testing-str")); + ut_assertok(ofnode_read_bootscript_address(&bootscr_address, &bootscr_offset)); ut_asserteq_64(0, bootscr_address);