All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-24 18:32 ` Hauke Mehrtens
  0 siblings, 0 replies; 22+ messages in thread
From: Hauke Mehrtens @ 2015-05-24 18:32 UTC (permalink / raw
  To: computersforpeace, f.fainelli
  Cc: devicetree, rjui, zajec5, jogo, linux-mtd, Hauke Mehrtens,
	bcm-kernel-feedback-list

This adds the NAND flash chip description for a standard chip found
connected to this SoC. This makes use of generic Broadcom NAND driver
with the iProc interface.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---

This would be the patch when we completely change to device tree only 
for the nand flash controller.

 arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts        |  4 +++
 arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts        |  4 +++
 arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts  |  4 +++
 arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts       |  8 ++---
 arch/arm/boot/dts/bcm4708-netgear-r6250.dts        |  4 +++
 arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts     |  4 +++
 arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts      |  4 +++
 arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts        |  4 +++
 arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts |  4 +++
 arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts  |  4 +++
 arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts  |  4 +++
 arch/arm/boot/dts/bcm4709-netgear-r8000.dts        |  4 +++
 arch/arm/boot/dts/bcm5301x.dtsi                    | 38 +++++++++++++++-------
 13 files changed, 74 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
index 71cff8d..f9e187f 100644
--- a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
+++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
index 8b62836..0559dcf 100644
--- a/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
+++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts b/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
index 78e95c0..6e1ab06 100644
--- a/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
+++ b/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	spi {
 		compatible = "spi-gpio";
 		num-chipselects = <1>;
diff --git a/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts b/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
index 946c728..1aa141c 100644
--- a/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
+++ b/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
@@ -23,12 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
-	axi@18000000 {
-		nand@28000 {
-			reg = <0x00028000 0x1000>;
-			#address-cells = <1>;
-			#size-cells = <1>;
+	nand: nand@18028000 {
+		status = "ok";
 
+		nandcs@0 {
 			partition@0 {
 				label = "ubi";
 				reg = <0x00000000 0x08000000>;
diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6250.dts b/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
index 2ed9e57..236e0ee 100644
--- a/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
+++ b/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
@@ -33,6 +33,10 @@
 		};
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
index 3991042..d36eee5 100644
--- a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
+++ b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts b/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
index 66dfb53..bd4ad04 100644
--- a/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
+++ b/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts b/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
index 0ee85ea..815d3b7 100644
--- a/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
+++ b/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts b/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
index db9131e..488d2b6 100644
--- a/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
+++ b/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	spi {
 		compatible = "spi-gpio";
 		num-chipselects = <1>;
diff --git a/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts b/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
index 7d6868a..547a70a 100644
--- a/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
+++ b/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		#address-cells = <1>;
diff --git a/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts b/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
index 548e93b..ad2b27f 100644
--- a/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
+++ b/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
index ea26dd3..ff50fd4 100644
--- a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
+++ b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index 78aec62..cb86748 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -124,17 +124,7 @@
 			<0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
 
 			/* Ethernet Controller 3 */
-			<0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
-
-			/* NAND Controller */
-			<0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+			<0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
 
 		chipcommon: chipcommon@0 {
 			reg = <0x00000000 0x1000>;
@@ -143,4 +133,30 @@
 			#gpio-cells = <2>;
 		};
 	};
+
+	nand: nand@18028000 {
+		compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
+		reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
+		reg-names = "nand", "iproc-idm", "iproc-ext";
+		interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+
+		status = "disabled";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		brcm,nand-has-wp;
+
+		nandcs@0 {
+			compatible = "brcm,nandcs";
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			nand-ecc-strength = <8>;
+			nand-ecc-step-size = <512>;
+
+			linux,part-probe = "ofpart", "bcm47xxpart";
+		};
+	};
 };
-- 
2.1.4

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

* [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-24 18:32 ` Hauke Mehrtens
  0 siblings, 0 replies; 22+ messages in thread
