[PATCH 12/19] rockchip: rk3588: Update bootph props

[Quentin Schulz]

Hi Jonas,

On 4/2/24 15:36, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 2024-04-02 13:20, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 3/29/24 20:01, Jonas Karlman wrote:
>>
>> [...]
>>
>>> diff --git a/arch/arm/dts/rk3588s-u-boot.dtsi b/arch/arm/dts/rk3588s-u-boot.dtsi
>>> index e4171bd24d2a..a502a82fae6a 100644
>>> --- a/arch/arm/dts/rk3588s-u-boot.dtsi
>>> +++ b/arch/arm/dts/rk3588s-u-boot.dtsi
>>> @@ -121,31 +121,35 @@
>>>    };
>>>    
>>>    &cru {
>>> -	bootph-pre-ram;
>>> +	bootph-all;
>>>    };
>>>    
>>>    &emmc_bus8 {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>>    };
>>>    
>>>    &emmc_clk {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>>    };
>>>    
>>>    &emmc_cmd {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>>    };
>>>    
>>>    &emmc_data_strobe {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>>    };
>>>    
>>>    &emmc_rstnout {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>>    };
>>>    
>>>    &ioc {
>>> -	bootph-pre-ram;
>>> +	bootph-all;
>>> +};
>>> +
>>> +&pcfg_pull_down {
>>> +	bootph-all;
>>>    };
>>>    
>>>    &pcfg_pull_none {
>>> @@ -157,6 +161,10 @@
>>>    };
>>>    
>>>    &pcfg_pull_up_drv_level_2 {
>>> +	bootph-pre-ram;
>>> +};
>>> +
>>> +&php_grf {
>>>    	bootph-all;
>>>    };
>>>    
>>> @@ -189,19 +197,23 @@
>>>    };
>>>    
>>>    &sdmmc_bus4 {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>>    };
>>>    
>>>    &sdmmc_clk {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>>    };
>>>    
>>>    &sdmmc_cmd {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>>    };
>>>    
>>>    &sdmmc_det {
>>> -	bootph-all;
>>> +	bootph-pre-ram;
>>> +};
>>> +
>>
>> Please add bootph-some-ram to all nodes related to eMMC/SD card
>> otherwise I assume some boards won't work anymore (e.g. the ones that
>> need to find MMC devices through DT in arch_env_get_location, e.g.
>> Theobroma's Jaguar (and soon Tiger)).
> 
> Sure I will add them in a v2.
> 
> When I tested your arch_env_get_location() it did not seem to require
> any pinctrl nodes at pre-reloc stage.
> 

Mmmmmm... I assume it is because the device checked in 
arch_env_get_location() in U-Boot proper pre-reloc is necessarily the 
same as the one used to load U-Boot proper from SPL, which means the SPL 
will have set the mux correctly (because of bootph-pre-ram). I don't 
think U-Boot proper pre-reloc has a separate DTB from U-Boot proper, so 
I think it checks only for boopth-some-ram property in the MMC 
controller node, and maybe not for pinctrl nodes?

> Ideally we should be able to skip use of DM, env and serial at pre-reloc
> stage for Rockchip. Serial and pinctrl already gets configured in SPL, and
> use of DM slows down boot by 200-700ms, but that is for another series ;-)
> 

Agreed :)

>>
>> c.f.
>> https://source.denx.de/u-boot/u-boot/-/commit/70f9212d61fe79c605b805c6eb0764b29f8ae3b6
>>
>> It was not easy to have this merged, so I'd prefer to avoid having to go
>> through it again to fix my board(s) :)
>>
>> It'd be nice to split this into multiple commits so we can have some
>> individual justification of why such a change is made, so that we know
>> if we need to revert/update it in the future.
> 
> Will try to split this in v2.
> 
> In summary some pinctrl or nodes referenced was not enabled for SPL
> stage, and some was enabled for TPL that should never be needed in TPL,
> if U-Boot TPL will be used in future.
> 
> TPL: uart node + dmc/ram and any node required/referenced
> SPL: TPL + sdmmc/sdhci/spi-flash nodes and related pinctrl

For the <soc>-u-boot.dtsi, I guess this is good enough, we may need more 
though for some boards (e.g. emmc-reset, gpios, etc...).

> pre-reloc: TPL + sdmmc/sdhci/spi-flash nodes
> 
> I created a python script [1] that can check the built tpl/spl/proper
> dtb for missing nodes and some config options for all soc targets.
> 
> [1] https://gist.github.com/Kwiboo/34c099fb42eb6ae5ed515a04275a7ed7
> 

Ooooooh this seems really neat. I'm always very careful when adding 
support for a new board and I'm always afraid to miss a few nodes, I'll 
try to not forget to have a look at this for adding support for Tiger 
RK3588 :)

Do you have any plan of submitting this on the ML? I think this could be 
beneficial to the project!

Cheers,
Quentin