diff mbox

regulator: twl: Add 'fixed' set_voltage callback

Message ID 1308862014-11212-1-git-send-email-jaswinder.singh@linaro.org (mailing list archive)
State Awaiting Upstream, archived
Delegated to: Tony Lindgren
Headers show

Commit Message

Jassi Brar June 23, 2011, 8:46 p.m. UTC
Define dummy set_voltage callback for fixed lines,
without which voltage constraints fail to apply.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 drivers/regulator/twl-regulator.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

Comments

Mark Brown June 24, 2011, 11:19 a.m. UTC | #1
On Fri, Jun 24, 2011 at 02:16:54AM +0530, Jassi Brar wrote:
> Define dummy set_voltage callback for fixed lines,
> without which voltage constraints fail to apply.
> 
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

No, this isn't sensible - fixed voltage regulators aren't an unusual
property of this device, they've very common.  The core should handle
this gracefully.  Probably whatever consumer is doing the setting also
ought to be updated to check that the voltages it wants to set are
supported at startup since otherwise it can end up repeatedly trying to
set something that'll never work.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jassi Brar June 24, 2011, 1:06 p.m. UTC | #2
On 24 June 2011 16:49, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Jun 24, 2011 at 02:16:54AM +0530, Jassi Brar wrote:
>> Define dummy set_voltage callback for fixed lines,
>> without which voltage constraints fail to apply.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>
> No, this isn't sensible - fixed voltage regulators aren't an unusual
> property of this device, they've very common.  The core should handle
> this gracefully.  Probably whatever consumer is doing the setting also
> ought to be updated to check that the voltages it wants to set are
> supported at startup since

Well, there's no consumer in the picture yet. The core even fails to register
the regulator.
After digging deeper, I realized removing
       .apply_uV = true;
from the constraints is the correct solution. Patch is coming next.

> otherwise it can end up repeatedly trying to
> set something that'll never work.
Though the semantic could be defined that the 'closest' possible value
is already
set after the set_{voltage,current_limit} returns.
The consumer has no point in retrying - just like sample-rate setting
in Alsa works.

Thanks,
Jassi
diff mbox

Patch

diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 87fe0f7..8223e49 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -591,10 +591,18 @@  static int twlfixed_get_voltage(struct regulator_dev *rdev)
 	return info->min_mV * 1000;
 }
 
+static int twlfixed_set_voltage(struct regulator_dev *rdev,
+	int min_uV, int max_uV, unsigned *selector)
+{
+	*selector = min_uV / 1000;
+	return 0;
+}
+
 static struct regulator_ops twl4030fixed_ops = {
 	.list_voltage	= twlfixed_list_voltage,
 
 	.get_voltage	= twlfixed_get_voltage,
+	.set_voltage	= twlfixed_set_voltage,
 
 	.enable		= twl4030reg_enable,
 	.disable	= twl4030reg_disable,
@@ -609,6 +617,7 @@  static struct regulator_ops twl6030fixed_ops = {
 	.list_voltage	= twlfixed_list_voltage,
 
 	.get_voltage	= twlfixed_get_voltage,
+	.set_voltage	= twlfixed_set_voltage,
 
 	.enable		= twl6030reg_enable,
 	.disable	= twl6030reg_disable,