From: Hauke Mehrtens @ 2015-05-24 18:32 UTC (permalink / raw
  To: computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w, Hauke Mehrtens

This adds the NAND flash chip description for a standard chip found
connected to this SoC. This makes use of generic Broadcom NAND driver
with the iProc interface.

Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
---

This would be the patch when we completely change to device tree only 
for the nand flash controller.

 arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts        |  4 +++
 arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts        |  4 +++
 arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts  |  4 +++
 arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts       |  8 ++---
 arch/arm/boot/dts/bcm4708-netgear-r6250.dts        |  4 +++
 arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts     |  4 +++
 arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts      |  4 +++
 arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts        |  4 +++
 arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts |  4 +++
 arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts  |  4 +++
 arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts  |  4 +++
 arch/arm/boot/dts/bcm4709-netgear-r8000.dts        |  4 +++
 arch/arm/boot/dts/bcm5301x.dtsi                    | 38 +++++++++++++++-------
 13 files changed, 74 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
index 71cff8d..f9e187f 100644
--- a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
+++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
index 8b62836..0559dcf 100644
--- a/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
+++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts b/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
index 78e95c0..6e1ab06 100644
--- a/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
+++ b/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	spi {
 		compatible = "spi-gpio";
 		num-chipselects = <1>;
diff --git a/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts b/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
index 946c728..1aa141c 100644
--- a/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
+++ b/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
@@ -23,12 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
-	axi@18000000 {
-		nand@28000 {
-			reg = <0x00028000 0x1000>;
-			#address-cells = <1>;
-			#size-cells = <1>;
+	nand: nand@18028000 {
+		status = "ok";
 
+		nandcs@0 {
 			partition@0 {
 				label = "ubi";
 				reg = <0x00000000 0x08000000>;
diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6250.dts b/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
index 2ed9e57..236e0ee 100644
--- a/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
+++ b/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
@@ -33,6 +33,10 @@
 		};
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
index 3991042..d36eee5 100644
--- a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
+++ b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts b/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
index 66dfb53..bd4ad04 100644
--- a/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
+++ b/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts b/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
index 0ee85ea..815d3b7 100644
--- a/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
+++ b/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts b/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
index db9131e..488d2b6 100644
--- a/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
+++ b/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	spi {
 		compatible = "spi-gpio";
 		num-chipselects = <1>;
diff --git a/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts b/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
index 7d6868a..547a70a 100644
--- a/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
+++ b/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		#address-cells = <1>;
diff --git a/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts b/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
index 548e93b..ad2b27f 100644
--- a/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
+++ b/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
index ea26dd3..ff50fd4 100644
--- a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
+++ b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
@@ -23,6 +23,10 @@
 		reg = <0x00000000 0x08000000>;
 	};
 
+	nand: nand@18028000 {
+		status = "ok";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index 78aec62..cb86748 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -124,17 +124,7 @@
 			<0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
 
 			/* Ethernet Controller 3 */
-			<0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
-
-			/* NAND Controller */
-			<0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
-			<0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+			<0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
 
 		chipcommon: chipcommon@0 {
 			reg = <0x00000000 0x1000>;
@@ -143,4 +133,30 @@
 			#gpio-cells = <2>;
 		};
 	};
+
+	nand: nand@18028000 {
+		compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
+		reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
+		reg-names = "nand", "iproc-idm", "iproc-ext";
+		interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+
+		status = "disabled";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		brcm,nand-has-wp;
+
+		nandcs@0 {
+			compatible = "brcm,nandcs";
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			nand-ecc-strength = <8>;
+			nand-ecc-step-size = <512>;
+
+			linux,part-probe = "ofpart", "bcm47xxpart";
+		};
+	};
 };
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-26 20:20   ` Rafał Miłecki
  0 siblings, 0 replies; 22+ messages in thread
From: Rafał Miłecki @ 2015-05-26 20:20 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: devicetree@vger.kernel.org, Florian Fainelli, Ray Jui,
	Jonas Gorski, bcm-kernel-feedback-list,
	linux-mtd@lists.infradead.org, Brian Norris

On 24 May 2015 at 20:32, Hauke Mehrtens <hauke@hauke-m.de> wrote:
> This adds the NAND flash chip description for a standard chip found
> connected to this SoC. This makes use of generic Broadcom NAND driver
> with the iProc interface.

I still wait/hope for Brian's reply in
[PATCH 5/7] mtd: brcmnand: add bcma driver

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-26 20:20   ` Rafał Miłecki
  0 siblings, 0 replies; 22+ messages in thread
From: Rafał Miłecki @ 2015-05-26 20:20 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: Brian Norris, Florian Fainelli,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Ray Jui, bcm-kernel-feedback-list,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jonas Gorski

On 24 May 2015 at 20:32, Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org> wrote:
> This adds the NAND flash chip description for a standard chip found
> connected to this SoC. This makes use of generic Broadcom NAND driver
> with the iProc interface.

I still wait/hope for Brian's reply in
[PATCH 5/7] mtd: brcmnand: add bcma driver
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
  2015-05-26 20:20   ` Rafał Miłecki
@ 2015-05-26 20:53     ` nick
  -1 siblings, 0 replies; 22+ messages in thread
From: nick @ 2015-05-26 20:53 UTC (permalink / raw
  To: Rafał Miłecki, Hauke Mehrtens
  Cc: devicetree@vger.kernel.org, Florian Fainelli, Ray Jui,
	Jonas Gorski, bcm-kernel-feedback-list,
	linux-mtd@lists.infradead.org, Brian Norris



On 2015-05-26 04:20 PM, Rafał Miłecki wrote:
> On 24 May 2015 at 20:32, Hauke Mehrtens <hauke@hauke-m.de> wrote:
>> This adds the NAND flash chip description for a standard chip found
>> connected to this SoC. This makes use of generic Broadcom NAND driver
>> with the iProc interface.
> 
> I still wait/hope for Brian's reply in
> [PATCH 5/7] mtd: brcmnand: add bcma driver
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
Rafal,
How long has it been as if its over a week I would like resend the patch.
Nick

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-26 20:53     ` nick
  0 siblings, 0 replies; 22+ messages in thread
From: nick @ 2015-05-26 20:53 UTC (permalink / raw
  To: Rafał Miłecki, Hauke Mehrtens
  Cc: devicetree@vger.kernel.org, Florian Fainelli, Ray Jui,
	Jonas Gorski, bcm-kernel-feedback-list,
	linux-mtd@lists.infradead.org, Brian Norris



On 2015-05-26 04:20 PM, Rafał Miłecki wrote:
> On 24 May 2015 at 20:32, Hauke Mehrtens <hauke@hauke-m.de> wrote:
>> This adds the NAND flash chip description for a standard chip found
>> connected to this SoC. This makes use of generic Broadcom NAND driver
>> with the iProc interface.
> 
> I still wait/hope for Brian's reply in
> [PATCH 5/7] mtd: brcmnand: add bcma driver
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
Rafal,
How long has it been as if its over a week I would like resend the patch.
Nick

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27  0:41   ` Brian Norris
  0 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2015-05-27  0:41 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: devicetree, f.fainelli, rjui, zajec5, jogo, linux-mtd,
	bcm-kernel-feedback-list

On Sun, May 24, 2015 at 08:32:29PM +0200, Hauke Mehrtens wrote:
> This adds the NAND flash chip description for a standard chip found
> connected to this SoC. This makes use of generic Broadcom NAND driver
> with the iProc interface.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> 
> This would be the patch when we completely change to device tree only 
> for the nand flash controller.
> 
>  arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts        |  4 +++
>  arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts        |  4 +++
>  arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts  |  4 +++
>  arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts       |  8 ++---
>  arch/arm/boot/dts/bcm4708-netgear-r6250.dts        |  4 +++
>  arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts     |  4 +++
>  arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts      |  4 +++
>  arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts        |  4 +++
>  arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts |  4 +++
>  arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts  |  4 +++
>  arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts  |  4 +++
>  arch/arm/boot/dts/bcm4709-netgear-r8000.dts        |  4 +++
>  arch/arm/boot/dts/bcm5301x.dtsi                    | 38 +++++++++++++++-------

I'm curious: what tree are you looking at? I don't see most of these
board DTS files. Or are they not intended for upstream, and you're just
using them as examples? (Edit: found them in linux-next. And I see
you're using unreviewed DT snippets, partly by virtue of not using a
'compatible' string for BCMA peripherals.)

>  13 files changed, 74 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> index 71cff8d..f9e187f 100644
> --- a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> +++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> @@ -23,6 +23,10 @@
>  		reg = <0x00000000 0x08000000>;
>  	};
>  
> +	nand: nand@18028000 {
> +		status = "ok";
> +	};

Are there any chips that don't have NAND enabled? I would expect the
presence of the NAND controller to be determined at the chip level, not
the board level. (I alluded to this in my comments to Rafal [1].) So
maybe migrate this to bcm4708.dtsi or bcm5301x.dtsi, depending on
whether there are any bcm5301x variants that have NAND disabled. Or
maybe better: just put the 'status = "ok"' part alongside wherever you
put the chip-select node, so you both enable the controller and a
particular chip-select at the same time.

> +
>  	leds {
>  		compatible = "gpio-leds";
>  
[...]
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
> index 78aec62..cb86748 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
...
> @@ -143,4 +133,30 @@
>  			#gpio-cells = <2>;
>  		};
>  	};
> +
> +	nand: nand@18028000 {
> +		compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
> +		reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
> +		reg-names = "nand", "iproc-idm", "iproc-ext";
> +		interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		status = "disabled";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		brcm,nand-has-wp;
> +
> +		nandcs@0 {
> +			compatible = "brcm,nandcs";
> +			reg = <0>;

So every board that uses a BCM5301x chips must use chip-select 0? What
if the board has NAND wired up on CS1? (I see this all the time on
BCM7xxx boards.)

> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			nand-ecc-strength = <8>;
> +			nand-ecc-step-size = <512>;

So, every board using the BCM5301x family must use BCH-8? What happens
if there are boards that use a less reliable flash that needs, e.g.,
BCH-16?

IOW, none of the nandcs@0 node belongs in the top-level chip
description. This all belongs in the board description. If it really is
repeated on tons of boards, then maybe you just need a separate
'nand-cs0-bch8.dtsi' or something like that.

> +
> +			linux,part-probe = "ofpart", "bcm47xxpart";

^ NAK to this line. You still haven't documented any semantics for this
property. And I gave you several comments on your previous patch about
what would need to change before I'd accept this property.

> +		};
> +	};
>  };

Brian

[1] http://lists.infradead.org/pipermail/linux-mtd/2015-May/059419.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27  0:41   ` Brian Norris
  0 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2015-05-27  0:41 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w

On Sun, May 24, 2015 at 08:32:29PM +0200, Hauke Mehrtens wrote:
> This adds the NAND flash chip description for a standard chip found
> connected to this SoC. This makes use of generic Broadcom NAND driver
> with the iProc interface.
> 
> Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
> ---
> 
> This would be the patch when we completely change to device tree only 
> for the nand flash controller.
> 
>  arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts        |  4 +++
>  arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts        |  4 +++
>  arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts  |  4 +++
>  arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts       |  8 ++---
>  arch/arm/boot/dts/bcm4708-netgear-r6250.dts        |  4 +++
>  arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts     |  4 +++
>  arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts      |  4 +++
>  arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts        |  4 +++
>  arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts |  4 +++
>  arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts  |  4 +++
>  arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts  |  4 +++
>  arch/arm/boot/dts/bcm4709-netgear-r8000.dts        |  4 +++
>  arch/arm/boot/dts/bcm5301x.dtsi                    | 38 +++++++++++++++-------

I'm curious: what tree are you looking at? I don't see most of these
board DTS files. Or are they not intended for upstream, and you're just
using them as examples? (Edit: found them in linux-next. And I see
you're using unreviewed DT snippets, partly by virtue of not using a
'compatible' string for BCMA peripherals.)

>  13 files changed, 74 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> index 71cff8d..f9e187f 100644
> --- a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> +++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> @@ -23,6 +23,10 @@
>  		reg = <0x00000000 0x08000000>;
>  	};
>  
> +	nand: nand@18028000 {
> +		status = "ok";
> +	};

Are there any chips that don't have NAND enabled? I would expect the
presence of the NAND controller to be determined at the chip level, not
the board level. (I alluded to this in my comments to Rafal [1].) So
maybe migrate this to bcm4708.dtsi or bcm5301x.dtsi, depending on
whether there are any bcm5301x variants that have NAND disabled. Or
maybe better: just put the 'status = "ok"' part alongside wherever you
put the chip-select node, so you both enable the controller and a
particular chip-select at the same time.

> +
>  	leds {
>  		compatible = "gpio-leds";
>  
[...]
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
> index 78aec62..cb86748 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
...
> @@ -143,4 +133,30 @@
>  			#gpio-cells = <2>;
>  		};
>  	};
> +
> +	nand: nand@18028000 {
> +		compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
> +		reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
> +		reg-names = "nand", "iproc-idm", "iproc-ext";
> +		interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		status = "disabled";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		brcm,nand-has-wp;
> +
> +		nandcs@0 {
> +			compatible = "brcm,nandcs";
> +			reg = <0>;

So every board that uses a BCM5301x chips must use chip-select 0? What
if the board has NAND wired up on CS1? (I see this all the time on
BCM7xxx boards.)

> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			nand-ecc-strength = <8>;
> +			nand-ecc-step-size = <512>;

So, every board using the BCM5301x family must use BCH-8? What happens
if there are boards that use a less reliable flash that needs, e.g.,
BCH-16?

IOW, none of the nandcs@0 node belongs in the top-level chip
description. This all belongs in the board description. If it really is
repeated on tons of boards, then maybe you just need a separate
'nand-cs0-bch8.dtsi' or something like that.

> +
> +			linux,part-probe = "ofpart", "bcm47xxpart";

^ NAK to this line. You still haven't documented any semantics for this
property. And I gave you several comments on your previous patch about
what would need to change before I'd accept this property.

> +		};
> +	};
>  };

