[PATCH next 0/5] rockchip: rk3399: allow to trigger sysreset in TPL

[PATCH next 0/5] rockchip: rk3399: allow to trigger sysreset in TPL

[Quentin Schulz]

A sysreset-gpio can be provided in an RK3399 platform's Device Tree and
if U-Boot detects a "warm" boot was done, it'll toggle that GPIO to
perform a reset of the PMIC, essentially forcing a cold boot to make
sure there are no non-default values in SoC registers.

For now, this was only supported in SPL, probably because when this was
implemented RK3399 (and specifically Puma) didn't have TPL support so
SPL was the earliest stage. Now that most RK3399 boards (and
specifically Puma) have TPL enabled, it makes sense to add support for
triggering this sysreset from it. It brings the following advantages:
- 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).

For this to be possible, the sysreset function is adapted to be able to
run in SPL and TPL by checking the appropriate symbols and a weak
callback is added in the TPL main C code so that we can hook this
sysreset function within the TPL code path.

@Cc Paul since he's trying to add support for this sysreset to the
Firefly ROC-RK3399-PC[1] and Pine64 ROCKPro64[2]. You'll want to have
gpio1 with boopth-pre-sram though so it makes it to the TPL DTB.

[1] https://lore.kernel.org/u-boot/20240926183111.1324284-1-paulk@sys-base.io/
[2] https://lore.kernel.org/u-boot/20240926183111.1324284-2-paulk@sys-base.io/

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
Quentin Schulz (5):
      pinctrl: rockchip: allow to build for TPL
      rockchip: rk3399: merge CRU check within rk3399_force_power_on_reset
      rockchip: tpl: allow to call board/SoC-specific code before DRAM init
      rockchip: rk3399: allow to handle sysreset-gpio in TPL
      rockchip: configs: puma-rk3399: do the system reset in TPL

 arch/arm/mach-rockchip/rk3399/rk3399.c | 58 +++++++++++++++++++---------------
 arch/arm/mach-rockchip/tpl.c           |  6 ++++
 configs/puma-rk3399_defconfig          |  3 ++
 drivers/pinctrl/Kconfig                |  8 +++++
 drivers/pinctrl/rockchip/Kconfig       |  7 ++++
 5 files changed, 56 insertions(+), 26 deletions(-)
---
base-commit: 56accc56b9aab87ef4809ccc588e1257969cd271
change-id: 20241105-rk3399-sysreset-gpio-tpl-50620781cdd9

Best regards,
-- 
Quentin Schulz <quentin.schulz@cherry.de>

[PATCH next 1/5] pinctrl: rockchip: allow to build for TPL

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

A later commit will make use of the pinctrl driver in TPL so let's add
the ability to build the Rockchip pinctrl driver in TPL.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 drivers/pinctrl/Kconfig          | 8 ++++++++
 drivers/pinctrl/rockchip/Kconfig | 7 +++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index a1d53cfbdbed5ef1030fff04715e1436f167554b..cff7c3ef45b1c7c29017a4512d4970bce4bac26b 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -127,6 +127,14 @@ config SPL_PINCTRL_GENERIC
 	  This option is an SPL-variant of the PINCTRL_GENERIC option.
 	  See the help of PINCTRL_GENERIC for details.
 
+config TPL_PINCTRL_GENERIC
+	bool "Support generic pin controllers in TPL"
+	depends on TPL_PINCTRL_FULL
+	default y
+	help
+	  This option is an TPL-variant of the PINCTRL_GENERIC option.
+	  See the help of PINCTRL_GENERIC for details.
+
 config SPL_PINMUX
 	bool "Support pin multiplexing controllers in SPL"
 	depends on SPL_PINCTRL_GENERIC
