Dear All,
There are cases where particular ldo's aren't actually connected
to anything and currently there is no easy way of disabling them
if the pmic brings them up by default. I propose adding an always
off parameter to the constraints to allow a minimal way of specifying
this within board configs. I haven't submitted this as a formal
patch as it is currently based on top of the patch pushing
locks out of _notifier_call_chain which I posted yesterday.
If people are happy I can produce a version not dependent on that.
Problems I can see with this small change are:
1) interaction between always on and always off. Obviously it's
a bit odd if both are set. Currently I'm giving always on preference.
2) Doesn't really make sense to have regulators that are always
disabled available under sysfs etc. Perhaps there is a lower
level way of doing this that I'm missing? There is not easy
way of permanently saving this stuff on the da9030 that I'm
using and moving it into the bootloader would be a pain.
There's actually no problem with leaving these regs running
and unmanaged except for the small amount of power being wasted
and a desire for tidiness ;)
---
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 245eee1..d11b5cf 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -722,6 +722,16 @@ static int set_machine_constraints(struct regulator_dev *rdev,
rdev->constraints = NULL;
goto out;
}
+ } else if (constraints->always_off && ops->disable &&
+ ((ops->is_enabled && ops->is_enabled(rdev)) ||
+ (!ops->is_enabled && constraints->boot_on))) {
+ ret = ops->disable(rdev);
+ if (ret < 0) {
+ printk(KERN_ERR "%s: failed to disable %s\n",
+ __func__, name);
+ rdev->constraints = NULL;
+ goto out;
+ }
}
print_constraints(rdev);
@@ -1002,7 +1012,7 @@ static int _regulator_enable(struct regulator_dev *rdev)
}
/* check voltage and requested load before enabling */
- if (rdev->desc->ops->enable) {
+ if (rdev->desc->ops->enable && !rdev->constraints->always_off) {
if (rdev->constraints &&
(rdev->constraints->valid_ops_mask &
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 3794773..56bbb89 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -113,6 +113,7 @@ struct regulation_constraints {
/* constriant flags */
unsigned always_on:1; /* regulator never off when system is on */
+ unsigned always_off:1; /* regulator never on under any circumstances */
unsigned boot_on:1; /* bootloader/firmware enabled regulator */
unsigned apply_uV:1; /* apply uV constraint iff min == max */
};
On Tue, Jan 20, 2009 at 12:07:05PM +0000, Jonathan Cameron wrote:
> 1) interaction between always on and always off. Obviously it's
> a bit odd if both are set. Currently I'm giving always on preference.
I'd just complain loudly and refuse to accept the constraints if they
are clearly broken like that.
One thing I would suggest is a slightly weaker version which marks the
regulator to be turned off at boot rather than always off. That's
slightly more flexible and would achieve the same effect if the
constraints don't otherwise allow for turning the regulator on layer.
> 2) Doesn't really make sense to have regulators that are always
> disabled available under sysfs etc. Perhaps there is a lower
> level way of doing this that I'm missing? There is not easy
> way of permanently saving this stuff on the da9030 that I'm
> using and moving it into the bootloader would be a pain.
It doesn't do *that* much harm - if nothing else it makes it more
discoverable what's happening.
Mark Brown wrote:
> On Tue, Jan 20, 2009 at 12:07:05PM +0000, Jonathan Cameron wrote:
>
>> 1) interaction between always on and always off. Obviously it's
>> a bit odd if both are set. Currently I'm giving always on preference.
>
> I'd just complain loudly and refuse to accept the constraints if they
> are clearly broken like that.
Will do.
>
> One thing I would suggest is a slightly weaker version which marks the
> regulator to be turned off at boot rather than always off. That's
> slightly more flexible and would achieve the same effect if the
> constraints don't otherwise allow for turning the regulator on layer.
Good idea. That should just mean removing the code in the enable bit
(and renaming the variable!) I'll repost shortly.
>
>> 2) Doesn't really make sense to have regulators that are always
>> disabled available under sysfs etc. Perhaps there is a lower
>> level way of doing this that I'm missing? There is not easy
>> way of permanently saving this stuff on the da9030 that I'm
>> using and moving it into the bootloader would be a pain.
>
> It doesn't do *that* much harm - if nothing else it makes it more
> discoverable what's happening.
Good point.
Thanks for the comments,
Jonathan
From: Jonathan Cameron <[email protected]>
regulator: Add disable_on_boot flag to constraints.
Signed-off-by: Jonathan Cameron <[email protected]>
---
Changes in response to Mark Browns comments.
Patch changed to disable_on_boot for more flexibility.
This only really involved removing the check in the enable code
found in the previous patch.
This is intended to allow the disabling of regulators during
registration. Typical use case is regulators that aren't
actually connected to anything.
This is against a clean 2.6.29-rc2-git1 tree.
drivers/regulator/core.c | 19 ++++++++++++++++++-
include/linux/regulator/machine.h | 7 ++++---
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f511a40..7d57338 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -709,7 +709,14 @@ static int set_machine_constraints(struct
regulator_dev *rdev,
goto out;
}
}
-
+ /* Sanity check */
+ if (constraints->always_on && constraints->disable_on_boot) {
+ printk(KERN_ERR "%s: always on and disable at boot both set for
%s\n",
+ __func__, name);
+ rdev->constraints = NULL;
+ ret = -EINVAL;
+ goto out;
+ }
/* if always_on is set then turn the regulator on if it's not
* already on. */
if (constraints->always_on && ops->enable &&
@@ -722,6 +729,16 @@ static int set_machine_constraints(struct
regulator_dev *rdev,
rdev->constraints = NULL;
goto out;
}
+ } else if (constraints->disable_on_boot && ops->disable &&
+ ((ops->is_enabled && ops->is_enabled(rdev)) ||
+ (!ops->is_enabled && constraints->boot_on))) {
+ ret = ops->disable(rdev);
+ if (ret < 0) {
+ printk(KERN_ERR "%s: failed to disable %s\n",
+ __func__, name);
+ rdev->constraints = NULL;
+ goto out;
+ }
}
print_constraints(rdev);
diff --git a/include/linux/regulator/machine.h
b/include/linux/regulator/machine.h
index 3794773..fc0a641 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -112,10 +112,10 @@ struct regulation_constraints {
suspend_state_t initial_state; /* suspend state to set at init */
/* constriant flags */
- unsigned always_on:1; /* regulator never off when system is on */
- unsigned boot_on:1; /* bootloader/firmware enabled regulator */
- unsigned apply_uV:1; /* apply uV constraint iff min == max */
+ unsigned always_on:1; /* regulator never off when system on */
+ unsigned boot_on:1; /* bootloader/firmware enabled reg */
+ unsigned disable_on_boot:1; /* regulator to be disabled at boot */
+ unsigned apply_uV:1; /* apply uV constraint iff min == max */
};
/**
On Tue, 2009-01-20 at 13:19 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <[email protected]>
>
> regulator: Add disable_on_boot flag to constraints.
>
> Signed-off-by: Jonathan Cameron <[email protected]>
>
> ---
> Changes in response to Mark Browns comments.
>
Applying regulator: Add disable_on_boot flag to constraints.
fatal: corrupt patch at line 23
Patch failed at 0001.
Could you recreate the patch against regulator for-next.
Thanks
Liam
On Tue, Jan 20, 2009 at 01:19:44PM +0000, Jonathan Cameron wrote:
> index 3794773..fc0a641 100644
> --- a/include/linux/regulator/machine.h
> +++ b/include/linux/regulator/machine.h
> @@ -112,10 +112,10 @@ struct regulation_constraints {
> suspend_state_t initial_state; /* suspend state to set at init */
>
> /* constriant flags */
> - unsigned always_on:1; /* regulator never off when system is on */
> - unsigned boot_on:1; /* bootloader/firmware enabled regulator */
> - unsigned apply_uV:1; /* apply uV constraint iff min == max */
> + unsigned always_on:1; /* regulator never off when system on */
> + unsigned boot_on:1; /* bootloader/firmware enabled reg */
> + unsigned disable_on_boot:1; /* regulator to be disabled at boot */
> + unsigned apply_uV:1; /* apply uV constraint iff min == max */
> };
This needs kerneldoc adding for the new entry. Other than that
Acked-by: Mark Brown <[email protected]>
From: Jonathan Cameron <[email protected]>
regulator: Add disable_on_boot flag to constraints.
Signed-off-by: Jonathan Cameron <[email protected]>
Acked-by: Mark Brown <[email protected]>
---
Now based on voltage / for-next and with kernel doc
added.
Not quite sure what trashed the previous patch, but
hopefully this one should work.
Changes in response to Mark Browns comments.
Patch changed to disable_on_boot for more flexibility.
This only really involved removing the check in the enable code
found in the previous patch.
This is intended to allow the disabling of regulators during
registration. Typical use case is regulators that aren't
actually connected to anything.
This is against a clean voltage for-next with Mark's
hoisting of struct regulator_dev patch applied. (not that
it materially changes this patch).
drivers/regulator/core.c | 19 ++++++++++++++++++-
include/linux/regulator/machine.h | 9 ++++++---
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 0ed13c2..ea7caaf 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -723,7 +723,14 @@ static int set_machine_constraints(struct regulator_dev *rdev,
goto out;
}
}
-
+ /* Sanity check */
+ if (constraints->always_on && constraints->disable_on_boot) {
+ printk(KERN_ERR "%s: always on and disable at boot both set for %s\n",
+ __func__, name);
+ rdev->constraints = NULL;
+ ret = -EINVAL;
+ goto out;
+ }
/* if always_on is set then turn the regulator on if it's not
* already on. */
if (constraints->always_on && ops->enable &&
@@ -736,6 +743,16 @@ static int set_machine_constraints(struct regulator_dev *rdev,
rdev->constraints = NULL;
goto out;
}
+ } else if (constraints->disable_on_boot && ops->disable &&
+ ((ops->is_enabled && ops->is_enabled(rdev)) ||
+ (!ops->is_enabled && constraints->boot_on))) {
+ ret = ops->disable(rdev);
+ if (ret < 0) {
+ printk(KERN_ERR "%s: failed to disable %s\n",
+ __func__, name);
+ rdev->constraints = NULL;
+ goto out;
+ }
}
print_constraints(rdev);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 3794773..f7d01dd 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -74,6 +74,8 @@ struct regulator_state {
* @always_on: Set if the regulator should never be disabled.
* @boot_on: Set if the regulator is enabled when the system is initially
* started.
+ * @disable_on_boot: Set if the regulator should be disabled. Typically
+ * for regulators that are not connected.
* @apply_uV: Apply the voltage constraint when initialising.
*
* @input_uV: Input voltage for regulator when supplied by another regulator.
@@ -112,9 +114,10 @@ struct regulation_constraints {
suspend_state_t initial_state; /* suspend state to set at init */
/* constriant flags */
- unsigned always_on:1; /* regulator never off when system is on */
- unsigned boot_on:1; /* bootloader/firmware enabled regulator */
- unsigned apply_uV:1; /* apply uV constraint iff min == max */
+ unsigned always_on:1; /* regulator never off when system on */
+ unsigned boot_on:1; /* bootloader/firmware enabled reg */
+ unsigned disable_on_boot:1; /* regulator to be disabled at boot */
+ unsigned apply_uV:1; /* apply uV constraint iff min == max */
};
/**
Dear All,
I'd forgotten about this one until I tried to clean up a patch dependent
on it.
Any more comments / requested changes?
If anyone wants this rebased against a current tree just ask.
( I don't think it ended up in any relevant trees. Sorry if I've just
missed it!)
Thanks,
Jonathan
> From: Jonathan Cameron <[email protected]>
>
> regulator: Add disable_on_boot flag to constraints.
>
> Signed-off-by: Jonathan Cameron <[email protected]>
> Acked-by: Mark Brown <[email protected]>
> ---
> Now based on voltage / for-next and with kernel doc
> added.
>
> Not quite sure what trashed the previous patch, but
> hopefully this one should work.
>
> Changes in response to Mark Browns comments.
>
> Patch changed to disable_on_boot for more flexibility.
> This only really involved removing the check in the enable code
> found in the previous patch.
>
> This is intended to allow the disabling of regulators during
> registration. Typical use case is regulators that aren't
> actually connected to anything.
>
> This is against a clean voltage for-next with Mark's
> hoisting of struct regulator_dev patch applied. (not that
> it materially changes this patch).
>
> drivers/regulator/core.c | 19 ++++++++++++++++++-
> include/linux/regulator/machine.h | 9 ++++++---
> 2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 0ed13c2..ea7caaf 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -723,7 +723,14 @@ static int set_machine_constraints(struct regulator_dev *rdev,
> goto out;
> }
> }
> -
> + /* Sanity check */
> + if (constraints->always_on && constraints->disable_on_boot) {
> + printk(KERN_ERR "%s: always on and disable at boot both set for %s\n",
> + __func__, name);
> + rdev->constraints = NULL;
> + ret = -EINVAL;
> + goto out;
> + }
> /* if always_on is set then turn the regulator on if it's not
> * already on. */
> if (constraints->always_on && ops->enable &&
> @@ -736,6 +743,16 @@ static int set_machine_constraints(struct regulator_dev *rdev,
> rdev->constraints = NULL;
> goto out;
> }
> + } else if (constraints->disable_on_boot && ops->disable &&
> + ((ops->is_enabled && ops->is_enabled(rdev)) ||
> + (!ops->is_enabled && constraints->boot_on))) {
> + ret = ops->disable(rdev);
> + if (ret < 0) {
> + printk(KERN_ERR "%s: failed to disable %s\n",
> + __func__, name);
> + rdev->constraints = NULL;
> + goto out;
> + }
> }
>
> print_constraints(rdev);
> diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
> index 3794773..f7d01dd 100644
> --- a/include/linux/regulator/machine.h
> +++ b/include/linux/regulator/machine.h
> @@ -74,6 +74,8 @@ struct regulator_state {
> * @always_on: Set if the regulator should never be disabled.
> * @boot_on: Set if the regulator is enabled when the system is initially
> * started.
> + * @disable_on_boot: Set if the regulator should be disabled. Typically
> + * for regulators that are not connected.
> * @apply_uV: Apply the voltage constraint when initialising.
> *
> * @input_uV: Input voltage for regulator when supplied by another regulator.
> @@ -112,9 +114,10 @@ struct regulation_constraints {
> suspend_state_t initial_state; /* suspend state to set at init */
>
> /* constriant flags */
> - unsigned always_on:1; /* regulator never off when system is on */
> - unsigned boot_on:1; /* bootloader/firmware enabled regulator */
> - unsigned apply_uV:1; /* apply uV constraint iff min == max */
> + unsigned always_on:1; /* regulator never off when system on */
> + unsigned boot_on:1; /* bootloader/firmware enabled reg */
> + unsigned disable_on_boot:1; /* regulator to be disabled at boot */
> + unsigned apply_uV:1; /* apply uV constraint iff min == max */
> };
>
> /**
>
On Sun, Mar 29, 2009 at 07:25:56PM +0000, Jonathan Cameron wrote:
> Any more comments / requested changes?
> If anyone wants this rebased against a current tree just ask.
> ( I don't think it ended up in any relevant trees. Sorry if I've just
> missed it!)
I'm happy with it but it does need redoing against the current regulator
branch. Might be worth renaming the option to disable_immediately or
something, though.
On Mon, Mar 30, 2009 at 10:52:45AM +0100, Mark Brown wrote:
> I'm happy with it but it does need redoing against the current regulator
> branch. Might be worth renaming the option to disable_immediately or
> something, though.
BTW, note that the regulator API has gained the ability to power off
unused regulators in late_initcall() if the board requests this via
regulator_has_full_constraints() (which will become the default at some
point). The ability to turn off immediately when applying the
constraints would still be useful, though.