Brian

[1] http://lists.infradead.org/pipermail/linux-mtd/2015-May/059419.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
  2015-05-27  0:41   ` Brian Norris
@ 2015-05-27  1:15     ` Brian Norris
  -1 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2015-05-27  1:15 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: devicetree, f.fainelli, rjui, zajec5, jogo, linux-mtd,
	bcm-kernel-feedback-list

On Tue, May 26, 2015 at 05:41:44PM -0700, Brian Norris wrote:
> > +			linux,part-probe = "ofpart", "bcm47xxpart";
> 
> ^ NAK to this line. You still haven't documented any semantics for this
> property. And I gave you several comments on your previous patch about
> what would need to change before I'd accept this property.

Just noticed you sent v2 of your previous patch over the weekend, which
does include documentation for this. That's a start, but I'm still not
very happy with the property. I'll put my full comments there
eventually.

Brian

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27  1:15     ` Brian Norris
  0 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2015-05-27  1:15 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w

On Tue, May 26, 2015 at 05:41:44PM -0700, Brian Norris wrote:
> > +			linux,part-probe = "ofpart", "bcm47xxpart";
> 
> ^ NAK to this line. You still haven't documented any semantics for this
> property. And I gave you several comments on your previous patch about
> what would need to change before I'd accept this property.

Just noticed you sent v2 of your previous patch over the weekend, which
does include documentation for this. That's a start, but I'm still not
very happy with the property. I'll put my full comments there
eventually.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27  7:37   ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2015-05-27  7:37 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: devicetree, f.fainelli, rjui, zajec5, jogo,
	bcm-kernel-feedback-list, linux-mtd, computersforpeace

On Sunday 24 May 2015 20:32:29 Hauke Mehrtens wrote:
> @@ -124,17 +124,7 @@
>                         <0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
>  
>                         /* Ethernet Controller 3 */
> -                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
> -
> -                       /* NAND Controller */
> -                       <0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
> +                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
>  
>                 chipcommon: chipcommon@0 {
>                         reg = <0x00000000 0x1000>;
> @@ -143,4 +133,30 @@
>                         #gpio-cells = <2>;
>                 };
>         };
> +
> +       nand: nand@18028000 {
> +               compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
> +               reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
> +               reg-names = "nand", "iproc-idm", "iproc-ext";
> +               interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;

I think I'd rather leave the interrupt-map in the brcm node, and use

	interrupts = <0>;

here.

> +               status = "disabled";
> +
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               brcm,nand-has-wp;
> +
> +               nandcs@0 {
> +                       compatible = "brcm,nandcs";
> +                       reg = <0>;
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +
> +                       nand-ecc-strength = <8>;
> +                       nand-ecc-step-size = <512>;
> +
> +                       linux,part-probe = "ofpart", "bcm47xxpart";
> +               };
> +       };

This seems fine in principle, once the exact binding has been nailed down.

"bcm47xxpart" does not seem like an appropriate string here.

	Arnd

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27  7:37   ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2015-05-27  7:37 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w

On Sunday 24 May 2015 20:32:29 Hauke Mehrtens wrote:
> @@ -124,17 +124,7 @@
>                         <0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
>  
>                         /* Ethernet Controller 3 */
> -                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
> -
> -                       /* NAND Controller */
> -                       <0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
> -                       <0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
> +                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
>  
>                 chipcommon: chipcommon@0 {
>                         reg = <0x00000000 0x1000>;
> @@ -143,4 +133,30 @@
>                         #gpio-cells = <2>;
>                 };
>         };
> +
> +       nand: nand@18028000 {
> +               compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
> +               reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
> +               reg-names = "nand", "iproc-idm", "iproc-ext";
> +               interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;

I think I'd rather leave the interrupt-map in the brcm node, and use

	interrupts = <0>;

here.

> +               status = "disabled";
> +
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               brcm,nand-has-wp;
> +
> +               nandcs@0 {
> +                       compatible = "brcm,nandcs";
> +                       reg = <0>;
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +
> +                       nand-ecc-strength = <8>;
> +                       nand-ecc-step-size = <512>;
> +
> +                       linux,part-probe = "ofpart", "bcm47xxpart";
> +               };
> +       };

This seems fine in principle, once the exact binding has been nailed down.

"bcm47xxpart" does not seem like an appropriate string here.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
  2015-05-27  7:37   ` Arnd Bergmann