diff --git a/drivers/pinctrl/rockchip/Kconfig b/drivers/pinctrl/rockchip/Kconfig
index dc4ba34ae5d581be76786fd05d679d26397fd467..6ad6b189715cee20f0e570d93b9b871e5acb99c3 100644
--- a/drivers/pinctrl/rockchip/Kconfig
+++ b/drivers/pinctrl/rockchip/Kconfig
@@ -14,4 +14,11 @@ config SPL_PINCTRL_ROCKCHIP
 	help
 	  This option is an SPL-variant of the PINCTRL_ROCKCHIP option.
 
+config TPL_PINCTRL_ROCKCHIP
+	bool "Support Rockchip pin controllers in TPL"
+	depends on ARCH_ROCKCHIP && TPL_PINCTRL_GENERIC
+	default y
+	help
+	  This option is an TPL-variant of the PINCTRL_ROCKCHIP option.
+
 endif

-- 
2.47.0

Re: [PATCH next 1/5] pinctrl: rockchip: allow to build for TPL

[Paul Kocialkowski]

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

Hi,

Le Tue 05 Nov 24, 16:36, Quentin Schulz a écrit :
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> A later commit will make use of the pinctrl driver in TPL so let's add
> the ability to build the Rockchip pinctrl driver in TPL.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>

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

Just a nit below in case you need to respin.

Cheers,

Paul

> ---
>  drivers/pinctrl/Kconfig          | 8 ++++++++
>  drivers/pinctrl/rockchip/Kconfig | 7 +++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index a1d53cfbdbed5ef1030fff04715e1436f167554b..cff7c3ef45b1c7c29017a4512d4970bce4bac26b 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -127,6 +127,14 @@ config SPL_PINCTRL_GENERIC
>  	  This option is an SPL-variant of the PINCTRL_GENERIC option.
>  	  See the help of PINCTRL_GENERIC for details.
>  
> +config TPL_PINCTRL_GENERIC
> +	bool "Support generic pin controllers in TPL"
> +	depends on TPL_PINCTRL_FULL
> +	default y
> +	help
> +	  This option is an TPL-variant of the PINCTRL_GENERIC option.
> +	  See the help of PINCTRL_GENERIC for details.

Typo: "a TPL-variant".

>  config SPL_PINMUX
>  	bool "Support pin multiplexing controllers in SPL"
>  	depends on SPL_PINCTRL_GENERIC
> diff --git a/drivers/pinctrl/rockchip/Kconfig b/drivers/pinctrl/rockchip/Kconfig
> index dc4ba34ae5d581be76786fd05d679d26397fd467..6ad6b189715cee20f0e570d93b9b871e5acb99c3 100644
> --- a/drivers/pinctrl/rockchip/Kconfig
> +++ b/drivers/pinctrl/rockchip/Kconfig
> @@ -14,4 +14,11 @@ config SPL_PINCTRL_ROCKCHIP
>  	help
>  	  This option is an SPL-variant of the PINCTRL_ROCKCHIP option.
>  
> +config TPL_PINCTRL_ROCKCHIP
> +	bool "Support Rockchip pin controllers in TPL"
> +	depends on ARCH_ROCKCHIP && TPL_PINCTRL_GENERIC
> +	default y
> +	help
> +	  This option is an TPL-variant of the PINCTRL_ROCKCHIP option.

Ditto.

> +
>  endif
> 
> -- 
> 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 --]

[PATCH next 2/5] rockchip: rk3399: merge CRU check within rk3399_force_power_on_reset

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

