[RFC PATCH 1/2] spl: enable regulator-boot-on and disable regulator-force-boot-off

[RFC PATCH 1/2] spl: enable regulator-boot-on and disable regulator-force-boot-off

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

This makes sure regulators that need to be turned on or off at boot are
turned on or off in the SPL.

This may be required for the SPL to do some operations, such as finding
possible loading media for U-Boot proper.

Cc: Quentin Schulz <foss+uboot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---

 - RFC because only tested on Puma Haikou RK3399

 common/spl/spl.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index c8c463f80b..762e9918c7 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -37,6 +37,9 @@
 #include <fdt_support.h>
 #include <bootcount.h>
 #include <wdt.h>
+#if CONFIG_IS_ENABLED(DM_REGULATOR)
+#include <power/regulator.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -766,6 +769,15 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	if (CONFIG_IS_ENABLED(GPIO_HOG))
 		gpio_hog_probe_all();
 
+	if (CONFIG_IS_ENABLED(DM_REGULATOR)) {
+		if (regulators_enable_boot_on(false))
+			debug("%s: Cannot enable boot on regulator\n",
+			      __func__);
+		if (regulators_enable_boot_off(false))
+			debug("%s: Cannot enable boot off regulator\n",
+			      __func__);
+	}
+
 #if CONFIG_IS_ENABLED(BOARD_INIT)
 	spl_board_init();
 #endif
-- 
2.36.1

[RFC PATCH 2/2] rockchip: rk3399: remove duplicate call to regulators_enable_boot_on

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

An earlier commit makes the common SPL code call
regulators_enable_boot_on and regulators_enable_boot_off before
iterating over possible boot media for U-Boot proper. There is therefore
no need to do this in the rk3399-specific code, so let's remove it.

Cc: Quentin Schulz <foss+uboot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---

- This patch depends on
  https://lore.kernel.org/u-boot/20220715150949.952853-1-foss+uboot@0leil.net/
  and
  https://lore.kernel.org/u-boot/20220715150949.952853-2-foss+uboot@0leil.net/

 arch/arm/mach-rockchip/rk3399/rk3399.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
index db8a6cb83a..691d69dc59 100644
--- a/arch/arm/mach-rockchip/rk3399/rk3399.c
+++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
@@ -277,15 +277,5 @@ void spl_board_init(void)
 		if (cru->glb_rst_st != 0)
 			rk3399_force_power_on_reset();
 	}
-
-	if (IS_ENABLED(CONFIG_SPL_DM_REGULATOR)) {
-		/*
-		 * Turning the eMMC and SPI back on (if disabled via the Qseven
-		 * BIOS_ENABLE) signal is done through a always-on regulator).
-		 */
-		if (regulators_enable_boot_on(false))
-			debug("%s: Cannot enable boot on regulator\n",
-			      __func__);
-	}
 }
 #endif
-- 
2.36.1

Re: [SPAM] [RFC PATCH 2/2] rockchip: rk3399: remove duplicate call to regulators_enable_boot_on

[Xavier Drudis Ferran]

El Fri, Jul 15, 2022 at 05:14:26PM +0200, Quentin Schulz deia:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> An earlier commit makes the common SPL code call
> regulators_enable_boot_on and regulators_enable_boot_off before
> iterating over possible boot media for U-Boot proper. There is therefore
> no need to do this in the rk3399-specific code, so let's remove it.
> 
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>

Tested on a Rock-Pi-4B and didn't see any regression.
Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat>


 
> - This patch depends on
>   https://lore.kernel.org/u-boot/20220715150949.952853-1-foss+uboot@0leil.net/
>   and
>   https://lore.kernel.org/u-boot/20220715150949.952853-2-foss+uboot@0leil.net/
> 
>  arch/arm/mach-rockchip/rk3399/rk3399.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
> index db8a6cb83a..691d69dc59 100644
> --- a/arch/arm/mach-rockchip/rk3399/rk3399.c
> +++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
> @@ -277,15 +277,5 @@ void spl_board_init(void)
>  		if (cru->glb_rst_st != 0)
>  			rk3399_force_power_on_reset();
>  	}
> -
> -	if (IS_ENABLED(CONFIG_SPL_DM_REGULATOR)) {
> -		/*
> -		 * Turning the eMMC and SPI back on (if disabled via the Qseven
> -		 * BIOS_ENABLE) signal is done through a always-on regulator).
> -		 */
> -		if (regulators_enable_boot_on(false))
> -			debug("%s: Cannot enable boot on regulator\n",
> -			      __func__);
> -	}
>  }
>  #endif
> -- 
> 2.36.1
> 

