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

William Zhang
2024-02-22T16:21:36Z

[-- Attachment #1.1: Type: text/plain, Size: 2194 bytes --]

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 #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]