[PATCH 0/8] regulator: core: allow regulator_register() with missing required supply

[PATCH 0/8] regulator: core: allow regulator_register() with missing required supply

[André Draszik]

Hi,

With these attached patches it becomes possible again to support
hardware designs with multiple PMICs where individual rails of each act
as required supplies for rails of the other (due to the latter being
e.g. always-on), and vice-versa.

Google Pixel 6 and 6 Pro (oriole and raven) are examples of such
designs.

Rather than returning -EPORBE_DEFER in regulator_register() when
set_machine_constraints() fails with -EPROBE_DEFER (due to missing
required supplies), we still allow rail registration and try to
reresolve supplies each time a new rail gets registered.

This is implemented using a bus (regulator bus), which allows the core
to reresolve supplies for regulators that still need them whenever new
regulators (i.e. devices) are added.

Using a bus also solves existing problems around late resolution of
supplies as mentioned in the commit message introducing that bus.

The series starts with a few bug fixes and the last two commits
implement the changes mentioned above, but do depend on the bug fixes.

Cheers,
Andre'

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
André Draszik (8):
      regulator: core: update two debug messages
      regulator: core: fix locking in regulator_resolve_supply() error path
      regulator: core: move supply check earlier in set_machine_constraints()
      regulator: core: streamline supply resolution for always-on/boot-on regulators
      regulator: core: remove dead code in regulator_resolve_supply()
      regulator: core: don't ignore errors from event forwarding setup
      regulator: core: reresolve unresolved supplies when available
      regulator: core: don't fail regulator_register() with missing required supply

 drivers/regulator/core.c         | 351 ++++++++++++++++++++++++++++-----------
 include/linux/regulator/driver.h |   2 +
 2 files changed, 254 insertions(+), 99 deletions(-)
---
base-commit: cc3aa43b44bdb43dfbac0fcb51c56594a11338a8
change-id: 20251224-regulators-defer-f5ffa668a3f6

Best regards,
-- 
André Draszik <andre.draszik@linaro.org>

[PATCH 1/8] regulator: core: update two debug messages

[André Draszik]

1)
In print_constraints_debug(), the power budget is printed as:

   lldo2: 450 <--> 1300 mV at 900 mV 2147483647 mW budge, enabled

(note the missing t in budget). This is because there is a --count just
below the call to scnprintf(), to make space for the comma. All similar
calls to scnprintf() above add an extra space to the format string to
allow for that, but this one doesn't, so the last character t is
stripped instead. Update the format string to fix the message.

2)
Add the name of the supply to the failure message printed when the
supply can not be resolved when debug messages are enabled to help with
debug.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 4b6182cde859adb356d160a012e54dcf1646da38..a723bd00e1716c2c3899c63314c6249015698216 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1183,7 +1183,7 @@ static void print_constraints_debug(struct regulator_dev *rdev)
 		count += scnprintf(buf + count, len - count, "standby ");
 
 	if (constraints->pw_budget_mW)
-		count += scnprintf(buf + count, len - count, "%d mW budget",
+		count += scnprintf(buf + count, len - count, "%d mW budget ",
 				   constraints->pw_budget_mW);
 
 	if (!count)
@@ -5697,7 +5697,8 @@ static int regulator_register_resolve_supply(struct device *dev, void *data)
 	struct regulator_dev *rdev = dev_to_rdev(dev);
 
 	if (regulator_resolve_supply(rdev))
-		rdev_dbg(rdev, "unable to resolve supply\n");
+		rdev_dbg(rdev, "unable to resolve supply '%s'\n",
+			 rdev->supply_name);
 
 	return 0;
 }

-- 
2.52.0.351.gbe84eed79e-goog

[PATCH 2/8] regulator: core: fix locking in regulator_resolve_supply() error path

[André Draszik]

If late enabling of a supply regulator fails in
regulator_resolve_supply(), the code currently triggers a lockdep
warning:

    WARNING: drivers/regulator/core.c:2649 at _regulator_put+0x80/0xa0, CPU#6: kworker/u32:4/596
    ...
    Call trace:
     _regulator_put+0x80/0xa0 (P)
     regulator_resolve_supply+0x7cc/0xbe0
     regulator_register_resolve_supply+0x28/0xb8

as the regulator_list_mutex must be held when calling _regulator_put().

To solve this, simply switch to using regulator_put().

While at it, we should also make sure that no concurrent access happens
to our rdev while we clear out the supply pointer. Add appropriate
locking to ensure that.

While the code in question will be removed altogether in a follow-up
commit, I believe it is still beneficial to have this corrected before
removal for future reference.

Fixes: 36a1f1b6ddc6 ("regulator: core: Fix memory leak in regulator_resolve_supply()")
Fixes: 8e5356a73604 ("regulator: core: Clear the supply pointer if enabling fails")
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/core.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a723bd00e1716c2c3899c63314c6249015698216..48c091de68d81e3e89eacecd8526255ab9a446b2 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2285,8 +2285,16 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	if (rdev->use_count) {
 		ret = regulator_enable(rdev->supply);
 		if (ret < 0) {
-			_regulator_put(rdev->supply);
+			struct regulator *supply;
+
+			regulator_lock_two(rdev, rdev->supply->rdev, &ww_ctx);
+
+			supply = rdev->supply;
 			rdev->supply = NULL;
+
+			regulator_unlock_two(rdev, supply->rdev, &ww_ctx);
+
+			regulator_put(supply);
 			goto out;
 		}
 	}

-- 
2.52.0.351.gbe84eed79e-goog

[PATCH 3/8] regulator: core: move supply check earlier in set_machine_constraints()

[André Draszik]

Since commit 98e48cd9283d ("regulator: core: resolve supply for
boot-on/always-on regulators"), set_machine_constraints() can return
-EPROBE_DEFER very late, after it has done a lot of work and
configuration of the regulator.

This means that configuration will happen multiple times for no
benefit in that case. Furthermore, this can lead to timing-dependent
voltage glitches as mentioned e.g. in commit 8a866d527ac0 ("regulator:
core: Resolve supply name earlier to prevent double-init").

We can know that it's going to fail very early, in particular before
going through the complete regulator configuration by moving some code
around a little.

Do so to avoid re-configuring the regulator multiple times, also
avoiding the voltage glitches if we can.

Fixes: 98e48cd9283d ("regulator: core: resolve supply for boot-on/always-on regulators")
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/core.c | 55 ++++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 48c091de68d81e3e89eacecd8526255ab9a446b2..9ce0eef1dcfcb39a072675e44a5577ae85d51982 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1444,6 +1444,33 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 	int ret = 0;
 	const struct regulator_ops *ops = rdev->desc->ops;
 
+	/*
+	 * If there is no mechanism for controlling the regulator then
+	 * flag it as always_on so we don't end up duplicating checks
+	 * for this so much.  Note that we could control the state of
+	 * a supply to control the output on a regulator that has no
+	 * direct control.
+	 */
+	if (!rdev->ena_pin && !ops->enable) {
+		if (rdev->supply_name && !rdev->supply)
+			return -EPROBE_DEFER;
+
+		if (rdev->supply)
+			rdev->constraints->always_on =
+				rdev->supply->rdev->constraints->always_on;
+		else
+			rdev->constraints->always_on = true;
+	}
+
+	/*
+	 * If we want to enable this regulator, make sure that we know the
+	 * supplying regulator.
+	 */
+	if (rdev->constraints->always_on || rdev->constraints->boot_on) {
+		if (rdev->supply_name && !rdev->supply)
+			return -EPROBE_DEFER;
+	}
+
 	ret = machine_constraints_voltage(rdev, rdev->constraints);
 	if (ret != 0)
 		return ret;
@@ -1609,37 +1636,15 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 		}
 	}
 
