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@xxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