Re: [SPAM] [RFC PATCH 1/2] spl: enable regulator-boot-on and disable regulator-force-boot-off

[Xavier Drudis Ferran]

Hello, and thanks for you work 

El Fri, Jul 15, 2022 at 05:14:25PM +0200, Quentin Schulz deia:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> This makes sure regulators that need to be turned on or off at boot are
> turned on or off in the SPL.
> 
> This may be required for the SPL to do some operations, such as finding
> possible loading media for U-Boot proper.
> 
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
> 
>  - RFC because only tested on Puma Haikou RK3399
> 
>  common/spl/spl.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index c8c463f80b..762e9918c7 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -37,6 +37,9 @@
>  #include <fdt_support.h>
>  #include <bootcount.h>
>  #include <wdt.h>
> +#if CONFIG_IS_ENABLED(DM_REGULATOR)
> +#include <power/regulator.h>
> +#endif
>

Can we remove the #if ? 
Otherwise I get 2 compilation warnings in tpl,
because I miss the dummy regulators_enable_boot_on (and _off)
  in include/power/regulator.h 

When compiling tpl for Rock-pi-4 
I have CONFIG_DM_REGULATOR=y, but not CONFIG_TPL_DM_REGULATOR or
CONFIG_SPL_DM_REGULATOR.

>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -766,6 +769,15 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  	if (CONFIG_IS_ENABLED(GPIO_HOG))
>  		gpio_hog_probe_all();
>  
> +	if (CONFIG_IS_ENABLED(DM_REGULATOR)) {
> +		if (regulators_enable_boot_on(false))
> +			debug("%s: Cannot enable boot on regulator\n",
> +			      __func__);
> +		if (regulators_enable_boot_off(false))
> +			debug("%s: Cannot enable boot off regulator\n",
> +			      __func__);
> +	}
> +

This still introduces a warning for me. 

warning: implicit declaration of function 'regulators_enable_boot_off'

So maybe the dummy function must be added to include/power/regulator.h
 commit 16cc5ad0b439 ("power: regulator: add dummy helper")
introduced the dummy  regulators_enable_boot_on but missed 
the regulators_enable_boot_off.

Alternatively one could replace the if with an #if, but I think that'd 
be against U-Boot policy. 

Re: [SPAM] [RFC PATCH 1/2] spl: enable regulator-boot-on and disable regulator-force-boot-off

[Xavier Drudis Ferran]

El Mon, Jul 18, 2022 at 10:34:54AM +0200, Xavier Drudis Ferran deia:
> 
> Can we remove the #if ? 

Not sure if I was clear. 
I meant remove the #if and #endif and leave an inconditional #include.

Re: [SPAM] [RFC PATCH 1/2] spl: enable regulator-boot-on and disable regulator-force-boot-off

[Xavier Drudis Ferran]

El Fri, Jul 15, 2022 at 05:14:25PM +0200, Quentin Schulz deia:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> This makes sure regulators that need to be turned on or off at boot are
> turned on or off in the SPL.
> 
> This may be required for the SPL to do some operations, such as finding
> possible loading media for U-Boot proper.
> 
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>

Tested on a Rock-Pi-4B and didn't see any regression.
Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
 
>  - RFC because only tested on Puma Haikou RK3399
> 
>  common/spl/spl.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index c8c463f80b..762e9918c7 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -37,6 +37,9 @@
>  #include <fdt_support.h>
>  #include <bootcount.h>
>  #include <wdt.h>
> +#if CONFIG_IS_ENABLED(DM_REGULATOR)
> +#include <power/regulator.h>
> +#endif
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -766,6 +769,15 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  	if (CONFIG_IS_ENABLED(GPIO_HOG))
>  		gpio_hog_probe_all();
>  
> +	if (CONFIG_IS_ENABLED(DM_REGULATOR)) {
> +		if (regulators_enable_boot_on(false))
> +			debug("%s: Cannot enable boot on regulator\n",
> +			      __func__);
> +		if (regulators_enable_boot_off(false))
> +			debug("%s: Cannot enable boot off regulator\n",
> +			      __func__);
> +	}
> +
>  #if CONFIG_IS_ENABLED(BOARD_INIT)
>  	spl_board_init();
>  #endif
> -- 
> 2.36.1
>