Re: [PATCH next v2 4/4] rockchip: rk3399: move sysreset-gpio logic to TPL

[Paul Kocialkowski <paulk@sys-base.io>]

[-- Attachment #1: Type: text/plain, Size: 4485 bytes --]

Hi,

Le Wed 06 Nov 24, 12:29, Quentin Schulz a écrit :
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> If TPL_GPIO and TPL_PINCTRL_ROCKCHIP are enabled and a sysreset-gpio is
> provided in the TPL Device Tree, this will trigger a system reset
> similar to what's currently been done in SPL whenever the RK3399 "warm"
> boots. Because there's currently only one user of sysreset-gpio logic,
> and TPL is enabled on that board, so let's migrate the logic and that
> board to do it in TPL.
> 
> There are three reasons for moving this earlier:
> - faster boot time as we don't need to reach SPL to be able to reset the
>   system on a condition we know is already met in TPL,
> - have less code to be impacted by the issue this system reset works
>   around (that is, "unclean" SoC registers after a reboot),
> - less confusion around the reason for restarting. Indeed when done from
>   SPL, the following log can be observed:
> 
> """
> U-Boot TPL 2025.01-rc1-00165-gd79216ca9878-dirty (Nov 05 2024 - 15:31:45)
> Channel 0: DDR3, 666MHz
> BW=32 Col=10 Bk=8 CS0 Row=16 CS=1 Die BW=16 Size=2048MB
> Channel 1: DDR3, 666MHz
> BW=32 Col=10 Bk=8 CS0 Row=16 CS=1 Die BW=16 Size=2048MB
> 256B stride
> Trying to boot from BOOTROM
> Returning to boot ROM...
> 
> U-Boot SPL 2025.01-rc1-00165-gd79216ca9878-dirty (Nov 05 2024 - 15:31:45 +0100)
> Trying to boot from MMC2
> 
> U-Boot TPL 2025.01-rc1-00165-gd79216ca9878-dirty (Nov 05 2024 - 15:31:45)
> """
> 
> possibly hinting at an issue within the SPL when loading the fitImage
> from MMC2 instead of the normal course of events (a system reset).
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks for keeping it only in TPL!

Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>

With just one small nit below:

> ---
>  arch/arm/mach-rockchip/rk3399/rk3399.c | 15 ++++++++++-----
>  configs/puma-rk3399_defconfig          |  3 +++
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
> index 7b6a822ed04b8151a5da147056dbf73ffdafd149..0c28241c603e343c5140322f2778678e95fa84fd 100644
> --- a/arch/arm/mach-rockchip/rk3399/rk3399.c
> +++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
> @@ -169,7 +169,8 @@ void board_debug_uart_init(void)
>  }
>  #endif
>  
> -#if defined(CONFIG_XPL_BUILD) && !defined(CONFIG_TPL_BUILD)
> +#if defined(CONFIG_XPL_BUILD)
> +#if defined(CONFIG_TPL_BUILD)
>  static void rk3399_force_power_on_reset(void)
>  {
>  	const struct rockchip_cru *cru = rockchip_get_cru();
> @@ -195,9 +196,9 @@ static void rk3399_force_power_on_reset(void)
>  	if (cru->glb_rst_st == 0)
>  		return;
>  
> -	if (!IS_ENABLED(CONFIG_SPL_GPIO)) {
> +	if (!IS_ENABLED(CONFIG_TPL_GPIO)) {
>  		debug("%s: trying to force a power-on reset but no GPIO "
> -		      "support in SPL!\n", __func__);
> +		      "support in TPL!\n", __func__);
>  		return;
>  	}
>  
> @@ -218,6 +219,11 @@ static void rk3399_force_power_on_reset(void)
>  	dm_gpio_set_value(&sysreset_gpio, 1);
>  }
>  
> +void tpl_board_init(void)
> +{
> +	rk3399_force_power_on_reset();
> +}
> +# else

No whitespace before "else" here.

>  void __weak led_setup(void)
>  {
>  }
> @@ -225,7 +231,6 @@ void __weak led_setup(void)
>  void spl_board_init(void)
>  {
>  	led_setup();
> -
> -	rk3399_force_power_on_reset();
>  }
>  #endif
> +#endif
> diff --git a/configs/puma-rk3399_defconfig b/configs/puma-rk3399_defconfig
> index 67c0ee72c925cdd49066980b0fde4131c86a99a8..7a180b1413036234d834773778f6c0f0a7e85380 100644
> --- a/configs/puma-rk3399_defconfig
> +++ b/configs/puma-rk3399_defconfig
> @@ -30,6 +30,7 @@ CONFIG_SPL_I2C=y
>  CONFIG_SPL_POWER=y
>  CONFIG_SPL_SPI_LOAD=y
>  CONFIG_TPL=y
> +CONFIG_TPL_GPIO=y
>  # CONFIG_BOOTM_NETBSD is not set
>  # CONFIG_BOOTM_PLAN9 is not set
>  # CONFIG_BOOTM_RTEMS is not set
> @@ -78,6 +79,8 @@ CONFIG_ETH_DESIGNWARE=y
>  CONFIG_GMAC_ROCKCHIP=y
>  CONFIG_PHY_ROCKCHIP_INNO_USB2=y
>  CONFIG_PHY_ROCKCHIP_TYPEC=y
> +CONFIG_TPL_PINCTRL=y
> +CONFIG_TPL_PINCTRL_FULL=y
>  CONFIG_DM_PMIC_FAN53555=y
>  CONFIG_PMIC_RK8XX=y
>  CONFIG_SPL_PMIC_RK8XX=y
> 
> -- 
> 2.47.0
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Specialist in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]