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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5716059B.3080503@nvidia.com>
Date:	Tue, 19 Apr 2016 11:16:59 +0100
From:	Jon Hunter <jonathanh@...dia.com>
To:	Mark Brown <broonie@...nel.org>,
	Thierry Reding <thierry.reding@...il.com>
CC:	Liam Girdwood <lgirdwood@...il.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/5] regulator: core: Resolve supply earlier


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);
 }