Hi Tom,
On Wed, 2 Oct 2024 at 19:40, Tom Rini <tr...@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 <ma...@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 <ma...@denx.de>
> > > ---
> > > Cc: Ben Wolsieffer <benwolsief...@gmail.com>
> > > Cc: Caleb Connolly <caleb.conno...@linaro.org>
> > > Cc: Chris Morgan <macromor...@hotmail.com>
> > > Cc: Dragan Simic <dsi...@manjaro.org>
> > > Cc: Eugen Hristev <eugen.hris...@collabora.com>
> > > Cc: Francesco Dolcini <francesco.dolc...@toradex.com>
> > > Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
> > > Cc: Jaehoon Chung <jh80.ch...@samsung.com>
> > > Cc: Jagan Teki <ja...@amarulasolutions.com>
> > > Cc: Jonas Karlman <jo...@kwiboo.se>
> > > Cc: Kever Yang <kever.y...@rock-chips.com>
> > > Cc: Matteo Lisi <matteo.l...@engicam.com>
> > > Cc: Mattijs Korpershoek <mkorpersh...@baylibre.com>
> > > Cc: Max Krummenacher <max.krummenac...@toradex.com>
> > > Cc: Neil Armstrong <neil.armstr...@linaro.org>
> > > Cc: Patrice Chotard <patrice.chot...@foss.st.com>
> > > Cc: Patrick Delaunay <patrick.delau...@foss.st.com>
> > > Cc: Philipp Tomsich <philipp.toms...@vrull.eu>
> > > Cc: Quentin Schulz <quentin.sch...@cherry.de>
> > > Cc: Sam Day <m...@samcday.com>
> > > Cc: Simon Glass <s...@chromium.org>
> > > Cc: Sumit Garg <sumit.g...@linaro.org>
> > > Cc: Svyatoslav Ryhel <clamo...@gmail.com>
> > > Cc: Thierry Reding <tred...@nvidia.com>
> > > Cc: Tom Rini <tr...@konsulko.com>
> > > Cc: Volodymyr Babchuk <volodymyr_babc...@epam.com>
> > > Cc: u-boot-amlo...@groups.io
> > > Cc: u-boot-q...@groups.io
> > > Cc: u-b...@dh-electronics.com
> > > Cc: u-boot@lists.denx.de
> > > Cc: uboot-st...@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
