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

[Jean-Jacques Hiblot]

On 28/03/2023 19:20, Andy Shevchenko wrote:
> On Tue, Mar 28, 2023 at 7:15 PM Jean-Jacques Hiblot
> <jjhiblot@xxxxxxxxxxxxxxx> 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.