devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
@ 2025-04-17  9:28 Wolfram Sang
  2025-04-17 11:39 ` Geert Uytterhoeven
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2025-04-17  9:28 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Wolfram Sang, Geert Uytterhoeven, Magnus Damm, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes since v2:
* using function, color, function-enumerator properties now

Honestly, this is better than using node names? With V2, the LEDs were
named as in the schematics, now they are called:

lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
...

Which gets even more confusing if we might later add LEDs not on this
board, but on the expansion board. 'green:programming-8' sits where?

I really wonder, but if this is the official way now...

 .../dts/renesas/r9a06g032-rzn1d400-db.dts     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dts b/arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dts
index fef40e288679..16babb38eb05 100644
--- a/arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dts
+++ b/arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dts
@@ -10,6 +10,7 @@
 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
 #include <dt-bindings/net/pcs-rzn1-miic.h>
 #include <dt-bindings/pinctrl/rzn1-pinctrl.h>
 
@@ -86,7 +87,74 @@ switch-8 {
 			debounce-interval = <20>;
 			gpios = <&pca9698 15 GPIO_ACTIVE_LOW>;
 		};
+	};
+
+	leds {
+		compatible = "gpio-leds";
 
+		led-dbg0 {
+			gpios = <&pca9698 0 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_PROGRAMMING;
+			function-enumerator = <0>;
+			default-state = "keep";
+		};
+
+		led-dbg1 {
+			gpios = <&pca9698 1 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_PROGRAMMING;
+			function-enumerator = <1>;
+			default-state = "keep";
+		};
+
+		led-dbg2 {
+			gpios = <&pca9698 2 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_PROGRAMMING;
+			function-enumerator = <2>;
+			default-state = "keep";
+		};
+
+		led-dbg3 {
+			gpios = <&pca9698 3 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_PROGRAMMING;
+			function-enumerator = <3>;
+			default-state = "keep";
+		};
+
+		led-dbg4 {
+			gpios = <&pca9698 4 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_PROGRAMMING;
+			function-enumerator = <4>;
+			default-state = "keep";
+		};
+
+		led-dbg5 {
+			gpios = <&pca9698 5 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_PROGRAMMING;
+			function-enumerator = <5>;
+			default-state = "keep";
+		};
+
+		led-dbg6 {
+			gpios = <&pca9698 6 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_PROGRAMMING;
+			function-enumerator = <6>;
+			default-state = "keep";
+		};
+
+		led-dbg7 {
+			gpios = <&pca9698 7 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_PROGRAMMING;
+			function-enumerator = <7>;
+			default-state = "keep";
+		};
 	};
 };
 
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-04-17  9:28 [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs Wolfram Sang
@ 2025-04-17 11:39 ` Geert Uytterhoeven
  2025-05-08  7:56   ` Wolfram Sang
  0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2025-04-17 11:39 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, devicetree, Lee Jones, Pavel Machek, linux-leds

Hi Wolfram,

CC leds

On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Changes since v2:
> * using function, color, function-enumerator properties now
>
> Honestly, this is better than using node names? With V2, the LEDs were
> named as in the schematics, now they are called:
>
> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
> ...
>
> Which gets even more confusing if we might later add LEDs not on this
> board, but on the expansion board. 'green:programming-8' sits where?
>
> I really wonder, but if this is the official way now...

Good point!  So I'm inclined to take v2...

Let's raise this with the LED people. I don't want to fight Pavel when
v2 hits the CiP tree ;-)

> --- a/arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dts
> +++ b/arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dts
> @@ -10,6 +10,7 @@
>
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
>  #include <dt-bindings/net/pcs-rzn1-miic.h>
>  #include <dt-bindings/pinctrl/rzn1-pinctrl.h>
>
> @@ -86,7 +87,74 @@ switch-8 {
>                         debounce-interval = <20>;
>                         gpios = <&pca9698 15 GPIO_ACTIVE_LOW>;
>                 };
> +       };
> +
> +       leds {
> +               compatible = "gpio-leds";
>
> +               led-dbg0 {
> +                       gpios = <&pca9698 0 GPIO_ACTIVE_HIGH>;
> +                       color = <LED_COLOR_ID_GREEN>;
> +                       function = LED_FUNCTION_PROGRAMMING;
> +                       function-enumerator = <0>;
> +                       default-state = "keep";
> +               };
> +
> +               led-dbg1 {
> +                       gpios = <&pca9698 1 GPIO_ACTIVE_HIGH>;
> +                       color = <LED_COLOR_ID_GREEN>;
> +                       function = LED_FUNCTION_PROGRAMMING;
> +                       function-enumerator = <1>;
> +                       default-state = "keep";
> +               };
> +
> +               led-dbg2 {
> +                       gpios = <&pca9698 2 GPIO_ACTIVE_HIGH>;
> +                       color = <LED_COLOR_ID_GREEN>;
> +                       function = LED_FUNCTION_PROGRAMMING;
> +                       function-enumerator = <2>;
> +                       default-state = "keep";
> +               };
> +
> +               led-dbg3 {
> +                       gpios = <&pca9698 3 GPIO_ACTIVE_HIGH>;
> +                       color = <LED_COLOR_ID_GREEN>;
> +                       function = LED_FUNCTION_PROGRAMMING;
> +                       function-enumerator = <3>;
> +                       default-state = "keep";
> +               };
> +
> +               led-dbg4 {
> +                       gpios = <&pca9698 4 GPIO_ACTIVE_HIGH>;
> +                       color = <LED_COLOR_ID_GREEN>;
> +                       function = LED_FUNCTION_PROGRAMMING;
> +                       function-enumerator = <4>;
> +                       default-state = "keep";
> +               };
> +
> +               led-dbg5 {
> +                       gpios = <&pca9698 5 GPIO_ACTIVE_HIGH>;
> +                       color = <LED_COLOR_ID_GREEN>;
> +                       function = LED_FUNCTION_PROGRAMMING;
> +                       function-enumerator = <5>;
> +                       default-state = "keep";
> +               };
> +
> +               led-dbg6 {
> +                       gpios = <&pca9698 6 GPIO_ACTIVE_HIGH>;
> +                       color = <LED_COLOR_ID_GREEN>;
> +                       function = LED_FUNCTION_PROGRAMMING;
> +                       function-enumerator = <6>;
> +                       default-state = "keep";
> +               };
> +
> +               led-dbg7 {
> +                       gpios = <&pca9698 7 GPIO_ACTIVE_HIGH>;
> +                       color = <LED_COLOR_ID_GREEN>;
> +                       function = LED_FUNCTION_PROGRAMMING;
> +                       function-enumerator = <7>;
> +                       default-state = "keep";
> +               };
>         };
>  };
>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-04-17 11:39 ` Geert Uytterhoeven
@ 2025-05-08  7:56   ` Wolfram Sang
  2025-05-08 13:49     ` Lee Jones
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2025-05-08  7:56 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-renesas-soc, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, devicetree, Lee Jones, Pavel Machek, linux-leds

