Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Jon Hunter]

On 22/04/16 14:53, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Apr 22, 2016 at 12:26:57PM +0100, Jon Hunter wrote:
> 
>> OK. Sorry if I have misunderstood you here, but this sounds more like
>> Thierry's initial proposal [0] but ignoring the any errors returned (and
>> we need to fix-up the locking in this patch). In the discussion that
> 
> Yes!
> 
>> followed I thought we agreed to only do this for the bypass case [1]. As
>> far as I am concerned either will work, but to confirm we should just
>> always try to resolve the supply early during regulator_register(), correct?
> 
> We need to only *fail* in the bypass case.

OK. So this is what I have now. Is it weird to return EPROBE_DEFER in 
_regulator_get_voltage()? If so, I could add a test for bypass in the
regulator_register().

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5b46d907e61d..7a6b7f667bcb 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3126,7 +3126,7 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 		if (bypassed) {
 			/* if bypassed the regulator must have a supply */
 			if (!rdev->supply)
-				return -EINVAL;
+				return -EPROBE_DEFER;
 
 			return _regulator_get_voltage(rdev->supply->rdev);
 		}
@@ -3943,8 +3943,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		rdev->dev.of_node = of_node_get(config->of_node);
 	}
 
-	mutex_lock(&regulator_list_mutex);
-
 	mutex_init(&rdev->mutex);
 	rdev->reg_data = config->driver_data;
 	rdev->owner = regulator_desc->owner;
@@ -3969,7 +3967,9 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 	if ((config->ena_gpio || config->ena_gpio_initialized) &&
 	    gpio_is_valid(config->ena_gpio)) {
+		mutex_lock(&regulator_list_mutex);
 		ret = regulator_ena_gpio_request(rdev, config);
+		mutex_unlock(&regulator_list_mutex);
 		if (ret != 0) {
 			rdev_err(rdev, "Failed to request enable GPIO%d: %d\n",
 				 config->ena_gpio, ret);
@@ -3987,31 +3987,40 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (init_data)
 		constraints = &init_data->constraints;
 
-	ret = set_machine_constraints(rdev, constraints);
-	if (ret < 0)
-		goto wash;
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
 		rdev->supply_name = regulator_desc->supply_name;
 
+	/*
+	 * Attempt to resolve the regulator supply, if specified,
+	 * but don't return an error if we fail because we will try
+	 * to resolve it again later as more regulators are added.
+	 */
+	if (regulator_resolve_supply(rdev))
+		rdev_dbg(rdev, "unable to resolve supply\n");
+
+	ret = set_machine_constraints(rdev, constraints);
+	if (ret < 0)
+		goto wash;
+
 	/* add consumers devices */
 	if (init_data) {
+		mutex_lock(&regulator_list_mutex);
 		for (i = 0; i < init_data->num_consumer_supplies; i++) {
 			ret = set_consumer_device_supply(rdev,
 				init_data->consumer_supplies[i].dev_name,
 				init_data->consumer_supplies[i].supply);
 			if (ret < 0) {
+				mutex_unlock(&regulator_list_mutex);
 				dev_err(dev, "Failed to set supply %s\n",
 					init_data->consumer_supplies[i].supply);
 				goto unset_supplies;
 			}
 		}
+		mutex_unlock(&regulator_list_mutex);
 	}
 
-	mutex_unlock(&regulator_list_mutex);
-
 	ret = device_register(&rdev->dev);
 	if (ret != 0) {
 		put_device(&rdev->dev);
@@ -4028,13 +4037,16 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	return rdev;
 
 unset_supplies:
+	mutex_lock(&regulator_list_mutex);
 	unset_regulator_supplies(rdev);
+	mutex_unlock(&regulator_list_mutex);
 wash:
 	kfree(rdev->constraints);
+	mutex_lock(&regulator_list_mutex);
 	regulator_ena_gpio_free(rdev);
+	mutex_unlock(&regulator_list_mutex);
 clean:
 	kfree(rdev);
-	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
 }
-- 
2.1.4