| Message ID | 20240207202257.271784-6-william.zhang@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: rawnand: brcmnand: driver and doc updates | expand |
On 2024-02-07 21:22, William Zhang wrote: > diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > index 2f124b027bbf..336016e334d9 100644 > --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > @@ -589,7 +589,7 @@ hsspi: spi@1000 { > status = "disabled"; > }; > > - nand-controller@1800 { > + nand_controller: nand-controller@1800 { > #address-cells = <1>; > #size-cells = <0>; > compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", > "brcm,brcmnand"; > @@ -597,7 +597,7 @@ nand-controller@1800 { > reg-names = "nand", "nand-int-base"; > interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; > interrupt-names = "nand_ctlrdy"; > - status = "okay"; > + status = "disabled"; > > nandcs: nand@0 { > compatible = "brcm,nandcs"; You disable NAND controller here breaking support for all dependant devices. I see you fix it afterwards in PATCH 06/12 but it's still a poor way of handling such change and this issue can be hit e.g. during git bisect. Also: given that 99% of BCM4908 devices use NAND controller can we just save ourselves efforts and keep it enabled in .dtsi?
Hi Rafal, On 2/22/24 00:27, Rafał Miłecki wrote: > On 2024-02-07 21:22, William Zhang wrote: >> diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi >> b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi >> index 2f124b027bbf..336016e334d9 100644 >> --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi >> +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi >> @@ -589,7 +589,7 @@ hsspi: spi@1000 { >> status = "disabled"; >> }; >> >> - nand-controller@1800 { >> + nand_controller: nand-controller@1800 { >> #address-cells = <1>; >> #size-cells = <0>; >> compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", >> "brcm,brcmnand"; >> @@ -597,7 +597,7 @@ nand-controller@1800 { >> reg-names = "nand", "nand-int-base"; >> interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; >> interrupt-names = "nand_ctlrdy"; >> - status = "okay"; >> + status = "disabled"; >> >> nandcs: nand@0 { >> compatible = "brcm,nandcs"; > > You disable NAND controller here breaking support for all dependant > devices. > > I see you fix it afterwards in PATCH 06/12 but it's still a poor way of > handling such change and this issue can be hit e.g. during git bisect. > > Also: given that 99% of BCM4908 devices use NAND controller can we just > save ourselves efforts and keep it enabled in .dtsi? > I was trying to fix this according to the dts convention per maintainers' requirement(only enable the node in the board dts, not soc include). We have 100% of BCMBCA devices use strap for ecc but we still have to put the property in board dts. I understand if someone try to bisect the commits, it may hit the problem but this will be very obvious that controller is not even shown in the boot up so hopefully it is easy to figure out. Anyway if maintainer(Minque and/or anyone from device tree side) is okay with your request, I can change it back to enable in the dtsi.
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi index 2f124b027bbf..336016e334d9 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi @@ -589,7 +589,7 @@ hsspi: spi@1000 { status = "disabled"; }; - nand-controller@1800 { + nand_controller: nand-controller@1800 { #address-cells = <1>; #size-cells = <0>; compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand"; @@ -597,7 +597,7 @@ nand-controller@1800 { reg-names = "nand", "nand-int-base"; interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "nand_ctlrdy"; - status = "okay"; + status = "disabled"; nandcs: nand@0 { compatible = "brcm,nandcs"; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi index d658c81f7285..14b2adfb817c 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi @@ -138,6 +138,20 @@ hsspi: spi@1000 { status = "disabled"; }; + nand_controller: nand-controller@1800 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand"; + reg = <0x1800 0x600>, <0x2000 0x10>; + reg-names = "nand", "nand-int-base"; + status = "disabled"; + + nandcs: nand@0 { + compatible = "brcm,nandcs"; + reg = <0>; + }; + }; + uart0: serial@12000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x12000 0x1000>; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi index 4f474d47022e..589b8a1efc72 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi @@ -119,6 +119,20 @@ hsspi: spi@1000 { status = "disabled"; }; + nand_controller: nand-controller@1800 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand"; + reg = <0x1800 0x600>, <0x2000 0x10>; + reg-names = "nand", "nand-int-base"; + status = "disabled"; + + nandcs: nand@0 { + compatible = "brcm,nandcs"; + reg = <0>; + }; + }; + uart0: serial@12000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x12000 0x1000>; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi index 909f254dc47d..48d618e75866 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi @@ -137,6 +137,20 @@ hsspi: spi@1000 { status = "disabled"; }; + nand_controller: nand-controller@1800 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand"; + reg = <0x1800 0x600>, <0x2000 0x10>; + reg-names = "nand", "nand-int-base"; + status = "disabled"; + + nandcs: nand@0 { + compatible = "brcm,nandcs"; + reg = <0>; + }; + }; + uart0: serial@12000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x12000 0x1000>; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi index 685ae32951c9..1d1303cf90f3 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi @@ -138,6 +138,20 @@ hsspi: spi@1000 { status = "disabled"; }; + nand_controller: nand-controller@1800 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand"; + reg = <0x1800 0x600>, <0x2000 0x10>; + reg-names = "nand", "nand-int-base"; + status = "disabled"; + + nandcs: nand@0 { + compatible = "brcm,nandcs"; + reg = <0>; + }; + }; + uart0: serial@12000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x12000 0x1000>; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi index 820553ce541b..00c62c1e5df0 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi @@ -119,5 +119,19 @@ hsspi: spi@1000 { num-cs = <8>; status = "disabled"; }; + + nand_controller: nand-controller@1800 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand"; + reg = <0x1800 0x600>, <0x2000 0x10>; + reg-names = "nand", "nand-int-base"; + status = "disabled"; + + nandcs: nand@0 { + compatible = "brcm,nandcs"; + reg = <0>; + }; + }; }; }; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi index 0eb93c298297..caeaf428dc15 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi @@ -156,5 +156,19 @@ hsspi: spi@1000 { num-cs = <8>; status = "disabled"; }; + + nand_controller: nand-controller@1800 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand"; + reg = <0x1800 0x600>, <0x2000 0x10>; + reg-names = "nand", "nand-int-base"; + status = "disabled"; + + nandcs: nand@0 { + compatible = "brcm,nandcs"; + reg = <0>; + }; + }; }; }; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts index c4e6e71f6310..030ffa5364fb 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts @@ -32,3 +32,13 @@ &uart0 { &hsspi { status = "okay"; }; + +&nand_controller { + brcm,wp-not-connected; + status = "okay"; +}; + +&nandcs { + nand-on-flash-bbt; + brcm,nand-ecc-use-strap; +}; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts index e69cd683211a..4b779e6c22e1 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts @@ -32,3 +32,13 @@ &uart0 { &hsspi { status = "okay"; }; + +&nand_controller { + brcm,wp-not-connected; + status = "okay"; +}; + +&nandcs { + nand-on-flash-bbt; + brcm,nand-ecc-use-strap; +}; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts index db2c82d6dfd8..2851e8e41bf4 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts @@ -32,3 +32,13 @@ &uart0 { &hsspi { status = "okay"; }; + +&nand_controller { + brcm,wp-not-connected; + status = "okay"; +}; + +&nandcs { + nand-on-flash-bbt; + brcm,nand-ecc-use-strap; +}; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts index 25c12bc63545..17dc594fe83f 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts @@ -32,3 +32,13 @@ &uart0 { &hsspi { status = "okay"; }; + +&nand_controller { + brcm,wp-not-connected; + status = "okay"; +}; + +&nandcs { + nand-on-flash-bbt; + brcm,nand-ecc-use-strap; +}; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts index faba21f03120..34832a734734 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts @@ -32,3 +32,13 @@ &uart0 { &hsspi { status = "okay"; }; + +&nand_controller { + brcm,wp-not-connected; + status = "okay"; +}; + +&nandcs { + nand-on-flash-bbt; + brcm,nand-ecc-use-strap; +}; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts index 9808331eede2..e1396b5544b7 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts @@ -32,3 +32,13 @@ &uart0 { &hsspi { status = "okay"; }; + +&nand_controller { + brcm,wp-not-connected; + status = "okay"; +}; + +&nandcs { + nand-on-flash-bbt; + brcm,nand-ecc-use-strap; +}; diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts index 1f561c8e13b0..30bbf6f2917e 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts @@ -32,3 +32,13 @@ &uart0 { &hsspi { status = "okay"; }; + +&nand_controller { + brcm,wp-not-connected; + status = "okay"; +}; + +&nandcs { + nand-on-flash-bbt; + brcm,nand-ecc-use-strap; +};