[-- Attachment #1: Type: text/plain, Size: 5464 bytes --]

On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> CC leds
> 
> On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >
> > Changes since v2:
> > * using function, color, function-enumerator properties now
> >
> > Honestly, this is better than using node names? With V2, the LEDs were
> > named as in the schematics, now they are called:
> >
> > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
> > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
> > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
> > ...
> >
> > Which gets even more confusing if we might later add LEDs not on this
> > board, but on the expansion board. 'green:programming-8' sits where?
> >
> > I really wonder, but if this is the official way now...
> 
> Good point!  So I'm inclined to take v2...
> 
> Let's raise this with the LED people. I don't want to fight Pavel when
> v2 hits the CiP tree ;-)

So, if there is no other opinion here, can we remove function, color,
function-enumerator and just use the node names which match the
schematics? Basically apply V2?

> 
> > --- a/arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dts
> > +++ b/arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dts
> > @@ -10,6 +10,7 @@
> >
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/input/input.h>
> > +#include <dt-bindings/leds/common.h>
> >  #include <dt-bindings/net/pcs-rzn1-miic.h>
> >  #include <dt-bindings/pinctrl/rzn1-pinctrl.h>
> >
> > @@ -86,7 +87,74 @@ switch-8 {
> >                         debounce-interval = <20>;
> >                         gpios = <&pca9698 15 GPIO_ACTIVE_LOW>;
> >                 };
> > +       };
> > +
> > +       leds {
> > +               compatible = "gpio-leds";
> >
> > +               led-dbg0 {
> > +                       gpios = <&pca9698 0 GPIO_ACTIVE_HIGH>;
> > +                       color = <LED_COLOR_ID_GREEN>;
> > +                       function = LED_FUNCTION_PROGRAMMING;
> > +                       function-enumerator = <0>;
> > +                       default-state = "keep";
> > +               };
> > +
> > +               led-dbg1 {
> > +                       gpios = <&pca9698 1 GPIO_ACTIVE_HIGH>;
> > +                       color = <LED_COLOR_ID_GREEN>;
> > +                       function = LED_FUNCTION_PROGRAMMING;
> > +                       function-enumerator = <1>;
> > +                       default-state = "keep";
> > +               };
> > +
> > +               led-dbg2 {
> > +                       gpios = <&pca9698 2 GPIO_ACTIVE_HIGH>;
> > +                       color = <LED_COLOR_ID_GREEN>;
> > +                       function = LED_FUNCTION_PROGRAMMING;
> > +                       function-enumerator = <2>;
> > +                       default-state = "keep";
> > +               };
> > +
> > +               led-dbg3 {
> > +                       gpios = <&pca9698 3 GPIO_ACTIVE_HIGH>;
> > +                       color = <LED_COLOR_ID_GREEN>;
> > +                       function = LED_FUNCTION_PROGRAMMING;
> > +                       function-enumerator = <3>;
> > +                       default-state = "keep";
> > +               };
> > +
> > +               led-dbg4 {
> > +                       gpios = <&pca9698 4 GPIO_ACTIVE_HIGH>;
> > +                       color = <LED_COLOR_ID_GREEN>;
> > +                       function = LED_FUNCTION_PROGRAMMING;
> > +                       function-enumerator = <4>;
> > +                       default-state = "keep";
> > +               };
> > +
> > +               led-dbg5 {
> > +                       gpios = <&pca9698 5 GPIO_ACTIVE_HIGH>;
> > +                       color = <LED_COLOR_ID_GREEN>;
> > +                       function = LED_FUNCTION_PROGRAMMING;
> > +                       function-enumerator = <5>;
> > +                       default-state = "keep";
> > +               };
> > +
> > +               led-dbg6 {
> > +                       gpios = <&pca9698 6 GPIO_ACTIVE_HIGH>;
> > +                       color = <LED_COLOR_ID_GREEN>;
> > +                       function = LED_FUNCTION_PROGRAMMING;
> > +                       function-enumerator = <6>;
> > +                       default-state = "keep";
> > +               };
> > +
> > +               led-dbg7 {
> > +                       gpios = <&pca9698 7 GPIO_ACTIVE_HIGH>;
> > +                       color = <LED_COLOR_ID_GREEN>;
> > +                       function = LED_FUNCTION_PROGRAMMING;
> > +                       function-enumerator = <7>;
> > +                       default-state = "keep";
> > +               };
> >         };
> >  };
> >
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-08  7:56   ` Wolfram Sang
@ 2025-05-08 13:49     ` Lee Jones
  2025-05-10 12:43       ` Jacek Anaszewski
  0 siblings, 1 reply; 18+ messages in thread
From: Lee Jones @ 2025-05-08 13:49 UTC (permalink / raw)
  To: Wolfram Sang, Geert Uytterhoeven, linux-renesas-soc, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	Pavel Machek, linux-leds, Jacek Anaszewski

On Thu, 08 May 2025, Wolfram Sang wrote:

> On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
> > Hi Wolfram,
> > 
> > CC leds
> > 
> > On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
> > <wsa+renesas@sang-engineering.com> wrote:
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > ---
> > >
> > > Changes since v2:
> > > * using function, color, function-enumerator properties now
> > >
> > > Honestly, this is better than using node names? With V2, the LEDs were
> > > named as in the schematics, now they are called:
> > >
> > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
> > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
> > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
> > > ...
> > >
> > > Which gets even more confusing if we might later add LEDs not on this
> > > board, but on the expansion board. 'green:programming-8' sits where?
> > >
> > > I really wonder, but if this is the official way now...
> > 
> > Good point!  So I'm inclined to take v2...
> > 
> > Let's raise this with the LED people. I don't want to fight Pavel when
> > v2 hits the CiP tree ;-)
> 
> So, if there is no other opinion here, can we remove function, color,
> function-enumerator and just use the node names which match the
> schematics? Basically apply V2?

I didn't author the semantics nor the rules surrounding them, but I am
obliged to enforce them.  Therefore "LED people" say, please stick to
convention as stated in the present documentation:

https://docs.kernel.org/leds/leds-class.html#led-device-naming

Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
that is more appropriate to your use-case.

Let's also bring Jacek into the conversion, since I know that he did a
bunch of work around this topic.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-08 13:49     ` Lee Jones
@ 2025-05-10 12:43       ` Jacek Anaszewski
  2025-05-12  7:13         ` Geert Uytterhoeven
  2025-05-16  7:35         ` Alexander Dahl
  0 siblings, 2 replies; 18+ messages in thread
From: Jacek Anaszewski @ 2025-05-10 12:43 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Geert Uytterhoeven, linux-renesas-soc,
	Magnus Damm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, Pavel Machek, linux-leds

Hi all,

On 5/8/25 15:49, Lee Jones wrote:
> On Thu, 08 May 2025, Wolfram Sang wrote:
> 
>> On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
>>> Hi Wolfram,
>>>
>>> CC leds
>>>
>>> On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
>>> <wsa+renesas@sang-engineering.com> wrote:
>>>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> ---
>>>>
>>>> Changes since v2:
>>>> * using function, color, function-enumerator properties now
>>>>
>>>> Honestly, this is better than using node names? With V2, the LEDs were
>>>> named as in the schematics, now they are called:
>>>>
>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
>>>> ...
>>>>
>>>> Which gets even more confusing if we might later add LEDs not on this
>>>> board, but on the expansion board. 'green:programming-8' sits where?
>>>>
>>>> I really wonder, but if this is the official way now...
>>>
>>> Good point!  So I'm inclined to take v2...
>>>
>>> Let's raise this with the LED people. I don't want to fight Pavel when
>>> v2 hits the CiP tree ;-)
>>
>> So, if there is no other opinion here, can we remove function, color,
>> function-enumerator and just use the node names which match the
>> schematics? Basically apply V2?
> 
> I didn't author the semantics nor the rules surrounding them, but I am
> obliged to enforce them.  Therefore "LED people" say, please stick to
> convention as stated in the present documentation:
> 
> https://docs.kernel.org/leds/leds-class.html#led-device-naming
> 
> Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
> that is more appropriate to your use-case.
> 
> Let's also bring Jacek into the conversion, since I know that he did a
> bunch of work around this topic.

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.

Regarding expansion boards - we never have control over what
LED names DT overlays will define, thus LED core adds numeric suffix to
the LED class device name in case of the name clash.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-10 12:43       ` Jacek Anaszewski
@ 2025-05-12  7:13         ` Geert Uytterhoeven
  2025-05-12 17:54           ` Jacek Anaszewski
  2025-05-16  7:35         ` Alexander Dahl
  1 sibling, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2025-05-12  7:13 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Lee Jones, Wolfram Sang, linux-renesas-soc, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	Pavel Machek, linux-leds

Hi Jacek,

Thanks for your answer!

On Sat, 10 May 2025 at 14:43, Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:
> On 5/8/25 15:49, Lee Jones wrote:
> > On Thu, 08 May 2025, Wolfram Sang wrote:
> >> On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
> >>> On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
> >>> <wsa+renesas@sang-engineering.com> wrote:
> >>>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >>>> ---
> >>>>
> >>>> Changes since v2:
> >>>> * using function, color, function-enumerator properties now
> >>>>
> >>>> Honestly, this is better than using node names? With V2, the LEDs were
> >>>> named as in the schematics, now they are called:
> >>>>
> >>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
> >>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
> >>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
> >>>> ...
> >>>>
> >>>> Which gets even more confusing if we might later add LEDs not on this
> >>>> board, but on the expansion board. 'green:programming-8' sits where?
> >>>>
> >>>> I really wonder, but if this is the official way now...
> >>>
> >>> Good point!  So I'm inclined to take v2...
> >>>
> >>> Let's raise this with the LED people. I don't want to fight Pavel when
> >>> v2 hits the CiP tree ;-)
> >>
> >> So, if there is no other opinion here, can we remove function, color,
> >> function-enumerator and just use the node names which match the
> >> schematics? Basically apply V2?
> >
> > I didn't author the semantics nor the rules surrounding them, but I am
> > obliged to enforce them.  Therefore "LED people" say, please stick to
> > convention as stated in the present documentation:
> >
> > https://docs.kernel.org/leds/leds-class.html#led-device-naming
> >
> > Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
> > that is more appropriate to your use-case.
> >
> > Let's also bring Jacek into the conversion, since I know that he did a
> > bunch of work around this topic.
>
> The question is if the LED name from the schematics tells anything to
> the user of the equipment?

As this is a development board and not a finished product, I would
answer yes.

> 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.

I agree this all makes perfect sense for a final product, where the
purpose of each LED is clear, and sometimes indicated by an icon
on the case.
For a development board, some LEDs may have a fixed purpose.
But typically there is also a collection of generic user LEDs, which
do not have a fixed purpose, and are identified by a label on the
schematics.  Imposing an arbitrary numbering scheme on the latter is
confusing for the user (developer).

> Regarding expansion boards - we never have control over what
> LED names DT overlays will define, thus LED core adds numeric suffix to
> the LED class device name in case of the name clash.

FTR, the RZN1D400 Expansion Board does not use a DT overlay.
Linux carries a DTS for it, which just includes the base board .dts,
and treats it as a single system.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-12  7:13         ` Geert Uytterhoeven
@ 2025-05-12 17:54           ` Jacek Anaszewski
  2025-05-14 15:28             ` Lee Jones
  0 siblings, 1 reply; 18+ messages in thread
