Re: [PATCH v5 01/11] led: toggle LED on initial SW blink

Michael Nazzareno Trimarchi
2024-10-03T12:51:39Z

Hi

On Tue, Oct 1, 2024 at 2:25 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> 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.
>
> Toggle the LED (ON if OFF, OFF if 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 | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/led/led_sw_blink.c b/drivers/led/led_sw_blink.c
> index 9e36edbee47..06a43db340c 100644
> --- a/drivers/led/led_sw_blink.c
> +++ b/drivers/led/led_sw_blink.c
> @@ -103,8 +103,21 @@ bool led_sw_on_state_change(struct udevice *dev, enum led_state_t state)
> return false;
>
> if (state == LEDST_BLINK) {
> - /* start blinking on next led_sw_blink() call */
> - sw_blink->state = LED_SW_BLINK_ST_OFF;
> + struct led_ops *ops = led_get_ops(dev);
> +
> + /*
> + * toggle LED initially and start blinking on next
> + * led_sw_blink() call.
> + */
> + switch (ops->get_state(dev)) {
> + case LEDST_ON:
> + ops->set_state(dev, LEDST_OFF);
> + sw_blink->state = LED_SW_BLINK_ST_OFF;
> + default:
> + ops->set_state(dev, LEDST_ON);
> + sw_blink->state = LED_SW_BLINK_ST_ON;
> + }
> +
> return true;

I think here error is not take in account. Is somenthing that we
should care? and if not we should
warning the user?

Apart of that
Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com>

michael

> }
>
> --
> 2.45.2
>

--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com