@ 2015-05-27 21:46     ` Hauke Mehrtens
  -1 siblings, 0 replies; 22+ messages in thread
From: Hauke Mehrtens @ 2015-05-27 21:46 UTC (permalink / raw
  To: Arnd Bergmann
  Cc: devicetree, f.fainelli, rjui, zajec5, jogo,
	bcm-kernel-feedback-list, linux-mtd, computersforpeace

On 05/27/2015 09:37 AM, Arnd Bergmann wrote:
> On Sunday 24 May 2015 20:32:29 Hauke Mehrtens wrote:
>> @@ -124,17 +124,7 @@
>>                         <0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
>>  
>>                         /* Ethernet Controller 3 */
>> -                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
>> -
>> -                       /* NAND Controller */
>> -                       <0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>> +                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
>>  
>>                 chipcommon: chipcommon@0 {
>>                         reg = <0x00000000 0x1000>;
>> @@ -143,4 +133,30 @@
>>                         #gpio-cells = <2>;
>>                 };
>>         };
>> +
>> +       nand: nand@18028000 {
>> +               compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
>> +               reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
>> +               reg-names = "nand", "iproc-idm", "iproc-ext";
>> +               interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> 
> I think I'd rather leave the interrupt-map in the brcm node, and use
> 
> 	interrupts = <0>;
> 
> here.

There are two conflicting interests ;-)

In the first approach we probed the NAND controller through bcma and
just added the additional information we can not probe through device
tree. That was this version:
https://patchwork.ozlabs.org/patch/473181/

This version needs some changes to the brcmnand driver and this
additional part:
https://patchwork.ozlabs.org/patch/473182/

This code is simular to the iproc_nand.c

Brian wanted us to use the iproc_nand.c which is possible, but then we
completely ignore what we have probed in bcma and only use device tree.
And this nand driver is not a subnode of the bcma bus any more so we can
not use the irqs defined there.

> 
>> +               status = "disabled";
>> +
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               brcm,nand-has-wp;
>> +
>> +               nandcs@0 {
>> +                       compatible = "brcm,nandcs";
>> +                       reg = <0>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <1>;
>> +
>> +                       nand-ecc-strength = <8>;
>> +                       nand-ecc-step-size = <512>;
>> +
>> +                       linux,part-probe = "ofpart", "bcm47xxpart";
>> +               };
>> +       };
> 
> This seems fine in principle, once the exact binding has been nailed down.
> 
> "bcm47xxpart" does not seem like an appropriate string here.

We are already thinking and talking about how to solve this problem.
There are some parsers that are able to parse some partition layout
based on information various sources or some vendor headers and some are
also guessing in addition, like they are assuming that the bootloader is
in the first block and n bytes long. The current module is that the
flash driver defines which partition parsers to take, it is hard coded
into the code. I do not like that. ;-)

I am trying to get a simple patch in which makes it possible to
overwrite the defaults from the flash driver with device tree.
As it uses the same format it should be easy to convert devices from the
old style to the new one. This deice tree attribute is already supported
by one driver I moved the code to make it available for all drivers.

The patch I am talking about: https://patchwork.ozlabs.org/patch/475988/

Hauke

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27 21:46     ` Hauke Mehrtens
  0 siblings, 0 replies; 22+ messages in thread
From: Hauke Mehrtens @ 2015-05-27 21:46 UTC (permalink / raw
  To: Arnd Bergmann
  Cc: computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w

On 05/27/2015 09:37 AM, Arnd Bergmann wrote:
> On Sunday 24 May 2015 20:32:29 Hauke Mehrtens wrote:
>> @@ -124,17 +124,7 @@
>>                         <0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
>>  
>>                         /* Ethernet Controller 3 */
>> -                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
>> -
>> -                       /* NAND Controller */
>> -                       <0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
>> -                       <0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>> +                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
>>  
>>                 chipcommon: chipcommon@0 {
>>                         reg = <0x00000000 0x1000>;
>> @@ -143,4 +133,30 @@
>>                         #gpio-cells = <2>;
>>                 };
>>         };
>> +
>> +       nand: nand@18028000 {
>> +               compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
>> +               reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
>> +               reg-names = "nand", "iproc-idm", "iproc-ext";
>> +               interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> 
> I think I'd rather leave the interrupt-map in the brcm node, and use
> 
> 	interrupts = <0>;
> 
> here.

There are two conflicting interests ;-)

In the first approach we probed the NAND controller through bcma and
just added the additional information we can not probe through device
tree. That was this version:
https://patchwork.ozlabs.org/patch/473181/

This version needs some changes to the brcmnand driver and this
additional part:
https://patchwork.ozlabs.org/patch/473182/

This code is simular to the iproc_nand.c

Brian wanted us to use the iproc_nand.c which is possible, but then we
completely ignore what we have probed in bcma and only use device tree.
And this nand driver is not a subnode of the bcma bus any more so we can
not use the irqs defined there.

> 
>> +               status = "disabled";
>> +
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               brcm,nand-has-wp;
>> +
>> +               nandcs@0 {
>> +                       compatible = "brcm,nandcs";
>> +                       reg = <0>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <1>;
>> +
>> +                       nand-ecc-strength = <8>;
>> +                       nand-ecc-step-size = <512>;
>> +
>> +                       linux,part-probe = "ofpart", "bcm47xxpart";
>> +               };
>> +       };
> 
> This seems fine in principle, once the exact binding has been nailed down.
> 
> "bcm47xxpart" does not seem like an appropriate string here.

We are already thinking and talking about how to solve this problem.
There are some parsers that are able to parse some partition layout
based on information various sources or some vendor headers and some are
also guessing in addition, like they are assuming that the bootloader is
in the first block and n bytes long. The current module is that the
flash driver defines which partition parsers to take, it is hard coded
into the code. I do not like that. ;-)

I am trying to get a simple patch in which makes it possible to
overwrite the defaults from the flash driver with device tree.
As it uses the same format it should be easy to convert devices from the
old style to the new one. This deice tree attribute is already supported
by one driver I moved the code to make it available for all drivers.

