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

Rasmus Villemoes rasmus.villemoes at prevas.dk
Tue Oct 17 15:29:35 CEST 2023


Hi,

I'm trying to resurrect an old submission of a driver for ti,lp5562, so
had occasion to dig into drivers/led/. And I think commit 83c63f0d118 is
buggy or at least incomplete.

Many of the drivers that were subsequently modified to not do that
"label" parsing rely, in their .probe method, on the top node being
distinguished by not having a ->label. And led-uclass itself does that

                /* Ignore the top-level LED node */
                if (uc_plat->label && !strcmp(label, uc_plat->label))
                        return uclass_get_device_tail(dev, 0, devp);

The drivers used to only do that label-parsing for subnodes of the top
node, so that worked, but the new code assigns some ->label for anything
bound to a LED uclass driver. There have been at least two patches to
individual drivers to fix this up since (the current top two patches
touching drivers/led/), but I think that's the wrong way to handle this.

At the same time, I actually think 83c63f0d118 didn't go far enough in
deduplicating, since all drivers are left with essentially the same loop:

static int led_gpio_bind(struct udevice *parent)
{
	struct udevice *dev;
	ofnode node;
	int ret;

	dev_for_each_subnode(node, parent) {
		ret = device_bind_driver_to_node(parent, "gpio_led",
						 ofnode_get_name(node),
						 node, &dev);
		if (ret)
			return ret;
	}

	return 0;
}

static int bcm6753_led_bind(struct udevice *parent)
{
	ofnode node;

	dev_for_each_subnode(node, parent) {
		struct udevice *dev;
		int ret;

		ret = device_bind_driver_to_node(parent, "bcm6753-led",
						 ofnode_get_name(node),
						 node, &dev);
		if (ret)
			return ret;
	}

	return 0;
}

and that string can, if I'm not mistaken, be gotten from
parent->driver->name. So we really should be able to create a
generic_led_bind() that does exactly this loop + the "label" parsing,
remove the label parsing from post_bind (so it doesn't apply to the top
nodes), and switch all drivers over to this generic_led_bind().

Alternatively, we can still create a generic_led_bind() that just does
the loop as above, but then somehow prevent the top nodes from gaining
->label, say by not adding the nodename fallback if the node has a
"compatible" property (though that seems like a hack, maybe there's a
cleaner way).

WDYT?

Rasmus



More information about the U-Boot mailing list