[PATCH v3 1/4] power: regulator: Trigger probe of regulators which are always-on or boot-on

[PATCH v3 1/4] power: regulator: Trigger probe of regulators which are always-on or boot-on

[Marek Vasut]

In case a regulator DT node contains regulator-always-on or regulator-boot-on
property, make sure the regulator gets correctly configured by U-Boot on start
up. Unconditionally probe such regulator drivers. This is a preparatory patch
for introduction of .regulator_post_probe() which would trigger the regulator
configuration.

Parsing of regulator-always-on and regulator-boot-on DT property has been
moved to regulator_post_bind() as the information is required early, the
rest of the DT parsing has been kept in regulator_pre_probe() to avoid
slowing down the boot process.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
Cc: Caleb Connolly <caleb.connolly@linaro.org>
Cc: Chris Morgan <macromorgan@hotmail.com>
Cc: Dragan Simic <dsimic@manjaro.org>
Cc: Eugen Hristev <eugen.hristev@collabora.com>
Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Kostya Porotchkin <kostap@marvell.com>
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Cc: Max Krummenacher <max.krummenacher@toradex.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
Cc: Quentin Schulz <quentin.schulz@cherry.de>
Cc: Sam Day <me@samcday.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Svyatoslav Ryhel <clamor95@gmail.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: u-boot-amlogic@groups.io
Cc: u-boot-qcom@groups.io
Cc: u-boot@dh-electronics.com
Cc: u-boot@lists.denx.de
Cc: uboot-stm32@st-md-mailman.stormreply.com
---
V2: - Rebase on current u-boot/next
    - Update test cases to handle already started regulators correctly
V3: - Fix conditional around DM_FLAG_PROBE_AFTER_BIND to ORR
---
 drivers/power/regulator/regulator-uclass.c | 22 +++++++++++++++-------
 test/dm/panel.c                            |  2 +-
 test/dm/regulator.c                        |  4 ++--
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 00922900292..1a970004540 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -439,6 +439,8 @@ static int regulator_post_bind(struct udevice *dev)
 	const char *property = "regulator-name";
 
 	uc_pdata = dev_get_uclass_plat(dev);
+	uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on");
+	uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
 
 	/* Regulator's mandatory constraint */
 	uc_pdata->name = dev_read_string(dev, property);
@@ -450,13 +452,21 @@ static int regulator_post_bind(struct udevice *dev)
 			return -EINVAL;
 	}
 
-	if (regulator_name_is_unique(dev, uc_pdata->name))
-		return 0;
+	if (!regulator_name_is_unique(dev, uc_pdata->name)) {
+		debug("'%s' of dev: '%s', has nonunique value: '%s\n",
+		      property, dev->name, uc_pdata->name);
+		return -EINVAL;
+	}
 
-	debug("'%s' of dev: '%s', has nonunique value: '%s\n",
-	      property, dev->name, uc_pdata->name);
+	/*
+	 * In case the regulator has regulator-always-on or
+	 * regulator-boot-on DT property, trigger probe() to
+	 * configure its default state during startup.
+	 */
+	if (uc_pdata->always_on || uc_pdata->boot_on)
+		dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
 
-	return -EINVAL;
+	return 0;
 }
 
 static int regulator_pre_probe(struct udevice *dev)
@@ -479,8 +489,6 @@ static int regulator_pre_probe(struct udevice *dev)
 						-ENODATA);
 	uc_pdata->max_uA = dev_read_u32_default(dev, "regulator-max-microamp",
 						-ENODATA);
-	uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on");
-	uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
 	uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
 						    0);
 	uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
diff --git a/test/dm/panel.c b/test/dm/panel.c
index ce835c96ed0..ec85a9b1e6e 100644
--- a/test/dm/panel.c
+++ b/test/dm/panel.c
@@ -33,7 +33,7 @@ static int dm_test_panel(struct unit_test_state *uts)
 	ut_assertok(sandbox_pwm_get_config(pwm, 0, &period_ns, &duty_ns,
 					   &enable, &polarity));
 	ut_asserteq(false, enable);
