| Message ID | 20220722100908.3853994-1-foss+uboot@0leil.net |
|---|---|
| State | Rejected |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | [v2,1/2] spl: enable regulator-boot-on and disable regulator-force-boot-off | expand |
Hi Quentin, I don't think we need to add regulators_enable_boot_on/off callback in SPL framework, the rk3399-puma/Qseven is the only board need to do this in the SPL right now. The hardware design for SPI/eMMC should always power on the storage which need by very early which should also used by bootrom. And for more device power will be enabled in U-Boot proper. Thanks, - Kever On 2022/7/22 18:09, Quentin Schulz wrote: > 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> > Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> > --- > > v2: > - added Tested-by, > - fixed build for boards with SPL_DM_REGULATOR disabled by always > included power/regulator.h and defining a dummy implementation for > regulators_enable_boot_off, > > common/spl/spl.c | 10 ++++++++++ > include/power/regulator.h | 5 +++++ > 2 files changed, 15 insertions(+) > > diff --git a/common/spl/spl.c b/common/spl/spl.c > index 29e0898f03..6ab997279d 100644 > --- a/common/spl/spl.c > +++ b/common/spl/spl.c > @@ -39,6 +39,7 @@ > #include <fdt_support.h> > #include <bootcount.h> > #include <wdt.h> > +#include <power/regulator.h> > > DECLARE_GLOBAL_DATA_PTR; > DECLARE_BINMAN_MAGIC_SYM; > @@ -773,6 +774,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 > diff --git a/include/power/regulator.h b/include/power/regulator.h > index ff1bfc2435..4bce61dd9f 100644 > --- a/include/power/regulator.h > +++ b/include/power/regulator.h > @@ -631,6 +631,11 @@ static inline int regulators_enable_boot_on(bool verbose) > return -ENOSYS; > } > > +static inline int regulators_enable_boot_off(bool verbose) > +{ > + return -ENOSYS; > +} > + > static inline int regulator_autoset(struct udevice *dev) > { > return -ENOSYS;
Hello, On Thu, Sep 01, 2022 at 08:36:23PM +0800, Kever Yang wrote: > Hi Quentin, > > I don't think we need to add regulators_enable_boot_on/off callback in > SPL framework, > > the rk3399-puma/Qseven is the only board need to do this in the SPL right > now. > > The hardware design for SPI/eMMC should always power on the storage > which > > need by very early which should also used by bootrom. And for more device > power > > will be enabled in U-Boot proper. It's not so clear. In SPL we have support for loading U-Boot from device other than the one from which SPL was loaded. Then it's completely possible for the boot rom to not power on MMC when loading SPL from SPI NOR, and then SPL may need to power on MMC. With inside knowledge of the specific boot rom in question you may know that all boot device are powered on even when not used because the system was loaded from earlier device in the boot sequence. Nonetheless, this is not true for all boot roms in general, and on some platforms loading u-boot from non-boot device may be desirable as well. Thanks Michal > > > Thanks, > > - Kever > > On 2022/7/22 18:09, Quentin Schulz wrote: > > 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> > > Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat> > > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> > > --- > > > > v2: > > - added Tested-by, > > - fixed build for boards with SPL_DM_REGULATOR disabled by always > > included power/regulator.h and defining a dummy implementation for > > regulators_enable_boot_off, > > > > common/spl/spl.c | 10 ++++++++++ > > include/power/regulator.h | 5 +++++ > > 2 files changed, 15 insertions(+) > > > > diff --git a/common/spl/spl.c b/common/spl/spl.c > > index 29e0898f03..6ab997279d 100644 > > --- a/common/spl/spl.c > > +++ b/common/spl/spl.c > > @@ -39,6 +39,7 @@ > > #include <fdt_support.h> > > #include <bootcount.h> > > #include <wdt.h> > > +#include <power/regulator.h> > > DECLARE_GLOBAL_DATA_PTR; > > DECLARE_BINMAN_MAGIC_SYM; > > @@ -773,6 +774,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 > > diff --git a/include/power/regulator.h b/include/power/regulator.h > > index ff1bfc2435..4bce61dd9f 100644 > > --- a/include/power/regulator.h > > +++ b/include/power/regulator.h > > @@ -631,6 +631,11 @@ static inline int regulators_enable_boot_on(bool verbose) > > return -ENOSYS; > > } > > +static inline int regulators_enable_boot_off(bool verbose) > > +{ > > + return -ENOSYS; > > +} > > + > > static inline int regulator_autoset(struct udevice *dev) > > { > > return -ENOSYS;
Hi all, Ping on the patch series. I don't need it for my boards anymore but I still think this is a nice to have for everybody. I won't ping again if there's no interest/feedback. Cheers, Quentin On 7/22/22 12:09, Quentin Schulz wrote: > 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> > Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> > --- > > v2: > - added Tested-by, > - fixed build for boards with SPL_DM_REGULATOR disabled by always > included power/regulator.h and defining a dummy implementation for > regulators_enable_boot_off, > > common/spl/spl.c | 10 ++++++++++ > include/power/regulator.h | 5 +++++ > 2 files changed, 15 insertions(+) > > diff --git a/common/spl/spl.c b/common/spl/spl.c > index 29e0898f03..6ab997279d 100644 > --- a/common/spl/spl.c > +++ b/common/spl/spl.c > @@ -39,6 +39,7 @@ > #include <fdt_support.h> > #include <bootcount.h> > #include <wdt.h> > +#include <power/regulator.h> > > DECLARE_GLOBAL_DATA_PTR; > DECLARE_BINMAN_MAGIC_SYM; > @@ -773,6 +774,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 > diff --git a/include/power/regulator.h b/include/power/regulator.h > index ff1bfc2435..4bce61dd9f 100644 > --- a/include/power/regulator.h > +++ b/include/power/regulator.h > @@ -631,6 +631,11 @@ static inline int regulators_enable_boot_on(bool verbose) > return -ENOSYS; > } > > +static inline int regulators_enable_boot_off(bool verbose) > +{ > + return -ENOSYS; > +} > + > static inline int regulator_autoset(struct udevice *dev) > { > return -ENOSYS;
diff --git a/common/spl/spl.c b/common/spl/spl.c index 29e0898f03..6ab997279d 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -39,6 +39,7 @@ #include <fdt_support.h> #include <bootcount.h> #include <wdt.h> +#include <power/regulator.h> DECLARE_GLOBAL_DATA_PTR; DECLARE_BINMAN_MAGIC_SYM; @@ -773,6 +774,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 diff --git a/include/power/regulator.h b/include/power/regulator.h index ff1bfc2435..4bce61dd9f 100644 --- a/include/power/regulator.h +++ b/include/power/regulator.h @@ -631,6 +631,11 @@ static inline int regulators_enable_boot_on(bool verbose) return -ENOSYS; } +static inline int regulators_enable_boot_off(bool verbose) +{ + return -ENOSYS; +} + static inline int regulator_autoset(struct udevice *dev) { return -ENOSYS;