From: Jacek Anaszewski @ 2025-05-12 17:54 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Lee Jones, Wolfram Sang, linux-renesas-soc, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	Pavel Machek, linux-leds

Hi Geert,

On 5/12/25 09:13, Geert Uytterhoeven wrote:
> Hi Jacek,
> 
> Thanks for your answer!

You're welcome.

> On Sat, 10 May 2025 at 14:43, Jacek Anaszewski
> <jacek.anaszewski@gmail.com> wrote:
>> On 5/8/25 15:49, Lee Jones wrote:
>>> On Thu, 08 May 2025, Wolfram Sang wrote:
>>>> On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
>>>>> On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
>>>>> <wsa+renesas@sang-engineering.com> wrote:
>>>>>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>>>> ---
>>>>>>
>>>>>> Changes since v2:
>>>>>> * using function, color, function-enumerator properties now
>>>>>>
>>>>>> Honestly, this is better than using node names? With V2, the LEDs were
>>>>>> named as in the schematics, now they are called:
>>>>>>
>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
>>>>>> ...
>>>>>>
>>>>>> Which gets even more confusing if we might later add LEDs not on this
>>>>>> board, but on the expansion board. 'green:programming-8' sits where?
>>>>>>
>>>>>> I really wonder, but if this is the official way now...
>>>>>
>>>>> Good point!  So I'm inclined to take v2...
>>>>>
>>>>> Let's raise this with the LED people. I don't want to fight Pavel when
>>>>> v2 hits the CiP tree ;-)
>>>>
>>>> So, if there is no other opinion here, can we remove function, color,
>>>> function-enumerator and just use the node names which match the
>>>> schematics? Basically apply V2?
>>>
>>> I didn't author the semantics nor the rules surrounding them, but I am
>>> obliged to enforce them.  Therefore "LED people" say, please stick to
>>> convention as stated in the present documentation:
>>>
>>> https://docs.kernel.org/leds/leds-class.html#led-device-naming
>>>
>>> Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
>>> that is more appropriate to your use-case.
>>>
>>> Let's also bring Jacek into the conversion, since I know that he did a
>>> bunch of work around this topic.
>>
>> The question is if the LED name from the schematics tells anything to
>> the user of the equipment?
> 
> As this is a development board and not a finished product, I would
> answer yes.

