[PATCH v4 0/9] misc: introduce STATUS LED activity function

[Heinrich Schuchardt]

On 6/20/24 01:03, Christian Marangi wrote:
> This series expand the STATUS LED framework with a new color
> and a big new feature. 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.
>
> To fix this and handle the problem of device not supporting
> HW blink, expand the STATUS LED framework with new API.
>
> We introduce a new config LED_STATUS_ACTIVITY, that similar
> to the RED, GREEN and others, takes the LED ID set in
> the LED_STATUS config and is used as the global LED for activity
> operations.
>
> We add status_led_activity_start/stop() used to turn the
> activity LED on and register a cyclic to make it blink.
> LED will continue to blink until _stop() is called.
>
> Function that will start some kind of action will first call
> _start() and then at the end will call stop().
> If (on the broken implementation) a _start() is called before
> calling a _stop(), the Cyclic is first unregister and is
> registered again.
>
> Support for this is initially added to the tftp, mtd and ubi
> command.
>
> This also contains a big fixup for the gpio_led driver that
> currently deviates from the Documentation and make the
> coloured status led feature unusable.
>
> (world tested with the azure pipeline)

Hello Christian,

What is the relationship between CONFIG_LED and CONFIG_LED_STATUS?
Can they be used at the same time or should they be exclusive?
Should CONFIG_LED_STATUS depend on CONFIG_LED?

This should be clarified both in Kconfig and in the documentation.

At least cmd/led.c and cmd/legacy_led.c are conflicting as both provide
a command 'led'.

Can we unify the too?

Best regards

Heinrich

>
> Changes v4:
> - Rebase on top of next
> - Rework for cyclic changes on next
> - Fix compilation error
> - Fix compilation warning due to missing header
> Changes v3:
> - Add log on Status LED error conditions
> Changes v2:
> - Add Documentation conversion
> - Rework to Cyclic and simplify implementation
>
> Christian Marangi (9):
>    misc: gpio_led: fix broken coloured LED status functions
>    led: status_led: add support for white LED colour
>    led: status_led: add function to toggle a status LED
>    led: status_led: add warning when an invalid Status LED ID is used
>    led: status_led: add new activity LED config and functions
>    tftp: implement support for LED status activity
>    mtd: implement support for LED status activity
>    ubi: implement support for LED status activity
>    doc: convert README.LED to .rst documentation
>
>   cmd/legacy_led.c          |   6 +
>   cmd/mtd.c                 |  19 ++++
>   cmd/ubi.c                 |  15 ++-
>   common/board_f.c          |   2 +
>   doc/README.LED            |  77 -------------
>   doc/api/index.rst         |   1 +
>   doc/api/status_led.rst    |  35 ++++++
>   drivers/led/Kconfig       |  30 +++++
>   drivers/misc/gpio_led.c   |  41 +++++--
>   drivers/misc/status_led.c |  75 ++++++++++++-
>   include/status_led.h      | 231 +++++++++++++++++++++++++++++++++++++-
>   net/tftp.c                |   7 ++
>   12 files changed, 440 insertions(+), 99 deletions(-)
>   delete mode 100644 doc/README.LED
>   create mode 100644 doc/api/status_led.rst
>