-	ut_asserteq(false, regulator_get_enable(reg));
+	ut_asserteq(true, regulator_get_enable(reg));
 
 	ut_assertok(panel_enable_backlight(dev));
 	ut_assertok(sandbox_pwm_get_config(pwm, 0, &period_ns, &duty_ns,
diff --git a/test/dm/regulator.c b/test/dm/regulator.c
index 532bbd82376..449748ad52f 100644
--- a/test/dm/regulator.c
+++ b/test/dm/regulator.c
@@ -186,7 +186,7 @@ int dm_test_power_regulator_set_enable_if_allowed(struct unit_test_state *uts)
 
 	/* Get BUCK1 - always on regulator */
 	platname = regulator_names[BUCK1][PLATNAME];
-	ut_assertok(regulator_autoset_by_name(platname, &dev_autoset));
+	ut_asserteq(-EALREADY, regulator_autoset_by_name(platname, &dev_autoset));
 	ut_assertok(regulator_get_by_platname(platname, &dev));
 
 	/* Try disabling always-on regulator */
@@ -288,7 +288,7 @@ static int dm_test_power_regulator_autoset(struct unit_test_state *uts)
 	 * Expected output state: uV=1200000; uA=200000; output enabled
 	 */
 	platname = regulator_names[BUCK1][PLATNAME];
-	ut_assertok(regulator_autoset_by_name(platname, &dev_autoset));
+	ut_asserteq(-EALREADY, regulator_autoset_by_name(platname, &dev_autoset));
 
 	/* Check, that the returned device is proper */
 	ut_assertok(regulator_get_by_platname(platname, &dev));
-- 
2.45.2

[PATCH v3 2/4] power: regulator: Convert regulators_enable_boot_on/off() to regulator_post_probe

[Marek Vasut]

Turn regulators_enable_boot_on() and regulators_enable_boot_off() into
empty functions. Implement matching functionality in regulator_post_probe()
instead. The regulator_post_probe() is called for all regulators after they
probe, and regulators that have regulator-always-on or regulator-boot-on DT
properties now always probe due to DM_FLAG_PROBE_AFTER_BIND being set on
such regulators in regulator_post_bind().

Finally, fold regulator_unset() functionality into regulator_autoset().

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
Cc: Caleb Connolly <caleb.connolly@linaro.org>
Cc: Chris Morgan <macromorgan@hotmail.com>
Cc: Dragan Simic <dsimic@manjaro.org>
Cc: Eugen Hristev <eugen.hristev@collabora.com>
Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Cc: Max Krummenacher <max.krummenacher@toradex.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
Cc: Quentin Schulz <quentin.schulz@cherry.de>
Cc: Sam Day <me@samcday.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Svyatoslav Ryhel <clamor95@gmail.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: u-boot-amlogic@groups.io
Cc: u-boot-qcom@groups.io
Cc: u-boot@dh-electronics.com
Cc: u-boot@lists.denx.de
Cc: uboot-stm32@st-md-mailman.stormreply.com
---
V2: Rebase on current u-boot/next
V3: No change
---
 drivers/power/regulator/regulator-uclass.c | 60 +++++++---------------
 1 file changed, 19 insertions(+), 41 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 1a970004540..9fcc4bd85b9 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -314,6 +314,11 @@ int regulator_autoset(struct udevice *dev)
 			return ret;
 	}
 
+	if (uc_pdata->force_off) {
+		ret = regulator_set_enable(dev, false);
+		goto out;
+	}
+
 	if (!uc_pdata->always_on && !uc_pdata->boot_on) {
 		ret = -EMEDIUMTYPE;
 		goto out;
@@ -518,56 +523,28 @@ static int regulator_pre_probe(struct udevice *dev)
 	return 0;
 }
 
-int regulators_enable_boot_on(bool verbose)
+static int regulator_post_probe(struct udevice *dev)
 {
-	struct udevice *dev;
-	struct uclass *uc;
 	int ret;
 
-	ret = uclass_get(UCLASS_REGULATOR, &uc);
-	if (ret)
+	ret = regulator_autoset(dev);
+	if (ret && ret != -EMEDIUMTYPE && ret != -EALREADY && ret != ENOSYS)
 		return ret;
-	for (uclass_first_device(UCLASS_REGULATOR, &dev);
-	     dev;
-	     uclass_next_device(&dev)) {
-		ret = regulator_autoset(dev);
-		if (ret == -EMEDIUMTYPE || ret == -EALREADY) {
-			ret = 0;
-			continue;
-		}
-		if (verbose)
-			regulator_show(dev, ret);
-		if (ret == -ENOSYS)
-			ret = 0;
-	}
 
-	return ret;
+	if (_DEBUG)
+		regulator_show(dev, ret);
+
+	return 0;
 }
 
-int regulators_enable_boot_off(bool verbose)
+int regulators_enable_boot_on(bool verbose)
 {
-	struct udevice *dev;
-	struct uclass *uc;
-	int ret;
-
-	ret = uclass_get(UCLASS_REGULATOR, &uc);
-	if (ret)
-		return ret;
-	for (uclass_first_device(UCLASS_REGULATOR, &dev);
-	     dev;
-	     uclass_next_device(&dev)) {
-		ret = regulator_unset(dev);
-		if (ret == -EMEDIUMTYPE) {
-			ret = 0;
-			continue;
-		}
-		if (verbose)
-			regulator_show(dev, ret);
-		if (ret == -ENOSYS)
-			ret = 0;
-	}
+	return 0;
+}
 
-	return ret;
+int regulators_enable_boot_off(bool verbose)
+{
+	return 0;
 }
 
 UCLASS_DRIVER(regulator) = {
@@ -575,5 +552,6 @@ UCLASS_DRIVER(regulator) = {
 	.name		= "regulator",
 	.post_bind	= regulator_post_bind,
 	.pre_probe	= regulator_pre_probe,
+	.post_probe	= regulator_post_probe,
 	.per_device_plat_auto	= sizeof(struct dm_regulator_uclass_plat),
 };
-- 
2.45.2

Re: [PATCH v3 2/4] power: regulator: Convert regulators_enable_boot_on/off() to regulator_post_probe

[Simon Glass]

Hi,

On Thu, 26 Sept 2024 at 17:15, Marek Vasut <marex@denx.de> wrote:
>
> Turn regulators_enable_boot_on() and regulators_enable_boot_off() into
> empty functions. Implement matching functionality in regulator_post_probe()
> instead. The regulator_post_probe() is called for all regulators after they
> probe, and regulators that have regulator-always-on or regulator-boot-on DT
> properties now always probe due to DM_FLAG_PROBE_AFTER_BIND being set on
> such regulators in regulator_post_bind().
>
> Finally, fold regulator_unset() functionality into regulator_autoset().
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
> Cc: Caleb Connolly <caleb.connolly@linaro.org>
> Cc: Chris Morgan <macromorgan@hotmail.com>
> Cc: Dragan Simic <dsimic@manjaro.org>
> Cc: Eugen Hristev <eugen.hristev@collabora.com>
> Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Matteo Lisi <matteo.lisi@engicam.com>
> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Cc: Max Krummenacher <max.krummenacher@toradex.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Patrice Chotard <patrice.chotard@foss.st.com>
> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
> Cc: Quentin Schulz <quentin.schulz@cherry.de>
> Cc: Sam Day <me@samcday.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: Svyatoslav Ryhel <clamor95@gmail.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> Cc: u-boot-amlogic@groups.io
> Cc: u-boot-qcom@groups.io
> Cc: u-boot@dh-electronics.com
> Cc: u-boot@lists.denx.de
> Cc: uboot-stm32@st-md-mailman.stormreply.com
> ---
> V2: Rebase on current u-boot/next
> V3: No change
> ---
>  drivers/power/regulator/regulator-uclass.c | 60 +++++++---------------
>  1 file changed, 19 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 1a970004540..9fcc4bd85b9 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -314,6 +314,11 @@ int regulator_autoset(struct udevice *dev)
>                         return ret;
>         }
>
> +       if (uc_pdata->force_off) {
> +               ret = regulator_set_enable(dev, false);
> +               goto out;
> +       }
> +
>         if (!uc_pdata->always_on && !uc_pdata->boot_on) {
>                 ret = -EMEDIUMTYPE;
>                 goto out;
> @@ -518,56 +523,28 @@ static int regulator_pre_probe(struct udevice *dev)
>         return 0;
>  }
>
> -int regulators_enable_boot_on(bool verbose)
> +static int regulator_post_probe(struct udevice *dev)
>  {
> -       struct udevice *dev;
> -       struct uclass *uc;
>         int ret;
>
> -       ret = uclass_get(UCLASS_REGULATOR, &uc);
> -       if (ret)
> +       ret = regulator_autoset(dev);
> +       if (ret && ret != -EMEDIUMTYPE && ret != -EALREADY && ret != ENOSYS)
>                 return ret;
> -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
> -            dev;
> -            uclass_next_device(&dev)) {
> -               ret = regulator_autoset(dev);
> -               if (ret == -EMEDIUMTYPE || ret == -EALREADY) {
> -                       ret = 0;
> -                       continue;
> -               }
> -               if (verbose)
> -                       regulator_show(dev, ret);
> -               if (ret == -ENOSYS)
> -                       ret = 0;
> -       }
>
> -       return ret;
> +       if (_DEBUG)
> +               regulator_show(dev, ret);
> +
> +       return 0;
>  }
>
> -int regulators_enable_boot_off(bool verbose)
> +int regulators_enable_boot_on(bool verbose)
>  {
> -       struct udevice *dev;
> -       struct uclass *uc;
> -       int ret;
> -
> -       ret = uclass_get(UCLASS_REGULATOR, &uc);
> -       if (ret)
> -               return ret;
> -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
> -            dev;
> -            uclass_next_device(&dev)) {
> -               ret = regulator_unset(dev);
> -               if (ret == -EMEDIUMTYPE) {
> -                       ret = 0;
> -                       continue;
> -               }
> -               if (verbose)
> -                       regulator_show(dev, ret);
> -               if (ret == -ENOSYS)
> -                       ret = 0;
> -       }
> +       return 0;
> +}
>
> -       return ret;
> +int regulators_enable_boot_off(bool verbose)
> +{
> +       return 0;
>  }
>
>  UCLASS_DRIVER(regulator) = {
> @@ -575,5 +552,6 @@ UCLASS_DRIVER(regulator) = {
>         .name           = "regulator",
>         .post_bind      = regulator_post_bind,
>         .pre_probe      = regulator_pre_probe,
> +       .post_probe     = regulator_post_probe,
>         .per_device_plat_auto   = sizeof(struct dm_regulator_uclass_plat),
>  };
> --
> 2.45.2
>

I thought I objected to this patch, but it seems to be in -next? Does
anyone know what has happened here?

I am seeing these errors now when running sandbox 'u-boot -D':

       i2c_emul_find() No emulators for device 'sandbox_pmic'
  sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
       out_set_value() PMIC write failed: -5
       i2c_emul_find() No emulators for device 'sandbox_pmic'
  sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
       out_set_value() PMIC write failed: -5

Regards,
Simon

Re: [PATCH v3 2/4] power: regulator: Convert regulators_enable_boot_on/off() to regulator_post_probe

[Tom Rini]

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

On Wed, Oct 02, 2024 at 04:55:39PM -0600, Simon Glass wrote:
> Hi,
> 
> On Thu, 26 Sept 2024 at 17:15, Marek Vasut <marex@denx.de> wrote:
> >
> > Turn regulators_enable_boot_on() and regulators_enable_boot_off() into
> > empty functions. Implement matching functionality in regulator_post_probe()
> > instead. The regulator_post_probe() is called for all regulators after they
> > probe, and regulators that have regulator-always-on or regulator-boot-on DT
> > properties now always probe due to DM_FLAG_PROBE_AFTER_BIND being set on
> > such regulators in regulator_post_bind().
> >
> > Finally, fold regulator_unset() functionality into regulator_autoset().
> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > ---
> > Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
> > Cc: Caleb Connolly <caleb.connolly@linaro.org>
> > Cc: Chris Morgan <macromorgan@hotmail.com>
> > Cc: Dragan Simic <dsimic@manjaro.org>
> > Cc: Eugen Hristev <eugen.hristev@collabora.com>
> > Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Jaehoon Chung <jh80.chung@samsung.com>
> > Cc: Jagan Teki <jagan@amarulasolutions.com>
> > Cc: Jonas Karlman <jonas@kwiboo.se>
> > Cc: Kever Yang <kever.yang@rock-chips.com>
> > Cc: Matteo Lisi <matteo.lisi@engicam.com>
> > Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> > Cc: Max Krummenacher <max.krummenacher@toradex.com>
> > Cc: Neil Armstrong <neil.armstrong@linaro.org>
> > Cc: Patrice Chotard <patrice.chotard@foss.st.com>
> > Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> > Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > Cc: Quentin Schulz <quentin.schulz@cherry.de>
> > Cc: Sam Day <me@samcday.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Sumit Garg <sumit.garg@linaro.org>
> > Cc: Svyatoslav Ryhel <clamor95@gmail.com>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> > Cc: u-boot-amlogic@groups.io
> > Cc: u-boot-qcom@groups.io
> > Cc: u-boot@dh-electronics.com
> > Cc: u-boot@lists.denx.de
> > Cc: uboot-stm32@st-md-mailman.stormreply.com
> > ---
> > V2: Rebase on current u-boot/next
> > V3: No change
> > ---
> >  drivers/power/regulator/regulator-uclass.c | 60 +++++++---------------
> >  1 file changed, 19 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> > index 1a970004540..9fcc4bd85b9 100644
> > --- a/drivers/power/regulator/regulator-uclass.c
> > +++ b/drivers/power/regulator/regulator-uclass.c
> > @@ -314,6 +314,11 @@ int regulator_autoset(struct udevice *dev)
> >                         return ret;
> >         }
> >
> > +       if (uc_pdata->force_off) {
> > +               ret = regulator_set_enable(dev, false);
> > +               goto out;
> > +       }
> > +
> >         if (!uc_pdata->always_on && !uc_pdata->boot_on) {
> >                 ret = -EMEDIUMTYPE;
> >                 goto out;
> > @@ -518,56 +523,28 @@ static int regulator_pre_probe(struct udevice *dev)
> >         return 0;
> >  }
> >
> > -int regulators_enable_boot_on(bool verbose)
> > +static int regulator_post_probe(struct udevice *dev)
> >  {
> > -       struct udevice *dev;
> > -       struct uclass *uc;
> >         int ret;
> >
> > -       ret = uclass_get(UCLASS_REGULATOR, &uc);
> > -       if (ret)
> > +       ret = regulator_autoset(dev);
> > +       if (ret && ret != -EMEDIUMTYPE && ret != -EALREADY && ret != ENOSYS)
> >                 return ret;
> > -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
> > -            dev;
> > -            uclass_next_device(&dev)) {
> > -               ret = regulator_autoset(dev);
> > -               if (ret == -EMEDIUMTYPE || ret == -EALREADY) {
> > -                       ret = 0;
> > -                       continue;
> > -               }
> > -               if (verbose)
> > -                       regulator_show(dev, ret);
> > -               if (ret == -ENOSYS)
> > -                       ret = 0;
> > -       }
> >
> > -       return ret;
> > +       if (_DEBUG)
> > +               regulator_show(dev, ret);
> > +
> > +       return 0;
> >  }
> >
> > -int regulators_enable_boot_off(bool verbose)
> > +int regulators_enable_boot_on(bool verbose)
> >  {
> > -       struct udevice *dev;
> > -       struct uclass *uc;
> > -       int ret;
> > -
> > -       ret = uclass_get(UCLASS_REGULATOR, &uc);
> > -       if (ret)
> > -               return ret;
> > -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
> > -            dev;
> > -            uclass_next_device(&dev)) {
> > -               ret = regulator_unset(dev);
> > -               if (ret == -EMEDIUMTYPE) {
> > -                       ret = 0;
> > -                       continue;
> > -               }
> > -               if (verbose)
> > -                       regulator_show(dev, ret);
> > -               if (ret == -ENOSYS)
> > -                       ret = 0;
> > -       }
> > +       return 0;
> > +}
> >
> > -       return ret;
> > +int regulators_enable_boot_off(bool verbose)
> > +{
> > +       return 0;
> >  }
> >
> >  UCLASS_DRIVER(regulator) = {
> > @@ -575,5 +552,6 @@ UCLASS_DRIVER(regulator) = {
> >         .name           = "regulator",
> >         .post_bind      = regulator_post_bind,
> >         .pre_probe      = regulator_pre_probe,
> > +       .post_probe     = regulator_post_probe,
> >         .per_device_plat_auto   = sizeof(struct dm_regulator_uclass_plat),
> >  };
> > --
> > 2.45.2
> >
> 
> I thought I objected to this patch, but it seems to be in -next? Does
> anyone know what has happened here?

Yes, you missed me noting that this fixes real problems and has been
needing to be fixed for a while. I said you should provide your
alternative approach and we'll get this sorted out in -next and I took
the work-around for master so the release wouldn't be broken.

> I am seeing these errors now when running sandbox 'u-boot -D':
> 
>        i2c_emul_find() No emulators for device 'sandbox_pmic'
>   sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
>        out_set_value() PMIC write failed: -5
>        i2c_emul_find() No emulators for device 'sandbox_pmic'
>   sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
>        out_set_value() PMIC write failed: -5

I suppose the good news is that perhaps this is also related to the
problem that Svyatoslav was reporting.

-- 
Tom

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

Re: [PATCH v3 2/4] power: regulator: Convert regulators_enable_boot_on/off() to regulator_post_probe

[Simon Glass]

Hi Tom,

On Wed, 2 Oct 2024 at 19:40, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Oct 02, 2024 at 04:55:39PM -0600, Simon Glass wrote:
> > Hi,
> >
> > On Thu, 26 Sept 2024 at 17:15, Marek Vasut <marex@denx.de> wrote:
> > >
> > > Turn regulators_enable_boot_on() and regulators_enable_boot_off() into
> > > empty functions. Implement matching functionality in regulator_post_probe()
> > > instead. The regulator_post_probe() is called for all regulators after they
> > > probe, and regulators that have regulator-always-on or regulator-boot-on DT
> > > properties now always probe due to DM_FLAG_PROBE_AFTER_BIND being set on
> > > such regulators in regulator_post_bind().
> > >
> > > Finally, fold regulator_unset() functionality into regulator_autoset().
> > >
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > ---
> > > Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
> > > Cc: Caleb Connolly <caleb.connolly@linaro.org>
> > > Cc: Chris Morgan <macromorgan@hotmail.com>
> > > Cc: Dragan Simic <dsimic@manjaro.org>
> > > Cc: Eugen Hristev <eugen.hristev@collabora.com>
> > > Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > Cc: Jaehoon Chung <jh80.chung@samsung.com>
> > > Cc: Jagan Teki <jagan@amarulasolutions.com>
> > > Cc: Jonas Karlman <jonas@kwiboo.se>
> > > Cc: Kever Yang <kever.yang@rock-chips.com>
> > > Cc: Matteo Lisi <matteo.lisi@engicam.com>
> > > Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> > > Cc: Max Krummenacher <max.krummenacher@toradex.com>
> > > Cc: Neil Armstrong <neil.armstrong@linaro.org>
> > > Cc: Patrice Chotard <patrice.chotard@foss.st.com>
> > > Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> > > Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > > Cc: Quentin Schulz <quentin.schulz@cherry.de>
> > > Cc: Sam Day <me@samcday.com>
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > Cc: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Cc: Thierry Reding <treding@nvidia.com>
> > > Cc: Tom Rini <trini@konsulko.com>
> > > Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> > > Cc: u-boot-amlogic@groups.io
> > > Cc: u-boot-qcom@groups.io
> > > Cc: u-boot@dh-electronics.com
> > > Cc: u-boot@lists.denx.de
> > > Cc: uboot-stm32@st-md-mailman.stormreply.com
> > > ---
> > > V2: Rebase on current u-boot/next
> > > V3: No change
> > > ---
> > >  drivers/power/regulator/regulator-uclass.c | 60 +++++++---------------
> > >  1 file changed, 19 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> > > index 1a970004540..9fcc4bd85b9 100644
> > > --- a/drivers/power/regulator/regulator-uclass.c
> > > +++ b/drivers/power/regulator/regulator-uclass.c
> > > @@ -314,6 +314,11 @@ int regulator_autoset(struct udevice *dev)
> > >                         return ret;
> > >         }
> > >
> > > +       if (uc_pdata->force_off) {
> > > +               ret = regulator_set_enable(dev, false);
> > > +               goto out;
> > > +       }
> > > +
> > >         if (!uc_pdata->always_on && !uc_pdata->boot_on) {
> > >                 ret = -EMEDIUMTYPE;
> > >                 goto out;
> > > @@ -518,56 +523,28 @@ static int regulator_pre_probe(struct udevice *dev)
> > >         return 0;
> > >  }
> > >
> > > -int regulators_enable_boot_on(bool verbose)
> > > +static int regulator_post_probe(struct udevice *dev)
> > >  {
> > > -       struct udevice *dev;
> > > -       struct uclass *uc;
> > >         int ret;
> > >
> > > -       ret = uclass_get(UCLASS_REGULATOR, &uc);
> > > -       if (ret)
> > > +       ret = regulator_autoset(dev);
> > > +       if (ret && ret != -EMEDIUMTYPE && ret != -EALREADY && ret != ENOSYS)
> > >                 return ret;
> > > -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
> > > -            dev;
> > > -            uclass_next_device(&dev)) {
> > > -               ret = regulator_autoset(dev);
> > > -               if (ret == -EMEDIUMTYPE || ret == -EALREADY) {
> > > -                       ret = 0;
> > > -                       continue;
> > > -               }
> > > -               if (verbose)
> > > -                       regulator_show(dev, ret);
> > > -               if (ret == -ENOSYS)
> > > -                       ret = 0;
> > > -       }
> > >
> > > -       return ret;
> > > +       if (_DEBUG)
> > > +               regulator_show(dev, ret);
> > > +
> > > +       return 0;
> > >  }
> > >
> > > -int regulators_enable_boot_off(bool verbose)
> > > +int regulators_enable_boot_on(bool verbose)
> > >  {
> > > -       struct udevice *dev;
> > > -       struct uclass *uc;
> > > -       int ret;
> > > -
> > > -       ret = uclass_get(UCLASS_REGULATOR, &uc);
> > > -       if (ret)
> > > -               return ret;
> > > -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
> > > -            dev;
> > > -            uclass_next_device(&dev)) {
> > > -               ret = regulator_unset(dev);
> > > -               if (ret == -EMEDIUMTYPE) {
> > > -                       ret = 0;
> > > -                       continue;
> > > -               }
> > > -               if (verbose)
> > > -                       regulator_show(dev, ret);
> > > -               if (ret == -ENOSYS)
> > > -                       ret = 0;
> > > -       }
> > > +       return 0;
> > > +}
> > >
> > > -       return ret;
> > > +int regulators_enable_boot_off(bool verbose)
> > > +{
> > > +       return 0;
> > >  }
> > >
> > >  UCLASS_DRIVER(regulator) = {
> > > @@ -575,5 +552,6 @@ UCLASS_DRIVER(regulator) = {
> > >         .name           = "regulator",
> > >         .post_bind      = regulator_post_bind,
> > >         .pre_probe      = regulator_pre_probe,
> > > +       .post_probe     = regulator_post_probe,
> > >         .per_device_plat_auto   = sizeof(struct dm_regulator_uclass_plat),
> > >  };
> > > --
> > > 2.45.2
> > >
> >
> > I thought I objected to this patch, but it seems to be in -next? Does
> > anyone know what has happened here?
>
> Yes, you missed me noting that this fixes real problems and has been
> needing to be fixed for a while. I said you should provide your
> alternative approach and we'll get this sorted out in -next and I took
> the work-around for master so the release wouldn't be broken.

OK ta. So long as we can revert/change that is fine.

>
> > I am seeing these errors now when running sandbox 'u-boot -D':
> >
> >        i2c_emul_find() No emulators for device 'sandbox_pmic'
> >   sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
> >        out_set_value() PMIC write failed: -5
> >        i2c_emul_find() No emulators for device 'sandbox_pmic'
> >   sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
> >        out_set_value() PMIC write failed: -5
>
> I suppose the good news is that perhaps this is also related to the
> problem that Svyatoslav was reporting.

OK, I actually never fully understood the problem, but yes, sandbox is
supposed to work similarly.

Regards,
Simon

Re: [PATCH v3 2/4] power: regulator: Convert regulators_enable_boot_on/off() to regulator_post_probe

[Svyatoslav Ryhel]

чт, 3 жовт. 2024 р. о 18:03 Simon Glass <sjg@chromium.org> пише:
>
> Hi Tom,
>
> On Wed, 2 Oct 2024 at 19:40, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Oct 02, 2024 at 04:55:39PM -0600, Simon Glass wrote:
> > > Hi,
> > >
> > > On Thu, 26 Sept 2024 at 17:15, Marek Vasut <marex@denx.de> wrote:
> > > >
> > > > Turn regulators_enable_boot_on() and regulators_enable_boot_off() into
> > > > empty functions. Implement matching functionality in regulator_post_probe()
> > > > instead. The regulator_post_probe() is called for all regulators after they
> > > > probe, and regulators that have regulator-always-on or regulator-boot-on DT
> > > > properties now always probe due to DM_FLAG_PROBE_AFTER_BIND being set on
> > > > such regulators in regulator_post_bind().
> > > >
> > > > Finally, fold regulator_unset() functionality into regulator_autoset().
> > > >
> > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > ---
> > > > Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
> > > > Cc: Caleb Connolly <caleb.connolly@linaro.org>
> > > > Cc: Chris Morgan <macromorgan@hotmail.com>
> > > > Cc: Dragan Simic <dsimic@manjaro.org>
> > > > Cc: Eugen Hristev <eugen.hristev@collabora.com>
> > > > Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > Cc: Jaehoon Chung <jh80.chung@samsung.com>
> > > > Cc: Jagan Teki <jagan@amarulasolutions.com>
> > > > Cc: Jonas Karlman <jonas@kwiboo.se>
> > > > Cc: Kever Yang <kever.yang@rock-chips.com>
> > > > Cc: Matteo Lisi <matteo.lisi@engicam.com>
> > > > Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> > > > Cc: Max Krummenacher <max.krummenacher@toradex.com>
> > > > Cc: Neil Armstrong <neil.armstrong@linaro.org>
> > > > Cc: Patrice Chotard <patrice.chotard@foss.st.com>
> > > > Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> > > > Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > > > Cc: Quentin Schulz <quentin.schulz@cherry.de>
> > > > Cc: Sam Day <me@samcday.com>
> > > > Cc: Simon Glass <sjg@chromium.org>
> > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > Cc: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > Cc: Thierry Reding <treding@nvidia.com>
> > > > Cc: Tom Rini <trini@konsulko.com>
> > > > Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> > > > Cc: u-boot-amlogic@groups.io
> > > > Cc: u-boot-qcom@groups.io
> > > > Cc: u-boot@dh-electronics.com
> > > > Cc: u-boot@lists.denx.de
> > > > Cc: uboot-stm32@st-md-mailman.stormreply.com
> > > > ---
> > > > V2: Rebase on current u-boot/next
> > > > V3: No change
> > > > ---
> > > >  drivers/power/regulator/regulator-uclass.c | 60 +++++++---------------
> > > >  1 file changed, 19 insertions(+), 41 deletions(-)
> > > >
> > > > diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> > > > index 1a970004540..9fcc4bd85b9 100644
> > > > --- a/drivers/power/regulator/regulator-uclass.c
> > > > +++ b/drivers/power/regulator/regulator-uclass.c
> > > > @@ -314,6 +314,11 @@ int regulator_autoset(struct udevice *dev)
> > > >                         return ret;
> > > >         }
> > > >
> > > > +       if (uc_pdata->force_off) {
> > > > +               ret = regulator_set_enable(dev, false);
> > > > +               goto out;
> > > > +       }
> > > > +
> > > >         if (!uc_pdata->always_on && !uc_pdata->boot_on) {
> > > >                 ret = -EMEDIUMTYPE;
> > > >                 goto out;
> > > > @@ -518,56 +523,28 @@ static int regulator_pre_probe(struct udevice *dev)
> > > >         return 0;
> > > >  }
> > > >
> > > > -int regulators_enable_boot_on(bool verbose)
> > > > +static int regulator_post_probe(struct udevice *dev)
> > > >  {
> > > > -       struct udevice *dev;
> > > > -       struct uclass *uc;
> > > >         int ret;
> > > >
> > > > -       ret = uclass_get(UCLASS_REGULATOR, &uc);
> > > > -       if (ret)
> > > > +       ret = regulator_autoset(dev);
> > > > +       if (ret && ret != -EMEDIUMTYPE && ret != -EALREADY && ret != ENOSYS)
> > > >                 return ret;
> > > > -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
> > > > -            dev;
> > > > -            uclass_next_device(&dev)) {
> > > > -               ret = regulator_autoset(dev);
> > > > -               if (ret == -EMEDIUMTYPE || ret == -EALREADY) {
> > > > -                       ret = 0;
> > > > -                       continue;
> > > > -               }
> > > > -               if (verbose)
> > > > -                       regulator_show(dev, ret);
> > > > -               if (ret == -ENOSYS)
> > > > -                       ret = 0;
> > > > -       }
> > > >
> > > > -       return ret;
> > > > +       if (_DEBUG)
> > > > +               regulator_show(dev, ret);
> > > > +
> > > > +       return 0;
> > > >  }
> > > >
> > > > -int regulators_enable_boot_off(bool verbose)
> > > > +int regulators_enable_boot_on(bool verbose)
> > > >  {
> > > > -       struct udevice *dev;
> > > > -       struct uclass *uc;
> > > > -       int ret;
> > > > -
> > > > -       ret = uclass_get(UCLASS_REGULATOR, &uc);
> > > > -       if (ret)
> > > > -               return ret;
> > > > -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
> > > > -            dev;
> > > > -            uclass_next_device(&dev)) {
> > > > -               ret = regulator_unset(dev);
> > > > -               if (ret == -EMEDIUMTYPE) {
> > > > -                       ret = 0;
> > > > -                       continue;
> > > > -               }
> > > > -               if (verbose)
> > > > -                       regulator_show(dev, ret);
> > > > -               if (ret == -ENOSYS)
> > > > -                       ret = 0;
> > > > -       }
> > > > +       return 0;
> > > > +}
> > > >
> > > > -       return ret;
> > > > +int regulators_enable_boot_off(bool verbose)
> > > > +{
> > > > +       return 0;
> > > >  }
> > > >
> > > >  UCLASS_DRIVER(regulator) = {
> > > > @@ -575,5 +552,6 @@ UCLASS_DRIVER(regulator) = {
> > > >         .name           = "regulator",
> > > >         .post_bind      = regulator_post_bind,
> > > >         .pre_probe      = regulator_pre_probe,
> > > > +       .post_probe     = regulator_post_probe,
> > > >         .per_device_plat_auto   = sizeof(struct dm_regulator_uclass_plat),
> > > >  };
> > > > --
> > > > 2.45.2
> > > >
> > >
> > > I thought I objected to this patch, but it seems to be in -next? Does
> > > anyone know what has happened here?
> >
> > Yes, you missed me noting that this fixes real problems and has been
> > needing to be fixed for a while. I said you should provide your
> > alternative approach and we'll get this sorted out in -next and I took
> > the work-around for master so the release wouldn't be broken.
>
> OK ta. So long as we can revert/change that is fine.
>
> >
> > > I am seeing these errors now when running sandbox 'u-boot -D':
> > >
> > >        i2c_emul_find() No emulators for device 'sandbox_pmic'
> > >   sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
> > >        out_set_value() PMIC write failed: -5
> > >        i2c_emul_find() No emulators for device 'sandbox_pmic'
> > >   sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
> > >        out_set_value() PMIC write failed: -5
> >
> > I suppose the good news is that perhaps this is also related to the
> > problem that Svyatoslav was reporting.
>
> OK, I actually never fully understood the problem, but yes, sandbox is
> supposed to work similarly.
>
> Regards,
> Simon

Simon,

Please inform me when you come up with a solution, I would like to
test it. Thanks.

Re: [PATCH v3 2/4] power: regulator: Convert regulators_enable_boot_on/off() to regulator_post_probe

[Marek Vasut]

On 10/3/24 12:55 AM, Simon Glass wrote:
> Hi,
> 
> On Thu, 26 Sept 2024 at 17:15, Marek Vasut <marex@denx.de> wrote:
>>
>> Turn regulators_enable_boot_on() and regulators_enable_boot_off() into
>> empty functions. Implement matching functionality in regulator_post_probe()
>> instead. The regulator_post_probe() is called for all regulators after they
>> probe, and regulators that have regulator-always-on or regulator-boot-on DT
>> properties now always probe due to DM_FLAG_PROBE_AFTER_BIND being set on
>> such regulators in regulator_post_bind().
>>
>> Finally, fold regulator_unset() functionality into regulator_autoset().
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
>> Cc: Caleb Connolly <caleb.connolly@linaro.org>
>> Cc: Chris Morgan <macromorgan@hotmail.com>
>> Cc: Dragan Simic <dsimic@manjaro.org>
>> Cc: Eugen Hristev <eugen.hristev@collabora.com>
>> Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Jagan Teki <jagan@amarulasolutions.com>
>> Cc: Jonas Karlman <jonas@kwiboo.se>
>> Cc: Kever Yang <kever.yang@rock-chips.com>
>> Cc: Matteo Lisi <matteo.lisi@engicam.com>
>> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> Cc: Max Krummenacher <max.krummenacher@toradex.com>
>> Cc: Neil Armstrong <neil.armstrong@linaro.org>
>> Cc: Patrice Chotard <patrice.chotard@foss.st.com>
>> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
>> Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
>> Cc: Quentin Schulz <quentin.schulz@cherry.de>
>> Cc: Sam Day <me@samcday.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Sumit Garg <sumit.garg@linaro.org>
>> Cc: Svyatoslav Ryhel <clamor95@gmail.com>
>> Cc: Thierry Reding <treding@nvidia.com>
>> Cc: Tom Rini <trini@konsulko.com>
>> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
>> Cc: u-boot-amlogic@groups.io
>> Cc: u-boot-qcom@groups.io
>> Cc: u-boot@dh-electronics.com
>> Cc: u-boot@lists.denx.de
>> Cc: uboot-stm32@st-md-mailman.stormreply.com
>> ---
>> V2: Rebase on current u-boot/next
>> V3: No change
>> ---
>>   drivers/power/regulator/regulator-uclass.c | 60 +++++++---------------
>>   1 file changed, 19 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>> index 1a970004540..9fcc4bd85b9 100644
>> --- a/drivers/power/regulator/regulator-uclass.c
>> +++ b/drivers/power/regulator/regulator-uclass.c
>> @@ -314,6 +314,11 @@ int regulator_autoset(struct udevice *dev)
>>                          return ret;
>>          }
>>
>> +       if (uc_pdata->force_off) {
>> +               ret = regulator_set_enable(dev, false);
>> +               goto out;
>> +       }
>> +
>>          if (!uc_pdata->always_on && !uc_pdata->boot_on) {
>>                  ret = -EMEDIUMTYPE;
>>                  goto out;
>> @@ -518,56 +523,28 @@ static int regulator_pre_probe(struct udevice *dev)
>>          return 0;
>>   }
>>
>> -int regulators_enable_boot_on(bool verbose)
>> +static int regulator_post_probe(struct udevice *dev)
>>   {
>> -       struct udevice *dev;
>> -       struct uclass *uc;
>>          int ret;
>>
>> -       ret = uclass_get(UCLASS_REGULATOR, &uc);
>> -       if (ret)
>> +       ret = regulator_autoset(dev);
>> +       if (ret && ret != -EMEDIUMTYPE && ret != -EALREADY && ret != ENOSYS)
>>                  return ret;
>> -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
>> -            dev;
>> -            uclass_next_device(&dev)) {
>> -               ret = regulator_autoset(dev);
>> -               if (ret == -EMEDIUMTYPE || ret == -EALREADY) {
>> -                       ret = 0;
>> -                       continue;
>> -               }
>> -               if (verbose)
>> -                       regulator_show(dev, ret);
>> -               if (ret == -ENOSYS)
>> -                       ret = 0;
>> -       }
>>
>> -       return ret;
>> +       if (_DEBUG)
>> +               regulator_show(dev, ret);
>> +
>> +       return 0;
>>   }
>>
>> -int regulators_enable_boot_off(bool verbose)
>> +int regulators_enable_boot_on(bool verbose)
>>   {
>> -       struct udevice *dev;
>> -       struct uclass *uc;
>> -       int ret;
>> -
>> -       ret = uclass_get(UCLASS_REGULATOR, &uc);
>> -       if (ret)
>> -               return ret;
>> -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
>> -            dev;
>> -            uclass_next_device(&dev)) {
>> -               ret = regulator_unset(dev);
>> -               if (ret == -EMEDIUMTYPE) {
>> -                       ret = 0;
>> -                       continue;
>> -               }
>> -               if (verbose)
>> -                       regulator_show(dev, ret);
>> -               if (ret == -ENOSYS)
>> -                       ret = 0;
>> -       }
>> +       return 0;
>> +}
>>
>> -       return ret;
>> +int regulators_enable_boot_off(bool verbose)
>> +{
>> +       return 0;
>>   }
>>
>>   UCLASS_DRIVER(regulator) = {
>> @@ -575,5 +552,6 @@ UCLASS_DRIVER(regulator) = {
>>          .name           = "regulator",
>>          .post_bind      = regulator_post_bind,
>>          .pre_probe      = regulator_pre_probe,
>> +       .post_probe     = regulator_post_probe,
>>          .per_device_plat_auto   = sizeof(struct dm_regulator_uclass_plat),
>>   };
>> --
>> 2.45.2
>>
> 
> I thought I objected to this patch, but it seems to be in -next? Does
> anyone know what has happened here?
> 
> I am seeing these errors now when running sandbox 'u-boot -D':
> 
>         i2c_emul_find() No emulators for device 'sandbox_pmic'
>    sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
>         out_set_value() PMIC write failed: -5
>         i2c_emul_find() No emulators for device 'sandbox_pmic'
>    sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
>         out_set_value() PMIC write failed: -5
See Re: [PATCH] sandbox: i2c: Make sure phandle dependency is probed , 
let's continue it there.

[PATCH v3 3/4] power: regulator: Drop regulator_unset()

[Marek Vasut]

This function is never called, drop it.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
Cc: Caleb Connolly <caleb.connolly@linaro.org>
Cc: Chris Morgan <macromorgan@hotmail.com>
Cc: Dragan Simic <dsimic@manjaro.org>
Cc: Eugen Hristev <eugen.hristev@collabora.com>
Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Cc: Max Krummenacher <max.krummenacher@toradex.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
Cc: Quentin Schulz <quentin.schulz@cherry.de>
Cc: Sam Day <me@samcday.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Svyatoslav Ryhel <clamor95@gmail.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: u-boot-amlogic@groups.io
Cc: u-boot-qcom@groups.io
Cc: u-boot@dh-electronics.com
Cc: u-boot@lists.denx.de
Cc: uboot-stm32@st-md-mailman.stormreply.com
---
V2: Rebase on current u-boot/next
V3: No change
---
 drivers/power/regulator/regulator-uclass.c | 11 -----------
 include/power/regulator.h                  | 14 +-------------
 2 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 9fcc4bd85b9..4e83819ff73 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -345,17 +345,6 @@ out:
 	return ret;
 }
 
-int regulator_unset(struct udevice *dev)
-{
-	struct dm_regulator_uclass_plat *uc_pdata;
-
-	uc_pdata = dev_get_uclass_plat(dev);
-	if (uc_pdata && uc_pdata->force_off)
-		return regulator_set_enable(dev, false);
-
-	return -EMEDIUMTYPE;
-}
-
 static void regulator_show(struct udevice *dev, int ret)
 {
 	struct dm_regulator_uclass_plat *uc_pdata;
diff --git a/include/power/regulator.h b/include/power/regulator.h
index bb07a814c79..5363483d02a 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -430,7 +430,7 @@ int regulators_enable_boot_on(bool verbose);
  *
  * This disables all regulators which are marked to be off at boot time.
  *
- * This effectively calls regulator_unset() for every regulator.
+ * This effectively does nothing.
  */
 int regulators_enable_boot_off(bool verbose);
 
@@ -453,18 +453,6 @@ int regulators_enable_boot_off(bool verbose);
  */
 int regulator_autoset(struct udevice *dev);
 
-/**
- * regulator_unset: turn off a regulator
- *
- * The setup depends on constraints found in device's uclass's platform data
- * (struct dm_regulator_uclass_platdata):
- *
- * - Disable - will set - if  'force_off' is set to true,
- *
- * The function returns on the first-encountered error.
- */
-int regulator_unset(struct udevice *dev);
-
 /**
  * regulator_autoset_by_name: setup the regulator given by its uclass's
  * platform data name field. The setup depends on constraints found in device's
-- 
2.45.2

[PATCH v3 4/4] power: regulator: Drop regulators_enable_boot_on/off()

[Marek Vasut]

Both regulators_enable_boot_on/off() are unused and superseded by
regulator uclass regulator_post_probe(). Remove both functions.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Ben Wolsieffer <benwolsieffer@gmail.com>
Cc: Caleb Connolly <caleb.connolly@linaro.org>
Cc: Chris Morgan <macromorgan@hotmail.com>
Cc: Dragan Simic <dsimic@manjaro.org>
Cc: Eugen Hristev <eugen.hristev@collabora.com>
Cc: Francesco Dolcini <francesco.dolcini@toradex.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Cc: Max Krummenacher <max.krummenacher@toradex.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
Cc: Quentin Schulz <quentin.schulz@cherry.de>
Cc: Sam Day <me@samcday.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Svyatoslav Ryhel <clamor95@gmail.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: u-boot-amlogic@groups.io
Cc: u-boot-qcom@groups.io
Cc: u-boot@dh-electronics.com
Cc: u-boot@lists.denx.de
Cc: uboot-stm32@st-md-mailman.stormreply.com
---
V2: Rebase on current u-boot/next
V3: No change
---
 arch/arm/mach-rockchip/board.c                |  8 ------
 arch/arm/mach-snapdragon/board.c              |  1 -
 arch/arm/mach-tegra/board2.c                  |  3 ---
 board/Marvell/octeontx2_cn913x/board.c        |  5 ----
 .../amlogic/odroid-go-ultra/odroid-go-ultra.c |  2 --
 board/dhelectronics/dh_imx6/dh_imx6.c         |  2 --
 .../dh_imx8mp/imx8mp_dhcom_pdk2.c             |  2 --
 board/dhelectronics/dh_stm32mp1/board.c       |  2 --
 board/engicam/stm32mp1/stm32mp1.c             |  3 ---
 board/google/veyron/veyron.c                  |  4 ---
 board/samsung/common/exynos5-dt.c             |  4 ---
 board/st/stm32mp1/stm32mp1.c                  |  2 --
 drivers/power/regulator/regulator-uclass.c    | 10 --------
 include/power/regulator.h                     | 25 -------------------
 14 files changed, 73 deletions(-)

diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index 0fdf9365b41..3fadf7e4122 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -202,14 +202,6 @@ int board_late_init(void)
 
 int board_init(void)
 {
-	int ret;
-
-#ifdef CONFIG_DM_REGULATOR
-	ret = regulators_enable_boot_on(false);
-	if (ret)
-		debug("%s: Cannot enable boot on regulator\n", __func__);
-#endif
-
 	return 0;
 }
 
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
index 0af297470a6..2ab2ceb5138 100644
--- a/arch/arm/mach-snapdragon/board.c
+++ b/arch/arm/mach-snapdragon/board.c
@@ -237,7 +237,6 @@ void __weak qcom_board_init(void)
 
 int board_init(void)
 {
-	regulators_enable_boot_on(false);
 	show_psci_version();
 	qcom_of_fixup_nodes();
 	qcom_board_init();
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c
index 7971e3b68d5..5c5838629b2 100644
--- a/arch/arm/mach-tegra/board2.c
+++ b/arch/arm/mach-tegra/board2.c
@@ -187,9 +187,6 @@ int board_init(void)
 	warmboot_prepare_code(TEGRA_LP0_ADDR, TEGRA_LP0_SIZE);
 #endif
 
-	/* Set up boot-on regulators */
-	regulators_enable_boot_on(_DEBUG);
-
 	return nvidia_board_init();
 }
 