OK.

>> 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.
> 
> I agree this all makes perfect sense for a final product, where the
> purpose of each LED is clear, and sometimes indicated by an icon
> on the case.
> For a development board, some LEDs may have a fixed purpose.
> But typically there is also a collection of generic user LEDs, which
> do not have a fixed purpose, and are identified by a label on the
> schematics.  Imposing an arbitrary numbering scheme on the latter is
> confusing for the user (developer).

Using DT child node name for LED class device name is only
a last resort fallback. However if it is devboard and we want to have
a reference to the schematics then I'd say it makes sense to take
LED names from DT nodes. What about the colors? Are the LEDs replaceable
or soldered?

>> Regarding expansion boards - we never have control over what
>> LED names DT overlays will define, thus LED core adds numeric suffix to
>> the LED class device name in case of the name clash.
> 
> FTR, the RZN1D400 Expansion Board does not use a DT overlay.
> Linux carries a DTS for it, which just includes the base board .dts,
> and treats it as a single system.

Ack.

-- 
Best regards,
Jacek Anaszewski


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-12 17:54           ` Jacek Anaszewski
@ 2025-05-14 15:28             ` Lee Jones
  2025-05-14 18:57               ` Jacek Anaszewski
  0 siblings, 1 reply; 18+ messages in thread
From: Lee Jones @ 2025-05-14 15:28 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	Pavel Machek, linux-leds

On Mon, 12 May 2025, Jacek Anaszewski wrote:

> Hi Geert,
> 
> On 5/12/25 09:13, Geert Uytterhoeven wrote:
> > Hi Jacek,
> > 
> > Thanks for your answer!
> 
> You're welcome.
> 
> > On Sat, 10 May 2025 at 14:43, Jacek Anaszewski
> > <jacek.anaszewski@gmail.com> wrote:
> > > On 5/8/25 15:49, Lee Jones wrote:
> > > > On Thu, 08 May 2025, Wolfram Sang wrote:
> > > > > On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
> > > > > > On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
> > > > > > <wsa+renesas@sang-engineering.com> wrote:
> > > > > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > > > > > ---
> > > > > > > 
> > > > > > > Changes since v2:
> > > > > > > * using function, color, function-enumerator properties now
> > > > > > > 
> > > > > > > Honestly, this is better than using node names? With V2, the LEDs were
> > > > > > > named as in the schematics, now they are called:
> > > > > > > 
> > > > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
> > > > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
> > > > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
> > > > > > > ...
> > > > > > > 
> > > > > > > Which gets even more confusing if we might later add LEDs not on this
> > > > > > > board, but on the expansion board. 'green:programming-8' sits where?
> > > > > > > 
> > > > > > > I really wonder, but if this is the official way now...
> > > > > > 
> > > > > > Good point!  So I'm inclined to take v2...
> > > > > > 
> > > > > > Let's raise this with the LED people. I don't want to fight Pavel when
> > > > > > v2 hits the CiP tree ;-)
> > > > > 
> > > > > So, if there is no other opinion here, can we remove function, color,
> > > > > function-enumerator and just use the node names which match the
> > > > > schematics? Basically apply V2?
> > > > 
> > > > I didn't author the semantics nor the rules surrounding them, but I am
> > > > obliged to enforce them.  Therefore "LED people" say, please stick to
> > > > convention as stated in the present documentation:
> > > > 
> > > > https://docs.kernel.org/leds/leds-class.html#led-device-naming
> > > > 
> > > > Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
> > > > that is more appropriate to your use-case.
> > > > 
> > > > Let's also bring Jacek into the conversion, since I know that he did a
> > > > bunch of work around this topic.
> > > 
> > > The question is if the LED name from the schematics tells anything to
> > > the user of the equipment?
> > 
> > As this is a development board and not a finished product, I would
> > answer yes.
> 
> OK.
> 
> > > 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.
> > 
> > I agree this all makes perfect sense for a final product, where the
> > purpose of each LED is clear, and sometimes indicated by an icon
> > on the case.
> > For a development board, some LEDs may have a fixed purpose.
> > But typically there is also a collection of generic user LEDs, which
> > do not have a fixed purpose, and are identified by a label on the
> > schematics.  Imposing an arbitrary numbering scheme on the latter is
> > confusing for the user (developer).
> 
> Using DT child node name for LED class device name is only
> a last resort fallback. However if it is devboard and we want to have
> a reference to the schematics then I'd say it makes sense to take
> LED names from DT nodes. What about the colors? Are the LEDs replaceable
> or soldered?

Looks like this option does what you want:

https://github.com/torvalds/linux/blob/master/drivers/leds/led-core.c#L578

For this to execute you need to provide init_data when calling
*led_classdev_register*(), omit the; label, function, color_present DT
properties and also init_data's default_label attribute.  At which point
the DT node name should be taken as the LED class name.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-14 15:28             ` Lee Jones
@ 2025-05-14 18:57               ` Jacek Anaszewski
  2025-05-15  6:53                 ` Lee Jones
  2025-05-15  8:27                 ` Wolfram Sang
  0 siblings, 2 replies; 18+ messages in thread
From: Jacek Anaszewski @ 2025-05-14 18:57 UTC (permalink / raw)
  To: Lee Jones
  Cc: Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	Pavel Machek, linux-leds

On 5/14/25 17:28, Lee Jones wrote:
> On Mon, 12 May 2025, Jacek Anaszewski wrote:
> 
>> Hi Geert,
>>
>> On 5/12/25 09:13, Geert Uytterhoeven wrote:
>>> Hi Jacek,
>>>
>>> Thanks for your answer!
>>
>> You're welcome.
>>
>>> On Sat, 10 May 2025 at 14:43, Jacek Anaszewski
>>> <jacek.anaszewski@gmail.com> wrote:
>>>> On 5/8/25 15:49, Lee Jones wrote:
>>>>> On Thu, 08 May 2025, Wolfram Sang wrote:
>>>>>> On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
>>>>>>> On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
>>>>>>> <wsa+renesas@sang-engineering.com> wrote:
>>>>>>>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Changes since v2:
>>>>>>>> * using function, color, function-enumerator properties now
>>>>>>>>
>>>>>>>> Honestly, this is better than using node names? With V2, the LEDs were
>>>>>>>> named as in the schematics, now they are called:
>>>>>>>>
>>>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
>>>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
>>>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
>>>>>>>> ...
>>>>>>>>
>>>>>>>> Which gets even more confusing if we might later add LEDs not on this
>>>>>>>> board, but on the expansion board. 'green:programming-8' sits where?
>>>>>>>>
>>>>>>>> I really wonder, but if this is the official way now...
>>>>>>>
>>>>>>> Good point!  So I'm inclined to take v2...
>>>>>>>
>>>>>>> Let's raise this with the LED people. I don't want to fight Pavel when
>>>>>>> v2 hits the CiP tree ;-)
>>>>>>
>>>>>> So, if there is no other opinion here, can we remove function, color,
>>>>>> function-enumerator and just use the node names which match the
>>>>>> schematics? Basically apply V2?
>>>>>
>>>>> I didn't author the semantics nor the rules surrounding them, but I am
>>>>> obliged to enforce them.  Therefore "LED people" say, please stick to
>>>>> convention as stated in the present documentation:
>>>>>
>>>>> https://docs.kernel.org/leds/leds-class.html#led-device-naming
>>>>>
>>>>> Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
>>>>> that is more appropriate to your use-case.
>>>>>
>>>>> Let's also bring Jacek into the conversion, since I know that he did a
>>>>> bunch of work around this topic.
>>>>
>>>> The question is if the LED name from the schematics tells anything to
>>>> the user of the equipment?
>>>
>>> As this is a development board and not a finished product, I would
>>> answer yes.
>>
>> OK.
>>
>>>> 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.
>>>
>>> I agree this all makes perfect sense for a final product, where the
>>> purpose of each LED is clear, and sometimes indicated by an icon
>>> on the case.
>>> For a development board, some LEDs may have a fixed purpose.
>>> But typically there is also a collection of generic user LEDs, which
>>> do not have a fixed purpose, and are identified by a label on the
>>> schematics.  Imposing an arbitrary numbering scheme on the latter is
>>> confusing for the user (developer).
>>
>> Using DT child node name for LED class device name is only
>> a last resort fallback. However if it is devboard and we want to have
>> a reference to the schematics then I'd say it makes sense to take
>> LED names from DT nodes. What about the colors? Are the LEDs replaceable
>> or soldered?
> 
> Looks like this option does what you want:
> 
> https://github.com/torvalds/linux/blob/master/drivers/leds/led-core.c#L578
> 
> For this to execute you need to provide init_data when calling
> *led_classdev_register*(), omit the; label, function, color_present DT
> properties and also init_data's default_label attribute.  At which point
> the DT node name should be taken as the LED class name.

Yep, I know how it works, I wrote that code after all.
Here, I wanted to clarify whether it wouldn't make sense to stick to
the approach with 'function' and 'color' properties if LEDs are fixed
on the board and the colors are known.

 From [0] I infer that LEDs are green, so we should use 'color' DT
property definitely. And as a 'function' we can assign plain text "pcaN"
if you feel it makes sense for that board.

This way you'll get LED name "pcaN:green", that will be according to the
naming standard.

So the node would look like this, for the pca1 LED:

led-1 {
	function = "pca1";
	color = <LED_COLOR_GREEN>;
	default-state = "keep";
};

[0] 
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20250328153134.2881-11-wsa+renesas@sang-engineering.com/#26336000

-- 
Best regards,
Jacek Anaszewski


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-14 18:57               ` Jacek Anaszewski
@ 2025-05-15  6:53                 ` Lee Jones
  2025-05-15 20:14                   ` Jacek Anaszewski
  2025-05-15  8:27                 ` Wolfram Sang
  1 sibling, 1 reply; 18+ messages in thread
From: Lee Jones @ 2025-05-15  6:53 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	Pavel Machek, linux-leds

On Wed, 14 May 2025, Jacek Anaszewski wrote:

> On 5/14/25 17:28, Lee Jones wrote:
> > On Mon, 12 May 2025, Jacek Anaszewski wrote:
> > 
> > > Hi Geert,
> > > 
> > > On 5/12/25 09:13, Geert Uytterhoeven wrote:
> > > > Hi Jacek,
> > > > 
> > > > Thanks for your answer!
> > > 
> > > You're welcome.
> > > 
> > > > On Sat, 10 May 2025 at 14:43, Jacek Anaszewski
> > > > <jacek.anaszewski@gmail.com> wrote:
> > > > > On 5/8/25 15:49, Lee Jones wrote:
> > > > > > On Thu, 08 May 2025, Wolfram Sang wrote:
> > > > > > > On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
> > > > > > > > On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
> > > > > > > > <wsa+renesas@sang-engineering.com> wrote:
> > > > > > > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > > > > > > > ---
> > > > > > > > > 
> > > > > > > > > Changes since v2:
> > > > > > > > > * using function, color, function-enumerator properties now
> > > > > > > > > 
> > > > > > > > > Honestly, this is better than using node names? With V2, the LEDs were
> > > > > > > > > named as in the schematics, now they are called:
> > > > > > > > > 
> > > > > > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
> > > > > > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
> > > > > > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
> > > > > > > > > ...
> > > > > > > > > 
> > > > > > > > > Which gets even more confusing if we might later add LEDs not on this
> > > > > > > > > board, but on the expansion board. 'green:programming-8' sits where?
> > > > > > > > > 
> > > > > > > > > I really wonder, but if this is the official way now...
> > > > > > > > 
> > > > > > > > Good point!  So I'm inclined to take v2...
> > > > > > > > 
> > > > > > > > Let's raise this with the LED people. I don't want to fight Pavel when
> > > > > > > > v2 hits the CiP tree ;-)
> > > > > > > 
> > > > > > > So, if there is no other opinion here, can we remove function, color,
> > > > > > > function-enumerator and just use the node names which match the
> > > > > > > schematics? Basically apply V2?
> > > > > > 
> > > > > > I didn't author the semantics nor the rules surrounding them, but I am
> > > > > > obliged to enforce them.  Therefore "LED people" say, please stick to
> > > > > > convention as stated in the present documentation:
> > > > > > 
> > > > > > https://docs.kernel.org/leds/leds-class.html#led-device-naming
> > > > > > 
> > > > > > Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
> > > > > > that is more appropriate to your use-case.
> > > > > > 
> > > > > > Let's also bring Jacek into the conversion, since I know that he did a
> > > > > > bunch of work around this topic.
> > > > > 
> > > > > The question is if the LED name from the schematics tells anything to
> > > > > the user of the equipment?
> > > > 
> > > > As this is a development board and not a finished product, I would
> > > > answer yes.
> > > 
> > > OK.
> > > 
> > > > > 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.
> > > > 
> > > > I agree this all makes perfect sense for a final product, where the
> > > > purpose of each LED is clear, and sometimes indicated by an icon
> > > > on the case.
> > > > For a development board, some LEDs may have a fixed purpose.
> > > > But typically there is also a collection of generic user LEDs, which
> > > > do not have a fixed purpose, and are identified by a label on the
> > > > schematics.  Imposing an arbitrary numbering scheme on the latter is
> > > > confusing for the user (developer).
> > > 
> > > Using DT child node name for LED class device name is only
> > > a last resort fallback. However if it is devboard and we want to have
> > > a reference to the schematics then I'd say it makes sense to take
> > > LED names from DT nodes. What about the colors? Are the LEDs replaceable
> > > or soldered?
> > 
> > Looks like this option does what you want:
> > 
> > https://github.com/torvalds/linux/blob/master/drivers/leds/led-core.c#L578
> > 
> > For this to execute you need to provide init_data when calling
> > *led_classdev_register*(), omit the; label, function, color_present DT
> > properties and also init_data's default_label attribute.  At which point
> > the DT node name should be taken as the LED class name.
> 
> Yep, I know how it works, I wrote that code after all.

Sorry for the ambiguity.  I was attempting to address Wolfram.

> Here, I wanted to clarify whether it wouldn't make sense to stick to
> the approach with 'function' and 'color' properties if LEDs are fixed
> on the board and the colors are known.
> 
> From [0] I infer that LEDs are green, so we should use 'color' DT
> property definitely. And as a 'function' we can assign plain text "pcaN"
> if you feel it makes sense for that board.

The 'function' documentation specifically says to use one of the
predefined LED_FUNCTION_* entries.  If we are officially accepting
caveats to that, we should probably update it.

> This way you'll get LED name "pcaN:green", that will be according to
> the naming standard.
> 
> So the node would look like this, for the pca1 LED:
> 
> led-1 { function = "pca1"; color = <LED_COLOR_GREEN>; default-state =
> "keep"; };
> 
> [0]
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20250328153134.2881-11-wsa+renesas@sang-engineering.com/#26336000

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-14 18:57               ` Jacek Anaszewski
  2025-05-15  6:53                 ` Lee Jones
@ 2025-05-15  8:27                 ` Wolfram Sang
  2025-05-15  9:15                   ` Lee Jones
  2025-05-15 19:53                   ` Jacek Anaszewski
  1 sibling, 2 replies; 18+ messages in thread