The patch I am talking about: https://patchwork.ozlabs.org/patch/475988/

Hauke
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27 21:57       ` Brian Norris
  0 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2015-05-27 21:57 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: devicetree, f.fainelli, Arnd Bergmann, rjui, zajec5, jogo,
	linux-mtd, bcm-kernel-feedback-list

On Wed, May 27, 2015 at 11:46:48PM +0200, Hauke Mehrtens wrote:
> This deice tree attribute is already supported
> by one driver I moved the code to make it available for all drivers.

To be clear here, though, the original binding was barely reviewed, let
alone documented. So I'd consider a redefinition is perfectly acceptable
if we plan to make wider use of it.

Brian

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27 21:57       ` Brian Norris
  0 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2015-05-27 21:57 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: Arnd Bergmann, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w

On Wed, May 27, 2015 at 11:46:48PM +0200, Hauke Mehrtens wrote:
> This deice tree attribute is already supported
> by one driver I moved the code to make it available for all drivers.

To be clear here, though, the original binding was barely reviewed, let
alone documented. So I'd consider a redefinition is perfectly acceptable
if we plan to make wider use of it.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
  2015-05-27  0:41   ` Brian Norris
@ 2015-05-27 22:04     ` Hauke Mehrtens
  -1 siblings, 0 replies; 22+ messages in thread
From: Hauke Mehrtens @ 2015-05-27 22:04 UTC (permalink / raw
  To: Brian Norris
  Cc: devicetree, f.fainelli, rjui, zajec5, jogo, linux-mtd,
	bcm-kernel-feedback-list

On 05/27/2015 02:41 AM, Brian Norris wrote:
> On Sun, May 24, 2015 at 08:32:29PM +0200, Hauke Mehrtens wrote:
>> This adds the NAND flash chip description for a standard chip found
>> connected to this SoC. This makes use of generic Broadcom NAND driver
>> with the iProc interface.
>>
>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>>
>> This would be the patch when we completely change to device tree only 
>> for the nand flash controller.
>>
>>  arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts        |  4 +++
>>  arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts        |  4 +++
>>  arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts  |  4 +++
>>  arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts       |  8 ++---
>>  arch/arm/boot/dts/bcm4708-netgear-r6250.dts        |  4 +++
>>  arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts     |  4 +++
>>  arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts      |  4 +++
>>  arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts        |  4 +++
>>  arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts |  4 +++
>>  arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts  |  4 +++
>>  arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts  |  4 +++
>>  arch/arm/boot/dts/bcm4709-netgear-r8000.dts        |  4 +++
>>  arch/arm/boot/dts/bcm5301x.dtsi                    | 38 +++++++++++++++-------
> 
> I'm curious: what tree are you looking at? I don't see most of these
> board DTS files. Or are they not intended for upstream, and you're just
> using them as examples? (Edit: found them in linux-next. And I see
> you're using unreviewed DT snippets, partly by virtue of not using a
> 'compatible' string for BCMA peripherals.)

This should go through Florian's Broadcom device tree tree and is based
on that. These additional device are in his tree.

> 
>>  13 files changed, 74 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
>> index 71cff8d..f9e187f 100644
>> --- a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
>> +++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
>> @@ -23,6 +23,10 @@
>>  		reg = <0x00000000 0x08000000>;
>>  	};
>>  
>> +	nand: nand@18028000 {
>> +		status = "ok";
>> +	};
> 
> Are there any chips that don't have NAND enabled? I would expect the
> presence of the NAND controller to be determined at the chip level, not
> the board level. (I alluded to this in my comments to Rafal [1].) So
> maybe migrate this to bcm4708.dtsi or bcm5301x.dtsi, depending on
> whether there are any bcm5301x variants that have NAND disabled. Or
> maybe better: just put the 'status = "ok"' part alongside wherever you
> put the chip-select node, so you both enable the controller and a
> particular chip-select at the same time.

To my knowledge all supported SoCs have a NAND controller, but some are
not using it. There are some device with only serial flash, but I do not
have one with only serial flash. If it is save we can activate it on all
SoCs and boards.

