[PATCH v2 0/9] led: introduce LED boot and activity function

[PATCH v2 0/9] led: introduce LED boot and activity function

[Christian Marangi]

This series is a reworked version of the previous seried: 
misc: introduce STATUS LED activity function

This series port and expand the legacy concept of LED boot from
the legacy Status LED API to new LED API.

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.

Additional Kconfg are also introduced to set the LED boot and
activity. Those are referenced by label.

A documentation for old and these new LED API is created.

(world tested with the azure pipeline)

Changes v2:
- Drop GPIO SW implementation
- Add fix for new LED SW BLINK

Christian Marangi (9):
  led: turn LED ON on initial SW blink
  led: implement led_set_state/period_by_label
  led: implement LED boot API
  common: board_r: rework BOOT LED handling
  led: implement LED activity API
  tftp: implement support for LED activity
  mtd: implement support for LED activity
  ubi: implement support for LED activity
  doc: introduce led.rst documentation

 cmd/mtd.c                  |  19 +++++
 cmd/ubi.c                  |  15 +++-
 common/board_r.c           |  25 ++++++-
 doc/api/index.rst          |   1 +
 doc/api/led.rst            |  10 +++
 drivers/led/Kconfig        |  41 +++++++++++
 drivers/led/led-uclass.c   |  28 ++++++++
 drivers/led/led_sw_blink.c |   5 +-
 include/led.h              | 144 +++++++++++++++++++++++++++++++++++++
 net/tftp.c                 |   7 ++
 10 files changed, 290 insertions(+), 5 deletions(-)
 create mode 100644 doc/api/led.rst

-- 
2.45.2

[PATCH v2 1/9] led: turn LED ON on initial SW blink

[Christian Marangi]

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.

Turn LED 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>
---
 drivers/led/led_sw_blink.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/led/led_sw_blink.c b/drivers/led/led_sw_blink.c
index 9e36edbee47..853278670b9 100644
--- a/drivers/led/led_sw_blink.c
+++ b/drivers/led/led_sw_blink.c
@@ -103,8 +103,11 @@ 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);
+
+		ops->set_state(dev, LEDST_ON);
 		/* start blinking on next led_sw_blink() call */
-		sw_blink->state = LED_SW_BLINK_ST_OFF;
+		sw_blink->state = LED_SW_BLINK_ST_ON;
 		return true;
 	}
 
-- 
2.45.2

[PATCH v2 2/9] led: implement led_set_state/period_by_label

[Christian Marangi]

Introduce new API led_set_state/period_by label as a shorthand to set
LED state and LED blink period by referencing them by label.

This is needed for the upcoming additional API that will declare LED in
.confg and reference them by their LED label name.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/led/led-uclass.c | 28 ++++++++++++++++++++++++++++
 include/led.h            | 18 ++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
index 199d68bc25a..b2b96d7d48b 100644
--- a/drivers/led/led-uclass.c
+++ b/drivers/led/led-uclass.c
@@ -65,6 +65,20 @@ int led_set_state(struct udevice *dev, enum led_state_t state)
 	return ops->set_state(dev, state);
 }
 
+int led_set_state_by_label(const char *label, enum led_state_t state)
+{
+	struct udevice *dev;
+	int ret;
+
+	ret = led_get_by_label(label, &dev);
+	if (ret) {
+		printf("Failed to get LED by label %s\n", label);
+		return ret;
+	}
+
+	return led_set_state(dev, state);
+}
+
 enum led_state_t led_get_state(struct udevice *dev)
 {
 	struct led_ops *ops = led_get_ops(dev);
@@ -94,6 +108,20 @@ int led_set_period(struct udevice *dev, int period_ms)
 	return -ENOSYS;
 }
 
