Re: [PATCH v5 05/12] arm64: dts: broadcom: bcmbca: Add NAND controller node

From: William Zhang
Date: Thu Feb 22 2024 - 11:21:32 EST


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.

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature