* [PATCH] regulator: core: Use parent voltage from the supply when bypassed @ 2016-03-30 17:32 Mark Brown 2016-03-31 14:27 ` Jon Hunter 0 siblings, 1 reply; 6+ messages in thread From: Mark Brown @ 2016-03-30 17:32 UTC (permalink / raw) To: Jon Hunter, Liam Girdwood; +Cc: linux-kernel, Mark Brown When a regulator is in bypass mode it is functioning as a switch returning the voltage set in the regulator will not give the voltage being output by the regulator as it's just passing through its supply. This means that when we are getting the voltage from a regulator we should check to see if it is in bypass mode and if it is we should report the voltage from the supply rather than that which is set on the regulator. Reported-by: Jon Hunter <jonathanh@nvidia.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- Completely untested. drivers/regulator/core.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 74e8a7a3b3e8..03042e450399 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -3118,6 +3118,20 @@ EXPORT_SYMBOL_GPL(regulator_sync_voltage); static int _regulator_get_voltage(struct regulator_dev *rdev) { int sel, ret; + bool bypassed; + + if (rdev->desc->ops->get_bypass) { + ret = rdev->desc->ops->get_bypass(rdev, &bypassed); + if (ret < 0) + return ret; + if (bypassed) { + if (rdev->supply) { + ret = _regulator_get_voltage(rdev->supply->rdev); + } else { + return -EINVAL; + } + } + } if (rdev->desc->ops->get_voltage_sel) { sel = rdev->desc->ops->get_voltage_sel(rdev); -- 2.8.0.rc3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: core: Use parent voltage from the supply when bypassed 2016-03-30 17:32 [PATCH] regulator: core: Use parent voltage from the supply when bypassed Mark Brown @ 2016-03-31 14:27 ` Jon Hunter 0 siblings, 0 replies; 6+ messages in thread From: Jon Hunter @ 2016-03-31 14:27 UTC (permalink / raw) To: Mark Brown, Liam Girdwood; +Cc: linux-kernel, linux-tegra On 30/03/16 18:32, Mark Brown wrote: > When a regulator is in bypass mode it is functioning as a switch > returning the voltage set in the regulator will not give the voltage > being output by the regulator as it's just passing through its supply. > This means that when we are getting the voltage from a regulator we > should check to see if it is in bypass mode and if it is we should > report the voltage from the supply rather than that which is set on the > regulator. > > Reported-by: Jon Hunter <jonathanh@nvidia.com> > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > > Completely untested. > > drivers/regulator/core.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index 74e8a7a3b3e8..03042e450399 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -3118,6 +3118,20 @@ EXPORT_SYMBOL_GPL(regulator_sync_voltage); > static int _regulator_get_voltage(struct regulator_dev *rdev) > { > int sel, ret; > + bool bypassed; > + > + if (rdev->desc->ops->get_bypass) { > + ret = rdev->desc->ops->get_bypass(rdev, &bypassed); > + if (ret < 0) > + return ret; > + if (bypassed) { > + if (rdev->supply) { > + ret = _regulator_get_voltage(rdev->supply->rdev); Should this be a return here? > + } else { > + return -EINVAL; > + } > + } > + } > > if (rdev->desc->ops->get_voltage_sel) { > sel = rdev->desc->ops->get_voltage_sel(rdev); > 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(). Jon ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: core: Use parent voltage from the supply when bypassed @ 2016-03-31 14:27 ` Jon Hunter 0 siblings, 0 replies; 6+ messages in thread From: Jon Hunter @ 2016-03-31 14:27 UTC (permalink / raw) To: Mark Brown, Liam Girdwood; +Cc: linux-kernel, linux-tegra On 30/03/16 18:32, Mark Brown wrote: > When a regulator is in bypass mode it is functioning as a switch > returning the voltage set in the regulator will not give the voltage > being output by the regulator as it's just passing through its supply. > This means that when we are getting the voltage from a regulator we > should check to see if it is in bypass mode and if it is we should > report the voltage from the supply rather than that which is set on the > regulator. > > Reported-by: Jon Hunter <jonathanh@nvidia.com> > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > > Completely untested. > > drivers/regulator/core.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index 74e8a7a3b3e8..03042e450399 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -3118,6 +3118,20 @@ EXPORT_SYMBOL_GPL(regulator_sync_voltage); > static int _regulator_get_voltage(struct regulator_dev *rdev) > { > int sel, ret; > + bool bypassed; > + > + if (rdev->desc->ops->get_bypass) { > + ret = rdev->desc->ops->get_bypass(rdev, &bypassed); > + if (ret < 0) > + return ret; > + if (bypassed) { > + if (rdev->supply) { > + ret = _regulator_get_voltage(rdev->supply->rdev); Should this be a return here? > + } else { > + return -EINVAL; > + } > + } > + } > > if (rdev->desc->ops->get_voltage_sel) { > sel = rdev->desc->ops->get_voltage_sel(rdev); > 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(). Jon ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <56FD33EE.6030109-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH] regulator: core: Use parent voltage from the supply when bypassed 2016-03-31 14:27 ` Jon Hunter @ 2016-03-31 17:55 ` Mark Brown -1 siblings, 0 replies; 6+ messages in thread From: Mark Brown @ 2016-03-31 17:55 UTC (permalink / raw) To: Jon Hunter Cc: Liam Girdwood, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 813 bytes --] 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. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: core: Use parent voltage from the supply when bypassed @ 2016-03-31 17:55 ` Mark Brown 0 siblings, 0 replies; 6+ messages in thread From: Mark Brown @ 2016-03-31 17:55 UTC (permalink / raw) To: Jon Hunter; +Cc: Liam Girdwood, linux-kernel, linux-tegra [-- Attachment #1: Type: text/plain, Size: 813 bytes --] 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. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: core: Use parent voltage from the supply when bypassed 2016-03-31 17:55 ` Mark Brown (?) @ 2016-04-01 16:58 ` Thierry Reding -1 siblings, 0 replies; 6+ messages in thread From: Thierry Reding @ 2016-04-01 16:58 UTC (permalink / raw) To: Mark Brown; +Cc: Jon Hunter, Liam Girdwood, linux-kernel, linux-tegra [-- Attachment #1: Type: text/plain, Size: 3437 bytes --] 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-01 16:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-30 17:32 [PATCH] regulator: core: Use parent voltage from the supply when bypassed Mark Brown
2016-03-31 14:27 ` Jon Hunter
2016-03-31 14:27 ` Jon Hunter
[not found] ` <56FD33EE.6030109-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-03-31 17:55 ` Mark Brown
2016-03-31 17:55 ` Mark Brown
2016-04-01 16:58 ` Thierry Reding
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.