[PATCH v3 0/9] TI-Nspire cleanups

[PATCH v3 0/9] TI-Nspire cleanups

[Andrew Davis]

Hello all,

This series is an extended version of the series started here[0].

We break out what was the first patch into one for DTS change and
one for code changes as suggested by Krzysztof. Those are now patches
2 and 8 of this series (I kept the ACKs, hope that is okay).

As also pointed out by Krzysztof syscon nodes need a specific
compatible, add that as patch 1.

While I was adding that, I noticed some other dtbs_check issues,
so while here fixed some of those up too (patches 3-6).

Thanks,
Andrew

[0] https://lore.kernel.org/lkml/20221026161302.5319-1-afd@ti.com/

Andrew Davis (9):
  dt-bindings: mfd: syscon: Add TI-Nspire misc registers compatible
  ARM: dts: nspire: Use syscon-reboot to handle restart
  ARM: dts: nspire: Fix cpu node to conform with DT binding
  ARM: dts: nspire: Fix sram node to conform with DT binding
  ARM: dts: nspire: Fix vbus_reg node to conform with DT binding
  ARM: dts: nspire: Fix uart node to conform with DT binding
  ARM: dts: nspire: Use MATRIX_KEY macro for linux,keymap
  ARM: nspire: Use syscon-reboot to handle restart
  ARM: nspire: Remove unused header file mmio.h

 .../devicetree/bindings/mfd/syscon.yaml       |   1 +
 arch/arm/boot/dts/nspire-clp.dts              | 128 ++++++++++++------
 arch/arm/boot/dts/nspire-cx.dts               |  92 +++++++++----
 arch/arm/boot/dts/nspire-tp.dts               | 126 +++++++++++------
 arch/arm/boot/dts/nspire.dtsi                 |  24 +++-
 arch/arm/mach-nspire/Kconfig                  |   2 +
 arch/arm/mach-nspire/mmio.h                   |  16 ---
 arch/arm/mach-nspire/nspire.c                 |  60 +++-----
 8 files changed, 284 insertions(+), 165 deletions(-)
 rewrite arch/arm/boot/dts/nspire-clp.dts (72%)
 rewrite arch/arm/boot/dts/nspire-tp.dts (72%)
 delete mode 100644 arch/arm/mach-nspire/mmio.h
 rewrite arch/arm/mach-nspire/nspire.c (61%)

-- 
2.37.3

[PATCH v3 1/9] dt-bindings: mfd: syscon: Add TI-Nspire misc registers compatible

[Andrew Davis]

The TI Nspire devices contain a set of registers with a seemingly
miscellaneous set of functionality. This area is known simply as the
"misc" region. As "syscon" nodes like this still need a specific
compatible, document "ti,nspire-misc" here.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 4e4baf53796d..37a00532ea79 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -69,6 +69,7 @@ properties:
               - samsung,exynos5433-sysreg
               - samsung,exynos850-sysreg
               - samsung,exynosautov9-sysreg
+              - ti,nspire-misc
 
           - const: syscon
 
-- 
2.37.3

[PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

[Andrew Davis]

Writing this bit can be handled by the syscon-reboot driver.
Add this node to DT.

Signed-off-by: Andrew Davis <afd@ti.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Tested-by: Fabian Vogt <fabian@ritter-vogt.de>
Reviewed-by: Fabian Vogt <fabian@ritter-vogt.de>
---
 arch/arm/boot/dts/nspire.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
index bb240e6a3a6f..48fbc9d533c3 100644
--- a/arch/arm/boot/dts/nspire.dtsi
+++ b/arch/arm/boot/dts/nspire.dtsi
@@ -172,7 +172,14 @@ rtc: rtc@90090000 {
 			};
 
 			misc: misc@900a0000 {
+				compatible = "ti,nspire-misc", "syscon", "simple-mfd";
 				reg = <0x900a0000 0x1000>;
+
+				reboot {
+					compatible = "syscon-reboot";
+					offset = <0x08>;
+					value = <0x02>;
+				};
 			};
 
 			pwr: pwr@900b0000 {
-- 
2.37.3

[PATCH v3 3/9] ARM: dts: nspire: Fix cpu node to conform with DT binding

[Andrew Davis]

This node does not follow the DT binding schema, correct this.
Should result in no functional change.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/boot/dts/nspire.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
index 48fbc9d533c3..cb7237051512 100644
--- a/arch/arm/boot/dts/nspire.dtsi
+++ b/arch/arm/boot/dts/nspire.dtsi
@@ -11,8 +11,13 @@ / {
 	interrupt-parent = <&intc>;
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		cpu@0 {
 			compatible = "arm,arm926ej-s";
+			device_type = "cpu";
+			reg = <0>;
 		};
 	};
 
-- 
2.37.3

[PATCH v3 4/9] ARM: dts: nspire: Fix sram node to conform with DT binding

[Andrew Davis]

This node does not follow the DT binding schema, correct this.
Should result in no functional change.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/boot/dts/nspire.dtsi | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
index cb7237051512..f979b28e2576 100644
--- a/arch/arm/boot/dts/nspire.dtsi
+++ b/arch/arm/boot/dts/nspire.dtsi
@@ -26,8 +26,15 @@ bootrom: bootrom@0 {
 	};
 
 	sram: sram@a4000000 {
-		device = "memory";
-		reg = <0xa4000000 0x20000>;
+		compatible = "mmio-sram";
+		reg = <0xa4000000 0x20000>; /* 128k */
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0xa4000000 0x20000>;
+
+		sram@0 {
+			reg = <0x0 0x20000>;
+		};
 	};
 
 	timer_clk: timer_clk {
-- 
2.37.3

[PATCH v3 5/9] ARM: dts: nspire: Fix vbus_reg node to conform with DT binding

[Andrew Davis]

This node does not follow the DT binding schema, correct this.
All "regulator-fixed" are voltage type, so drop "regulator-type".
Should result in no functional change.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/boot/dts/nspire.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
index f979b28e2576..9587e1ebeb93 100644
--- a/arch/arm/boot/dts/nspire.dtsi
+++ b/arch/arm/boot/dts/nspire.dtsi
@@ -71,7 +71,6 @@ vbus_reg: vbus_reg {
 		compatible = "regulator-fixed";
 
 		regulator-name = "USB VBUS output";
-		regulator-type = "voltage";
 
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
-- 
2.37.3

[PATCH v3 6/9] ARM: dts: nspire: Fix uart node to conform with DT binding

[Andrew Davis]

This node does not follow the DT binding schema, correct this.
The arm,pl011 binding requires the first clock to be named "uartclk".
Should result in no functional change.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/boot/dts/nspire-cx.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/nspire-cx.dts b/arch/arm/boot/dts/nspire-cx.dts
index 590b7dff6ae5..837dbdd9af55 100644
--- a/arch/arm/boot/dts/nspire-cx.dts
+++ b/arch/arm/boot/dts/nspire-cx.dts
@@ -24,7 +24,7 @@ &uart {
 	compatible = "arm,pl011", "arm,primecell";
 
 	clocks = <&uart_clk>, <&apb_pclk>;
-	clock-names = "uart_clk", "apb_pclk";
+	clock-names = "uartclk", "apb_pclk";
 };
 
 &timer0 {
-- 
2.37.3

[PATCH v3 7/9] ARM: dts: nspire: Use MATRIX_KEY macro for linux,keymap

[Andrew Davis]

This looks better and allows us to see the row and column numbers
more easily. Switch to this macro here.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/boot/dts/nspire-clp.dts | 128 +++++++++++++++++++++----------
 arch/arm/boot/dts/nspire-cx.dts  |  90 ++++++++++++++++------
 arch/arm/boot/dts/nspire-tp.dts  | 126 ++++++++++++++++++++----------
 3 files changed, 241 insertions(+), 103 deletions(-)
 rewrite arch/arm/boot/dts/nspire-clp.dts (72%)
 rewrite arch/arm/boot/dts/nspire-tp.dts (72%)

diff --git a/arch/arm/boot/dts/nspire-clp.dts b/arch/arm/boot/dts/nspire-clp.dts
dissimilarity index 72%
index f52f38c61588..916ede0c2499 100644
--- a/arch/arm/boot/dts/nspire-clp.dts
+++ b/arch/arm/boot/dts/nspire-clp.dts
@@ -1,41 +1,87 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- *  linux/arch/arm/boot/nspire-clp.dts
- *
- *  Copyright (C) 2013 Daniel Tang <tangrs@tangrs.id.au>
- */
-/dts-v1/;
-
-/include/ "nspire-classic.dtsi"
-
-&keypad {
-	linux,keymap = <
-	0x0000001c 	0x0001001c 	0x00020039
-	0x0004002c 	0x00050034 	0x00060015
-	0x0007000b 	0x0008002d 	0x01000033
-	0x0101004e 	0x01020011 	0x01030004
-	0x0104002f 	0x01050003 	0x01060016
-	0x01070002 	0x01080014 	0x02000062
-	0x0201000c 	0x0202001f 	0x02030007
-	0x02040013 	0x02050006 	0x02060010
-	0x02070005 	0x02080019 	0x03000027
-	0x03010037 	0x03020018 	0x0303000a
-	0x03040031 	0x03050009 	0x03060032
-	0x03070008 	0x03080026 	0x04000028
-	0x04010035 	0x04020025 	0x04040024
-	0x04060017 	0x04080023 	0x05000028
-	0x05020022 	0x0503001b 	0x05040021
-	0x0505001a 	0x05060012 	0x0507006f
-	0x05080020 	0x0509002a 	0x0601001c
-	0x0602002e 	0x06030068 	0x06040030
-	0x0605006d 	0x0606001e 	0x06070001
-	0x0608002b 	0x0609000f 	0x07000067
-	0x0702006a 	0x0704006c 	0x07060069
-	0x0707000e 	0x0708001d 	0x070a000d
-	>;
-};
-
-/ {
-	model = "TI-NSPIRE Clickpad";
-	compatible = "ti,nspire-clp";
-};
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  linux/arch/arm/boot/nspire-clp.dts
+ *
+ *  Copyright (C) 2013 Daniel Tang <tangrs@tangrs.id.au>
+ */
+/dts-v1/;
+
+#include <dt-bindings/input/input.h>
+
+/include/ "nspire-classic.dtsi"
+
+&keypad {
+	linux,keymap = <
+		MATRIX_KEY(0,  0, 0x1c)
+		MATRIX_KEY(0,  1, 0x1c)
+		MATRIX_KEY(0,  2, 0x39)
+		MATRIX_KEY(0,  4, 0x2c)
+		MATRIX_KEY(0,  5, 0x34)
+		MATRIX_KEY(0,  6, 0x15)
+		MATRIX_KEY(0,  7, 0x0b)
+		MATRIX_KEY(0,  8, 0x2d)
+		MATRIX_KEY(1,  0, 0x33)
+		MATRIX_KEY(1,  1, 0x4e)
+		MATRIX_KEY(1,  2, 0x11)
+		MATRIX_KEY(1,  3, 0x04)
+		MATRIX_KEY(1,  4, 0x2f)
+		MATRIX_KEY(1,  5, 0x03)
+		MATRIX_KEY(1,  6, 0x16)
+		MATRIX_KEY(1,  7, 0x02)
+		MATRIX_KEY(1,  8, 0x14)
+		MATRIX_KEY(2,  0, 0x62)
+		MATRIX_KEY(2,  1, 0x0c)
+		MATRIX_KEY(2,  2, 0x1f)
+		MATRIX_KEY(2,  3, 0x07)
+		MATRIX_KEY(2,  4, 0x13)
+		MATRIX_KEY(2,  5, 0x06)
+		MATRIX_KEY(2,  6, 0x10)
+		MATRIX_KEY(2,  7, 0x05)
+		MATRIX_KEY(2,  8, 0x19)
+		MATRIX_KEY(3,  0, 0x27)
+		MATRIX_KEY(3,  1, 0x37)
+		MATRIX_KEY(3,  2, 0x18)
+		MATRIX_KEY(3,  3, 0x0a)
+		MATRIX_KEY(3,  4, 0x31)
+		MATRIX_KEY(3,  5, 0x09)
+		MATRIX_KEY(3,  6, 0x32)
+		MATRIX_KEY(3,  7, 0x08)
+		MATRIX_KEY(3,  8, 0x26)
+		MATRIX_KEY(4,  0, 0x28)
+		MATRIX_KEY(4,  1, 0x35)
+		MATRIX_KEY(4,  2, 0x25)
+		MATRIX_KEY(4,  4, 0x24)
+		MATRIX_KEY(4,  6, 0x17)
+		MATRIX_KEY(4,  8, 0x23)
+		MATRIX_KEY(5,  0, 0x28)
+		MATRIX_KEY(5,  2, 0x22)
+		MATRIX_KEY(5,  3, 0x1b)
+		MATRIX_KEY(5,  4, 0x21)
+		MATRIX_KEY(5,  5, 0x1a)
+		MATRIX_KEY(5,  6, 0x12)
+		MATRIX_KEY(5,  7, 0x6f)
+		MATRIX_KEY(5,  8, 0x20)
+		MATRIX_KEY(5,  9, 0x2a)
+		MATRIX_KEY(6,  1, 0x1c)
+		MATRIX_KEY(6,  2, 0x2e)
+		MATRIX_KEY(6,  3, 0x68)
+		MATRIX_KEY(6,  4, 0x30)
+		MATRIX_KEY(6,  5, 0x6d)
+		MATRIX_KEY(6,  6, 0x1e)
+		MATRIX_KEY(6,  7, 0x01)
+		MATRIX_KEY(6,  8, 0x2b)
+		MATRIX_KEY(6,  9, 0x0f)
+		MATRIX_KEY(7,  0, 0x67)
+		MATRIX_KEY(7,  2, 0x6a)
+		MATRIX_KEY(7,  4, 0x6c)
+		MATRIX_KEY(7,  6, 0x69)
+		MATRIX_KEY(7,  7, 0x0e)
+		MATRIX_KEY(7,  8, 0x1d)
+		MATRIX_KEY(7, 10, 0x0d)
+	>;
+};
+
+/ {
+	model = "TI-NSPIRE Clickpad";
+	compatible = "ti,nspire-clp";
+};
diff --git a/arch/arm/boot/dts/nspire-cx.dts b/arch/arm/boot/dts/nspire-cx.dts
index 837dbdd9af55..96c48fc52203 100644
--- a/arch/arm/boot/dts/nspire-cx.dts
+++ b/arch/arm/boot/dts/nspire-cx.dts
@@ -6,6 +6,8 @@
  */
 /dts-v1/;
 
+#include <dt-bindings/input/input.h>
+
 /include/ "nspire.dtsi"
 
 &lcd {
@@ -45,28 +47,72 @@ &ahb_clk {
 
 &keypad {
 	linux,keymap = <
-	0x0000001c 	0x0001001c 	0x00040039
-	0x0005002c 	0x00060015 	0x0007000b
-	0x0008000f 	0x0100002d 	0x01010011
-	0x0102002f 	0x01030004 	0x01040016
-	0x01050014 	0x0106001f 	0x01070002
-	0x010a006a 	0x02000013 	0x02010010
-	0x02020019 	0x02030007 	0x02040018
-	0x02050031 	0x02060032 	0x02070005
-	0x02080028 	0x0209006c 	0x03000026
-	0x03010025 	0x03020024 	0x0303000a
-	0x03040017 	0x03050023 	0x03060022
-	0x03070008 	0x03080035 	0x03090069
-	0x04000021 	0x04010012 	0x04020020
-	0x0404002e 	0x04050030 	0x0406001e
-	0x0407000d 	0x04080037 	0x04090067
-	0x05010038 	0x0502000c 	0x0503001b
-	0x05040034 	0x0505001a 	0x05060006
-	0x05080027 	0x0509000e 	0x050a006f
-	0x0600002b 	0x0602004e 	0x06030068
-	0x06040003 	0x0605006d 	0x06060009
-	0x06070001 	0x0609000f 	0x0708002a
-	0x0709001d 	0x070a0033 	>;
+		MATRIX_KEY(0,  0, 0x1c)
+		MATRIX_KEY(0,  1, 0x1c)
+		MATRIX_KEY(0,  4, 0x39)
+		MATRIX_KEY(0,  5, 0x2c)
+		MATRIX_KEY(0,  6, 0x15)
+		MATRIX_KEY(0,  7, 0x0b)
+		MATRIX_KEY(0,  8, 0x0f)
+		MATRIX_KEY(1,  0, 0x2d)
+		MATRIX_KEY(1,  1, 0x11)
+		MATRIX_KEY(1,  2, 0x2f)
+		MATRIX_KEY(1,  3, 0x04)
+		MATRIX_KEY(1,  4, 0x16)
+		MATRIX_KEY(1,  5, 0x14)
+		MATRIX_KEY(1,  6, 0x1f)
+		MATRIX_KEY(1,  7, 0x02)
+		MATRIX_KEY(1, 10, 0x6a)
+		MATRIX_KEY(2,  0, 0x13)
+		MATRIX_KEY(2,  1, 0x10)
+		MATRIX_KEY(2,  2, 0x19)
+		MATRIX_KEY(2,  3, 0x07)
+		MATRIX_KEY(2,  4, 0x18)
+		MATRIX_KEY(2,  5, 0x31)
+		MATRIX_KEY(2,  6, 0x32)
+		MATRIX_KEY(2,  7, 0x05)
+		MATRIX_KEY(2,  8, 0x28)
+		MATRIX_KEY(2,  9, 0x6c)
+		MATRIX_KEY(3,  0, 0x26)
+		MATRIX_KEY(3,  1, 0x25)
+		MATRIX_KEY(3,  2, 0x24)
+		MATRIX_KEY(3,  3, 0x0a)
+		MATRIX_KEY(3,  4, 0x17)
+		MATRIX_KEY(3,  5, 0x23)
+		MATRIX_KEY(3,  6, 0x22)
+		MATRIX_KEY(3,  7, 0x08)
+		MATRIX_KEY(3,  8, 0x35)
+		MATRIX_KEY(3,  9, 0x69)
+		MATRIX_KEY(4,  0, 0x21)
+		MATRIX_KEY(4,  1, 0x12)
+		MATRIX_KEY(4,  2, 0x20)
+		MATRIX_KEY(4,  4, 0x2e)
+		MATRIX_KEY(4,  5, 0x30)
+		MATRIX_KEY(4,  6, 0x1e)
+		MATRIX_KEY(4,  7, 0x0d)
+		MATRIX_KEY(4,  8, 0x37)
+		MATRIX_KEY(4,  9, 0x67)
+		MATRIX_KEY(5,  1, 0x38)
+		MATRIX_KEY(5,  2, 0x0c)
+		MATRIX_KEY(5,  3, 0x1b)
+		MATRIX_KEY(5,  4, 0x34)
+		MATRIX_KEY(5,  5, 0x1a)
+		MATRIX_KEY(5,  6, 0x06)
+		MATRIX_KEY(5,  8, 0x27)
+		MATRIX_KEY(5,  9, 0x0e)
+		MATRIX_KEY(5, 10, 0x6f)
+		MATRIX_KEY(6,  0, 0x2b)
+		MATRIX_KEY(6,  2, 0x4e)
+		MATRIX_KEY(6,  3, 0x68)
+		MATRIX_KEY(6,  4, 0x03)
+		MATRIX_KEY(6,  5, 0x6d)
+		MATRIX_KEY(6,  6, 0x09)
+		MATRIX_KEY(6,  7, 0x01)
+		MATRIX_KEY(6,  9, 0x0f)
+		MATRIX_KEY(7,  8, 0x2a)
+		MATRIX_KEY(7,  9, 0x1d)
+		MATRIX_KEY(7, 10, 0x33)
+	>;
 };
 
 &vbus_reg {
diff --git a/arch/arm/boot/dts/nspire-tp.dts b/arch/arm/boot/dts/nspire-tp.dts
dissimilarity index 72%
index f7d0faacd4cc..235cd4264da7 100644
--- a/arch/arm/boot/dts/nspire-tp.dts
+++ b/arch/arm/boot/dts/nspire-tp.dts
@@ -1,40 +1,86 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- *  linux/arch/arm/boot/nspire-tp.dts
- *
- *  Copyright (C) 2013 Daniel Tang <tangrs@tangrs.id.au>
- */
-/dts-v1/;
-
-/include/ "nspire-classic.dtsi"
-
-&keypad {
-	linux,keymap = <
-	0x0000001c 	0x0001001c 	0x00040039
-	0x0005002c 	0x00060015 	0x0007000b
-	0x0008000f 	0x0100002d 	0x01010011
-	0x0102002f 	0x01030004 	0x01040016
-	0x01050014 	0x0106001f 	0x01070002
-	0x010a006a 	0x02000013 	0x02010010
-	0x02020019 	0x02030007 	0x02040018
-	0x02050031 	0x02060032 	0x02070005
-	0x02080028 	0x0209006c 	0x03000026
-	0x03010025 	0x03020024 	0x0303000a
-	0x03040017 	0x03050023 	0x03060022
-	0x03070008 	0x03080035 	0x03090069
-	0x04000021 	0x04010012 	0x04020020
-	0x0404002e 	0x04050030 	0x0406001e
-	0x0407000d 	0x04080037 	0x04090067
-	0x05010038 	0x0502000c 	0x0503001b
-	0x05040034 	0x0505001a 	0x05060006
-	0x05080027 	0x0509000e 	0x050a006f
-	0x0600002b 	0x0602004e 	0x06030068
-	0x06040003 	0x0605006d 	0x06060009
-	0x06070001 	0x0609000f 	0x0708002a
-	0x0709001d 	0x070a0033 	>;
-};
-
-/ {
-	model = "TI-NSPIRE Touchpad";
-	compatible = "ti,nspire-tp";
-};
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  linux/arch/arm/boot/nspire-tp.dts
+ *
+ *  Copyright (C) 2013 Daniel Tang <tangrs@tangrs.id.au>
+ */
+/dts-v1/;
+
+#include <dt-bindings/input/input.h>
+
+/include/ "nspire-classic.dtsi"
+
+&keypad {
+	linux,keymap = <
+		MATRIX_KEY(0,  0, 0x1c)
+		MATRIX_KEY(0,  1, 0x1c)
+		MATRIX_KEY(0,  4, 0x39)
+		MATRIX_KEY(0,  5, 0x2c)
+		MATRIX_KEY(0,  6, 0x15)
+		MATRIX_KEY(0,  7, 0x0b)
+		MATRIX_KEY(0,  8, 0x0f)
+		MATRIX_KEY(1,  0, 0x2d)
+		MATRIX_KEY(1,  1, 0x11)
+		MATRIX_KEY(1,  2, 0x2f)
+		MATRIX_KEY(1,  3, 0x04)
+		MATRIX_KEY(1,  4, 0x16)
+		MATRIX_KEY(1,  5, 0x14)
+		MATRIX_KEY(1,  6, 0x1f)
+		MATRIX_KEY(1,  7, 0x02)
+		MATRIX_KEY(1, 10, 0x6a)
+		MATRIX_KEY(2,  0, 0x13)
+		MATRIX_KEY(2,  1, 0x10)
+		MATRIX_KEY(2,  2, 0x19)
+		MATRIX_KEY(2,  3, 0x07)
+		MATRIX_KEY(2,  4, 0x18)
+		MATRIX_KEY(2,  5, 0x31)
+		MATRIX_KEY(2,  6, 0x32)
+		MATRIX_KEY(2,  7, 0x05)
+		MATRIX_KEY(2,  8, 0x28)
+		MATRIX_KEY(2,  9, 0x6c)
+		MATRIX_KEY(3,  0, 0x26)
+		MATRIX_KEY(3,  1, 0x25)
+		MATRIX_KEY(3,  2, 0x24)
+		MATRIX_KEY(3,  3, 0x0a)
+		MATRIX_KEY(3,  4, 0x17)
+		MATRIX_KEY(3,  5, 0x23)
+		MATRIX_KEY(3,  6, 0x22)
+		MATRIX_KEY(3,  7, 0x08)
+		MATRIX_KEY(3,  8, 0x35)
+		MATRIX_KEY(3,  9, 0x69)
+		MATRIX_KEY(4,  0, 0x21)
+		MATRIX_KEY(4,  1, 0x12)
+		MATRIX_KEY(4,  2, 0x20)
+		MATRIX_KEY(4,  4, 0x2e)
+		MATRIX_KEY(4,  5, 0x30)
+		MATRIX_KEY(4,  6, 0x1e)
+		MATRIX_KEY(4,  7, 0x0d)
+		MATRIX_KEY(4,  8, 0x37)
+		MATRIX_KEY(4,  9, 0x67)
+		MATRIX_KEY(5,  1, 0x38)
+		MATRIX_KEY(5,  2, 0x0c)
+		MATRIX_KEY(5,  3, 0x1b)
+		MATRIX_KEY(5,  4, 0x34)
+		MATRIX_KEY(5,  5, 0x1a)
+		MATRIX_KEY(5,  6, 0x06)
+		MATRIX_KEY(5,  8, 0x27)
+		MATRIX_KEY(5,  9, 0x0e)
+		MATRIX_KEY(5, 10, 0x6f)
+		MATRIX_KEY(6,  0, 0x2b)
+		MATRIX_KEY(6,  2, 0x4e)
+		MATRIX_KEY(6,  3, 0x68)
+		MATRIX_KEY(6,  4, 0x03)
+		MATRIX_KEY(6,  5, 0x6d)
+		MATRIX_KEY(6,  6, 0x09)
+		MATRIX_KEY(6,  7, 0x01)
+		MATRIX_KEY(6,  9, 0x0f)
+		MATRIX_KEY(7,  8, 0x2a)
+		MATRIX_KEY(7,  9, 0x1d)
+		MATRIX_KEY(7, 10, 0x33)
+	>;
+};
+
+/ {
+	model = "TI-NSPIRE Touchpad";
+	compatible = "ti,nspire-tp";
+};
-- 
2.37.3

[PATCH v3 8/9] ARM: nspire: Use syscon-reboot to handle restart

[Andrew Davis]

Writing this bit can be handled by the DT syscon-reboot driver.
Enable that driver and remove the machine_desc version.

Signed-off-by: Andrew Davis <afd@ti.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Tested-by: Fabian Vogt <fabian@ritter-vogt.de>
Reviewed-by: Fabian Vogt <fabian@ritter-vogt.de>
---
 arch/arm/mach-nspire/Kconfig  |  2 ++
 arch/arm/mach-nspire/mmio.h   |  3 ---
 arch/arm/mach-nspire/nspire.c | 10 ----------
 3 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-nspire/Kconfig b/arch/arm/mach-nspire/Kconfig
index b7a3871876d7..0ffdcaca1e6b 100644
--- a/arch/arm/mach-nspire/Kconfig
+++ b/arch/arm/mach-nspire/Kconfig
@@ -9,5 +9,7 @@ config ARCH_NSPIRE
 	select ARM_VIC
 	select ARM_TIMER_SP804
 	select NSPIRE_TIMER
+	select POWER_RESET
+	select POWER_RESET_SYSCON
 	help
 	  This enables support for systems using the TI-NSPIRE CPU
diff --git a/arch/arm/mach-nspire/mmio.h b/arch/arm/mach-nspire/mmio.h
index 48e32f13f311..2ce0656139ec 100644
--- a/arch/arm/mach-nspire/mmio.h
+++ b/arch/arm/mach-nspire/mmio.h
@@ -5,9 +5,6 @@
  *	Copyright (C) 2013 Daniel Tang <tangrs@tangrs.id.au>
  */
 
-#define NSPIRE_MISC_PHYS_BASE		0x900A0000
-#define NSPIRE_MISC_HWRESET		0x08
-
 #define NSPIRE_PWR_PHYS_BASE		0x900B0000
 #define NSPIRE_PWR_VIRT_BASE		0xFEEB0000
 #define NSPIRE_PWR_BUS_DISABLE1		0x18
diff --git a/arch/arm/mach-nspire/nspire.c b/arch/arm/mach-nspire/nspire.c
index 2d4abb0288b9..1e13337972dd 100644
--- a/arch/arm/mach-nspire/nspire.c
+++ b/arch/arm/mach-nspire/nspire.c
@@ -27,16 +27,6 @@ static const char *const nspire_dt_match[] __initconst = {
 	NULL,
 };
 
-static void nspire_restart(enum reboot_mode mode, const char *cmd)
-{
-	void __iomem *base = ioremap(NSPIRE_MISC_PHYS_BASE, SZ_4K);
-	if (!base)
-		return;
-
-	writel(2, base + NSPIRE_MISC_HWRESET);
-}
-
 DT_MACHINE_START(NSPIRE, "TI-NSPIRE")
 	.dt_compat	= nspire_dt_match,
-	.restart	= nspire_restart,
 MACHINE_END
-- 
2.37.3

[PATCH v3 9/9] ARM: nspire: Remove unused header file mmio.h

[Andrew Davis]

The Nspire boardfile platform drivers have all been converted. None
of the definitions in this header are used anymore. Remove it.

While here lets remove all the other unused headers and the file
name from in the file itself.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/mach-nspire/mmio.h   | 13 -------------
 arch/arm/mach-nspire/nspire.c | 14 --------------
 2 files changed, 27 deletions(-)
 delete mode 100644 arch/arm/mach-nspire/mmio.h

diff --git a/arch/arm/mach-nspire/mmio.h b/arch/arm/mach-nspire/mmio.h
deleted file mode 100644
index 2ce0656139ec..000000000000
--- a/arch/arm/mach-nspire/mmio.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- *	linux/arch/arm/mach-nspire/mmio.h
- *
- *	Copyright (C) 2013 Daniel Tang <tangrs@tangrs.id.au>
- */
-
-#define NSPIRE_PWR_PHYS_BASE		0x900B0000
-#define NSPIRE_PWR_VIRT_BASE		0xFEEB0000
-#define NSPIRE_PWR_BUS_DISABLE1		0x18
-#define NSPIRE_PWR_BUS_DISABLE2		0x20
-
-#define NSPIRE_LCD_PHYS_BASE		0xC0000000
diff --git a/arch/arm/mach-nspire/nspire.c b/arch/arm/mach-nspire/nspire.c
index 1e13337972dd..2fbfc23237ff 100644
--- a/arch/arm/mach-nspire/nspire.c
+++ b/arch/arm/mach-nspire/nspire.c
@@ -1,23 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- *	linux/arch/arm/mach-nspire/nspire.c
- *
  *	Copyright (C) 2013 Daniel Tang <tangrs@tangrs.id.au>
  */
-#include <linux/init.h>
-#include <linux/of_irq.h>
-#include <linux/of_address.h>
-#include <linux/of_platform.h>
-#include <linux/irqchip.h>
-#include <linux/irqchip/arm-vic.h>
-#include <linux/clkdev.h>
-#include <linux/amba/bus.h>
 
 #include <asm/mach/arch.h>
-#include <asm/mach-types.h>
-#include <asm/mach/map.h>
-
-#include "mmio.h"
 
 static const char *const nspire_dt_match[] __initconst = {
 	"ti,nspire",
-- 
2.37.3

Re: [PATCH v3 1/9] dt-bindings: mfd: syscon: Add TI-Nspire misc registers compatible

[Krzysztof Kozlowski]

On 27/10/2022 14:13, Andrew Davis wrote:
> The TI Nspire devices contain a set of registers with a seemingly
> miscellaneous set of functionality. This area is known simply as the
> "misc" region. As "syscon" nodes like this still need a specific
> compatible, document "ti,nspire-misc" here.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> index 4e4baf53796d..37a00532ea79 100644
> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> @@ -69,6 +69,7 @@ properties:
>                - samsung,exynos5433-sysreg
>                - samsung,exynos850-sysreg
>                - samsung,exynosautov9-sysreg
> +              - ti,nspire-misc

This is not what your DTS is saying...

Best regards,
Krzysztof

Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

[Krzysztof Kozlowski]

On 27/10/2022 14:13, Andrew Davis wrote:
> Writing this bit can be handled by the syscon-reboot driver.
> Add this node to DT.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Tested-by: Fabian Vogt <fabian@ritter-vogt.de>
> Reviewed-by: Fabian Vogt <fabian@ritter-vogt.de>
> ---
>  arch/arm/boot/dts/nspire.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
> index bb240e6a3a6f..48fbc9d533c3 100644
> --- a/arch/arm/boot/dts/nspire.dtsi
> +++ b/arch/arm/boot/dts/nspire.dtsi
> @@ -172,7 +172,14 @@ rtc: rtc@90090000 {
>  			};
>  
>  			misc: misc@900a0000 {
> +				compatible = "ti,nspire-misc", "syscon", "simple-mfd";

You have syscon and simple-mfd, but bindings in patch #1 say only syscon.


Best regards,
Krzysztof

Re: [PATCH v3 6/9] ARM: dts: nspire: Fix uart node to conform with DT binding

[Krzysztof Kozlowski]

On 27/10/2022 14:13, Andrew Davis wrote:
> This node does not follow the DT binding schema, correct this.
> The arm,pl011 binding requires the first clock to be named "uartclk".
> Should result in no functional change.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---


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

Best regards,
Krzysztof

Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

[Andrew Davis]

On 10/27/22 2:33 PM, Krzysztof Kozlowski wrote:
> On 27/10/2022 14:13, Andrew Davis wrote:
>> Writing this bit can be handled by the syscon-reboot driver.
>> Add this node to DT.
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> Tested-by: Fabian Vogt <fabian@ritter-vogt.de>
>> Reviewed-by: Fabian Vogt <fabian@ritter-vogt.de>
>> ---
>>   arch/arm/boot/dts/nspire.dtsi | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
>> index bb240e6a3a6f..48fbc9d533c3 100644
>> --- a/arch/arm/boot/dts/nspire.dtsi
>> +++ b/arch/arm/boot/dts/nspire.dtsi
>> @@ -172,7 +172,14 @@ rtc: rtc@90090000 {
>>   			};
>>   
>>   			misc: misc@900a0000 {
>> +				compatible = "ti,nspire-misc", "syscon", "simple-mfd";
> 
> You have syscon and simple-mfd, but bindings in patch #1 say only syscon.
> 

I'm not following, are you just saying my wording in the patch message just
wasn't complete?

Or are you saying something more about nodes that are both syscon and simple-mfd?
In that case, having both syscon and simple-mfd seems rather common, looks like
you added the rule for it[0].

Thinking on this, they almost represent the same thing. simple-mfd says "my child
nodes should be considered devices", why do we need that? Couldn't we simply state
that "syscon" node's children are always devices, I mean what else could they be,
syscon is an MFD after all (and lives in drivers/mfd/).

"syscon" often just says, others can use the registers within this node, so as a
different option, make "syscon" a property of "simple-mfd" nodes. I'm seeing all
these examples of devices that should have been children of the "syscon" device,
but instead use

regmap = <&x>;
syscon = <&x>;

or similar and put the device node out somewhere random. And in those cases,
wouldn't it have been more correct to use the normal "reg" and "regions" to
define the registers belonging to the child node/device?..

Thanks,
Andrew

[0] https://lore.kernel.org/all/20220817142246.828762-5-krzysztof.kozlowski@linaro.org/

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

[Krzysztof Kozlowski]

On 27/10/2022 17:07, Andrew Davis wrote:
> On 10/27/22 2:33 PM, Krzysztof Kozlowski wrote:
>> On 27/10/2022 14:13, Andrew Davis wrote:
>>> Writing this bit can be handled by the syscon-reboot driver.
>>> Add this node to DT.
>>>
>>> Signed-off-by: Andrew Davis <afd@ti.com>
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>> Tested-by: Fabian Vogt <fabian@ritter-vogt.de>
>>> Reviewed-by: Fabian Vogt <fabian@ritter-vogt.de>
>>> ---
>>>   arch/arm/boot/dts/nspire.dtsi | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
>>> index bb240e6a3a6f..48fbc9d533c3 100644
>>> --- a/arch/arm/boot/dts/nspire.dtsi
>>> +++ b/arch/arm/boot/dts/nspire.dtsi
>>> @@ -172,7 +172,14 @@ rtc: rtc@90090000 {
>>>   			};
>>>   
>>>   			misc: misc@900a0000 {
>>> +				compatible = "ti,nspire-misc", "syscon", "simple-mfd";
>>
>> You have syscon and simple-mfd, but bindings in patch #1 say only syscon.
>>
> 
> I'm not following, are you just saying my wording in the patch message just
> wasn't complete?

Your binding patch adds nspire compatible to the list of two items, so
you have two items in total - nspire followed by syscon.

What you implemented here is different.

> 
> Or are you saying something more about nodes that are both syscon and simple-mfd?
> In that case, having both syscon and simple-mfd seems rather common, looks like
> you added the rule for it[0].
> 
> Thinking on this, they almost represent the same thing. simple-mfd says "my child
> nodes should be considered devices", why do we need that? Couldn't we simply state
> that "syscon" node's children are always devices, I mean what else could they be,
> syscon is an MFD after all (and lives in drivers/mfd/).

No, syscon is not an MFD. Syscon means system controller and alone it
does not have children.

> 
> "syscon" often just says, others can use the registers within this node, so as a
> different option, make "syscon" a property of "simple-mfd" nodes. I'm seeing all
> these examples of devices that should have been children of the "syscon" device,
> but instead use
> 
> regmap = <&x>;
> syscon = <&x>;
> 
> or similar and put the device node out somewhere random. And in those cases,
> wouldn't it have been more correct to use the normal "reg" and "regions" to
> define the registers belonging to the child node/device?..

Sorry, I do not follow. How this is even related to your patch?

Your bindings say A, DTS say B. A != B. This needs fixing.

Unless you are asking me what your device is in general. This I don't
really know, but if you want to use it as regmap provider for system
registers and as a parent of syscon-based reboot device, then your
device is syscon and simple-mfd. With a specific compatible. Was this
your question?

Best regards,
Krzysztof

Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

[Andrew Davis]

On 10/27/22 4:27 PM, Krzysztof Kozlowski wrote:
> On 27/10/2022 17:07, Andrew Davis wrote:
>> On 10/27/22 2:33 PM, Krzysztof Kozlowski wrote:
>>> On 27/10/2022 14:13, Andrew Davis wrote:
>>>> Writing this bit can be handled by the syscon-reboot driver.
>>>> Add this node to DT.
>>>>
>>>> Signed-off-by: Andrew Davis <afd@ti.com>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Tested-by: Fabian Vogt <fabian@ritter-vogt.de>
>>>> Reviewed-by: Fabian Vogt <fabian@ritter-vogt.de>
>>>> ---
>>>>    arch/arm/boot/dts/nspire.dtsi | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
>>>> index bb240e6a3a6f..48fbc9d533c3 100644
>>>> --- a/arch/arm/boot/dts/nspire.dtsi
>>>> +++ b/arch/arm/boot/dts/nspire.dtsi
>>>> @@ -172,7 +172,14 @@ rtc: rtc@90090000 {
>>>>    			};
>>>>    
>>>>    			misc: misc@900a0000 {
>>>> +				compatible = "ti,nspire-misc", "syscon", "simple-mfd";
>>>
>>> You have syscon and simple-mfd, but bindings in patch #1 say only syscon.
>>>
>>
>> I'm not following, are you just saying my wording in the patch message just
>> wasn't complete?
> 
> Your binding patch adds nspire compatible to the list of two items, so
> you have two items in total - nspire followed by syscon.
> 
> What you implemented here is different.
> 

Is there a list of three items I can add this compatible? If instead you
mean I should go make a new binding, just say so :)

>>
>> Or are you saying something more about nodes that are both syscon and simple-mfd?
>> In that case, having both syscon and simple-mfd seems rather common, looks like
>> you added the rule for it[0].
>>
>> Thinking on this, they almost represent the same thing. simple-mfd says "my child
>> nodes should be considered devices", why do we need that? Couldn't we simply state
>> that "syscon" node's children are always devices, I mean what else could they be,
>> syscon is an MFD after all (and lives in drivers/mfd/).
> 
> No, syscon is not an MFD. Syscon means system controller and alone it
> does not have children.
> 

The binding lives in devicetree/bindings/*mfd*/, it is mentioned as one
in devicetree/bindings/mfd/mfd.txt. If it is not an MFD then the bindings
are giving out mixed signals here..

>>
>> "syscon" often just says, others can use the registers within this node, so as a
>> different option, make "syscon" a property of "simple-mfd" nodes. I'm seeing all
>> these examples of devices that should have been children of the "syscon" device,
>> but instead use
>>
>> regmap = <&x>;
>> syscon = <&x>;
>>
>> or similar and put the device node out somewhere random. And in those cases,
>> wouldn't it have been more correct to use the normal "reg" and "regions" to
>> define the registers belonging to the child node/device?..
> 
> Sorry, I do not follow. How this is even related to your patch?
> 
> Your bindings say A, DTS say B. A != B. This needs fixing.
> 

I said it was compatible with "syscon", not that it is incompatible
with "simple-mfd" devices.

What I've done here gives no dtbs_check warnings and
"devicetree/bindings/mfd/mfd.txt" explicitly allows what I am doing.
Unless we do not consider the old bindings valid? If so, would you
like me to convert mfd.txt to yaml, just let me know.

> Unless you are asking me what your device is in general. This I don't
> really know, but if you want to use it as regmap provider for system
> registers and as a parent of syscon-based reboot device, then your
> device is syscon and simple-mfd. With a specific compatible. Was this
> your question?
> 

Yes, I would like to use it as a regmap provider, my question here is
a much more general one: why do I need to specify that in device tree?
That is not a hardware description, my hardware is not "regmap" hardware.
This "syscon" stuff feels like a bodge to make the Linux drivers and bus
frameworks interact the way we want.

I know at this point this has little to do with this series, but I'd like
to just think this out for a moment. The latest Devicetree Specification
talks about "simple-bus" as a special compatible that communicates that
child nodes with compatible strings need probed also. ("simple-mfd" seems
to be used the same way but without needing a "ranges" property..)

Both of these are properties of a node, not something a device is "compatible"
with. "compatibles" are also supposed to be listed "from most specific to
most general", so which is more specific, "simple-mfd" or "syscon", etc..

Seems like Rob might agree[0], these are not really compatibles. We cant fix
history, but for new nodes, instead of growing the problem and forcing these to
be overloaded compatibles, we allow these to become new standard node properties.

For instance:

main_conf: syscon@43000000 {
	compatible = "ti,j721e-system-controller";
	reg = <0x0 0x43000000 0x0 0x20000>;

	simple-bus;
	syscon;

	...
};

Thoughts?

Thanks,
Andrew

[0] https://lore.kernel.org/all/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com/

> Best regards,
> Krzysztof
> 

Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

[Rob Herring]

On Mon, Oct 31, 2022 at 09:30:45AM -0500, Andrew Davis wrote:
> On 10/27/22 4:27 PM, Krzysztof Kozlowski wrote:
> > On 27/10/2022 17:07, Andrew Davis wrote:
> > > On 10/27/22 2:33 PM, Krzysztof Kozlowski wrote:
> > > > On 27/10/2022 14:13, Andrew Davis wrote:
> > > > > Writing this bit can be handled by the syscon-reboot driver.
> > > > > Add this node to DT.
> > > > > 
> > > > > Signed-off-by: Andrew Davis <afd@ti.com>
> > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > Tested-by: Fabian Vogt <fabian@ritter-vogt.de>
> > > > > Reviewed-by: Fabian Vogt <fabian@ritter-vogt.de>
> > > > > ---
> > > > >    arch/arm/boot/dts/nspire.dtsi | 7 +++++++
> > > > >    1 file changed, 7 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
> > > > > index bb240e6a3a6f..48fbc9d533c3 100644
> > > > > --- a/arch/arm/boot/dts/nspire.dtsi
> > > > > +++ b/arch/arm/boot/dts/nspire.dtsi
> > > > > @@ -172,7 +172,14 @@ rtc: rtc@90090000 {
> > > > >    			};
> > > > >    			misc: misc@900a0000 {
> > > > > +				compatible = "ti,nspire-misc", "syscon", "simple-mfd";
> > > > 
> > > > You have syscon and simple-mfd, but bindings in patch #1 say only syscon.
> > > > 
> > > 
> > > I'm not following, are you just saying my wording in the patch message just
> > > wasn't complete?
> > 
> > Your binding patch adds nspire compatible to the list of two items, so
> > you have two items in total - nspire followed by syscon.
> > 
> > What you implemented here is different.
> > 
> 
> Is there a list of three items I can add this compatible? If instead you
> mean I should go make a new binding, just say so :)

An MFD should define its own schema file.

The original intent of syscon.yaml was for just single nodes with 
'syscon' (and a specific compatible). Adding in simple-mfd was probably 
a mistake. Certainly we need to rework the schema as you should get a 
warning in this case.

> > > Or are you saying something more about nodes that are both syscon and simple-mfd?
> > > In that case, having both syscon and simple-mfd seems rather common, looks like
> > > you added the rule for it[0].
> > > 
> > > Thinking on this, they almost represent the same thing. simple-mfd says "my child
> > > nodes should be considered devices", why do we need that? Couldn't we simply state
> > > that "syscon" node's children are always devices, I mean what else could they be,
> > > syscon is an MFD after all (and lives in drivers/mfd/).
> > 
> > No, syscon is not an MFD. Syscon means system controller and alone it
> > does not have children.
> > 
> 
> The binding lives in devicetree/bindings/*mfd*/, it is mentioned as one
> in devicetree/bindings/mfd/mfd.txt. If it is not an MFD then the bindings
> are giving out mixed signals here..
> 
> > > 
> > > "syscon" often just says, others can use the registers within this node, so as a
> > > different option, make "syscon" a property of "simple-mfd" nodes. I'm seeing all
> > > these examples of devices that should have been children of the "syscon" device,
> > > but instead use
> > > 
> > > regmap = <&x>;
> > > syscon = <&x>;
> > > 
> > > or similar and put the device node out somewhere random. And in those cases,
> > > wouldn't it have been more correct to use the normal "reg" and "regions" to
> > > define the registers belonging to the child node/device?..
> > 
> > Sorry, I do not follow. How this is even related to your patch?
> > 
> > Your bindings say A, DTS say B. A != B. This needs fixing.
> > 
> 
> I said it was compatible with "syscon", not that it is incompatible
> with "simple-mfd" devices.
> 
> What I've done here gives no dtbs_check warnings and
> "devicetree/bindings/mfd/mfd.txt" explicitly allows what I am doing.
> Unless we do not consider the old bindings valid? 

Only that the example is not because it doesn't have a specific 
compatible.

What needs to be clarified is that MFDs must define all the child nodes 
whether they are 'simple' or not.

> If so, would you
> like me to convert mfd.txt to yaml, just let me know.

No, because I don't think there is anything to define as a schema.


> > Unless you are asking me what your device is in general. This I don't
> > really know, but if you want to use it as regmap provider for system
> > registers and as a parent of syscon-based reboot device, then your
> > device is syscon and simple-mfd. With a specific compatible. Was this
> > your question?
> > 
> 
> Yes, I would like to use it as a regmap provider, my question here is
> a much more general one: why do I need to specify that in device tree?
> That is not a hardware description, my hardware is not "regmap" hardware.
> This "syscon" stuff feels like a bodge to make the Linux drivers and bus
> frameworks interact the way we want.

Bingo! It's a hint for create a regmap. We could just have a compatible 
list in the kernel for compatibles needing a regmap. Maybe that list 
would be too long though. So call it h/w description for this h/w is 
referenced by other places.


> I know at this point this has little to do with this series, but I'd like
> to just think this out for a moment. The latest Devicetree Specification
> talks about "simple-bus" as a special compatible that communicates that
> child nodes with compatible strings need probed also. ("simple-mfd" seems
> to be used the same way but without needing a "ranges" property..)

Yes, both cases are saying there is no dependency or setup of the parent 
needs. If the child nodes depend on the regmap, then it's not a 
'simple-mfd' IMO. Therefore 'syscon' together with 'simple-mfd' is wrong 
unless it's other nodes that need the regmap. The schema can't really 
check that.

> Both of these are properties of a node, not something a device is "compatible"
> with. "compatibles" are also supposed to be listed "from most specific to
> most general", so which is more specific, "simple-mfd" or "syscon", etc..

I would say 'syscon' is more specific if I have to pick. It implies some 
registers exist. 'simple-mfd' should mean there are no parent resources 
(...the children depend on).

We've probably got enough of a mixture of the order, it wouldn't be 
worth the effort to try to enforce the order here.

> Seems like Rob might agree[0], these are not really compatibles. We cant fix
> history, but for new nodes, instead of growing the problem and forcing these to
> be overloaded compatibles, we allow these to become new standard node properties.
> 
> For instance:
> 
> main_conf: syscon@43000000 {
> 	compatible = "ti,j721e-system-controller";
> 	reg = <0x0 0x43000000 0x0 0x20000>;
> 
> 	simple-bus;
> 	syscon;

Umm, no. This ship already sailed and we don't need a 2nd way to do 
things.

Rob

Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

[Andrew Davis]

On 10/31/22 12:14 PM, Rob Herring wrote:
> On Mon, Oct 31, 2022 at 09:30:45AM -0500, Andrew Davis wrote:
>> On 10/27/22 4:27 PM, Krzysztof Kozlowski wrote:
>>> On 27/10/2022 17:07, Andrew Davis wrote:
>>>> On 10/27/22 2:33 PM, Krzysztof Kozlowski wrote:
>>>>> On 27/10/2022 14:13, Andrew Davis wrote:
>>>>>> Writing this bit can be handled by the syscon-reboot driver.
>>>>>> Add this node to DT.
>>>>>>
>>>>>> Signed-off-by: Andrew Davis <afd@ti.com>
>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>>> Tested-by: Fabian Vogt <fabian@ritter-vogt.de>
>>>>>> Reviewed-by: Fabian Vogt <fabian@ritter-vogt.de>
>>>>>> ---
>>>>>>     arch/arm/boot/dts/nspire.dtsi | 7 +++++++
>>>>>>     1 file changed, 7 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
>>>>>> index bb240e6a3a6f..48fbc9d533c3 100644
>>>>>> --- a/arch/arm/boot/dts/nspire.dtsi
>>>>>> +++ b/arch/arm/boot/dts/nspire.dtsi
>>>>>> @@ -172,7 +172,14 @@ rtc: rtc@90090000 {
>>>>>>     			};
>>>>>>     			misc: misc@900a0000 {
>>>>>> +				compatible = "ti,nspire-misc", "syscon", "simple-mfd";
>>>>>
>>>>> You have syscon and simple-mfd, but bindings in patch #1 say only syscon.
>>>>>
>>>>
>>>> I'm not following, are you just saying my wording in the patch message just
>>>> wasn't complete?
>>>
>>> Your binding patch adds nspire compatible to the list of two items, so
>>> you have two items in total - nspire followed by syscon.
>>>
>>> What you implemented here is different.
>>>
>>
>> Is there a list of three items I can add this compatible? If instead you
>> mean I should go make a new binding, just say so :)
> 
> An MFD should define its own schema file.
> 
> The original intent of syscon.yaml was for just single nodes with
> 'syscon' (and a specific compatible). Adding in simple-mfd was probably
> a mistake. Certainly we need to rework the schema as you should get a
> warning in this case.
> 
>>>> Or are you saying something more about nodes that are both syscon and simple-mfd?
>>>> In that case, having both syscon and simple-mfd seems rather common, looks like
>>>> you added the rule for it[0].
>>>>
>>>> Thinking on this, they almost represent the same thing. simple-mfd says "my child
>>>> nodes should be considered devices", why do we need that? Couldn't we simply state
>>>> that "syscon" node's children are always devices, I mean what else could they be,
>>>> syscon is an MFD after all (and lives in drivers/mfd/).
>>>
>>> No, syscon is not an MFD. Syscon means system controller and alone it
>>> does not have children.
>>>
>>
>> The binding lives in devicetree/bindings/*mfd*/, it is mentioned as one
>> in devicetree/bindings/mfd/mfd.txt. If it is not an MFD then the bindings
>> are giving out mixed signals here..
>>
>>>>
>>>> "syscon" often just says, others can use the registers within this node, so as a
>>>> different option, make "syscon" a property of "simple-mfd" nodes. I'm seeing all
>>>> these examples of devices that should have been children of the "syscon" device,
>>>> but instead use
>>>>
>>>> regmap = <&x>;
>>>> syscon = <&x>;
>>>>
>>>> or similar and put the device node out somewhere random. And in those cases,
>>>> wouldn't it have been more correct to use the normal "reg" and "regions" to
>>>> define the registers belonging to the child node/device?..
>>>
>>> Sorry, I do not follow. How this is even related to your patch?
>>>
>>> Your bindings say A, DTS say B. A != B. This needs fixing.
>>>
>>
>> I said it was compatible with "syscon", not that it is incompatible
>> with "simple-mfd" devices.
>>
>> What I've done here gives no dtbs_check warnings and
>> "devicetree/bindings/mfd/mfd.txt" explicitly allows what I am doing.
>> Unless we do not consider the old bindings valid?
> 
> Only that the example is not because it doesn't have a specific
> compatible.
> 
> What needs to be clarified is that MFDs must define all the child nodes
> whether they are 'simple' or not.
> 
>> If so, would you
>> like me to convert mfd.txt to yaml, just let me know.
> 
> No, because I don't think there is anything to define as a schema.
> 

It would allow for simple register regions to be 'simple-mfd' without
needing a whole new binding document for each. Same as we already have
with 'syscon.yaml'.

Making every simple MMIO space create a new binding document is not
reasonable. Neither is defining all nodes up front in that binding,
we don't expect that for top level nodes or 'simple-bus', why should
we for 'simple-mfd'?

My point with mfd.txt is that this *was allowed*, and there are already
a large number of users of the existing style.

> 
>>> Unless you are asking me what your device is in general. This I don't
>>> really know, but if you want to use it as regmap provider for system
>>> registers and as a parent of syscon-based reboot device, then your
>>> device is syscon and simple-mfd. With a specific compatible. Was this
>>> your question?
>>>
>>
>> Yes, I would like to use it as a regmap provider, my question here is
>> a much more general one: why do I need to specify that in device tree?
>> That is not a hardware description, my hardware is not "regmap" hardware.
>> This "syscon" stuff feels like a bodge to make the Linux drivers and bus
>> frameworks interact the way we want.
> 
> Bingo! It's a hint for create a regmap. We could just have a compatible
> list in the kernel for compatibles needing a regmap. Maybe that list
> would be too long though. So call it h/w description for this h/w is
> referenced by other places.
> 
> 
>> I know at this point this has little to do with this series, but I'd like
>> to just think this out for a moment. The latest Devicetree Specification
>> talks about "simple-bus" as a special compatible that communicates that
>> child nodes with compatible strings need probed also. ("simple-mfd" seems
>> to be used the same way but without needing a "ranges" property..)
> 
> Yes, both cases are saying there is no dependency or setup of the parent
> needs. If the child nodes depend on the regmap, then it's not a
> 'simple-mfd' IMO. Therefore 'syscon' together with 'simple-mfd' is wrong
> unless it's other nodes that need the regmap. The schema can't really
> check that.
> 

'syscon' also provides for reusing the same single register by multiple
users, such as bit-mapped registers. It also allows re-using the existing
simple syscon device compatibles. Again because people do not like writing
bindings for simple nodes.

Andrew

>> Both of these are properties of a node, not something a device is "compatible"
>> with. "compatibles" are also supposed to be listed "from most specific to
>> most general", so which is more specific, "simple-mfd" or "syscon", etc..
> 
> I would say 'syscon' is more specific if I have to pick. It implies some
> registers exist. 'simple-mfd' should mean there are no parent resources
> (...the children depend on).
> 
> We've probably got enough of a mixture of the order, it wouldn't be
> worth the effort to try to enforce the order here.
> 
>> Seems like Rob might agree[0], these are not really compatibles. We cant fix
>> history, but for new nodes, instead of growing the problem and forcing these to
>> be overloaded compatibles, we allow these to become new standard node properties.
>>
>> For instance:
>>
>> main_conf: syscon@43000000 {
>> 	compatible = "ti,j721e-system-controller";
>> 	reg = <0x0 0x43000000 0x0 0x20000>;
>>
>> 	simple-bus;
>> 	syscon;
> 
> Umm, no. This ship already sailed and we don't need a 2nd way to do
> things.
> 
> Rob