Re: [PATCH 12/19] rockchip: rk3588: Update bootph props
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 @@
};
{
- bootph-pre-ram;
+ bootph-all;
};
_bus8 {
- bootph-all;
+ bootph-pre-ram;
};
_clk {
- bootph-all;
+ bootph-pre-ram;
};
_cmd {
- bootph-all;
+ bootph-pre-ram;
};
_data_strobe {
- bootph-all;
+ bootph-pre-ram;
};
_rstnout {
- bootph-all;
+ bootph-pre-ram;
};
{
- bootph-pre-ram;
+ bootph-all;
+};
+
+_pull_down {
+ bootph-all;
};
_pull_none {
@@ -157,6 +161,10 @@
};
_pull_up_drv_level_2 {
+ bootph-pre-ram;
+};
+
+_grf {
bootph-all;
};
@@ -189,19 +197,23 @@
};
_bus4 {
- bootph-all;
+ bootph-pre-ram;
};
_clk {
- bootph-all;
+ bootph-pre-ram;
};
_cmd {
- bootph-all;
+ bootph-pre-ram;
};
_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.
Mm... 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 -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
Ooh 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
Re: [PATCH 12/19] rockchip: rk3588: Update bootph props
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 @@
>> };
>>
>>{
>> -bootph-pre-ram;
>> +bootph-all;
>> };
>>
>> _bus8 {
>> -bootph-all;
>> +bootph-pre-ram;
>> };
>>
>> _clk {
>> -bootph-all;
>> +bootph-pre-ram;
>> };
>>
>> _cmd {
>> -bootph-all;
>> +bootph-pre-ram;
>> };
>>
>> _data_strobe {
>> -bootph-all;
>> +bootph-pre-ram;
>> };
>>
>> _rstnout {
>> -bootph-all;
>> +bootph-pre-ram;
>> };
>>
>>{
>> -bootph-pre-ram;
>> +bootph-all;
>> +};
>> +
>> +_pull_down {
>> +bootph-all;
>> };
>>
>> _pull_none {
>> @@ -157,6 +161,10 @@
>> };
>>
>> _pull_up_drv_level_2 {
>> +bootph-pre-ram;
>> +};
>> +
>> +_grf {
>> bootph-all;
>> };
>>
>> @@ -189,19 +197,23 @@
>> };
>>
>> _bus4 {
>> -bootph-all;
>> +bootph-pre-ram;
>> };
>>
>> _clk {
>> -bootph-all;
>> +bootph-pre-ram;
>> };
>>
>> _cmd {
>> -bootph-all;
>> +bootph-pre-ram;
>> };
>>
>> _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.
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 ;-)
>
> 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
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
Regards,
Jonas
>
> Cheers,
> Quentin
Re: [PATCH 12/19] rockchip: rk3588: Update bootph props
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 @@
};
{
- bootph-pre-ram;
+ bootph-all;
};
_bus8 {
- bootph-all;
+ bootph-pre-ram;
};
_clk {
- bootph-all;
+ bootph-pre-ram;
};
_cmd {
- bootph-all;
+ bootph-pre-ram;
};
_data_strobe {
- bootph-all;
+ bootph-pre-ram;
};
_rstnout {
- bootph-all;
+ bootph-pre-ram;
};
{
- bootph-pre-ram;
+ bootph-all;
+};
+
+_pull_down {
+ bootph-all;
};
_pull_none {
@@ -157,6 +161,10 @@
};
_pull_up_drv_level_2 {
+ bootph-pre-ram;
+};
+
+_grf {
bootph-all;
};
@@ -189,19 +197,23 @@
};
_bus4 {
- bootph-all;
+ bootph-pre-ram;
};
_clk {
- bootph-all;
+ bootph-pre-ram;
};
_cmd {
- bootph-all;
+ bootph-pre-ram;
};
_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)).
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.
Cheers,
Quentin
