commit 83c63f0d118 (led: Move OF "label" property parsing to core)

[Rasmus Villemoes]

On 17/10/2023 17.33, Marek Vasut wrote:
> On 10/17/23 15:29, Rasmus Villemoes wrote:

>> static int led_gpio_bind(struct udevice *parent)
>> {
...
>>         ret = device_bind_driver_to_node(parent, "gpio_led",
>>                          ofnode_get_name(node),
>>                          node, &dev);
>>
>> static int bcm6753_led_bind(struct udevice *parent)
>> {
>>
...
>>         ret = device_bind_driver_to_node(parent, "bcm6753-led",
>>                          ofnode_get_name(node),
>>                          node, &dev);
>>
>> and that string can, if I'm not mistaken, be gotten from
>> parent->driver->name.
> 
> Which string ? The "bcm6753-led" is driver name , so that would have to
> be parametrized.

Exactly. The only difference between the two examples (apart from the
scope of the variables) is the driver name, but I think a
generic_led_bind() could just do this inside the loop:

  device_bind_driver_to_node(parent, parent->driver->name,
                          ofnode_get_name(node),
                          node, &dev);

and with that we could have most, if not all, existing .bind methods
assigned directly to generic_led_bind(), we don't even need one-line
wrapper functions in each driver passing that string explicitly.
> Make sure you test the 'led' command. The LEDs might be bound, but not
> probed (so label parsing in LED probe would be too late), and if I
> recall it right, the label parsing has to be in post-bind for the labels
> to correctly show in the 'led' command listing.

It's the led command I'm testing with (with my "new" driver for the
lp5562), and where I see the problems. Currently, the top node also gets
listed in "led list" as led-controller at 30 (it's an i2c device), but
probe of that device fails because the logic in the .probe method thinks
this is a subnode (the pattern is the same as in e.g. cortina_led_probe).

Since I'm simply copying the pattern from existing drivers, I'm pretty
sure those other drivers are also currently broken (see also the fixups
I mentioned).

> I am all for generic_led_bind() .

I'll try to write some real patches.

Rasmus