From: Wolfram Sang @ 2025-05-15  8:27 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Lee Jones, Geert Uytterhoeven, linux-renesas-soc, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	Pavel Machek, linux-leds

[-- Attachment #1: Type: text/plain, Size: 1122 bytes --]

Hi all,

thank you for providing all this input. I appreciate this a lot. And
please excuse the slow response. I am currently at the EmbeddedRecipes
conference which needed a bit of preparation on my side.

> So the node would look like this, for the pca1 LED:
> 
> led-1 {
> 	function = "pca1";
> 	color = <LED_COLOR_GREEN>;
> 	default-state = "keep";
> };

This looks optimal to me, if this is acceptable. I totally understand
the advantages and desire to unify LED naming. The main problem for me
here is that the GPIO-driven LEDs have no 'device' part in the generic
name. And only 'function:color' seems suboptimal for the board here in
question. I kinda arranged with the option of using "LED_FUNCTION_DEBUG"
for the above LEDs and some other function for the LED on the carrier
board. This seems OK enough for a development board, but ideal would be
the above solution. So, if you can live with the above, I'll happily
make use of it. If you want me to live with the different
LED_FUNCTION_* solution, I will survive this as well...

Happy hacking and greetings from Nice,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-15  8:27                 ` Wolfram Sang
@ 2025-05-15  9:15                   ` Lee Jones
  2025-05-15 19:53                   ` Jacek Anaszewski
  1 sibling, 0 replies; 18+ messages in thread
From: Lee Jones @ 2025-05-15  9:15 UTC (permalink / raw)
  To: Wolfram Sang, Jacek Anaszewski, Geert Uytterhoeven,
	linux-renesas-soc, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, devicetree, Pavel Machek, linux-leds

On Thu, 15 May 2025, Wolfram Sang wrote:

> Hi all,
> 
> thank you for providing all this input. I appreciate this a lot. And
> please excuse the slow response. I am currently at the EmbeddedRecipes
> conference which needed a bit of preparation on my side.
> 
> > So the node would look like this, for the pca1 LED:
> > 
> > led-1 {
> > 	function = "pca1";
> > 	color = <LED_COLOR_GREEN>;
> > 	default-state = "keep";
> > };
> 
> This looks optimal to me, if this is acceptable. I totally understand
> the advantages and desire to unify LED naming. The main problem for me
> here is that the GPIO-driven LEDs have no 'device' part in the generic
> name. And only 'function:color' seems suboptimal for the board here in
> question. I kinda arranged with the option of using "LED_FUNCTION_DEBUG"
> for the above LEDs and some other function for the LED on the carrier
> board. This seems OK enough for a development board, but ideal would be
> the above solution. So, if you can live with the above, I'll happily
> make use of it. If you want me to live with the different
> LED_FUNCTION_* solution, I will survive this as well...

My only fear would be one of setting a precedent for bespoke function
strings.  However, seeing as we already have some that slipped through
the gaps [0], I guess one more wouldn't hurt.

FTR however, my preference would be to use LED_FUNCTION_DEBUG as
previously discussed.  If you can fight your slightly leaning partiality
for a bespoke string, please choose one of the predefined options.  If
you cannot live with it, go ahead with the bespoke option.

[0]
 arch/arm/boot/dts/broadcom/bcm6846-genexis-xg6846b.dts                
 arch/arm/boot/dts/intel/ixp/intel-ixp42x-netgear-wg302v1.dts          
 arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-netgear-r8000p.dts        
 arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-tplink-archer-c2300-v1.dts


-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-15  8:27                 ` Wolfram Sang
  2025-05-15  9:15                   ` Lee Jones
@ 2025-05-15 19:53                   ` Jacek Anaszewski
  1 sibling, 0 replies; 18+ messages in thread
From: Jacek Anaszewski @ 2025-05-15 19:53 UTC (permalink / raw)
  To: Wolfram Sang, Lee Jones, Geert Uytterhoeven, linux-renesas-soc,
	Magnus Damm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, Pavel Machek, linux-leds

Hi Wolfram,

On 5/15/25 10:27, Wolfram Sang wrote:
> Hi all,
> 
> thank you for providing all this input. I appreciate this a lot. And
> please excuse the slow response. I am currently at the EmbeddedRecipes
> conference which needed a bit of preparation on my side.
> 
>> So the node would look like this, for the pca1 LED:
>>
>> led-1 {
>> 	function = "pca1";
>> 	color = <LED_COLOR_GREEN>;
>> 	default-state = "keep";
>> };
> 
> This looks optimal to me, if this is acceptable. I totally understand
> the advantages and desire to unify LED naming. The main problem for me
> here is that the GPIO-driven LEDs have no 'device' part in the generic

Actually devicename section is expected mostly for LEDs that are somehow
associated with other devices. In most cases it can be skipped, see [0].

[0] 
https://github.com/torvalds/linux/blob/master/Documentation/leds/leds-class.rst#led-device-naming

-- 
Best regards,
Jacek Anaszewski


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-15  6:53                 ` Lee Jones
@ 2025-05-15 20:14                   ` Jacek Anaszewski
  0 siblings, 0 replies; 18+ messages in thread
From: Jacek Anaszewski @ 2025-05-15 20:14 UTC (permalink / raw)
  To: Lee Jones
  Cc: Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	Pavel Machek, linux-leds

On 5/15/25 08:53, Lee Jones wrote:
> On Wed, 14 May 2025, Jacek Anaszewski wrote:
> 
>> On 5/14/25 17:28, Lee Jones wrote:
>>> On Mon, 12 May 2025, Jacek Anaszewski wrote:
>>>
>>>> Hi Geert,
>>>>
>>>> On 5/12/25 09:13, Geert Uytterhoeven wrote:
>>>>> Hi Jacek,
>>>>>
>>>>> Thanks for your answer!
>>>>
>>>> You're welcome.
>>>>
>>>>> On Sat, 10 May 2025 at 14:43, Jacek Anaszewski
>>>>> <jacek.anaszewski@gmail.com> wrote:
>>>>>> On 5/8/25 15:49, Lee Jones wrote:
>>>>>>> On Thu, 08 May 2025, Wolfram Sang wrote:
>>>>>>>> On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
>>>>>>>>> On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
>>>>>>>>> <wsa+renesas@sang-engineering.com> wrote:
>>>>>>>>>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> Changes since v2:
>>>>>>>>>> * using function, color, function-enumerator properties now
>>>>>>>>>>
>>>>>>>>>> Honestly, this is better than using node names? With V2, the LEDs were
>>>>>>>>>> named as in the schematics, now they are called:
>>>>>>>>>>
>>>>>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
>>>>>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
>>>>>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
>>>>>>>>>> ...
>>>>>>>>>>
>>>>>>>>>> Which gets even more confusing if we might later add LEDs not on this
>>>>>>>>>> board, but on the expansion board. 'green:programming-8' sits where?
>>>>>>>>>>
>>>>>>>>>> I really wonder, but if this is the official way now...
>>>>>>>>>
>>>>>>>>> Good point!  So I'm inclined to take v2...
>>>>>>>>>
>>>>>>>>> Let's raise this with the LED people. I don't want to fight Pavel when
>>>>>>>>> v2 hits the CiP tree ;-)
>>>>>>>>
>>>>>>>> So, if there is no other opinion here, can we remove function, color,
>>>>>>>> function-enumerator and just use the node names which match the
>>>>>>>> schematics? Basically apply V2?
>>>>>>>
>>>>>>> I didn't author the semantics nor the rules surrounding them, but I am
>>>>>>> obliged to enforce them.  Therefore "LED people" say, please stick to
>>>>>>> convention as stated in the present documentation:
>>>>>>>
>>>>>>> https://docs.kernel.org/leds/leds-class.html#led-device-naming
>>>>>>>
>>>>>>> Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
>>>>>>> that is more appropriate to your use-case.
>>>>>>>
>>>>>>> Let's also bring Jacek into the conversion, since I know that he did a
>>>>>>> bunch of work around this topic.
>>>>>>
>>>>>> The question is if the LED name from the schematics tells anything to
>>>>>> the user of the equipment?
>>>>>
>>>>> As this is a development board and not a finished product, I would
>>>>> answer yes.
>>>>
>>>> OK.
>>>>
>>>>>> 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.
>>>>>
>>>>> I agree this all makes perfect sense for a final product, where the
>>>>> purpose of each LED is clear, and sometimes indicated by an icon
>>>>> on the case.
>>>>> For a development board, some LEDs may have a fixed purpose.
>>>>> But typically there is also a collection of generic user LEDs, which
>>>>> do not have a fixed purpose, and are identified by a label on the
>>>>> schematics.  Imposing an arbitrary numbering scheme on the latter is
>>>>> confusing for the user (developer).
>>>>
>>>> Using DT child node name for LED class device name is only
>>>> a last resort fallback. However if it is devboard and we want to have
>>>> a reference to the schematics then I'd say it makes sense to take
>>>> LED names from DT nodes. What about the colors? Are the LEDs replaceable
>>>> or soldered?
>>>
>>> Looks like this option does what you want:
>>>
>>> https://github.com/torvalds/linux/blob/master/drivers/leds/led-core.c#L578
>>>
>>> For this to execute you need to provide init_data when calling
>>> *led_classdev_register*(), omit the; label, function, color_present DT
>>> properties and also init_data's default_label attribute.  At which point
>>> the DT node name should be taken as the LED class name.
>>
>> Yep, I know how it works, I wrote that code after all.
> 
> Sorry for the ambiguity.  I was attempting to address Wolfram.

