[PATCH] rockchip: rk3399: pass platform parameter to TF-A by default

[Jerome Forissier]

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 at 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 at 0leil.net>
>>> Signed-off-by: Quentin Schulz <quentin.schulz at 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 at 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,
-- 
Jerome