-	/*
-	 * If there is no mechanism for controlling the regulator then
-	 * flag it as always_on so we don't end up duplicating checks
-	 * for this so much.  Note that we could control the state of
-	 * a supply to control the output on a regulator that has no
-	 * direct control.
-	 */
-	if (!rdev->ena_pin && !ops->enable) {
-		if (rdev->supply_name && !rdev->supply)
-			return -EPROBE_DEFER;
-
-		if (rdev->supply)
-			rdev->constraints->always_on =
-				rdev->supply->rdev->constraints->always_on;
-		else
-			rdev->constraints->always_on = true;
-	}
-
 	/* If the constraints say the regulator should be on at this point
 	 * and we have control then make sure it is enabled.
 	 */
 	if (rdev->constraints->always_on || rdev->constraints->boot_on) {
 		bool supply_enabled = false;
 
-		/* If we want to enable this regulator, make sure that we know
-		 * the supplying regulator.
-		 */
-		if (rdev->supply_name && !rdev->supply)
-			return -EPROBE_DEFER;
-
-		/* If supplying regulator has already been enabled,
+		/* We have ensured a potential supply has been resolved above.
+		 *
+		 * If supplying regulator has already been enabled,
 		 * it's not intended to have use_count increment
 		 * when rdev is only boot-on.
 		 */

-- 
2.52.0.351.gbe84eed79e-goog

[PATCH 4/8] regulator: core: streamline supply resolution for always-on/boot-on regulators

[André Draszik]

For always-on and boot-on regulators, regulator_register() is currently
trying to anticipate the requirement to resolve a supply early.

Unfortunately, this code executes too early, before we have potentially
updated the regulator's always_on constraint as part of
set_machine_constraints(), causing it to miss cases.

Rather than trying to hack it more, just defer to the outcome of
set_machine_constraints(). The latter returns early (without doing any
regulator initialisation) with -EPROBE_DEFER as of commit 'regulator:
core: move supply check earlier in set_machine_constraints()' and is
therefore safe to call multiple times to determine if supplies need to
be resolved early.

Commit 8a866d527ac0 ("regulator: core: Resolve supply name earlier to
prevent double-init") (later updated by
commit 520fb178212d ("regulator: core: Fix regulator supply
registration with sysfs")) added these tests originally to avoid
calling set_machine_constraints() multiple times to try to avoid
voltage glitches due to all the regulator initialisation happening each
time. This isn't an issue anymore as per above.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/core.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 9ce0eef1dcfcb39a072675e44a5577ae85d51982..08bdb1e4175e6d47d154e7a2d859a9d39dcfd021 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5937,7 +5937,6 @@ regulator_register(struct device *dev,
 	bool dangling_cfg_gpiod = false;
 	bool dangling_of_gpiod = false;
 	int ret, i;
-	bool resolved_early = false;
 
 	if (cfg == NULL)
 		return ERR_PTR(-EINVAL);
@@ -6075,17 +6074,6 @@ regulator_register(struct device *dev,
 			goto wash;
 	}
 
-	if ((rdev->supply_name && !rdev->supply) &&
-		(rdev->constraints->always_on ||
-		 rdev->constraints->boot_on)) {
-		ret = regulator_resolve_supply(rdev);
-		if (ret)
-			rdev_dbg(rdev, "unable to resolve supply early: %pe\n",
-					 ERR_PTR(ret));
-
-		resolved_early = true;
-	}
-
 	if (config->ena_gpiod) {
 		ret = regulator_ena_gpio_request(rdev, config);
 		if (ret != 0) {
@@ -6099,9 +6087,10 @@ regulator_register(struct device *dev,
 	}
 
 	ret = set_machine_constraints(rdev);
-	if (ret == -EPROBE_DEFER && !resolved_early) {
-		/* Regulator might be in bypass mode and so needs its supply
-		 * to set the constraints
+	if (ret == -EPROBE_DEFER) {
+		/* Regulator might be in bypass mode or an always-on or boot-on
+		 * regulator and so needs its supply to set the constraints or
+		 * for enable.
 		 */
 		/* FIXME: this currently triggers a chicken-and-egg problem
 		 * when creating -SUPPLY symlink in sysfs to a regulator

-- 
2.52.0.351.gbe84eed79e-goog

[PATCH 5/8] regulator: core: remove dead code in regulator_resolve_supply()

[André Draszik]

Since commit 98e48cd9283d ("regulator: core: resolve supply for
boot-on/always-on regulators") we require that a regulator's supply has
been resolved before enabling the regulator. Furthermore,
regulator_get() also fails if the supply hasn't been resolved yet
(preventing consumers from enabling a regulator without its supply
known). In combination this means that regulator_resolve_supply() now
always runs before the regulator has been enabled via
set_machine_constraints().

The code here was meant to run after enabling the regulator in case the
supply hadn't been resolved at that time and can therefore never
execute anymore since that commit.

Remove it.

No functional change intended.

Fixes: 98e48cd9283d ("regulator: core: resolve supply for boot-on/always-on regulators")
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/core.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 08bdb1e4175e6d47d154e7a2d859a9d39dcfd021..fd8da369c0529da12d6e80d90032e07bed414316 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2282,28 +2282,6 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	/* rdev->supply was created in set_supply() */
 	link_and_create_debugfs(rdev->supply, r, &rdev->dev);
 
-	/*
-	 * In set_machine_constraints() we may have turned this regulator on
-	 * but we couldn't propagate to the supply if it hadn't been resolved
-	 * yet.  Do it now.
-	 */
-	if (rdev->use_count) {
-		ret = regulator_enable(rdev->supply);
-		if (ret < 0) {
-			struct regulator *supply;
-
-			regulator_lock_two(rdev, rdev->supply->rdev, &ww_ctx);
-
-			supply = rdev->supply;
-			rdev->supply = NULL;
-
-			regulator_unlock_two(rdev, supply->rdev, &ww_ctx);
-
-			regulator_put(supply);
-			goto out;
-		}
-	}
-
 out:
 	return ret;
 }

-- 
2.52.0.351.gbe84eed79e-goog

Re: [PATCH 5/8] regulator: core: remove dead code in regulator_resolve_supply()

[Mark Brown]

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

On Sat, Dec 27, 2025 at 12:17:49PM +0000, André Draszik wrote:

> No functional change intended.

> Fixes: 98e48cd9283d ("regulator: core: resolve supply for boot-on/always-on regulators")

If this is just a cleanup then clearly it's not fixing a bug.  Please
don't throw Fixes tags on random commits that don't actually fix bugs,
it just makes noise for people who are looking for actual fixes.

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

Re: [PATCH 5/8] regulator: core: remove dead code in regulator_resolve_supply()

[André Draszik]

Hi Mark,

On Sat, 2025-12-27 at 20:01 +0000, Mark Brown wrote:
> On Sat, Dec 27, 2025 at 12:17:49PM +0000, André Draszik wrote:
> 
> > No functional change intended.
> 
> > Fixes: 98e48cd9283d ("regulator: core: resolve supply for boot-on/always-on regulators")
> 
> If this is just a cleanup then clearly it's not fixing a bug.  Please
> don't throw Fixes tags on random commits that don't actually fix bugs,
> it just makes noise for people who are looking for actual fixes.

I added it to highlight that the dead code removal should have been
part of that commit, but left out CC:stable for the reason that this
commit is not strictly required. Especially the now incorrect comment
in the code was a bit confusing since that commit, and I wanted to make
it easier for future reader of that code, no matter which kernel version
somebody else might be looking at.

Anyway, will drop in next version.

Cheers,
Andre'

[PATCH 6/8] regulator: core: don't ignore errors from event forwarding setup

[André Draszik]

Receiving and forwarding critical supply events seems like they're
important information and we shouldn't ignore errors occurring during
registration for such events.

With this change the supply is unset on event registration failure,
allowing us to potentially retry another time.

Fixes: 433e294c3c5b ("regulator: core: forward undervoltage events downstream by default")
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/core.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index fd8da369c0529da12d6e80d90032e07bed414316..86dbee3ffda0b950619db8b52d6c6eab8be31a53 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2273,10 +2273,21 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	 * under-voltage.
 	 */
 	ret = register_regulator_event_forwarding(rdev);
-	if (ret < 0)
+	if (ret < 0) {
+		struct regulator *supply;
+
 		rdev_warn(rdev, "Failed to register event forwarding: %pe\n",
 			  ERR_PTR(ret));
 
+		supply = rdev->supply;
+		rdev->supply = NULL;
+
+		regulator_unlock_two(rdev, supply->rdev, &ww_ctx);
+
+		regulator_put(supply);
+		goto out;
+	}
+
 	regulator_unlock_two(rdev, r, &ww_ctx);
 
 	/* rdev->supply was created in set_supply() */

-- 
2.52.0.351.gbe84eed79e-goog

[PATCH 7/8] regulator: core: reresolve unresolved supplies when available

[André Draszik]

When a regulator A and its supply B are provided by different devices,
the driver implementing B might be last to probe (with A still
pending resolution of its supply B). While we try to resolve all
pending supplies for all regulators (including A) during
regulator_register() of B via regulator_register_resolve_supply(),
Supply resolution will still not work for A as the driver for B hasn't
finished binding to the PMIC device corresponding to B at that stage
yet. The regulator core explicitly only allows supplies from other
devices to be used once the relevant driver has fully bound, mainly to
avoid having to deal with cases where B itself might -EPROBE_DEFER.

In this case, A's supply will only be resolved as part of the core's
regulator_init_complete_work_function(), which currently is scheduled
to run after 30s. This was added as a work-around in
commit 3827b64dba27 ("regulator: core: Resolve supplies before
disabling unused regulators") to cover this situation.

There are two problems with that approach:
* it potentially runs long after all our consumers have probed
* an upcoming change will allow regulator_register() to complete
  successfully even when required supplies (e.g. due to always-on or
  boot-on) are missing at register time, deferring full configuration
  of the regulator (and usability by consumers, i.e. usually consumer
  probe) until the supply becomes available.
  Resolving supplies in the late work func can therefore make it
  impossible for consumers to probe at all, as the driver core will not
  know to reprobe consumers when supplies have resolved.

We could schedule an earlier work to try to resolve supplies sooner,
but that'd be racy as consumers of A might try to probe before A's
supply gets fully resolved via this extra work.

Instead, add a very simple regulator bus and add a dummy device with a
corresponding driver to it for each regulator that is missing its
supply during regulator_register(). This way, the driver core will call
our bus' probe whenever a new (regulator) device was successfully
bound, allowing us to retry resolving the supply during (our bus) probe
and to bind this dummy device if successful. In turn this means the
driver core will see a newly bound device and retry probing of all
pending consumers, if any.

With that in place, we can avoid walking the full list of all known
regulators to try resolve missing supplies during regulator_register(),
as the driver core will invoke the bus probe for regulators that are
still pending their supplies. We can also drop the code trying to
resolve supplies one last time before unused regulators get disabled,
as all supplies should have resolved at that point in time, and if they
haven't then there's no point in trying again, as the outcome won't
change.

Note: We can not reuse the existing struct device created for each
rail, as a device can not be part of a class and a bus simultaneously.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/core.c         | 121 +++++++++++++++++++++++++++++++--------
 include/linux/regulator/driver.h |   1 +
 2 files changed, 98 insertions(+), 24 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 86dbee3ffda0b950619db8b52d6c6eab8be31a53..08e92b1ba2dc2ff9efdabaa16187a4a38cf66fb2 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -44,6 +44,8 @@ static LIST_HEAD(regulator_supply_alias_list);
 static LIST_HEAD(regulator_coupler_list);
 static bool has_full_constraints;
 
+static const struct bus_type regulator_bus;
+
 static struct dentry *debugfs_root;
 
 /*
@@ -5694,17 +5696,6 @@ static void rdev_init_debugfs(struct regulator_dev *rdev)
 			   &rdev->bypass_count);
 }
 
-static int regulator_register_resolve_supply(struct device *dev, void *data)
-{
-	struct regulator_dev *rdev = dev_to_rdev(dev);
-
-	if (regulator_resolve_supply(rdev))
-		rdev_dbg(rdev, "unable to resolve supply '%s'\n",
-			 rdev->supply_name);
-
-	return 0;
-}
-
 int regulator_coupler_register(struct regulator_coupler *coupler)
 {
 	mutex_lock(&regulator_list_mutex);
@@ -5923,6 +5914,7 @@ regulator_register(struct device *dev,
 	struct regulator_config *config = NULL;
 	static atomic_t regulator_no = ATOMIC_INIT(-1);
 	struct regulator_dev *rdev;
+	bool tried_supply_resolve = false;
 	bool dangling_cfg_gpiod = false;
 	bool dangling_of_gpiod = false;
 	int ret, i;
@@ -6093,6 +6085,7 @@ regulator_register(struct device *dev,
 		else
 			rdev_dbg(rdev, "unable to resolve supply early: %pe\n",
 				 ERR_PTR(ret));
+		tried_supply_resolve = true;
 	}
 	if (ret < 0)
 		goto wash;
@@ -6124,6 +6117,37 @@ regulator_register(struct device *dev,
 	if (ret != 0)
 		goto unset_supplies;
 
+	if (!tried_supply_resolve) {
+		/*
+		 * As an optimisation, try to resolve our supply (if any) now to
+		 * avoid adding the bus device. Errors are not fatal at this
+		 * stage, we'll simply try again later.
+		 */
+		ret = regulator_resolve_supply(rdev);
+		if (ret)
+			rdev_dbg(rdev,
+				 "unable to resolve supply (ignoring): %pe\n",
+				 ERR_PTR(ret));
+	}
+
+	/*
+	 * If we have a supply but couldn't resolve it yet, register a device
+	 * with our bus, so that the bus probe gets called whenever any new
+	 * driver binds, allowing us to retry matching supplies and which then
+	 * triggers (re)probe of consumers if successful.
+	 */
+	if (rdev->supply_name && !rdev->supply) {
+		device_initialize(&rdev->bdev);
+		rdev->bdev.bus = &regulator_bus;
+		rdev->bdev.parent = &rdev->dev;
+		device_set_pm_not_required(&rdev->dev);
+		dev_set_name(&rdev->bdev, "%s.bdev", dev_name(&rdev->dev));
+
+		ret = device_add(&rdev->bdev);
+		if (ret)
+			goto del_cdev_and_bdev;
+	}
+
 	rdev_init_debugfs(rdev);
 
 	/* try to resolve regulators coupling since a new one was registered */
@@ -6131,12 +6155,13 @@ regulator_register(struct device *dev,
 	regulator_resolve_coupling(rdev);
 	mutex_unlock(&regulator_list_mutex);
 
-	/* try to resolve regulators supply since a new one was registered */
-	class_for_each_device(&regulator_class, NULL, NULL,
-			      regulator_register_resolve_supply);
 	kfree(config);
 	return rdev;
 
+del_cdev_and_bdev:
+	if (rdev->bdev.bus == &regulator_bus)
+		put_device(&rdev->bdev);
+	device_del(&rdev->dev);
 unset_supplies:
 	mutex_lock(&regulator_list_mutex);
 	unset_regulator_supplies(rdev);
@@ -6189,6 +6214,9 @@ void regulator_unregister(struct regulator_dev *rdev)
 	unset_regulator_supplies(rdev);
 	list_del(&rdev->list);
 	regulator_ena_gpio_free(rdev);
+	if (rdev->bdev.bus == &regulator_bus)
+		/* only if the device was added in the first place */
+		device_unregister(&rdev->bdev);
 	device_unregister(&rdev->dev);
 
 	mutex_unlock(&regulator_list_mutex);
@@ -6269,6 +6297,45 @@ const struct class regulator_class = {
 	.pm = &regulator_pm_ops,
 #endif
 };
+
+#define bdev_to_rdev(__bdev) container_of_const(__bdev, struct regulator_dev, bdev)
+
+static int regulator_bus_match(struct device *bdev,
+			       const struct device_driver *drv)
+{
+	/* Match always succeeds, we only have one driver */
+	return 1;
+}
+
+static int regulator_bus_probe(struct device *bdev)
+{
+	struct regulator_dev *rdev = bdev_to_rdev(bdev);
+	int ret;
+
+	ret = regulator_resolve_supply(rdev);
+	if (ret)
+		rdev_dbg(rdev,
+			 "unable to resolve supply or constraints '%s': %pe\n",
+			 rdev->supply_name, ERR_PTR(ret));
+	else
+		rdev_dbg(rdev, "resolved supply '%s'\n", rdev->supply_name);
+
+	return ret;
+}
+
+static const struct bus_type regulator_bus = {
+	.name = "regulator",
+	.match = regulator_bus_match,
+	.probe = regulator_bus_probe,
+};
+
+static struct device_driver regulator_bus_driver = {
+	.name = "regulator-bus-drv",
+	.bus = &regulator_bus,
+	.suppress_bind_attrs = true,
+	.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+};
+
 /**
  * regulator_has_full_constraints - the system has fully specified constraints
  *
@@ -6602,7 +6669,17 @@ static int __init regulator_init(void)
 {
 	int ret;
 
+	ret = bus_register(&regulator_bus);
+	if (ret)
+		return ret;
+
 	ret = class_register(&regulator_class);
+	if (ret)
+		goto err_class;
+
+	ret = driver_register(&regulator_bus_driver);
+	if (ret)
+		goto err_driver;
 
 	debugfs_root = debugfs_create_dir("regulator", NULL);
 	if (IS_ERR(debugfs_root))
@@ -6619,6 +6696,12 @@ static int __init regulator_init(void)
 
 	regulator_coupler_register(&generic_regulator_coupler);
 
+	return 0;
+
+err_driver:
+	class_unregister(&regulator_class);
+err_class:
+	bus_unregister(&regulator_bus);
 	return ret;
 }
 
@@ -6679,16 +6762,6 @@ __setup("regulator_ignore_unused", regulator_ignore_unused_setup);
 
 static void regulator_init_complete_work_function(struct work_struct *work)
 {
-	/*
-	 * Regulators may had failed to resolve their input supplies
-	 * when were registered, either because the input supply was
-	 * not registered yet or because its parent device was not
-	 * bound yet. So attempt to resolve the input supplies for
-	 * pending regulators before trying to disable unused ones.
-	 */
-	class_for_each_device(&regulator_class, NULL, NULL,
-			      regulator_register_resolve_supply);
-
 	/*
 	 * For debugging purposes, it may be useful to prevent unused
 	 * regulators from being disabled.
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 978cf593b6624228fe1fd9c2a3e186b53ef172f8..d38353f2b56f8bbab865d903ad0ec97ca0b5c834 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -635,6 +635,7 @@ struct regulator_dev {
 	int ref_cnt;
 	struct module *owner;
 	struct device dev;
+	struct device bdev;
 	struct regulation_constraints *constraints;
 	struct regulator *supply;	/* for tree */
 	const char *supply_name;

-- 
2.52.0.351.gbe84eed79e-goog

[PATCH 8/8] regulator: core: don't fail regulator_register() with missing required supply

[André Draszik]

Since commit 98e48cd9283d ("regulator: core: resolve supply for
boot-on/always-on regulators"), the regulator core returns
-EPROBE_DEFER if a supply can not be resolved at regulator_register()
time due to set_machine_constraints() requiring that supply (e.g.
because of always-on or boot-on).

In some hardware designs, multiple PMICs are used where individual
rails of each act as supplies for rails of the other, and vice-versa.
In such a design no PMIC driver can probe when registering one top-
level regulator device (as is common practice for almost all regulator
drivers in Linux) since that commit. Supplies are only considered when
their driver has fully bound, but because in a design like the above
two drivers / devices depend on each other, neither will have fully
bound while the other probes. The Google Pixel 6 and 6 Pro (oriole and
raven) are examples of such a design.

One way to make this work would be to register each rail as an
individual device, rather than just one top-level regulator device.
Then, fw-devlink and Linux' driver core could do their usual handling
of deferred device probe as each rail would be probed individually.
This approach was dismissed in [1] as each regulator driver would have
to take care of this itself.

Alternatively, we can change the regulator core to not fail
regulator_register() if a rail's required supply can not be resolved
while keeping the intended change from above mentioned commit, and
instead retry whenever a new rail is registered. This commit implements
such an approach:

    If set_machine_constraints() requests probe deferral,
    regulator_register() still succeeds and we retry setting
    constraints as part of regulator_resolve_supply().
    We still do not enable the regulator or allow consumers to use it
    until constraints have been set (including resolution of the
    supply) to prevent enabling of a regulator before its supply.

With this change, we keep track of regulators with missing required
supplies and can therefore try to resolve them again and try to set
the constraints again once more regulators become available.

Care has to be taken to not allow consumers to use regulators that
haven't had their constraints set yet. regulator_get() ensures that
and now returns -EPROBE_DEFER in that case.

The implementation is straight-forward, thanks to our newly introduced
regulator-bus. Locking in regulator_resolve_supply() has to be done
carefully, as a combination of regulator_(un)lock_two() and
regulator_(un)lock_dependent() is needed. The reason is that
set_machine_constraints() might call regulator_enable() which needs
rdev and all its dependents locked, but everything else requires to
only have rdev and its supply locked.

Link: https://lore.kernel.org/all/aRn_-o-vie_QoDXD@sirena.co.uk/ [1]
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/core.c         | 148 +++++++++++++++++++++++++++++++--------
 include/linux/regulator/driver.h |   1 +
 2 files changed, 119 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 08e92b1ba2dc2ff9efdabaa16187a4a38cf66fb2..8c2fd20edd50591c962454a358459e52e97c8ac0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -98,6 +98,7 @@ struct regulator_event_work {
 	unsigned long event;
 };
 
+static int _regulator_enable(struct regulator *regulator);
 static int _regulator_is_enabled(struct regulator_dev *rdev);
 static int _regulator_disable(struct regulator *regulator);
 static int _regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags);
@@ -1432,6 +1433,7 @@ static int handle_notify_limits(struct regulator_dev *rdev,
 /**
  * set_machine_constraints - sets regulator constraints
  * @rdev: regulator source
+ * @is_locked: whether or not this is called with locks held already
  *
  * Allows platform initialisation code to define and constrain
  * regulator circuits e.g. valid voltage/current ranges, etc.  NOTE:
@@ -1441,7 +1443,8 @@ static int handle_notify_limits(struct regulator_dev *rdev,
  *
  * Return: 0 on success or a negative error number on failure.
  */
-static int set_machine_constraints(struct regulator_dev *rdev)
+static int set_machine_constraints(struct regulator_dev *rdev,
+				   bool is_locked)
 {
 	int ret = 0;
 	const struct regulator_ops *ops = rdev->desc->ops;
@@ -1653,7 +1656,9 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 		if (rdev->supply &&
 		    (rdev->constraints->always_on ||
 		     !regulator_is_enabled(rdev->supply))) {
-			ret = regulator_enable(rdev->supply);
+			ret = (is_locked
+			       ? _regulator_enable(rdev->supply)
+			       : regulator_enable(rdev->supply));
 			if (ret < 0) {
 				_regulator_put(rdev->supply);
 				rdev->supply = NULL;
@@ -1781,6 +1786,15 @@ static int register_regulator_event_forwarding(struct regulator_dev *rdev)
 	return 0;
 }
 
+static void unregister_regulator_event_forwarding(struct regulator_dev *rdev)
+{
+	if (!rdev->supply_fwd_nb.notifier_call)
+		return;
+
+	regulator_unregister_notifier(rdev->supply, &rdev->supply_fwd_nb);
+	rdev->supply_fwd_nb.notifier_call = NULL;
+}
+
 /**
  * set_supply - set regulator supply regulator
  * @rdev: regulator (locked)
@@ -2169,6 +2183,8 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	struct regulator_dev *r;
 	struct device *dev = rdev->dev.parent;
 	struct ww_acquire_ctx ww_ctx;
+	struct regulator *supply;
+	bool do_final_setup;
 	int ret = 0;
 
 	/* No supply to resolve? */
@@ -2176,7 +2192,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 		return 0;
 
 	/* Supply already resolved? (fast-path without locking contention) */
-	if (rdev->supply)
+	if (rdev->supply && !rdev->constraints_pending)
 		return 0;
 
 	/* first do a dt based lookup on the node described in the virtual
@@ -2257,46 +2273,115 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 
 	/* Supply just resolved by a concurrent task? */
 	if (rdev->supply) {
+		/* Constraints might still be pending due to concurrency. */
+		bool done = !rdev->constraints_pending;
+
+		supply = rdev->supply;
+
 		regulator_unlock_two(rdev, r, &ww_ctx);
 		put_device(&r->dev);
-		goto out;
-	}
 
-	ret = set_supply(rdev, r);
-	if (ret < 0) {
+		/*
+		 * Supply resolved by concurrent task, and constraints set as
+		 * well (or not required): fast path.
+		 */
+		if (done)
+			goto out;
+
+		do_final_setup = false;
+	} else {
+		ret = set_supply(rdev, r);
+		if (ret < 0) {
+			regulator_unlock_two(rdev, r, &ww_ctx);
+			put_device(&r->dev);
+			goto out;
+		}
+
+		supply = rdev->supply;
+
+		/*
+		 * Automatically register for event forwarding from the new
+		 * supply. This creates the downstream propagation link for
+		 * events like under-voltage.
+		 */
+		ret = register_regulator_event_forwarding(rdev);
+		if (ret < 0) {
+			rdev_warn(rdev,
+				  "Failed to register event forwarding: %pe\n",
+				  ERR_PTR(ret));
+
+			goto unset_supply;
+		}
+
 		regulator_unlock_two(rdev, r, &ww_ctx);
-		put_device(&r->dev);
-		goto out;
+
+		do_final_setup = true;
 	}
 
 	/*
-	 * Automatically register for event forwarding from the new supply.
-	 * This creates the downstream propagation link for events like
-	 * under-voltage.
+	 * Now that we have the supply, we can retry setting the machine
+	 * constraints, if necessary.
 	 */
-	ret = register_regulator_event_forwarding(rdev);
-	if (ret < 0) {
-		struct regulator *supply;
-
-		rdev_warn(rdev, "Failed to register event forwarding: %pe\n",
-			  ERR_PTR(ret));
-
-		supply = rdev->supply;
-		rdev->supply = NULL;
+	regulator_lock_dependent(rdev, &ww_ctx);
+	if (rdev->constraints_pending) {
+		if (!rdev->supply) {
+			/*
+			 * Supply could have been released by another task that
+			 * failed to set the constraints or event forwarding.
+			 */
+			regulator_unlock_dependent(rdev, &ww_ctx);
+			ret = -EPROBE_DEFER;
+			goto out;
+		}
 
-		regulator_unlock_two(rdev, supply->rdev, &ww_ctx);
+		ret = set_machine_constraints(rdev, true);
+		if (ret < 0) {
+			regulator_unlock_dependent(rdev, &ww_ctx);
+
+			rdev_warn(rdev,
+				  "Failed to set machine constraints: %pe\n",
+				  ERR_PTR(ret));
+
+			regulator_lock_two(rdev, r, &ww_ctx);
+
+			if (supply != rdev->supply) {
+				/*
+				 * Supply could have been released by another
+				 * task that got here before us. If it did, it
+				 * will have released 'supply' (i.e. the
+				 * previous rdev->supply) and we shouldn't do
+				 * that again via unset_supply.
+				 */
+				regulator_unlock_two(rdev, r, &ww_ctx);
+				goto out;
+			}
 
-		regulator_put(supply);
-		goto out;
+			unregister_regulator_event_forwarding(rdev);
+			rdev->constraints_pending = true;
+			goto unset_supply;
+		}
+		rdev->constraints_pending = false;
 	}
+	regulator_unlock_dependent(rdev, &ww_ctx);
 
-	regulator_unlock_two(rdev, r, &ww_ctx);
+	if (!do_final_setup)
+		goto out;
 
 	/* rdev->supply was created in set_supply() */
-	link_and_create_debugfs(rdev->supply, r, &rdev->dev);
+	link_and_create_debugfs(rdev->supply, rdev->supply->rdev, &rdev->dev);
 
 out:
 	return ret;
+
+unset_supply:
+	lockdep_assert_held_once(&rdev->mutex.base);
+	lockdep_assert_held_once(&r->mutex.base);
+	rdev->supply = NULL;
+	regulator_unlock_two(rdev, supply->rdev, &ww_ctx);
+
+	regulator_put(supply);
+
+	return ret;
 }
 
 /* common pre-checks for regulator requests */
@@ -6067,7 +6152,7 @@ regulator_register(struct device *dev,
 		dangling_of_gpiod = false;
 	}
 
-	ret = set_machine_constraints(rdev);
+	ret = set_machine_constraints(rdev, false);
 	if (ret == -EPROBE_DEFER) {
 		/* Regulator might be in bypass mode or an always-on or boot-on
 		 * regulator and so needs its supply to set the constraints or
@@ -6081,14 +6166,17 @@ regulator_register(struct device *dev,
 			 rdev->supply_name);
 		ret = regulator_resolve_supply(rdev);
 		if (!ret)
-			ret = set_machine_constraints(rdev);
+			ret = set_machine_constraints(rdev, false);
 		else
 			rdev_dbg(rdev, "unable to resolve supply early: %pe\n",
 				 ERR_PTR(ret));
 		tried_supply_resolve = true;
 	}
-	if (ret < 0)
-		goto wash;
+	if (ret < 0) {
+		if (ret != -EPROBE_DEFER)
+			goto wash;
+		rdev->constraints_pending = true;
+	}
 
 	ret = regulator_init_coupling(rdev);
 	if (ret < 0)
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index d38353f2b56f8bbab865d903ad0ec97ca0b5c834..09f3b67638f9e63a32cfdbaf9c8654afbd02a547 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -650,6 +650,7 @@ struct regulator_dev {
 	struct regulator_enable_gpio *ena_pin;
 	unsigned int ena_gpio_state:1;
 
+	unsigned int constraints_pending:1;
 	unsigned int is_switch:1;
 
 	/* time when this regulator was disabled last time */

-- 
2.52.0.351.gbe84eed79e-goog