diff --git a/board/Marvell/octeontx2_cn913x/board.c b/board/Marvell/octeontx2_cn913x/board.c
index 3d20cfb2fab..3ffe15d42b8 100644
--- a/board/Marvell/octeontx2_cn913x/board.c
+++ b/board/Marvell/octeontx2_cn913x/board.c
@@ -23,11 +23,6 @@ int board_early_init_f(void)
 
 int board_early_init_r(void)
 {
-	if (CONFIG_IS_ENABLED(DM_REGULATOR)) {
-		/* Check if any existing regulator should be turned down */
-		regulators_enable_boot_off(false);
-	}
-
 	return 0;
 }
 
diff --git a/board/amlogic/odroid-go-ultra/odroid-go-ultra.c b/board/amlogic/odroid-go-ultra/odroid-go-ultra.c
index 8f3f2045d74..f9412071737 100644
--- a/board/amlogic/odroid-go-ultra/odroid-go-ultra.c
+++ b/board/amlogic/odroid-go-ultra/odroid-go-ultra.c
@@ -16,7 +16,5 @@ int mmc_get_env_dev(void)
 
 int board_init(void)
 {
-	regulators_enable_boot_on(_DEBUG);
-
 	return 0;
 }
diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c b/board/dhelectronics/dh_imx6/dh_imx6.c
index ada44e01424..f2b14bf701a 100644
--- a/board/dhelectronics/dh_imx6/dh_imx6.c
+++ b/board/dhelectronics/dh_imx6/dh_imx6.c
@@ -128,8 +128,6 @@ int board_init(void)
 
 	setup_fec_clock();
 
