devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties
@ 2022-06-24 17:07 Krzysztof Kozlowski
  2022-06-24 17:07 ` [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema Krzysztof Kozlowski
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2022-06-24 17:07 UTC (permalink / raw)
  To: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski,
	Thomas Bogendoerfer, Paul Cercueil, Matthias Brugger,
	Arınç ÜNAL, Sergio Paracuellos, linux-mips,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Krzysztof Kozlowski

gpio-keys children do not use unit addresses.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

See: https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@linaro.org/
---
 arch/mips/boot/dts/img/pistachio_marduk.dts   |  4 +--
 arch/mips/boot/dts/ingenic/gcw0.dts           | 31 +++++++++----------
 arch/mips/boot/dts/ingenic/rs90.dts           | 18 +++++------
 arch/mips/boot/dts/pic32/pic32mzda_sk.dts     |  9 ++----
 .../boot/dts/qca/ar9132_tl_wr1043nd_v1.dts    |  6 ++--
 arch/mips/boot/dts/qca/ar9331_dpt_module.dts  |  4 +--
 .../mips/boot/dts/qca/ar9331_dragino_ms14.dts |  6 ++--
 arch/mips/boot/dts/qca/ar9331_omega.dts       |  4 +--
 .../qca/ar9331_openembed_som9331_board.dts    |  4 +--
 arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts   |  8 ++---
 10 files changed, 37 insertions(+), 57 deletions(-)

diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
index a8708783f04b..a8da2f992b1a 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -59,12 +59,12 @@ led-1 {
 
 	keys {
 		compatible = "gpio-keys";
-		button@1 {
+		button-1 {
 			label = "Button 1";
 			linux,code = <0x101>; /* BTN_1 */
 			gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
 		};
-		button@2 {
+		button-2 {
 			label = "Button 2";
 			linux,code = <0x102>; /* BTN_2 */
 			gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
diff --git a/arch/mips/boot/dts/ingenic/gcw0.dts b/arch/mips/boot/dts/ingenic/gcw0.dts
index 4abb0318416c..5d33f26fd28c 100644
--- a/arch/mips/boot/dts/ingenic/gcw0.dts
+++ b/arch/mips/boot/dts/ingenic/gcw0.dts
@@ -130,89 +130,86 @@ backlight: backlight {
 
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
 		autorepeat;
 
-		button@0 {
+		button-0 {
 			label = "D-pad up";
 			linux,code = <KEY_UP>;
 			linux,can-disable;
 			gpios = <&gpe 21 GPIO_ACTIVE_LOW>;
 		};
 
-		button@1 {
+		button-1 {
 			label = "D-pad down";
 			linux,code = <KEY_DOWN>;
 			linux,can-disable;
 			gpios = <&gpe 25 GPIO_ACTIVE_LOW>;
 		};
 
-		button@2 {
+		button-2 {
 			label = "D-pad left";
 			linux,code = <KEY_LEFT>;
 			linux,can-disable;
 			gpios = <&gpe 23 GPIO_ACTIVE_LOW>;
 		};
 
-		button@3 {
+		button-3 {
 			label = "D-pad right";
 			linux,code = <KEY_RIGHT>;
 			linux,can-disable;
 			gpios = <&gpe 24 GPIO_ACTIVE_LOW>;
 		};
 
-		button@4 {
+		button-4 {
 			label = "Button A";
 			linux,code = <KEY_LEFTCTRL>;
 			linux,can-disable;
 			gpios = <&gpe 29 GPIO_ACTIVE_LOW>;
 		};
 
-		button@5 {
+		button-5 {
 			label = "Button B";
 			linux,code = <KEY_LEFTALT>;
 			linux,can-disable;
 			gpios = <&gpe 20 GPIO_ACTIVE_LOW>;
 		};
 
-		button@6 {
+		button-6 {
 			label = "Button Y";
 			linux,code = <KEY_SPACE>;
 			linux,can-disable;
 			gpios = <&gpe 27 GPIO_ACTIVE_LOW>;
 		};
 
-		button@7 {
+		button-7 {
 			label = "Button X";
 			linux,code = <KEY_LEFTSHIFT>;
 			linux,can-disable;
 			gpios = <&gpe 28 GPIO_ACTIVE_LOW>;
 		};
 
-		button@8 {
+		button-8 {
 			label = "Left shoulder button";
 			linux,code = <KEY_TAB>;
 			linux,can-disable;
 			gpios = <&gpb 20 GPIO_ACTIVE_LOW>;
 		};
 
-		button@9 {
+		button-9 {
 			label = "Right shoulder button";
 			linux,code = <KEY_BACKSPACE>;
 			linux,can-disable;
 			gpios = <&gpe 26 GPIO_ACTIVE_LOW>;
 		};
 
-		button@10 {
+		button-10 {
 			label = "Start button";
 			linux,code = <KEY_ENTER>;
 			linux,can-disable;
 			gpios = <&gpb 21 GPIO_ACTIVE_LOW>;
 		};
 
-		button@11 {
+		button-11 {
 			label = "Select button";
 			linux,code = <KEY_ESC>;
 			linux,can-disable;
@@ -223,7 +220,7 @@ button@11 {
 			gpios = <&gpd 18 GPIO_ACTIVE_HIGH>;
 		};
 
-		button@12 {
+		button-12 {
 			label = "Power slider";
 			linux,code = <KEY_POWER>;
 			linux,can-disable;
@@ -231,7 +228,7 @@ button@12 {
 			wakeup-source;
 		};
 
-		button@13 {
+		button-13 {
 			label = "Power hold";
 			linux,code = <KEY_PAUSE>;
 			linux,can-disable;
diff --git a/arch/mips/boot/dts/ingenic/rs90.dts b/arch/mips/boot/dts/ingenic/rs90.dts
index 74fee7f01352..e8df70dd42bf 100644
--- a/arch/mips/boot/dts/ingenic/rs90.dts
+++ b/arch/mips/boot/dts/ingenic/rs90.dts
@@ -52,53 +52,51 @@ backlight: backlight {
 
 	keys@0 {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
 
-		key@0 {
+		key-0 {
 			label = "D-pad up";
 			linux,code = <KEY_UP>;
 			gpios = <&gpc 10 GPIO_ACTIVE_LOW>;
 		};
 
-		key@1 {
+		key-1 {
 			label = "D-pad down";
 			linux,code = <KEY_DOWN>;
 			gpios = <&gpc 11 GPIO_ACTIVE_LOW>;
 		};
 
-		key@2 {
+		key-2 {
 			label = "D-pad left";
 			linux,code = <KEY_LEFT>;
 			gpios = <&gpb 31 GPIO_ACTIVE_LOW>;
 		};
 
-		key@3 {
+		key-3 {
 			label = "D-pad right";
 			linux,code = <KEY_RIGHT>;
 			gpios = <&gpd 21 GPIO_ACTIVE_LOW>;
 		};
 
-		key@4 {
+		key-4 {
 			label = "Button A";
 			linux,code = <KEY_LEFTCTRL>;
 			gpios = <&gpc 31 GPIO_ACTIVE_LOW>;
 		};
 
-		key@5 {
+		key-5 {
 			label = "Button B";
 			linux,code = <KEY_LEFTALT>;
 			gpios = <&gpc 30 GPIO_ACTIVE_LOW>;
 		};
 
-		key@6 {
+		key-6 {
 			label = "Right shoulder button";
 			linux,code = <KEY_BACKSPACE>;
 			gpios = <&gpc 12 GPIO_ACTIVE_LOW>;
 			debounce-interval = <10>;
 		};
 
-		key@7 {
+		key-7 {
 			label = "Start button";
 			linux,code = <KEY_ENTER>;
 			gpios = <&gpd 17 GPIO_ACTIVE_LOW>;
diff --git a/arch/mips/boot/dts/pic32/pic32mzda_sk.dts b/arch/mips/boot/dts/pic32/pic32mzda_sk.dts
index d7fa5d55dbf3..ab70637bbec5 100644
--- a/arch/mips/boot/dts/pic32/pic32mzda_sk.dts
+++ b/arch/mips/boot/dts/pic32/pic32mzda_sk.dts
@@ -52,22 +52,19 @@ keys0 {
 		pinctrl-0 = <&user_buttons_s0>;
 		pinctrl-names = "default";
 
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		button@sw1 {
+		button-1 {
 			label = "ESC";
 			linux,code = <1>;
 			gpios = <&gpio1 12 0>;
 		};
 
-		button@sw2 {
+		button-2 {
 			label = "Home";
 			linux,code = <102>;
 			gpios = <&gpio1 13 0>;
 		};
 
-		button@sw3 {
+		button-3 {
 			label = "Menu";
 			linux,code = <139>;
 			gpios = <&gpio1 14 0>;
diff --git a/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts b/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
index 7fccf6357225..f3dff4009ab5 100644
--- a/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
+++ b/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
@@ -23,17 +23,15 @@ extosc: ref {
 
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
 
-		button@0 {
+		button-0 {
 			label = "reset";
 			linux,code = <KEY_RESTART>;
 			gpios = <&gpio 3 GPIO_ACTIVE_LOW>;
 			debounce-interval = <60>;
 		};
 
-		button@1 {
+		button-1 {
 			label = "qss";
 			linux,code = <KEY_WPS_BUTTON>;
 			gpios = <&gpio 7 GPIO_ACTIVE_LOW>;
diff --git a/arch/mips/boot/dts/qca/ar9331_dpt_module.dts b/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
index 7695d326df11..c857cd22f7db 100644
--- a/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
+++ b/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
@@ -33,10 +33,8 @@ led-0 {
 
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
 
-		button@0 {
+		button-0 {
 			label = "reset";
 			linux,code = <KEY_RESTART>;
 			gpios = <&gpio 11 GPIO_ACTIVE_LOW>;
diff --git a/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts b/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
index d38aa73f1a2e..40e4c5da0e65 100644
--- a/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
+++ b/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
@@ -49,16 +49,14 @@ system {
 
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
 
-		button@0 {
+		button-0 {
 			label = "jumpstart";
 			linux,code = <KEY_WPS_BUTTON>;
 			gpios = <&gpio 11 GPIO_ACTIVE_LOW>;
 		};
 
-		button@1 {
+		button-1 {
 			label = "reset";
 			linux,code = <KEY_RESTART>;
 			gpios = <&gpio 12 GPIO_ACTIVE_LOW>;
diff --git a/arch/mips/boot/dts/qca/ar9331_omega.dts b/arch/mips/boot/dts/qca/ar9331_omega.dts
index 11778abacf66..ed184d861d5f 100644
--- a/arch/mips/boot/dts/qca/ar9331_omega.dts
+++ b/arch/mips/boot/dts/qca/ar9331_omega.dts
@@ -31,10 +31,8 @@ system {
 
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
 
-		button@0 {
+		button-0 {
 			label = "reset";
 			linux,code = <KEY_RESTART>;
 			gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
diff --git a/arch/mips/boot/dts/qca/ar9331_openembed_som9331_board.dts b/arch/mips/boot/dts/qca/ar9331_openembed_som9331_board.dts
index e6622f8e8c2b..dc65ebd60bbc 100644
--- a/arch/mips/boot/dts/qca/ar9331_openembed_som9331_board.dts
+++ b/arch/mips/boot/dts/qca/ar9331_openembed_som9331_board.dts
@@ -33,10 +33,8 @@ led-0 {
 
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
 
-		button@0 {
+		button-0 {
 			label = "reset";
 			linux,code = <KEY_RESTART>;
 			gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
diff --git a/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts b/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
index c8290d36cfbe..5f424c2cd781 100644
--- a/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
+++ b/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
@@ -49,22 +49,20 @@ led3g {
 
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
 
-		button@0 {
+		button-0 {
 			label = "wps";
 			linux,code = <KEY_WPS_BUTTON>;
 			gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
 		};
 
-		button@1 {
+		button-1 {
 			label = "sw1";
 			linux,code = <BTN_0>;
 			gpios = <&gpio 18 GPIO_ACTIVE_HIGH>;
 		};
 
-		button@2 {
+		button-2 {
 			label = "sw2";
 			linux,code = <BTN_1>;
 			gpios = <&gpio 20 GPIO_ACTIVE_HIGH>;
-- 
2.34.1


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

* [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema
  2022-06-24 17:07 [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties Krzysztof Kozlowski
@ 2022-06-24 17:07 ` Krzysztof Kozlowski
  2022-06-29 12:12   ` Arınç ÜNAL
  2022-07-14  9:56   ` Thomas Bogendoerfer
  2022-06-24 18:40 ` [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties Paul Cercueil
  2022-07-14  9:55 ` Thomas Bogendoerfer
  2 siblings, 2 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2022-06-24 17:07 UTC (permalink / raw)
  To: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski,
	Thomas Bogendoerfer, Paul Cercueil, Matthias Brugger,
	Arınç ÜNAL, Sergio Paracuellos, linux-mips,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Krzysztof Kozlowski