> 
>> +
>>  	leds {
>>  		compatible = "gpio-leds";
>>  
> [...]
>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
>> index 78aec62..cb86748 100644
>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> ...
>> @@ -143,4 +133,30 @@
>>  			#gpio-cells = <2>;
>>  		};
>>  	};
>> +
>> +	nand: nand@18028000 {
>> +		compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
>> +		reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
>> +		reg-names = "nand", "iproc-idm", "iproc-ext";
>> +		interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +		status = "disabled";
>> +
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		brcm,nand-has-wp;
>> +
>> +		nandcs@0 {
>> +			compatible = "brcm,nandcs";
>> +			reg = <0>;
> 
> So every board that uses a BCM5301x chips must use chip-select 0? What
> if the board has NAND wired up on CS1? (I see this all the time on
> BCM7xxx boards.)
> 
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			nand-ecc-strength = <8>;
>> +			nand-ecc-step-size = <512>;
> 
> So, every board using the BCM5301x family must use BCH-8? What happens
> if there are boards that use a less reliable flash that needs, e.g.,
> BCH-16?
> 
> IOW, none of the nandcs@0 node belongs in the top-level chip
> description. This all belongs in the board description. If it really is
> repeated on tons of boards, then maybe you just need a separate
> 'nand-cs0-bch8.dtsi' or something like that.

Ok, I can change it that way. The nand-cs0-bch8.dtsi sonds good.
Currently I do not have a complete overview, but I think at least the
vendor driver only supports Chip select 0 as far as I think and the ecc
values are auto detected and calculated by some strange algorithm I
haven't fully understood.

> 
>> +
>> +			linux,part-probe = "ofpart", "bcm47xxpart";
> 
> ^ NAK to this line. You still haven't documented any semantics for this
> property. And I gave you several comments on your previous patch about
> what would need to change before I'd accept this property.

We should talk about this in the patch adding the binding, I will adapt
this depending on the result of the discussion.

> 
>> +		};
>> +	};
>>  };
> 
> Brian
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2015-May/059419.html
> 

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-27 22:04     ` Hauke Mehrtens
  0 siblings, 0 replies; 22+ messages in thread
From: Hauke Mehrtens @ 2015-05-27 22:04 UTC (permalink / raw
  To: Brian Norris
  Cc: f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w

On 05/27/2015 02:41 AM, Brian Norris wrote:
> On Sun, May 24, 2015 at 08:32:29PM +0200, Hauke Mehrtens wrote:
>> This adds the NAND flash chip description for a standard chip found
>> connected to this SoC. This makes use of generic Broadcom NAND driver
>> with the iProc interface.
>>
>> Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
>> ---
>>
>> This would be the patch when we completely change to device tree only 
>> for the nand flash controller.
>>
>>  arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts        |  4 +++
>>  arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts        |  4 +++
>>  arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts  |  4 +++
>>  arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts       |  8 ++---
>>  arch/arm/boot/dts/bcm4708-netgear-r6250.dts        |  4 +++
>>  arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts     |  4 +++
>>  arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts      |  4 +++
>>  arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts        |  4 +++
>>  arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts |  4 +++
>>  arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts  |  4 +++
>>  arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts  |  4 +++
>>  arch/arm/boot/dts/bcm4709-netgear-r8000.dts        |  4 +++
>>  arch/arm/boot/dts/bcm5301x.dtsi                    | 38 +++++++++++++++-------
> 
> I'm curious: what tree are you looking at? I don't see most of these
> board DTS files. Or are they not intended for upstream, and you're just
> using them as examples? (Edit: found them in linux-next. And I see
> you're using unreviewed DT snippets, partly by virtue of not using a
> 'compatible' string for BCMA peripherals.)

This should go through Florian's Broadcom device tree tree and is based
on that. These additional device are in his tree.

> 
>>  13 files changed, 74 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
>> index 71cff8d..f9e187f 100644
>> --- a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
>> +++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
>> @@ -23,6 +23,10 @@
>>  		reg = <0x00000000 0x08000000>;
>>  	};
>>  
>> +	nand: nand@18028000 {
>> +		status = "ok";
>> +	};
> 
> Are there any chips that don't have NAND enabled? I would expect the
> presence of the NAND controller to be determined at the chip level, not
> the board level. (I alluded to this in my comments to Rafal [1].) So
> maybe migrate this to bcm4708.dtsi or bcm5301x.dtsi, depending on
> whether there are any bcm5301x variants that have NAND disabled. Or
> maybe better: just put the 'status = "ok"' part alongside wherever you
> put the chip-select node, so you both enable the controller and a
> particular chip-select at the same time.

To my knowledge all supported SoCs have a NAND controller, but some are
not using it. There are some device with only serial flash, but I do not
have one with only serial flash. If it is save we can activate it on all
SoCs and boards.

> 
>> +
>>  	leds {
>>  		compatible = "gpio-leds";
>>  
> [...]
>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
>> index 78aec62..cb86748 100644
>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> ...
>> @@ -143,4 +133,30 @@
>>  			#gpio-cells = <2>;
>>  		};
>>  	};
>> +
>> +	nand: nand@18028000 {
>> +		compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
>> +		reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
>> +		reg-names = "nand", "iproc-idm", "iproc-ext";
>> +		interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +		status = "disabled";
>> +
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		brcm,nand-has-wp;
>> +
>> +		nandcs@0 {
>> +			compatible = "brcm,nandcs";
>> +			reg = <0>;
> 
> So every board that uses a BCM5301x chips must use chip-select 0? What
> if the board has NAND wired up on CS1? (I see this all the time on
> BCM7xxx boards.)
> 
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			nand-ecc-strength = <8>;
>> +			nand-ecc-step-size = <512>;
> 
> So, every board using the BCM5301x family must use BCH-8? What happens
> if there are boards that use a less reliable flash that needs, e.g.,
> BCH-16?
> 
> IOW, none of the nandcs@0 node belongs in the top-level chip
> description. This all belongs in the board description. If it really is
> repeated on tons of boards, then maybe you just need a separate
> 'nand-cs0-bch8.dtsi' or something like that.

Ok, I can change it that way. The nand-cs0-bch8.dtsi sonds good.
Currently I do not have a complete overview, but I think at least the
vendor driver only supports Chip select 0 as far as I think and the ecc
values are auto detected and calculated by some strange algorithm I
haven't fully understood.

> 
>> +
>> +			linux,part-probe = "ofpart", "bcm47xxpart";
> 
> ^ NAK to this line. You still haven't documented any semantics for this
> property. And I gave you several comments on your previous patch about
> what would need to change before I'd accept this property.

We should talk about this in the patch adding the binding, I will adapt
this depending on the result of the discussion.

> 
>> +		};
>> +	};
>>  };
> 
> Brian
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2015-May/059419.html
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-28  8:21       ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2015-05-28  8:21 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: devicetree, f.fainelli, rjui, zajec5, jogo,
	bcm-kernel-feedback-list, linux-mtd, computersforpeace

On Wednesday 27 May 2015 23:46:48 Hauke Mehrtens wrote:
> On 05/27/2015 09:37 AM, Arnd Bergmann wrote:
> > On Sunday 24 May 2015 20:32:29 Hauke Mehrtens wrote:
> >> @@ -124,17 +124,7 @@
> >>                         <0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
> >>  
> >>                         /* Ethernet Controller 3 */
> >> -                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
> >> -
> >> -                       /* NAND Controller */
> >> -                       <0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
> >> +                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
> >>  
> >>                 chipcommon: chipcommon@0 {
> >>                         reg = <0x00000000 0x1000>;
> >> @@ -143,4 +133,30 @@
> >>                         #gpio-cells = <2>;
> >>                 };
> >>         };
> >> +
> >> +       nand: nand@18028000 {
> >> +               compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
> >> +               reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
> >> +               reg-names = "nand", "iproc-idm", "iproc-ext";
> >> +               interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> > 
> > I think I'd rather leave the interrupt-map in the brcm node, and use
> > 
> > 	interrupts = <0>;
> > 
> > here.
> 
> There are two conflicting interests ;-)
> 
> In the first approach we probed the NAND controller through bcma and
> just added the additional information we can not probe through device
> tree. That was this version:
> https://patchwork.ozlabs.org/patch/473181/
> 
> This version needs some changes to the brcmnand driver and this
> additional part:
> https://patchwork.ozlabs.org/patch/473182/
> 
> This code is simular to the iproc_nand.c
> 
> Brian wanted us to use the iproc_nand.c which is possible, but then we
> completely ignore what we have probed in bcma and only use device tree.
> And this nand driver is not a subnode of the bcma bus any more so we can
> not use the irqs defined there.

Ok, I see. Maybe we should however leave the interrupt map entries
in the bcma node, to leave the option of doing it either way?

> > 
> >> +               status = "disabled";
> >> +
> >> +               #address-cells = <1>;
> >> +               #size-cells = <0>;
> >> +
> >> +               brcm,nand-has-wp;
> >> +
> >> +               nandcs@0 {
> >> +                       compatible = "brcm,nandcs";
> >> +                       reg = <0>;
> >> +                       #address-cells = <1>;
> >> +                       #size-cells = <1>;
> >> +
> >> +                       nand-ecc-strength = <8>;
> >> +                       nand-ecc-step-size = <512>;
> >> +
> >> +                       linux,part-probe = "ofpart", "bcm47xxpart";
> >> +               };
> >> +       };
> > 
> > This seems fine in principle, once the exact binding has been nailed down.
> > 
> > "bcm47xxpart" does not seem like an appropriate string here.
> 
> We are already thinking and talking about how to solve this problem.
> There are some parsers that are able to parse some partition layout
> based on information various sources or some vendor headers and some are
> also guessing in addition, like they are assuming that the bootloader is
> in the first block and n bytes long. The current module is that the
> flash driver defines which partition parsers to take, it is hard coded
> into the code. I do not like that. ;-)
> 
> I am trying to get a simple patch in which makes it possible to
> overwrite the defaults from the flash driver with device tree.
> As it uses the same format it should be easy to convert devices from the
> old style to the new one. This deice tree attribute is already supported
> by one driver I moved the code to make it available for all drivers.
> 
> The patch I am talking about: https://patchwork.ozlabs.org/patch/475988/

I was mainly referring to the use of 'xx' wildcards in the bcm47xxpart
string. We try to avoid wildcards in compatible strings, and I think it
makes sense to follow the same model here.

	Arnd

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-28  8:21       ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2015-05-28  8:21 UTC (permalink / raw
  To: Hauke Mehrtens
  Cc: computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w

On Wednesday 27 May 2015 23:46:48 Hauke Mehrtens wrote:
> On 05/27/2015 09:37 AM, Arnd Bergmann wrote:
> > On Sunday 24 May 2015 20:32:29 Hauke Mehrtens wrote:
> >> @@ -124,17 +124,7 @@
> >>                         <0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
> >>  
> >>                         /* Ethernet Controller 3 */
> >> -                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
> >> -
> >> -                       /* NAND Controller */
> >> -                       <0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
> >> -                       <0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
> >> +                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
> >>  
> >>                 chipcommon: chipcommon@0 {
> >>                         reg = <0x00000000 0x1000>;
> >> @@ -143,4 +133,30 @@
> >>                         #gpio-cells = <2>;
> >>                 };
> >>         };
> >> +
> >> +       nand: nand@18028000 {
> >> +               compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
> >> +               reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
> >> +               reg-names = "nand", "iproc-idm", "iproc-ext";
> >> +               interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> > 
> > I think I'd rather leave the interrupt-map in the brcm node, and use
> > 
> > 	interrupts = <0>;
> > 
> > here.
> 
> There are two conflicting interests ;-)
> 
> In the first approach we probed the NAND controller through bcma and
> just added the additional information we can not probe through device
> tree. That was this version:
> https://patchwork.ozlabs.org/patch/473181/
> 
> This version needs some changes to the brcmnand driver and this
> additional part:
> https://patchwork.ozlabs.org/patch/473182/
> 
> This code is simular to the iproc_nand.c
> 
> Brian wanted us to use the iproc_nand.c which is possible, but then we
> completely ignore what we have probed in bcma and only use device tree.
> And this nand driver is not a subnode of the bcma bus any more so we can
> not use the irqs defined there.

Ok, I see. Maybe we should however leave the interrupt map entries
in the bcma node, to leave the option of doing it either way?

> > 
> >> +               status = "disabled";
> >> +
> >> +               #address-cells = <1>;
> >> +               #size-cells = <0>;
> >> +
> >> +               brcm,nand-has-wp;
> >> +
> >> +               nandcs@0 {
> >> +                       compatible = "brcm,nandcs";
> >> +                       reg = <0>;
> >> +                       #address-cells = <1>;
> >> +                       #size-cells = <1>;
> >> +
> >> +                       nand-ecc-strength = <8>;
> >> +                       nand-ecc-step-size = <512>;
> >> +
> >> +                       linux,part-probe = "ofpart", "bcm47xxpart";
> >> +               };
> >> +       };
> > 
> > This seems fine in principle, once the exact binding has been nailed down.
> > 
> > "bcm47xxpart" does not seem like an appropriate string here.
> 
> We are already thinking and talking about how to solve this problem.
> There are some parsers that are able to parse some partition layout
> based on information various sources or some vendor headers and some are
> also guessing in addition, like they are assuming that the bootloader is
> in the first block and n bytes long. The current module is that the
> flash driver defines which partition parsers to take, it is hard coded
> into the code. I do not like that. ;-)
> 
> I am trying to get a simple patch in which makes it possible to
> overwrite the defaults from the flash driver with device tree.
> As it uses the same format it should be easy to convert devices from the
> old style to the new one. This deice tree attribute is already supported
> by one driver I moved the code to make it available for all drivers.
> 
> The patch I am talking about: https://patchwork.ozlabs.org/patch/475988/

I was mainly referring to the use of 'xx' wildcards in the bcm47xxpart
string. We try to avoid wildcards in compatible strings, and I think it
makes sense to follow the same model here.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
  2015-05-28  8:21       ` Arnd Bergmann
