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
>> +
>> /*
>> * In case the LED has default-state DT property, trigger
>> * probe() to configure its default state during startup.
>> @@ -132,12 +141,28 @@ static int led_post_bind(struct udevice *dev)
>> static int led_post_probe(struct udevice *dev)
>> {
>> struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> + int rc = 0;
>>
>> - if (uc_plat->default_state == LEDST_ON ||
>> - uc_plat->default_state == LEDST_OFF)
>> - led_set_state(dev, uc_plat->default_state);
>> + switch (uc_plat->default_state) {
>> + case LEDST_ON:
>> + case LEDST_OFF:
>> + rc = led_set_state(dev, uc_plat->default_state);
>> + break;
>> +#ifdef CONFIG_LED_BLINK
>> + case LEDST_BLINK: {
>> + const int default_period_ms = 1000;
>>
>> - return 0;
>> + rc = led_set_period(dev, default_period_ms);
>> + if (rc == 0)
>> + rc = led_set_state(dev, uc_plat->default_state);
>> + break;
>> + }
>> +#endif
>> + default:
>> + break;
>> + }
>> +
>> + return rc;
>> }
>>
>> UCLASS_DRIVER(led) = {
>> --
>> 2.39.2
>>
> Regards,
> Simon