-	regulators_enable_boot_on(_DEBUG);
-
 	return 0;
 }
 
diff --git a/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c b/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c
index a389ab3c2d9..78aae412350 100644
--- a/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c
+++ b/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c
@@ -112,8 +112,6 @@ int dh_setup_mac_address(void)
 
 int board_init(void)
 {
-	regulators_enable_boot_on(_DEBUG);
-
 	return 0;
 }
 
diff --git a/board/dhelectronics/dh_stm32mp1/board.c b/board/dhelectronics/dh_stm32mp1/board.c
index 4f4f537fee5..24c5f37c12f 100644
--- a/board/dhelectronics/dh_stm32mp1/board.c
+++ b/board/dhelectronics/dh_stm32mp1/board.c
@@ -622,8 +622,6 @@ static void board_init_regulator_av96(void)
 static void board_init_regulator(void)
 {
 	board_init_regulator_av96();
-
-	regulators_enable_boot_on(_DEBUG);
 }
 #else
 static inline int board_get_regulator_buck3_nvm_uv_av96(int *uv)
diff --git a/board/engicam/stm32mp1/stm32mp1.c b/board/engicam/stm32mp1/stm32mp1.c
index bc2af66d8e9..56557d56429 100644
--- a/board/engicam/stm32mp1/stm32mp1.c
+++ b/board/engicam/stm32mp1/stm32mp1.c
@@ -37,9 +37,6 @@ int checkboard(void)
 /* board dependent setup after realloc */
 int board_init(void)
 {
-	if (IS_ENABLED(CONFIG_DM_REGULATOR))
-		regulators_enable_boot_on(_DEBUG);
-
 	return 0;
 }
 
