Re: [PATCH 1/5] regulator: core: Resolve supply earlier

[Jon Hunter]

On 11/04/16 15:16, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Mon, Apr 11, 2016 at 04:11:01PM +0200, Thierry Reding wrote:
>> On Mon, Apr 11, 2016 at 03:03:00PM +0100, Mark Brown wrote:
> 
>>> This shouldn't be a hard dependency: most regulators won't be in bypass
>>> mode or otherwise depend on their parents enough to need this.
> 
>> I had initially proposed to resolve the supply only when necessary
>> during regulator_get_voltage() when checking for bypass, perhaps that
>> would after all be more appropriate here?
> 
> Yes, that had been what I'd expected.

So the following seems to work, but only item I am uncertain about
is if it is ok to move the mutex_lock to after the
machine_set_constraints()?

Jon

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 61d3918f329e..742d10371e2d 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3126,8 +3126,13 @@ static int _regulator_get_voltage(struct regulator_dev 
*rdev)
                        return ret;
                if (bypassed) {
                        /* if bypassed the regulator must have a supply */
-                       if (!rdev->supply)
-                               return -EINVAL;
+                       if (!rdev->supply) {
+                               ret = regulator_resolve_supply(rdev);
+                               if (ret < 0)
+                                       return ret;
+                               if (!rdev->supply)
+                                       return -EINVAL;
+                       }
 
                        return _regulator_get_voltage(rdev->supply->rdev);
                }
@@ -3939,8 +3944,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;
@@ -3983,23 +3986,26 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
        if (init_data)
                constraints = &init_data->constraints;
 
+       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;
+
        ret = set_machine_constraints(rdev, constraints);
        if (ret < 0)
                goto wash;
 
+       mutex_lock(&regulator_list_mutex);
+
        ret = device_register(&rdev->dev);
        if (ret != 0) {
                put_device(&rdev->dev);
+               mutex_unlock(&regulator_list_mutex);
                goto wash;
        }
 
        dev_set_drvdata(&rdev->dev, rdev);
 
-       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;
-
        /* add consumers devices */
        if (init_data) {
                for (i = 0; i < init_data->num_consumer_supplies; i++) {
@@ -4009,6 +4015,7 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
                        if (ret < 0) {
                                dev_err(dev, "Failed to set supply %s\n",
                                        init_data->consumer_supplies[i].supply);
+                               mutex_unlock(&regulator_list_mutex);
                                goto unset_supplies;
                        }
                }
@@ -4036,7 +4043,6 @@ wash:
 clean:
        kfree(rdev);
 out:
-       mutex_unlock(&regulator_list_mutex);
        kfree(config);
        return ERR_PTR(ret);
 }