[PATCH] rockchip: rk3399: pass platform parameter to TF-A by default
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Long are gone the times TF-A couldn't handle the FDT passed by U-Boot. Specifically, since commit e7b586987c0a ("rockchip: don't crash if we get an FDT we can't parse") in TF-A, failure to parse the FDT will use the fallback mechanism. This patch was merged in TF-A v2.4-rc0 from two years ago.
Therefore, let's finally pass the FDT to TF-A so that it can get the serial configuration from U-Boot FDT instead of requiring the user to patch TF-A hardcoded fallback values.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- rockchip: rk3399: pass platform parameter to TF-A
Finally pass the FDT address to TF-A since it now gracefully fallbacks to hardcoded defaults if it cannot parse it. This allows us to avoid modifying hardcoded values in TF-A to enable the console.
This was tested with TF-A v2.7.0 on Puma Haikou RK3399.
To: Simon Glass sjg@chromium.org To: Philipp Tomsich philipp.tomsich@vrull.eu To: Kever Yang kever.yang@rock-chips.com Cc: Hugh Cole-Baker sigmaris@gmail.com Cc: Walter Lozano walter.lozano@collabora.com Cc: u-boot@lists.denx.de --- arch/arm/mach-rockchip/Kconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 69d51ff378..2fcc23f9fa 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -249,7 +249,6 @@ config ROCKCHIP_RK3399 imply PRE_CONSOLE_BUFFER imply ROCKCHIP_COMMON_BOARD imply ROCKCHIP_SDRAM_COMMON - imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF imply SPL_ROCKCHIP_COMMON_BOARD imply TPL_SERIAL imply TPL_LIBCOMMON_SUPPORT
--- base-commit: 0cbeed4f6648e0e4966475e3544280a69ecb59d3 change-id: 20221114-rk3399-tf-a-plat-param-3ab055f40b9e
Best regards,
On 11/14/22 18:37, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Long are gone the times TF-A couldn't handle the FDT passed by U-Boot. Specifically, since commit e7b586987c0a ("rockchip: don't crash if we get an FDT we can't parse") in TF-A, failure to parse the FDT will use the fallback mechanism. This patch was merged in TF-A v2.4-rc0 from two years ago.
Therefore, let's finally pass the FDT to TF-A so that it can get the serial configuration from U-Boot FDT instead of requiring the user to patch TF-A hardcoded fallback values.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com
rockchip: rk3399: pass platform parameter to TF-A
Finally pass the FDT address to TF-A since it now gracefully fallbacks to hardcoded defaults if it cannot parse it. This allows us to avoid modifying hardcoded values in TF-A to enable the console.
Does this mean that with this patch TF-A will properly set the baudrate of the console UART to 1.5MBd? I'm asking because I am about to send the following patch to TF-A (if I can figure out the Gerrit stuff that is...):
=============
From 51c1aa9277f6386b3a8055ad8ad582f894ab9230 Mon Sep 17 00:00:00 2001
From: Jerome Forissier jerome.forissier@linaro.org Date: Thu, 10 Nov 2022 21:38:30 +0100 Subject: [PATCH] rk3399: set console baudrate to 1500000
The default speed for the console UART of the rk3399 SoC is 1.5 MBaud. It is the value used by U-Boot so if TF-A has a different value there is no output when the boot switches from TPL to TF-A BL31 (note that OP-TEE which does not change the speed). The Rockchip loader aka flash helper (rk3399_loader_v1.20.119.bin [1][2]) also uses the same speed. Therefore set the default baudrate to 1500000.
Link: [1] https://dl.radxa.com/rockpi/images/loader/ Link: [2] https://wiki.radxa.com/Rockpi4/dev/usb-install Signed-off-by: Jerome Forissier jerome.forissier@linaro.org --- plat/rockchip/rk3399/rk3399_def.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plat/rockchip/rk3399/rk3399_def.h b/plat/rockchip/rk3399/rk3399_def.h index ba83242eb..8d6ecfbe6 100644 --- a/plat/rockchip/rk3399/rk3399_def.h +++ b/plat/rockchip/rk3399/rk3399_def.h @@ -17,7 +17,7 @@ /************************************************************************** * UART related constants **************************************************************************/ -#define RK3399_BAUDRATE 115200 +#define RK3399_BAUDRATE 1500000 #define RK3399_UART_CLOCK 24000000
/******************************************************************************
Hi Jerome,
On 11/14/22 22:13, Jerome Forissier wrote:
On 11/14/22 18:37, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Long are gone the times TF-A couldn't handle the FDT passed by U-Boot. Specifically, since commit e7b586987c0a ("rockchip: don't crash if we get an FDT we can't parse") in TF-A, failure to parse the FDT will use the fallback mechanism. This patch was merged in TF-A v2.4-rc0 from two years ago.
Therefore, let's finally pass the FDT to TF-A so that it can get the serial configuration from U-Boot FDT instead of requiring the user to patch TF-A hardcoded fallback values.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com
rockchip: rk3399: pass platform parameter to TF-A
Finally pass the FDT address to TF-A since it now gracefully fallbacks to hardcoded defaults if it cannot parse it. This allows us to avoid modifying hardcoded values in TF-A to enable the console.
Does this mean that with this patch TF-A will properly set the baudrate of the console UART to 1.5MBd? I'm asking because I am about to send the
Yes. I tested on my Puma RK3399 which defaults to 115200 with the following patch: diff --git a/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi b/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi index d2349ae90e..1c8ec97d44 100644 --- a/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi +++ b/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi @@ -22,7 +22,7 @@ };
chosen { - stdout-path = "serial0:115200n8"; + stdout-path = "serial0:1500000n8"; u-boot,spl-boot-order = \ "same-as-spl", &norflash, &sdhci, &sdmmc; }; diff --git a/configs/puma-rk3399_defconfig b/configs/puma-rk3399_defconfig index 91f31b37e8..0aa0c1d911 100644 --- a/configs/puma-rk3399_defconfig +++ b/configs/puma-rk3399_defconfig @@ -92,6 +92,7 @@ CONFIG_PWM_ROCKCHIP=y CONFIG_DM_RESET=y CONFIG_DM_RTC=y CONFIG_RTC_ISL1208=y +CONFIG_BAUDRATE=1500000 CONFIG_DEBUG_UART_SHIFT=2 CONFIG_ROCKCHIP_SPI=y CONFIG_SYSRESET=y
and TF-A prints at that baudrate after reading it from the DT passed by U-Boot (I added my own debug messages in TF-A to validate this claim).
This is supported since v2.2, commit 30970e0f2979 ("rockchip: make uart baudrate configurable").
following patch to TF-A (if I can figure out the Gerrit stuff that is...):
Since I went through this yesterday: Link your GitHub account to the Gerrit instance. Add your public ssh key in https://review.trustedfirmware.org/settings/#SSHKeys Then git clone the repo locally. For some reason the instructions in https://review.trustedfirmware.org/admin/repos/TF-A/trusted-firmware-a do not work for me, instead I did: git clone "https://review.trustedfirmware.org/TF-A/trusted-firmware-a" wget https://review.trustedfirmware.org/tools/hooks/commit-msg -O trusted-firmware-a/.git/hooks/commit-msg chmod +x trusted-firmware-a/.git/hooks/commit-msg then you should be able to push with: git push origin HEAD:refs/for/integration -o topic=<some-topic>
============= From 51c1aa9277f6386b3a8055ad8ad582f894ab9230 Mon Sep 17 00:00:00 2001 From: Jerome Forissier jerome.forissier@linaro.org Date: Thu, 10 Nov 2022 21:38:30 +0100 Subject: [PATCH] rk3399: set console baudrate to 1500000
The default speed for the console UART of the rk3399 SoC is 1.5 MBaud. It is the value used by U-Boot so if TF-A has a different value there is no output when the boot switches from TPL to TF-A BL31 (note that OP-TEE which does not change the speed). The Rockchip loader aka flash helper (rk3399_loader_v1.20.119.bin [1][2]) also uses the same speed. Therefore set the default baudrate to 1500000.
This has already been discussed, see discussion there: https://developer.trustedfirmware.org/T762, I would suggest to try to revive the discussion there.
Cheers, Quentin
Hi Qentin,
On 11/15/22 11:05, Quentin Schulz wrote:
Hi Jerome,
On 11/14/22 22:13, Jerome Forissier wrote:
On 11/14/22 18:37, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Long are gone the times TF-A couldn't handle the FDT passed by U-Boot. Specifically, since commit e7b586987c0a ("rockchip: don't crash if we get an FDT we can't parse") in TF-A, failure to parse the FDT will use the fallback mechanism. This patch was merged in TF-A v2.4-rc0 from two years ago.
Therefore, let's finally pass the FDT to TF-A so that it can get the serial configuration from U-Boot FDT instead of requiring the user to patch TF-A hardcoded fallback values.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com
rockchip: rk3399: pass platform parameter to TF-A
Finally pass the FDT address to TF-A since it now gracefully fallbacks to hardcoded defaults if it cannot parse it. This allows us to avoid modifying hardcoded values in TF-A to enable the console.
Does this mean that with this patch TF-A will properly set the baudrate of the console UART to 1.5MBd? I'm asking because I am about to send the
Yes. I tested on my Puma RK3399 which defaults to 115200 with the following patch: diff --git a/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi b/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi index d2349ae90e..1c8ec97d44 100644 --- a/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi +++ b/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi @@ -22,7 +22,7 @@ };
chosen { - stdout-path = "serial0:115200n8"; + stdout-path = "serial0:1500000n8"; u-boot,spl-boot-order = \ "same-as-spl", &norflash, &sdhci, &sdmmc; }; diff --git a/configs/puma-rk3399_defconfig b/configs/puma-rk3399_defconfig index 91f31b37e8..0aa0c1d911 100644 --- a/configs/puma-rk3399_defconfig +++ b/configs/puma-rk3399_defconfig @@ -92,6 +92,7 @@ CONFIG_PWM_ROCKCHIP=y CONFIG_DM_RESET=y CONFIG_DM_RTC=y CONFIG_RTC_ISL1208=y +CONFIG_BAUDRATE=1500000 CONFIG_DEBUG_UART_SHIFT=2 CONFIG_ROCKCHIP_SPI=y CONFIG_SYSRESET=y
and TF-A prints at that baudrate after reading it from the DT passed by U-Boot (I added my own debug messages in TF-A to validate this claim).
This is supported since v2.2, commit 30970e0f2979 ("rockchip: make uart baudrate configurable").
Cool. So your patch would fix Rockpi4 automagically it seems.
following patch to TF-A (if I can figure out the Gerrit stuff that is...):
Since I went through this yesterday: Link your GitHub account to the Gerrit instance. Add your public ssh key in https://review.trustedfirmware.org/settings/#SSHKeys Then git clone the repo locally. For some reason the instructions in https://review.trustedfirmware.org/admin/repos/TF-A/trusted-firmware-a do not work for me, instead I did: git clone "https://review.trustedfirmware.org/TF-A/trusted-firmware-a" wget https://review.trustedfirmware.org/tools/hooks/commit-msg -O trusted-firmware-a/.git/hooks/commit-msg chmod +x trusted-firmware-a/.git/hooks/commit-msg then you should be able to push with: git push origin HEAD:refs/for/integration -o topic=<some-topic>
Weird, I get a "Permission denied (public key)" error although my SSH key *is* visible in the settings. Never mind since I don't need to send my patch anymore ;-)
============= From 51c1aa9277f6386b3a8055ad8ad582f894ab9230 Mon Sep 17 00:00:00 2001 From: Jerome Forissier jerome.forissier@linaro.org Date: Thu, 10 Nov 2022 21:38:30 +0100 Subject: [PATCH] rk3399: set console baudrate to 1500000
The default speed for the console UART of the rk3399 SoC is 1.5 MBaud. It is the value used by U-Boot so if TF-A has a different value there is no output when the boot switches from TPL to TF-A BL31 (note that OP-TEE which does not change the speed). The Rockchip loader aka flash helper (rk3399_loader_v1.20.119.bin [1][2]) also uses the same speed. Therefore set the default baudrate to 1500000.
This has already been discussed, see discussion there: https://developer.trustedfirmware.org/T762, I would suggest to try to revive the discussion there.
Interesting. It looks like there is little hope of getting the default speed changed. TBH I don't even understand why BL31 resets the console when it could just use it, assuming a prior stage has set it up already (BL1/BL2 or in my case U-Boot).
Thanks,
Hi Quentin,
I would prefer you to remove SPL_ATF_NO_PLATFORM_PARAM in those boards you have test,
there may have some boards using legacy ATF binary but still want to use mainline U-Boot
which may have problem with this update.
Thanks,
- Kever
On 2022/11/15 01:37, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Long are gone the times TF-A couldn't handle the FDT passed by U-Boot. Specifically, since commit e7b586987c0a ("rockchip: don't crash if we get an FDT we can't parse") in TF-A, failure to parse the FDT will use the fallback mechanism. This patch was merged in TF-A v2.4-rc0 from two years ago.
Therefore, let's finally pass the FDT to TF-A so that it can get the serial configuration from U-Boot FDT instead of requiring the user to patch TF-A hardcoded fallback values.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com
rockchip: rk3399: pass platform parameter to TF-A
Finally pass the FDT address to TF-A since it now gracefully fallbacks to hardcoded defaults if it cannot parse it. This allows us to avoid modifying hardcoded values in TF-A to enable the console.
This was tested with TF-A v2.7.0 on Puma Haikou RK3399.
To: Simon Glass sjg@chromium.org To: Philipp Tomsich philipp.tomsich@vrull.eu To: Kever Yang kever.yang@rock-chips.com Cc: Hugh Cole-Baker sigmaris@gmail.com Cc: Walter Lozano walter.lozano@collabora.com Cc: u-boot@lists.denx.de
arch/arm/mach-rockchip/Kconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 69d51ff378..2fcc23f9fa 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -249,7 +249,6 @@ config ROCKCHIP_RK3399 imply PRE_CONSOLE_BUFFER imply ROCKCHIP_COMMON_BOARD imply ROCKCHIP_SDRAM_COMMON
- imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF imply SPL_ROCKCHIP_COMMON_BOARD imply TPL_SERIAL imply TPL_LIBCOMMON_SUPPORT
base-commit: 0cbeed4f6648e0e4966475e3544280a69ecb59d3 change-id: 20221114-rk3399-tf-a-plat-param-3ab055f40b9e
Best regards,
Hello,
On Sun, Dec 18, 2022 at 06:00:13PM +0800, Kever Yang wrote:
Hi Quentin,
I would prefer you to remove SPL_ATF_NO_PLATFORM_PARAM in those boards you have test,
then we will have no end of this problem.
there may have some boards using legacy ATF binary but still want to use mainline U-Boot
Why would they do it?
The ATF is SoC-specific, the only board-specific part is the console speed which is resolved by this change.
If people are using old kernel because they have binary driver for a special device they can still change the option in the kconfig.
Maybe adding a note in the rk3399 documentation woukl be dessirable but that's about it.
Thanks
Michal
which may have problem with this update.
Thanks,
- Kever
On 2022/11/15 01:37, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Long are gone the times TF-A couldn't handle the FDT passed by U-Boot. Specifically, since commit e7b586987c0a ("rockchip: don't crash if we get an FDT we can't parse") in TF-A, failure to parse the FDT will use the fallback mechanism. This patch was merged in TF-A v2.4-rc0 from two years ago.
Therefore, let's finally pass the FDT to TF-A so that it can get the serial configuration from U-Boot FDT instead of requiring the user to patch TF-A hardcoded fallback values.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com
rockchip: rk3399: pass platform parameter to TF-A
Finally pass the FDT address to TF-A since it now gracefully fallbacks to hardcoded defaults if it cannot parse it. This allows us to avoid modifying hardcoded values in TF-A to enable the console.
This was tested with TF-A v2.7.0 on Puma Haikou RK3399.
To: Simon Glass sjg@chromium.org To: Philipp Tomsich philipp.tomsich@vrull.eu To: Kever Yang kever.yang@rock-chips.com Cc: Hugh Cole-Baker sigmaris@gmail.com Cc: Walter Lozano walter.lozano@collabora.com Cc: u-boot@lists.denx.de
arch/arm/mach-rockchip/Kconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 69d51ff378..2fcc23f9fa 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -249,7 +249,6 @@ config ROCKCHIP_RK3399 imply PRE_CONSOLE_BUFFER imply ROCKCHIP_COMMON_BOARD imply ROCKCHIP_SDRAM_COMMON
- imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF imply SPL_ROCKCHIP_COMMON_BOARD imply TPL_SERIAL imply TPL_LIBCOMMON_SUPPORT
base-commit: 0cbeed4f6648e0e4966475e3544280a69ecb59d3 change-id: 20221114-rk3399-tf-a-plat-param-3ab055f40b9e
Best regards,
Hi Michal, Kever,
On 12/18/22 3:03 PM, Michal Suchánek msuchanek@suse.de wrote:
Hello,
On Sun, Dec 18, 2022 at 06:00:13PM +0800, Kever Yang wrote:
Hi Quentin,
I would prefer you to remove SPL_ATF_NO_PLATFORM_PARAM in those boards you have test,
then we will have no end of this problem.
I could suggest something else actually. Still go for the removal of the imply, so that the default is to pass data to TF-A but add the option to all RK3399 devices defconfig. This makes it absolutely obvious this option is selected since it'll appear in the defconfig and people can remove it for their platform once they support it and new RK3399-based platforms do not have it on by default, using the much saner alternative.
I think this would satisfy everyone here?
Thoughts? Cheers, Quentin
participants (6)
-
Jerome Forissier -
Kever Yang -
Michal Suchánek -
Quentin Schulz -
Quentin Schulz -
quentin.schulz＠theobroma-systems.com