On 03/02/2018 11:46 PM, Sean Wang wrote:

On Fri, 2018-03-02 at 09:40 -0600, Rob Herring wrote:

quoted

On Fri, Feb 23, 2018 at 06:16:33PM +0800, sean.wang at mediatek.com wrote:

quoted

From: Sean Wang <sean.wang@mediatek.com>

Since those LEDs are parts of PMIC MT6323, it is reasonable to merge
those LEDs node definition back into mt6323.dtsi. This way can improve
the reusability of those nodes among different boards with the same PMIC.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Cc: Lee Jones <redacted>
---
 arch/arm/boot/dts/mt6323.dtsi                 | 26 ++++++++++++++++++++++++-
 arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 28 ---------------------------
 2 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/arch/arm/boot/dts/mt6323.dtsi b/arch/arm/boot/dts/mt6323.dtsi
index 7c783d6..44c5642 100644
--- a/arch/arm/boot/dts/mt6323.dtsi
+++ b/arch/arm/boot/dts/mt6323.dtsi

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 MediaTek Inc.
+ * Copyright (c) 2017-2018 MediaTek Inc.
  * Author: John Crispin <john@phrozen.org>
  *	   Sean Wang <sean.wang@mediatek.com>
  * This program is free software; you can redistribute it and/or modify

@@ -237,5 +237,29 @@
 				regulator-enable-ramp-delay = <216>;
 			};
 		};
+
+		leds {
+			compatible = "mediatek,mt6323-led";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			led at 0 {
+				reg = <0>;
+				label = "mt6323:isink:green";

Changing the label potentially breaks userspace. And the fact that it is 
a green LED is very much board specific. So I think the old location was 
correct. You could put the parent node here and leave these child nodes 
in the board specific dts file.

yes, I really made a mistake on breaking userspace with a label change.

I will keep the parent node here and then leave these child nodes into
board specific dts files. thanks for your idea!

Sorry I didn't see your email when I wrote my first comment. Yes this sounds
like the proper way to do it.

Regards,
Matthias

quoted

quoted

+				default-state = "off";
+			};
+
+			led at 1 {
+				reg = <1>;
+				label = "mt6323:isink:red";
+				default-state = "off";
+			};
+
+			led at 2 {
+				reg = <2>;
+				label = "mt6323:isink:blue";
+				default-state = "off";
+			};
+		};
 	};
 };

diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
index 75e61c4..767b225 100644
--- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
+++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts

@@ -483,34 +483,6 @@
 	status = "okay";
 };
 
-&pwrap {
-	mt6323 {
-		mt6323led: led {
-			compatible = "mediatek,mt6323-led";
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			led at 0 {
-				reg = <0>;
-				label = "bpi-r2:isink:green";
-				default-state = "off";
-			};
-
-			led at 1 {
-				reg = <1>;
-				label = "bpi-r2:isink:red";
-				default-state = "off";
-			};
-
-			led at 2 {
-				reg = <2>;
-				label = "bpi-r2:isink:blue";
-				default-state = "off";
-			};
-		};
-	};
-};
-
 &spi0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&spi0_pins_a>;

-- 
2.7.4