Re: [PATCH 1/6] led-uclass: do not create fallback label for top-level node

[Rasmus Villemoes <rasmus.villemoes@prevas.dk>]

On 19/10/2023 15.51, Marek Vasut wrote:
> On 10/19/23 11:58, Rasmus Villemoes wrote:
>> Many existing drivers, and led-uclass itself, rely on uc_plat->label
>> being NULL for the device representing the top node, as opposed to the
>> child nodes representing individual LEDs. This means that the drivers
>> whose .probe methods rely on this were broken by 83c63f0d1185 ("led:
>> Move OF "label" property parsing to core"), and also that the top node
>> wrongly shows up with 'led list'. Some drivers have since been fixed
>> up individually, e.g.
>>
>> e3aa76644c2a "led: gpio: Check device compatible string to determine
>> the top level node"
>> 01074697801b "led: gpio: Use NOP uclass driver for top-level node"
>> 910b01c27c04 "drivers: led: bcm6753: do not use null label to find the
>> top"
>>
>> Binding the same driver to the top node as to the individual child
>> nodes is arguably wrong, and the approach of using a UCLASS_NOP driver
>> for the top node is probably better.
> 
> Note that
> 83c63f0d1185 ("led: Move OF "label" property parsing to core")
> and
> 01074697801b ("led: gpio: Use NOP uclass driver for top-level node")
> were applied shortly after each other, so I don't see the point of the
> aforementioned rant.

What rant? I'm merely trying to write down what I found out while trying
to figure out why 83c63f0d1185 broke stuff, while acknowledging that the
fixes that have been applied to some drivers are probably the approach
in general.

> I sort-of understand what you are trying to do in this patch based on
> $SUBJECT of this email, but not from this wall of text, so can you
> abbreviate the commit message ?

Sure, I can try and make it shorter.

>> -    if (!uc_plat->label)
>> +    if (!uc_plat->label && !dev_read_string(dev, "compatible"))
>>           uc_plat->label = ofnode_get_name(dev_ofnode(dev));
> 
> Is there an existing driver which has a top-level DT node with "label"
> property ?

-ENOPARSE? A driver doesn't have a top-level DT node.

And regardless, this wouldn't change anything for a top-level DT node
with a "label" property, as we're still unconditionally first trying to
read a "label" property, this is merely avoiding adding a fallback value
for top-level DT nodes (using "having a "compatible" DT property as
proxy for that "is a top-level DT node"). So I really don't understand
what you are trying to ask.

Rasmus