@ 2015-05-29 15:29         ` Hauke Mehrtens
  -1 siblings, 0 replies; 22+ messages in thread
From: Hauke Mehrtens @ 2015-05-29 15:29 UTC (permalink / raw
  To: Arnd Bergmann
  Cc: devicetree, f.fainelli, rjui, zajec5, jogo,
	bcm-kernel-feedback-list, linux-mtd, computersforpeace



On 05/28/2015 10:21 AM, Arnd Bergmann wrote:
> On Wednesday 27 May 2015 23:46:48 Hauke Mehrtens wrote:
>> On 05/27/2015 09:37 AM, Arnd Bergmann wrote:
>>> On Sunday 24 May 2015 20:32:29 Hauke Mehrtens wrote:
>>>> @@ -124,17 +124,7 @@
>>>>                         <0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
>>>>  
>>>>                         /* Ethernet Controller 3 */
>>>> -                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
>>>> -
>>>> -                       /* NAND Controller */
>>>> -                       <0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> +                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
>>>>  
>>>>                 chipcommon: chipcommon@0 {
>>>>                         reg = <0x00000000 0x1000>;
>>>> @@ -143,4 +133,30 @@
>>>>                         #gpio-cells = <2>;
>>>>                 };
>>>>         };
>>>> +
>>>> +       nand: nand@18028000 {
>>>> +               compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
>>>> +               reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
>>>> +               reg-names = "nand", "iproc-idm", "iproc-ext";
>>>> +               interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
>>>
>>> I think I'd rather leave the interrupt-map in the brcm node, and use
>>>
>>> 	interrupts = <0>;
>>>
>>> here.
>>
>> There are two conflicting interests ;-)
>>
>> In the first approach we probed the NAND controller through bcma and
>> just added the additional information we can not probe through device
>> tree. That was this version:
>> https://patchwork.ozlabs.org/patch/473181/
>>
>> This version needs some changes to the brcmnand driver and this
>> additional part:
>> https://patchwork.ozlabs.org/patch/473182/
>>
>> This code is simular to the iproc_nand.c
>>
>> Brian wanted us to use the iproc_nand.c which is possible, but then we
>> completely ignore what we have probed in bcma and only use device tree.
>> And this nand driver is not a subnode of the bcma bus any more so we can
>> not use the irqs defined there.
> 
> Ok, I see. Maybe we should however leave the interrupt map entries
> in the bcma node, to leave the option of doing it either way?

Ok, I will do that as there are already there.

> 
>>>
>>>> +               status = "disabled";
>>>> +
>>>> +               #address-cells = <1>;
>>>> +               #size-cells = <0>;
>>>> +
>>>> +               brcm,nand-has-wp;
>>>> +
>>>> +               nandcs@0 {
>>>> +                       compatible = "brcm,nandcs";
>>>> +                       reg = <0>;
>>>> +                       #address-cells = <1>;
>>>> +                       #size-cells = <1>;
>>>> +
>>>> +                       nand-ecc-strength = <8>;
>>>> +                       nand-ecc-step-size = <512>;
>>>> +
>>>> +                       linux,part-probe = "ofpart", "bcm47xxpart";
>>>> +               };
>>>> +       };
>>>
>>> This seems fine in principle, once the exact binding has been nailed down.
>>>
>>> "bcm47xxpart" does not seem like an appropriate string here.
>>
>> We are already thinking and talking about how to solve this problem.
>> There are some parsers that are able to parse some partition layout
>> based on information various sources or some vendor headers and some are
>> also guessing in addition, like they are assuming that the bootloader is
>> in the first block and n bytes long. The current module is that the
>> flash driver defines which partition parsers to take, it is hard coded
>> into the code. I do not like that. ;-)
>>
>> I am trying to get a simple patch in which makes it possible to
>> overwrite the defaults from the flash driver with device tree.
>> As it uses the same format it should be easy to convert devices from the
>> old style to the new one. This deice tree attribute is already supported
>> by one driver I moved the code to make it available for all drivers.
>>
>> The patch I am talking about: https://patchwork.ozlabs.org/patch/475988/
> 
> I was mainly referring to the use of 'xx' wildcards in the bcm47xxpart
> string. We try to avoid wildcards in compatible strings, and I think it
> makes sense to follow the same model here.

Ah, ok currently this is referring to the old names of the partition
parsers and this code was added before we know that there will be arm
SoCs using this. As it is not clear how this will be represented in
device tree I would remove it for now form our device tree.

Hauke

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

* Re: [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-29 15:29         ` Hauke Mehrtens
  0 siblings, 0 replies; 22+ messages in thread
