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