*** BLURB HERE ***
Peter 'p2' De Schrijver (1):
Activate VDD1, VDD2 and VPLL1 at startup
drivers/regulator/twl4030-regulator.c | 18 +++++++++++++++++-
1 files changed, 17 insertions(+), 1 deletions(-)
This patch activates VDD1, VDD2 and VPLL1 when booting. This is necessary
because these resources are in warm reset state after a reboot. This means
their voltage levels cannot be modified so DVFS and smartreflex don't work.
Signed-off-by: Peter 'p2' De Schrijver <[email protected]>
---
drivers/regulator/twl4030-regulator.c | 18 +++++++++++++++++-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/drivers/regulator/twl4030-regulator.c b/drivers/regulator/twl4030-regulator.c
index 80a4e10..ab2a726 100644
--- a/drivers/regulator/twl4030-regulator.c
+++ b/drivers/regulator/twl4030-regulator.c
@@ -506,6 +506,22 @@ static int twl4030reg_probe(struct platform_device *pdev)
}
platform_set_drvdata(pdev, rdev);
+ /* VDD1, VDD2 and VPLL1 are left in warm reset state after a reboot.
+ * We need to put them back to active state for DVFS and smartreflex.
+ */
+
+ if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VDD1,
+ RES_STATE_ACTIVE)) < 0)
+ pr_err("Unable to activate VDD1\n");
+
+ if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VDD2,
+ RES_STATE_ACTIVE)) < 0)
+ pr_err("Unable to activate VDD2\n");
+
+ if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VPLL1,
+ RES_STATE_ACTIVE)) < 0)
+ pr_err("Unable to activate VPLL1\n");
+
/* NOTE: many regulators support short-circuit IRQs (presentable
* as REGULATOR_OVER_CURRENT notifications?) configured via:
* - SC_CONFIG
--
1.5.6.3
On Thu, Apr 23, 2009 at 04:10:08PM +0300, Peter 'p2' De Schrijver wrote:
> This patch activates VDD1, VDD2 and VPLL1 when booting. This is necessary
> because these resources are in warm reset state after a reboot. This means
> their voltage levels cannot be modified so DVFS and smartreflex don't work.
> Signed-off-by: Peter 'p2' De Schrijver <[email protected]>
Adding Liam again.
> ---
> drivers/regulator/twl4030-regulator.c | 18 +++++++++++++++++-
> 1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/regulator/twl4030-regulator.c b/drivers/regulator/twl4030-regulator.c
> index 80a4e10..ab2a726 100644
> --- a/drivers/regulator/twl4030-regulator.c
> +++ b/drivers/regulator/twl4030-regulator.c
> @@ -506,6 +506,22 @@ static int twl4030reg_probe(struct platform_device *pdev)
> }
> platform_set_drvdata(pdev, rdev);
>
> + /* VDD1, VDD2 and VPLL1 are left in warm reset state after a reboot.
> + * We need to put them back to active state for DVFS and smartreflex.
> + */
> +
> + if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VDD1,
> + RES_STATE_ACTIVE)) < 0)
> + pr_err("Unable to activate VDD1\n");
> +
> + if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VDD2,
> + RES_STATE_ACTIVE)) < 0)
> + pr_err("Unable to activate VDD2\n");
> +
> + if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VPLL1,
> + RES_STATE_ACTIVE)) < 0)
> + pr_err("Unable to activate VPLL1\n");
> +
> /* NOTE: many regulators support short-circuit IRQs (presentable
> * as REGULATOR_OVER_CURRENT notifications?) configured via:
> * - SC_CONFIG
> --
> 1.5.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
"You grabbed my hand and we fell into it, like a daydream - or a fever."
On Thursday 23 April 2009, Peter 'p2' De Schrijver wrote:
> This patch activates VDD1, VDD2 and VPLL1 when booting. This is necessary
> because these resources are in warm reset state after a reboot.
Warm reset state? I thought there were only ACTIVE, SLEEP, and OFF
states for individual resources. Do you mean SLEEP? And if so, is
this not something that should be dealt with by the power script
which runs inside the twl4030 when it handles the warm reset event?
I thought those regulators couldn't provide enough power from SLEEP
state to let the system boot. So being able to run this code means
they're not in SLEEP state...
Please explain. (Remembering that most of us haven't really looked
in much detail at this level of reset even handling!)
> This means
> their voltage levels cannot be modified so DVFS and smartreflex don't work.
Three thoughts on this:
- These three switching regulators are currently ignored by this
driver, because I've expected them to be managed as part of the
DVFS framework. (Mostly via the hardware SmartReflex support.
They don't seem particularly geared for software control.)
It seems odd to add these hooks for regulators that are otherwise
consciously ignored by this driver, and not software-controlled...
- This *could* be done with the twl4030-power.c (nyet in mainline)
resource_config hooks. But those hooks are board-specific (and
nyet in mainline), while these should "always" be done.
Should we maybe have all those power resource init hooks done
by the twl4030_core.c code? So as to work even if regulator
and power (script) drivers aren't present.
- The policy you described is specific to OMAP3 ... so shouldn't
these changes be conditionalized so they only kick in on OMAP3
based platforms? (Just as code-cleanliness for now; no other
platform yet uses these PMIC solutions, that I've heard of.)
And if they only matter for DVFS + SmartReflex ... should they
maybe be conditionalized for those, too? (Minor point at best;
it "shouldn't" hurt to do this at other times too.) Or maybe
even put into a twl4030-smartreflex.c driver, if there'd be
much for that to do. Setting FLOOR and CEILING voltages and
other stuff in section 5.5.1 of the tps65950 manual (step 4),
for example.
Having this in twl4030-core.c would affect the patch you sent to
move the "send PowerBus message" logic to its own function; that
would need to be in twl4030_core too.
- Dave
>
> Signed-off-by: Peter 'p2' De Schrijver <[email protected]>
> ---
> drivers/regulator/twl4030-regulator.c | 18 +++++++++++++++++-
> 1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/regulator/twl4030-regulator.c b/drivers/regulator/twl4030-regulator.c
> index 80a4e10..ab2a726 100644
> --- a/drivers/regulator/twl4030-regulator.c
> +++ b/drivers/regulator/twl4030-regulator.c
> @@ -506,6 +506,22 @@ static int twl4030reg_probe(struct platform_device *pdev)
> }
> platform_set_drvdata(pdev, rdev);
>
> + /* VDD1, VDD2 and VPLL1 are left in warm reset state after a reboot.
> + * We need to put them back to active state for DVFS and smartreflex.
> + */
> +
> + if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VDD1,
> + RES_STATE_ACTIVE)) < 0)
> + pr_err("Unable to activate VDD1\n");
> +
> + if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VDD2,
> + RES_STATE_ACTIVE)) < 0)
> + pr_err("Unable to activate VDD2\n");
> +
> + if (twl4030_send_pb_msg(MSG_SINGULAR(DEV_GRP_P1, RES_VPLL1,
> + RES_STATE_ACTIVE)) < 0)
> + pr_err("Unable to activate VPLL1\n");
> +
> /* NOTE: many regulators support short-circuit IRQs (presentable
> * as REGULATOR_OVER_CURRENT notifications?) configured via:
> * - SC_CONFIG
> --
> 1.5.6.3
>
>
On Thu, Apr 23, 2009 at 11:53:47PM +0200, ext David Brownell wrote:
> On Thursday 23 April 2009, Peter 'p2' De Schrijver wrote:
> > This patch activates VDD1, VDD2 and VPLL1 when booting. This is necessary
> > because these resources are in warm reset state after a reboot.
>
> Warm reset state? I thought there were only ACTIVE, SLEEP, and OFF
> states for individual resources. Do you mean SLEEP? And if so, is
> this not something that should be dealt with by the power script
> which runs inside the twl4030 when it handles the warm reset event?
>
No. I don't mean SLEEP. WARM-RESET state means the output level has its
default value. I think this is used to prevent any DVFS or smartreflex
related changes during warm reset. At least the TRM suggests to do this
from the 'software'. I will ask my contact at TI why they suggest to do
it this way.
> I thought those regulators couldn't provide enough power from SLEEP
> state to let the system boot. So being able to run this code means
> they're not in SLEEP state...
>
That's correct.
> Please explain. (Remembering that most of us haven't really looked
> in much detail at this level of reset even handling!)
>
I haven't read the public TRM on this part, so I don't know how much of
this is undocumented for you.
>
> > This means
> > their voltage levels cannot be modified so DVFS and smartreflex don't work.
>
> Three thoughts on this:
>
> - These three switching regulators are currently ignored by this
> driver, because I've expected them to be managed as part of the
> DVFS framework. (Mostly via the hardware SmartReflex support.
> They don't seem particularly geared for software control.)
>
VPLL1 is not managed by DVFS or smartreflex. OTOH it needs to be on all
the time, so there is no point in controlling them from software I
guess.
> It seems odd to add these hooks for regulators that are otherwise
> consciously ignored by this driver, and not software-controlled...
>
> - This *could* be done with the twl4030-power.c (nyet in mainline)
> resource_config hooks. But those hooks are board-specific (and
> nyet in mainline), while these should "always" be done.
>
Indeed. At least that's my understanding now.
> Should we maybe have all those power resource init hooks done
> by the twl4030_core.c code? So as to work even if regulator
> and power (script) drivers aren't present.
>
You mean moving this to twl4030_core.c ?
> - The policy you described is specific to OMAP3 ... so shouldn't
> these changes be conditionalized so they only kick in on OMAP3
> based platforms? (Just as code-cleanliness for now; no other
> platform yet uses these PMIC solutions, that I've heard of.)
>
I don't really know. Even on non OMAP3 platforms, I would expect VDD1
and VDD2 to control core voltages, as those are the ones which you can
easily use for DVFS and they are SMPS. I don't really see why you would
use TWL4030 (and cope with its complexity) if you don't want to make use
of these features.
> And if they only matter for DVFS + SmartReflex ... should they
> maybe be conditionalized for those, too? (Minor point at best;
> it "shouldn't" hurt to do this at other times too.) Or maybe
> even put into a twl4030-smartreflex.c driver, if there'd be
> much for that to do. Setting FLOOR and CEILING voltages and
> other stuff in section 5.5.1 of the tps65950 manual (step 4),
> for example.
>
> Having this in twl4030-core.c would affect the patch you sent to
> move the "send PowerBus message" logic to its own function; that
> would need to be in twl4030_core too.
>
I think that might be a good idea anyway. It seems sending these
powerbus messages needs to be done more often then we expected when
initially writing the twl4030 code.
Cheers,
Peter.
--
goa is a state of mind