From: Hauke Mehrtens @ 2015-05-29 15:29 UTC (permalink / raw
  To: Arnd Bergmann
  Cc: computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rjui-dY08KVG/lbpWk0Htik3J/w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w



On 05/28/2015 10:21 AM, Arnd Bergmann wrote:
> On Wednesday 27 May 2015 23:46:48 Hauke Mehrtens wrote:
>> On 05/27/2015 09:37 AM, Arnd Bergmann wrote:
>>> On Sunday 24 May 2015 20:32:29 Hauke Mehrtens wrote:
>>>> @@ -124,17 +124,7 @@
>>>>                         <0x00026000 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
>>>>  
>>>>                         /* Ethernet Controller 3 */
>>>> -                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
>>>> -
>>>> -                       /* NAND Controller */
>>>> -                       <0x00028000 0 &gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 1 &gic GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 2 &gic GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 3 &gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 4 &gic GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 5 &gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 6 &gic GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
>>>> -                       <0x00028000 7 &gic GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> +                       <0x00027000 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
>>>>  
>>>>                 chipcommon: chipcommon@0 {
>>>>                         reg = <0x00000000 0x1000>;
>>>> @@ -143,4 +133,30 @@
>>>>                         #gpio-cells = <2>;
>>>>                 };
>>>>         };
>>>> +
>>>> +       nand: nand@18028000 {
>>>> +               compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
>>>> +               reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
>>>> +               reg-names = "nand", "iproc-idm", "iproc-ext";
>>>> +               interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
>>>
>>> I think I'd rather leave the interrupt-map in the brcm node, and use
>>>
>>> 	interrupts = <0>;
>>>
>>> here.
>>
>> There are two conflicting interests ;-)
>>
>> In the first approach we probed the NAND controller through bcma and
>> just added the additional information we can not probe through device
>> tree. That was this version:
>> https://patchwork.ozlabs.org/patch/473181/
>>
>> This version needs some changes to the brcmnand driver and this
>> additional part:
>> https://patchwork.ozlabs.org/patch/473182/
>>
>> This code is simular to the iproc_nand.c
>>
>> Brian wanted us to use the iproc_nand.c which is possible, but then we
>> completely ignore what we have probed in bcma and only use device tree.
>> And this nand driver is not a subnode of the bcma bus any more so we can
>> not use the irqs defined there.
> 
> Ok, I see. Maybe we should however leave the interrupt map entries
> in the bcma node, to leave the option of doing it either way?

Ok, I will do that as there are already there.

> 
>>>
>>>> +               status = "disabled";
>>>> +
>>>> +               #address-cells = <1>;
>>>> +               #size-cells = <0>;
>>>> +
>>>> +               brcm,nand-has-wp;
>>>> +
>>>> +               nandcs@0 {
>>>> +                       compatible = "brcm,nandcs";
>>>> +                       reg = <0>;
>>>> +                       #address-cells = <1>;
>>>> +                       #size-cells = <1>;
>>>> +
>>>> +                       nand-ecc-strength = <8>;
>>>> +                       nand-ecc-step-size = <512>;
>>>> +
>>>> +                       linux,part-probe = "ofpart", "bcm47xxpart";
>>>> +               };
>>>> +       };
>>>
>>> This seems fine in principle, once the exact binding has been nailed down.
>>>
>>> "bcm47xxpart" does not seem like an appropriate string here.
>>
>> We are already thinking and talking about how to solve this problem.
>> There are some parsers that are able to parse some partition layout
>> based on information various sources or some vendor headers and some are
>> also guessing in addition, like they are assuming that the bootloader is
>> in the first block and n bytes long. The current module is that the
>> flash driver defines which partition parsers to take, it is hard coded
>> into the code. I do not like that. ;-)
>>
>> I am trying to get a simple patch in which makes it possible to
>> overwrite the defaults from the flash driver with device tree.
>> As it uses the same format it should be easy to convert devices from the
>> old style to the new one. This deice tree attribute is already supported
>> by one driver I moved the code to make it available for all drivers.
>>
>> The patch I am talking about: https://patchwork.ozlabs.org/patch/475988/
> 
> I was mainly referring to the use of 'xx' wildcards in the bcm47xxpart
> string. We try to avoid wildcards in compatible strings, and I think it
> makes sense to follow the same model here.

Ah, ok currently this is referring to the old names of the partition
parsers and this code was added before we know that there will be arm
SoCs using this. As it is not clear how this will be represented in
device tree I would remove it for now form our device tree.

Hauke
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2015-05-29 15:29 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-24 18:32 [RFC] ARM: BCM5301X: add NAND flash chip description Hauke Mehrtens
2015-05-24 18:32 ` Hauke Mehrtens
2015-05-26 20:20 ` Rafał Miłecki
2015-05-26 20:20   ` Rafał Miłecki
2015-05-26 20:53   ` nick
2015-05-26 20:53     ` nick
2015-05-27  0:41 ` Brian Norris
2015-05-27  0:41   ` Brian Norris
2015-05-27  1:15   ` Brian Norris
2015-05-27  1:15     ` Brian Norris
2015-05-27 22:04   ` Hauke Mehrtens
2015-05-27 22:04     ` Hauke Mehrtens
2015-05-27  7:37 ` Arnd Bergmann
2015-05-27  7:37   ` Arnd Bergmann
2015-05-27 21:46   ` Hauke Mehrtens
2015-05-27 21:46     ` Hauke Mehrtens
2015-05-27 21:57     ` Brian Norris
2015-05-27 21:57       ` Brian Norris
2015-05-28  8:21     ` Arnd Bergmann
2015-05-28  8:21       ` Arnd Bergmann
2015-05-29 15:29       ` Hauke Mehrtens
2015-05-29 15:29         ` Hauke Mehrtens

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.