+int led_set_period_by_label(const char *label, int period_ms)
+{
+	struct udevice *dev;
+	int ret;
+
+	ret = led_get_by_label(label, &dev);
+	if (ret) {
+		printf("Failed to get LED by label %s\n", label);
+		return ret;
+	}
+
+	return led_set_period(dev, period_ms);
+}
+
 static int led_post_bind(struct udevice *dev)
 {
 	struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
diff --git a/include/led.h b/include/led.h
index 99f93c5ef86..c1f3380f253 100644
--- a/include/led.h
+++ b/include/led.h
@@ -111,6 +111,15 @@ int led_get_by_label(const char *label, struct udevice **devp);
  */
 int led_set_state(struct udevice *dev, enum led_state_t state);
 
+/**
+ * led_set_state_by_label - set the state of an LED referenced by Label
+ *
+ * @label:	LED label
+ * @state:	LED state to set
+ * Return: 0 if OK, -ve on error
+ */
+int led_set_state_by_label(const char *label, enum led_state_t state);
+
 /**
  * led_get_state() - get the state of an LED
  *
@@ -128,6 +137,15 @@ enum led_state_t led_get_state(struct udevice *dev);
  */
 int led_set_period(struct udevice *dev, int period_ms);
 
+/**
+ * led_set_period_by_label - set the blink period of an LED referenced by Label
+ *
+ * @label:	LED label
+ * @period_ms:	LED blink period in milliseconds
+ * Return: 0 if OK, -ve on error
+ */
+int led_set_period_by_label(const char *label, int period_ms);
+
 /**
  * led_bind_generic() - bind children of parent to given driver
  *
-- 
2.45.2

[PATCH v2 3/9] led: implement LED boot API

[Christian Marangi]

Implement LED boot API to signal correct boot of the system.

led_boot_on/off/blink() are introduced to turn ON, OFF and BLINK the
designated boot LED.

New Kconfig are introduced, CONFIG_LED_BOOT_ENABLE to enable the feature,
CONFIG_LED_BOOT_LABEL to declare the LED label in DT to reference
the LED for BOOT usage and CONFIG_LED_BOOT_PERIOD to declare the
blinking period.

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 | 20 ++++++++++++++++++++
 include/led.h       | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
index bee74b25751..fd9442edaf3 100644
--- a/drivers/led/Kconfig
+++ b/drivers/led/Kconfig
@@ -9,6 +9,26 @@ config LED
 	  can provide access to board-specific LEDs. Use of the device tree
 	  for configuration is encouraged.
 
+config LED_BOOT_ENABLE
+	bool "Enable LED boot support"
+	help
+	  Enable LED boot support.
+
+	  LED boot is a specific LED assigned to signal boot operation status.
+
+config LED_BOOT_LABEL
+	string "LED boot label"
+	depends on LED_BOOT_ENABLE
+	help
+	  LED label defined in DT to assign for LED boot usage.
+
+config LED_BOOT_PERIOD
+	int "LED boot period"
+	depends on LED_BOOT_ENABLE && (LED_BLINK || LED_SW_BLINK)
+	default 2
+	help
+	  LED boot blink period in ms.
+
 config LED_BCM6328
 	bool "LED Support for BCM6328"
 	depends on LED && ARCH_BMIPS
diff --git a/include/led.h b/include/led.h
index c1f3380f253..7bbe165d838 100644
--- a/include/led.h
+++ b/include/led.h
@@ -159,4 +159,48 @@ int led_sw_set_period(struct udevice *dev, int period_ms);
 bool led_sw_is_blinking(struct udevice *dev);
 bool led_sw_on_state_change(struct udevice *dev, enum led_state_t state);
 
+#ifdef CONFIG_LED_BOOT_ENABLE
+
+/**
+ * led_boot_on() - turn ON the designated LED for booting
+ *
+ * Return: 0 if OK, -ve on error
+ */
+static inline int led_boot_on(void)
+{
+	return led_set_state_by_label(CONFIG_LED_BOOT_LABEL, LEDST_ON);
+}
+
+/**
+ * led_boot_off() - turn OFF the designated LED for booting
+ *
+ * Return: 0 if OK, -ve on error
+ */
+static inline int led_boot_off(void)
+{
+	return led_set_state_by_label(CONFIG_LED_BOOT_LABEL, LEDST_OFF);
+}
+
+#if defined(CONFIG_LED_BLINK) || defined(CONFIG_LED_SW_BLINK)
+/**
+ * led_boot_blink() - turn ON the designated LED for booting
+ *
+ * Return: 0 if OK, -ve on error
+ */
+static inline int led_boot_blink(void)
+{
+	int ret;
+
+	ret = led_set_period_by_label(CONFIG_LED_BOOT_LABEL, CONFIG_LED_BOOT_PERIOD);
+	if (ret)
+		return ret;
+
+	return led_set_state_by_label(CONFIG_LED_BOOT_LABEL, LEDST_BLINK);
+}
+#else
+/* If LED BLINK is not supported/enabled, fallback to LED ON */
+#define led_boot_blink led_boot_on
+#endif
+#endif
+
 #endif
-- 
2.45.2

[PATCH v2 4/9] common: board_r: rework BOOT LED handling

[Christian Marangi]

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(-)

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,
 };
 
