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 <ansuels...@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
> 

Reply via email to