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

Christian Marangi
2024-10-03T12:51:47Z

On Thu, Oct 03, 2024 at 08:22:21AM +0200, Michael Nazzareno Trimarchi wrote:
> 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?

Yes it's intended. Here it's assumed these function works and are
present. The check are already done before calling the on_state_change.

>
> 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
> www.amarulasolutions.com

--
Ansuel