[PATCH v5 2/2] led: Add dts property to specify blinking of the led

[Simon Glass]

Hi Mikhail,

On Sat, 13 Jul 2024 at 17:32, Mikhail Kshevetskiy
<mikhail.kshevetskiy at genexis.eu> wrote:
>
>
> On 13.07.2024 18:13, Simon Glass wrote:
> > Hi Mikhail,
> >
> > On Fri, 12 Jul 2024 at 06:25, Mikhail Kshevetskiy
> > <mikhail.kshevetskiy at iopsys.eu> wrote:
> >> From: Michael Polyntsov <michael.polyntsov at iopsys.eu>
> >>
> >> The standard property
> >>
> >>     linux,default-trigger = "pattern";
> >>
> >> used to get an effect. No blinking parameters can be set yet.
> >>
> >> Signed-off-by: Michael Polyntsov <michael.polyntsov at iopsys.eu>
> >> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu>
> >> ---
> >>  drivers/led/led-uclass.c | 33 +++++++++++++++++++++++++++++----
> >>  1 file changed, 29 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
> >> index 4f5dd66765e..90047ad35ba 100644
> >> --- a/drivers/led/led-uclass.c
> >> +++ b/drivers/led/led-uclass.c
> >> @@ -100,6 +100,9 @@ static int led_post_bind(struct udevice *dev)
> >>  {
> >>         struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> >>         const char *default_state;
> >> +#ifdef CONFIG_LED_BLINK
> >> +       const char *trigger;
> >> +#endif
> >>
> >>         if (!uc_plat->label)
> >>                 uc_plat->label = dev_read_string(dev, "label");
> >> @@ -120,6 +123,12 @@ static int led_post_bind(struct udevice *dev)
> >>         else
> >>                 return 0;
> >>
> >> +#ifdef CONFIG_LED_BLINK
> >> +       trigger = dev_read_string(dev, "linux,default-trigger");
> >> +       if (trigger && !strncmp(trigger, "pattern", 7))
> >> +               uc_plat->default_state = LEDST_BLINK;
> >> +#endif
> > I still wonder whether you can use IS_ENABLED() instead of adding #ifdefs?
>
> LEDST_BLINK depends on CONFIG_LED_BLINK, so the code will not compile if CONFIG_LED_BLINK is not defined.
> Defining a LEDST_BLINK unconditionally will produce an issue with cmd/led.c

We should not use #ifdefs around such declarations. What issue is
caused by defining it always?

Regards,
Simon