Re: [PATCH] regulator: core: Use parent voltage from the supply when bypassed

[Thierry Reding]

On Thu, Mar 31, 2016 at 10:55:27AM -0700, Mark Brown wrote:
> On Thu, Mar 31, 2016 at 03:27:58PM +0100, Jon Hunter wrote:
> > On 30/03/16 18:32, Mark Brown wrote:
> 
> > > +		if (bypassed) {
> > > +			if (rdev->supply) {
> > > +				ret = _regulator_get_voltage(rdev->supply->rdev);
> 
> > Should this be a return here?
> 
> Gnargh, yes.
> 
> > I gave this a quick test on tegra124 having populated the
> > get/set_bypass() operators for the as3722. In this case, there is still
> > a problem because _regulator_get_voltage() is called during regulator
> > registration when set_machine_constraints() is called, which is before
> > we have called regulator_register_resolve_supply(). Therefore, it seems
> > to me that we still need to resolve the supply before we call
> > set_machine_constraints().
> 
> OK, we need to change the code there to defer probe in the case where
> the device is in bypass mode as well.

The following on top of your patch here and Jon's two patches to add
bypass support for the LDO6 on AS3722 make Jetson TK1 boot fine for me
again.

I can turn this into a proper commit if you want, or perhaps it might be
worth splitting this into several patches, one rearranging the supply
resolution vs. constraints application and another fixing up the bypass
patch you sent earlier (probably best to fold it into your original).

Thierry

--- >8 ---
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 6e5085034b77..23c8c4c86389 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3125,11 +3125,11 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 		if (ret < 0)
 			return ret;
 		if (bypassed) {
-			if (rdev->supply) {
-				ret = _regulator_get_voltage(rdev->supply->rdev);
-			} else {
+			/* if bypassed the regulator must have a supply */
+			if (!rdev->supply)
 				return -EINVAL;
-			}
+
+			return _regulator_get_voltage(rdev->supply->rdev);
 		}
 	}
 
@@ -3986,18 +3986,27 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 	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;
+
+	/*
+	 * set_machine_constraints() needs the supply to be resolved in order
+	 * to support querying the current voltage in bypass mode. Resolve it
+	 * here to more easily handle deferred probing.
+	 */
+	ret = regulator_resolve_supply(rdev);
+	if (ret < 0)
+		goto scrub;
+
 	/* set regulator constraints */
 	if (init_data)
 		constraints = &init_data->constraints;
 
 	ret = set_machine_constraints(rdev, constraints);
 	if (ret < 0)
-		goto scrub;
-
-	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;
+		goto tumble;
 
 	/* add consumers devices */
 	if (init_data) {
@@ -4024,7 +4033,13 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 unset_supplies:
 	unset_regulator_supplies(rdev);
+tumble:
+	if (rdev->supply) {
+		if (_regulator_is_enabled(rdev))
+			regulator_disable(rdev->supply);
 
+		_regulator_put(rdev->supply);
+	}
 scrub:
 	regulator_ena_gpio_free(rdev);
 	device_unregister(&rdev->dev);

Attachment: signature.asc
Description: PGP signature