Re: [PATCH v2 2/2] leds: spi-byte: add single byte SPI LED driver

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Hi Christian,

On 5/6/19 10:48 AM, Christian Mauderer wrote:
> On 05/05/2019 22:12, Pavel Machek wrote:
>> Hi!
>>
>>>> +	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
>>>> +	if (!led)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	led->spi = spi;
>>>> +	strlcpy(led->name, name, sizeof(led->name));
>>>> +	mutex_init(&led->mutex);
>>>> +	led->off_value = off_value;
>>>> +	led->max_value = max_value;
>>>> +	led->ldev.name = led->name;
>>>> +	led->ldev.brightness = LED_OFF;
>>>
>>> This line is redundant - already zeroed by kzalloc.
>>
>> Actually I'd prefer to leave it in. Yes, LED_OFF == 0, and will
>> probably stay == 0 in future, but...
>> 									Pavel
>>
> 
> Before I send v4: Currently the initial value isn't written to the LED
> anywhere. The state that is set by U-Boot is just kept till the first
> write to the brightness file.
> 
> I didn't implement "default-state" because the OpenWRT user space sets
> the LED anyway a few seconds later (which is still my use case for that
> driver). But now I noted that there is a remark in the documentation of
> the option "default-state" in devicetree/bindings/leds/common.txt: "The
> default is off if this property is not present."
> 
> Should I send an initial value to the device during initialization or is
> it OK to just keep the original state?

Yes, I would make use of default-state in this case.

-- 
Best regards,
Jacek Anaszewski