No problem.

>> Here, I wanted to clarify whether it wouldn't make sense to stick to
>> the approach with 'function' and 'color' properties if LEDs are fixed
>> on the board and the colors are known.
>>
>>  From [0] I infer that LEDs are green, so we should use 'color' DT
>> property definitely. And as a 'function' we can assign plain text "pcaN"
>> if you feel it makes sense for that board.
> 
> The 'function' documentation specifically says to use one of the
> predefined LED_FUNCTION_* entries.  If we are officially accepting
> caveats to that, we should probably update it.

We could add a note saying that if required function name is unlikely
to be reused in other devices, then it is acceptable to use custom
string.

-- 
Best regards,
Jacek Anaszewski


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-10 12:43       ` Jacek Anaszewski
  2025-05-12  7:13         ` Geert Uytterhoeven
@ 2025-05-16  7:35         ` Alexander Dahl
  2025-05-18 14:36           ` Jacek Anaszewski
  1 sibling, 1 reply; 18+ messages in thread
From: Alexander Dahl @ 2025-05-16  7:35 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Lee Jones, Wolfram Sang, Geert Uytterhoeven, linux-renesas-soc,
	Magnus Damm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, Pavel Machek, linux-leds

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,
> 
> On 5/8/25 15:49, Lee Jones wrote:
> > On Thu, 08 May 2025, Wolfram Sang wrote:
> > 
> > > On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
> > > > Hi Wolfram,
> > > > 
> > > > CC leds
> > > > 
> > > > On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
> > > > <wsa+renesas@sang-engineering.com> wrote:
> > > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > > > ---
> > > > > 
> > > > > Changes since v2:
> > > > > * using function, color, function-enumerator properties now
> > > > > 
> > > > > Honestly, this is better than using node names? With V2, the LEDs were
> > > > > named as in the schematics, now they are called:
> > > > > 
> > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
> > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
> > > > > lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
> > > > > ...
> > > > > 
> > > > > Which gets even more confusing if we might later add LEDs not on this
> > > > > board, but on the expansion board. 'green:programming-8' sits where?
> > > > > 
> > > > > I really wonder, but if this is the official way now...
> > > > 
> > > > Good point!  So I'm inclined to take v2...
> > > > 
> > > > Let's raise this with the LED people. I don't want to fight Pavel when
> > > > v2 hits the CiP tree ;-)
> > > 
> > > So, if there is no other opinion here, can we remove function, color,
> > > function-enumerator and just use the node names which match the
> > > schematics? Basically apply V2?
> > 
> > I didn't author the semantics nor the rules surrounding them, but I am
> > obliged to enforce them.  Therefore "LED people" say, please stick to
> > convention as stated in the present documentation:
> > 
> > https://docs.kernel.org/leds/leds-class.html#led-device-naming
> > 
> > Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
> > that is more appropriate to your use-case.
> > 
> > Let's also bring Jacek into the conversion, since I know that he did a
> > bunch of work around this topic.
> 
> 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";
+			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?

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?

