[PATCH] rockchip: Move Bob specific bits to it's specific u-boot.dtsi

[Alper Nebi Yasak]

On 10/11/2020 20:18, Hugh Cole-Baker wrote:
> Hi,
> 
>> On 10 Nov 2020, at 07:34, Kever Yang <kever.yang at rock-chips.com> wrote:
>>
>> Hi Peter,
>>
>> On 2020/11/9 上午7:02, Peter Robinson wrote:
>>> Move the bits that are device specific to the -u-boot.dtsi as the
>>> bits may be different on other devices and hence breaks SPI on
>>> those devices such as the Pinebook Pro.
>>>
>>> Signed-off-by: Peter Robinson <pbrobinson at gmail.com>
>>> Fixes: c4cea2bbf995 ("rockchip: Enable building a SPI ROM image on bob")
>>> Cc: Simon Glass <sjg at chromium.org>
>>> ---
>>>  arch/arm/dts/rk3399-gru-u-boot.dtsi | 30 +++++++++++++++++++++++++++++
>>>  arch/arm/dts/rk3399-u-boot.dtsi     | 25 ------------------------
>>>  2 files changed, 30 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/rk3399-gru-u-boot.dtsi b/arch/arm/dts/rk3399-gru-u-boot.dtsi
>>> index 390ac2bb5a..5e95cacfea 100644
>>> --- a/arch/arm/dts/rk3399-gru-u-boot.dtsi
>>> +++ b/arch/arm/dts/rk3399-gru-u-boot.dtsi
>>> @@ -5,6 +5,36 @@
>>>    #include "rk3399-u-boot.dtsi"
>>>  +/ {
>>> +	aliases {
>>> +		spi1 = &spi1;
>>> +	};
>>> +};
>>
>> Does this still need to remove from common code after your another patch applied? It look reasonable and
>>
>> not likely to break others.
>>
>> https://patchwork.ozlabs.org/project/uboot/patch/20201108140023.32501-1-sigmaris@gmail.com/
> 
> My patch linked above was aiming to fix SPI boot on the rockpro64 by
> adapting to the spi1 alias that's now in the rk3399-u-boot.dtsi rather
> than removing it (in fact, my patch wouldn't work correctly if the spi1
> alias was removed). This seemed like one good solution as the RK3399 does
> physically have SPI buses 0 to 5, and out of those the SPI flash is on
> bus 1, so I thought it'd be better to refer to it as bus 1 instead of
> aliasing it to bus 0.

It looks like something similar to your patch works on the Pinebook Pro,
see this other thread about rk3399 SPI breakage [1] and one recent mail
from it [2].

[1] https://lists.denx.de/pipermail/u-boot/2020-November/432046.html
[2] https://lists.denx.de/pipermail/u-boot/2020-November/432505.html

> I see now, though, that it's not just the rockpro64 that's affected, but
> also Pinebook Pro and it seems rk3399-roc-pc. If the consensus is that
> my patch is the right approach, I can send another series with the same
> type of fix for the PBP and rk3399-roc-pc. Otherwise this patch should
> be used and my patch shouldn't be applied as it relies on the spi1 alias
> that this patch removes.

Looks to me like the full list is (though I don't have those boards):

    pinebook-pro-rk3399         rk3399-pinebook-pro-u-boot.dtsi
    puma-rk3399                 rk3399-puma-haikou-u-boot.dtsi
    roc-pc-rk3399               rk3399-roc-pc-u-boot.dtsi
    roc-pc-mezzanine-rk3399             ``
    rockpro64-rk3399            rk3399-rockpro64-u-boot.dtsi

> Regards,
> Hugh
> 
>>
>>> +
>>> +#ifdef CONFIG_ROCKCHIP_SPI_IMAGE
>>> +&binman {
>>> +	rom {
>>> +		filename = "u-boot.rom";
>>> +		size = <0x400000>;
>>> +		pad-byte = <0xff>;
>>> +
>>> +		mkimage {
>>> +			args = "-n rk3399 -T rkspi";
>>> +			u-boot-spl {
>>> +			};
>>> +		};
>>> +		u-boot-img {
>>> +			offset = <0x40000>;
>>> +		};
>>> +		u-boot {
>>> +			offset = <0x300000>;
>>> +		};
>>> +		fdtmap {
>>> +		};
>>> +	};
>>> +};
>>> +#endif
>>
>>
>> What's the image space mapping for Pinebook Pro do you using?
>>
>> I think there should be another binman config if this is not common .
>>
>>
>> Thanks,
>>
>> - Kever
>>
>>> +
>>>  &spi_flash {
>>>  	u-boot,dm-pre-reloc;
>>>  };
>>> diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
>>> index ecd230c720..26b0a34e64 100644
>>> --- a/arch/arm/dts/rk3399-u-boot.dtsi
>>> +++ b/arch/arm/dts/rk3399-u-boot.dtsi
>>> @@ -11,7 +11,6 @@
>>>  		mmc0 = &sdhci;
>>>  		mmc1 = &sdmmc;
>>>  		pci0 = &pcie0;
>>> -		spi1 = &spi1;
>>>  	};
>>>    	cic: syscon at ff620000 {
>>> @@ -60,30 +59,6 @@
>>>    };
>>>  -#ifdef CONFIG_ROCKCHIP_SPI_IMAGE
>>> -&binman {
>>> -	rom {
>>> -		filename = "u-boot.rom";
>>> -		size = <0x400000>;
>>> -		pad-byte = <0xff>;
>>> -
>>> -		mkimage {
>>> -			args = "-n rk3399 -T rkspi";
>>> -			u-boot-spl {
>>> -			};
>>> -		};
>>> -		u-boot-img {
>>> -			offset = <0x40000>;
>>> -		};
>>> -		u-boot {
>>> -			offset = <0x300000>;
>>> -		};
>>> -		fdtmap {
>>> -		};
>>> -	};
>>> -};
>>> -#endif
>>> -
>>>  &cru {
>>>  	u-boot,dm-pre-reloc;
>>>  };
>