On Sat, Sep 21, 2024 at 01:14:36PM +0200, Michael Nazzareno Trimarchi wrote: > Hi > > On Sat, Sep 21, 2024 at 12:51 AM Christian Marangi <[email protected]> > 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 <[email protected]> > > Reviewed-by: Simon Glass <[email protected]> > > --- > > drivers/led/led_sw_blink.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/led/led_sw_blink.c b/drivers/led/led_sw_blink.c > > index 9e36edbee47..9d9820720c6 100644 > > --- a/drivers/led/led_sw_blink.c > > +++ b/drivers/led/led_sw_blink.c > > @@ -103,8 +103,16 @@ 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); > > + enum led_state_t curr_state = led_get_state(dev); > > + > > + curr_state = ops->get_state(dev); > > > The led_get_state return curr_state. You need to use a) led_set/get > state or them from ops > > if (state == LEDST_BLINK) { > const struct led_ops *ops = led_get_ops(dev); > enum led_state_t curr_state = ops->get_state(dev); > enun led_state_t next_state; > > switch (curr_state) { > case LEDST_ON: > sw_blink->state = LED_SW_BLINK_ST_OFF; > next_state = LEDST_OFF; > break; > case LEDST_OFF: > sw_blink->state = LED_SW_BLINK_ST_ON; > next_state = LEDST_ON; > break; > } > > ops->set_state(dev, next_state); > return true; > } >
Hi, I'm not following how this is different than the proposed code... Is this a suggestion to make it more readable, I'm a bit confused. > > > + /* toggle led initially */ > > + ops->set_state(dev, curr_state == LEDST_ON ? LEDST_OFF : > > + LEDST_ON); > > /* start blinking on next led_sw_blink() call */ > > - sw_blink->state = LED_SW_BLINK_ST_OFF; > > + sw_blink->state = curr_state == LEDST_ON ? > > LED_SW_BLINK_ST_OFF : > > + LED_SW_BLINK_ST_ON; > > > > return true; > > } > > > > -- > > 2.45.2 > > > > > -- > Michael Nazzareno Trimarchi > Co-Founder & Chief Executive Officer > M. +39 347 913 2170 > [email protected] > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > [email protected] > www.amarulasolutions.com -- Ansuel