The node names should be generic and DT schema expects certain pattern
(e.g. with key/button/switch).

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/mips/boot/dts/ingenic/ci20.dts                        | 2 +-
 arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts | 2 +-
 arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc1.dts         | 2 +-
 arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc2.dts         | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
index ab6e3dc0bc1d..37c46720c719 100644
--- a/arch/mips/boot/dts/ingenic/ci20.dts
+++ b/arch/mips/boot/dts/ingenic/ci20.dts
@@ -31,7 +31,7 @@ memory {
 	gpio-keys {
 		compatible = "gpio-keys";
 
-		sw1 {
+		switch {
 			label = "ci20:sw1";
 			linux,code = <KEY_F13>;
 			gpios = <&gpd 17 GPIO_ACTIVE_HIGH>;
diff --git a/arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts b/arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts
index 826e91b840a3..179558161f85 100644
--- a/arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts
+++ b/arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts
@@ -26,7 +26,7 @@ gpio-keys {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinmux_gpio_gpio>;	/* GPIO11 */
 
-		user_btn1 {
+		button {
 			label = "USER_BTN1";
 			gpios = <&gpio 11 GPIO_ACTIVE_LOW>;
 			linux,code =<KEY_PROG1> ;
diff --git a/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc1.dts b/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc1.dts
index 37037e4f3c3b..24eebc5a85b1 100644
--- a/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc1.dts
+++ b/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc1.dts
@@ -29,7 +29,7 @@ i2c@900 {
 	gpio-keys {
 		compatible = "gpio-keys";
 
-		reset {
+		key-reset {
 			label = "reset";
 			gpios = <&gpio 18 GPIO_ACTIVE_HIGH>;
 			linux,code = <KEY_RESTART>;
diff --git a/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc2.dts b/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc2.dts
index a6201a119a1f..34006e667780 100644
--- a/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc2.dts
+++ b/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc2.dts
@@ -29,7 +29,7 @@ i2c@900 {
 	gpio-keys {
 		compatible = "gpio-keys";
 
-		reset {
+		key-reset {
 			label = "reset";
 			gpios = <&gpio 18 GPIO_ACTIVE_HIGH>;
 			linux,code = <KEY_RESTART>;
-- 
2.34.1


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

* Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties
  2022-06-24 17:07 [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties Krzysztof Kozlowski
  2022-06-24 17:07 ` [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema Krzysztof Kozlowski
@ 2022-06-24 18:40 ` Paul Cercueil
  2022-06-25 19:58   ` Krzysztof Kozlowski
  2022-07-14  9:55 ` Thomas Bogendoerfer
  2 siblings, 1 reply; 10+ messages in thread
From: Paul Cercueil @ 2022-06-24 18:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski,
	Thomas Bogendoerfer, Matthias Brugger, Arınç ÜNAL,
	Sergio Paracuellos, linux-mips, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

Hi Krzysztof,

Le ven., juin 24 2022 at 19:07:39 +0200, Krzysztof Kozlowski 
<krzysztof.kozlowski@linaro.org> a écrit :
> gpio-keys children do not use unit addresses.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> See: 
> https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@linaro.org/
> ---
>  arch/mips/boot/dts/img/pistachio_marduk.dts   |  4 +--
>  arch/mips/boot/dts/ingenic/gcw0.dts           | 31 
> +++++++++----------
>  arch/mips/boot/dts/ingenic/rs90.dts           | 18 +++++------
>  arch/mips/boot/dts/pic32/pic32mzda_sk.dts     |  9 ++----
>  .../boot/dts/qca/ar9132_tl_wr1043nd_v1.dts    |  6 ++--
>  arch/mips/boot/dts/qca/ar9331_dpt_module.dts  |  4 +--
>  .../mips/boot/dts/qca/ar9331_dragino_ms14.dts |  6 ++--
>  arch/mips/boot/dts/qca/ar9331_omega.dts       |  4 +--
>  .../qca/ar9331_openembed_som9331_board.dts    |  4 +--
>  arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts   |  8 ++---
>  10 files changed, 37 insertions(+), 57 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts 
> b/arch/mips/boot/dts/img/pistachio_marduk.dts
> index a8708783f04b..a8da2f992b1a 100644
> --- a/arch/mips/boot/dts/img/pistachio_marduk.dts
> +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
> @@ -59,12 +59,12 @@ led-1 {
> 
>  	keys {
>  		compatible = "gpio-keys";
> -		button@1 {
> +		button-1 {
>  			label = "Button 1";
>  			linux,code = <0x101>; /* BTN_1 */
>  			gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
>  		};
> -		button@2 {
> +		button-2 {
>  			label = "Button 2";
>  			linux,code = <0x102>; /* BTN_2 */
>  			gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
> diff --git a/arch/mips/boot/dts/ingenic/gcw0.dts 
> b/arch/mips/boot/dts/ingenic/gcw0.dts
> index 4abb0318416c..5d33f26fd28c 100644
> --- a/arch/mips/boot/dts/ingenic/gcw0.dts
> +++ b/arch/mips/boot/dts/ingenic/gcw0.dts
> @@ -130,89 +130,86 @@ backlight: backlight {
> 
>  	gpio-keys {
>  		compatible = "gpio-keys";
> -		#address-cells = <1>;
> -		#size-cells = <0>;

Are you sure you can remove these?

Looking at paragraph 2.3.5 of the DT spec, I would think they have to 
stay (although with #address-cells = <0>).

Cheers,
-Paul

> -
>  		autorepeat;
> 
> -		button@0 {
> +		button-0 {
>  			label = "D-pad up";
>  			linux,code = <KEY_UP>;
>  			linux,can-disable;
>  			gpios = <&gpe 21 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@1 {
> +		button-1 {
>  			label = "D-pad down";
>  			linux,code = <KEY_DOWN>;
>  			linux,can-disable;
>  			gpios = <&gpe 25 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@2 {
> +		button-2 {
>  			label = "D-pad left";
>  			linux,code = <KEY_LEFT>;
>  			linux,can-disable;
>  			gpios = <&gpe 23 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@3 {
> +		button-3 {
>  			label = "D-pad right";
>  			linux,code = <KEY_RIGHT>;
>  			linux,can-disable;
>  			gpios = <&gpe 24 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@4 {
> +		button-4 {
>  			label = "Button A";
>  			linux,code = <KEY_LEFTCTRL>;
>  			linux,can-disable;
>  			gpios = <&gpe 29 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@5 {
> +		button-5 {
>  			label = "Button B";
>  			linux,code = <KEY_LEFTALT>;
>  			linux,can-disable;
>  			gpios = <&gpe 20 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@6 {
> +		button-6 {
>  			label = "Button Y";
>  			linux,code = <KEY_SPACE>;
>  			linux,can-disable;
>  			gpios = <&gpe 27 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@7 {
> +		button-7 {
>  			label = "Button X";
>  			linux,code = <KEY_LEFTSHIFT>;
>  			linux,can-disable;
>  			gpios = <&gpe 28 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@8 {
> +		button-8 {
>  			label = "Left shoulder button";
>  			linux,code = <KEY_TAB>;
>  			linux,can-disable;
>  			gpios = <&gpb 20 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@9 {
> +		button-9 {
>  			label = "Right shoulder button";
>  			linux,code = <KEY_BACKSPACE>;
>  			linux,can-disable;
>  			gpios = <&gpe 26 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@10 {
> +		button-10 {
>  			label = "Start button";
>  			linux,code = <KEY_ENTER>;
>  			linux,can-disable;
>  			gpios = <&gpb 21 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@11 {
> +		button-11 {
>  			label = "Select button";
>  			linux,code = <KEY_ESC>;
>  			linux,can-disable;
> @@ -223,7 +220,7 @@ button@11 {
>  			gpios = <&gpd 18 GPIO_ACTIVE_HIGH>;
>  		};
> 
> -		button@12 {
> +		button-12 {
>  			label = "Power slider";
>  			linux,code = <KEY_POWER>;
>  			linux,can-disable;
> @@ -231,7 +228,7 @@ button@12 {
>  			wakeup-source;
>  		};
> 
> -		button@13 {
> +		button-13 {
>  			label = "Power hold";
>  			linux,code = <KEY_PAUSE>;
>  			linux,can-disable;
> diff --git a/arch/mips/boot/dts/ingenic/rs90.dts 
> b/arch/mips/boot/dts/ingenic/rs90.dts
> index 74fee7f01352..e8df70dd42bf 100644
> --- a/arch/mips/boot/dts/ingenic/rs90.dts
> +++ b/arch/mips/boot/dts/ingenic/rs90.dts
> @@ -52,53 +52,51 @@ backlight: backlight {
> 
>  	keys@0 {
>  		compatible = "gpio-keys";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> 
> -		key@0 {
> +		key-0 {
>  			label = "D-pad up";
>  			linux,code = <KEY_UP>;
>  			gpios = <&gpc 10 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		key@1 {
> +		key-1 {
>  			label = "D-pad down";
>  			linux,code = <KEY_DOWN>;
>  			gpios = <&gpc 11 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		key@2 {
> +		key-2 {
>  			label = "D-pad left";
>  			linux,code = <KEY_LEFT>;
>  			gpios = <&gpb 31 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		key@3 {
> +		key-3 {
>  			label = "D-pad right";
>  			linux,code = <KEY_RIGHT>;
>  			gpios = <&gpd 21 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		key@4 {
> +		key-4 {
>  			label = "Button A";
>  			linux,code = <KEY_LEFTCTRL>;
>  			gpios = <&gpc 31 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		key@5 {
> +		key-5 {
>  			label = "Button B";
>  			linux,code = <KEY_LEFTALT>;
>  			gpios = <&gpc 30 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		key@6 {
> +		key-6 {
>  			label = "Right shoulder button";
>  			linux,code = <KEY_BACKSPACE>;
>  			gpios = <&gpc 12 GPIO_ACTIVE_LOW>;
>  			debounce-interval = <10>;
>  		};
> 
> -		key@7 {
> +		key-7 {
>  			label = "Start button";
>  			linux,code = <KEY_ENTER>;
>  			gpios = <&gpd 17 GPIO_ACTIVE_LOW>;
> diff --git a/arch/mips/boot/dts/pic32/pic32mzda_sk.dts 
> b/arch/mips/boot/dts/pic32/pic32mzda_sk.dts
> index d7fa5d55dbf3..ab70637bbec5 100644
> --- a/arch/mips/boot/dts/pic32/pic32mzda_sk.dts
> +++ b/arch/mips/boot/dts/pic32/pic32mzda_sk.dts
> @@ -52,22 +52,19 @@ keys0 {
>  		pinctrl-0 = <&user_buttons_s0>;
>  		pinctrl-names = "default";
> 
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		button@sw1 {
> +		button-1 {
>  			label = "ESC";
>  			linux,code = <1>;
>  			gpios = <&gpio1 12 0>;
>  		};
> 
> -		button@sw2 {
> +		button-2 {
>  			label = "Home";
>  			linux,code = <102>;
>  			gpios = <&gpio1 13 0>;
>  		};
> 
> -		button@sw3 {
> +		button-3 {
>  			label = "Menu";
>  			linux,code = <139>;
>  			gpios = <&gpio1 14 0>;
> diff --git a/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts 
> b/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
> index 7fccf6357225..f3dff4009ab5 100644
> --- a/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
> +++ b/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
> @@ -23,17 +23,15 @@ extosc: ref {
> 
>  	gpio-keys {
>  		compatible = "gpio-keys";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> 
> -		button@0 {
> +		button-0 {
>  			label = "reset";
>  			linux,code = <KEY_RESTART>;
>  			gpios = <&gpio 3 GPIO_ACTIVE_LOW>;
>  			debounce-interval = <60>;
>  		};
> 
> -		button@1 {
> +		button-1 {
>  			label = "qss";
>  			linux,code = <KEY_WPS_BUTTON>;
>  			gpios = <&gpio 7 GPIO_ACTIVE_LOW>;
> diff --git a/arch/mips/boot/dts/qca/ar9331_dpt_module.dts 
> b/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
> index 7695d326df11..c857cd22f7db 100644
> --- a/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
> +++ b/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
> @@ -33,10 +33,8 @@ led-0 {
> 
>  	gpio-keys {
>  		compatible = "gpio-keys";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> 
> -		button@0 {
> +		button-0 {
>  			label = "reset";
>  			linux,code = <KEY_RESTART>;
>  			gpios = <&gpio 11 GPIO_ACTIVE_LOW>;
> diff --git a/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts 
> b/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
> index d38aa73f1a2e..40e4c5da0e65 100644
> --- a/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
> +++ b/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
> @@ -49,16 +49,14 @@ system {
> 
>  	gpio-keys {
>  		compatible = "gpio-keys";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> 
> -		button@0 {
> +		button-0 {
>  			label = "jumpstart";
>  			linux,code = <KEY_WPS_BUTTON>;
>  			gpios = <&gpio 11 GPIO_ACTIVE_LOW>;
>  		};
> 
> -		button@1 {
> +		button-1 {
>  			label = "reset";
>  			linux,code = <KEY_RESTART>;
>  			gpios = <&gpio 12 GPIO_ACTIVE_LOW>;
> diff --git a/arch/mips/boot/dts/qca/ar9331_omega.dts 
> b/arch/mips/boot/dts/qca/ar9331_omega.dts
> index 11778abacf66..ed184d861d5f 100644
> --- a/arch/mips/boot/dts/qca/ar9331_omega.dts
> +++ b/arch/mips/boot/dts/qca/ar9331_omega.dts
> @@ -31,10 +31,8 @@ system {
> 
>  	gpio-keys {
>  		compatible = "gpio-keys";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> 
> -		button@0 {
> +		button-0 {
>  			label = "reset";
>  			linux,code = <KEY_RESTART>;
>  			gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
> diff --git 
> a/arch/mips/boot/dts/qca/ar9331_openembed_som9331_board.dts 
> b/arch/mips/boot/dts/qca/ar9331_openembed_som9331_board.dts
> index e6622f8e8c2b..dc65ebd60bbc 100644
> --- a/arch/mips/boot/dts/qca/ar9331_openembed_som9331_board.dts
> +++ b/arch/mips/boot/dts/qca/ar9331_openembed_som9331_board.dts
> @@ -33,10 +33,8 @@ led-0 {
> 
>  	gpio-keys {
>  		compatible = "gpio-keys";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> 
> -		button@0 {
> +		button-0 {
>  			label = "reset";
>  			linux,code = <KEY_RESTART>;
>  			gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
> diff --git a/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts 
> b/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
> index c8290d36cfbe..5f424c2cd781 100644
> --- a/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
> +++ b/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
> @@ -49,22 +49,20 @@ led3g {
> 
>  	gpio-keys {
>  		compatible = "gpio-keys";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> 
> -		button@0 {
> +		button-0 {
>  			label = "wps";
>  			linux,code = <KEY_WPS_BUTTON>;
>  			gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
>  		};
> 
> -		button@1 {
> +		button-1 {
>  			label = "sw1";
>  			linux,code = <BTN_0>;
>  			gpios = <&gpio 18 GPIO_ACTIVE_HIGH>;
>  		};
> 
> -		button@2 {
> +		button-2 {
>  			label = "sw2";
>  			linux,code = <BTN_1>;
>  			gpios = <&gpio 20 GPIO_ACTIVE_HIGH>;
> --
> 2.34.1
> 



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

* Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties
  2022-06-24 18:40 ` [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties Paul Cercueil
@ 2022-06-25 19:58   ` Krzysztof Kozlowski
  2022-06-25 20:15     ` Paul Cercueil
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2022-06-25 19:58 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski,
	Thomas Bogendoerfer, Matthias Brugger, Arınç ÜNAL,
	Sergio Paracuellos, linux-mips, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

On 24/06/2022 20:40, Paul Cercueil wrote:
> Hi Krzysztof,
> 
> Le ven., juin 24 2022 at 19:07:39 +0200, Krzysztof Kozlowski 
> <krzysztof.kozlowski@linaro.org> a écrit :
>> gpio-keys children do not use unit addresses.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> See: 
>> https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@linaro.org/
>> ---
>>  arch/mips/boot/dts/img/pistachio_marduk.dts   |  4 +--
>>  arch/mips/boot/dts/ingenic/gcw0.dts           | 31 
>> +++++++++----------
>>  arch/mips/boot/dts/ingenic/rs90.dts           | 18 +++++------
>>  arch/mips/boot/dts/pic32/pic32mzda_sk.dts     |  9 ++----
>>  .../boot/dts/qca/ar9132_tl_wr1043nd_v1.dts    |  6 ++--
>>  arch/mips/boot/dts/qca/ar9331_dpt_module.dts  |  4 +--
>>  .../mips/boot/dts/qca/ar9331_dragino_ms14.dts |  6 ++--
>>  arch/mips/boot/dts/qca/ar9331_omega.dts       |  4 +--
>>  .../qca/ar9331_openembed_som9331_board.dts    |  4 +--
>>  arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts   |  8 ++---
>>  10 files changed, 37 insertions(+), 57 deletions(-)
>>
>> diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts 
>> b/arch/mips/boot/dts/img/pistachio_marduk.dts
>> index a8708783f04b..a8da2f992b1a 100644
>> --- a/arch/mips/boot/dts/img/pistachio_marduk.dts
>> +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
>> @@ -59,12 +59,12 @@ led-1 {
>>
>>  	keys {
>>  		compatible = "gpio-keys";
>> -		button@1 {
>> +		button-1 {
>>  			label = "Button 1";
>>  			linux,code = <0x101>; /* BTN_1 */
>>  			gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
>>  		};
>> -		button@2 {
>> +		button-2 {
>>  			label = "Button 2";
>>  			linux,code = <0x102>; /* BTN_2 */
>>  			gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
>> diff --git a/arch/mips/boot/dts/ingenic/gcw0.dts 
>> b/arch/mips/boot/dts/ingenic/gcw0.dts
>> index 4abb0318416c..5d33f26fd28c 100644
>> --- a/arch/mips/boot/dts/ingenic/gcw0.dts
>> +++ b/arch/mips/boot/dts/ingenic/gcw0.dts
>> @@ -130,89 +130,86 @@ backlight: backlight {
>>
>>  	gpio-keys {
>>  		compatible = "gpio-keys";
>> -		#address-cells = <1>;
>> -		#size-cells = <0>;
> 
> Are you sure you can remove these?

Yes, from DT spec point of view, DT bindings and Linux implementation.
However this particular change was not tested, except building.

> 
> Looking at paragraph 2.3.5 of the DT spec, I would think they have to 
> stay (although with #address-cells = <0>).

The paragraph 2.3.5 says nothing about regular properties (which can be
also child nodes). It says about children of a bus, right? It's not
related here, it's not a bus.

Second, why exactly this one gpio-keys node is different than all other
gpio-keys everywhere and than bindings? Why this one has to be
incompatible/wrong according to bindings (which do not allow
address-cells and nodes with unit addresses)?


Best regards,
Krzysztof

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

* Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties
  2022-06-25 19:58   ` Krzysztof Kozlowski
@ 2022-06-25 20:15     ` Paul Cercueil
  2022-06-25 20:25       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Cercueil @ 2022-06-25 20:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski,
	Thomas Bogendoerfer, Matthias Brugger, Arınç ÜNAL,
	Sergio Paracuellos, linux-mips, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

Hi Krzysztof,

Le sam., juin 25 2022 at 21:58:08 +0200, Krzysztof Kozlowski 
<krzysztof.kozlowski@linaro.org> a écrit :
> On 24/06/2022 20:40, Paul Cercueil wrote:
>>  Hi Krzysztof,
>> 
>>  Le ven., juin 24 2022 at 19:07:39 +0200, Krzysztof Kozlowski
>>  <krzysztof.kozlowski@linaro.org> a écrit :
>>>  gpio-keys children do not use unit addresses.
>>> 
>>>  Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> 
>>>  ---
>>> 
>>>  See:
>>>  
>>> https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@linaro.org/
>>>  ---
>>>   arch/mips/boot/dts/img/pistachio_marduk.dts   |  4 +--
>>>   arch/mips/boot/dts/ingenic/gcw0.dts           | 31
>>>  +++++++++----------
>>>   arch/mips/boot/dts/ingenic/rs90.dts           | 18 +++++------
>>>   arch/mips/boot/dts/pic32/pic32mzda_sk.dts     |  9 ++----
>>>   .../boot/dts/qca/ar9132_tl_wr1043nd_v1.dts    |  6 ++--
>>>   arch/mips/boot/dts/qca/ar9331_dpt_module.dts  |  4 +--
>>>   .../mips/boot/dts/qca/ar9331_dragino_ms14.dts |  6 ++--
>>>   arch/mips/boot/dts/qca/ar9331_omega.dts       |  4 +--
>>>   .../qca/ar9331_openembed_som9331_board.dts    |  4 +--
>>>   arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts   |  8 ++---
>>>   10 files changed, 37 insertions(+), 57 deletions(-)
>>> 
>>>  diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>  b/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>  index a8708783f04b..a8da2f992b1a 100644
>>>  --- a/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>  +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>  @@ -59,12 +59,12 @@ led-1 {
>>> 
>>>   	keys {
>>>   		compatible = "gpio-keys";
>>>  -		button@1 {
>>>  +		button-1 {
>>>   			label = "Button 1";
>>>   			linux,code = <0x101>; /* BTN_1 */
>>>   			gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
>>>   		};
>>>  -		button@2 {
>>>  +		button-2 {
>>>   			label = "Button 2";
>>>   			linux,code = <0x102>; /* BTN_2 */
>>>   			gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
>>>  diff --git a/arch/mips/boot/dts/ingenic/gcw0.dts
>>>  b/arch/mips/boot/dts/ingenic/gcw0.dts
>>>  index 4abb0318416c..5d33f26fd28c 100644
>>>  --- a/arch/mips/boot/dts/ingenic/gcw0.dts
>>>  +++ b/arch/mips/boot/dts/ingenic/gcw0.dts
>>>  @@ -130,89 +130,86 @@ backlight: backlight {
>>> 
>>>   	gpio-keys {
>>>   		compatible = "gpio-keys";
>>>  -		#address-cells = <1>;
>>>  -		#size-cells = <0>;
>> 
>>  Are you sure you can remove these?
> 
> Yes, from DT spec point of view, DT bindings and Linux implementation.
> However this particular change was not tested, except building.
> 
>> 
>>  Looking at paragraph 2.3.5 of the DT spec, I would think they have 
>> to
>>  stay (although with #address-cells = <0>).
> 
> The paragraph 2.3.5 says nothing about regular properties (which can 
> be
> also child nodes). It says about children of a bus, right? It's not
> related here, it's not a bus.

I quote:
"A DTSpec-compliant boot program shall supply #address-cells and 
#size-cells on all nodes that have children."

The gpio-keys node has children nodes, therefore it should have 
#address-cells and #size-cells, there's no room for interpretation here.

> Second, why exactly this one gpio-keys node is different than all 
> other
> gpio-keys everywhere and than bindings? Why this one has to be
> incompatible/wrong according to bindings (which do not allow
> address-cells and nodes with unit addresses)?

Nothing is different. I'm just stating that your proposed fix is 
invalid if we want to enforce compliance with the DT spec.

Cheers,
-Paul
> 



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

* Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties
  2022-06-25 20:15     ` Paul Cercueil
@ 2022-06-25 20:25       ` Krzysztof Kozlowski
  2022-06-29 12:08         ` Arınç ÜNAL
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2022-06-25 20:25 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski,
	Thomas Bogendoerfer, Matthias Brugger, Arınç ÜNAL,
	Sergio Paracuellos, linux-mips, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

On 25/06/2022 22:15, Paul Cercueil wrote:
> Hi Krzysztof,
> 
> Le sam., juin 25 2022 at 21:58:08 +0200, Krzysztof Kozlowski 
> <krzysztof.kozlowski@linaro.org> a écrit :
>> On 24/06/2022 20:40, Paul Cercueil wrote:
>>>  Hi Krzysztof,
>>>
>>>  Le ven., juin 24 2022 at 19:07:39 +0200, Krzysztof Kozlowski
>>>  <krzysztof.kozlowski@linaro.org> a écrit :
>>>>  gpio-keys children do not use unit addresses.
>>>>
>>>>  Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>
>>>>  ---
>>>>
>>>>  See:
>>>>  
>>>> https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@linaro.org/
>>>>  ---
>>>>   arch/mips/boot/dts/img/pistachio_marduk.dts   |  4 +--
>>>>   arch/mips/boot/dts/ingenic/gcw0.dts           | 31
>>>>  +++++++++----------
>>>>   arch/mips/boot/dts/ingenic/rs90.dts           | 18 +++++------
>>>>   arch/mips/boot/dts/pic32/pic32mzda_sk.dts     |  9 ++----
>>>>   .../boot/dts/qca/ar9132_tl_wr1043nd_v1.dts    |  6 ++--
>>>>   arch/mips/boot/dts/qca/ar9331_dpt_module.dts  |  4 +--
>>>>   .../mips/boot/dts/qca/ar9331_dragino_ms14.dts |  6 ++--
>>>>   arch/mips/boot/dts/qca/ar9331_omega.dts       |  4 +--
>>>>   .../qca/ar9331_openembed_som9331_board.dts    |  4 +--
>>>>   arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts   |  8 ++---
>>>>   10 files changed, 37 insertions(+), 57 deletions(-)
>>>>
>>>>  diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>>  b/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>>  index a8708783f04b..a8da2f992b1a 100644
>>>>  --- a/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>>  +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>>  @@ -59,12 +59,12 @@ led-1 {
>>>>
>>>>   	keys {
>>>>   		compatible = "gpio-keys";
>>>>  -		button@1 {
>>>>  +		button-1 {
>>>>   			label = "Button 1";
>>>>   			linux,code = <0x101>; /* BTN_1 */
>>>>   			gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
>>>>   		};
>>>>  -		button@2 {
>>>>  +		button-2 {
>>>>   			label = "Button 2";
>>>>   			linux,code = <0x102>; /* BTN_2 */
>>>>   			gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
>>>>  diff --git a/arch/mips/boot/dts/ingenic/gcw0.dts
>>>>  b/arch/mips/boot/dts/ingenic/gcw0.dts
>>>>  index 4abb0318416c..5d33f26fd28c 100644
>>>>  --- a/arch/mips/boot/dts/ingenic/gcw0.dts
>>>>  +++ b/arch/mips/boot/dts/ingenic/gcw0.dts
>>>>  @@ -130,89 +130,86 @@ backlight: backlight {
>>>>
>>>>   	gpio-keys {
>>>>   		compatible = "gpio-keys";
>>>>  -		#address-cells = <1>;
>>>>  -		#size-cells = <0>;
>>>
>>>  Are you sure you can remove these?
>>
>> Yes, from DT spec point of view, DT bindings and Linux implementation.
>> However this particular change was not tested, except building.
>>
>>>
>>>  Looking at paragraph 2.3.5 of the DT spec, I would think they have 
>>> to
>>>  stay (although with #address-cells = <0>).
>>
>> The paragraph 2.3.5 says nothing about regular properties (which can 
>> be
>> also child nodes). It says about children of a bus, right? It's not
>> related here, it's not a bus.
> 
> I quote:
> "A DTSpec-compliant boot program shall supply #address-cells and 
> #size-cells on all nodes that have children."

And paragraph 2.2.3 says:
"A unit address may be omitted if the full path to the node is unambiguous."

You have address/size cells for nodes with children having unit
addresses. If they don't unit addresses, you don't add address/size
cells (with some exceptions).

The paragraph 2.3.5 mentions "child device nodes" and these properties
are not devices, although I agree that DT spec here is actually confusing.

> 
> The gpio-keys node has children nodes, therefore it should have 
> #address-cells and #size-cells, there's no room for interpretation here.
> 
>> Second, why exactly this one gpio-keys node is different than all 
>> other
>> gpio-keys everywhere and than bindings? Why this one has to be
>> incompatible/wrong according to bindings (which do not allow
>> address-cells and nodes with unit addresses)?
> 
> Nothing is different. I'm just stating that your proposed fix is 
> invalid if we want to enforce compliance with the DT spec.

In such case, we rather enforce the compliance with the bindings.

Best regards,
Krzysztof

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

* Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties
  2022-06-25 20:25       ` Krzysztof Kozlowski
@ 2022-06-29 12:08         ` Arınç ÜNAL
  0 siblings, 0 replies; 10+ messages in thread
From: Arınç ÜNAL @ 2022-06-29 12:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Paul Cercueil
  Cc: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski,
	Thomas Bogendoerfer, Matthias Brugger, Sergio Paracuellos,
	linux-mips, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek

On 25.06.2022 23:25, Krzysztof Kozlowski wrote:
> On 25/06/2022 22:15, Paul Cercueil wrote:
>> Hi Krzysztof,
>>
>> Le sam., juin 25 2022 at 21:58:08 +0200, Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> a écrit :
>>> On 24/06/2022 20:40, Paul Cercueil wrote:
>>>>   Hi Krzysztof,
>>>>
>>>>   Le ven., juin 24 2022 at 19:07:39 +0200, Krzysztof Kozlowski
>>>>   <krzysztof.kozlowski@linaro.org> a écrit :
>>>>>   gpio-keys children do not use unit addresses.
>>>>>
>>>>>   Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>
>>>>>   ---
>>>>>
>>>>>   See:
>>>>>   
>>>>> https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@linaro.org/
>>>>>   ---
>>>>>    arch/mips/boot/dts/img/pistachio_marduk.dts   |  4 +--
>>>>>    arch/mips/boot/dts/ingenic/gcw0.dts           | 31
>>>>>   +++++++++----------
>>>>>    arch/mips/boot/dts/ingenic/rs90.dts           | 18 +++++------
>>>>>    arch/mips/boot/dts/pic32/pic32mzda_sk.dts     |  9 ++----
>>>>>    .../boot/dts/qca/ar9132_tl_wr1043nd_v1.dts    |  6 ++--
>>>>>    arch/mips/boot/dts/qca/ar9331_dpt_module.dts  |  4 +--
>>>>>    .../mips/boot/dts/qca/ar9331_dragino_ms14.dts |  6 ++--
>>>>>    arch/mips/boot/dts/qca/ar9331_omega.dts       |  4 +--
>>>>>    .../qca/ar9331_openembed_som9331_board.dts    |  4 +--
>>>>>    arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts   |  8 ++---
>>>>>    10 files changed, 37 insertions(+), 57 deletions(-)
>>>>>
>>>>>   diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>>>   b/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>>>   index a8708783f04b..a8da2f992b1a 100644
>>>>>   --- a/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>>>   +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
>>>>>   @@ -59,12 +59,12 @@ led-1 {
>>>>>
>>>>>    	keys {
>>>>>    		compatible = "gpio-keys";
>>>>>   -		button@1 {
>>>>>   +		button-1 {
>>>>>    			label = "Button 1";
>>>>>    			linux,code = <0x101>; /* BTN_1 */
>>>>>    			gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
>>>>>    		};
>>>>>   -		button@2 {
>>>>>   +		button-2 {
>>>>>    			label = "Button 2";
>>>>>    			linux,code = <0x102>; /* BTN_2 */
>>>>>    			gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
>>>>>   diff --git a/arch/mips/boot/dts/ingenic/gcw0.dts
>>>>>   b/arch/mips/boot/dts/ingenic/gcw0.dts
>>>>>   index 4abb0318416c..5d33f26fd28c 100644
>>>>>   --- a/arch/mips/boot/dts/ingenic/gcw0.dts
>>>>>   +++ b/arch/mips/boot/dts/ingenic/gcw0.dts
>>>>>   @@ -130,89 +130,86 @@ backlight: backlight {
>>>>>
>>>>>    	gpio-keys {
>>>>>    		compatible = "gpio-keys";
>>>>>   -		#address-cells = <1>;
>>>>>   -		#size-cells = <0>;
>>>>
>>>>   Are you sure you can remove these?
>>>
>>> Yes, from DT spec point of view, DT bindings and Linux implementation.
>>> However this particular change was not tested, except building.
>>>
>>>>
>>>>   Looking at paragraph 2.3.5 of the DT spec, I would think they have
>>>> to
>>>>   stay (although with #address-cells = <0>).
>>>
>>> The paragraph 2.3.5 says nothing about regular properties (which can
>>> be
>>> also child nodes). It says about children of a bus, right? It's not
>>> related here, it's not a bus.
>>
>> I quote:
>> "A DTSpec-compliant boot program shall supply #address-cells and
>> #size-cells on all nodes that have children."
> 
> And paragraph 2.2.3 says:
> "A unit address may be omitted if the full path to the node is unambiguous."
> 
> You have address/size cells for nodes with children having unit
> addresses. If they don't unit addresses, you don't add address/size
> cells (with some exceptions).
> 
> The paragraph 2.3.5 mentions "child device nodes" and these properties
> are not devices, although I agree that DT spec here is actually confusing.
> 
>>
>> The gpio-keys node has children nodes, therefore it should have
>> #address-cells and #size-cells, there's no room for interpretation here.
>>
>>> Second, why exactly this one gpio-keys node is different than all
>>> other
>>> gpio-keys everywhere and than bindings? Why this one has to be
>>> incompatible/wrong according to bindings (which do not allow
>>> address-cells and nodes with unit addresses)?
>>
>> Nothing is different. I'm just stating that your proposed fix is
>> invalid if we want to enforce compliance with the DT spec.
> 
> In such case, we rather enforce the compliance with the bindings.
> 
> Best regards,
> Krzysztof

I recall them to be unnecessary as well. I have a patch of mine applied 
identical to this:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8c9f00d4b05134164e462f27b21c8295255ffa64

Also, I don't see any warnings with this patch applied:

$ ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- make clean dtbs -j$(nproc)
   SYNC    include/config/auto.conf.cmd
   HOSTCC  scripts/basic/fixdep
   HOSTCC  scripts/kconfig/conf.o
   HOSTCC  scripts/kconfig/confdata.o
   HOSTCC  scripts/kconfig/expr.o
   LEX     scripts/kconfig/lexer.lex.c
   YACC    scripts/kconfig/parser.tab.[ch]
   HOSTCC  scripts/kconfig/menu.o
   HOSTCC  scripts/kconfig/preprocess.o
   HOSTCC  scripts/kconfig/symbol.o
   HOSTCC  scripts/kconfig/util.o
   HOSTCC  scripts/kconfig/lexer.lex.o
   HOSTCC  scripts/kconfig/parser.tab.o
   HOSTLD  scripts/kconfig/conf
   HOSTCC  scripts/dtc/dtc.o
   HOSTCC  scripts/dtc/flattree.o
   HOSTCC  scripts/dtc/fstree.o
   HOSTCC  scripts/dtc/data.o
   HOSTCC  scripts/dtc/livetree.o
   HOSTCC  scripts/dtc/treesource.o
   HOSTCC  scripts/dtc/srcpos.o
   HOSTCC  scripts/dtc/checks.o
   HOSTCC  scripts/dtc/util.o
   LEX     scripts/dtc/dtc-lexer.lex.c
   YACC    scripts/dtc/dtc-parser.tab.[ch]
   HOSTCC  scripts/dtc/libfdt/fdt.o
   HOSTCC  scripts/dtc/libfdt/fdt_ro.o
   HOSTCC  scripts/dtc/libfdt/fdt_wip.o
   HOSTCC  scripts/dtc/libfdt/fdt_sw.o
   HOSTCC  scripts/dtc/libfdt/fdt_rw.o
   HOSTCC  scripts/dtc/libfdt/fdt_strerror.o
   HOSTCC  scripts/dtc/libfdt/fdt_empty_tree.o
   HOSTCC  scripts/dtc/libfdt/fdt_addresses.o
   HOSTCC  scripts/dtc/libfdt/fdt_overlay.o
   HOSTCC  scripts/dtc/fdtoverlay.o
   HOSTCC  scripts/dtc/dtc-lexer.lex.o
   HOSTCC  scripts/dtc/dtc-parser.tab.o
   HOSTLD  scripts/dtc/fdtoverlay
   HOSTLD  scripts/dtc/dtc
   DTC     arch/mips/boot/dts/ingenic/gcw0.dtb

Have my acked-by.

Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Arınç

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

* Re: [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema
  2022-06-24 17:07 ` [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema Krzysztof Kozlowski
@ 2022-06-29 12:12   ` Arınç ÜNAL
  2022-07-14  9:56   ` Thomas Bogendoerfer
  1 sibling, 0 replies; 10+ messages in thread
From: Arınç ÜNAL @ 2022-06-29 12:12 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rahul Bedarkar, Rob Herring,
	Krzysztof Kozlowski, Thomas Bogendoerfer, Paul Cercueil,
	Matthias Brugger, Sergio Paracuellos, linux-mips, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On 24.06.2022 20:07, Krzysztof Kozlowski wrote:
> The node names should be generic and DT schema expects certain pattern
> (e.g. with key/button/switch).
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Cheers.
Arınç

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

* Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties
  2022-06-24 17:07 [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties Krzysztof Kozlowski
  2022-06-24 17:07 ` [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema Krzysztof Kozlowski
  2022-06-24 18:40 ` [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties Paul Cercueil
@ 2022-07-14  9:55 ` Thomas Bogendoerfer
  2 siblings, 0 replies; 10+ messages in thread
From: Thomas Bogendoerfer @ 2022-07-14  9:55 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski, Paul Cercueil,
	Matthias Brugger, Arınç ÜNAL, Sergio Paracuellos,
	linux-mips, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek

On Fri, Jun 24, 2022 at 07:07:39PM +0200, Krzysztof Kozlowski wrote:
> gpio-keys children do not use unit addresses.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> See: https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@linaro.org/
> ---
>  arch/mips/boot/dts/img/pistachio_marduk.dts   |  4 +--
>  arch/mips/boot/dts/ingenic/gcw0.dts           | 31 +++++++++----------
>  arch/mips/boot/dts/ingenic/rs90.dts           | 18 +++++------
>  arch/mips/boot/dts/pic32/pic32mzda_sk.dts     |  9 ++----
>  .../boot/dts/qca/ar9132_tl_wr1043nd_v1.dts    |  6 ++--
>  arch/mips/boot/dts/qca/ar9331_dpt_module.dts  |  4 +--
>  .../mips/boot/dts/qca/ar9331_dragino_ms14.dts |  6 ++--
>  arch/mips/boot/dts/qca/ar9331_omega.dts       |  4 +--
>  .../qca/ar9331_openembed_som9331_board.dts    |  4 +--
>  arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts   |  8 ++---
>  10 files changed, 37 insertions(+), 57 deletions(-)

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema
  2022-06-24 17:07 ` [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema Krzysztof Kozlowski
  2022-06-29 12:12   ` Arınç ÜNAL
@ 2022-07-14  9:56   ` Thomas Bogendoerfer
  1 sibling, 0 replies; 10+ messages in thread
From: Thomas Bogendoerfer @ 2022-07-14  9:56 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rahul Bedarkar, Rob Herring, Krzysztof Kozlowski, Paul Cercueil,
	Matthias Brugger, Arınç ÜNAL, Sergio Paracuellos,
	linux-mips, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek

On Fri, Jun 24, 2022 at 07:07:40PM +0200, Krzysztof Kozlowski wrote:
> The node names should be generic and DT schema expects certain pattern
> (e.g. with key/button/switch).
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  arch/mips/boot/dts/ingenic/ci20.dts                        | 2 +-
>  arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts | 2 +-
>  arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc1.dts         | 2 +-
>  arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc2.dts         | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

end of thread, other threads:[~2022-07-14  9:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-24 17:07 [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties Krzysztof Kozlowski
2022-06-24 17:07 ` [PATCH 2/2] MIPS: dts: align gpio-key node names with dtschema Krzysztof Kozlowski
2022-06-29 12:12   ` Arınç ÜNAL
2022-07-14  9:56   ` Thomas Bogendoerfer
2022-06-24 18:40 ` [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties Paul Cercueil
2022-06-25 19:58   ` Krzysztof Kozlowski
2022-06-25 20:15     ` Paul Cercueil
2022-06-25 20:25       ` Krzysztof Kozlowski
2022-06-29 12:08         ` Arınç ÜNAL
2022-07-14  9:55 ` Thomas Bogendoerfer

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