[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 at 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