Or would that be too confusing?

Greets
Alex

> Regarding expansion boards - we never have control over what
> LED names DT overlays will define, thus LED core adds numeric suffix to
> the LED class device name in case of the name clash.
> 
> -- 
> Best regards,
> Jacek Anaszewski
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-16  7:35         ` Alexander Dahl
@ 2025-05-18 14:36           ` Jacek Anaszewski
  2025-05-19  7:37             ` Alexander Dahl
  0 siblings, 1 reply; 18+ messages in thread
From: Jacek Anaszewski @ 2025-05-18 14:36 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Geert Uytterhoeven, linux-renesas-soc,
	Magnus Damm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, Pavel Machek, linux-leds

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,
>>
>> On 5/8/25 15:49, Lee Jones wrote:
>>> On Thu, 08 May 2025, Wolfram Sang wrote:
>>>
>>>> On Thu, Apr 17, 2025 at 01:39:14PM +0200, Geert Uytterhoeven wrote:
>>>>> Hi Wolfram,
>>>>>
>>>>> CC leds
>>>>>
>>>>> On Thu, 17 Apr 2025 at 11:33, Wolfram Sang
>>>>> <wsa+renesas@sang-engineering.com> wrote:
>>>>>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>>>> ---
>>>>>>
>>>>>> Changes since v2:
>>>>>> * using function, color, function-enumerator properties now
>>>>>>
>>>>>> Honestly, this is better than using node names? With V2, the LEDs were
>>>>>> named as in the schematics, now they are called:
>>>>>>
>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-0 -> ../../devices/platform/leds/leds/green:programming-0
>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-1 -> ../../devices/platform/leds/leds/green:programming-1
>>>>>> lrwxrwxrwx    1 root     root             0 May 12 12:10 green:programming-2 -> ../../devices/platform/leds/leds/green:programming-2
>>>>>> ...
>>>>>>
>>>>>> Which gets even more confusing if we might later add LEDs not on this
>>>>>> board, but on the expansion board. 'green:programming-8' sits where?
>>>>>>
>>>>>> I really wonder, but if this is the official way now...
>>>>>
>>>>> Good point!  So I'm inclined to take v2...
>>>>>
>>>>> Let's raise this with the LED people. I don't want to fight Pavel when
>>>>> v2 hits the CiP tree ;-)
>>>>
>>>> So, if there is no other opinion here, can we remove function, color,
>>>> function-enumerator and just use the node names which match the
>>>> schematics? Basically apply V2?
>>>
>>> I didn't author the semantics nor the rules surrounding them, but I am
>>> obliged to enforce them.  Therefore "LED people" say, please stick to
>>> convention as stated in the present documentation:
>>>
>>> https://docs.kernel.org/leds/leds-class.html#led-device-naming
>>>
>>> Please note that a "debug" (LED_FUNCTION_DEBUG) option already exists if
>>> that is more appropriate to your use-case.
>>>
>>> Let's also bring Jacek into the conversion, since I know that he did a
>>> bunch of work around this topic.
>>
>> 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.

> 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

-- 
Best regards,
Jacek Anaszewski


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-18 14:36           ` Jacek Anaszewski
@ 2025-05-19  7:37             ` Alexander Dahl
  2025-05-19 21:14               ` Jacek Anaszewski
  0 siblings, 1 reply; 18+ messages in thread
