On 7/15/24 11:11, Simon Glass wrote:
> Hi Mikhail,
>
> On Sat, 13 Jul 2024 at 17:32, Mikhail Kshevetskiy
> <mikhail.kshevets...@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.kshevets...@iopsys.eu> wrote:
>>>> From: Michael Polyntsov <michael.polynt...@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.polynt...@iopsys.eu>
>>>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevets...@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?
We need to patch cmd/led.c. Without it we will get:
* something like out of boundary access
* enumeration value ‘LEDST_BLINK’ not handled in switch
can be fixed, but probably it should be a separate patch
>
> Regards,
> Simon
