* [RFC] ARM: BCM5301X: add NAND flash chip description
@ 2015-05-24 18:32 Hauke Mehrtens
[not found] ` <1432492349-18517-1-git-send-email-hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
0 siblings, 1 reply; 11+ 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] 11+ messages in thread[parent not found: <1432492349-18517-1-git-send-email-hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>]
* Re: [RFC] ARM: BCM5301X: add NAND flash chip description [not found] ` <1432492349-18517-1-git-send-email-hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org> @ 2015-05-26 20:20 ` Rafał Miłecki 2015-05-26 20:53 ` nick 2015-05-27 0:41 ` Brian Norris 2015-05-27 7:37 ` Arnd Bergmann 2 siblings, 1 reply; 11+ 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] 11+ 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 0 siblings, 0 replies; 11+ 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] 11+ messages in thread
* Re: [RFC] ARM: BCM5301X: add NAND flash chip description [not found] ` <1432492349-18517-1-git-send-email-hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org> 2015-05-26 20:20 ` Rafał Miłecki @ 2015-05-27 0:41 ` Brian Norris 2015-05-27 1:15 ` Brian Norris 2015-05-27 22:04 ` Hauke Mehrtens 2015-05-27 7:37 ` Arnd Bergmann 2 siblings, 2 replies; 11+ 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] 11+ 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 2015-05-27 22:04 ` Hauke Mehrtens 1 sibling, 0 replies; 11+ 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] 11+ 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 @ 2015-05-27 22:04 ` Hauke Mehrtens 1 sibling, 0 replies; 11+ 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] 11+ messages in thread
* Re: [RFC] ARM: BCM5301X: add NAND flash chip description [not found] ` <1432492349-18517-1-git-send-email-hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org> 2015-05-26 20:20 ` Rafał Miłecki 2015-05-27 0:41 ` Brian Norris @ 2015-05-27 7:37 ` Arnd Bergmann 2015-05-27 21:46 ` Hauke Mehrtens 2 siblings, 1 reply; 11+ 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] 11+ 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 [not found] ` <55663B48.5060306-5/S+JYg5SzeELgA04lAiVw@public.gmane.org> 0 siblings, 1 reply; 11+ 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] 11+ messages in thread
[parent not found: <55663B48.5060306-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>]
* Re: [RFC] ARM: BCM5301X: add NAND flash chip description [not found] ` <55663B48.5060306-5/S+JYg5SzeELgA04lAiVw@public.gmane.org> @ 2015-05-27 21:57 ` Brian Norris 2015-05-28 8:21 ` Arnd Bergmann 1 sibling, 0 replies; 11+ 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] 11+ messages in thread
* Re: [RFC] ARM: BCM5301X: add NAND flash chip description [not found] ` <55663B48.5060306-5/S+JYg5SzeELgA04lAiVw@public.gmane.org> 2015-05-27 21:57 ` Brian Norris @ 2015-05-28 8:21 ` Arnd Bergmann 2015-05-29 15:29 ` Hauke Mehrtens 1 sibling, 1 reply; 11+ 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] 11+ 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 0 siblings, 0 replies; 11+ 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] 11+ messages in thread
end of thread, other threads:[~2015-05-29 15:29 UTC | newest]
Thread overview: 11+ 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
[not found] ` <1432492349-18517-1-git-send-email-hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
2015-05-26 20:20 ` Rafał Miłecki
2015-05-26 20:53 ` nick
2015-05-27 0:41 ` Brian Norris
2015-05-27 1:15 ` Brian Norris
2015-05-27 22:04 ` Hauke Mehrtens
2015-05-27 7:37 ` Arnd Bergmann
2015-05-27 21:46 ` Hauke Mehrtens
[not found] ` <55663B48.5060306-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
2015-05-27 21:57 ` Brian Norris
2015-05-28 8:21 ` Arnd Bergmann
2015-05-29 15:29 ` Hauke Mehrtens
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).