diff --git a/board/google/veyron/veyron.c b/board/google/veyron/veyron.c
index bd8ce633772..674f19ba03c 100644
--- a/board/google/veyron/veyron.c
+++ b/board/google/veyron/veyron.c
@@ -57,10 +57,6 @@ static int veyron_init(void)
 	if (ret)
 		return log_msg_ret("s33", ret);
 
-	ret = regulators_enable_boot_on(false);
-	if (ret)
-		return log_msg_ret("boo", ret);
-
 	return 0;
 }
 #endif
diff --git a/board/samsung/common/exynos5-dt.c b/board/samsung/common/exynos5-dt.c
index 56862bcb34d..68edd1ec282 100644
--- a/board/samsung/common/exynos5-dt.c
+++ b/board/samsung/common/exynos5-dt.c
@@ -88,10 +88,6 @@ int exynos_power_init(void)
 	if (ret == -ENODEV)
 		return 0;
 
-	ret = regulators_enable_boot_on(false);
-	if (ret)
-		return ret;
-
 	ret = exynos_set_regulator("vdd_mif", 1100000);
 	if (ret)
 		return ret;
diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 97532a8156f..d5e5e776d2a 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -665,8 +665,6 @@ int board_init(void)
 	if (board_is_stm32mp15x_dk2())
 		board_stm32mp15x_dk2_init();
 
