* [PATCH] regulator: core: Fix the init of DT defined fixed regulators
@ 2014-05-19 11:12 Alban Bedel
2014-05-19 14:39 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Alban Bedel @ 2014-05-19 11:12 UTC (permalink / raw)
To: linux-kernel; +Cc: Mark Brown, Liam Girdwood, Alban Bedel
When a regulator is defined using DT and it has a single voltage the
regulator init always tries to apply this voltage. However this fails
if the regulator isn't settable. So skip this step if the regulator
doesn't provides any set method.
Signed-off-by: Alban Bedel <alban.bedel@avionic-design.de>
---
drivers/regulator/core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 9a09f3c..e57450f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -843,7 +843,9 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
/* do we need to apply the constraint voltage */
if (rdev->constraints->apply_uV &&
- rdev->constraints->min_uV == rdev->constraints->max_uV) {
+ rdev->constraints->min_uV == rdev->constraints->max_uV &&
+ (rdev->desc->ops->set_voltage ||
+ rdev->desc->ops->set_voltage_sel)) {
ret = _regulator_do_set_voltage(rdev,
rdev->constraints->min_uV,
rdev->constraints->max_uV);
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] regulator: core: Fix the init of DT defined fixed regulators
2014-05-19 11:12 [PATCH] regulator: core: Fix the init of DT defined fixed regulators Alban Bedel
@ 2014-05-19 14:39 ` Mark Brown
2014-05-19 15:25 ` Alban Bedel
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2014-05-19 14:39 UTC (permalink / raw)
To: Alban Bedel; +Cc: linux-kernel, Liam Girdwood
[-- Attachment #1: Type: text/plain, Size: 485 bytes --]
On Mon, May 19, 2014 at 01:12:34PM +0200, Alban Bedel wrote:
> When a regulator is defined using DT and it has a single voltage the
> regulator init always tries to apply this voltage. However this fails
> if the regulator isn't settable. So skip this step if the regulator
> doesn't provides any set method.
No, this means we'll just ignore the voltage someone tried to set. A
bigger question is why someone is trying to configure the voltage of a
fixed voltage regulator in DT...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: core: Fix the init of DT defined fixed regulators
2014-05-19 14:39 ` Mark Brown
@ 2014-05-19 15:25 ` Alban Bedel
2014-05-19 16:18 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Alban Bedel @ 2014-05-19 15:25 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Liam Girdwood
[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]
On Mon, 19 May 2014 15:39:49 +0100
Mark Brown <broonie@kernel.org> wrote:
> On Mon, May 19, 2014 at 01:12:34PM +0200, Alban Bedel wrote:
>
> > When a regulator is defined using DT and it has a single voltage the
> > regulator init always tries to apply this voltage. However this fails
> > if the regulator isn't settable. So skip this step if the regulator
> > doesn't provides any set method.
>
> No, this means we'll just ignore the voltage someone tried to set. A
> bigger question is why someone is trying to configure the voltage of a
> fixed voltage regulator in DT...
We have a platform where a TPS658621 has been replaced by a TPS658640
in newer version of the hardware. One of the few difference between both
regulator is that one output is now fixed. One could write a new DT that
doesn't "set" this output. But IMHO the current DT should still be usable
as the voltage declared for the now fixed regulator is still correct.
Would checking that the regulator is at the voltage declared in the DT
better? Or should I just add a pseudo set method to the regulator driver
that only accept the current voltage? That could also be implemented at
the core level using a default method for all regulators that have a get
but no set method.
Alban
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: core: Fix the init of DT defined fixed regulators
2014-05-19 15:25 ` Alban Bedel
@ 2014-05-19 16:18 ` Mark Brown
2014-05-19 16:37 ` Alban Bedel
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2014-05-19 16:18 UTC (permalink / raw)
To: Alban Bedel; +Cc: linux-kernel, Liam Girdwood
[-- Attachment #1: Type: text/plain, Size: 660 bytes --]
On Mon, May 19, 2014 at 05:25:56PM +0200, Alban Bedel wrote:
> Would checking that the regulator is at the voltage declared in the DT
> better? Or should I just add a pseudo set method to the regulator driver
> that only accept the current voltage? That could also be implemented at
> the core level using a default method for all regulators that have a get
> but no set method.
We already have code in the core to accept set_voltage() on fixed
voltage regulators - are you sure you're working with current code?
Otherwise fixing things to go through the standard set_voltage() path
would be OK. There should be no need to add a dummy function for
setting.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: core: Fix the init of DT defined fixed regulators
2014-05-19 16:18 ` Mark Brown
@ 2014-05-19 16:37 ` Alban Bedel
2014-05-19 16:55 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Alban Bedel @ 2014-05-19 16:37 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Liam Girdwood
[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]
On Mon, 19 May 2014 17:18:38 +0100
Mark Brown <broonie@kernel.org> wrote:
> On Mon, May 19, 2014 at 05:25:56PM +0200, Alban Bedel wrote:
>
> > Would checking that the regulator is at the voltage declared in the DT
> > better? Or should I just add a pseudo set method to the regulator driver
> > that only accept the current voltage? That could also be implemented at
> > the core level using a default method for all regulators that have a get
> > but no set method.
>
> We already have code in the core to accept set_voltage() on fixed
> voltage regulators - are you sure you're working with current code?
> Otherwise fixing things to go through the standard set_voltage() path
> would be OK. There should be no need to add a dummy function for
> setting.
The regulator_set_voltage() function do have code to handle such
read-only regulators. However the regulator init directly call the low
level function _regulator_do_set_voltage() which just error out when no
set method is available, hence my original patch.
Unless there is some better proposal, or objections, I'll submit a new
patch tomorrow to call _regulator_do_set_voltage() only if the current
voltage isn't correct.
Alban
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: core: Fix the init of DT defined fixed regulators
2014-05-19 16:37 ` Alban Bedel
@ 2014-05-19 16:55 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-05-19 16:55 UTC (permalink / raw)
To: Alban Bedel; +Cc: linux-kernel, Liam Girdwood
[-- Attachment #1: Type: text/plain, Size: 267 bytes --]
On Mon, May 19, 2014 at 06:37:49PM +0200, Alban Bedel wrote:
> Unless there is some better proposal, or objections, I'll submit a new
> patch tomorrow to call _regulator_do_set_voltage() only if the current
> voltage isn't correct.
Right, that's the best solution.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-19 16:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-19 11:12 [PATCH] regulator: core: Fix the init of DT defined fixed regulators Alban Bedel
2014-05-19 14:39 ` Mark Brown
2014-05-19 15:25 ` Alban Bedel
2014-05-19 16:18 ` Mark Brown
2014-05-19 16:37 ` Alban Bedel
2014-05-19 16:55 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox