linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml
@ 2021-09-07 11:32 Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 1/8] ARM: dts: omap: Fixup GPMC child nodes Roger Quadros
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

Hi,

This series converts ti,gpmc memory controller and ti,gpmc-nand and
ti,gpmc-onenand MTD controller bindings to yaml.

cheers,
-roger

Changelog:
v3:
- fix indentation
- split GPMC child timings/settings into ti,gpmc-child.yaml
This allows us to refer to it at 3 places and avoid use of
'additionalProperties: true' at 2 places.
- specify defaults where applicable
- reordered patches
- added patch for making "gpmc,device-width" optional with defaults.
- address all review comments.

v2:
- Fix all errors in dtbs_check and dt_bindings_check
- remove references to gpmc-omap.txt
- Convert ti,gpmc-nand and ti,gpmc-onenand bindings to yaml as well

Roger Quadros (8):
  ARM: dts: omap: Fixup GPMC child nodes
  dt-bindings: mtd: Remove gpmc-nor.txt
  dt-bindings: net: Remove gpmc-eth.txt
  dt-bindings: memory-controllers: Introduce ti,gpmc-child
  dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  dt-bindings: mtd: ti,gpmc-onenand: Convert to yaml
  dt-bindings: memory-controllers: ti,gpmc: Convert to yaml
  memory: gpmc-omap: "gpmc,device-width" DT property is optional

 .../bindings/memory-controllers/omap-gpmc.txt | 157 -----------
 .../memory-controllers/ti,gpmc-child.yaml     | 245 ++++++++++++++++++
 .../bindings/memory-controllers/ti,gpmc.yaml  | 174 +++++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt     | 147 -----------
 .../devicetree/bindings/mtd/gpmc-nor.txt      |  98 -------
 .../devicetree/bindings/mtd/gpmc-onenand.txt  |  48 ----
 .../devicetree/bindings/mtd/ti,gpmc-nand.yaml | 110 ++++++++
 .../bindings/mtd/ti,gpmc-onenand.yaml         |  69 +++++
 .../devicetree/bindings/net/gpmc-eth.txt      |  97 -------
 .../boot/dts/logicpd-som-lv-baseboard.dtsi    |   2 +-
 .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |   2 +-
 .../boot/dts/logicpd-torpedo-baseboard.dtsi   |   2 +-
 arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi     |  62 +++--
 arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi     |  59 ++---
 arch/arm/boot/dts/omap-zoom-common.dtsi       |  16 +-
 arch/arm/boot/dts/omap2430-sdp.dts            |   6 +-
 arch/arm/boot/dts/omap3-cm-t3x30.dtsi         |   6 +-
 .../arm/boot/dts/omap3-devkit8000-common.dtsi |   4 +-
 arch/arm/boot/dts/omap3-evm-37xx.dts          |   1 +
 arch/arm/boot/dts/omap3-evm-common.dtsi       |   9 -
 .../boot/dts/omap3-evm-processor-common.dtsi  |   5 +-
 arch/arm/boot/dts/omap3-evm.dts               |   1 +
 arch/arm/boot/dts/omap3-igep0020-common.dtsi  |   5 +-
 arch/arm/boot/dts/omap3-ldp.dts               |   5 +-
 arch/arm/boot/dts/omap3-n900.dts              |   2 +-
 .../dts/omap3-overo-chestnut43-common.dtsi    |   6 +-
 .../arm/boot/dts/omap3-overo-tobi-common.dtsi |   6 +-
 .../boot/dts/omap3-overo-tobiduo-common.dtsi  |   8 +-
 arch/arm/boot/dts/omap3-sb-t35.dtsi           |   4 +-
 arch/arm/boot/dts/omap4-duovero-parlor.dts    |   6 +-
 drivers/memory/omap-gpmc.c                    |  41 +--
 31 files changed, 729 insertions(+), 674 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
 delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
 delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nor.txt
 delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
 create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/gpmc-eth.txt

-- 
2.17.1


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

* [PATCH v3 1/8] ARM: dts: omap: Fixup GPMC child nodes
  2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
@ 2021-09-07 11:32 ` Roger Quadros
  2021-09-07 12:44   ` Krzysztof Kozlowski
  2021-09-07 11:32 ` [PATCH v3 2/8] dt-bindings: mtd: Remove gpmc-nor.txt Roger Quadros
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

Fixes up the GPMC child nodes to prevent dtbs_check errors
after device tree binding conversion to yaml.

- Use reg address as node name
- gpmc,cycle2cycle-samecsen and gpmc,cycle2cycle-diffcsen are
  boolean properties.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../boot/dts/logicpd-som-lv-baseboard.dtsi    |  2 +-
 .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |  2 +-
 .../boot/dts/logicpd-torpedo-baseboard.dtsi   |  2 +-
 arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi     | 62 +++++++++----------
 arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi     | 59 +++++++++---------
 arch/arm/boot/dts/omap-zoom-common.dtsi       | 16 ++---
 arch/arm/boot/dts/omap2430-sdp.dts            |  6 +-
 arch/arm/boot/dts/omap3-cm-t3x30.dtsi         |  6 +-
 .../arm/boot/dts/omap3-devkit8000-common.dtsi |  4 +-
 arch/arm/boot/dts/omap3-evm-37xx.dts          |  1 +
 arch/arm/boot/dts/omap3-evm-common.dtsi       |  9 ---
 .../boot/dts/omap3-evm-processor-common.dtsi  |  5 +-
 arch/arm/boot/dts/omap3-evm.dts               |  1 +
 arch/arm/boot/dts/omap3-igep0020-common.dtsi  |  5 +-
 arch/arm/boot/dts/omap3-ldp.dts               |  5 +-
 arch/arm/boot/dts/omap3-n900.dts              |  2 +-
 .../dts/omap3-overo-chestnut43-common.dtsi    |  6 +-
 .../arm/boot/dts/omap3-overo-tobi-common.dtsi |  6 +-
 .../boot/dts/omap3-overo-tobiduo-common.dtsi  |  8 +--
 arch/arm/boot/dts/omap3-sb-t35.dtsi           |  4 +-
 arch/arm/boot/dts/omap4-duovero-parlor.dts    |  6 +-
 21 files changed, 105 insertions(+), 112 deletions(-)

diff --git a/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi b/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
index 7d0468a23781..f2364cb114c5 100644
--- a/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
+++ b/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
@@ -65,7 +65,7 @@
 		  1 0 0x2c000000 0x1000000	/* CS1: 16MB for LAN9221 */
 		  2 0 0x10000000 0x2000000>;    /* CS2: 32MB for NOR */
 
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@1,0 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&lan9221_pins>;
 		interrupt-parent = <&gpio5>;
diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
index 5532db04046c..6357915d207b 100644
--- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
+++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
@@ -4,8 +4,8 @@
 
 #include "omap36xx.dtsi"
 #include "logicpd-torpedo-som.dtsi"
-#include "omap-gpmc-smsc9221.dtsi"
 #include "logicpd-torpedo-baseboard.dtsi"
+#include "omap-gpmc-smsc9221.dtsi"
 
 / {
 	model = "LogicPD Zoom DM3730 Torpedo + Wireless Development Kit";
diff --git a/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi b/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
index 533a47bc4a53..05049a34b6f1 100644
--- a/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
+++ b/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
@@ -95,7 +95,7 @@
 	ranges = <0 0 0x30000000 0x1000000	/* CS0: 16MB for NAND */
 		  1 0 0x2c000000 0x1000000>;	/* CS1: 16MB for LAN9221 */
 
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@1,0 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&lan9221_pins>;
 		interrupt-parent = <&gpio5>;
diff --git a/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi b/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
index ded7e8fec9eb..2a462cb65a7d 100644
--- a/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
+++ b/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
@@ -20,36 +20,34 @@
 	};
 };
 
-&gpmc {
-	ethernet@gpmc {
-		compatible = "smsc,lan9221", "smsc,lan9115";
-		bank-width = <2>;
-		gpmc,device-width = <1>;
-		gpmc,cycle2cycle-samecsen = <1>;
-		gpmc,cycle2cycle-diffcsen = <1>;
-		gpmc,cs-on-ns = <5>;
-		gpmc,cs-rd-off-ns = <150>;
-		gpmc,cs-wr-off-ns = <150>;
-		gpmc,adv-on-ns = <0>;
-		gpmc,adv-rd-off-ns = <15>;
-		gpmc,adv-wr-off-ns = <40>;
-		gpmc,oe-on-ns = <45>;
-		gpmc,oe-off-ns = <140>;
-		gpmc,we-on-ns = <45>;
-		gpmc,we-off-ns = <140>;
-		gpmc,rd-cycle-ns = <155>;
-		gpmc,wr-cycle-ns = <155>;
-		gpmc,access-ns = <120>;
-		gpmc,page-burst-access-ns = <20>;
-		gpmc,bus-turnaround-ns = <75>;
-		gpmc,cycle2cycle-delay-ns = <75>;
-		gpmc,wait-monitoring-ns = <0>;
-		gpmc,clk-activation-ns = <0>;
-		gpmc,wr-data-mux-bus-ns = <0>;
-		gpmc,wr-access-ns = <0>;
-		vddvario-supply = <&vddvario>;
-		vdd33a-supply = <&vdd33a>;
-		reg-io-width = <4>;
-		smsc,save-mac-address;
-	};
+&gpmc_ethernet {
+	compatible = "smsc,lan9221", "smsc,lan9115";
+	bank-width = <2>;
+	gpmc,device-width = <1>;
+	gpmc,cycle2cycle-samecsen;
+	gpmc,cycle2cycle-diffcsen;
+	gpmc,cs-on-ns = <5>;
+	gpmc,cs-rd-off-ns = <150>;
+	gpmc,cs-wr-off-ns = <150>;
+	gpmc,adv-on-ns = <0>;
+	gpmc,adv-rd-off-ns = <15>;
+	gpmc,adv-wr-off-ns = <40>;
+	gpmc,oe-on-ns = <45>;
+	gpmc,oe-off-ns = <140>;
+	gpmc,we-on-ns = <45>;
+	gpmc,we-off-ns = <140>;
+	gpmc,rd-cycle-ns = <155>;
+	gpmc,wr-cycle-ns = <155>;
+	gpmc,access-ns = <120>;
+	gpmc,page-burst-access-ns = <20>;
+	gpmc,bus-turnaround-ns = <75>;
+	gpmc,cycle2cycle-delay-ns = <75>;
+	gpmc,wait-monitoring-ns = <0>;
+	gpmc,clk-activation-ns = <0>;
+	gpmc,wr-data-mux-bus-ns = <0>;
+	gpmc,wr-access-ns = <0>;
+	vddvario-supply = <&vddvario>;
+	vdd33a-supply = <&vdd33a>;
+	reg-io-width = <4>;
+	smsc,save-mac-address;
 };
diff --git a/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi b/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
index 7f6aefd13451..c1e78f929d2b 100644
--- a/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
+++ b/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
@@ -24,36 +24,33 @@
 	};
 };
 
-&gpmc {
-	ethernet@gpmc {
-		compatible = "smsc,lan9221","smsc,lan9115";
-		bank-width = <2>;
+&gpmc_ethernet {
+	compatible = "smsc,lan9221","smsc,lan9115";
+	bank-width = <2>;
+	gpmc,mux-add-data = <0>;
+	gpmc,cs-on-ns = <0>;
+	gpmc,cs-rd-off-ns = <42>;
+	gpmc,cs-wr-off-ns = <36>;
+	gpmc,adv-on-ns = <6>;
+	gpmc,adv-rd-off-ns = <12>;
+	gpmc,adv-wr-off-ns = <12>;
+	gpmc,oe-on-ns = <0>;
+	gpmc,oe-off-ns = <42>;
+	gpmc,we-on-ns = <0>;
+	gpmc,we-off-ns = <36>;
+	gpmc,rd-cycle-ns = <60>;
+	gpmc,wr-cycle-ns = <54>;
+	gpmc,access-ns = <36>;
+	gpmc,page-burst-access-ns = <0>;
+	gpmc,bus-turnaround-ns = <0>;
+	gpmc,cycle2cycle-delay-ns = <0>;
+	gpmc,wr-data-mux-bus-ns = <18>;
+	gpmc,wr-access-ns = <42>;
+	gpmc,cycle2cycle-samecsen;
+	gpmc,cycle2cycle-diffcsen;
 
-		gpmc,mux-add-data;
-		gpmc,cs-on-ns = <0>;
-		gpmc,cs-rd-off-ns = <42>;
-		gpmc,cs-wr-off-ns = <36>;
-		gpmc,adv-on-ns = <6>;
-		gpmc,adv-rd-off-ns = <12>;
-		gpmc,adv-wr-off-ns = <12>;
-		gpmc,oe-on-ns = <0>;
-		gpmc,oe-off-ns = <42>;
-		gpmc,we-on-ns = <0>;
-		gpmc,we-off-ns = <36>;
-		gpmc,rd-cycle-ns = <60>;
-		gpmc,wr-cycle-ns = <54>;
-		gpmc,access-ns = <36>;
-		gpmc,page-burst-access-ns = <0>;
-		gpmc,bus-turnaround-ns = <0>;
-		gpmc,cycle2cycle-delay-ns = <0>;
-		gpmc,wr-data-mux-bus-ns = <18>;
-		gpmc,wr-access-ns = <42>;
-		gpmc,cycle2cycle-samecsen;
-		gpmc,cycle2cycle-diffcsen;
-
-		vddvario-supply = <&vddvario>;
-		vdd33a-supply = <&vdd33a>;
-		reg-io-width = <4>;
-		smsc,save-mac-address;
-	};
+	vddvario-supply = <&vddvario>;
+	vdd33a-supply = <&vdd33a>;
+	reg-io-width = <4>;
+	smsc,save-mac-address;
 };
diff --git a/arch/arm/boot/dts/omap-zoom-common.dtsi b/arch/arm/boot/dts/omap-zoom-common.dtsi
index d4ad9e58b199..47192c617cd1 100644
--- a/arch/arm/boot/dts/omap-zoom-common.dtsi
+++ b/arch/arm/boot/dts/omap-zoom-common.dtsi
@@ -3,8 +3,6 @@
  * Common features on the Zoom debug board
  */
 
-#include "omap-gpmc-smsc911x.dtsi"
-
 &gpmc {
 	ranges = <3 0 0x10000000 0x1000000>,	/* CS3: 16MB for UART */
 		 <7 0 0x2c000000 0x01000000>;
@@ -27,8 +25,8 @@
 		gpmc,mux-add-data = <0>;
 		gpmc,device-width = <1>;
 		gpmc,wait-pin = <1>;
-		gpmc,cycle2cycle-samecsen = <1>;
-		gpmc,cycle2cycle-diffcsen = <1>;
+		gpmc,cycle2cycle-samecsen;
+		gpmc,cycle2cycle-diffcsen;
 		gpmc,cs-on-ns = <5>;
 		gpmc,cs-rd-off-ns = <155>;
 		gpmc,cs-wr-off-ns = <155>;
@@ -50,7 +48,7 @@
 		gpmc,wr-data-mux-bus-ns = <45>;
 		gpmc,wr-access-ns = <145>;
 	};
-	uart@3,1 {
+	uart@3,100 {
 		compatible = "ns16550a";
 		reg = <3 0x100 8>;	/* CS3, offset 0x100, IO size 8 */
 		bank-width = <2>;
@@ -61,7 +59,7 @@
 		clock-frequency = <1843200>;
 		current-speed = <115200>;
 	};
-	uart@3,2 {
+	uart@3,200 {
 		compatible = "ns16550a";
 		reg = <3 0x200 8>;	/* CS3, offset 0x200, IO size 8 */
 		bank-width = <2>;
@@ -72,7 +70,7 @@
 		clock-frequency = <1843200>;
 		current-speed = <115200>;
 	};
-	uart@3,3 {
+	uart@3,300 {
 		compatible = "ns16550a";
 		reg = <3 0x300 8>;	/* CS3, offset 0x300, IO size 8 */
 		bank-width = <2>;
@@ -84,9 +82,11 @@
 		current-speed = <115200>;
 	};
 
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@7,0 {
 		reg = <7 0 0xff>;
 		interrupt-parent = <&gpio5>;
 		interrupts = <30 IRQ_TYPE_LEVEL_LOW>;	/* gpio158 */
 	};
 };
+
+#include "omap-gpmc-smsc911x.dtsi"
diff --git a/arch/arm/boot/dts/omap2430-sdp.dts b/arch/arm/boot/dts/omap2430-sdp.dts
index 7d27e907533f..f74906e4721b 100644
--- a/arch/arm/boot/dts/omap2430-sdp.dts
+++ b/arch/arm/boot/dts/omap2430-sdp.dts
@@ -34,7 +34,7 @@
 
 &gpmc {
 	ranges = <5 0 0x08000000 0x01000000>;
-	ethernet@gpmc {
+	ethernet@5,300 {
 		compatible = "smsc,lan91c94";
 		interrupt-parent = <&gpio5>;
 		interrupts = <21 IRQ_TYPE_LEVEL_LOW>;	/* gpio149 */
@@ -43,8 +43,8 @@
 		gpmc,sync-clk-ps = <0>;
 		gpmc,mux-add-data = <2>;
 		gpmc,device-width = <1>;
-		gpmc,cycle2cycle-samecsen = <1>;
-		gpmc,cycle2cycle-diffcsen = <1>;
+		gpmc,cycle2cycle-samecsen;
+		gpmc,cycle2cycle-diffcsen;
 		gpmc,cs-on-ns = <6>;
 		gpmc,cs-rd-off-ns = <187>;
 		gpmc,cs-wr-off-ns = <187>;
diff --git a/arch/arm/boot/dts/omap3-cm-t3x30.dtsi b/arch/arm/boot/dts/omap3-cm-t3x30.dtsi
index 5e8943539fcc..2059463de9d6 100644
--- a/arch/arm/boot/dts/omap3-cm-t3x30.dtsi
+++ b/arch/arm/boot/dts/omap3-cm-t3x30.dtsi
@@ -47,13 +47,11 @@
 	};
 };
 
-#include "omap-gpmc-smsc911x.dtsi"
-
 &gpmc {
 	ranges = <5 0 0x2c000000 0x01000000>, /* CM-T3x30 SMSC9x Eth */
 		 <0 0 0x00000000 0x01000000>; /* CM-T3x NAND */
 
-	smsc1: ethernet@gpmc {
+	gpmc_ethernet: ethernet@5,0 {
 		compatible = "smsc,lan9221", "smsc,lan9115";
 		pinctrl-names = "default";
 		pinctrl-0 = <&smsc1_pins>;
@@ -63,6 +61,8 @@
 	};
 };
 
+#include "omap-gpmc-smsc911x.dtsi"
+
 &i2c1 {
 	twl: twl@48 {
 		reg = <0x48>;
diff --git a/arch/arm/boot/dts/omap3-devkit8000-common.dtsi b/arch/arm/boot/dts/omap3-devkit8000-common.dtsi
index 2c19d6e255bd..5e55198e4576 100644
--- a/arch/arm/boot/dts/omap3-devkit8000-common.dtsi
+++ b/arch/arm/boot/dts/omap3-devkit8000-common.dtsi
@@ -267,8 +267,8 @@
 		gpmc,mux-add-data = <0>;
 		gpmc,device-width = <1>;
 		gpmc,wait-pin = <0>;
-		gpmc,cycle2cycle-samecsen = <1>;
-		gpmc,cycle2cycle-diffcsen = <1>;
+		gpmc,cycle2cycle-samecsen;
+		gpmc,cycle2cycle-diffcsen;
 
 		gpmc,cs-on-ns = <6>;
 		gpmc,cs-rd-off-ns = <180>;
diff --git a/arch/arm/boot/dts/omap3-evm-37xx.dts b/arch/arm/boot/dts/omap3-evm-37xx.dts
index c9332195d096..077b1a19f171 100644
--- a/arch/arm/boot/dts/omap3-evm-37xx.dts
+++ b/arch/arm/boot/dts/omap3-evm-37xx.dts
@@ -7,6 +7,7 @@
 #include "omap36xx.dtsi"
 #include "omap3-evm-common.dtsi"
 #include "omap3-evm-processor-common.dtsi"
+#include "omap-gpmc-smsc911x.dtsi"
 
 / {
 	model = "TI OMAP37XX EVM (TMDSEVM3730)";
diff --git a/arch/arm/boot/dts/omap3-evm-common.dtsi b/arch/arm/boot/dts/omap3-evm-common.dtsi
index 17c89df6ce6b..2f7bdb248a51 100644
--- a/arch/arm/boot/dts/omap3-evm-common.dtsi
+++ b/arch/arm/boot/dts/omap3-evm-common.dtsi
@@ -4,7 +4,6 @@
  */
 
 #include <dt-bindings/input/input.h>
-#include "omap-gpmc-smsc911x.dtsi"
 
 / {
 	cpus {
@@ -182,14 +181,6 @@
 	power = <50>;
 };
 
-&gpmc {
-	ethernet@gpmc {
-		interrupt-parent = <&gpio6>;
-		interrupts = <16 8>;
-		reg = <5 0 0xff>;
-	};
-};
-
 &vaux2 {
 	regulator-name = "usb_1v8";
 	regulator-min-microvolt = <1800000>;
diff --git a/arch/arm/boot/dts/omap3-evm-processor-common.dtsi b/arch/arm/boot/dts/omap3-evm-processor-common.dtsi
index e6ba30a21166..3adb967b96c4 100644
--- a/arch/arm/boot/dts/omap3-evm-processor-common.dtsi
+++ b/arch/arm/boot/dts/omap3-evm-processor-common.dtsi
@@ -217,8 +217,11 @@
 	ranges = <0 0 0x30000000 0x1000000>,	/* CS0: 16MB for NAND */
 		 <5 0 0x2c000000 0x01000000>;	/* CS5: 16MB for LAN9220 */
 
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@5,0 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&smsc911x_pins>;
+		interrupt-parent = <&gpio6>;
+		interrupts = <16 8>;
+		reg = <5 0 0xff>;
 	};
 };
diff --git a/arch/arm/boot/dts/omap3-evm.dts b/arch/arm/boot/dts/omap3-evm.dts
index 5cc0cf7cd16c..805856d47cda 100644
--- a/arch/arm/boot/dts/omap3-evm.dts
+++ b/arch/arm/boot/dts/omap3-evm.dts
@@ -7,6 +7,7 @@
 #include "omap34xx.dtsi"
 #include "omap3-evm-common.dtsi"
 #include "omap3-evm-processor-common.dtsi"
+#include "omap-gpmc-smsc911x.dtsi"
 
 / {
 	model = "TI OMAP35XX EVM (TMDSEVM3530)";
diff --git a/arch/arm/boot/dts/omap3-igep0020-common.dtsi b/arch/arm/boot/dts/omap3-igep0020-common.dtsi
index 73d8f471b9ec..104df84ea720 100644
--- a/arch/arm/boot/dts/omap3-igep0020-common.dtsi
+++ b/arch/arm/boot/dts/omap3-igep0020-common.dtsi
@@ -7,7 +7,6 @@
  */
 
 #include "omap3-igep.dtsi"
-#include "omap-gpmc-smsc9221.dtsi"
 
 / {
 
@@ -217,7 +216,7 @@
 	ranges = <0 0 0x30000000 0x01000000>,	/* CS0: 16MB for NAND */
 		 <5 0 0x2c000000 0x01000000>;	/* CS5: 16MB for ethernet */
 
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@5,0 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&smsc9221_pins>;
 		reg = <5 0 0xff>;
@@ -226,6 +225,8 @@
 	};
 };
 
+#include "omap-gpmc-smsc9221.dtsi"
+
 &uart2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart2_pins>;
diff --git a/arch/arm/boot/dts/omap3-ldp.dts b/arch/arm/boot/dts/omap3-ldp.dts
index 9c6a92724590..a299f04217fd 100644
--- a/arch/arm/boot/dts/omap3-ldp.dts
+++ b/arch/arm/boot/dts/omap3-ldp.dts
@@ -6,7 +6,6 @@
 
 #include <dt-bindings/input/input.h>
 #include "omap34xx.dtsi"
-#include "omap-gpmc-smsc911x.dtsi"
 
 / {
 	model = "TI OMAP3430 LDP (Zoom1 Labrador)";
@@ -148,13 +147,15 @@
 		};
 	};
 
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@1,0 {
 		interrupt-parent = <&gpio5>;
 		interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
 		reg = <1 0 0xff>;
 	};
 };
 
+#include "omap-gpmc-smsc911x.dtsi"
+
 &i2c1 {
 	clock-frequency = <2600000>;
 
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 32335d4ce478..35602d7876aa 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -936,7 +936,7 @@
 	};
 
 	/* Ethernet is on some early development boards and qemu */
-	ethernet@gpmc {
+	ethernet@1,0 {
 		compatible = "smsc,lan91c94";
 		interrupt-parent = <&gpio2>;
 		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;	/* gpio54 */
diff --git a/arch/arm/boot/dts/omap3-overo-chestnut43-common.dtsi b/arch/arm/boot/dts/omap3-overo-chestnut43-common.dtsi
index 2d2c61d7aa86..34b805851dd2 100644
--- a/arch/arm/boot/dts/omap3-overo-chestnut43-common.dtsi
+++ b/arch/arm/boot/dts/omap3-overo-chestnut43-common.dtsi
@@ -49,16 +49,16 @@
 	};
 };
 
-#include "omap-gpmc-smsc9221.dtsi"
-
 &gpmc {
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@5,0 {
 		reg = <5 0 0xff>;
 		interrupt-parent = <&gpio6>;
 		interrupts = <16 IRQ_TYPE_LEVEL_LOW>;	/* GPIO 176 */
 	};
 };
 
+#include "omap-gpmc-smsc9221.dtsi"
+
 &lis33de {
 	status = "disabled";
 };
diff --git a/arch/arm/boot/dts/omap3-overo-tobi-common.dtsi b/arch/arm/boot/dts/omap3-overo-tobi-common.dtsi
index 9bf4b88a4b50..895803654e3c 100644
--- a/arch/arm/boot/dts/omap3-overo-tobi-common.dtsi
+++ b/arch/arm/boot/dts/omap3-overo-tobi-common.dtsi
@@ -21,16 +21,16 @@
 	};
 };
 
-#include "omap-gpmc-smsc9221.dtsi"
-
 &gpmc {
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@5,0 {
 		reg = <5 0 0xff>;
 		interrupt-parent = <&gpio6>;
 		interrupts = <16 IRQ_TYPE_LEVEL_LOW>;	/* GPIO 176 */
 	};
 };
 
+#include "omap-gpmc-smsc9221.dtsi"
+
 &lis33de {
 	status = "disabled";
 };
diff --git a/arch/arm/boot/dts/omap3-overo-tobiduo-common.dtsi b/arch/arm/boot/dts/omap3-overo-tobiduo-common.dtsi
index e5da3bc6f105..c212d72dcfe9 100644
--- a/arch/arm/boot/dts/omap3-overo-tobiduo-common.dtsi
+++ b/arch/arm/boot/dts/omap3-overo-tobiduo-common.dtsi
@@ -9,10 +9,8 @@
 
 #include "omap3-overo-common-peripherals.dtsi"
 
-#include "omap-gpmc-smsc9221.dtsi"
-
 &gpmc {
-	smsc1: ethernet@gpmc {
+	gpmc_ethernet: ethernet@5,0 {
 		reg = <5 0 0xff>;
 		interrupt-parent = <&gpio6>;
 		interrupts = <16 IRQ_TYPE_LEVEL_LOW>;	/* GPIO 176 */
@@ -22,7 +20,7 @@
 		compatible = "smsc,lan9221","smsc,lan9115";
 		bank-width = <2>;
 
-		gpmc,mux-add-data;
+		gpmc,mux-add-data = <0>;
 		gpmc,cs-on-ns = <0>;
 		gpmc,cs-rd-off-ns = <42>;
 		gpmc,cs-wr-off-ns = <36>;
@@ -54,6 +52,8 @@
 	};
 };
 
+#include "omap-gpmc-smsc9221.dtsi"
+
 &lis33de {
 	status = "disabled";
 };
diff --git a/arch/arm/boot/dts/omap3-sb-t35.dtsi b/arch/arm/boot/dts/omap3-sb-t35.dtsi
index fb9842fa922c..5ec0893415e0 100644
--- a/arch/arm/boot/dts/omap3-sb-t35.dtsi
+++ b/arch/arm/boot/dts/omap3-sb-t35.dtsi
@@ -108,8 +108,8 @@
 		reg = <4 0 0xff>;
 		bank-width = <2>;
 		gpmc,device-width = <1>;
-		gpmc,cycle2cycle-samecsen = <1>;
-		gpmc,cycle2cycle-diffcsen = <1>;
+		gpmc,cycle2cycle-samecsen;
+		gpmc,cycle2cycle-diffcsen;
 		gpmc,cs-on-ns = <5>;
 		gpmc,cs-rd-off-ns = <150>;
 		gpmc,cs-wr-off-ns = <150>;
diff --git a/arch/arm/boot/dts/omap4-duovero-parlor.dts b/arch/arm/boot/dts/omap4-duovero-parlor.dts
index b294c22177cb..dfc50764c405 100644
--- a/arch/arm/boot/dts/omap4-duovero-parlor.dts
+++ b/arch/arm/boot/dts/omap4-duovero-parlor.dts
@@ -131,12 +131,10 @@
 	status = "disabled";
 };
 
-#include "omap-gpmc-smsc911x.dtsi"
-
 &gpmc {
 	ranges = <5 0 0x2c000000 0x1000000>;			/* CS5 */
 
-	ethernet@gpmc {
+	gpmc_ethernet: ethernet@5,0 {
 		reg = <5 0 0xff>;
 		interrupt-parent = <&gpio2>;
 		interrupts = <12 IRQ_TYPE_LEVEL_LOW>;		/* gpio_44 */
@@ -170,6 +168,8 @@
 	};
 };
 
+#include "omap-gpmc-smsc911x.dtsi"
+
 &dss {
 	status = "okay";
 };
-- 
2.17.1


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

* [PATCH v3 2/8] dt-bindings: mtd: Remove gpmc-nor.txt
  2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 1/8] ARM: dts: omap: Fixup GPMC child nodes Roger Quadros
@ 2021-09-07 11:32 ` Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 3/8] dt-bindings: net: Remove gpmc-eth.txt Roger Quadros
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

There is no GPMC NOR compatible or device driver. GPMC is just
a bus interface over which standard (CFI/JEDC) NOR Flash chips
can be attached.

For NOR chip bindings, please refer to
Documentation/devicetree/bindings/mtd/mtd-physmap.yaml

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../devicetree/bindings/mtd/gpmc-nor.txt      | 98 -------------------
 1 file changed, 98 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nor.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nor.txt b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
deleted file mode 100644
index c8567b40fe13..000000000000
--- a/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
+++ /dev/null
@@ -1,98 +0,0 @@
-Device tree bindings for NOR flash connect to TI GPMC
-
-NOR flash connected to the TI GPMC (found on OMAP boards) are represented as
-child nodes of the GPMC controller with a name of "nor".
-
-All timing relevant properties as well as generic GPMC child properties are
-explained in a separate documents. Please refer to
-Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
-
-Required properties:
-- bank-width: 		Width of NOR flash in bytes. GPMC supports 8-bit and
-			16-bit devices and so must be either 1 or 2 bytes.
-- compatible:		Documentation/devicetree/bindings/mtd/mtd-physmap.txt
-- gpmc,cs-on-ns:		Chip-select assertion time
-- gpmc,cs-rd-off-ns:	Chip-select de-assertion time for reads
-- gpmc,cs-wr-off-ns:	Chip-select de-assertion time for writes
-- gpmc,oe-on-ns:	Output-enable assertion time
-- gpmc,oe-off-ns:	Output-enable de-assertion time
-- gpmc,we-on-ns		Write-enable assertion time
-- gpmc,we-off-ns:	Write-enable de-assertion time
-- gpmc,access-ns:	Start cycle to first data capture (read access)
-- gpmc,rd-cycle-ns:	Total read cycle time
-- gpmc,wr-cycle-ns:	Total write cycle time
-- linux,mtd-name:	Documentation/devicetree/bindings/mtd/mtd-physmap.txt
-- reg:			Chip-select, base address (relative to chip-select)
-			and size of NOR flash. Note that base address will be
-			typically 0 as this is the start of the chip-select.
-
-Optional properties:
-- gpmc,XXX		Additional GPMC timings and settings parameters. See
-			Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
-
-Optional properties for partition table parsing:
-- #address-cells: should be set to 1
-- #size-cells: should be set to 1
-
-Example:
-
-gpmc: gpmc@6e000000 {
-	compatible = "ti,omap3430-gpmc", "simple-bus";
-	ti,hwmods = "gpmc";
-	reg = <0x6e000000 0x1000>;
-	interrupts = <20>;
-	gpmc,num-cs = <8>;
-	gpmc,num-waitpins = <4>;
-	#address-cells = <2>;
-	#size-cells = <1>;
-
-	ranges = <0 0 0x10000000 0x08000000>;
-
-	nor@0,0 {
-		compatible = "cfi-flash";
-		linux,mtd-name= "intel,pf48f6000m0y1be";
-		#address-cells = <1>;
-		#size-cells = <1>;
-		reg = <0 0 0x08000000>;
-		bank-width = <2>;
-
-		gpmc,mux-add-data;
-		gpmc,cs-on-ns = <0>;
-		gpmc,cs-rd-off-ns = <186>;
-		gpmc,cs-wr-off-ns = <186>;
-		gpmc,adv-on-ns = <12>;
-		gpmc,adv-rd-off-ns = <48>;
-		gpmc,adv-wr-off-ns = <48>;
-		gpmc,oe-on-ns = <54>;
-		gpmc,oe-off-ns = <168>;
-		gpmc,we-on-ns = <54>;
-		gpmc,we-off-ns = <168>;
-		gpmc,rd-cycle-ns = <186>;
-		gpmc,wr-cycle-ns = <186>;
-		gpmc,access-ns = <114>;
-		gpmc,page-burst-access-ns = <6>;
-		gpmc,bus-turnaround-ns = <12>;
-		gpmc,cycle2cycle-delay-ns = <18>;
-		gpmc,wr-data-mux-bus-ns = <90>;
-		gpmc,wr-access-ns = <186>;
-		gpmc,cycle2cycle-samecsen;
-		gpmc,cycle2cycle-diffcsen;
-
-		partition@0 {
-			label = "bootloader-nor";
-			reg = <0 0x40000>;
-		};
-		partition@40000 {
-			label = "params-nor";
-			reg = <0x40000 0x40000>;
-		};
-		partition@80000 {
-			label = "kernel-nor";
-			reg = <0x80000 0x200000>;
-		};
-		partition@280000 {
-			label = "filesystem-nor";
-			reg = <0x240000 0x7d80000>;
-		};
-	};
-};
-- 
2.17.1


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

* [PATCH v3 3/8] dt-bindings: net: Remove gpmc-eth.txt
  2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 1/8] ARM: dts: omap: Fixup GPMC child nodes Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 2/8] dt-bindings: mtd: Remove gpmc-nor.txt Roger Quadros
@ 2021-09-07 11:32 ` Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 4/8] dt-bindings: memory-controllers: Introduce ti,gpmc-child Roger Quadros
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

There is no GPMC Ethernet compatible or device driver. GPMC is
just a bus interface over which devices like Ethernet controller
can be to.

For SMSC 911x Ethernet chip bindings, please refer to
Documentation/devicetree/bindings/net/smsc,lan9115.yaml

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../devicetree/bindings/net/gpmc-eth.txt      | 97 -------------------
 1 file changed, 97 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/gpmc-eth.txt

diff --git a/Documentation/devicetree/bindings/net/gpmc-eth.txt b/Documentation/devicetree/bindings/net/gpmc-eth.txt
deleted file mode 100644
index 32821066a85b..000000000000
--- a/Documentation/devicetree/bindings/net/gpmc-eth.txt
+++ /dev/null
@@ -1,97 +0,0 @@
-Device tree bindings for Ethernet chip connected to TI GPMC
-
-Besides being used to interface with external memory devices, the
-General-Purpose Memory Controller can be used to connect Pseudo-SRAM devices
-such as ethernet controllers to processors using the TI GPMC as a data bus.
-
-Ethernet controllers connected to TI GPMC are represented as child nodes of
-the GPMC controller with an "ethernet" name.
-
-All timing relevant properties as well as generic GPMC child properties are
-explained in a separate documents. Please refer to
-Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
-
-For the properties relevant to the ethernet controller connected to the GPMC
-refer to the binding documentation of the device. For example, the documentation
-for the SMSC 911x is Documentation/devicetree/bindings/net/smsc,lan9115.yaml
-
-Child nodes need to specify the GPMC bus address width using the "bank-width"
-property but is possible that an ethernet controller also has a property to
-specify the I/O registers address width. Even when the GPMC has a maximum 16-bit
-address width, it supports devices with 32-bit word registers.
-For example with an SMSC LAN911x/912x controller connected to the TI GPMC on an
-OMAP2+ board, "bank-width = <2>;" and "reg-io-width = <4>;".
-
-Required properties:
-- bank-width: 		Address width of the device in bytes. GPMC supports 8-bit
-			and 16-bit devices and so must be either 1 or 2 bytes.
-- compatible:		Compatible string property for the ethernet child device.
-- gpmc,cs-on-ns:	Chip-select assertion time
-- gpmc,cs-rd-off-ns:	Chip-select de-assertion time for reads
-- gpmc,cs-wr-off-ns:	Chip-select de-assertion time for writes
-- gpmc,oe-on-ns:	Output-enable assertion time
-- gpmc,oe-off-ns:	Output-enable de-assertion time
-- gpmc,we-on-ns:	Write-enable assertion time
-- gpmc,we-off-ns:	Write-enable de-assertion time
-- gpmc,access-ns:	Start cycle to first data capture (read access)
-- gpmc,rd-cycle-ns:	Total read cycle time
-- gpmc,wr-cycle-ns:	Total write cycle time
-- reg:			Chip-select, base address (relative to chip-select)
-			and size of the memory mapped for the device.
-			Note that base address will be typically 0 as this
-			is the start of the chip-select.
-
-Optional properties:
-- gpmc,XXX		Additional GPMC timings and settings parameters. See
-			Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
-
-Example:
-
-gpmc: gpmc@6e000000 {
-	compatible = "ti,omap3430-gpmc";
-	ti,hwmods = "gpmc";
-	reg = <0x6e000000 0x1000>;
-	interrupts = <20>;
-	gpmc,num-cs = <8>;
-	gpmc,num-waitpins = <4>;
-	#address-cells = <2>;
-	#size-cells = <1>;
-
-	ranges = <5 0 0x2c000000 0x1000000>;
-
-	ethernet@5,0 {
-		compatible = "smsc,lan9221", "smsc,lan9115";
-		reg = <5 0 0xff>;
-		bank-width = <2>;
-
-		gpmc,mux-add-data;
-		gpmc,cs-on-ns = <0>;
-		gpmc,cs-rd-off-ns = <186>;
-		gpmc,cs-wr-off-ns = <186>;
-		gpmc,adv-on-ns = <12>;
-		gpmc,adv-rd-off-ns = <48>;
-		gpmc,adv-wr-off-ns = <48>;
-		gpmc,oe-on-ns = <54>;
-		gpmc,oe-off-ns = <168>;
-		gpmc,we-on-ns = <54>;
-		gpmc,we-off-ns = <168>;
-		gpmc,rd-cycle-ns = <186>;
-		gpmc,wr-cycle-ns = <186>;
-		gpmc,access-ns = <114>;
-		gpmc,page-burst-access-ns = <6>;
-		gpmc,bus-turnaround-ns = <12>;
-		gpmc,cycle2cycle-delay-ns = <18>;
-		gpmc,wr-data-mux-bus-ns = <90>;
-		gpmc,wr-access-ns = <186>;
-		gpmc,cycle2cycle-samecsen;
-		gpmc,cycle2cycle-diffcsen;
-
-		interrupt-parent = <&gpio6>;
-		interrupts = <16>;
-		vmmc-supply = <&vddvario>;
-		vmmc_aux-supply = <&vdd33a>;
-		reg-io-width = <4>;
-
-		smsc,save-mac-address;
-	};
-};
-- 
2.17.1


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

* [PATCH v3 4/8] dt-bindings: memory-controllers: Introduce ti,gpmc-child
  2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
                   ` (2 preceding siblings ...)
  2021-09-07 11:32 ` [PATCH v3 3/8] dt-bindings: net: Remove gpmc-eth.txt Roger Quadros
@ 2021-09-07 11:32 ` Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml Roger Quadros
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

This binding is meant for the child nodes of the TI GPMC node. The node
represents any device connected to the GPMC bus. It may be a Flash chip,
RAM chip or Ethernet controller, etc. These properties are meant for
configuring the GPMC settings/timings and will accompany the bindings
supported by the respective device.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../memory-controllers/ti,gpmc-child.yaml     | 245 ++++++++++++++++++
 1 file changed, 245 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml

diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
new file mode 100644
index 000000000000..6e3995bb1630
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
@@ -0,0 +1,245 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/ti,gpmc-child.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: device tree bindings for children of the Texas Instruments GPMC
+
+maintainers:
+  - Tony Lindgren <tony@atomide.com>
+  - Roger Quadros <rogerq@kernel.org>
+
+description:
+  This binding is meant for the child nodes of the GPMC node. The node
+  represents any device connected to the GPMC bus. It may be a Flash chip,
+  RAM chip or Ethernet controller, etc. These properties are meant for
+  configuring the GPMC settings/timings and will accompany the bindings
+  supported by the respective device.
+
+properties:
+  reg: true
+
+# GPMC Timing properties for child nodes. All are optional and default to 0.
+  gpmc,sync-clk-ps:
+    description: Minimum clock period for synchronous mode
+    default: 0
+
+# Chip-select signal timings corresponding to GPMC_CONFIG2:
+  gpmc,cs-on-ns:
+    description: Assertion time
+    default: 0
+
+  gpmc,cs-rd-off-ns:
+    description: Read deassertion time
+    default: 0
+
+  gpmc,cs-wr-off-ns:
+    description: Write deassertion time
+    default: 0
+
+# ADV signal timings corresponding to GPMC_CONFIG3:
+  gpmc,adv-on-ns:
+    description: Assertion time
+    default: 0
+
+  gpmc,adv-rd-off-ns:
+    description: Read deassertion time
+    default: 0
+
+  gpmc,adv-wr-off-ns:
+    description: Write deassertion time
+    default: 0
+
+  gpmc,adv-aad-mux-on-ns:
+    description: Assertion time for AAD
+    default: 0
+
+  gpmc,adv-aad-mux-rd-off-ns:
+    description: Read deassertion time for AAD
+    default: 0
+
+  gpmc,adv-aad-mux-wr-off-ns:
+    description: Write deassertion time for AAD
+    default: 0
+
+# WE signals timings corresponding to GPMC_CONFIG4:
+  gpmc,we-on-ns:
+    description: Assertion time
+    default: 0
+
+  gpmc,we-off-ns:
+    description: Deassertion time
+    default: 0
+
+# OE signals timings corresponding to GPMC_CONFIG4:
+  gpmc,oe-on-ns:
+    description: Assertion time
+    default: 0
+
+  gpmc,oe-off-ns:
+    description: Deassertion time
+    default: 0
+
+  gpmc,oe-aad-mux-on-ns:
+    description: Assertion time for AAD
+    default: 0
+
+  gpmc,oe-aad-mux-off-ns:
+    description: Deassertion time for AAD
+    default: 0
+
+# Access time and cycle time timings (in nanoseconds) corresponding to
+# GPMC_CONFIG5:
+  gpmc,page-burst-access-ns:
+    description: Multiple access word delay
+    default: 0
+
+  gpmc,access-ns:
+    description: Start-cycle to first data valid delay
+    default: 0
+
+  gpmc,rd-cycle-ns:
+    description: Total read cycle time
+    default: 0
+
+  gpmc,wr-cycle-ns:
+    description: Total write cycle time
+    default: 0
+
+  gpmc,bus-turnaround-ns:
+    description: Turn-around time between successive accesses
+    default: 0
+
+  gpmc,cycle2cycle-delay-ns:
+    description: Delay between chip-select pulses
+    default: 0
+
+  gpmc,clk-activation-ns:
+    description: GPMC clock activation time
+    default: 0
+
+  gpmc,wait-monitoring-ns:
+    description: Start of wait monitoring with regard to valid data
+    default: 0
+
+# Boolean timing parameters. If property is present, parameter is enabled
+# otherwise disabled.
+  gpmc,adv-extra-delay:
+    description: ADV signal is delayed by half GPMC clock
+    type: boolean
+
+  gpmc,cs-extra-delay:
+    description: CS signal is delayed by half GPMC clock
+    type: boolean
+
+  gpmc,cycle2cycle-diffcsen:
+    description: |
+      Add "cycle2cycle-delay" between successive accesses
+      to a different CS
+    type: boolean
+
+  gpmc,cycle2cycle-samecsen:
+    description: |
+      Add "cycle2cycle-delay" between successive accesses
+      to the same CS
+    type: boolean
+
+  gpmc,oe-extra-delay:
+    description: OE signal is delayed by half GPMC clock
+    type: boolean
+
+  gpmc,we-extra-delay:
+    description: WE signal is delayed by half GPMC clock
+    type: boolean
+
+  gpmc,time-para-granularity:
+    description: Multiply all access times by 2
+    type: boolean
+
+# The following two properties are applicable only to OMAP3+ and AM335x:
+  gpmc,wr-access-ns:
+    description: |
+      In synchronous write mode, for single or
+      burst accesses, defines the number of
+      GPMC_FCLK cycles from start access time
+      to the GPMC_CLK rising edge used by the
+      memory device for the first data capture.
+    default: 0
+
+  gpmc,wr-data-mux-bus-ns:
+    description: |
+      In address-data multiplex mode, specifies
+      the time when the first data is driven on
+      the address-data bus.
+    default: 0
+
+# GPMC chip-select settings properties for child nodes. All are optional.
+  gpmc,burst-length:
+    description: Page/burst length.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 4, 8, 16]
+    default: 0
+
+  gpmc,burst-wrap:
+    description: Enables wrap bursting
+    type: boolean
+
+  gpmc,burst-read:
+    description: Enables read page/burst mode
+    type: boolean
+
+  gpmc,burst-write:
+    description: Enables write page/burst mode
+    type: boolean
+
+  gpmc,device-width:
+    description: |
+      Total width of device(s) connected to a GPMC
+      chip-select in bytes. The GPMC supports 8-bit
+      and 16-bit devices and so this property must be
+      1 or 2.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+    default: 1
+
+  gpmc,mux-add-data:
+    description: |
+      Address and data multiplexing configuration.
+      Valid values are
+      0 for Non multiplexed mode
+      1 for address-address-data multiplexing mode and
+      2 for address-data multiplexing mode.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2]
+
+  gpmc,sync-read:
+    description: |
+      Enables synchronous read. Defaults to asynchronous
+      is this is not set.
+    type: boolean
+
+  gpmc,sync-write:
+    description: |
+      Enables synchronous writes. Defaults to asynchronous
+      is this is not set.
+    type: boolean
+
+  gpmc,wait-pin:
+    description: |
+      Wait-pin used by client. Must be less than "gpmc,num-waitpins".
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  gpmc,wait-on-read:
+    description: Enables wait monitoring on reads.
+    type: boolean
+
+  gpmc,wait-on-write:
+    description: Enables wait monitoring on writes.
+    type: boolean
+
+required:
+  - reg
+
+# the GPMC child will have its own native properties
+additionalProperties: true
-- 
2.17.1


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

