linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements
@ 2020-04-27  7:31 Chen-Yu Tsai
  2020-04-27  7:31 ` [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers Chen-Yu Tsai
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27  7:31 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Jacek Anaszewski, Pavel Machek, Dan Murphy
  Cc: Chen-Yu Tsai, linux-rockchip, linux-arm-kernel, devicetree,
	linux-leds, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

Hi,

Here is the remaining patch for roc-rk3399-pc, along with a few other
new patches.

  - Based on discussions from v1, patch one, newly added, drops the list
    of valid values for linux,default-triggers.

  - Patch two is the same as in v1

  - Patch three, new, sets dr_mode to "host" to the dwc2 OTG controller,
    matching what the board uses it for, a host port. This gets rid of
    a warning from the kernel.

Please have a look.


Regards
ChenYu


Chen-Yu Tsai (3):
  dt-bindings: leds: common: Drop enumeration for linux,default-triggers
  arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED
    triggers
  arm64: dts: rockchip: rk3328-roc-cc: Set dr_mode to "host" for OTG

 .../devicetree/bindings/leds/common.yaml      | 21 +------------------
 .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts |  1 +
 .../dts/rockchip/rk3399-roc-pc-mezzanine.dts  |  8 +++++++
 .../boot/dts/rockchip/rk3399-roc-pc.dtsi      |  4 ++--
 4 files changed, 12 insertions(+), 22 deletions(-)

-- 
2.26.0


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

* [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers
  2020-04-27  7:31 [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements Chen-Yu Tsai
@ 2020-04-27  7:31 ` Chen-Yu Tsai
  2020-04-27  8:33   ` Johan Jonker
  2020-05-11 23:12   ` Rob Herring
  2020-04-27  7:31 ` [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers Chen-Yu Tsai
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27  7:31 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Jacek Anaszewski, Pavel Machek, Dan Murphy
  Cc: Chen-Yu Tsai, linux-rockchip, linux-arm-kernel, devicetree,
	linux-leds, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The bindings currently list a very small subset of valid triggers for
LEDs. Since many drivers or subsystems in Linux register custom
triggers, the list would become very hard to maintain.

Instead, just drop the list and allow free form strings.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 .../devicetree/bindings/leds/common.yaml      | 21 +------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index 4c270fde4567..3b3cdab3fc15 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -79,26 +79,7 @@ properties:
     description:
       This parameter, if present, is a string defining the trigger assigned to
       the LED.
-    allOf:
-      - $ref: /schemas/types.yaml#definitions/string
-    enum:
-        # LED will act as a back-light, controlled by the framebuffer system
-      - backlight
-        # LED will turn on (but for leds-gpio see "default-state" property in
-        # Documentation/devicetree/bindings/leds/leds-gpio.yaml)
-      - default-on
-        # LED "double" flashes at a load average based rate
-      - heartbeat
-        # LED indicates disk activity
-      - disk-activity
-        # LED indicates IDE disk activity (deprecated), in new implementations
-        # use "disk-activity"
-      - ide-disk
-        # LED flashes at a fixed, configurable rate
-      - timer
-        # LED alters the brightness for the specified duration with one software
-        # timer (requires "led-pattern" property)
-      - pattern
+    $ref: /schemas/types.yaml#definitions/string
 
   led-pattern:
     description: |
-- 
2.26.0


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

* [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27  7:31 [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements Chen-Yu Tsai
  2020-04-27  7:31 ` [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers Chen-Yu Tsai
@ 2020-04-27  7:31 ` Chen-Yu Tsai
  2020-04-27  8:57   ` Johan Jonker
  2020-04-27  7:31 ` [PATCH v2 3/3] arm64: dts: rockchip: rk3328-roc-cc: Set dr_mode to "host" for OTG Chen-Yu Tsai
  2020-05-05  6:24 ` [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements Chen-Yu Tsai
  3 siblings, 1 reply; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27  7:31 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Jacek Anaszewski, Pavel Machek, Dan Murphy
  Cc: Chen-Yu Tsai, linux-rockchip, linux-arm-kernel, devicetree,
	linux-leds, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

With SDIO now enabled, the numbering of the existing MMC host controllers
gets incremented by 1, as the SDIO host is the first one.

Increment the numbering of the MMC LED triggers to match.

Fixes: cf3c5397835f ("arm64: dts: rockchip: Enable sdio0 and uart0 on rk3399-roc-pc-mezzanine")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts | 8 ++++++++
 arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi          | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
index 2acb3d500fb9..f0686fc276be 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
@@ -38,6 +38,10 @@ vcc3v3_pcie: vcc3v3-pcie {
 	};
 };
 
+&diy_led {
+	linux,default-trigger = "mmc2";
+};
+
 &pcie_phy {
 	status = "okay";
 };
@@ -91,3 +95,7 @@ &uart0 {
 	pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
 	status = "okay";
 };
+
+&yellow_led {
+	linux,default-trigger = "mmc1";
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
index 9f225e9c3d54..bc060ac7972d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
@@ -70,14 +70,14 @@ work-led {
 			linux,default-trigger = "heartbeat";
 		};
 
-		diy-led {
+		diy_led: diy-led {
 			label = "red:diy";
 			gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 			linux,default-trigger = "mmc1";
 		};
 
-		yellow-led {
+		yellow_led: yellow-led {
 			label = "yellow:yellow-led";
 			gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
-- 
2.26.0


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

* [PATCH v2 3/3] arm64: dts: rockchip: rk3328-roc-cc: Set dr_mode to "host" for OTG
  2020-04-27  7:31 [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements Chen-Yu Tsai
  2020-04-27  7:31 ` [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers Chen-Yu Tsai
  2020-04-27  7:31 ` [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers Chen-Yu Tsai
@ 2020-04-27  7:31 ` Chen-Yu Tsai
  2020-05-05  6:24 ` [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements Chen-Yu Tsai
  3 siblings, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27  7:31 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Jacek Anaszewski, Pavel Machek, Dan Murphy
  Cc: Chen-Yu Tsai, linux-rockchip, linux-arm-kernel, devicetree,
	linux-leds, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The board has a standard USB A female port connected to the USB OTG
controller's data pins. Set dr_mode in the OTG controller node to
indicate this usage, instead of having the implementation guess.

Fixes: 2171f4fdac06 ("arm64: dts: rockchip: add roc-rk3328-cc board")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts b/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
index 8d553c92182a..56e79ce9101b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
@@ -334,6 +334,7 @@ &uart2 {
 };
 
 &usb20_otg {
+	dr_mode = "host";
 	status = "okay";
 };
 
-- 
2.26.0


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

* Re: [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers
  2020-04-27  7:31 ` [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers Chen-Yu Tsai
@ 2020-04-27  8:33   ` Johan Jonker
  2020-04-27  9:21     ` Chen-Yu Tsai
  2020-05-11 23:12   ` Rob Herring
  1 sibling, 1 reply; 16+ messages in thread
From: Johan Jonker @ 2020-04-27  8:33 UTC (permalink / raw)
  To: wens
  Cc: devicetree, dmurphy, heiko, jacek.anaszewski, linux-arm-kernel,
	linux-kernel, linux-leds, linux-rockchip, pavel, robh+dt, wens

Hi Chen-Yu,

> From: Chen-Yu Tsai <wens@csie.org>
> 
> The bindings currently list a very small subset of valid triggers for
> LEDs. Since many drivers or subsystems in Linux register custom
> triggers, the list would become very hard to maintain.
> 
> Instead, just drop the list and allow free form strings.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  .../devicetree/bindings/leds/common.yaml      | 21 +------------------
>  1 file changed, 1 insertion(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> index 4c270fde4567..3b3cdab3fc15 100644
> --- a/Documentation/devicetree/bindings/leds/common.yaml
> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> @@ -79,26 +79,7 @@ properties:
>      description:
>        This parameter, if present, is a string defining the trigger assigned to
>        the LED.
> -    allOf:
> -      - $ref: /schemas/types.yaml#definitions/string
> -    enum:
> -        # LED will act as a back-light, controlled by the framebuffer system
> -      - backlight
> -        # LED will turn on (but for leds-gpio see "default-state" property in
> -        # Documentation/devicetree/bindings/leds/leds-gpio.yaml)
> -      - default-on
> -        # LED "double" flashes at a load average based rate
> -      - heartbeat
> -        # LED indicates disk activity
> -      - disk-activity
> -        # LED indicates IDE disk activity (deprecated), in new implementations
> -        # use "disk-activity"
> -      - ide-disk
> -        # LED flashes at a fixed, configurable rate
> -      - timer
> -        # LED alters the brightness for the specified duration with one software
> -        # timer (requires "led-pattern" property)
> -      - pattern
> +    $ref: /schemas/types.yaml#definitions/string

This makes it free form, but deletes the documentation of options that
are standard available for people without custom driver.
Where should that info go?

>  
>    led-pattern:
>      description: |
> -- 
> 2.26.0


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

* Re: [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27  7:31 ` [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers Chen-Yu Tsai
@ 2020-04-27  8:57   ` Johan Jonker
  2020-04-27  9:17     ` Chen-Yu Tsai
  0 siblings, 1 reply; 16+ messages in thread
From: Johan Jonker @ 2020-04-27  8:57 UTC (permalink / raw)
  To: wens
  Cc: devicetree, dmurphy, heiko, jacek.anaszewski, linux-arm-kernel,
	linux-kernel, linux-leds, linux-rockchip, pavel, robh+dt, wens

Hi Chen-Yu,

> From: Chen-Yu Tsai <wens@csie.org>
> 
> With SDIO now enabled, the numbering of the existing MMC host controllers
> gets incremented by 1, as the SDIO host is the first one.
> 
> Increment the numbering of the MMC LED triggers to match.
> 
> Fixes: cf3c5397835f ("arm64: dts: rockchip: Enable sdio0 and uart0 on rk3399-roc-pc-mezzanine")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts | 8 ++++++++
>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi          | 4 ++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> index 2acb3d500fb9..f0686fc276be 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> @@ -38,6 +38,10 @@ vcc3v3_pcie: vcc3v3-pcie {
>  	};
>  };
>  
> +&diy_led {
> +	linux,default-trigger = "mmc2";
> +};
> +
>  &pcie_phy {
>  	status = "okay";
>  };
> @@ -91,3 +95,7 @@ &uart0 {
>  	pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
>  	status = "okay";
>  };
> +
> +&yellow_led {
> +	linux,default-trigger = "mmc1";
> +};
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> index 9f225e9c3d54..bc060ac7972d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> @@ -70,14 +70,14 @@ work-led {
>  			linux,default-trigger = "heartbeat";
>  		};
>  

> -		diy-led {
> +		diy_led: diy-led {

This changes an existing nodename into something that is still not the
preferred way. In the current Rockchip dts there are nodenames like
'work', 'yellow' that causing warnings with the command:

make -k ARCH=arm dtbs_check

Could you give a generic guide line/example, so all these changes are
treated the same way? As if the naming follows the preferred 'led-0' line.

>  			label = "red:diy";
>  			gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  			linux,default-trigger = "mmc1";
>  		};
>  
> -		yellow-led {
> +		yellow_led: yellow-led {
>  			label = "yellow:yellow-led";
>  			gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
> -- 
> 2.26.0


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

* Re: [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27  8:57   ` Johan Jonker
@ 2020-04-27  9:17     ` Chen-Yu Tsai
  2020-04-27 10:08       ` Johan Jonker
  0 siblings, 1 reply; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27  9:17 UTC (permalink / raw)
  To: Johan Jonker
  Cc: Chen-Yu Tsai, devicetree, dmurphy, Heiko Stübner,
	jacek.anaszewski, linux-arm-kernel, linux-kernel, linux-leds,
	open list:ARM/Rockchip SoC...,
	Pavel Machek, Rob Herring

On Mon, Apr 27, 2020 at 4:57 PM Johan Jonker <jbx6244@gmail.com> wrote:
>
> Hi Chen-Yu,
>
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > With SDIO now enabled, the numbering of the existing MMC host controllers
> > gets incremented by 1, as the SDIO host is the first one.
> >
> > Increment the numbering of the MMC LED triggers to match.
> >
> > Fixes: cf3c5397835f ("arm64: dts: rockchip: Enable sdio0 and uart0 on rk3399-roc-pc-mezzanine")
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts | 8 ++++++++
> >  arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi          | 4 ++--
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> > index 2acb3d500fb9..f0686fc276be 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> > @@ -38,6 +38,10 @@ vcc3v3_pcie: vcc3v3-pcie {
> >       };
> >  };
> >
> > +&diy_led {
> > +     linux,default-trigger = "mmc2";
> > +};
> > +
> >  &pcie_phy {
> >       status = "okay";
> >  };
> > @@ -91,3 +95,7 @@ &uart0 {
> >       pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
> >       status = "okay";
> >  };
> > +
> > +&yellow_led {
> > +     linux,default-trigger = "mmc1";
> > +};
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > index 9f225e9c3d54..bc060ac7972d 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > @@ -70,14 +70,14 @@ work-led {
> >                       linux,default-trigger = "heartbeat";
> >               };
> >
>
> > -             diy-led {
> > +             diy_led: diy-led {
>
> This changes an existing nodename into something that is still not the
> preferred way. In the current Rockchip dts there are nodenames like
> 'work', 'yellow' that causing warnings with the command:

This doesn't change the node name at all. It only adds a label.
If it doesn't pass the check now, it didn't pass the check before.

I just realized that the footnote I added before is gone because I
regenerated the patches. The original footnote was something along
the lines of:

I opted to not change the node names nor the labels as the discussion
had not concluded. The other reason being that people may have scripts
or device tree overlays depending on the existing node names.

Previously I asked the following but got no response:

    Is changing this after it has been in some kernel releases OK? Wouldn't
    it be considered a break of sysfs ABI?

    Also, is there some guideline on how to name the labels? For sunxi we've
    been doing "${vendor}:${color}:${function}" since forever.

    As far as I can tell, the hardware vendor [1] has no specific uses for
    these two (red and yellow) LEDs designed in. And their GPIO lines are
    simply labeled "DIY" (for the red one) and "YELLOW". So I'm not sure
    if putting "our" interpretations and the default-trigger into the
    label is wise.

    For reference, the green one has its GPIO line labeled "WORK", and their
    intention from [1] is to have it as some sort of power / activity indicator.
    Hence it is named / labeled "work".

    As for the node names, I think we can keep it as is for now. It's not
    the preferred form, but there's really no need to change it either.
    And some overlay or script might actually expect that name.

> make -k ARCH=arm dtbs_check
>
> Could you give a generic guide line/example, so all these changes are
> treated the same way? As if the naming follows the preferred 'led-0' line.

I'm not sure what you are asking for.

ChenYu

> >                       label = "red:diy";
> >                       gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
> >                       default-state = "off";
> >                       linux,default-trigger = "mmc1";
> >               };
> >
> > -             yellow-led {
> > +             yellow_led: yellow-led {
> >                       label = "yellow:yellow-led";
> >                       gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
> >                       default-state = "off";
> > --
> > 2.26.0
>

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

* Re: [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers
  2020-04-27  8:33   ` Johan Jonker
@ 2020-04-27  9:21     ` Chen-Yu Tsai
  0 siblings, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27  9:21 UTC (permalink / raw)
  To: Johan Jonker
  Cc: Chen-Yu Tsai, devicetree, dmurphy, Heiko Stübner,
	jacek.anaszewski, linux-arm-kernel, linux-kernel, linux-leds,
	open list:ARM/Rockchip SoC...,
	Pavel Machek, Rob Herring

On Mon, Apr 27, 2020 at 4:33 PM Johan Jonker <jbx6244@gmail.com> wrote:
>
> Hi Chen-Yu,
>
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > The bindings currently list a very small subset of valid triggers for
> > LEDs. Since many drivers or subsystems in Linux register custom
> > triggers, the list would become very hard to maintain.
> >
> > Instead, just drop the list and allow free form strings.
> >
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >  .../devicetree/bindings/leds/common.yaml      | 21 +------------------
> >  1 file changed, 1 insertion(+), 20 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> > index 4c270fde4567..3b3cdab3fc15 100644
> > --- a/Documentation/devicetree/bindings/leds/common.yaml
> > +++ b/Documentation/devicetree/bindings/leds/common.yaml
> > @@ -79,26 +79,7 @@ properties:
> >      description:
> >        This parameter, if present, is a string defining the trigger assigned to
> >        the LED.
> > -    allOf:
> > -      - $ref: /schemas/types.yaml#definitions/string
> > -    enum:
> > -        # LED will act as a back-light, controlled by the framebuffer system
> > -      - backlight
> > -        # LED will turn on (but for leds-gpio see "default-state" property in
> > -        # Documentation/devicetree/bindings/leds/leds-gpio.yaml)
> > -      - default-on
> > -        # LED "double" flashes at a load average based rate
> > -      - heartbeat
> > -        # LED indicates disk activity
> > -      - disk-activity
> > -        # LED indicates IDE disk activity (deprecated), in new implementations
> > -        # use "disk-activity"
> > -      - ide-disk
> > -        # LED flashes at a fixed, configurable rate
> > -      - timer
> > -        # LED alters the brightness for the specified duration with one software
> > -        # timer (requires "led-pattern" property)
> > -      - pattern
> > +    $ref: /schemas/types.yaml#definitions/string
>
> This makes it free form, but deletes the documentation of options that
> are standard available for people without custom driver.
> Where should that info go?

As far as I know, there is no canonical list of standard triggers.
In addition, what triggers are available also depend on the kernel
configuration, so there really is no "standard".

Since this is also configurable via sysfs, maybe it should be part
of the sysfs ABI document? Either way I believe this will be up to
the LED subsystem maintainers.

ChenYu

> >
> >    led-pattern:
> >      description: |
> > --
> > 2.26.0
>

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

* Re: [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27  9:17     ` Chen-Yu Tsai
@ 2020-04-27 10:08       ` Johan Jonker
  2020-04-27 10:33         ` Chen-Yu Tsai
  0 siblings, 1 reply; 16+ messages in thread
From: Johan Jonker @ 2020-04-27 10:08 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: devicetree, dmurphy, Heiko Stübner, jacek.anaszewski,
	linux-arm-kernel, linux-kernel, linux-leds,
	open list:ARM/Rockchip SoC...,
	Pavel Machek, Rob Herring

On 4/27/20 11:17 AM, Chen-Yu Tsai wrote:
> On Mon, Apr 27, 2020 at 4:57 PM Johan Jonker <jbx6244@gmail.com> wrote:
>>
>> Hi Chen-Yu,
>>
>>> From: Chen-Yu Tsai <wens@csie.org>
>>>
>>> With SDIO now enabled, the numbering of the existing MMC host controllers
>>> gets incremented by 1, as the SDIO host is the first one.
>>>
>>> Increment the numbering of the MMC LED triggers to match.
>>>
>>> Fixes: cf3c5397835f ("arm64: dts: rockchip: Enable sdio0 and uart0 on rk3399-roc-pc-mezzanine")
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts | 8 ++++++++
>>>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi          | 4 ++--
>>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>> index 2acb3d500fb9..f0686fc276be 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>> @@ -38,6 +38,10 @@ vcc3v3_pcie: vcc3v3-pcie {
>>>       };
>>>  };
>>>
>>> +&diy_led {

>>> +     linux,default-trigger = "mmc2";

If you decide to use a free form trigger that is not a 'standard' one,
then it becomes a user property.
User defined properties should not go in a generic dts.
It's up to the user what function he/she gives to that led!

>>> +};
>>> +
>>>  &pcie_phy {
>>>       status = "okay";
>>>  };
>>> @@ -91,3 +95,7 @@ &uart0 {
>>>       pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
>>>       status = "okay";
>>>  };
>>> +
>>> +&yellow_led {
>>> +     linux,default-trigger = "mmc1";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> index 9f225e9c3d54..bc060ac7972d 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> @@ -70,14 +70,14 @@ work-led {
>>>                       linux,default-trigger = "heartbeat";
>>>               };
>>>
>>
>>> -             diy-led {
>>> +             diy_led: diy-led {
>>
>> This changes an existing nodename into something that is still not the

Correction:
It takes an existing nodename and adds a label.

>> preferred way. In the current Rockchip dts there are nodenames like
>> 'work', 'yellow' that causing warnings with the command:
> 
> This doesn't change the node name at all. It only adds a label.
> If it doesn't pass the check now, it didn't pass the check before.
> 
> I just realized that the footnote I added before is gone because I
> regenerated the patches. The original footnote was something along
> the lines of:
> 
> I opted to not change the node names nor the labels as the discussion
> had not concluded. The other reason being that people may have scripts
> or device tree overlays depending on the existing node names.
> 
> Previously I asked the following but got no response:
> 
>     Is changing this after it has been in some kernel releases OK? Wouldn't
>     it be considered a break of sysfs ABI?
> 
>     Also, is there some guideline on how to name the labels? For sunxi we've
>     been doing "${vendor}:${color}:${function}" since forever.
> 
>     As far as I can tell, the hardware vendor [1] has no specific uses for
>     these two (red and yellow) LEDs designed in. And their GPIO lines are
>     simply labeled "DIY" (for the red one) and "YELLOW". So I'm not sure
>     if putting "our" interpretations and the default-trigger into the
>     label is wise.
> 
>     For reference, the green one has its GPIO line labeled "WORK", and their
>     intention from [1] is to have it as some sort of power / activity indicator.
>     Hence it is named / labeled "work".
> 
>     As for the node names, I think we can keep it as is for now. It's not
>     the preferred form, but there's really no need to change it either.
>     And some overlay or script might actually expect that name.
> 
>> make -k ARCH=arm dtbs_check
>>
>> Could you give a generic guide line/example, so all these changes are
>> treated the same way? As if the naming follows the preferred 'led-0' line.
> 
> I'm not sure what you are asking for.

Your nodename just happend to contain 'led' to pass the regex.
There are many other names in use.
'If' the DT maintainer (=Heiko) decides the get rid of all the warnings
for led nodes then that change would require all nodename to be handled
to same (=preferred way):

Change this:

diy_led: diy-led
yellow_led: yellow-led

Into something like:

led_0: led-0
led_1: led-1

> 
> ChenYu
> 
>>>                       label = "red:diy";
>>>                       gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
>>>                       default-state = "off";
>>>                       linux,default-trigger = "mmc1";
>>>               };
>>>
>>> -             yellow-led {
>>> +             yellow_led: yellow-led {
>>>                       label = "yellow:yellow-led";
>>>                       gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
>>>                       default-state = "off";
>>> --
>>> 2.26.0
>>


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

* Re: [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27 10:08       ` Johan Jonker
@ 2020-04-27 10:33         ` Chen-Yu Tsai
  2020-04-27 10:55           ` Johan Jonker
  0 siblings, 1 reply; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27 10:33 UTC (permalink / raw)
  To: Johan Jonker
  Cc: Chen-Yu Tsai, devicetree, dmurphy, Heiko Stübner,
	jacek.anaszewski, linux-arm-kernel, linux-kernel, linux-leds,
	open list:ARM/Rockchip SoC...,
	Pavel Machek, Rob Herring

On Mon, Apr 27, 2020 at 6:09 PM Johan Jonker <jbx6244@gmail.com> wrote:
>
> On 4/27/20 11:17 AM, Chen-Yu Tsai wrote:
> > On Mon, Apr 27, 2020 at 4:57 PM Johan Jonker <jbx6244@gmail.com> wrote:
> >>
> >> Hi Chen-Yu,
> >>
> >>> From: Chen-Yu Tsai <wens@csie.org>
> >>>
> >>> With SDIO now enabled, the numbering of the existing MMC host controllers
> >>> gets incremented by 1, as the SDIO host is the first one.
> >>>
> >>> Increment the numbering of the MMC LED triggers to match.
> >>>
> >>> Fixes: cf3c5397835f ("arm64: dts: rockchip: Enable sdio0 and uart0 on rk3399-roc-pc-mezzanine")
> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >>> ---
> >>>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts | 8 ++++++++
> >>>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi          | 4 ++--
> >>>  2 files changed, 10 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> >>> index 2acb3d500fb9..f0686fc276be 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
> >>> @@ -38,6 +38,10 @@ vcc3v3_pcie: vcc3v3-pcie {
> >>>       };
> >>>  };
> >>>
> >>> +&diy_led {
>
> >>> +     linux,default-trigger = "mmc2";
>
> If you decide to use a free form trigger that is not a 'standard' one,
> then it becomes a user property.
> User defined properties should not go in a generic dts.
> It's up to the user what function he/she gives to that led!

The original (in the base .dtsi file for this series of boards) trigger
is already a non-standard, i.e. not listed in the bindings, one.

Now I would very much like to get rid of user specific stuff, and I
also mentioned that in the previous round of discussions. No one said
anything.

> >>> +};
> >>> +
> >>>  &pcie_phy {
> >>>       status = "okay";
> >>>  };
> >>> @@ -91,3 +95,7 @@ &uart0 {
> >>>       pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
> >>>       status = "okay";
> >>>  };
> >>> +
> >>> +&yellow_led {
> >>> +     linux,default-trigger = "mmc1";
> >>> +};
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> >>> index 9f225e9c3d54..bc060ac7972d 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> >>> @@ -70,14 +70,14 @@ work-led {
> >>>                       linux,default-trigger = "heartbeat";
> >>>               };
> >>>
> >>
> >>> -             diy-led {
> >>> +             diy_led: diy-led {
> >>
> >> This changes an existing nodename into something that is still not the
>
> Correction:
> It takes an existing nodename and adds a label.

OK.

> >> preferred way. In the current Rockchip dts there are nodenames like
> >> 'work', 'yellow' that causing warnings with the command:
> >
> > This doesn't change the node name at all. It only adds a label.
> > If it doesn't pass the check now, it didn't pass the check before.
> >
> > I just realized that the footnote I added before is gone because I
> > regenerated the patches. The original footnote was something along
> > the lines of:
> >
> > I opted to not change the node names nor the labels as the discussion
> > had not concluded. The other reason being that people may have scripts
> > or device tree overlays depending on the existing node names.
> >
> > Previously I asked the following but got no response:
> >
> >     Is changing this after it has been in some kernel releases OK? Wouldn't
> >     it be considered a break of sysfs ABI?
> >
> >     Also, is there some guideline on how to name the labels? For sunxi we've
> >     been doing "${vendor}:${color}:${function}" since forever.
> >
> >     As far as I can tell, the hardware vendor [1] has no specific uses for
> >     these two (red and yellow) LEDs designed in. And their GPIO lines are
> >     simply labeled "DIY" (for the red one) and "YELLOW". So I'm not sure
> >     if putting "our" interpretations and the default-trigger into the
> >     label is wise.
> >
> >     For reference, the green one has its GPIO line labeled "WORK", and their
> >     intention from [1] is to have it as some sort of power / activity indicator.
> >     Hence it is named / labeled "work".
> >
> >     As for the node names, I think we can keep it as is for now. It's not
> >     the preferred form, but there's really no need to change it either.
> >     And some overlay or script might actually expect that name.
> >
> >> make -k ARCH=arm dtbs_check
> >>
> >> Could you give a generic guide line/example, so all these changes are
> >> treated the same way? As if the naming follows the preferred 'led-0' line.
> >
> > I'm not sure what you are asking for.
>
> Your nodename just happend to contain 'led' to pass the regex.
> There are many other names in use.

Right. So if it passes, what's the problem?

> 'If' the DT maintainer (=Heiko) decides the get rid of all the warnings
> for led nodes then that change would require all nodename to be handled
> to same (=preferred way):
>
> Change this:
>
> diy_led: diy-led
> yellow_led: yellow-led
>
> Into something like:
>
> led_0: led-0
> led_1: led-1

As I already said, if the maintainers want to clean this up, I am happy to
provide patches towards this. That is not the case right now. Furthermore,
that cleanup is not directly related to what I'm trying to fix in this
patch, which is, from the original submitter's point of view, incorrect
triggers are used when the mezzanine board is added.

Also, DT labels "led_0" and "led_1" are useless. They have no relation to
what is used in the schematics, which are "work", "diy", and "yellow". The
board itself doesn't have anything silk-screened on, so on that end the only
thing to go with is the color.

So for fixing up the LED node names, we'd probably want the following:

    diy_led: led-0
    yellow_led: led-1
    work_led: led-2

Is that what you're asking for?


ChenYu

> >
> > ChenYu
> >
> >>>                       label = "red:diy";
> >>>                       gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
> >>>                       default-state = "off";
> >>>                       linux,default-trigger = "mmc1";
> >>>               };
> >>>
> >>> -             yellow-led {
> >>> +             yellow_led: yellow-led {
> >>>                       label = "yellow:yellow-led";
> >>>                       gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
> >>>                       default-state = "off";
> >>> --
> >>> 2.26.0
> >>
>

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

* Re: [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27 10:33         ` Chen-Yu Tsai
@ 2020-04-27 10:55           ` Johan Jonker
  2020-04-27 14:12             ` Johan Jonker
  0 siblings, 1 reply; 16+ messages in thread
From: Johan Jonker @ 2020-04-27 10:55 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: devicetree, dmurphy, Heiko Stübner, jacek.anaszewski,
	linux-arm-kernel, linux-kernel, linux-leds,
	open list:ARM/Rockchip SoC...,
	Pavel Machek, Rob Herring

On 4/27/20 12:33 PM, Chen-Yu Tsai wrote:
> On Mon, Apr 27, 2020 at 6:09 PM Johan Jonker <jbx6244@gmail.com> wrote:
>>
>> On 4/27/20 11:17 AM, Chen-Yu Tsai wrote:
>>> On Mon, Apr 27, 2020 at 4:57 PM Johan Jonker <jbx6244@gmail.com> wrote:
>>>>
>>>> Hi Chen-Yu,
>>>>
>>>>> From: Chen-Yu Tsai <wens@csie.org>
>>>>>
>>>>> With SDIO now enabled, the numbering of the existing MMC host controllers
>>>>> gets incremented by 1, as the SDIO host is the first one.
>>>>>
>>>>> Increment the numbering of the MMC LED triggers to match.
>>>>>
>>>>> Fixes: cf3c5397835f ("arm64: dts: rockchip: Enable sdio0 and uart0 on rk3399-roc-pc-mezzanine")
>>>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>>>> ---
>>>>>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts | 8 ++++++++
>>>>>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi          | 4 ++--
>>>>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>>>> index 2acb3d500fb9..f0686fc276be 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>>>> @@ -38,6 +38,10 @@ vcc3v3_pcie: vcc3v3-pcie {
>>>>>       };
>>>>>  };
>>>>>
>>>>> +&diy_led {
>>
>>>>> +     linux,default-trigger = "mmc2";
>>
>> If you decide to use a free form trigger that is not a 'standard' one,
>> then it becomes a user property.
>> User defined properties should not go in a generic dts.
>> It's up to the user what function he/she gives to that led!
> 
> The original (in the base .dtsi file for this series of boards) trigger
> is already a non-standard, i.e. not listed in the bindings, one.
> 
> Now I would very much like to get rid of user specific stuff, and I
> also mentioned that in the previous round of discussions. No one said
> anything.
> 
>>>>> +};
>>>>> +
>>>>>  &pcie_phy {
>>>>>       status = "okay";
>>>>>  };
>>>>> @@ -91,3 +95,7 @@ &uart0 {
>>>>>       pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
>>>>>       status = "okay";
>>>>>  };
>>>>> +
>>>>> +&yellow_led {
>>>>> +     linux,default-trigger = "mmc1";
>>>>> +};
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>>>> index 9f225e9c3d54..bc060ac7972d 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>>>> @@ -70,14 +70,14 @@ work-led {
>>>>>                       linux,default-trigger = "heartbeat";
>>>>>               };
>>>>>
>>>>
>>>>> -             diy-led {
>>>>> +             diy_led: diy-led {
>>>>
>>>> This changes an existing nodename into something that is still not the
>>
>> Correction:
>> It takes an existing nodename and adds a label.
> 
> OK.
> 
>>>> preferred way. In the current Rockchip dts there are nodenames like
>>>> 'work', 'yellow' that causing warnings with the command:
>>>
>>> This doesn't change the node name at all. It only adds a label.
>>> If it doesn't pass the check now, it didn't pass the check before.
>>>
>>> I just realized that the footnote I added before is gone because I
>>> regenerated the patches. The original footnote was something along
>>> the lines of:
>>>
>>> I opted to not change the node names nor the labels as the discussion
>>> had not concluded. The other reason being that people may have scripts
>>> or device tree overlays depending on the existing node names.
>>>
>>> Previously I asked the following but got no response:
>>>
>>>     Is changing this after it has been in some kernel releases OK? Wouldn't
>>>     it be considered a break of sysfs ABI?
>>>
>>>     Also, is there some guideline on how to name the labels? For sunxi we've
>>>     been doing "${vendor}:${color}:${function}" since forever.
>>>
>>>     As far as I can tell, the hardware vendor [1] has no specific uses for
>>>     these two (red and yellow) LEDs designed in. And their GPIO lines are
>>>     simply labeled "DIY" (for the red one) and "YELLOW". So I'm not sure
>>>     if putting "our" interpretations and the default-trigger into the
>>>     label is wise.
>>>
>>>     For reference, the green one has its GPIO line labeled "WORK", and their
>>>     intention from [1] is to have it as some sort of power / activity indicator.
>>>     Hence it is named / labeled "work".
>>>
>>>     As for the node names, I think we can keep it as is for now. It's not
>>>     the preferred form, but there's really no need to change it either.
>>>     And some overlay or script might actually expect that name.
>>>
>>>> make -k ARCH=arm dtbs_check
>>>>
>>>> Could you give a generic guide line/example, so all these changes are
>>>> treated the same way? As if the naming follows the preferred 'led-0' line.
>>>
>>> I'm not sure what you are asking for.
>>
>> Your nodename just happend to contain 'led' to pass the regex.
>> There are many other names in use.
> 
> Right. So if it passes, what's the problem?
> 
>> 'If' the DT maintainer (=Heiko) decides the get rid of all the warnings
>> for led nodes then that change would require all nodename to be handled
>> to same (=preferred way):
>>
>> Change this:
>>
>> diy_led: diy-led
>> yellow_led: yellow-led
>>
>> Into something like:
>>
>> led_0: led-0
>> led_1: led-1
> 
> As I already said, if the maintainers want to clean this up, I am happy to
> provide patches towards this. That is not the case right now. Furthermore,
> that cleanup is not directly related to what I'm trying to fix in this
> patch, which is, from the original submitter's point of view, incorrect
> triggers are used when the mezzanine board is added.
> 
> Also, DT labels "led_0" and "led_1" are useless. They have no relation to
> what is used in the schematics, which are "work", "diy", and "yellow". The
> board itself doesn't have anything silk-screened on, so on that end the only
> thing to go with is the color.
> 
> So for fixing up the LED node names, we'd probably want the following:
> 
>     diy_led: led-0
>     yellow_led: led-1
>     work_led: led-2

That doesn't look pretty either.
Would like to hear the maintainers view on how to handle other cases
without 'led' like for example 'blue' for mk808.

> 
> Is that what you're asking for?
> 
> 
> ChenYu
> 
>>>
>>> ChenYu
>>>
>>>>>                       label = "red:diy";
>>>>>                       gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
>>>>>                       default-state = "off";
>>>>>                       linux,default-trigger = "mmc1";
>>>>>               };
>>>>>
>>>>> -             yellow-led {
>>>>> +             yellow_led: yellow-led {
>>>>>                       label = "yellow:yellow-led";
>>>>>                       gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
>>>>>                       default-state = "off";
>>>>> --
>>>>> 2.26.0
>>>>
>>


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

* Re: [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27 10:55           ` Johan Jonker
@ 2020-04-27 14:12             ` Johan Jonker
  2020-04-27 14:45               ` Chen-Yu Tsai
  2020-04-27 15:13               ` Robin Murphy
  0 siblings, 2 replies; 16+ messages in thread
From: Johan Jonker @ 2020-04-27 14:12 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: devicetree, dmurphy, Heiko Stübner, jacek.anaszewski,
	linux-arm-kernel, linux-kernel, linux-leds,
	open list:ARM/Rockchip SoC...,
	Pavel Machek, Rob Herring

Hi,

>> So for fixing up the LED node names, we'd probably want the following:
>>
>>     diy_led: led-0
>>     yellow_led: led-1
>>     work_led: led-2

Change proposal for led nodes to comply with preexisting dts.
Does this work?

diy_led: led_0: led-0
yellow_led: led_1: led-1
work_led: led_2: led-2


blue: led_0: led-0

A check does not give any warnings.

make -k ARCH=arm dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml

> 
> That doesn't look pretty either.
> Would like to hear the maintainers view on how to handle other cases
> without 'led' like for example 'blue' for mk808.
> 

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

* Re: [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27 14:12             ` Johan Jonker
@ 2020-04-27 14:45               ` Chen-Yu Tsai
  2020-04-27 15:13               ` Robin Murphy
  1 sibling, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27 14:45 UTC (permalink / raw)
  To: Johan Jonker
  Cc: Chen-Yu Tsai, devicetree, dmurphy, Heiko Stübner,
	jacek.anaszewski, linux-arm-kernel, linux-kernel, linux-leds,
	open list:ARM/Rockchip SoC...,
	Pavel Machek, Rob Herring

On Mon, Apr 27, 2020 at 10:12 PM Johan Jonker <jbx6244@gmail.com> wrote:
>
> Hi,
>
> >> So for fixing up the LED node names, we'd probably want the following:
> >>
> >>     diy_led: led-0
> >>     yellow_led: led-1
> >>     work_led: led-2
>
> Change proposal for led nodes to comply with preexisting dts.
> Does this work?
>
> diy_led: led_0: led-0
> yellow_led: led_1: led-1
> work_led: led_2: led-2
>
>
> blue: led_0: led-0
>
> A check does not give any warnings.
>
> make -k ARCH=arm dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml

IMO the led_N labels are kind of useless... they don't convey the
information needed to identify the device.

A reader seeing

    &led_0 {
        linux,default_triggger = "activity";
    };

in some device tree (overlay) has no idea what this snippet is supposed
to do. The person has to go back to the base dts / dtsi file to figure
that out.

But seriously, you should start a separate thread to discuss this issue.

ChenYu

> >
> > That doesn't look pretty either.
> > Would like to hear the maintainers view on how to handle other cases
> > without 'led' like for example 'blue' for mk808.
> >

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

* Re: [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
  2020-04-27 14:12             ` Johan Jonker
  2020-04-27 14:45               ` Chen-Yu Tsai
@ 2020-04-27 15:13               ` Robin Murphy
  1 sibling, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2020-04-27 15:13 UTC (permalink / raw)
  To: Johan Jonker, Chen-Yu Tsai
  Cc: devicetree, Heiko Stübner, Pavel Machek, linux-kernel,
	open list:ARM/Rockchip SoC...,
	Rob Herring, jacek.anaszewski, linux-leds, linux-arm-kernel,
	dmurphy

On 2020-04-27 3:12 pm, Johan Jonker wrote:
> Hi,
> 
>>> So for fixing up the LED node names, we'd probably want the following:
>>>
>>>      diy_led: led-0
>>>      yellow_led: led-1
>>>      work_led: led-2
> 
> Change proposal for led nodes to comply with preexisting dts.
> Does this work?
> 
> diy_led: led_0: led-0
> yellow_led: led_1: led-1
> work_led: led_2: led-2

Yuck, why?

Labels are simply human-readable source annotations for the sake of 
referencing nodes more easily. Meaningful label names - that correlate 
to SoC or board components, schematic names, or physical labels on the 
board/device - make the DT sources easier to read, review, and maintain. 
There are a handful of cases where one node might have multiple labels, 
e.g. if two logical supply nets come from the same regulator on certain 
board variants, but there is zero point in defining redundant labels 
that just meaninglessly echo the DT's own structure.

> blue: led_0: led-0
> 
> A check does not give any warnings.

I should hope not. Labels are there to be consumed by DT compilers (and 
whatever symbolic overlay handlers count as... DT linkers, maybe?) - 
they have no business being within the scope of the bindings that define 
a contract for system software consuming the final DTB.

Robin.

> make -k ARCH=arm dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml
> 
>>
>> That doesn't look pretty either.
>> Would like to hear the maintainers view on how to handle other cases
>> without 'led' like for example 'blue' for mk808.
>>
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

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

* Re: [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements
  2020-04-27  7:31 [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements Chen-Yu Tsai
                   ` (2 preceding siblings ...)
  2020-04-27  7:31 ` [PATCH v2 3/3] arm64: dts: rockchip: rk3328-roc-cc: Set dr_mode to "host" for OTG Chen-Yu Tsai
@ 2020-05-05  6:24 ` Chen-Yu Tsai
  3 siblings, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2020-05-05  6:24 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Jacek Anaszewski, Pavel Machek, Dan Murphy
  Cc: open list:ARM/Rockchip SoC...,
	linux-arm-kernel, devicetree, linux-leds, linux-kernel,
	Chen-Yu Tsai

Hi,

On Mon, Apr 27, 2020 at 3:31 PM Chen-Yu Tsai <wens@kernel.org> wrote:
>
> From: Chen-Yu Tsai <wens@csie.org>
>
> Hi,
>
> Here is the remaining patch for roc-rk3399-pc, along with a few other
> new patches.
>
>   - Based on discussions from v1, patch one, newly added, drops the list
>     of valid values for linux,default-triggers.
>
>   - Patch two is the same as in v1
>
>   - Patch three, new, sets dr_mode to "host" to the dwc2 OTG controller,
>     matching what the board uses it for, a host port. This gets rid of
>     a warning from the kernel.
>
> Please have a look.

Are there any comments from the maintainers?

I feel like this is getting needlessly sidetracked by the endless discussion
about GPIO LED node names which I left untouched on purpose in this series.

ChenYu


> Regards
> ChenYu
>
>
> Chen-Yu Tsai (3):
>   dt-bindings: leds: common: Drop enumeration for linux,default-triggers
>   arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED
>     triggers
>   arm64: dts: rockchip: rk3328-roc-cc: Set dr_mode to "host" for OTG
>
>  .../devicetree/bindings/leds/common.yaml      | 21 +------------------
>  .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts |  1 +
>  .../dts/rockchip/rk3399-roc-pc-mezzanine.dts  |  8 +++++++
>  .../boot/dts/rockchip/rk3399-roc-pc.dtsi      |  4 ++--
>  4 files changed, 12 insertions(+), 22 deletions(-)
>
> --
> 2.26.0
>

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

* Re: [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers
  2020-04-27  7:31 ` [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers Chen-Yu Tsai
  2020-04-27  8:33   ` Johan Jonker
@ 2020-05-11 23:12   ` Rob Herring
  1 sibling, 0 replies; 16+ messages in thread
From: Rob Herring @ 2020-05-11 23:12 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Heiko Stuebner, Jacek Anaszewski, Pavel Machek, Dan Murphy,
	Chen-Yu Tsai, linux-rockchip, linux-arm-kernel, devicetree,
	linux-leds, linux-kernel

On Mon, Apr 27, 2020 at 03:31:30PM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The bindings currently list a very small subset of valid triggers for
> LEDs. Since many drivers or subsystems in Linux register custom
> triggers, the list would become very hard to maintain.

The idea was we'll consolidate around standardized names and that new 
users should use 'function' instead.

There was discussion around this when adding 'function'.

> 
> Instead, just drop the list and allow free form strings.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  .../devicetree/bindings/leds/common.yaml      | 21 +------------------
>  1 file changed, 1 insertion(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> index 4c270fde4567..3b3cdab3fc15 100644
> --- a/Documentation/devicetree/bindings/leds/common.yaml
> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> @@ -79,26 +79,7 @@ properties:
>      description:
>        This parameter, if present, is a string defining the trigger assigned to
>        the LED.
> -    allOf:
> -      - $ref: /schemas/types.yaml#definitions/string
> -    enum:
> -        # LED will act as a back-light, controlled by the framebuffer system
> -      - backlight
> -        # LED will turn on (but for leds-gpio see "default-state" property in
> -        # Documentation/devicetree/bindings/leds/leds-gpio.yaml)
> -      - default-on
> -        # LED "double" flashes at a load average based rate
> -      - heartbeat
> -        # LED indicates disk activity
> -      - disk-activity
> -        # LED indicates IDE disk activity (deprecated), in new implementations
> -        # use "disk-activity"
> -      - ide-disk
> -        # LED flashes at a fixed, configurable rate
> -      - timer
> -        # LED alters the brightness for the specified duration with one software
> -        # timer (requires "led-pattern" property)
> -      - pattern
> +    $ref: /schemas/types.yaml#definitions/string
>  
>    led-pattern:
>      description: |
> -- 
> 2.26.0
> 

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

end of thread, other threads:[~2020-05-11 23:12 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-27  7:31 [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements Chen-Yu Tsai
2020-04-27  7:31 ` [PATCH v2 1/3] dt-bindings: leds: common: Drop enumeration for linux,default-triggers Chen-Yu Tsai
2020-04-27  8:33   ` Johan Jonker
2020-04-27  9:21     ` Chen-Yu Tsai
2020-05-11 23:12   ` Rob Herring
2020-04-27  7:31 ` [PATCH v2 2/3] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers Chen-Yu Tsai
2020-04-27  8:57   ` Johan Jonker
2020-04-27  9:17     ` Chen-Yu Tsai
2020-04-27 10:08       ` Johan Jonker
2020-04-27 10:33         ` Chen-Yu Tsai
2020-04-27 10:55           ` Johan Jonker
2020-04-27 14:12             ` Johan Jonker
2020-04-27 14:45               ` Chen-Yu Tsai
2020-04-27 15:13               ` Robin Murphy
2020-04-27  7:31 ` [PATCH v2 3/3] arm64: dts: rockchip: rk3328-roc-cc: Set dr_mode to "host" for OTG Chen-Yu Tsai
2020-05-05  6:24 ` [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements Chen-Yu Tsai

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