Re: [PATCH 2/6] led-uclass: honour ->label field populated by driver's own .bind

[Pavel Machek <pavel@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

Hi!

> > > > > > diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
> > > > > > index 5a5d07b9a7..0232fa84de 100644
> > > > > > --- a/drivers/led/led-uclass.c
> > > > > > +++ b/drivers/led/led-uclass.c
> > > > > > @@ -71,7 +71,9 @@ static int led_post_bind(struct udevice *dev)
> > > > > >         struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> > > > > >         const char *default_state;
> > > > > >     -    uc_plat->label = dev_read_string(dev, "label");
> > > > > > +    if (!uc_plat->label)
> > > > > > +        uc_plat->label = dev_read_string(dev, "label");
> > > > > > +
> > > > > 
> > > > > One thing I have to wonder about is, why does this controller have label
> > > > > property in the top-level node , what is that used for ?
> > > > > 
> > > > > (see Linux Documentation/devicetree/bindings/leds/leds-lp55xx.yaml)
> > > > > 
> > > > > Reviewed-by: Marek Vasut <marex@denx.de>
> > > > 
> > > > Reading the linux driver, it seems that the top-level label, if any, is
> > > > used as part of the naming for individual channels if they don't have
> > > > individual chan-name properties:
> > > > 
> > > > 
> > > >           if (pdata->led_config[chan].name) {
> > > >                   led->cdev.name = pdata->led_config[chan].name;
> > > >           } else {
> > > >                   snprintf(name, sizeof(name), "%s:channel%d",
> > > >                           pdata->label ? : chip->cl->name, chan);
> > > >                   led->cdev.name = name;
> > > >           }
> > > > 
> > > > but I think the rationale in d1188adb2dabc is a bit weak, since the only
> > > > example also does have individual chan-name properties.
> > > > 
> > > > [Complete aside: At first I thought it was related to the multi-color
> > > > LED work that has been ongoing for many many years (I think there was an
> > > > LWN article at some point), where this could be exposed as a single
> > > > multi-color LED, as opposed to the "traditional" three/four individual
> > > > LEDs. In the former case, there would only be one sysfs entry, but with
> > > > attributes exposing the multicolor functionality. I must admit I don't
> > > > know the status of that work, when something reaches v31,
> > > > http://archive.lwn.net:8080/linux-kernel/20200722071055.GA8984@amd/t/ ,
> > > > it's hard to know if it ever lands, or if pieces of it has landed.]
> > > 
> > > +CC Pavel
> 
> I think you want to coordinate the effort with Rasmus here .

This is u-boot, not Linux, right?

Anyway cc-ing linux-leds may be better than cc-ing me directly, but
don't expect much.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]