* [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
                   ` (3 preceding siblings ...)
  2021-09-07 11:32 ` [PATCH v3 4/8] dt-bindings: memory-controllers: Introduce ti,gpmc-child Roger Quadros
@ 2021-09-07 11:32 ` Roger Quadros
  2021-09-07 14:03   ` Miquel Raynal
  2021-09-07 17:01   ` Rob Herring
  2021-09-07 11:32 ` [PATCH v3 6/8] dt-bindings: mtd: ti,gpmc-onenand: " Roger Quadros
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

Convert gpmc-nand.txt to ti,gpmc-nand.yaml.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../devicetree/bindings/mtd/gpmc-nand.txt     | 147 ------------------
 .../devicetree/bindings/mtd/ti,gpmc-nand.yaml | 110 +++++++++++++
 2 files changed, 110 insertions(+), 147 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
deleted file mode 100644
index 44919d48d241..000000000000
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ /dev/null
@@ -1,147 +0,0 @@
-Device tree bindings for GPMC connected NANDs
-
-GPMC connected NAND (found on OMAP boards) are represented as child nodes of
-the GPMC controller with a name of "nand".
-
-All timing relevant properties as well as generic gpmc child properties are
-explained in a separate documents - please refer to
-Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
-
-For NAND specific properties such as ECC modes or bus width, please refer to
-Documentation/devicetree/bindings/mtd/nand-controller.yaml
-
-
-Required properties:
-
- - compatible:	"ti,omap2-nand"
- - reg:		range id (CS number), base offset and length of the
-		NAND I/O space
- - interrupts:	Two interrupt specifiers, one for fifoevent, one for termcount.
-
-Optional properties:
-
- - nand-bus-width: 		Set this numeric value to 16 if the hardware
-				is wired that way. If not specified, a bus
-				width of 8 is assumed.
-
- - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
-		"sw"		1-bit Hamming ecc code via software
-		"hw"		<deprecated> use "ham1" instead
-		"hw-romcode"	<deprecated> use "ham1" instead
-		"ham1"		1-bit Hamming ecc code
-		"bch4"		4-bit BCH ecc code
-		"bch8"		8-bit BCH ecc code
-		"bch16"		16-bit BCH ECC code
-		Refer below "How to select correct ECC scheme for your device ?"
-
- - ti,nand-xfer-type:		A string setting the data transfer type. One of:
-
-		"prefetch-polled"	Prefetch polled mode (default)
-		"polled"		Polled mode, without prefetch
-		"prefetch-dma"		Prefetch enabled DMA mode
-		"prefetch-irq"		Prefetch enabled irq mode
-
- - elm_id:	<deprecated> use "ti,elm-id" instead
- - ti,elm-id:	Specifies phandle of the ELM devicetree node.
-		ELM is an on-chip hardware engine on TI SoC which is used for
-		locating ECC errors for BCHx algorithms. SoC devices which have
-		ELM hardware engines should specify this device node in .dtsi
-		Using ELM for ECC error correction frees some CPU cycles.
- - rb-gpios:	GPIO specifier for the ready/busy# pin.
-
-For inline partition table parsing (optional):
-
- - #address-cells: should be set to 1
- - #size-cells: should be set to 1
-
-Example for an AM33xx board:
-
-	gpmc: gpmc@50000000 {
-		compatible = "ti,am3352-gpmc";
-		ti,hwmods = "gpmc";
-		reg = <0x50000000 0x36c>;
-		interrupts = <100>;
-		gpmc,num-cs = <8>;
-		gpmc,num-waitpins = <2>;
-		#address-cells = <2>;
-		#size-cells = <1>;
-		ranges = <0 0 0x08000000 0x1000000>;	/* CS0 space, 16MB */
-		elm_id = <&elm>;
-		interrupt-controller;
-		#interrupt-cells = <2>;
-
-		nand@0,0 {
-			compatible = "ti,omap2-nand";
-			reg = <0 0 4>;		/* CS0, offset 0, NAND I/O window 4 */
-			interrupt-parent = <&gpmc>;
-			interrupts = <0 IRQ_TYPE_NONE>, <1 IRQ_TYPE NONE>;
-			nand-bus-width = <16>;
-			ti,nand-ecc-opt = "bch8";
-			ti,nand-xfer-type = "polled";
-			rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
-
-			gpmc,sync-clk-ps = <0>;
-			gpmc,cs-on-ns = <0>;
-			gpmc,cs-rd-off-ns = <44>;
-			gpmc,cs-wr-off-ns = <44>;
-			gpmc,adv-on-ns = <6>;
-			gpmc,adv-rd-off-ns = <34>;
-			gpmc,adv-wr-off-ns = <44>;
-			gpmc,we-off-ns = <40>;
-			gpmc,oe-off-ns = <54>;
-			gpmc,access-ns = <64>;
-			gpmc,rd-cycle-ns = <82>;
-			gpmc,wr-cycle-ns = <82>;
-			gpmc,wr-access-ns = <40>;
-			gpmc,wr-data-mux-bus-ns = <0>;
-
-			#address-cells = <1>;
-			#size-cells = <1>;
-
-			/* partitions go here */
-		};
-	};
-
-How to select correct ECC scheme for your device ?
---------------------------------------------------
-Higher ECC scheme usually means better protection against bit-flips and
-increased system lifetime. However, selection of ECC scheme is dependent
-on various other factors also like;
-
-(1) support of built in hardware engines.
-	Some legacy OMAP SoC do not have ELM harware engine, so those SoC cannot
-	support ecc-schemes with hardware error-correction (BCHx_HW). However
-	such SoC can use ecc-schemes with software library for error-correction
-	(BCHx_HW_DETECTION_SW). The error correction capability with software
-	library remains equivalent to their hardware counter-part, but there is
-	slight CPU penalty when too many bit-flips are detected during reads.
-
-(2) Device parameters like OOBSIZE.
-	Other factor which governs the selection of ecc-scheme is oob-size.
-	Higher ECC schemes require more OOB/Spare area to store ECC syndrome,
-	so the device should have enough free bytes available its OOB/Spare
-	area to accommodate ECC for entire page. In general following expression
-	helps in determining if given device can accommodate ECC syndrome:
-	"2 + (PAGESIZE / 512) * ECC_BYTES" >= OOBSIZE"
-	where
-		OOBSIZE		number of bytes in OOB/spare area
-		PAGESIZE	number of bytes in main-area of device page
-		ECC_BYTES	number of ECC bytes generated to protect
-		                512 bytes of data, which is:
-				'3' for HAM1_xx ecc schemes
-				'7' for BCH4_xx ecc schemes
-				'14' for BCH8_xx ecc schemes
-				'26' for BCH16_xx ecc schemes
-
-	Example(a): For a device with PAGESIZE = 2048 and OOBSIZE = 64 and
-		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
-		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
-		which is greater than capacity of NAND device (OOBSIZE=64)
-		Hence, BCH16 cannot be supported on given device. But it can
-		probably use lower ecc-schemes like BCH8.
-
-	Example(b): For a device with PAGESIZE = 2048 and OOBSIZE = 128 and
-		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
-		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
-		which can be accommodated in the OOB/Spare area of this device
-		(OOBSIZE=128). So this device can use BCH16 ecc-scheme.
diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
new file mode 100644
index 000000000000..db36f2e944ef
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/ti,gpmc-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments GPMC NAND Flash controller.
+
+maintainers:
+  - Tony Lindgren <tony@atomide.com>
+  - Roger Quadros <rogerq@kernel.org>
+
+description:
+  GPMC NAND controller/Flash is represented as a child of the
+  GPMC controller node.
+
+properties:
+  compatible:
+    const: ti,omap2-nand
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Interrupt for fifoevent
+      - description: Interrupt for termcount
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  ti,nand-ecc-opt:
+    description: Desired ECC algorithm
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [sw, ham1, bch4, bch8, bch16]
+
+  ti,nand-xfer-type:
+    description: Data transfer method between controller and chip.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [prefetch-polled, polled, prefetch-dma, prefetch-irq]
+    default: prefetch-polled
+
+  ti,elm-id:
+    description:
+      phandle to the ELM (Error Location Module).
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  nand-bus-width:
+    description:
+      Bus width to the NAND chip
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [8, 16]
+    default: 8
+
+allOf:
+  - $ref: "../memory-controllers/ti,gpmc-child.yaml"
+
+required:
+  - compatible
+  - reg
+  - ti,nand-ecc-opt
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    gpmc: memory-controller@50000000 {
+      compatible = "ti,am3352-gpmc";
+      dmas = <&edma 52 0>;
+      dma-names = "rxtx";
+      clocks = <&l3s_gclk>;
+      clock-names = "fck";
+      reg = <0x50000000 0x2000>;
+      interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+      gpmc,num-cs = <7>;
+      gpmc,num-waitpins = <2>;
+      #address-cells = <2>;
+      #size-cells = <1>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      gpio-controller;
+      #gpio-cells = <2>;
+
+      ranges = <0 0 0x08000000 0x01000000>;   /* CS0 space. Min partition = 16MB */
+      nand@0,0 {
+        compatible = "ti,omap2-nand";
+        reg = <0 0 4>;          /* device IO registers */
+        interrupt-parent = <&gpmc>;
+        interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
+                     <1 IRQ_TYPE_NONE>; /* termcount */
+        ti,nand-xfer-type = "prefetch-dma";
+        ti,nand-ecc-opt = "bch16";
+        ti,elm-id = <&elm>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        /* NAND generic properties */
+        nand-bus-width = <8>;
+        rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>;  /* gpmc_wait0 */
+
+        /* GPMC properties*/
+        gpmc,device-width = <1>;
+      };
+    };
-- 
2.17.1


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

* [PATCH v3 6/8] dt-bindings: mtd: ti,gpmc-onenand: Convert to yaml
  2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
                   ` (4 preceding siblings ...)
  2021-09-07 11:32 ` [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml Roger Quadros
@ 2021-09-07 11:32 ` Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 7/8] dt-bindings: memory-controllers: ti,gpmc: " Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional Roger Quadros
  7 siblings, 0 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

Convert gpmc-onenand.txt to ti,gpmc-onenand.yaml.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../devicetree/bindings/mtd/gpmc-onenand.txt  | 48 -------------
 .../bindings/mtd/ti,gpmc-onenand.yaml         | 69 +++++++++++++++++++
 2 files changed, 69 insertions(+), 48 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
deleted file mode 100644
index e9f01a963a0a..000000000000
--- a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
+++ /dev/null
@@ -1,48 +0,0 @@
-Device tree bindings for GPMC connected OneNANDs
-
-GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
-the GPMC controller with a name of "onenand".
-
-All timing relevant properties as well as generic gpmc child properties are
-explained in a separate documents - please refer to
-Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
-
-Required properties:
-
- - compatible:		"ti,omap2-onenand"
- - reg:			The CS line the peripheral is connected to
- - gpmc,device-width:	Width of the ONENAND device connected to the GPMC
-			in bytes. Must be 1 or 2.
-
-Optional properties:
-
- - int-gpios:		GPIO specifier for the INT pin.
-
-For inline partition table parsing (optional):
-
- - #address-cells: should be set to 1
- - #size-cells: should be set to 1
-
-Example for an OMAP3430 board:
-
-	gpmc: gpmc@6e000000 {
-		compatible = "ti,omap3430-gpmc";
-		ti,hwmods = "gpmc";
-		reg = <0x6e000000 0x1000000>;
-		interrupts = <20>;
-		gpmc,num-cs = <8>;
-		gpmc,num-waitpins = <4>;
-		#address-cells = <2>;
-		#size-cells = <1>;
-
-		onenand@0 {
-			compatible = "ti,omap2-onenand";
-			reg = <0 0 0>; /* CS0, offset 0 */
-			gpmc,device-width = <2>;
-
-			#address-cells = <1>;
-			#size-cells = <1>;
-
-			/* partitions go here */
-		};
-	};
diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml
new file mode 100644
index 000000000000..44001364092e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/ti,gpmc-onenand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OneNAND over Texas Instruments GPMC bus.
+
+maintainers:
+  - Tony Lindgren <tony@atomide.com>
+  - Roger Quadros <rogerq@kernel.org>
+
+description:
+  GPMC connected OneNAND (found on OMAP boards) are represented
+  as child nodes of the GPMC controller.
+
+properties:
+  compatible:
+    const: ti,omap2-onenand
+
+  reg:
+    items:
+      - description: |
+          Chip Select number, register offset and size of
+          OneNAND register window.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  int-gpios:
+    description: GPIO specifier for the INT pin.
+
+allOf:
+  - $ref: "../memory-controllers/ti,gpmc-child.yaml"
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    gpmc: memory-controller@6e000000 {
+      compatible = "ti,omap3430-gpmc";
+      reg = <0x6e000000 0x02d0>;
+      interrupts = <20>;
+      gpmc,num-cs = <8>;
+      gpmc,num-waitpins = <4>;
+      clocks = <&l3s_clkctrl>;
+      clock-names = "fck";
+      #address-cells = <2>;
+      #size-cells = <1>;
+
+      ranges = <0 0 0x01000000 0x01000000>,   /* 16 MB for OneNAND */
+               <1 0 0x02000000 0x01000000>;   /* 16 MB for smc91c96 */
+
+      onenand@0,0 {
+        compatible = "ti,omap2-onenand";
+        reg = <0 0 0x20000>;    /* CS0, offset 0, IO size 128K */
+        #address-cells = <1>;
+        #size-cells = <1>;
+      };
+    };
-- 
2.17.1


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

* [PATCH v3 7/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml
  2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
                   ` (5 preceding siblings ...)
  2021-09-07 11:32 ` [PATCH v3 6/8] dt-bindings: mtd: ti,gpmc-onenand: " Roger Quadros
@ 2021-09-07 11:32 ` Roger Quadros
  2021-09-07 11:32 ` [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional Roger Quadros
  7 siblings, 0 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

Convert omap-gpmc.txt to ti,gpmc.yaml.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../bindings/memory-controllers/omap-gpmc.txt | 157 ----------------
 .../bindings/memory-controllers/ti,gpmc.yaml  | 174 ++++++++++++++++++
 2 files changed, 174 insertions(+), 157 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml

diff --git a/Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt b/Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
deleted file mode 100644
index c1359f4d48d7..000000000000
--- a/Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
+++ /dev/null
@@ -1,157 +0,0 @@
-Device tree bindings for OMAP general purpose memory controllers (GPMC)
-
-The actual devices are instantiated from the child nodes of a GPMC node.
-
-Required properties:
-
- - compatible:		Should be set to one of the following:
-
-			ti,omap2420-gpmc (omap2420)
-			ti,omap2430-gpmc (omap2430)
-			ti,omap3430-gpmc (omap3430 & omap3630)
-			ti,omap4430-gpmc (omap4430 & omap4460 & omap543x)
-			ti,am3352-gpmc   (am335x devices)
-
- - reg:			A resource specifier for the register space
-			(see the example below)
- - ti,hwmods:		Should be set to "ti,gpmc" until the DT transition is
-			completed.
- - #address-cells:	Must be set to 2 to allow memory address translation
- - #size-cells:		Must be set to 1 to allow CS address passing
- - gpmc,num-cs:		The maximum number of chip-select lines that controller
-			can support.
- - gpmc,num-waitpins:	The maximum number of wait pins that controller can
-			support.
- - ranges:		Must be set up to reflect the memory layout with four
-			integer values for each chip-select line in use:
-
-			   <cs-number> 0 <physical address of mapping> <size>
-
-			Currently, calculated values derived from the contents
-			of the per-CS register GPMC_CONFIG7 (as set up by the
-			bootloader) are used for the physical address decoding.
-			As this will change in the future, filling correct
-			values here is a requirement.
- - interrupt-controller: The GPMC driver implements and interrupt controller for
-			the NAND events "fifoevent" and "termcount" plus the
-			rising/falling edges on the GPMC_WAIT pins.
-			The interrupt number mapping is as follows
-			0 - NAND_fifoevent
-			1 - NAND_termcount
-			2 - GPMC_WAIT0 pin edge
-			3 - GPMC_WAIT1 pin edge, and so on.
- - interrupt-cells:	Must be set to 2
- - gpio-controller:	The GPMC driver implements a GPIO controller for the
-			GPMC WAIT pins that can be used as general purpose inputs.
-			0 maps to GPMC_WAIT0 pin.
- - gpio-cells:		Must be set to 2
-
-Required properties when using NAND prefetch dma:
- - dmas			GPMC NAND prefetch dma channel
- - dma-names		Must be set to "rxtx"
-
-Timing properties for child nodes. All are optional and default to 0.
-
- - gpmc,sync-clk-ps:	Minimum clock period for synchronous mode, in picoseconds
-
- Chip-select signal timings (in nanoseconds) corresponding to GPMC_CONFIG2:
- - gpmc,cs-on-ns:	Assertion time
- - gpmc,cs-rd-off-ns:	Read deassertion time
- - gpmc,cs-wr-off-ns:	Write deassertion time
-
- ADV signal timings (in nanoseconds) corresponding to GPMC_CONFIG3:
- - gpmc,adv-on-ns:	Assertion time
- - gpmc,adv-rd-off-ns:	Read deassertion time
- - gpmc,adv-wr-off-ns:	Write deassertion time
- - gpmc,adv-aad-mux-on-ns:	Assertion time for AAD
- - gpmc,adv-aad-mux-rd-off-ns:	Read deassertion time for AAD
- - gpmc,adv-aad-mux-wr-off-ns:	Write deassertion time for AAD
-
- WE signals timings (in nanoseconds) corresponding to GPMC_CONFIG4:
- - gpmc,we-on-ns	Assertion time
- - gpmc,we-off-ns:	Deassertion time
-
- OE signals timings (in nanoseconds) corresponding to GPMC_CONFIG4:
- - gpmc,oe-on-ns:	Assertion time
- - gpmc,oe-off-ns:	Deassertion time
- - gpmc,oe-aad-mux-on-ns:	Assertion time for AAD
- - gpmc,oe-aad-mux-off-ns:	Deassertion time for AAD
-
- Access time and cycle time timings (in nanoseconds) corresponding to
- GPMC_CONFIG5:
- - gpmc,page-burst-access-ns: 	Multiple access word delay
- - gpmc,access-ns:		Start-cycle to first data valid delay
- - gpmc,rd-cycle-ns:		Total read cycle time
- - gpmc,wr-cycle-ns:		Total write cycle time
- - gpmc,bus-turnaround-ns:	Turn-around time between successive accesses
- - gpmc,cycle2cycle-delay-ns:	Delay between chip-select pulses
- - gpmc,clk-activation-ns: 	GPMC clock activation time
- - gpmc,wait-monitoring-ns:	Start of wait monitoring with regard to valid
-				data
-
-Boolean timing parameters. If property is present parameter enabled and
-disabled if omitted:
- - gpmc,adv-extra-delay:	ADV signal is delayed by half GPMC clock
- - gpmc,cs-extra-delay:		CS signal is delayed by half GPMC clock
- - gpmc,cycle2cycle-diffcsen:	Add "cycle2cycle-delay" between successive
-				accesses to a different CS
- - gpmc,cycle2cycle-samecsen:	Add "cycle2cycle-delay" between successive
-				accesses to the same CS
- - gpmc,oe-extra-delay:		OE signal is delayed by half GPMC clock
- - gpmc,we-extra-delay:		WE signal is delayed by half GPMC clock
- - gpmc,time-para-granularity:	Multiply all access times by 2
-
-The following are only applicable to OMAP3+ and AM335x:
- - gpmc,wr-access-ns:		In synchronous write mode, for single or
-				burst accesses, defines the number of
-				GPMC_FCLK cycles from start access time
-				to the GPMC_CLK rising edge used by the
-				memory device for the first data capture.
- - gpmc,wr-data-mux-bus-ns:	In address-data multiplex mode, specifies
-				the time when the first data is driven on
-				the address-data bus.
-
-GPMC chip-select settings properties for child nodes. All are optional.
-
-- gpmc,burst-length	Page/burst length. Must be 4, 8 or 16.
-- gpmc,burst-wrap	Enables wrap bursting
-- gpmc,burst-read	Enables read page/burst mode
-- gpmc,burst-write	Enables write page/burst mode
-- gpmc,device-width	Total width of device(s) connected to a GPMC
-			chip-select in bytes. The GPMC supports 8-bit
-			and 16-bit devices and so this property must be
-			1 or 2.
-- gpmc,mux-add-data	Address and data multiplexing configuration.
-			Valid values are 1 for address-address-data
-			multiplexing mode and 2 for address-data
-			multiplexing mode.
-- gpmc,sync-read	Enables synchronous read. Defaults to asynchronous
-			is this is not set.
-- gpmc,sync-write	Enables synchronous writes. Defaults to asynchronous
-			is this is not set.
-- gpmc,wait-pin		Wait-pin used by client. Must be less than
-			"gpmc,num-waitpins".
-- gpmc,wait-on-read	Enables wait monitoring on reads.
-- gpmc,wait-on-write	Enables wait monitoring on writes.
-
-Example for an AM33xx board:
-
-	gpmc: gpmc@50000000 {
-		compatible = "ti,am3352-gpmc";
-		ti,hwmods = "gpmc";
-		reg = <0x50000000 0x2000>;
-		interrupts = <100>;
-		dmas = <&edma 52 0>;
-		dma-names = "rxtx";
-		gpmc,num-cs = <8>;
-		gpmc,num-waitpins = <2>;
-		#address-cells = <2>;
-		#size-cells = <1>;
-		ranges = <0 0 0x08000000 0x10000000>; /* CS0 @addr 0x8000000, size 0x10000000 */
-		interrupt-controller;
-		#interrupt-cells = <2>;
-		gpio-controller;
-		#gpio-cells = <2>;
-
-		/* child nodes go here */
-	};
diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
new file mode 100644
index 000000000000..3062e8cfdf89
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
@@ -0,0 +1,174 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/ti,gpmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments GPMC Memory Controller device-tree bindings
+
+maintainers:
+  - Tony Lindgren <tony@atomide.com>
+  - Roger Quadros <rogerq@kernel.org>
+
+description:
+  The GPMC is a unified memory controller dedicated for interfacing
+  with external memory devices like
+  - Asynchronous SRAM-like memories and ASICs
+  - Asynchronous, synchronous, and page mode burst NOR flash
+  - NAND flash
+  - Pseudo-SRAM devices
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - ti,omap2420-gpmc
+          - ti,omap2430-gpmc
+          - ti,omap3430-gpmc
+          - ti,omap4430-gpmc
+          - ti,am3352-gpmc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: |
+      Functional clock. Used for bus timing calculations and
+      GPMC configuration.
+
+  clock-names:
+    items:
+      - const: fck
+
+  dmas:
+    items:
+      - description: DMA channel for GPMC NAND prefetch
+
+  dma-names:
+    items:
+      - const: rxtx
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 1
+
+  gpmc,num-cs:
+    description: maximum number of supported chip-select lines.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  gpmc,num-waitpins:
+    description: maximum number of supported wait pins.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  ranges:
+    minItems: 1
+    description: |
+      Must be set up to reflect the memory layout with four
+      integer values for each chip-select line in use,
+      <cs-number> 0 <physical address of mapping> <size>
+    items:
+      - description: NAND bank 0
+      - description: NOR/SRAM bank 0
+      - description: NOR/SRAM bank 1
+
+  '#interrupt-cells':
+    const: 2
+
+  interrupt-controller:
+    description: |
+      The GPMC driver implements and interrupt controller for
+      the NAND events "fifoevent" and "termcount" plus the
+      rising/falling edges on the GPMC_WAIT pins.
+      The interrupt number mapping is as follows
+      0 - NAND_fifoevent
+      1 - NAND_termcount
+      2 - GPMC_WAIT0 pin edge
+      3 - GPMC_WAIT1 pin edge, and so on.
+
+  '#gpio-cells':
+    const: 2
+
+  gpio-controller:
+    description: |
+      The GPMC driver implements a GPIO controller for the
+      GPMC WAIT pins that can be used as general purpose inputs.
+      0 maps to GPMC_WAIT0 pin.
+
+  ti,hwmods:
+    description:
+      Name of the HWMOD associated with GPMC. This is for legacy
+      omap2/3 platforms only.
+    $ref: /schemas/types.yaml#/definitions/string
+    deprecated: true
+
+  ti,no-idle-on-init:
+    description:
+      Prevent idling the module at init. This is for legacy omap2/3
+      platforms only.
+    type: boolean
+    deprecated: true
+
+patternProperties:
+  "@[0-7],[a-f0-9]+$":
+    type: object
+    description: |
+      The child device node represents the device connected to the GPMC
+      bus. The device can be a NAND chip, SRAM device, NOR device
+      or an ASIC.
+
+    allOf:
+      - $ref: "ti,gpmc-child.yaml"
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - gpmc,num-cs
+  - gpmc,num-waitpins
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    gpmc: memory-controller@50000000 {
+      compatible = "ti,am3352-gpmc";
+      reg = <0x50000000 0x2000>;
+      interrupts = <100>;
+      clocks = <&l3s_clkctrl>;
+      clock-names = "fck";
+      dmas = <&edma 52 0>;
+      dma-names = "rxtx";
+      gpmc,num-cs = <8>;
+      gpmc,num-waitpins = <2>;
+      #address-cells = <2>;
+      #size-cells = <1>;
+      ranges = <0 0 0x08000000 0x10000000>; /* CS0 @addr 0x8000000, size 0x10000000 */
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      gpio-controller;
+      #gpio-cells = <2>;
+
+      nand@0,0 {
+        compatible = "ti,omap2-nand";
+        reg = <0 0 4>;
+        interrupt-parent = <&gpmc>;
+        interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
+                     <1 IRQ_TYPE_NONE>; /* termcount */
+        ti,nand-xfer-type = "prefetch-dma";
+        ti,nand-ecc-opt = "bch16";
+        ti,elm-id = <&elm>;
+        rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 pin */
+      };
+    };
-- 
2.17.1


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

* [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional
  2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
                   ` (6 preceding siblings ...)
  2021-09-07 11:32 ` [PATCH v3 7/8] dt-bindings: memory-controllers: ti,gpmc: " Roger Quadros
@ 2021-09-07 11:32 ` Roger Quadros
  2021-09-07 12:36   ` Krzysztof Kozlowski
  7 siblings, 1 reply; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 11:32 UTC (permalink / raw)
  To: tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel, Roger Quadros

Check for valid gpmc,device-width, nand-bus-width and bank-width
at one place. Default to 8-bit width if none present.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 drivers/memory/omap-gpmc.c | 41 ++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index f80c2ea39ca4..32d7c665f33c 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -2171,10 +2171,8 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 		}
 	}
 
-	if (of_device_is_compatible(child, "ti,omap2-nand")) {
-		/* NAND specific setup */
-		val = 8;
-		of_property_read_u32(child, "nand-bus-width", &val);
+	/* DT node can have "nand-bus-width" or "bank-width" or "gpmc,device-width" */
+	if (!of_property_read_u32(child, "nand-bus-width", &val)) {
 		switch (val) {
 		case 8:
 			gpmc_s.device_width = GPMC_DEVWIDTH_8BIT;
@@ -2183,24 +2181,37 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 			gpmc_s.device_width = GPMC_DEVWIDTH_16BIT;
 			break;
 		default:
-			dev_err(&pdev->dev, "%pOFn: invalid 'nand-bus-width'\n",
-				child);
+			dev_err(&pdev->dev,
+				"%pOFn: invalid 'nand-bus-width':%d\n", child, val);
+			ret = -EINVAL;
+			goto err;
+		}
+	} else if (!of_property_read_u32(child, "bank-width", &val)) {
+		if (val != 1 && val != 2) {
+			dev_err(&pdev->dev,
+				"%pOFn: invalid 'bank-width':%d\n", child, val);
 			ret = -EINVAL;
 			goto err;
 		}
+		gpmc_s.device_width = val;
+	} else if (!of_property_read_u32(child, "gpmc,device-width", &val)) {
+		if (val != 1 && val != 2) {
+			dev_err(&pdev->dev,
+				"%pOFn: invalid 'gpmc,device-width':%d\n", child, val);
+			ret = -EINVAL;
+			goto err;
+		}
+		gpmc_s.device_width = val;
+	} else {
+		/* default to 8-bit */
+		gpmc_s.device_width = GPMC_DEVWIDTH_8BIT;
+	}
 
+	if (of_device_is_compatible(child, "ti,omap2-nand")) {
+		/* NAND specific setup */
 		/* disable write protect */
 		gpmc_configure(GPMC_CONFIG_WP, 0);
 		gpmc_s.device_nand = true;
-	} else {
-		ret = of_property_read_u32(child, "bank-width",
-					   &gpmc_s.device_width);
-		if (ret < 0 && !gpmc_s.device_width) {
-			dev_err(&pdev->dev,
-				"%pOF has no 'gpmc,device-width' property\n",
-				child);
-			goto err;
-		}
 	}
 
 	/* Reserve wait pin if it is required and valid */
-- 
2.17.1


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

* Re: [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional
  2021-09-07 11:32 ` [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional Roger Quadros
@ 2021-09-07 12:36   ` Krzysztof Kozlowski
  2021-09-15  9:11     ` Roger Quadros
  0 siblings, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2021-09-07 12:36 UTC (permalink / raw)
  To: Roger Quadros, tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	miquel.raynal, devicetree, linux-mtd, linux-omap, linux-kernel

On 07/09/2021 13:32, Roger Quadros wrote:
> Check for valid gpmc,device-width, nand-bus-width and bank-width
> at one place. Default to 8-bit width if none present.

I don't understand the message in the context of the patch. The title
says one property is optional - that's it. The message says you
consolidate checks. How is this related to the title?

The patch itself moves around checking of properties and reads
nand-bus-width *always*. It does not "check at one place" but rather
"check always". In the same time, the patch does not remove
gpmc,device-width check in other place.

All three elements - the title, message and patch - do different things.
What did you want to achieve here? Can you help in clarifying it?


Best regards,
Krzysztof


> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  drivers/memory/omap-gpmc.c | 41 ++++++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index f80c2ea39ca4..32d7c665f33c 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -2171,10 +2171,8 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>  		}
>  	}
>  
> -	if (of_device_is_compatible(child, "ti,omap2-nand")) {
> -		/* NAND specific setup */
> -		val = 8;
> -		of_property_read_u32(child, "nand-bus-width", &val);
> +	/* DT node can have "nand-bus-width" or "bank-width" or "gpmc,device-width" */
> +	if (!of_property_read_u32(child, "nand-bus-width", &val)) {
>  		switch (val) {
>  		case 8:
>  			gpmc_s.device_width = GPMC_DEVWIDTH_8BIT;
> @@ -2183,24 +2181,37 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>  			gpmc_s.device_width = GPMC_DEVWIDTH_16BIT;
>  			break;
>  		default:
> -			dev_err(&pdev->dev, "%pOFn: invalid 'nand-bus-width'\n",
> -				child);
> +			dev_err(&pdev->dev,
> +				"%pOFn: invalid 'nand-bus-width':%d\n", child, val);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +	} else if (!of_property_read_u32(child, "bank-width", &val)) {
> +		if (val != 1 && val != 2) {
> +			dev_err(&pdev->dev,
> +				"%pOFn: invalid 'bank-width':%d\n", child, val);
>  			ret = -EINVAL;
>  			goto err;
>  		}
> +		gpmc_s.device_width = val;
> +	} else if (!of_property_read_u32(child, "gpmc,device-width", &val)) {
> +		if (val != 1 && val != 2) {
> +			dev_err(&pdev->dev,
> +				"%pOFn: invalid 'gpmc,device-width':%d\n", child, val);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +		gpmc_s.device_width = val;
> +	} else {
> +		/* default to 8-bit */
> +		gpmc_s.device_width = GPMC_DEVWIDTH_8BIT;
> +	}
>  
> +	if (of_device_is_compatible(child, "ti,omap2-nand")) {
> +		/* NAND specific setup */
>  		/* disable write protect */
>  		gpmc_configure(GPMC_CONFIG_WP, 0);
>  		gpmc_s.device_nand = true;
> -	} else {
> -		ret = of_property_read_u32(child, "bank-width",
> -					   &gpmc_s.device_width);
> -		if (ret < 0 && !gpmc_s.device_width) {
> -			dev_err(&pdev->dev,
> -				"%pOF has no 'gpmc,device-width' property\n",
> -				child);
> -			goto err;
> -		}
>  	}
>  
>  	/* Reserve wait pin if it is required and valid */
> 



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

* Re: [PATCH v3 1/8] ARM: dts: omap: Fixup GPMC child nodes
  2021-09-07 11:32 ` [PATCH v3 1/8] ARM: dts: omap: Fixup GPMC child nodes Roger Quadros
@ 2021-09-07 12:44   ` Krzysztof Kozlowski
  2021-09-15  8:53     ` Roger Quadros
  0 siblings, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2021-09-07 12:44 UTC (permalink / raw)
  To: Roger Quadros, tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	miquel.raynal, devicetree, linux-mtd, linux-omap, linux-kernel

On 07/09/2021 13:32, Roger Quadros wrote:
> Fixes up the GPMC child nodes to prevent dtbs_check errors
> after device tree binding conversion to yaml.
> 
> - Use reg address as node name
> - gpmc,cycle2cycle-samecsen and gpmc,cycle2cycle-diffcsen are
>   boolean properties.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  .../boot/dts/logicpd-som-lv-baseboard.dtsi    |  2 +-
>  .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |  2 +-
>  .../boot/dts/logicpd-torpedo-baseboard.dtsi   |  2 +-
>  arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi     | 62 +++++++++----------
>  arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi     | 59 +++++++++---------
>  arch/arm/boot/dts/omap-zoom-common.dtsi       | 16 ++---
>  arch/arm/boot/dts/omap2430-sdp.dts            |  6 +-
>  arch/arm/boot/dts/omap3-cm-t3x30.dtsi         |  6 +-
>  .../arm/boot/dts/omap3-devkit8000-common.dtsi |  4 +-
>  arch/arm/boot/dts/omap3-evm-37xx.dts          |  1 +
>  arch/arm/boot/dts/omap3-evm-common.dtsi       |  9 ---
>  .../boot/dts/omap3-evm-processor-common.dtsi  |  5 +-
>  arch/arm/boot/dts/omap3-evm.dts               |  1 +
>  arch/arm/boot/dts/omap3-igep0020-common.dtsi  |  5 +-
>  arch/arm/boot/dts/omap3-ldp.dts               |  5 +-
>  arch/arm/boot/dts/omap3-n900.dts              |  2 +-
>  .../dts/omap3-overo-chestnut43-common.dtsi    |  6 +-
>  .../arm/boot/dts/omap3-overo-tobi-common.dtsi |  6 +-
>  .../boot/dts/omap3-overo-tobiduo-common.dtsi  |  8 +--
>  arch/arm/boot/dts/omap3-sb-t35.dtsi           |  4 +-
>  arch/arm/boot/dts/omap4-duovero-parlor.dts    |  6 +-
>  21 files changed, 105 insertions(+), 112 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi b/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
> index 7d0468a23781..f2364cb114c5 100644
> --- a/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
> +++ b/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
> @@ -65,7 +65,7 @@
>  		  1 0 0x2c000000 0x1000000	/* CS1: 16MB for LAN9221 */
>  		  2 0 0x10000000 0x2000000>;    /* CS2: 32MB for NOR */
>  
> -	ethernet@gpmc {
> +	gpmc_ethernet: ethernet@1,0 {
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&lan9221_pins>;
>  		interrupt-parent = <&gpio5>;
> diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
> index 5532db04046c..6357915d207b 100644
> --- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
> +++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
> @@ -4,8 +4,8 @@
>  
>  #include "omap36xx.dtsi"
>  #include "logicpd-torpedo-som.dtsi"
> -#include "omap-gpmc-smsc9221.dtsi"
>  #include "logicpd-torpedo-baseboard.dtsi"
> +#include "omap-gpmc-smsc9221.dtsi"
>  
>  / {
>  	model = "LogicPD Zoom DM3730 Torpedo + Wireless Development Kit";
> diff --git a/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi b/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
> index 533a47bc4a53..05049a34b6f1 100644
> --- a/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
> +++ b/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
> @@ -95,7 +95,7 @@
>  	ranges = <0 0 0x30000000 0x1000000	/* CS0: 16MB for NAND */
>  		  1 0 0x2c000000 0x1000000>;	/* CS1: 16MB for LAN9221 */
>  
> -	ethernet@gpmc {
> +	gpmc_ethernet: ethernet@1,0 {
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&lan9221_pins>;
>  		interrupt-parent = <&gpio5>;
> diff --git a/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi b/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
> index ded7e8fec9eb..2a462cb65a7d 100644
> --- a/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
> +++ b/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
> @@ -20,36 +20,34 @@
>  	};
>  };
>  
> -&gpmc {
> -	ethernet@gpmc {
> -		compatible = "smsc,lan9221", "smsc,lan9115";
> -		bank-width = <2>;
> -		gpmc,device-width = <1>;
> -		gpmc,cycle2cycle-samecsen = <1>;
> -		gpmc,cycle2cycle-diffcsen = <1>;
> -		gpmc,cs-on-ns = <5>;
> -		gpmc,cs-rd-off-ns = <150>;
> -		gpmc,cs-wr-off-ns = <150>;
> -		gpmc,adv-on-ns = <0>;
> -		gpmc,adv-rd-off-ns = <15>;
> -		gpmc,adv-wr-off-ns = <40>;
> -		gpmc,oe-on-ns = <45>;
> -		gpmc,oe-off-ns = <140>;
> -		gpmc,we-on-ns = <45>;
> -		gpmc,we-off-ns = <140>;
> -		gpmc,rd-cycle-ns = <155>;
> -		gpmc,wr-cycle-ns = <155>;
> -		gpmc,access-ns = <120>;
> -		gpmc,page-burst-access-ns = <20>;
> -		gpmc,bus-turnaround-ns = <75>;
> -		gpmc,cycle2cycle-delay-ns = <75>;
> -		gpmc,wait-monitoring-ns = <0>;
> -		gpmc,clk-activation-ns = <0>;
> -		gpmc,wr-data-mux-bus-ns = <0>;
> -		gpmc,wr-access-ns = <0>;
> -		vddvario-supply = <&vddvario>;
> -		vdd33a-supply = <&vdd33a>;
> -		reg-io-width = <4>;
> -		smsc,save-mac-address;
> -	};
> +&gpmc_ethernet {
> +	compatible = "smsc,lan9221", "smsc,lan9115";

This looks like regular override-by-label instead of full path.
Unfortunately change of the indentation causes difficulties to find the
real difference - if there is such. Can you split it into separate patch?

The point is that override-by-label should have zero effect on
functionality and produce same dtb. This is easy to compare with
dtx_diff or fdt-decompile but if you mix it with other changes, the
comparison is not straight-forward.

> +	bank-width = <2>;
> +	gpmc,device-width = <1>;
> +	gpmc,cycle2cycle-samecsen;
> +	gpmc,cycle2cycle-diffcsen;
> +	gpmc,cs-on-ns = <5>;
> +	gpmc,cs-rd-off-ns = <150>;
> +	gpmc,cs-wr-off-ns = <150>;
> +	gpmc,adv-on-ns = <0>;
> +	gpmc,adv-rd-off-ns = <15>;
> +	gpmc,adv-wr-off-ns = <40>;
> +	gpmc,oe-on-ns = <45>;
> +	gpmc,oe-off-ns = <140>;
> +	gpmc,we-on-ns = <45>;
> +	gpmc,we-off-ns = <140>;
> +	gpmc,rd-cycle-ns = <155>;
> +	gpmc,wr-cycle-ns = <155>;
> +	gpmc,access-ns = <120>;
> +	gpmc,page-burst-access-ns = <20>;
> +	gpmc,bus-turnaround-ns = <75>;
> +	gpmc,cycle2cycle-delay-ns = <75>;
> +	gpmc,wait-monitoring-ns = <0>;
> +	gpmc,clk-activation-ns = <0>;
> +	gpmc,wr-data-mux-bus-ns = <0>;
> +	gpmc,wr-access-ns = <0>;
> +	vddvario-supply = <&vddvario>;
> +	vdd33a-supply = <&vdd33a>;
> +	reg-io-width = <4>;
> +	smsc,save-mac-address;
>  };
> diff --git a/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi b/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
> index 7f6aefd13451..c1e78f929d2b 100644
> --- a/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
> +++ b/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
> @@ -24,36 +24,33 @@
>  	};
>  };
>  
> -&gpmc {
> -	ethernet@gpmc {
> -		compatible = "smsc,lan9221","smsc,lan9115";
> -		bank-width = <2>;
> +&gpmc_ethernet {
> +	compatible = "smsc,lan9221","smsc,lan9115";
> +	bank-width = <2>;
> +	gpmc,mux-add-data = <0>;
> +	gpmc,cs-on-ns = <0>;
> +	gpmc,cs-rd-off-ns = <42>;
> +	gpmc,cs-wr-off-ns = <36>;
> +	gpmc,adv-on-ns = <6>;
> +	gpmc,adv-rd-off-ns = <12>;
> +	gpmc,adv-wr-off-ns = <12>;
> +	gpmc,oe-on-ns = <0>;
> +	gpmc,oe-off-ns = <42>;
> +	gpmc,we-on-ns = <0>;
> +	gpmc,we-off-ns = <36>;
> +	gpmc,rd-cycle-ns = <60>;
> +	gpmc,wr-cycle-ns = <54>;
> +	gpmc,access-ns = <36>;
> +	gpmc,page-burst-access-ns = <0>;
> +	gpmc,bus-turnaround-ns = <0>;
> +	gpmc,cycle2cycle-delay-ns = <0>;
> +	gpmc,wr-data-mux-bus-ns = <18>;
> +	gpmc,wr-access-ns = <42>;
> +	gpmc,cycle2cycle-samecsen;
> +	gpmc,cycle2cycle-diffcsen;
>  
> -		gpmc,mux-add-data;

Same here and in other places. Actually here a sneaky change is visible
- different property.

Best regards,
Krzysztof

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 11:32 ` [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml Roger Quadros
@ 2021-09-07 14:03   ` Miquel Raynal
  2021-09-07 15:27     ` Grygorii Strashko
  2021-09-07 17:01   ` Rob Herring
  1 sibling, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2021-09-07 14:03 UTC (permalink / raw)
  To: Roger Quadros
  Cc: tony, robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, devicetree, linux-mtd, linux-omap,
	linux-kernel

Hi Roger,

rogerq@kernel.org wrote on Tue,  7 Sep 2021 14:32:23 +0300:

> Convert gpmc-nand.txt to ti,gpmc-nand.yaml.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  .../devicetree/bindings/mtd/gpmc-nand.txt     | 147 ------------------
>  .../devicetree/bindings/mtd/ti,gpmc-nand.yaml | 110 +++++++++++++
>  2 files changed, 110 insertions(+), 147 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>  create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> deleted file mode 100644
> index 44919d48d241..000000000000
> --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> +++ /dev/null
> @@ -1,147 +0,0 @@
> -Device tree bindings for GPMC connected NANDs
> -
> -GPMC connected NAND (found on OMAP boards) are represented as child nodes of
> -the GPMC controller with a name of "nand".
> -
> -All timing relevant properties as well as generic gpmc child properties are
> -explained in a separate documents - please refer to
> -Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
> -
> -For NAND specific properties such as ECC modes or bus width, please refer to
> -Documentation/devicetree/bindings/mtd/nand-controller.yaml
> -
> -
> -Required properties:
> -
> - - compatible:	"ti,omap2-nand"
> - - reg:		range id (CS number), base offset and length of the
> -		NAND I/O space
> - - interrupts:	Two interrupt specifiers, one for fifoevent, one for termcount.
> -
> -Optional properties:
> -
> - - nand-bus-width: 		Set this numeric value to 16 if the hardware
> -				is wired that way. If not specified, a bus
> -				width of 8 is assumed.
> -
> - - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
> -		"sw"		1-bit Hamming ecc code via software
> -		"hw"		<deprecated> use "ham1" instead
> -		"hw-romcode"	<deprecated> use "ham1" instead
> -		"ham1"		1-bit Hamming ecc code
> -		"bch4"		4-bit BCH ecc code
> -		"bch8"		8-bit BCH ecc code
> -		"bch16"		16-bit BCH ECC code
> -		Refer below "How to select correct ECC scheme for your device ?"
> -
> - - ti,nand-xfer-type:		A string setting the data transfer type. One of:
> -
> -		"prefetch-polled"	Prefetch polled mode (default)
> -		"polled"		Polled mode, without prefetch
> -		"prefetch-dma"		Prefetch enabled DMA mode
> -		"prefetch-irq"		Prefetch enabled irq mode
> -
> - - elm_id:	<deprecated> use "ti,elm-id" instead
> - - ti,elm-id:	Specifies phandle of the ELM devicetree node.
> -		ELM is an on-chip hardware engine on TI SoC which is used for
> -		locating ECC errors for BCHx algorithms. SoC devices which have
> -		ELM hardware engines should specify this device node in .dtsi
> -		Using ELM for ECC error correction frees some CPU cycles.
> - - rb-gpios:	GPIO specifier for the ready/busy# pin.
> -
> -For inline partition table parsing (optional):
> -
> - - #address-cells: should be set to 1
> - - #size-cells: should be set to 1
> -
> -Example for an AM33xx board:
> -
> -	gpmc: gpmc@50000000 {
> -		compatible = "ti,am3352-gpmc";
> -		ti,hwmods = "gpmc";
> -		reg = <0x50000000 0x36c>;
> -		interrupts = <100>;
> -		gpmc,num-cs = <8>;
> -		gpmc,num-waitpins = <2>;
> -		#address-cells = <2>;
> -		#size-cells = <1>;
> -		ranges = <0 0 0x08000000 0x1000000>;	/* CS0 space, 16MB */
> -		elm_id = <&elm>;
> -		interrupt-controller;
> -		#interrupt-cells = <2>;
> -
> -		nand@0,0 {
> -			compatible = "ti,omap2-nand";
> -			reg = <0 0 4>;		/* CS0, offset 0, NAND I/O window 4 */
> -			interrupt-parent = <&gpmc>;
> -			interrupts = <0 IRQ_TYPE_NONE>, <1 IRQ_TYPE NONE>;
> -			nand-bus-width = <16>;
> -			ti,nand-ecc-opt = "bch8";
> -			ti,nand-xfer-type = "polled";
> -			rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
> -
> -			gpmc,sync-clk-ps = <0>;
> -			gpmc,cs-on-ns = <0>;
> -			gpmc,cs-rd-off-ns = <44>;
> -			gpmc,cs-wr-off-ns = <44>;
> -			gpmc,adv-on-ns = <6>;
> -			gpmc,adv-rd-off-ns = <34>;
> -			gpmc,adv-wr-off-ns = <44>;
> -			gpmc,we-off-ns = <40>;
> -			gpmc,oe-off-ns = <54>;
> -			gpmc,access-ns = <64>;
> -			gpmc,rd-cycle-ns = <82>;
> -			gpmc,wr-cycle-ns = <82>;
> -			gpmc,wr-access-ns = <40>;
> -			gpmc,wr-data-mux-bus-ns = <0>;
> -
> -			#address-cells = <1>;
> -			#size-cells = <1>;
> -
> -			/* partitions go here */
> -		};
> -	};
> -
> -How to select correct ECC scheme for your device ?
> ---------------------------------------------------
> -Higher ECC scheme usually means better protection against bit-flips and
> -increased system lifetime. However, selection of ECC scheme is dependent
> -on various other factors also like;
> -
> -(1) support of built in hardware engines.
> -	Some legacy OMAP SoC do not have ELM harware engine, so those SoC cannot
> -	support ecc-schemes with hardware error-correction (BCHx_HW). However
> -	such SoC can use ecc-schemes with software library for error-correction
> -	(BCHx_HW_DETECTION_SW). The error correction capability with software
> -	library remains equivalent to their hardware counter-part, but there is
> -	slight CPU penalty when too many bit-flips are detected during reads.
> -
> -(2) Device parameters like OOBSIZE.
> -	Other factor which governs the selection of ecc-scheme is oob-size.
> -	Higher ECC schemes require more OOB/Spare area to store ECC syndrome,
> -	so the device should have enough free bytes available its OOB/Spare
> -	area to accommodate ECC for entire page. In general following expression
> -	helps in determining if given device can accommodate ECC syndrome:
> -	"2 + (PAGESIZE / 512) * ECC_BYTES" >= OOBSIZE"
> -	where
> -		OOBSIZE		number of bytes in OOB/spare area
> -		PAGESIZE	number of bytes in main-area of device page
> -		ECC_BYTES	number of ECC bytes generated to protect
> -		                512 bytes of data, which is:
> -				'3' for HAM1_xx ecc schemes
> -				'7' for BCH4_xx ecc schemes
> -				'14' for BCH8_xx ecc schemes
> -				'26' for BCH16_xx ecc schemes
> -
> -	Example(a): For a device with PAGESIZE = 2048 and OOBSIZE = 64 and
> -		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
> -		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
> -		which is greater than capacity of NAND device (OOBSIZE=64)
> -		Hence, BCH16 cannot be supported on given device. But it can
> -		probably use lower ecc-schemes like BCH8.
> -
> -	Example(b): For a device with PAGESIZE = 2048 and OOBSIZE = 128 and
> -		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
> -		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
> -		which can be accommodated in the OOB/Spare area of this device
> -		(OOBSIZE=128). So this device can use BCH16 ecc-scheme.
> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> new file mode 100644
> index 000000000000..db36f2e944ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/ti,gpmc-nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments GPMC NAND Flash controller.
> +
> +maintainers:
> +  - Tony Lindgren <tony@atomide.com>
> +  - Roger Quadros <rogerq@kernel.org>
> +
> +description:
> +  GPMC NAND controller/Flash is represented as a child of the
> +  GPMC controller node.
> +
> +properties:
> +  compatible:
> +    const: ti,omap2-nand
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: Interrupt for fifoevent
> +      - description: Interrupt for termcount
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  ti,nand-ecc-opt:
> +    description: Desired ECC algorithm
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [sw, ham1, bch4, bch8, bch16]
> +
> +  ti,nand-xfer-type:
> +    description: Data transfer method between controller and chip.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [prefetch-polled, polled, prefetch-dma, prefetch-irq]
> +    default: prefetch-polled
> +
> +  ti,elm-id:
> +    description:
> +      phandle to the ELM (Error Location Module).
> +    $ref: /schemas/types.yaml#/definitions/phandle

Should perhaps keep the elm-id property documented but set to
'deprecated'.

> +
> +  nand-bus-width:
> +    description:
> +      Bus width to the NAND chip
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [8, 16]
> +    default: 8

This is part of nand-controller.yaml binding and should not be there.

> +
> +allOf:
> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"

Maybe you need to reference the nand controller bindings as well

> +
> +required:
> +  - compatible
> +  - reg
> +  - ti,nand-ecc-opt
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    gpmc: memory-controller@50000000 {
> +      compatible = "ti,am3352-gpmc";
> +      dmas = <&edma 52 0>;
> +      dma-names = "rxtx";
> +      clocks = <&l3s_gclk>;
> +      clock-names = "fck";
> +      reg = <0x50000000 0x2000>;
> +      interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> +      gpmc,num-cs = <7>;
> +      gpmc,num-waitpins = <2>;
> +      #address-cells = <2>;
> +      #size-cells = <1>;
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +      gpio-controller;
> +      #gpio-cells = <2>;
> +
> +      ranges = <0 0 0x08000000 0x01000000>;   /* CS0 space. Min partition = 16MB */
> +      nand@0,0 {
> +        compatible = "ti,omap2-nand";
> +        reg = <0 0 4>;          /* device IO registers */
> +        interrupt-parent = <&gpmc>;
> +        interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> +                     <1 IRQ_TYPE_NONE>; /* termcount */
> +        ti,nand-xfer-type = "prefetch-dma";
> +        ti,nand-ecc-opt = "bch16";
> +        ti,elm-id = <&elm>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        /* NAND generic properties */
> +        nand-bus-width = <8>;
> +        rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>;  /* gpmc_wait0 */
> +
> +        /* GPMC properties*/
> +        gpmc,device-width = <1>;
> +      };
> +    };


Thanks,
Miquèl

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 14:03   ` Miquel Raynal
@ 2021-09-07 15:27     ` Grygorii Strashko
  2021-09-07 16:35       ` Miquel Raynal
  0 siblings, 1 reply; 24+ messages in thread
From: Grygorii Strashko @ 2021-09-07 15:27 UTC (permalink / raw)
  To: Miquel Raynal, Roger Quadros
  Cc: tony, robh+dt, nm, lokeshvutla, nsekhar, krzysztof.kozlowski,
	devicetree, linux-mtd, linux-omap, linux-kernel



On 07/09/2021 17:03, Miquel Raynal wrote:
> Hi Roger,
> 
> rogerq@kernel.org wrote on Tue,  7 Sep 2021 14:32:23 +0300:
> 
>> Convert gpmc-nand.txt to ti,gpmc-nand.yaml.
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>   .../devicetree/bindings/mtd/gpmc-nand.txt     | 147 ------------------
>>   .../devicetree/bindings/mtd/ti,gpmc-nand.yaml | 110 +++++++++++++
>>   2 files changed, 110 insertions(+), 147 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>>   create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>> deleted file mode 100644
>> index 44919d48d241..000000000000
>> --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>> +++ /dev/null
>> @@ -1,147 +0,0 @@
>> -Device tree bindings for GPMC connected NANDs
>> -
>> -GPMC connected NAND (found on OMAP boards) are represented as child nodes of
>> -the GPMC controller with a name of "nand".
>> -
>> -All timing relevant properties as well as generic gpmc child properties are
>> -explained in a separate documents - please refer to
>> -Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>> -
>> -For NAND specific properties such as ECC modes or bus width, please refer to
>> -Documentation/devicetree/bindings/mtd/nand-controller.yaml
>> -
>> -
>> -Required properties:
>> -
>> - - compatible:	"ti,omap2-nand"
>> - - reg:		range id (CS number), base offset and length of the
>> -		NAND I/O space
>> - - interrupts:	Two interrupt specifiers, one for fifoevent, one for termcount.
>> -
>> -Optional properties:
>> -
>> - - nand-bus-width: 		Set this numeric value to 16 if the hardware
>> -				is wired that way. If not specified, a bus
>> -				width of 8 is assumed.
>> -
>> - - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
>> -		"sw"		1-bit Hamming ecc code via software
>> -		"hw"		<deprecated> use "ham1" instead
>> -		"hw-romcode"	<deprecated> use "ham1" instead
>> -		"ham1"		1-bit Hamming ecc code
>> -		"bch4"		4-bit BCH ecc code
>> -		"bch8"		8-bit BCH ecc code
>> -		"bch16"		16-bit BCH ECC code
>> -		Refer below "How to select correct ECC scheme for your device ?"
>> -
>> - - ti,nand-xfer-type:		A string setting the data transfer type. One of:
>> -
>> -		"prefetch-polled"	Prefetch polled mode (default)
>> -		"polled"		Polled mode, without prefetch
>> -		"prefetch-dma"		Prefetch enabled DMA mode
>> -		"prefetch-irq"		Prefetch enabled irq mode
>> -
>> - - elm_id:	<deprecated> use "ti,elm-id" instead
>> - - ti,elm-id:	Specifies phandle of the ELM devicetree node.
>> -		ELM is an on-chip hardware engine on TI SoC which is used for
>> -		locating ECC errors for BCHx algorithms. SoC devices which have
>> -		ELM hardware engines should specify this device node in .dtsi
>> -		Using ELM for ECC error correction frees some CPU cycles.
>> - - rb-gpios:	GPIO specifier for the ready/busy# pin.
>> -
>> -For inline partition table parsing (optional):
>> -
>> - - #address-cells: should be set to 1
>> - - #size-cells: should be set to 1
>> -
>> -Example for an AM33xx board:
>> -
>> -	gpmc: gpmc@50000000 {
>> -		compatible = "ti,am3352-gpmc";
>> -		ti,hwmods = "gpmc";
>> -		reg = <0x50000000 0x36c>;
>> -		interrupts = <100>;
>> -		gpmc,num-cs = <8>;
>> -		gpmc,num-waitpins = <2>;
>> -		#address-cells = <2>;
>> -		#size-cells = <1>;
>> -		ranges = <0 0 0x08000000 0x1000000>;	/* CS0 space, 16MB */
>> -		elm_id = <&elm>;
>> -		interrupt-controller;
>> -		#interrupt-cells = <2>;
>> -
>> -		nand@0,0 {
>> -			compatible = "ti,omap2-nand";
>> -			reg = <0 0 4>;		/* CS0, offset 0, NAND I/O window 4 */
>> -			interrupt-parent = <&gpmc>;
>> -			interrupts = <0 IRQ_TYPE_NONE>, <1 IRQ_TYPE NONE>;
>> -			nand-bus-width = <16>;
>> -			ti,nand-ecc-opt = "bch8";
>> -			ti,nand-xfer-type = "polled";
>> -			rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
>> -
>> -			gpmc,sync-clk-ps = <0>;
>> -			gpmc,cs-on-ns = <0>;
>> -			gpmc,cs-rd-off-ns = <44>;
>> -			gpmc,cs-wr-off-ns = <44>;
>> -			gpmc,adv-on-ns = <6>;
>> -			gpmc,adv-rd-off-ns = <34>;
>> -			gpmc,adv-wr-off-ns = <44>;
>> -			gpmc,we-off-ns = <40>;
>> -			gpmc,oe-off-ns = <54>;
>> -			gpmc,access-ns = <64>;
>> -			gpmc,rd-cycle-ns = <82>;
>> -			gpmc,wr-cycle-ns = <82>;
>> -			gpmc,wr-access-ns = <40>;
>> -			gpmc,wr-data-mux-bus-ns = <0>;
>> -
>> -			#address-cells = <1>;
>> -			#size-cells = <1>;
>> -
>> -			/* partitions go here */
>> -		};
>> -	};
>> -
>> -How to select correct ECC scheme for your device ?
>> ---------------------------------------------------
>> -Higher ECC scheme usually means better protection against bit-flips and
>> -increased system lifetime. However, selection of ECC scheme is dependent
>> -on various other factors also like;
>> -
>> -(1) support of built in hardware engines.
>> -	Some legacy OMAP SoC do not have ELM harware engine, so those SoC cannot
>> -	support ecc-schemes with hardware error-correction (BCHx_HW). However
>> -	such SoC can use ecc-schemes with software library for error-correction
>> -	(BCHx_HW_DETECTION_SW). The error correction capability with software
>> -	library remains equivalent to their hardware counter-part, but there is
>> -	slight CPU penalty when too many bit-flips are detected during reads.
>> -
>> -(2) Device parameters like OOBSIZE.
>> -	Other factor which governs the selection of ecc-scheme is oob-size.
>> -	Higher ECC schemes require more OOB/Spare area to store ECC syndrome,
>> -	so the device should have enough free bytes available its OOB/Spare
>> -	area to accommodate ECC for entire page. In general following expression
>> -	helps in determining if given device can accommodate ECC syndrome:
>> -	"2 + (PAGESIZE / 512) * ECC_BYTES" >= OOBSIZE"
>> -	where
>> -		OOBSIZE		number of bytes in OOB/spare area
>> -		PAGESIZE	number of bytes in main-area of device page
>> -		ECC_BYTES	number of ECC bytes generated to protect
>> -		                512 bytes of data, which is:
>> -				'3' for HAM1_xx ecc schemes
>> -				'7' for BCH4_xx ecc schemes
>> -				'14' for BCH8_xx ecc schemes
>> -				'26' for BCH16_xx ecc schemes
>> -
>> -	Example(a): For a device with PAGESIZE = 2048 and OOBSIZE = 64 and
>> -		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
>> -		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
>> -		which is greater than capacity of NAND device (OOBSIZE=64)
>> -		Hence, BCH16 cannot be supported on given device. But it can
>> -		probably use lower ecc-schemes like BCH8.
>> -
>> -	Example(b): For a device with PAGESIZE = 2048 and OOBSIZE = 128 and
>> -		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
>> -		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
>> -		which can be accommodated in the OOB/Spare area of this device
>> -		(OOBSIZE=128). So this device can use BCH16 ecc-scheme.
>> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>> new file mode 100644
>> index 000000000000..db36f2e944ef
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mtd/ti,gpmc-nand.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Texas Instruments GPMC NAND Flash controller.
>> +
>> +maintainers:
>> +  - Tony Lindgren <tony@atomide.com>
>> +  - Roger Quadros <rogerq@kernel.org>
>> +
>> +description:
>> +  GPMC NAND controller/Flash is represented as a child of the
>> +  GPMC controller node.
>> +
>> +properties:
>> +  compatible:
>> +    const: ti,omap2-nand
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    items:
>> +      - description: Interrupt for fifoevent
>> +      - description: Interrupt for termcount
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 1
>> +
>> +  ti,nand-ecc-opt:
>> +    description: Desired ECC algorithm
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    enum: [sw, ham1, bch4, bch8, bch16]
>> +
>> +  ti,nand-xfer-type:
>> +    description: Data transfer method between controller and chip.
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    enum: [prefetch-polled, polled, prefetch-dma, prefetch-irq]
>> +    default: prefetch-polled
>> +
>> +  ti,elm-id:
>> +    description:
>> +      phandle to the ELM (Error Location Module).
>> +    $ref: /schemas/types.yaml#/definitions/phandle
> 
> Should perhaps keep the elm-id property documented but set to
> 'deprecated'.
> 
>> +
>> +  nand-bus-width:
>> +    description:
>> +      Bus width to the NAND chip
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [8, 16]
>> +    default: 8
> 
> This is part of nand-controller.yaml binding and should not be there.
> 
>> +
>> +allOf:
>> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"
> 
> Maybe you need to reference the nand controller bindings as well
> 

This will not work out of the box :( as nand-controller.yaml defines both
  nand controller and nand memory. It potentially might work if it will be possible to split
nand memory definition (or nand memory properties) out of and-controller.yaml, similarly to
ti,gpmc-child.yaml from this series.

-- 
Best regards,
grygorii

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 15:27     ` Grygorii Strashko
@ 2021-09-07 16:35       ` Miquel Raynal
  2021-09-07 16:57         ` Roger Quadros
  0 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2021-09-07 16:35 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Roger Quadros, tony, robh+dt, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, devicetree, linux-mtd, linux-omap,
	linux-kernel

Hi Grygorii,

> >   
> >> +
> >> +  nand-bus-width:
> >> +    description:
> >> +      Bus width to the NAND chip
> >> +    $ref: /schemas/types.yaml#/definitions/uint32
> >> +    enum: [8, 16]
> >> +    default: 8  
> > 
> > This is part of nand-controller.yaml binding and should not be there.
> >   
> >> +
> >> +allOf:
> >> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"  
> > 
> > Maybe you need to reference the nand controller bindings as well
> >   
> 
> This will not work out of the box :( as nand-controller.yaml defines both
>   nand controller and nand memory. It potentially might work if it will be possible to split
> nand memory definition (or nand memory properties) out of and-controller.yaml, similarly to
> ti,gpmc-child.yaml from this series.

What you think would be the issue?

I am not opposed to split nand-controller.yaml into
nand-controller.yaml and nand-chip.yaml if it simplifies the
description of controllers but I don't get why it would be needed. In
particular since we expect all drivers to support the

nand-controller {
	controller-props;
	nand-chip {
		chip-props;
	}
}

organization which has been enforced since at least 2018. Having a
controller vs. chip representation is fundamentally right. But here I
see how "legacy" are these bindings with so much unneeded specific "ti,"
properties... On one side it would be good to verify that the driver
supports this representation (which I believe is true) and on the other
side maybe it's time to advertise "better" bindings as well.


Thanks,
Miquèl

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 16:35       ` Miquel Raynal
@ 2021-09-07 16:57         ` Roger Quadros
  2021-09-07 22:24           ` Rob Herring
  0 siblings, 1 reply; 24+ messages in thread
From: Roger Quadros @ 2021-09-07 16:57 UTC (permalink / raw)
  To: Miquel Raynal, Grygorii Strashko
  Cc: tony, robh+dt, nm, lokeshvutla, nsekhar, krzysztof.kozlowski,
	devicetree, linux-mtd, linux-omap, linux-kernel

Hi Miquel,

On 07/09/2021 19:35, Miquel Raynal wrote:
> Hi Grygorii,
> 
>>>   
>>>> +
>>>> +  nand-bus-width:
>>>> +    description:
>>>> +      Bus width to the NAND chip
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    enum: [8, 16]
>>>> +    default: 8  
>>>
>>> This is part of nand-controller.yaml binding and should not be there.
>>>   
>>>> +
>>>> +allOf:
>>>> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"  
>>>
>>> Maybe you need to reference the nand controller bindings as well
>>>   
>>
>> This will not work out of the box :( as nand-controller.yaml defines both
>>   nand controller and nand memory. It potentially might work if it will be possible to split
>> nand memory definition (or nand memory properties) out of and-controller.yaml, similarly to
>> ti,gpmc-child.yaml from this series.
> 
> What you think would be the issue?

The issue is that dt_binding checks will fail if I reference nand-controller.yaml
as we currently represent the controller as follows

memory-controller { /* GPMC controller */
	memory-controller-props;
	nand-chip {
		/* @chip select 0 */
		nand-controller-props;
		memory-controller-timing-props;
		chip-props;
	}
	nand-chip {
		/* @chip select 1 */
		nand-controller-props;
		memory-controller-timing-props;
		chip-props;
	}
	nor-chip {
		/* @chip select 2 */
		memory-controller-timing-props;
		chip-props;
	}
}

The NAND controller IO registers are at different addresses for different
chip select regions. Also, this is one way we can specify GPMC settings/timings
for different chip selects.

> 
> I am not opposed to split nand-controller.yaml into
> nand-controller.yaml and nand-chip.yaml if it simplifies the
> description of controllers but I don't get why it would be needed. In
> particular since we expect all drivers to support the
> 
> nand-controller {
> 	controller-props;
> 	nand-chip {
> 		chip-props;
> 	}
> }

Changing to this format will cause a lot of churn in DT files, which I'm not sure
if it gives enough benefit. 
TI platforms will never have 2 NAND chips in the same chip select region.

> 
> organization which has been enforced since at least 2018. Having a
> controller vs. chip representation is fundamentally right. But here I
> see how "legacy" are these bindings with so much unneeded specific "ti,"
> properties... On one side it would be good to verify that the driver
> supports this representation (which I believe is true) and on the other
> side maybe it's time to advertise "better" bindings as well.

Yes, I'm OK to mark ti specific properties deprecated and use standard NAND chip
bindings.

cheers,
-roger

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 11:32 ` [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml Roger Quadros
  2021-09-07 14:03   ` Miquel Raynal
@ 2021-09-07 17:01   ` Rob Herring
  2021-09-08 11:14     ` Roger Quadros
  1 sibling, 1 reply; 24+ messages in thread
From: Rob Herring @ 2021-09-07 17:01 UTC (permalink / raw)
  To: Roger Quadros
  Cc: tony, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel

On Tue, Sep 07, 2021 at 02:32:23PM +0300, Roger Quadros wrote:
> Convert gpmc-nand.txt to ti,gpmc-nand.yaml.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  .../devicetree/bindings/mtd/gpmc-nand.txt     | 147 ------------------
>  .../devicetree/bindings/mtd/ti,gpmc-nand.yaml | 110 +++++++++++++
>  2 files changed, 110 insertions(+), 147 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>  create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> deleted file mode 100644
> index 44919d48d241..000000000000
> --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> +++ /dev/null
> @@ -1,147 +0,0 @@
> -Device tree bindings for GPMC connected NANDs
> -
> -GPMC connected NAND (found on OMAP boards) are represented as child nodes of
> -the GPMC controller with a name of "nand".
> -
> -All timing relevant properties as well as generic gpmc child properties are
> -explained in a separate documents - please refer to
> -Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
> -
> -For NAND specific properties such as ECC modes or bus width, please refer to
> -Documentation/devicetree/bindings/mtd/nand-controller.yaml
> -
> -
> -Required properties:
> -
> - - compatible:	"ti,omap2-nand"
> - - reg:		range id (CS number), base offset and length of the
> -		NAND I/O space
> - - interrupts:	Two interrupt specifiers, one for fifoevent, one for termcount.
> -
> -Optional properties:
> -
> - - nand-bus-width: 		Set this numeric value to 16 if the hardware
> -				is wired that way. If not specified, a bus
> -				width of 8 is assumed.
> -
> - - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
> -		"sw"		1-bit Hamming ecc code via software
> -		"hw"		<deprecated> use "ham1" instead
> -		"hw-romcode"	<deprecated> use "ham1" instead
> -		"ham1"		1-bit Hamming ecc code
> -		"bch4"		4-bit BCH ecc code
> -		"bch8"		8-bit BCH ecc code
> -		"bch16"		16-bit BCH ECC code
> -		Refer below "How to select correct ECC scheme for your device ?"
> -
> - - ti,nand-xfer-type:		A string setting the data transfer type. One of:
> -
> -		"prefetch-polled"	Prefetch polled mode (default)
> -		"polled"		Polled mode, without prefetch
> -		"prefetch-dma"		Prefetch enabled DMA mode
> -		"prefetch-irq"		Prefetch enabled irq mode
> -
> - - elm_id:	<deprecated> use "ti,elm-id" instead
> - - ti,elm-id:	Specifies phandle of the ELM devicetree node.
> -		ELM is an on-chip hardware engine on TI SoC which is used for
> -		locating ECC errors for BCHx algorithms. SoC devices which have
> -		ELM hardware engines should specify this device node in .dtsi
> -		Using ELM for ECC error correction frees some CPU cycles.
> - - rb-gpios:	GPIO specifier for the ready/busy# pin.
> -
> -For inline partition table parsing (optional):
> -
> - - #address-cells: should be set to 1
> - - #size-cells: should be set to 1
> -
> -Example for an AM33xx board:
> -
> -	gpmc: gpmc@50000000 {
> -		compatible = "ti,am3352-gpmc";
> -		ti,hwmods = "gpmc";
> -		reg = <0x50000000 0x36c>;
> -		interrupts = <100>;
> -		gpmc,num-cs = <8>;
> -		gpmc,num-waitpins = <2>;
> -		#address-cells = <2>;
> -		#size-cells = <1>;
> -		ranges = <0 0 0x08000000 0x1000000>;	/* CS0 space, 16MB */
> -		elm_id = <&elm>;
> -		interrupt-controller;
> -		#interrupt-cells = <2>;
> -
> -		nand@0,0 {
> -			compatible = "ti,omap2-nand";
> -			reg = <0 0 4>;		/* CS0, offset 0, NAND I/O window 4 */
> -			interrupt-parent = <&gpmc>;
> -			interrupts = <0 IRQ_TYPE_NONE>, <1 IRQ_TYPE NONE>;
> -			nand-bus-width = <16>;
> -			ti,nand-ecc-opt = "bch8";
> -			ti,nand-xfer-type = "polled";
> -			rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
> -
> -			gpmc,sync-clk-ps = <0>;
> -			gpmc,cs-on-ns = <0>;
> -			gpmc,cs-rd-off-ns = <44>;
> -			gpmc,cs-wr-off-ns = <44>;
> -			gpmc,adv-on-ns = <6>;
> -			gpmc,adv-rd-off-ns = <34>;
> -			gpmc,adv-wr-off-ns = <44>;
> -			gpmc,we-off-ns = <40>;
> -			gpmc,oe-off-ns = <54>;
> -			gpmc,access-ns = <64>;
> -			gpmc,rd-cycle-ns = <82>;
> -			gpmc,wr-cycle-ns = <82>;
> -			gpmc,wr-access-ns = <40>;
> -			gpmc,wr-data-mux-bus-ns = <0>;
> -
> -			#address-cells = <1>;
> -			#size-cells = <1>;
> -
> -			/* partitions go here */
> -		};
> -	};
> -
> -How to select correct ECC scheme for your device ?
> ---------------------------------------------------
> -Higher ECC scheme usually means better protection against bit-flips and
> -increased system lifetime. However, selection of ECC scheme is dependent
> -on various other factors also like;
> -
> -(1) support of built in hardware engines.
> -	Some legacy OMAP SoC do not have ELM harware engine, so those SoC cannot
> -	support ecc-schemes with hardware error-correction (BCHx_HW). However
> -	such SoC can use ecc-schemes with software library for error-correction
> -	(BCHx_HW_DETECTION_SW). The error correction capability with software
> -	library remains equivalent to their hardware counter-part, but there is
> -	slight CPU penalty when too many bit-flips are detected during reads.
> -
> -(2) Device parameters like OOBSIZE.
> -	Other factor which governs the selection of ecc-scheme is oob-size.
> -	Higher ECC schemes require more OOB/Spare area to store ECC syndrome,
> -	so the device should have enough free bytes available its OOB/Spare
> -	area to accommodate ECC for entire page. In general following expression
> -	helps in determining if given device can accommodate ECC syndrome:
> -	"2 + (PAGESIZE / 512) * ECC_BYTES" >= OOBSIZE"
> -	where
> -		OOBSIZE		number of bytes in OOB/spare area
> -		PAGESIZE	number of bytes in main-area of device page
> -		ECC_BYTES	number of ECC bytes generated to protect
> -		                512 bytes of data, which is:
> -				'3' for HAM1_xx ecc schemes
> -				'7' for BCH4_xx ecc schemes
> -				'14' for BCH8_xx ecc schemes
> -				'26' for BCH16_xx ecc schemes
> -
> -	Example(a): For a device with PAGESIZE = 2048 and OOBSIZE = 64 and
> -		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
> -		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
> -		which is greater than capacity of NAND device (OOBSIZE=64)
> -		Hence, BCH16 cannot be supported on given device. But it can
> -		probably use lower ecc-schemes like BCH8.
> -
> -	Example(b): For a device with PAGESIZE = 2048 and OOBSIZE = 128 and
> -		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
> -		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
> -		which can be accommodated in the OOB/Spare area of this device
> -		(OOBSIZE=128). So this device can use BCH16 ecc-scheme.
> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> new file mode 100644
> index 000000000000..db36f2e944ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/ti,gpmc-nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments GPMC NAND Flash controller.
> +
> +maintainers:
> +  - Tony Lindgren <tony@atomide.com>
> +  - Roger Quadros <rogerq@kernel.org>
> +
> +description:
> +  GPMC NAND controller/Flash is represented as a child of the
> +  GPMC controller node.
> +
> +properties:
> +  compatible:
> +    const: ti,omap2-nand
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: Interrupt for fifoevent
> +      - description: Interrupt for termcount
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  ti,nand-ecc-opt:
> +    description: Desired ECC algorithm
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [sw, ham1, bch4, bch8, bch16]
> +
> +  ti,nand-xfer-type:
> +    description: Data transfer method between controller and chip.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [prefetch-polled, polled, prefetch-dma, prefetch-irq]
> +    default: prefetch-polled
> +
> +  ti,elm-id:
> +    description:
> +      phandle to the ELM (Error Location Module).
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  nand-bus-width:
> +    description:
> +      Bus width to the NAND chip
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [8, 16]
> +    default: 8
> +
> +allOf:
> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"

Use /schemas/... rather than '..'

> +
> +required:
> +  - compatible
> +  - reg
> +  - ti,nand-ecc-opt
> +
> +unevaluatedProperties: false

This will throw errors if you have partition nodes. You need to 
reference the partitions schema.

Or minimally restrict to nodes with: 

unevaluatedProperties:
  type: object

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    gpmc: memory-controller@50000000 {
> +      compatible = "ti,am3352-gpmc";
> +      dmas = <&edma 52 0>;
> +      dma-names = "rxtx";
> +      clocks = <&l3s_gclk>;
> +      clock-names = "fck";
> +      reg = <0x50000000 0x2000>;
> +      interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> +      gpmc,num-cs = <7>;
> +      gpmc,num-waitpins = <2>;
> +      #address-cells = <2>;
> +      #size-cells = <1>;
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +      gpio-controller;
> +      #gpio-cells = <2>;
> +
> +      ranges = <0 0 0x08000000 0x01000000>;   /* CS0 space. Min partition = 16MB */
> +      nand@0,0 {
> +        compatible = "ti,omap2-nand";
> +        reg = <0 0 4>;          /* device IO registers */
> +        interrupt-parent = <&gpmc>;
> +        interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
> +                     <1 IRQ_TYPE_NONE>; /* termcount */
> +        ti,nand-xfer-type = "prefetch-dma";
> +        ti,nand-ecc-opt = "bch16";
> +        ti,elm-id = <&elm>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        /* NAND generic properties */
> +        nand-bus-width = <8>;
> +        rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>;  /* gpmc_wait0 */
> +
> +        /* GPMC properties*/
> +        gpmc,device-width = <1>;
> +      };
> +    };
> -- 
> 2.17.1
> 
> 

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 16:57         ` Roger Quadros
@ 2021-09-07 22:24           ` Rob Herring
  2021-09-08  6:55             ` Roger Quadros
  0 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2021-09-07 22:24 UTC (permalink / raw)
  To: Roger Quadros
  Cc: Miquel Raynal, Grygorii Strashko, tony, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, devicetree, linux-mtd, linux-omap,
	linux-kernel

On Tue, Sep 7, 2021 at 11:57 AM Roger Quadros <rogerq@kernel.org> wrote:
>
> Hi Miquel,
>
> On 07/09/2021 19:35, Miquel Raynal wrote:
> > Hi Grygorii,
> >
> >>>
> >>>> +
> >>>> +  nand-bus-width:
> >>>> +    description:
> >>>> +      Bus width to the NAND chip
> >>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>> +    enum: [8, 16]
> >>>> +    default: 8
> >>>
> >>> This is part of nand-controller.yaml binding and should not be there.
> >>>
> >>>> +
> >>>> +allOf:
> >>>> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"
> >>>
> >>> Maybe you need to reference the nand controller bindings as well
> >>>
> >>
> >> This will not work out of the box :( as nand-controller.yaml defines both
> >>   nand controller and nand memory. It potentially might work if it will be possible to split
> >> nand memory definition (or nand memory properties) out of and-controller.yaml, similarly to
> >> ti,gpmc-child.yaml from this series.
> >
> > What you think would be the issue?
>
> The issue is that dt_binding checks will fail if I reference nand-controller.yaml
> as we currently represent the controller as follows
>
> memory-controller { /* GPMC controller */
>         memory-controller-props;
>         nand-chip {
>                 /* @chip select 0 */
>                 nand-controller-props;
>                 memory-controller-timing-props;
>                 chip-props;
>         }
>         nand-chip {
>                 /* @chip select 1 */
>                 nand-controller-props;
>                 memory-controller-timing-props;
>                 chip-props;
>         }
>         nor-chip {
>                 /* @chip select 2 */
>                 memory-controller-timing-props;
>                 chip-props;
>         }
> }
>
> The NAND controller IO registers are at different addresses for different
> chip select regions. Also, this is one way we can specify GPMC settings/timings
> for different chip selects.
>
> >
> > I am not opposed to split nand-controller.yaml into
> > nand-controller.yaml and nand-chip.yaml if it simplifies the
> > description of controllers but I don't get why it would be needed. In
> > particular since we expect all drivers to support the
> >
> > nand-controller {
> >       controller-props;
> >       nand-chip {
> >               chip-props;
> >       }
> > }
>
> Changing to this format will cause a lot of churn in DT files, which I'm not sure
> if it gives enough benefit.
> TI platforms will never have 2 NAND chips in the same chip select region.

Probably best to just leave this alone. Unless this is getting used in
new chips? If so, I'd say it's a separate change.

> > organization which has been enforced since at least 2018. Having a
> > controller vs. chip representation is fundamentally right. But here I
> > see how "legacy" are these bindings with so much unneeded specific "ti,"
> > properties... On one side it would be good to verify that the driver
> > supports this representation (which I believe is true) and on the other
> > side maybe it's time to advertise "better" bindings as well.
>
> Yes, I'm OK to mark ti specific properties deprecated and use standard NAND chip
> bindings.

I don't think it's really worth it to go half way using common
properties but not the common structure.

Rob

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 22:24           ` Rob Herring
@ 2021-09-08  6:55             ` Roger Quadros
  0 siblings, 0 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-08  6:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: Miquel Raynal, Grygorii Strashko, tony, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, devicetree, linux-mtd, linux-omap,
	linux-kernel



On 08/09/2021 01:24, Rob Herring wrote:
> On Tue, Sep 7, 2021 at 11:57 AM Roger Quadros <rogerq@kernel.org> wrote:
>>
>> Hi Miquel,
>>
>> On 07/09/2021 19:35, Miquel Raynal wrote:
>>> Hi Grygorii,
>>>
>>>>>
>>>>>> +
>>>>>> +  nand-bus-width:
>>>>>> +    description:
>>>>>> +      Bus width to the NAND chip
>>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>>> +    enum: [8, 16]
>>>>>> +    default: 8
>>>>>
>>>>> This is part of nand-controller.yaml binding and should not be there.
>>>>>
>>>>>> +
>>>>>> +allOf:
>>>>>> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"
>>>>>
>>>>> Maybe you need to reference the nand controller bindings as well
>>>>>
>>>>
>>>> This will not work out of the box :( as nand-controller.yaml defines both
>>>>   nand controller and nand memory. It potentially might work if it will be possible to split
>>>> nand memory definition (or nand memory properties) out of and-controller.yaml, similarly to
>>>> ti,gpmc-child.yaml from this series.
>>>
>>> What you think would be the issue?
>>
>> The issue is that dt_binding checks will fail if I reference nand-controller.yaml
>> as we currently represent the controller as follows
>>
>> memory-controller { /* GPMC controller */
>>         memory-controller-props;
>>         nand-chip {
>>                 /* @chip select 0 */
>>                 nand-controller-props;
>>                 memory-controller-timing-props;
>>                 chip-props;
>>         }
>>         nand-chip {
>>                 /* @chip select 1 */
>>                 nand-controller-props;
>>                 memory-controller-timing-props;
>>                 chip-props;
>>         }
>>         nor-chip {
>>                 /* @chip select 2 */
>>                 memory-controller-timing-props;
>>                 chip-props;
>>         }
>> }
>>
>> The NAND controller IO registers are at different addresses for different
>> chip select regions. Also, this is one way we can specify GPMC settings/timings
>> for different chip selects.
>>
>>>
>>> I am not opposed to split nand-controller.yaml into
>>> nand-controller.yaml and nand-chip.yaml if it simplifies the
>>> description of controllers but I don't get why it would be needed. In
>>> particular since we expect all drivers to support the
>>>
>>> nand-controller {
>>>       controller-props;
>>>       nand-chip {
>>>               chip-props;
>>>       }
>>> }
>>
>> Changing to this format will cause a lot of churn in DT files, which I'm not sure
>> if it gives enough benefit.
>> TI platforms will never have 2 NAND chips in the same chip select region.
> 
> Probably best to just leave this alone. Unless this is getting used in
> new chips? If so, I'd say it's a separate change.
> 
>>> organization which has been enforced since at least 2018. Having a
>>> controller vs. chip representation is fundamentally right. But here I
>>> see how "legacy" are these bindings with so much unneeded specific "ti,"
>>> properties... On one side it would be good to verify that the driver
>>> supports this representation (which I believe is true) and on the other
>>> side maybe it's time to advertise "better" bindings as well.
>>
>> Yes, I'm OK to mark ti specific properties deprecated and use standard NAND chip
>> bindings.
> 
> I don't think it's really worth it to go half way using common
> properties but not the common structure.

I agree.
We will be having new chips that will use this driver but we will migrate to new
common structure when adding support for those chips.

So I will leave this patch as it is for now.

cheers,
-roger

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-07 17:01   ` Rob Herring
@ 2021-09-08 11:14     ` Roger Quadros
  2021-09-08 12:46       ` Rob Herring
  0 siblings, 1 reply; 24+ messages in thread
From: Roger Quadros @ 2021-09-08 11:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: tony, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel



On 07/09/2021 20:01, Rob Herring wrote:
> On Tue, Sep 07, 2021 at 02:32:23PM +0300, Roger Quadros wrote:
>> Convert gpmc-nand.txt to ti,gpmc-nand.yaml.
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  .../devicetree/bindings/mtd/gpmc-nand.txt     | 147 ------------------
>>  .../devicetree/bindings/mtd/ti,gpmc-nand.yaml | 110 +++++++++++++
>>  2 files changed, 110 insertions(+), 147 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>>  create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>> deleted file mode 100644
>> index 44919d48d241..000000000000
>> --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>> +++ /dev/null
>> @@ -1,147 +0,0 @@
>> -Device tree bindings for GPMC connected NANDs
>> -
>> -GPMC connected NAND (found on OMAP boards) are represented as child nodes of
>> -the GPMC controller with a name of "nand".
>> -
>> -All timing relevant properties as well as generic gpmc child properties are
>> -explained in a separate documents - please refer to
>> -Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>> -
>> -For NAND specific properties such as ECC modes or bus width, please refer to
>> -Documentation/devicetree/bindings/mtd/nand-controller.yaml
>> -
>> -
>> -Required properties:
>> -
>> - - compatible:	"ti,omap2-nand"
>> - - reg:		range id (CS number), base offset and length of the
>> -		NAND I/O space
>> - - interrupts:	Two interrupt specifiers, one for fifoevent, one for termcount.
>> -
>> -Optional properties:
>> -
>> - - nand-bus-width: 		Set this numeric value to 16 if the hardware
>> -				is wired that way. If not specified, a bus
>> -				width of 8 is assumed.
>> -
>> - - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
>> -		"sw"		1-bit Hamming ecc code via software
>> -		"hw"		<deprecated> use "ham1" instead
>> -		"hw-romcode"	<deprecated> use "ham1" instead
>> -		"ham1"		1-bit Hamming ecc code
>> -		"bch4"		4-bit BCH ecc code
>> -		"bch8"		8-bit BCH ecc code
>> -		"bch16"		16-bit BCH ECC code
>> -		Refer below "How to select correct ECC scheme for your device ?"
>> -
>> - - ti,nand-xfer-type:		A string setting the data transfer type. One of:
>> -
>> -		"prefetch-polled"	Prefetch polled mode (default)
>> -		"polled"		Polled mode, without prefetch
>> -		"prefetch-dma"		Prefetch enabled DMA mode
>> -		"prefetch-irq"		Prefetch enabled irq mode
>> -
>> - - elm_id:	<deprecated> use "ti,elm-id" instead
>> - - ti,elm-id:	Specifies phandle of the ELM devicetree node.
>> -		ELM is an on-chip hardware engine on TI SoC which is used for
>> -		locating ECC errors for BCHx algorithms. SoC devices which have
>> -		ELM hardware engines should specify this device node in .dtsi
>> -		Using ELM for ECC error correction frees some CPU cycles.
>> - - rb-gpios:	GPIO specifier for the ready/busy# pin.
>> -
>> -For inline partition table parsing (optional):
>> -
>> - - #address-cells: should be set to 1
>> - - #size-cells: should be set to 1
>> -
>> -Example for an AM33xx board:
>> -
>> -	gpmc: gpmc@50000000 {
>> -		compatible = "ti,am3352-gpmc";
>> -		ti,hwmods = "gpmc";
>> -		reg = <0x50000000 0x36c>;
>> -		interrupts = <100>;
>> -		gpmc,num-cs = <8>;
>> -		gpmc,num-waitpins = <2>;
>> -		#address-cells = <2>;
>> -		#size-cells = <1>;
>> -		ranges = <0 0 0x08000000 0x1000000>;	/* CS0 space, 16MB */
>> -		elm_id = <&elm>;
>> -		interrupt-controller;
>> -		#interrupt-cells = <2>;
>> -
>> -		nand@0,0 {
>> -			compatible = "ti,omap2-nand";
>> -			reg = <0 0 4>;		/* CS0, offset 0, NAND I/O window 4 */
>> -			interrupt-parent = <&gpmc>;
>> -			interrupts = <0 IRQ_TYPE_NONE>, <1 IRQ_TYPE NONE>;
>> -			nand-bus-width = <16>;
>> -			ti,nand-ecc-opt = "bch8";
>> -			ti,nand-xfer-type = "polled";
>> -			rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
>> -
>> -			gpmc,sync-clk-ps = <0>;
>> -			gpmc,cs-on-ns = <0>;
>> -			gpmc,cs-rd-off-ns = <44>;
>> -			gpmc,cs-wr-off-ns = <44>;
>> -			gpmc,adv-on-ns = <6>;
>> -			gpmc,adv-rd-off-ns = <34>;
>> -			gpmc,adv-wr-off-ns = <44>;
>> -			gpmc,we-off-ns = <40>;
>> -			gpmc,oe-off-ns = <54>;
>> -			gpmc,access-ns = <64>;
>> -			gpmc,rd-cycle-ns = <82>;
>> -			gpmc,wr-cycle-ns = <82>;
>> -			gpmc,wr-access-ns = <40>;
>> -			gpmc,wr-data-mux-bus-ns = <0>;
>> -
>> -			#address-cells = <1>;
>> -			#size-cells = <1>;
>> -
>> -			/* partitions go here */
>> -		};
>> -	};
>> -
>> -How to select correct ECC scheme for your device ?
>> ---------------------------------------------------
>> -Higher ECC scheme usually means better protection against bit-flips and
>> -increased system lifetime. However, selection of ECC scheme is dependent
>> -on various other factors also like;
>> -
>> -(1) support of built in hardware engines.
>> -	Some legacy OMAP SoC do not have ELM harware engine, so those SoC cannot
>> -	support ecc-schemes with hardware error-correction (BCHx_HW). However
>> -	such SoC can use ecc-schemes with software library for error-correction
>> -	(BCHx_HW_DETECTION_SW). The error correction capability with software
>> -	library remains equivalent to their hardware counter-part, but there is
>> -	slight CPU penalty when too many bit-flips are detected during reads.
>> -
>> -(2) Device parameters like OOBSIZE.
>> -	Other factor which governs the selection of ecc-scheme is oob-size.
>> -	Higher ECC schemes require more OOB/Spare area to store ECC syndrome,
>> -	so the device should have enough free bytes available its OOB/Spare
>> -	area to accommodate ECC for entire page. In general following expression
>> -	helps in determining if given device can accommodate ECC syndrome:
>> -	"2 + (PAGESIZE / 512) * ECC_BYTES" >= OOBSIZE"
>> -	where
>> -		OOBSIZE		number of bytes in OOB/spare area
>> -		PAGESIZE	number of bytes in main-area of device page
>> -		ECC_BYTES	number of ECC bytes generated to protect
>> -		                512 bytes of data, which is:
>> -				'3' for HAM1_xx ecc schemes
>> -				'7' for BCH4_xx ecc schemes
>> -				'14' for BCH8_xx ecc schemes
>> -				'26' for BCH16_xx ecc schemes
>> -
>> -	Example(a): For a device with PAGESIZE = 2048 and OOBSIZE = 64 and
>> -		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
>> -		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
>> -		which is greater than capacity of NAND device (OOBSIZE=64)
>> -		Hence, BCH16 cannot be supported on given device. But it can
>> -		probably use lower ecc-schemes like BCH8.
>> -
>> -	Example(b): For a device with PAGESIZE = 2048 and OOBSIZE = 128 and
>> -		trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
>> -		Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
>> -		which can be accommodated in the OOB/Spare area of this device
>> -		(OOBSIZE=128). So this device can use BCH16 ecc-scheme.
>> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>> new file mode 100644
>> index 000000000000..db36f2e944ef
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mtd/ti,gpmc-nand.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Texas Instruments GPMC NAND Flash controller.
>> +
>> +maintainers:
>> +  - Tony Lindgren <tony@atomide.com>
>> +  - Roger Quadros <rogerq@kernel.org>
>> +
>> +description:
>> +  GPMC NAND controller/Flash is represented as a child of the
>> +  GPMC controller node.
>> +
>> +properties:
>> +  compatible:
>> +    const: ti,omap2-nand
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    items:
>> +      - description: Interrupt for fifoevent
>> +      - description: Interrupt for termcount
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 1
>> +
>> +  ti,nand-ecc-opt:
>> +    description: Desired ECC algorithm
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    enum: [sw, ham1, bch4, bch8, bch16]
>> +
>> +  ti,nand-xfer-type:
>> +    description: Data transfer method between controller and chip.
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    enum: [prefetch-polled, polled, prefetch-dma, prefetch-irq]
>> +    default: prefetch-polled
>> +
>> +  ti,elm-id:
>> +    description:
>> +      phandle to the ELM (Error Location Module).
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> +  nand-bus-width:
>> +    description:
>> +      Bus width to the NAND chip
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [8, 16]
>> +    default: 8
>> +
>> +allOf:
>> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"
> 
> Use /schemas/... rather than '..'

ok.

> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - ti,nand-ecc-opt
>> +
>> +unevaluatedProperties: false
> 
> This will throw errors if you have partition nodes. You need to 
> reference the partitions schema.

I didn't get any errors with dtbs_check so far. But I will
try referencing the partition schema here.

> 
> Or minimally restrict to nodes with: 
> 
> unevaluatedProperties:
>   type: object
> 
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    gpmc: memory-controller@50000000 {
>> +      compatible = "ti,am3352-gpmc";
>> +      dmas = <&edma 52 0>;
>> +      dma-names = "rxtx";
>> +      clocks = <&l3s_gclk>;
>> +      clock-names = "fck";
>> +      reg = <0x50000000 0x2000>;
>> +      interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
>> +      gpmc,num-cs = <7>;
>> +      gpmc,num-waitpins = <2>;
>> +      #address-cells = <2>;
>> +      #size-cells = <1>;
>> +      interrupt-controller;
>> +      #interrupt-cells = <2>;
>> +      gpio-controller;
>> +      #gpio-cells = <2>;
>> +
>> +      ranges = <0 0 0x08000000 0x01000000>;   /* CS0 space. Min partition = 16MB */
>> +      nand@0,0 {
>> +        compatible = "ti,omap2-nand";
>> +        reg = <0 0 4>;          /* device IO registers */
>> +        interrupt-parent = <&gpmc>;
>> +        interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
>> +                     <1 IRQ_TYPE_NONE>; /* termcount */
>> +        ti,nand-xfer-type = "prefetch-dma";
>> +        ti,nand-ecc-opt = "bch16";
>> +        ti,elm-id = <&elm>;
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +
>> +        /* NAND generic properties */
>> +        nand-bus-width = <8>;
>> +        rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>;  /* gpmc_wait0 */
>> +
>> +        /* GPMC properties*/
>> +        gpmc,device-width = <1>;
>> +      };
>> +    };
>> -- 
>> 2.17.1
>>
>>

cheers,
-roger

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

* Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml
  2021-09-08 11:14     ` Roger Quadros
@ 2021-09-08 12:46       ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2021-09-08 12:46 UTC (permalink / raw)
  To: Roger Quadros
  Cc: tony, grygorii.strashko, nm, lokeshvutla, nsekhar,
	krzysztof.kozlowski, miquel.raynal, devicetree, linux-mtd,
	linux-omap, linux-kernel

On Wed, Sep 8, 2021 at 6:14 AM Roger Quadros <rogerq@kernel.org> wrote:
>
>
>
> On 07/09/2021 20:01, Rob Herring wrote:
> > On Tue, Sep 07, 2021 at 02:32:23PM +0300, Roger Quadros wrote:
> >> Convert gpmc-nand.txt to ti,gpmc-nand.yaml.
> >>
> >> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> >> ---
> >>  .../devicetree/bindings/mtd/gpmc-nand.txt     | 147 ------------------
> >>  .../devicetree/bindings/mtd/ti,gpmc-nand.yaml | 110 +++++++++++++
> >>  2 files changed, 110 insertions(+), 147 deletions(-)
> >>  delete mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> >>  create mode 100644 Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> >> deleted file mode 100644
> >> index 44919d48d241..000000000000
> >> --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> >> +++ /dev/null
> >> @@ -1,147 +0,0 @@
> >> -Device tree bindings for GPMC connected NANDs
> >> -
> >> -GPMC connected NAND (found on OMAP boards) are represented as child nodes of
> >> -the GPMC controller with a name of "nand".
> >> -
> >> -All timing relevant properties as well as generic gpmc child properties are
> >> -explained in a separate documents - please refer to
> >> -Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
> >> -
> >> -For NAND specific properties such as ECC modes or bus width, please refer to
> >> -Documentation/devicetree/bindings/mtd/nand-controller.yaml
> >> -
> >> -
> >> -Required properties:
> >> -
> >> - - compatible:      "ti,omap2-nand"
> >> - - reg:             range id (CS number), base offset and length of the
> >> -            NAND I/O space
> >> - - interrupts:      Two interrupt specifiers, one for fifoevent, one for termcount.
> >> -
> >> -Optional properties:
> >> -
> >> - - nand-bus-width:          Set this numeric value to 16 if the hardware
> >> -                            is wired that way. If not specified, a bus
> >> -                            width of 8 is assumed.
> >> -
> >> - - ti,nand-ecc-opt:         A string setting the ECC layout to use. One of:
> >> -            "sw"            1-bit Hamming ecc code via software
> >> -            "hw"            <deprecated> use "ham1" instead
> >> -            "hw-romcode"    <deprecated> use "ham1" instead
> >> -            "ham1"          1-bit Hamming ecc code
> >> -            "bch4"          4-bit BCH ecc code
> >> -            "bch8"          8-bit BCH ecc code
> >> -            "bch16"         16-bit BCH ECC code
> >> -            Refer below "How to select correct ECC scheme for your device ?"
> >> -
> >> - - ti,nand-xfer-type:               A string setting the data transfer type. One of:
> >> -
> >> -            "prefetch-polled"       Prefetch polled mode (default)
> >> -            "polled"                Polled mode, without prefetch
> >> -            "prefetch-dma"          Prefetch enabled DMA mode
> >> -            "prefetch-irq"          Prefetch enabled irq mode
> >> -
> >> - - elm_id:  <deprecated> use "ti,elm-id" instead
> >> - - ti,elm-id:       Specifies phandle of the ELM devicetree node.
> >> -            ELM is an on-chip hardware engine on TI SoC which is used for
> >> -            locating ECC errors for BCHx algorithms. SoC devices which have
> >> -            ELM hardware engines should specify this device node in .dtsi
> >> -            Using ELM for ECC error correction frees some CPU cycles.
> >> - - rb-gpios:        GPIO specifier for the ready/busy# pin.
> >> -
> >> -For inline partition table parsing (optional):
> >> -
> >> - - #address-cells: should be set to 1
> >> - - #size-cells: should be set to 1
> >> -
> >> -Example for an AM33xx board:
> >> -
> >> -    gpmc: gpmc@50000000 {
> >> -            compatible = "ti,am3352-gpmc";
> >> -            ti,hwmods = "gpmc";
> >> -            reg = <0x50000000 0x36c>;
> >> -            interrupts = <100>;
> >> -            gpmc,num-cs = <8>;
> >> -            gpmc,num-waitpins = <2>;
> >> -            #address-cells = <2>;
> >> -            #size-cells = <1>;
> >> -            ranges = <0 0 0x08000000 0x1000000>;    /* CS0 space, 16MB */
> >> -            elm_id = <&elm>;
> >> -            interrupt-controller;
> >> -            #interrupt-cells = <2>;
> >> -
> >> -            nand@0,0 {
> >> -                    compatible = "ti,omap2-nand";
> >> -                    reg = <0 0 4>;          /* CS0, offset 0, NAND I/O window 4 */
> >> -                    interrupt-parent = <&gpmc>;
> >> -                    interrupts = <0 IRQ_TYPE_NONE>, <1 IRQ_TYPE NONE>;
> >> -                    nand-bus-width = <16>;
> >> -                    ti,nand-ecc-opt = "bch8";
> >> -                    ti,nand-xfer-type = "polled";
> >> -                    rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
> >> -
> >> -                    gpmc,sync-clk-ps = <0>;
> >> -                    gpmc,cs-on-ns = <0>;
> >> -                    gpmc,cs-rd-off-ns = <44>;
> >> -                    gpmc,cs-wr-off-ns = <44>;
> >> -                    gpmc,adv-on-ns = <6>;
> >> -                    gpmc,adv-rd-off-ns = <34>;
> >> -                    gpmc,adv-wr-off-ns = <44>;
> >> -                    gpmc,we-off-ns = <40>;
> >> -                    gpmc,oe-off-ns = <54>;
> >> -                    gpmc,access-ns = <64>;
> >> -                    gpmc,rd-cycle-ns = <82>;
> >> -                    gpmc,wr-cycle-ns = <82>;
> >> -                    gpmc,wr-access-ns = <40>;
> >> -                    gpmc,wr-data-mux-bus-ns = <0>;
> >> -
> >> -                    #address-cells = <1>;
> >> -                    #size-cells = <1>;
> >> -
> >> -                    /* partitions go here */
> >> -            };
> >> -    };
> >> -
> >> -How to select correct ECC scheme for your device ?
> >> ---------------------------------------------------
> >> -Higher ECC scheme usually means better protection against bit-flips and
> >> -increased system lifetime. However, selection of ECC scheme is dependent
> >> -on various other factors also like;
> >> -
> >> -(1) support of built in hardware engines.
> >> -    Some legacy OMAP SoC do not have ELM harware engine, so those SoC cannot
> >> -    support ecc-schemes with hardware error-correction (BCHx_HW). However
> >> -    such SoC can use ecc-schemes with software library for error-correction
> >> -    (BCHx_HW_DETECTION_SW). The error correction capability with software
> >> -    library remains equivalent to their hardware counter-part, but there is
> >> -    slight CPU penalty when too many bit-flips are detected during reads.
> >> -
> >> -(2) Device parameters like OOBSIZE.
> >> -    Other factor which governs the selection of ecc-scheme is oob-size.
> >> -    Higher ECC schemes require more OOB/Spare area to store ECC syndrome,
> >> -    so the device should have enough free bytes available its OOB/Spare
> >> -    area to accommodate ECC for entire page. In general following expression
> >> -    helps in determining if given device can accommodate ECC syndrome:
> >> -    "2 + (PAGESIZE / 512) * ECC_BYTES" >= OOBSIZE"
> >> -    where
> >> -            OOBSIZE         number of bytes in OOB/spare area
> >> -            PAGESIZE        number of bytes in main-area of device page
> >> -            ECC_BYTES       number of ECC bytes generated to protect
> >> -                            512 bytes of data, which is:
> >> -                            '3' for HAM1_xx ecc schemes
> >> -                            '7' for BCH4_xx ecc schemes
> >> -                            '14' for BCH8_xx ecc schemes
> >> -                            '26' for BCH16_xx ecc schemes
> >> -
> >> -    Example(a): For a device with PAGESIZE = 2048 and OOBSIZE = 64 and
> >> -            trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
> >> -            Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
> >> -            which is greater than capacity of NAND device (OOBSIZE=64)
> >> -            Hence, BCH16 cannot be supported on given device. But it can
> >> -            probably use lower ecc-schemes like BCH8.
> >> -
> >> -    Example(b): For a device with PAGESIZE = 2048 and OOBSIZE = 128 and
> >> -            trying to use BCH16 (ECC_BYTES=26) ecc-scheme.
> >> -            Number of ECC bytes per page = (2 + (2048 / 512) * 26) = 106 B
> >> -            which can be accommodated in the OOB/Spare area of this device
> >> -            (OOBSIZE=128). So this device can use BCH16 ecc-scheme.
> >> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> >> new file mode 100644
> >> index 000000000000..db36f2e944ef
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> >> @@ -0,0 +1,110 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/mtd/ti,gpmc-nand.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Texas Instruments GPMC NAND Flash controller.
> >> +
> >> +maintainers:
> >> +  - Tony Lindgren <tony@atomide.com>
> >> +  - Roger Quadros <rogerq@kernel.org>
> >> +
> >> +description:
> >> +  GPMC NAND controller/Flash is represented as a child of the
> >> +  GPMC controller node.
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: ti,omap2-nand
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +  interrupts:
> >> +    items:
> >> +      - description: Interrupt for fifoevent
> >> +      - description: Interrupt for termcount
> >> +
> >> +  "#address-cells":
> >> +    const: 1
> >> +
> >> +  "#size-cells":
> >> +    const: 1
> >> +
> >> +  ti,nand-ecc-opt:
> >> +    description: Desired ECC algorithm
> >> +    $ref: /schemas/types.yaml#/definitions/string
> >> +    enum: [sw, ham1, bch4, bch8, bch16]
> >> +
> >> +  ti,nand-xfer-type:
> >> +    description: Data transfer method between controller and chip.
> >> +    $ref: /schemas/types.yaml#/definitions/string
> >> +    enum: [prefetch-polled, polled, prefetch-dma, prefetch-irq]
> >> +    default: prefetch-polled
> >> +
> >> +  ti,elm-id:
> >> +    description:
> >> +      phandle to the ELM (Error Location Module).
> >> +    $ref: /schemas/types.yaml#/definitions/phandle
> >> +
> >> +  nand-bus-width:
> >> +    description:
> >> +      Bus width to the NAND chip
> >> +    $ref: /schemas/types.yaml#/definitions/uint32
> >> +    enum: [8, 16]
> >> +    default: 8
> >> +
> >> +allOf:
> >> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"
> >
> > Use /schemas/... rather than '..'
>
> ok.
>
> >
> >> +
> >> +required:
> >> +  - compatible
> >> +  - reg
> >> +  - ti,nand-ecc-opt
> >> +
> >> +unevaluatedProperties: false
> >
> > This will throw errors if you have partition nodes. You need to
> > reference the partitions schema.
>
> I didn't get any errors with dtbs_check so far. But I will
> try referencing the partition schema here.

Not yet because 'unevaluatedProperties' isn't actually supported in
the jsonschema package yet. It will be in the next release which I
expect soonish.

Rob

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

* Re: [PATCH v3 1/8] ARM: dts: omap: Fixup GPMC child nodes
  2021-09-07 12:44   ` Krzysztof Kozlowski
@ 2021-09-15  8:53     ` Roger Quadros
  0 siblings, 0 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-15  8:53 UTC (permalink / raw)
  To: Krzysztof Kozlowski, tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	miquel.raynal, devicetree, linux-mtd, linux-omap, linux-kernel

Hi Krzysztof,

On 07/09/2021 15:44, Krzysztof Kozlowski wrote:
> On 07/09/2021 13:32, Roger Quadros wrote:
>> Fixes up the GPMC child nodes to prevent dtbs_check errors
>> after device tree binding conversion to yaml.
>>
>> - Use reg address as node name
>> - gpmc,cycle2cycle-samecsen and gpmc,cycle2cycle-diffcsen are
>>   boolean properties.
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  .../boot/dts/logicpd-som-lv-baseboard.dtsi    |  2 +-
>>  .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |  2 +-
>>  .../boot/dts/logicpd-torpedo-baseboard.dtsi   |  2 +-
>>  arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi     | 62 +++++++++----------
>>  arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi     | 59 +++++++++---------
>>  arch/arm/boot/dts/omap-zoom-common.dtsi       | 16 ++---
>>  arch/arm/boot/dts/omap2430-sdp.dts            |  6 +-
>>  arch/arm/boot/dts/omap3-cm-t3x30.dtsi         |  6 +-
>>  .../arm/boot/dts/omap3-devkit8000-common.dtsi |  4 +-
>>  arch/arm/boot/dts/omap3-evm-37xx.dts          |  1 +
>>  arch/arm/boot/dts/omap3-evm-common.dtsi       |  9 ---
>>  .../boot/dts/omap3-evm-processor-common.dtsi  |  5 +-
>>  arch/arm/boot/dts/omap3-evm.dts               |  1 +
>>  arch/arm/boot/dts/omap3-igep0020-common.dtsi  |  5 +-
>>  arch/arm/boot/dts/omap3-ldp.dts               |  5 +-
>>  arch/arm/boot/dts/omap3-n900.dts              |  2 +-
>>  .../dts/omap3-overo-chestnut43-common.dtsi    |  6 +-
>>  .../arm/boot/dts/omap3-overo-tobi-common.dtsi |  6 +-
>>  .../boot/dts/omap3-overo-tobiduo-common.dtsi  |  8 +--
>>  arch/arm/boot/dts/omap3-sb-t35.dtsi           |  4 +-
>>  arch/arm/boot/dts/omap4-duovero-parlor.dts    |  6 +-
>>  21 files changed, 105 insertions(+), 112 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi b/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
>> index 7d0468a23781..f2364cb114c5 100644
>> --- a/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
>> +++ b/arch/arm/boot/dts/logicpd-som-lv-baseboard.dtsi
>> @@ -65,7 +65,7 @@
>>  		  1 0 0x2c000000 0x1000000	/* CS1: 16MB for LAN9221 */
>>  		  2 0 0x10000000 0x2000000>;    /* CS2: 32MB for NOR */
>>  
>> -	ethernet@gpmc {
>> +	gpmc_ethernet: ethernet@1,0 {
>>  		pinctrl-names = "default";
>>  		pinctrl-0 = <&lan9221_pins>;
>>  		interrupt-parent = <&gpio5>;
>> diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
>> index 5532db04046c..6357915d207b 100644
>> --- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
>> +++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
>> @@ -4,8 +4,8 @@
>>  
>>  #include "omap36xx.dtsi"
>>  #include "logicpd-torpedo-som.dtsi"
>> -#include "omap-gpmc-smsc9221.dtsi"
>>  #include "logicpd-torpedo-baseboard.dtsi"
>> +#include "omap-gpmc-smsc9221.dtsi"
>>  
>>  / {
>>  	model = "LogicPD Zoom DM3730 Torpedo + Wireless Development Kit";
>> diff --git a/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi b/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
>> index 533a47bc4a53..05049a34b6f1 100644
>> --- a/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
>> +++ b/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
>> @@ -95,7 +95,7 @@
>>  	ranges = <0 0 0x30000000 0x1000000	/* CS0: 16MB for NAND */
>>  		  1 0 0x2c000000 0x1000000>;	/* CS1: 16MB for LAN9221 */
>>  
>> -	ethernet@gpmc {
>> +	gpmc_ethernet: ethernet@1,0 {
>>  		pinctrl-names = "default";
>>  		pinctrl-0 = <&lan9221_pins>;
>>  		interrupt-parent = <&gpio5>;
>> diff --git a/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi b/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
>> index ded7e8fec9eb..2a462cb65a7d 100644
>> --- a/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
>> +++ b/arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi
>> @@ -20,36 +20,34 @@
>>  	};
>>  };
>>  
>> -&gpmc {
>> -	ethernet@gpmc {
>> -		compatible = "smsc,lan9221", "smsc,lan9115";
>> -		bank-width = <2>;
>> -		gpmc,device-width = <1>;
>> -		gpmc,cycle2cycle-samecsen = <1>;
>> -		gpmc,cycle2cycle-diffcsen = <1>;
>> -		gpmc,cs-on-ns = <5>;
>> -		gpmc,cs-rd-off-ns = <150>;
>> -		gpmc,cs-wr-off-ns = <150>;
>> -		gpmc,adv-on-ns = <0>;
>> -		gpmc,adv-rd-off-ns = <15>;
>> -		gpmc,adv-wr-off-ns = <40>;
>> -		gpmc,oe-on-ns = <45>;
>> -		gpmc,oe-off-ns = <140>;
>> -		gpmc,we-on-ns = <45>;
>> -		gpmc,we-off-ns = <140>;
>> -		gpmc,rd-cycle-ns = <155>;
>> -		gpmc,wr-cycle-ns = <155>;
>> -		gpmc,access-ns = <120>;
>> -		gpmc,page-burst-access-ns = <20>;
>> -		gpmc,bus-turnaround-ns = <75>;
>> -		gpmc,cycle2cycle-delay-ns = <75>;
>> -		gpmc,wait-monitoring-ns = <0>;
>> -		gpmc,clk-activation-ns = <0>;
>> -		gpmc,wr-data-mux-bus-ns = <0>;
>> -		gpmc,wr-access-ns = <0>;
>> -		vddvario-supply = <&vddvario>;
>> -		vdd33a-supply = <&vdd33a>;
>> -		reg-io-width = <4>;
>> -		smsc,save-mac-address;
>> -	};
>> +&gpmc_ethernet {
>> +	compatible = "smsc,lan9221", "smsc,lan9115";
> 
> This looks like regular override-by-label instead of full path.
> Unfortunately change of the indentation causes difficulties to find the
> real difference - if there is such. Can you split it into separate patch?
> 
> The point is that override-by-label should have zero effect on
> functionality and produce same dtb. This is easy to compare with
> dtx_diff or fdt-decompile but if you mix it with other changes, the
> comparison is not straight-forward.

OK. I will split the overide-by-label to separate patch and ensure there is no
change in compiled dtb for that patch.

> 
>> +	bank-width = <2>;
>> +	gpmc,device-width = <1>;
>> +	gpmc,cycle2cycle-samecsen;
>> +	gpmc,cycle2cycle-diffcsen;
>> +	gpmc,cs-on-ns = <5>;
>> +	gpmc,cs-rd-off-ns = <150>;
>> +	gpmc,cs-wr-off-ns = <150>;
>> +	gpmc,adv-on-ns = <0>;
>> +	gpmc,adv-rd-off-ns = <15>;
>> +	gpmc,adv-wr-off-ns = <40>;
>> +	gpmc,oe-on-ns = <45>;
>> +	gpmc,oe-off-ns = <140>;
>> +	gpmc,we-on-ns = <45>;
>> +	gpmc,we-off-ns = <140>;
>> +	gpmc,rd-cycle-ns = <155>;
>> +	gpmc,wr-cycle-ns = <155>;
>> +	gpmc,access-ns = <120>;
>> +	gpmc,page-burst-access-ns = <20>;
>> +	gpmc,bus-turnaround-ns = <75>;
>> +	gpmc,cycle2cycle-delay-ns = <75>;
>> +	gpmc,wait-monitoring-ns = <0>;
>> +	gpmc,clk-activation-ns = <0>;
>> +	gpmc,wr-data-mux-bus-ns = <0>;
>> +	gpmc,wr-access-ns = <0>;
>> +	vddvario-supply = <&vddvario>;
>> +	vdd33a-supply = <&vdd33a>;
>> +	reg-io-width = <4>;
>> +	smsc,save-mac-address;
>>  };
>> diff --git a/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi b/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
>> index 7f6aefd13451..c1e78f929d2b 100644
>> --- a/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
>> +++ b/arch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
>> @@ -24,36 +24,33 @@
>>  	};
>>  };
>>  
>> -&gpmc {
>> -	ethernet@gpmc {
>> -		compatible = "smsc,lan9221","smsc,lan9115";
>> -		bank-width = <2>;
>> +&gpmc_ethernet {
>> +	compatible = "smsc,lan9221","smsc,lan9115";
>> +	bank-width = <2>;
>> +	gpmc,mux-add-data = <0>;
>> +	gpmc,cs-on-ns = <0>;
>> +	gpmc,cs-rd-off-ns = <42>;
>> +	gpmc,cs-wr-off-ns = <36>;
>> +	gpmc,adv-on-ns = <6>;
>> +	gpmc,adv-rd-off-ns = <12>;
>> +	gpmc,adv-wr-off-ns = <12>;
>> +	gpmc,oe-on-ns = <0>;
>> +	gpmc,oe-off-ns = <42>;
>> +	gpmc,we-on-ns = <0>;
>> +	gpmc,we-off-ns = <36>;
>> +	gpmc,rd-cycle-ns = <60>;
>> +	gpmc,wr-cycle-ns = <54>;
>> +	gpmc,access-ns = <36>;
>> +	gpmc,page-burst-access-ns = <0>;
>> +	gpmc,bus-turnaround-ns = <0>;
>> +	gpmc,cycle2cycle-delay-ns = <0>;
>> +	gpmc,wr-data-mux-bus-ns = <18>;
>> +	gpmc,wr-access-ns = <42>;
>> +	gpmc,cycle2cycle-samecsen;
>> +	gpmc,cycle2cycle-diffcsen;
>>  
>> -		gpmc,mux-add-data;
> 
> Same here and in other places. Actually here a sneaky change is visible
> - different property.
> 
> Best regards,
> Krzysztof
> 

cheers,
-roger

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

* Re: [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional
  2021-09-07 12:36   ` Krzysztof Kozlowski
@ 2021-09-15  9:11     ` Roger Quadros
  2021-09-16 10:48       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 24+ messages in thread
From: Roger Quadros @ 2021-09-15  9:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski, tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	miquel.raynal, devicetree, linux-mtd, linux-omap, linux-kernel

Hi Krzysztof,

On 07/09/2021 15:36, Krzysztof Kozlowski wrote:
> On 07/09/2021 13:32, Roger Quadros wrote:
>> Check for valid gpmc,device-width, nand-bus-width and bank-width
>> at one place. Default to 8-bit width if none present.
> 
> I don't understand the message in the context of the patch. The title
> says one property is optional - that's it. The message says you
> consolidate checks. How is this related to the title?
> 
> The patch itself moves around checking of properties and reads
> nand-bus-width *always*. It does not "check at one place" but rather
> "check always". In the same time, the patch does not remove
> gpmc,device-width check in other place.
> 
> All three elements - the title, message and patch - do different things.
> What did you want to achieve here? Can you help in clarifying it?
> 

OK I will explain it better in commit log in next revision. Let me explain here a bit.

Prior to this patch it was working like this

	/* in gpmc_read_settings_dt() */
	s->device_width = 0;	/* invalid width, should be 1 for 8-bit, 2 for 16-bit */
	of_property_read_u32(np, "gpmc,device-width", s->device_width);

	/* in gpmc_probe_generic_child () */
	if (of_device_is_compatible(child, "ti,omap2-nand")) {
		/* check for nand-bus-width, if absent set s->device_width to 1 (i.e. 8-bit) */
	} else {
		/* check for bank-width, if absent and s->device_width not set, error out */
	}

So that means if all three, "gpmc,device-width". "nand-bus-width" and "bank-width" are missing then
it would create an error situation.

The patch is doing 3 things.
1) Make sure all DT checks related to bus width are being done at one place for better readability.
2) even if all 3 width properties are absent, we will not treat it as error and default to 8-bit.
3) check for nand-bus-width regardless of whether compatible to "ti,omap2-nand" or not.

Hope this explains well.

cheers,
-roger

> 
> Best regards,
> Krzysztof
> 
> 
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  drivers/memory/omap-gpmc.c | 41 ++++++++++++++++++++++++--------------
>>  1 file changed, 26 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>> index f80c2ea39ca4..32d7c665f33c 100644
>> --- a/drivers/memory/omap-gpmc.c
>> +++ b/drivers/memory/omap-gpmc.c
>> @@ -2171,10 +2171,8 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>  		}
>>  	}
>>  
>> -	if (of_device_is_compatible(child, "ti,omap2-nand")) {
>> -		/* NAND specific setup */
>> -		val = 8;
>> -		of_property_read_u32(child, "nand-bus-width", &val);
>> +	/* DT node can have "nand-bus-width" or "bank-width" or "gpmc,device-width" */
>> +	if (!of_property_read_u32(child, "nand-bus-width", &val)) {
>>  		switch (val) {
>>  		case 8:
>>  			gpmc_s.device_width = GPMC_DEVWIDTH_8BIT;
>> @@ -2183,24 +2181,37 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>  			gpmc_s.device_width = GPMC_DEVWIDTH_16BIT;
>>  			break;
>>  		default:
>> -			dev_err(&pdev->dev, "%pOFn: invalid 'nand-bus-width'\n",
>> -				child);
>> +			dev_err(&pdev->dev,
>> +				"%pOFn: invalid 'nand-bus-width':%d\n", child, val);
>> +			ret = -EINVAL;
>> +			goto err;
>> +		}
>> +	} else if (!of_property_read_u32(child, "bank-width", &val)) {
>> +		if (val != 1 && val != 2) {
>> +			dev_err(&pdev->dev,
>> +				"%pOFn: invalid 'bank-width':%d\n", child, val);
>>  			ret = -EINVAL;
>>  			goto err;
>>  		}
>> +		gpmc_s.device_width = val;
>> +	} else if (!of_property_read_u32(child, "gpmc,device-width", &val)) {
>> +		if (val != 1 && val != 2) {
>> +			dev_err(&pdev->dev,
>> +				"%pOFn: invalid 'gpmc,device-width':%d\n", child, val);
>> +			ret = -EINVAL;
>> +			goto err;
>> +		}
>> +		gpmc_s.device_width = val;
>> +	} else {
>> +		/* default to 8-bit */
>> +		gpmc_s.device_width = GPMC_DEVWIDTH_8BIT;
>> +	}
>>  
>> +	if (of_device_is_compatible(child, "ti,omap2-nand")) {
>> +		/* NAND specific setup */
>>  		/* disable write protect */
>>  		gpmc_configure(GPMC_CONFIG_WP, 0);
>>  		gpmc_s.device_nand = true;
>> -	} else {
>> -		ret = of_property_read_u32(child, "bank-width",
>> -					   &gpmc_s.device_width);
>> -		if (ret < 0 && !gpmc_s.device_width) {
>> -			dev_err(&pdev->dev,
>> -				"%pOF has no 'gpmc,device-width' property\n",
>> -				child);
>> -			goto err;
>> -		}
>>  	}
>>  
>>  	/* Reserve wait pin if it is required and valid */
>>
> 
> 

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

* Re: [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional
  2021-09-15  9:11     ` Roger Quadros
@ 2021-09-16 10:48       ` Krzysztof Kozlowski
  2021-09-17  7:17         ` Roger Quadros
  0 siblings, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2021-09-16 10:48 UTC (permalink / raw)
  To: Roger Quadros, tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	miquel.raynal, devicetree, linux-mtd, linux-omap, linux-kernel

On 15/09/2021 11:11, Roger Quadros wrote:
> Hi Krzysztof,
> 
> On 07/09/2021 15:36, Krzysztof Kozlowski wrote:
>> On 07/09/2021 13:32, Roger Quadros wrote:
>>> Check for valid gpmc,device-width, nand-bus-width and bank-width
>>> at one place. Default to 8-bit width if none present.
>>
>> I don't understand the message in the context of the patch. The title
>> says one property is optional - that's it. The message says you
>> consolidate checks. How is this related to the title?
>>
>> The patch itself moves around checking of properties and reads
>> nand-bus-width *always*. It does not "check at one place" but rather
>> "check always". In the same time, the patch does not remove
>> gpmc,device-width check in other place.
>>
>> All three elements - the title, message and patch - do different things.
>> What did you want to achieve here? Can you help in clarifying it?
>>
> 
> OK I will explain it better in commit log in next revision. Let me explain here a bit.
> 
> Prior to this patch it was working like this
> 
> 	/* in gpmc_read_settings_dt() */
> 	s->device_width = 0;	/* invalid width, should be 1 for 8-bit, 2 for 16-bit */
> 	of_property_read_u32(np, "gpmc,device-width", s->device_width);
> 
> 	/* in gpmc_probe_generic_child () */
> 	if (of_device_is_compatible(child, "ti,omap2-nand")) {
> 		/* check for nand-bus-width, if absent set s->device_width to 1 (i.e. 8-bit) */
> 	} else {
> 		/* check for bank-width, if absent and s->device_width not set, error out */
> 	}
> 
> So that means if all three, "gpmc,device-width". "nand-bus-width" and "bank-width" are missing then
> it would create an error situation.
> 
> The patch is doing 3 things.
> 1) Make sure all DT checks related to bus width are being done at one place for better readability.

Not entirely. The gpmc,device-width is still done in the other place
because you did not remove it from the code. Unless you meant parsing of
gpmc,device-width not reading from DT? But then another round of checks
is in gpmc_cs_program_settings() so not in one place.

If you consolidate the checks to one place, I would expect the code to
be removed from other places, so from gpmc_cs_program_settings() and
gpmc_read_settings_dt(). Since this is not happening, the message
confuses me.

> 2) even if all 3 width properties are absent, we will not treat it as error and default to 8-bit.

This is not mentioned in commit msg.

> 3) check for nand-bus-width regardless of whether compatible to "ti,omap2-nand" or not.

Also not mentioned in commit msg.

Your commit reorganizes parsing and validating the child DT properties
but it does not change from "multiple place" to "one place".

At least I don't see it.

Best regards,
Krzysztof

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

* Re: [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional
  2021-09-16 10:48       ` Krzysztof Kozlowski
@ 2021-09-17  7:17         ` Roger Quadros
  0 siblings, 0 replies; 24+ messages in thread
From: Roger Quadros @ 2021-09-17  7:17 UTC (permalink / raw)
  To: Krzysztof Kozlowski, tony
  Cc: robh+dt, grygorii.strashko, nm, lokeshvutla, nsekhar,
	miquel.raynal, devicetree, linux-mtd, linux-omap, linux-kernel


On 16/09/2021 13:48, Krzysztof Kozlowski wrote:
> On 15/09/2021 11:11, Roger Quadros wrote:
>> Hi Krzysztof,
>>
>> On 07/09/2021 15:36, Krzysztof Kozlowski wrote:
>>> On 07/09/2021 13:32, Roger Quadros wrote:
>>>> Check for valid gpmc,device-width, nand-bus-width and bank-width
>>>> at one place. Default to 8-bit width if none present.
>>>
>>> I don't understand the message in the context of the patch. The title
>>> says one property is optional - that's it. The message says you
>>> consolidate checks. How is this related to the title?
>>>
>>> The patch itself moves around checking of properties and reads
>>> nand-bus-width *always*. It does not "check at one place" but rather
>>> "check always". In the same time, the patch does not remove
>>> gpmc,device-width check in other place.
>>>
>>> All three elements - the title, message and patch - do different things.
>>> What did you want to achieve here? Can you help in clarifying it?
>>>
>>
>> OK I will explain it better in commit log in next revision. Let me explain here a bit.
>>
>> Prior to this patch it was working like this
>>
>> 	/* in gpmc_read_settings_dt() */
>> 	s->device_width = 0;	/* invalid width, should be 1 for 8-bit, 2 for 16-bit */
>> 	of_property_read_u32(np, "gpmc,device-width", s->device_width);
>>
>> 	/* in gpmc_probe_generic_child () */
>> 	if (of_device_is_compatible(child, "ti,omap2-nand")) {
>> 		/* check for nand-bus-width, if absent set s->device_width to 1 (i.e. 8-bit) */
>> 	} else {
>> 		/* check for bank-width, if absent and s->device_width not set, error out */
>> 	}
>>
>> So that means if all three, "gpmc,device-width". "nand-bus-width" and "bank-width" are missing then
>> it would create an error situation.
>>
>> The patch is doing 3 things.
>> 1) Make sure all DT checks related to bus width are being done at one place for better readability.
> 
> Not entirely. The gpmc,device-width is still done in the other place
> because you did not remove it from the code. Unless you meant parsing of
> gpmc,device-width not reading from DT? But then another round of checks
> is in gpmc_cs_program_settings() so not in one place.

By checking I meant parsing. But you are right, I missed the part in gpmc_cs_program_settings().

> 
> If you consolidate the checks to one place, I would expect the code to
> be removed from other places, so from gpmc_cs_program_settings() and
> gpmc_read_settings_dt(). Since this is not happening, the message
> confuses me.
> 
>> 2) even if all 3 width properties are absent, we will not treat it as error and default to 8-bit.
> 
> This is not mentioned in commit msg.
> 
>> 3) check for nand-bus-width regardless of whether compatible to "ti,omap2-nand" or not.
> 
> Also not mentioned in commit msg.
> 
> Your commit reorganizes parsing and validating the child DT properties
> but it does not change from "multiple place" to "one place".
> 
> At least I don't see it.

OK. I will write a better commit log next time. Thanks for the review :)

cheers,
-roger

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

end of thread, other threads:[~2021-09-17  7:17 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-07 11:32 [PATCH v3 0/8] dt-bindings: memory-controllers: ti,gpmc: Convert to yaml Roger Quadros
2021-09-07 11:32 ` [PATCH v3 1/8] ARM: dts: omap: Fixup GPMC child nodes Roger Quadros
2021-09-07 12:44   ` Krzysztof Kozlowski
2021-09-15  8:53     ` Roger Quadros
2021-09-07 11:32 ` [PATCH v3 2/8] dt-bindings: mtd: Remove gpmc-nor.txt Roger Quadros
2021-09-07 11:32 ` [PATCH v3 3/8] dt-bindings: net: Remove gpmc-eth.txt Roger Quadros
2021-09-07 11:32 ` [PATCH v3 4/8] dt-bindings: memory-controllers: Introduce ti,gpmc-child Roger Quadros
2021-09-07 11:32 ` [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml Roger Quadros
2021-09-07 14:03   ` Miquel Raynal
2021-09-07 15:27     ` Grygorii Strashko
2021-09-07 16:35       ` Miquel Raynal
2021-09-07 16:57         ` Roger Quadros
2021-09-07 22:24           ` Rob Herring
2021-09-08  6:55             ` Roger Quadros
2021-09-07 17:01   ` Rob Herring
2021-09-08 11:14     ` Roger Quadros
2021-09-08 12:46       ` Rob Herring
2021-09-07 11:32 ` [PATCH v3 6/8] dt-bindings: mtd: ti,gpmc-onenand: " Roger Quadros
2021-09-07 11:32 ` [PATCH v3 7/8] dt-bindings: memory-controllers: ti,gpmc: " Roger Quadros
2021-09-07 11:32 ` [PATCH v3 8/8] memory: gpmc-omap: "gpmc,device-width" DT property is optional Roger Quadros
2021-09-07 12:36   ` Krzysztof Kozlowski
2021-09-15  9:11     ` Roger Quadros
2021-09-16 10:48       ` Krzysztof Kozlowski
2021-09-17  7:17         ` Roger Quadros

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