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 <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?
>
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
