Re: [PATCH v8 6/6] leds: Add a multicolor LED driver to group monochromatic LEDs

[Jean-Jacques Hiblot <jjhiblot@traphandler.com>]

On 28/03/2023 19:20, Andy Shevchenko wrote:
> On Tue, Mar 28, 2023 at 7:15 PM Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>>
>> Grouping multiple monochrome LEDs into a multicolor LED device has a few
>> benefits over handling the group in user-space:
>> - The state of the LEDs relative to each other is consistent. In other
>>    words, if 2 threads competes to set the LED to green and red, the
>>    end-result cannot be black or yellow.
>> - The multicolor LED as a whole can be driven through the sysfs LED
>>    interface.
> 
> ...
> 
>> +config LEDS_GROUP_MULTICOLOR
>> +       tristate "LEDs group multi-color support"
>> +       depends on OF || COMPILE_TEST
> 
> Why is OF a dependency?
> Can't we make it a firmware provider agnostic solution from day 1?
That would be possible, unfortunately I have no way of testing the ACPI 
stuff. That's why I prefer sticking to OF.

> 
>> +       help
>> +         This option enables support for monochrome LEDs that are
>> +         grouped into multicolor LEDs.
>> +         This useful in the case where LEDs of different colors are
> 
> This is
> 
>> +         physically grouped in a single multi-color LED and driven
>> +         by a controller that doesn't have multi-color support.
>> +
>> +         To compile this driver as a module, choose M here: the module
>> +         will be called leds-group-multicolor.
> 
> ...
> 
>> +               led_cdev = devm_of_led_get_optional(dev, count);
> 
> I don't see how this is OF specific to this driver. Maybe it needs to
> be patched first, so we will have something non-OF specific?
> 
>> +               if (IS_ERR(led_cdev))
>> +                       return dev_err_probe(dev, PTR_ERR(led_cdev), "Unable to get LED #%d",
>> +                                            count);
> 
> At least we need to have a FIXME or so here.
>