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@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
