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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/19/25 09:37, Alexander Dahl wrote:
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@xxxxxxxxx/

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.

It would for sure allow to limit improper application of
LED common bindings.

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?

LED common bindings should be always the main source of truth when
adding LED controller node to a dts file.

--
Best regards,
Jacek Anaszewski





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux