Hi,
> On 10 Nov 2020, at 07:34, Kever Yang <kever.y...@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 <pbrobin...@gmail.com>
>> Fixes: c4cea2bbf995 ("rockchip: Enable building a SPI ROM image on bob")
>> Cc: Simon Glass <s...@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-sigma...@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.
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.
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@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;
>> };