-- 
2.45.2

[PATCH v2 5/9] led: implement LED activity API

[Christian Marangi]

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 are implemented similar to BOOT LED, LED_ACTIVITY_ENABLE to
enable support for it, LED_ACTIVITY_LABEL to assign the LED in DT for
activity usage and LED_ACTIVITY_PERIOD to set the blinking period.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/led/Kconfig | 21 +++++++++++++++++++++
 include/led.h       | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
index fd9442edaf3..1336f943dab 100644
--- a/drivers/led/Kconfig
+++ b/drivers/led/Kconfig
@@ -29,6 +29,27 @@ config LED_BOOT_PERIOD
 	help
 	  LED boot blink period in ms.
 
+config LED_ACTIVITY_ENABLE
+	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_ACTIVITY_LABEL
+	string "LED activity label"
+	depends on LED_ACTIVITY_ENABLE
+	help
+	  LED label defined in DT to assign for LED activity usage.
+
+config LED_ACTIVITY_PERIOD
+	int "LED activity period"
+	depends on LED_ACTIVITY_ENABLE && (LED_BLINK || LED_SW_BLINK)
+	default 2
+	help
+	  LED activity blink period in ms.
+
 config LED_BCM6328
 	bool "LED Support for BCM6328"
 	depends on LED && ARCH_BMIPS
diff --git a/include/led.h b/include/led.h
index 7bbe165d838..61ece70a975 100644
--- a/include/led.h
+++ b/include/led.h
@@ -203,4 +203,48 @@ static inline int led_boot_blink(void)
 #endif
 #endif
 
+#ifdef CONFIG_LED_ACTIVITY_ENABLE
+
+/**
+ * led_activity_on() - turn ON the designated LED for activity
+ *
+ * Return: 0 if OK, -ve on error
+ */
+static inline int led_activity_on(void)
+{
+	return led_set_state_by_label(CONFIG_LED_ACTIVITY_LABEL, LEDST_ON);
+}
+
+/**
+ * led_activity_off() - turn OFF the designated LED for activity
+ *
+ * Return: 0 if OK, -ve on error
+ */
+static inline int led_activity_off(void)
+{
+	return led_set_state_by_label(CONFIG_LED_ACTIVITY_LABEL, LEDST_OFF);
+}
+
+#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
+ */
+static inline int led_activity_blink(void)
+{
+	int ret;
+
+	ret = led_set_period_by_label(CONFIG_LED_ACTIVITY_LABEL, CONFIG_LED_BOOT_PERIOD);
+	if (ret)
+		return ret;
+
+	return led_set_state_by_label(CONFIG_LED_ACTIVITY_LABEL, LEDST_BLINK);
+}
+#else
+/* If LED BLINK is not supported/enabled, fallback to LED ON */
+#define led_activity_blink led_activity_on
+#endif
+#endif
+
 #endif
-- 
2.45.2

[PATCH v2 6/9] tftp: implement support for LED activity

[Christian Marangi]

Implement support for LED 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 2e073183d5a..45c2455336a 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -10,6 +10,7 @@
 #include <efi_loader.h>
 #include <env.h>
 #include <image.h>
+#include <led.h>
 #include <lmb.h>
 #include <log.h>
 #include <mapmem.h>
@@ -192,6 +193,9 @@ static void new_transfer(void)
 #ifdef CONFIG_CMD_TFTPPUT
 	tftp_put_final_block_sent = 0;
 #endif
+#ifdef CONFIG_LED_ACTIVITY_ENABLE
+	led_activity_blink();
+#endif
 }
 
 #ifdef CONFIG_CMD_TFTPPUT
@@ -301,6 +305,9 @@ static void tftp_complete(void)
 			time_start * 1000, "/s");
 	}
 	puts("\ndone\n");
+#ifdef CONFIG_LED_ACTIVITY_ENABLE
+	led_activity_off();
+#endif
 	if (!tftp_put_active)
 		efi_set_bootdev("Net", "", tftp_filename,
 				map_sysmem(tftp_load_addr, 0),
-- 
2.45.2

[PATCH v2 7/9] mtd: implement support for LED activity

[Christian Marangi]

Implement support for LED 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 795aaa2b37d..ba5ee0d4d71 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -10,6 +10,7 @@
 
 #include <command.h>
 #include <console.h>
+#include <led.h>
 #if CONFIG_IS_ENABLED(CMD_MTD_OTP)
 #include <hexdump.h>
 #endif
@@ -558,6 +559,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_ACTIVITY_ENABLE
+	if (!read)
+		led_activity_blink();
+#endif
+
 	/* Loop over the pages to do the actual read/write */
 	while (remaining) {
 		/* Skip the block if it is bad */
@@ -585,6 +591,11 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 		io_op.oobbuf += io_op.oobretlen;
 	}
 
+#ifdef CONFIG_LED_ACTIVITY_ENABLE
+	if (!read)
+		led_activity_off();
+#endif
+
 	if (!ret && dump)
 		mtd_dump_device_buf(mtd, start_off, buf, len, woob);
 
@@ -652,6 +663,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_ACTIVITY_ENABLE
+	led_activity_blink();
+#endif
+
 	while (len) {
 		if (!scrub) {
 			ret = mtd_block_isbad(mtd, erase_op.addr);
@@ -680,6 +695,10 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc,
 		erase_op.addr += mtd->erasesize;
 	}
 
+#ifdef CONFIG_LED_ACTIVITY_ENABLE
+	led_activity_off();
+#endif
+
 	if (ret && ret != -EIO)
 		ret = CMD_RET_FAILURE;
 	else
-- 
2.45.2

[PATCH v2 8/9] ubi: implement support for LED activity

[Christian Marangi]

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 | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 92998af2b02..e663e6dfafb 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>
@@ -425,7 +426,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_ACTIVITY_ENABLE
+	led_activity_blink();
+#endif
+
+	ret = ubi_volume_begin_write(volume, buf, size, size);
+
+#ifdef CONFIG_LED_ACTIVITY_ENABLE
+	led_activity_off();
+#endif
+
+	return ret;
 }
 
 int ubi_volume_read(char *volume, char *buf, size_t size)
-- 
2.45.2

[PATCH v2 9/9] doc: introduce led.rst documentation

[Christian Marangi]

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

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 doc/api/index.rst |  1 +
 doc/api/led.rst   | 10 ++++++++++
 include/led.h     | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 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 61ece70a975..77b18ddffbf 100644
--- a/include/led.h
+++ b/include/led.h
@@ -10,6 +10,44 @@
 #include <stdbool.h>
 #include <cyclic.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_ENABLE` and define `CONFIG_LED_BOOT_LABEL`. 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_ENABLE` and define
+ * `CONFIG_LED_ACTIVITY_LABEL`.
+ * 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 `_PERIOD` option if `CONFIG_LED_BLINK` or
+ * `CONFIG_LED_SW_BLINK` is enabled for LED blink operations, which is usually used by
+ * the Activity LED.
+ *
+ * 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` is disabled,
+ * these APIs will behave like the `led_boot_on()` and `led_activity_on()` APIs, respectively.
+ */
+
 struct udevice;
 
 enum led_state_t {
-- 
2.45.2

Re: [PATCH v2 9/9] doc: introduce led.rst documentation

[Alexander Dahl]

Hello Christian,

Am Wed, Aug 07, 2024 at 09:54:12PM +0200 schrieb Christian Marangi:
> Introduce simple led.rst documentation to document all the additional
> Kconfig and the current limitation of LED_BLINK and GPIO software blink.

This is a good idea.  An overview of all the LED possibilities in the
Documentation is much appreciated.  Remarks below.

> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  doc/api/index.rst |  1 +
>  doc/api/led.rst   | 10 ++++++++++
>  include/led.h     | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 49 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 61ece70a975..77b18ddffbf 100644
> --- a/include/led.h
> +++ b/include/led.h
> @@ -10,6 +10,44 @@
>  #include <stdbool.h>
>  #include <cyclic.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_ENABLE` and define `CONFIG_LED_BOOT_LABEL`. This
> + * will enable the specified LED to blink and turn ON when the bootloader initializes correctly.

This is somehow redundant to defining "u-boot,boot-led" in
*-u-boot.dtsi snippets, isn't it?  This is documented in
doc/device-tree-bindings/config.txt and used by roughly a dozen dtsi
files and five times in board code.  It also stores a LED label, which
board code can make use of then, but I think it's not further
integrated in any driver or class code.  Are you aware of that mechanism
and hwo does it fit into your rework of the boot led?

Greets
Alex

> + *
> + * To enable an Activity LED, enable `CONFIG_LED_ACTIVITY_ENABLE` and define
> + * `CONFIG_LED_ACTIVITY_LABEL`.
> + * 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 `_PERIOD` option if `CONFIG_LED_BLINK` or
> + * `CONFIG_LED_SW_BLINK` is enabled for LED blink operations, which is usually used by
> + * the Activity LED.
> + *
> + * 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` is disabled,
> + * these APIs will behave like the `led_boot_on()` and `led_activity_on()` APIs, respectively.
> + */
> +
>  struct udevice;
>  
>  enum led_state_t {
> -- 
> 2.45.2
> 

Re: [PATCH v2 9/9] doc: introduce led.rst documentation

[Christian Marangi]

On Thu, Aug 08, 2024 at 08:34:32AM +0200, Alexander Dahl wrote:
> Hello Christian,
> 
> Am Wed, Aug 07, 2024 at 09:54:12PM +0200 schrieb Christian Marangi:
> > Introduce simple led.rst documentation to document all the additional
> > Kconfig and the current limitation of LED_BLINK and GPIO software blink.
> 
> This is a good idea.  An overview of all the LED possibilities in the
> Documentation is much appreciated.  Remarks below.
> 
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  doc/api/index.rst |  1 +
> >  doc/api/led.rst   | 10 ++++++++++
> >  include/led.h     | 38 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 49 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 61ece70a975..77b18ddffbf 100644
> > --- a/include/led.h
> > +++ b/include/led.h
> > @@ -10,6 +10,44 @@
> >  #include <stdbool.h>
> >  #include <cyclic.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_ENABLE` and define `CONFIG_LED_BOOT_LABEL`. This
> > + * will enable the specified LED to blink and turn ON when the bootloader initializes correctly.
> 
> This is somehow redundant to defining "u-boot,boot-led" in
> *-u-boot.dtsi snippets, isn't it?  This is documented in
> doc/device-tree-bindings/config.txt and used by roughly a dozen dtsi
> files and five times in board code.  It also stores a LED label, which
> board code can make use of then, but I think it's not further
> integrated in any driver or class code.  Are you aware of that mechanism
> and hwo does it fit into your rework of the boot led?
>

No I wasn't aware of those property. I will check how that can be
expanded and implemented here as it seems a better way than raw configs.

> 
> > + *
> > + * To enable an Activity LED, enable `CONFIG_LED_ACTIVITY_ENABLE` and define
> > + * `CONFIG_LED_ACTIVITY_LABEL`.
> > + * 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 `_PERIOD` option if `CONFIG_LED_BLINK` or
> > + * `CONFIG_LED_SW_BLINK` is enabled for LED blink operations, which is usually used by
> > + * the Activity LED.
> > + *
> > + * 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` is disabled,
> > + * these APIs will behave like the `led_boot_on()` and `led_activity_on()` APIs, respectively.
> > + */
> > +
> >  struct udevice;
> >  
> >  enum led_state_t {
> > -- 
> > 2.45.2
> > 

-- 
	Ansuel