To prepare to support forcing power on reset from TPL which would have
the exact same logic, just in an earlier stage, let's merge the CRU
check that triggers the power on reset with the rest of the logic.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 arch/arm/mach-rockchip/rk3399/rk3399.c | 43 +++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
index edccb2a3980d0e3921aa4073b67aea6a00f18b8d..7b6a822ed04b8151a5da147056dbf73ffdafd149 100644
--- a/arch/arm/mach-rockchip/rk3399/rk3399.c
+++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
@@ -172,9 +172,29 @@ void board_debug_uart_init(void)
 #if defined(CONFIG_XPL_BUILD) && !defined(CONFIG_TPL_BUILD)
 static void rk3399_force_power_on_reset(void)
 {
+	const struct rockchip_cru *cru = rockchip_get_cru();
 	ofnode node;
 	struct gpio_desc sysreset_gpio;
 
+	/*
+	 * The RK3399 resets only 'almost all logic' (see also in the
+	 * TRM "3.9.4 Global software reset"), when issuing a software
+	 * reset. This may cause issues during boot-up for some
+	 * configurations of the application software stack.
+	 *
+	 * To work around this, we test whether the last reset reason
+	 * was a power-on reset and (if not) issue an overtemp-reset to
+	 * reset the entire module.
+	 *
+	 * While this was previously fixed by modifying the various
+	 * places that could generate a software reset (e.g. U-Boot's
+	 * sysreset driver, the ATF or Linux), we now have it here to
+	 * ensure that we no longer have to track this through the
+	 * various components.
+	 */
+	if (cru->glb_rst_st == 0)
+		return;
+
 	if (!IS_ENABLED(CONFIG_SPL_GPIO)) {
 		debug("%s: trying to force a power-on reset but no GPIO "
 		      "support in SPL!\n", __func__);
@@ -206,27 +226,6 @@ void spl_board_init(void)
 {
 	led_setup();
 
-	if (IS_ENABLED(CONFIG_SPL_GPIO)) {
-		struct rockchip_cru *cru = rockchip_get_cru();
-
-		/*
-		 * The RK3399 resets only 'almost all logic' (see also in the
-		 * TRM "3.9.4 Global software reset"), when issuing a software
-		 * reset. This may cause issues during boot-up for some
-		 * configurations of the application software stack.
-		 *
-		 * To work around this, we test whether the last reset reason
-		 * was a power-on reset and (if not) issue an overtemp-reset to
-		 * reset the entire module.
-		 *
-		 * While this was previously fixed by modifying the various
-		 * places that could generate a software reset (e.g. U-Boot's
-		 * sysreset driver, the ATF or Linux), we now have it here to
-		 * ensure that we no longer have to track this through the
-		 * various components.
-		 */
-		if (cru->glb_rst_st != 0)
-			rk3399_force_power_on_reset();
-	}
+	rk3399_force_power_on_reset();
 }
 #endif

-- 
2.47.0

Re: [PATCH next 2/5] rockchip: rk3399: merge CRU check within rk3399_force_power_on_reset

[Paul Kocialkowski]

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

Hi,

Le Tue 05 Nov 24, 16:36, Quentin Schulz a écrit :
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> To prepare to support forcing power on reset from TPL which would have
> the exact same logic, just in an earlier stage, let's merge the CRU
> check that triggers the power on reset with the rest of the logic.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>

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

Cheers,

Paul

> ---
>  arch/arm/mach-rockchip/rk3399/rk3399.c | 43 +++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
> index edccb2a3980d0e3921aa4073b67aea6a00f18b8d..7b6a822ed04b8151a5da147056dbf73ffdafd149 100644
> --- a/arch/arm/mach-rockchip/rk3399/rk3399.c
> +++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
> @@ -172,9 +172,29 @@ void board_debug_uart_init(void)
>  #if defined(CONFIG_XPL_BUILD) && !defined(CONFIG_TPL_BUILD)
>  static void rk3399_force_power_on_reset(void)
>  {
> +	const struct rockchip_cru *cru = rockchip_get_cru();
>  	ofnode node;
>  	struct gpio_desc sysreset_gpio;
>  
> +	/*
> +	 * The RK3399 resets only 'almost all logic' (see also in the
> +	 * TRM "3.9.4 Global software reset"), when issuing a software
> +	 * reset. This may cause issues during boot-up for some
> +	 * configurations of the application software stack.
> +	 *
> +	 * To work around this, we test whether the last reset reason
> +	 * was a power-on reset and (if not) issue an overtemp-reset to
> +	 * reset the entire module.
> +	 *
> +	 * While this was previously fixed by modifying the various
> +	 * places that could generate a software reset (e.g. U-Boot's
> +	 * sysreset driver, the ATF or Linux), we now have it here to
> +	 * ensure that we no longer have to track this through the
> +	 * various components.
> +	 */
> +	if (cru->glb_rst_st == 0)
> +		return;
> +
>  	if (!IS_ENABLED(CONFIG_SPL_GPIO)) {
>  		debug("%s: trying to force a power-on reset but no GPIO "
>  		      "support in SPL!\n", __func__);
> @@ -206,27 +226,6 @@ void spl_board_init(void)
>  {
>  	led_setup();
>  
> -	if (IS_ENABLED(CONFIG_SPL_GPIO)) {
> -		struct rockchip_cru *cru = rockchip_get_cru();
> -
> -		/*
> -		 * The RK3399 resets only 'almost all logic' (see also in the
> -		 * TRM "3.9.4 Global software reset"), when issuing a software
> -		 * reset. This may cause issues during boot-up for some
> -		 * configurations of the application software stack.
> -		 *
> -		 * To work around this, we test whether the last reset reason
> -		 * was a power-on reset and (if not) issue an overtemp-reset to
> -		 * reset the entire module.
> -		 *
> -		 * While this was previously fixed by modifying the various
> -		 * places that could generate a software reset (e.g. U-Boot's
> -		 * sysreset driver, the ATF or Linux), we now have it here to
> -		 * ensure that we no longer have to track this through the
> -		 * various components.
> -		 */
> -		if (cru->glb_rst_st != 0)
> -			rk3399_force_power_on_reset();
> -	}
> +	rk3399_force_power_on_reset();
>  }
>  #endif
> 
> -- 
> 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 --]

[PATCH next 3/5] rockchip: tpl: allow to call board/SoC-specific code before DRAM init

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

This defines a weak tpl_board_init function that can be used for running
board/SoC-specific code before the DRAM init happens, similarly to
spl_board_init() for SPL.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 arch/arm/mach-rockchip/tpl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c
index bbb9329e725af79ea4c4049aa7890a4a143e7df5..6b880f19f84e57e7bc0c93b16b188bc56267827e 100644
--- a/arch/arm/mach-rockchip/tpl.c
+++ b/arch/arm/mach-rockchip/tpl.c
@@ -21,6 +21,10 @@
 #include <timestamp.h>
 #endif
 
+__weak void tpl_board_init(void)
+{
+}
+
 void board_init_f(ulong dummy)
 {
 	struct udevice *dev;
@@ -54,6 +58,8 @@ void board_init_f(ulong dummy)
 	if (IS_ENABLED(CONFIG_SYS_ARCH_TIMER))
 		timer_init();
 
+	tpl_board_init();
+
 	ret = uclass_get_device(UCLASS_RAM, 0, &dev);
 	if (ret) {
 		printf("DRAM init failed: %d\n", ret);

-- 
2.47.0

Re: [PATCH next 3/5] rockchip: tpl: allow to call board/SoC-specific code before DRAM init

[Paul Kocialkowski]

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

Hi,

Le Tue 05 Nov 24, 16:36, Quentin Schulz a écrit :
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> This defines a weak tpl_board_init function that can be used for running
> board/SoC-specific code before the DRAM init happens, similarly to
> spl_board_init() for SPL.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>

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

Cheers,

Paul

> ---
>  arch/arm/mach-rockchip/tpl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c
> index bbb9329e725af79ea4c4049aa7890a4a143e7df5..6b880f19f84e57e7bc0c93b16b188bc56267827e 100644
> --- a/arch/arm/mach-rockchip/tpl.c
> +++ b/arch/arm/mach-rockchip/tpl.c
> @@ -21,6 +21,10 @@
>  #include <timestamp.h>
>  #endif
>  
> +__weak void tpl_board_init(void)
> +{
> +}
> +
>  void board_init_f(ulong dummy)
>  {
>  	struct udevice *dev;
> @@ -54,6 +58,8 @@ void board_init_f(ulong dummy)
>  	if (IS_ENABLED(CONFIG_SYS_ARCH_TIMER))
>  		timer_init();
>  
> +	tpl_board_init();
> +
>  	ret = uclass_get_device(UCLASS_RAM, 0, &dev);
>  	if (ret) {
>  		printf("DRAM init failed: %d\n", ret);
> 
> -- 
> 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 --]

[PATCH next 4/5] rockchip: rk3399: allow to handle sysreset-gpio in TPL

[Quentin Schulz]

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.

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>
---
 arch/arm/mach-rockchip/rk3399/rk3399.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
index 7b6a822ed04b8151a5da147056dbf73ffdafd149..26fd04691699d81f5fecd282964c7b3aff999717 100644
--- a/arch/arm/mach-rockchip/rk3399/rk3399.c
+++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
@@ -169,7 +169,7 @@ void board_debug_uart_init(void)
 }
 #endif
 
-#if defined(CONFIG_XPL_BUILD) && !defined(CONFIG_TPL_BUILD)
+#if defined(CONFIG_XPL_BUILD)
 static void rk3399_force_power_on_reset(void)
 {
 	const struct rockchip_cru *cru = rockchip_get_cru();
@@ -195,9 +195,9 @@ static void rk3399_force_power_on_reset(void)
 	if (cru->glb_rst_st == 0)
 		return;
 
-	if (!IS_ENABLED(CONFIG_SPL_GPIO)) {
+	if (!CONFIG_IS_ENABLED(GPIO)) {
 		debug("%s: trying to force a power-on reset but no GPIO "
-		      "support in SPL!\n", __func__);
+		      "support in " PHASE_NAME "!\n", __func__);
 		return;
 	}
 
@@ -218,6 +218,7 @@ static void rk3399_force_power_on_reset(void)
 	dm_gpio_set_value(&sysreset_gpio, 1);
 }
 
+# if !defined(CONFIG_TPL_BUILD)
 void __weak led_setup(void)
 {
 }
@@ -228,4 +229,10 @@ void spl_board_init(void)
 
 	rk3399_force_power_on_reset();
 }
-#endif
+# else  // !defined(CONFIG_TPL_BUILD)
+void tpl_board_init(void)
+{
+	rk3399_force_power_on_reset();
+}
+# endif // !defined(CONFIG_TPL_BUILD)
+#endif // defined(CONFIG_XPL_BUILD)

-- 
2.47.0

Re: [PATCH next 4/5] rockchip: rk3399: allow to handle sysreset-gpio in TPL

[Paul Kocialkowski]

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

Hi,

Le Tue 05 Nov 24, 16:36, 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.
> 
> 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>

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

See a nit below.

Cheers,

Paul

> ---
>  arch/arm/mach-rockchip/rk3399/rk3399.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
> index 7b6a822ed04b8151a5da147056dbf73ffdafd149..26fd04691699d81f5fecd282964c7b3aff999717 100644
> --- a/arch/arm/mach-rockchip/rk3399/rk3399.c
> +++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
> @@ -169,7 +169,7 @@ void board_debug_uart_init(void)
>  }
>  #endif
>  
> -#if defined(CONFIG_XPL_BUILD) && !defined(CONFIG_TPL_BUILD)
> +#if defined(CONFIG_XPL_BUILD)
>  static void rk3399_force_power_on_reset(void)
>  {
>  	const struct rockchip_cru *cru = rockchip_get_cru();
> @@ -195,9 +195,9 @@ static void rk3399_force_power_on_reset(void)
>  	if (cru->glb_rst_st == 0)
>  		return;
>  
> -	if (!IS_ENABLED(CONFIG_SPL_GPIO)) {
> +	if (!CONFIG_IS_ENABLED(GPIO)) {
>  		debug("%s: trying to force a power-on reset but no GPIO "
> -		      "support in SPL!\n", __func__);
> +		      "support in " PHASE_NAME "!\n", __func__);
>  		return;
>  	}
>  
> @@ -218,6 +218,7 @@ static void rk3399_force_power_on_reset(void)
>  	dm_gpio_set_value(&sysreset_gpio, 1);
>  }
>  
> +# if !defined(CONFIG_TPL_BUILD)

Coding style in the file doesn't put a space after # (even when nested).

>  void __weak led_setup(void)
>  {
>  }
> @@ -228,4 +229,10 @@ void spl_board_init(void)
>  
>  	rk3399_force_power_on_reset();
>  }
> -#endif
> +# else  // !defined(CONFIG_TPL_BUILD)

Ditto and also doesn't have such comments.

> +void tpl_board_init(void)
> +{
> +	rk3399_force_power_on_reset();
> +}
> +# endif // !defined(CONFIG_TPL_BUILD)

Ditto.

> +#endif // defined(CONFIG_XPL_BUILD)
> 
> -- 
> 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 --]

Re: [PATCH next 4/5] rockchip: rk3399: allow to handle sysreset-gpio in TPL

[Paul Kocialkowski]

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

Hi again,

Le Tue 05 Nov 24, 16:36, 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.
> 
> 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:

Just one other thing here: you could as well remove the call in spl_board_init.
There's just one user of this currently (the Puma board) so I don't think we
need to keep backwards-compatibility in the SPL.

Cheers,

Paul

> """
> 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>
> ---
>  arch/arm/mach-rockchip/rk3399/rk3399.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
> index 7b6a822ed04b8151a5da147056dbf73ffdafd149..26fd04691699d81f5fecd282964c7b3aff999717 100644
> --- a/arch/arm/mach-rockchip/rk3399/rk3399.c
> +++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
> @@ -169,7 +169,7 @@ void board_debug_uart_init(void)
>  }
>  #endif
>  
> -#if defined(CONFIG_XPL_BUILD) && !defined(CONFIG_TPL_BUILD)
> +#if defined(CONFIG_XPL_BUILD)
>  static void rk3399_force_power_on_reset(void)
>  {
>  	const struct rockchip_cru *cru = rockchip_get_cru();
> @@ -195,9 +195,9 @@ static void rk3399_force_power_on_reset(void)
>  	if (cru->glb_rst_st == 0)
>  		return;
>  
> -	if (!IS_ENABLED(CONFIG_SPL_GPIO)) {
> +	if (!CONFIG_IS_ENABLED(GPIO)) {
>  		debug("%s: trying to force a power-on reset but no GPIO "
> -		      "support in SPL!\n", __func__);
> +		      "support in " PHASE_NAME "!\n", __func__);
>  		return;
>  	}
>  
> @@ -218,6 +218,7 @@ static void rk3399_force_power_on_reset(void)
>  	dm_gpio_set_value(&sysreset_gpio, 1);
>  }
>  
> +# if !defined(CONFIG_TPL_BUILD)
>  void __weak led_setup(void)
>  {
>  }
> @@ -228,4 +229,10 @@ void spl_board_init(void)
>  
>  	rk3399_force_power_on_reset();
>  }
> -#endif
> +# else  // !defined(CONFIG_TPL_BUILD)
> +void tpl_board_init(void)
> +{
> +	rk3399_force_power_on_reset();
> +}
> +# endif // !defined(CONFIG_TPL_BUILD)
> +#endif // defined(CONFIG_XPL_BUILD)
> 
> -- 
> 2.47.0
> 

-- 
Paul Kocialkowski,

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

Contributor to fully free software support for selected hardware.

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

[PATCH next 5/5] rockchip: configs: puma-rk3399: do the system reset in TPL

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

RK3399 Puma can trigger a full system reset at runtime by toggling a
GPIO. This is done whenever the system has NOT cold booted (i.e. from a
power cycle). For RK3399 Puma, this is currently done in SPL but only
because when this was implemented TPL wasn't used on RK3399 (or at least
not Puma). Now that we have TPL on RK3399 (and Puma), it makes sense to
trigger this reset as early as possible, so let's enable the drivers
required in TPL (Rockchip GPIO and pinctrl).

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 configs/puma-rk3399_defconfig | 3 +++
 1 file changed, 3 insertions(+)

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

Re: [PATCH next 5/5] rockchip: configs: puma-rk3399: do the system reset in TPL

[Paul Kocialkowski]

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

Hi,

Le Tue 05 Nov 24, 16:36, Quentin Schulz a écrit :
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> RK3399 Puma can trigger a full system reset at runtime by toggling a
> GPIO. This is done whenever the system has NOT cold booted (i.e. from a
> power cycle). For RK3399 Puma, this is currently done in SPL but only
> because when this was implemented TPL wasn't used on RK3399 (or at least
> not Puma). Now that we have TPL on RK3399 (and Puma), it makes sense to
> trigger this reset as early as possible, so let's enable the drivers
> required in TPL (Rockchip GPIO and pinctrl).
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>

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

Cheers,

Paul

> ---
>  configs/puma-rk3399_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 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 --]

Re: [PATCH next 0/5] rockchip: rk3399: allow to trigger sysreset in TPL

[Paul Kocialkowski]

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

Hi,

Le Tue 05 Nov 24, 16:36, Quentin Schulz a écrit :
> A sysreset-gpio can be provided in an RK3399 platform's Device Tree and
> if U-Boot detects a "warm" boot was done, it'll toggle that GPIO to
> perform a reset of the PMIC, essentially forcing a cold boot to make
> sure there are no non-default values in SoC registers.
> 
> For now, this was only supported in SPL, probably because when this was
> implemented RK3399 (and specifically Puma) didn't have TPL support so
> SPL was the earliest stage. Now that most RK3399 boards (and
> specifically Puma) have TPL enabled, it makes sense to add support for
> triggering this sysreset from it. It brings the following advantages:
> - 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:

Thanks for doing this, I think it makes a lot of sense to have it in the TPL!

Cheers,

Paul

> """
> 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).
> 
> For this to be possible, the sysreset function is adapted to be able to
> run in SPL and TPL by checking the appropriate symbols and a weak
> callback is added in the TPL main C code so that we can hook this
> sysreset function within the TPL code path.
> 
> @Cc Paul since he's trying to add support for this sysreset to the
> Firefly ROC-RK3399-PC[1] and Pine64 ROCKPro64[2]. You'll want to have
> gpio1 with boopth-pre-sram though so it makes it to the TPL DTB.
> 
> [1] https://lore.kernel.org/u-boot/20240926183111.1324284-1-paulk@sys-base.io/
> [2] https://lore.kernel.org/u-boot/20240926183111.1324284-2-paulk@sys-base.io/
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
> Quentin Schulz (5):
>       pinctrl: rockchip: allow to build for TPL
>       rockchip: rk3399: merge CRU check within rk3399_force_power_on_reset
>       rockchip: tpl: allow to call board/SoC-specific code before DRAM init
>       rockchip: rk3399: allow to handle sysreset-gpio in TPL
>       rockchip: configs: puma-rk3399: do the system reset in TPL
> 
>  arch/arm/mach-rockchip/rk3399/rk3399.c | 58 +++++++++++++++++++---------------
>  arch/arm/mach-rockchip/tpl.c           |  6 ++++
>  configs/puma-rk3399_defconfig          |  3 ++
>  drivers/pinctrl/Kconfig                |  8 +++++
>  drivers/pinctrl/rockchip/Kconfig       |  7 ++++
>  5 files changed, 56 insertions(+), 26 deletions(-)
> ---
> base-commit: 56accc56b9aab87ef4809ccc588e1257969cd271
> change-id: 20241105-rk3399-sysreset-gpio-tpl-50620781cdd9
> 
> Best regards,
> -- 
> Quentin Schulz <quentin.schulz@cherry.de>
> 

-- 
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 --]