-	regulators_enable_boot_on(_DEBUG);
-
 	/*
 	 * sysconf initialisation done only when U-Boot is running in secure
 	 * done in TF-A for TFABOOT.
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 4e83819ff73..decd0802c84 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -526,16 +526,6 @@ static int regulator_post_probe(struct udevice *dev)
 	return 0;
 }
 
-int regulators_enable_boot_on(bool verbose)
-{
-	return 0;
-}
-
-int regulators_enable_boot_off(bool verbose)
-{
-	return 0;
-}
-
 UCLASS_DRIVER(regulator) = {
 	.id		= UCLASS_REGULATOR,
 	.name		= "regulator",
diff --git a/include/power/regulator.h b/include/power/regulator.h
index 5363483d02a..8a914dfc74f 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -414,26 +414,6 @@ int regulator_get_mode(struct udevice *dev);
  */
 int regulator_set_mode(struct udevice *dev, int mode_id);
 
-/**
- * regulators_enable_boot_on() - enable regulators needed for boot
- *
- * This enables all regulators which are marked to be on at boot time. This
- * only works for regulators which don't have a range for voltage/current,
- * since in that case it is not possible to know which value to use.
- *
- * This effectively calls regulator_autoset() for every regulator.
- */
-int regulators_enable_boot_on(bool verbose);
-
-/**
- * regulators_enable_boot_off() - disable regulators needed for boot
- *
- * This disables all regulators which are marked to be off at boot time.
- *
- * This effectively does nothing.
- */
-int regulators_enable_boot_off(bool verbose);
-
 /**
  * regulator_autoset: setup the voltage/current on a regulator
  *
@@ -617,11 +597,6 @@ static inline int regulator_set_mode(struct udevice *dev, int mode_id)
 	return -ENOSYS;
 }
 
-static inline int regulators_enable_boot_on(bool verbose)
-{
-	return -ENOSYS;
-}
-
 static inline int regulator_autoset(struct udevice *dev)
 {
 	return -ENOSYS;
-- 
2.45.2

Re: [PATCH v3 1/4] power: regulator: Trigger probe of regulators which are always-on or boot-on

[Tom Rini]

On Fri, 27 Sep 2024 01:14:12 +0200, Marek Vasut wrote:

> In case a regulator DT node contains regulator-always-on or regulator-boot-on
> property, make sure the regulator gets correctly configured by U-Boot on start
> up. Unconditionally probe such regulator drivers. This is a preparatory patch
> for introduction of .regulator_post_probe() which would trigger the regulator
> configuration.
> 
> Parsing of regulator-always-on and regulator-boot-on DT property has been
> moved to regulator_post_bind() as the information is required early, the
> rest of the DT parsing has been kept in regulator_pre_probe() to avoid
> slowing down the boot process.
> 
> [...]

Applied to u-boot/next, thanks!

-- 
Tom