From: Alexander Dahl @ 2025-05-19  7:37 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Lee Jones, Wolfram Sang, Geert Uytterhoeven, linux-renesas-soc,
	Magnus Damm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, Pavel Machek, linux-leds

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs
  2025-05-19  7:37             ` Alexander Dahl
@ 2025-05-19 21:14               ` Jacek Anaszewski
  0 siblings, 0 replies; 18+ messages in thread
From: Jacek Anaszewski @ 2025-05-19 21:14 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Geert Uytterhoeven, linux-renesas-soc,
	Magnus Damm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, Pavel Machek, linux-leds

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@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.

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-05-19 21:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-17  9:28 [PATCH v3] ARM: dts: renesas: r9a06g032-rzn1d400-db: describe Debug LEDs Wolfram Sang
2025-04-17 11:39 ` Geert Uytterhoeven
2025-05-08  7:56   ` Wolfram Sang
2025-05-08 13:49     ` Lee Jones
2025-05-10 12:43       ` Jacek Anaszewski
2025-05-12  7:13         ` Geert Uytterhoeven
2025-05-12 17:54           ` Jacek Anaszewski
2025-05-14 15:28             ` Lee Jones
2025-05-14 18:57               ` Jacek Anaszewski
2025-05-15  6:53                 ` Lee Jones
2025-05-15 20:14                   ` Jacek Anaszewski
2025-05-15  8:27                 ` Wolfram Sang
2025-05-15  9:15                   ` Lee Jones
2025-05-15 19:53                   ` Jacek Anaszewski
2025-05-16  7:35         ` Alexander Dahl
2025-05-18 14:36           ` Jacek Anaszewski
2025-05-19  7:37             ` Alexander Dahl
2025-05-19 21:14               ` Jacek Anaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).