Re: [PATCH v3 1/9] led: turn LED ON on initial SW blink

[Christian Marangi <ansuelsmth@gmail.com>]

On Thu, Sep 19, 2024 at 04:13:44PM +0200, Simon Glass wrote:
> Hi Christian,
> 
> On Mon, 12 Aug 2024 at 12:33, 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.
> >
> > Turn LED 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 | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> nit below
> 
> >
> > diff --git a/drivers/led/led_sw_blink.c b/drivers/led/led_sw_blink.c
> > index 9e36edbee47..853278670b9 100644
> > --- a/drivers/led/led_sw_blink.c
> > +++ b/drivers/led/led_sw_blink.c
> > @@ -103,8 +103,11 @@ bool led_sw_on_state_change(struct udevice *dev, enum led_state_t state)
> >                 return false;
> >
> >         if (state == LEDST_BLINK) {
> > +               struct led_ops *ops = led_get_ops(dev);
> > +
> > +               ops->set_state(dev, LEDST_ON);
> 
> Normally in drivers we define functions like led_set_state() in the
> uclass, rather than calling things directly like this.
> 

I used ops directly as I'm following how it's done in led_sw_blink and
because the support for these ops is already validated and we don't need
to check for the -ENOSYS condition.

Hope it's ok. Also as suggested I changed the function to toggle the LED
as suggested. I added the review tag. Tell me if I have to drop it in
the next revision.

> >                 /* start blinking on next led_sw_blink() call */
> > -               sw_blink->state = LED_SW_BLINK_ST_OFF;
> > +               sw_blink->state = LED_SW_BLINK_ST_ON;
> >                 return true;
> >         }
> >
> > --
> > 2.45.2
> >
> 
> Regards,
> Simon

-- 
	Ansuel