regulator: core: allow regulator_register() with

Merge series from André Draszik <andre.draszik@linaro.org>:

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.

2 files changed, 255 insertions, 99 deletions

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 4b6182cde859..8c2fd20edd50 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;
 
 /*
@@ -96,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);
@@ -1183,7 +1186,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)
@@ -1430,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:
@@ -1439,11 +1443,39 @@ 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;
 
+	/*
+	 * 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,44 +1641,24 @@ 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.
 		 */
 		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;
@@ -1774,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)
@@ -2162,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? */
@@ -2169,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
@@ -2250,49 +2273,115 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 
 	/* Supply just resolved by a concurrent task? */
 	if (rdev->supply) {
-		regulator_unlock_two(rdev, r, &ww_ctx);
-		put_device(&r->dev);
-		goto out;
-	}
+		/* Constraints might still be pending due to concurrency. */
+		bool done = !rdev->constraints_pending;
+
+		supply = rdev->supply;
 
-	ret = set_supply(rdev, r);
-	if (ret < 0) {
 		regulator_unlock_two(rdev, r, &ww_ctx);
 		put_device(&r->dev);
-		goto out;
-	}
 
-	/*
-	 * 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));
+		/*
+		 * Supply resolved by concurrent task, and constraints set as
+		 * well (or not required): fast path.
+		 */
+		if (done)
+			goto out;
 
-	regulator_unlock_two(rdev, r, &ww_ctx);
+		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;
+		}
 
-	/* rdev->supply was created in set_supply() */
-	link_and_create_debugfs(rdev->supply, r, &rdev->dev);
+		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);
+
+		do_final_setup = true;
+	}
 
 	/*
-	 * 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.
+	 * Now that we have the supply, we can retry setting the machine
+	 * constraints, if necessary.
 	 */
-	if (rdev->use_count) {
-		ret = regulator_enable(rdev->supply);
-		if (ret < 0) {
-			_regulator_put(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;
 		}
+
+		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;
+			}
+
+			unregister_regulator_event_forwarding(rdev);
+			rdev->constraints_pending = true;
+			goto unset_supply;
+		}
+		rdev->constraints_pending = false;
 	}
+	regulator_unlock_dependent(rdev, &ww_ctx);
+
+	if (!do_final_setup)
+		goto out;
+
+	/* rdev->supply was created in set_supply() */
+	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 */
@@ -5692,16 +5781,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\n");
-
-	return 0;
-}
-
 int regulator_coupler_register(struct regulator_coupler *coupler)
 {
 	mutex_lock(&regulator_list_mutex);
@@ -5920,10 +5999,10 @@ 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;
-	bool resolved_early = false;
 
 	if (cfg == NULL)
 		return ERR_PTR(-EINVAL);
@@ -6061,17 +6140,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) {
@@ -6084,10 +6152,11 @@ regulator_register(struct device *dev,
 		dangling_of_gpiod = false;
 	}
 
-	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
+	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
+		 * for enable.
 		 */
 		/* FIXME: this currently triggers a chicken-and-egg problem
 		 * when creating -SUPPLY symlink in sysfs to a regulator
@@ -6097,13 +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) {
+		if (ret != -EPROBE_DEFER)
+			goto wash;
+		rdev->constraints_pending = true;
 	}
-	if (ret < 0)
-		goto wash;
 
 	ret = regulator_init_coupling(rdev);
 	if (ret < 0)
@@ -6132,6 +6205,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 */
@@ -6139,12 +6243,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);
@@ -6197,6 +6302,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);
@@ -6277,6 +6385,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
  *
@@ -6610,7 +6757,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))
@@ -6627,6 +6784,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;
 }
 
@@ -6688,16 +6851,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/drivers/regulator/fp9931.c b/drivers/regulator/fp9931.c
index fef0bb07fd5d..69b3c712e5d5 100644
--- a/drivers/regulator/fp9931.c
+++ b/drivers/regulator/fp9931.c
@@ -391,6 +391,7 @@ static const struct regulator_desc regulators[] = {
 	{
 		.name = "v3p3",
 		.of_match = of_match_ptr("v3p3"),
+		.regulators_node = of_match_ptr("regulators"),
 		.id = 0,
 		.ops = &fp9931_v3p3ops,
 		.type = REGULATOR_VOLTAGE,
@@ -403,6 +404,7 @@ static const struct regulator_desc regulators[] = {
 	{
 		.name = "vposneg",
 		.of_match = of_match_ptr("vposneg"),
+		.regulators_node = of_match_ptr("regulators"),
 		.id = 1,
 		.ops = &fp9931_vposneg_ops,
 		.type = REGULATOR_VOLTAGE,
@@ -415,6 +417,7 @@ static const struct regulator_desc regulators[] = {
 	{
 		.name = "vcom",
 		.of_match = of_match_ptr("vcom"),
+		.regulators_node = of_match_ptr("regulators"),
 		.id = 2,
 		.ops = &fp9931_vcom_ops,
 		.type = REGULATOR_VOLTAGE,