Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs

[Alexander Dahl <ada@thorsis.com>]

Hello Jacek,

Am Sun, May 18, 2025 at 04:36:52PM +0200 schrieb Jacek Anaszewski:
> Hi Alexander,
> 
> On 5/16/25 09:35, Alexander Dahl wrote:
> > Hei hei,
> > 
> > just wanted to create a new thread on a similar topic, but this is so
> > close, just hooking in here …
> > 
> > Am Sat, May 10, 2025 at 02:43:45PM +0200 schrieb Jacek Anaszewski:
> > > Hi all,

[…]

> > > The question is if the LED name from the schematics tells anything to
> > > the user of the equipment?
> > > 
> > > The idea behind LED naming is to facilitate matching the LED class
> > > device name as reported by the system with the LED location on the
> > > equipment.
> > > 
> > > The LED naming standardization intended to enforce consistent
> > > LED naming, and not allowing to add multiple interchangeable
> > > names like wifi/wlan. It also helps to keep LED name sections order in
> > > accordance with Linux documentation, which before had been often
> > > abused by allowing to assign anything to the now deprecated 'label'
> > > DT property.
> > 
> > You see devicetree changes frequently which change the sysfs path of
> > existing LEDs, last example I saw today:
> > 
> > https://lore.kernel.org/linux-devicetree/20250513170056.96259-1-didi.debian@cknow.org/
> > 
> > Consider this change:
> > 
> >   		led-lan1 {
> >   			color = <LED_COLOR_ID_GREEN>;
> > +			default-state = "off";
> >   			function = LED_FUNCTION_LAN;
> >   			function-enumerator = <1>;
> >   			gpios = <&gpio3 RK_PD6 GPIO_ACTIVE_HIGH>;
> > +			label = "LAN-1";
> 
> So this change was made without understanding how LED naming works,
> and without reading LED common bindings [0], which clearly states
> that 'label' property is deprecated. It makes no sense to add 'label'
> when there are already 'function' and 'color' properties present.
> Label takes precedence to keep backwards compatibility.
> 
> > +			linux,default-trigger = "netdev";
> >   		};
> > 
> > Before the sysfs path probably was /sys/class/leds/green:lan-1 and
> > with the addition of the label property now it's probably
> > /sys/class/leds/LAN-1 … so it changed.  This might break userspace,
> > which relies on certain sysfs paths, maybe.
> > 
> > The main question is: Is that sysfs path considered to be a stable
> > interface for accessing a particular LED or not?
> 
> It should be stable, but since LED sysfs interface is influenced by
> DT implementation, then the responsibility for keeping it stable is on
> given dts file maintainer.

Okay thanks for clarification.

Follow-up question: should the linux-leds list be included in Cc if
someone changes LED related DTS properties?  This is often not the
case, like in the case quoted above.

> > I've seen this pattern also the other way round, were an old dts only
> > has the node name determing the sysfs path, people change the node
> > name or add color/function properties, gone is the supposedly stable
> > path.
> > 
> > New idea: what about making this somewhat more flexible and less
> > suprising by _always_ creating the standardized sysfs entry based on
> > color/function by default, and let label only create an additional
> > symlink linking to that?
> > 
> > So in the above example /sys/class/leds/green:lan-1 would be the
> > canonical name/path of that LED, and /sys/class/leds/LAN-1 would only
> > be a symlink on it?
> 
> IMO it would be cheaper to keep DTS implementation stable.
> 
> [0] Documentation/devicetree/bindings/leds/common.yaml

Ack.  Sounds for me like it would be okay to point users to those
bindings and the deprecation notice, if one stumbles over such changes
on the devicetree list?

Greets
Alex