Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
* Tony Lindgren t...@atomide.com [140423 13:49]:
* Tero Kristo t-kri...@ti.com [140423 00:51]:
On 04/12/2014 06:02 PM, Tony Lindgren wrote:
* Tero Kristo t-kri...@ti.com [140412 02:01]:
On 04/11/2014 02:47 AM, Tony Lindgren wrote:
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+omap3_vc_set_pmic_signaling(core_next_state);
if (core_next_state == PWRDM_POWER_OFF) {
omap3_core_save_context();
omap3_cm_save_context();
I think this implementation is sub-optimal, as it only scales
voltages if core is entering idle state. MPU only idle is possible,
however you do not scale voltages in this case.
Hmm that's same as PWRDM_POWER_RET? That's working too.
Or do you have something else in mind that I'm not aware
of?
No, I mean the case where core_next_state == PWRDM_POWER_ON, but
mpu_next_state = PWRDM_POWER_RET. You could scale MPU voltage in this case
but you don't with this implementation.
OK makes sense to pass both mpu_next_state and core_next_state
to the function then.
I've changed things around to implement the register caching
Grazvydas suggested, and calling omap3_vc_set_pmic_signaling()
always omap_sram_idle(). The default signaling is to use
I2C4 control except for core-off that uses the sys_off_mode,
so there should not be any blockers for core-on + mpu-off.
And after playing with enable_off_mode I'm seeing at least
all cpuidle states being hit.
# cat /sys/devices/system/cpu/cpu0/cpuidle/state*/usage
603
7378
7087
342
17
1
45
OK, sounds like you already have a patch for that in your
3.14-rc4-cm-prm-driver-wip branch?
Yes, there is a patch for that.
OK I'll pick it from there.
I ended up doing the read/write in a slightly different way as
we can pass the pwrdm to the init function.
Regards,
Tony
8
From: Tony Lindgren t...@atomide.com
Date: Mon, 5 May 2014 17:27:35 -0700
Subject: [PATCH] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and
sys_off_mode
While debugging legacy mode vs device tree booted PM regressions,
I noticed that omap3 is not toggling sys_clkreq and sys_off_mode
pins like it should.
The sys_clkreq and sys_off_mode pins are not toggling because of
the following issues:
1. The default polarity for the sys_off_mode pin is wrong.
OFFMODE_POL needs to be cleared for sys_off_mode to go down when
hitting off-idle, while CLKREQ_POL needs to be set so sys_clkreq
goes down when hitting retention.
2. The values for voltctrl register need to be updated dynamically.
We need to set either the retention idle bits, or off idle bits
in the voltctrl register depending the idle mode we're targeting
to hit.
Let's fix these two issues as otherwise the system will just
hang if any twl4030 PMIC idle scripts are loaded. The only case
where the system does not hang is if only retention idle over I2C4
is configured by the bootloader.
Note that even without the twl4030 PMIC scripts, these fixes will
do the proper signaling of sys_clkreq and sys_off_mode pins, so
the fixes are needed to fix monitoring of PM states with LEDs or
an oscilloscope.
Cc: Kevin Hilman khil...@linaro.org
Cc: Nishanth Menon n...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Tero Kristo t-kri...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 87099bb..3c8bb8f 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -50,6 +50,7 @@
#include sdrc.h
#include sram.h
#include control.h
+#include vc.h
/* pm34xx errata defined in pm.h */
u16 pm34xx_errata;
@@ -288,6 +289,9 @@ void omap_sram_idle(void)
}
}
+ /* Configure PMIC signaling for I2C4 or sys_off_mode */
+ omap3_vc_set_pmic_signaling(core_next_state);
+
omap3_intc_prepare_idle();
/*
diff --git a/arch/arm/mach-omap2/prm-regbits-34xx.h
b/arch/arm/mach-omap2/prm-regbits-34xx.h
index cebad56..106132d 100644
--- a/arch/arm/mach-omap2/prm-regbits-34xx.h
+++ b/arch/arm/mach-omap2/prm-regbits-34xx.h
@@ -123,8 +123,15 @@
#define OMAP3430_GLOBAL_SW_RST_SHIFT 1
#define OMAP3430_GLOBAL_COLD_RST_SHIFT 0
#define OMAP3430_GLOBAL_COLD_RST_MASK (1 0)
-#define OMAP3430_SEL_OFF_MASK (1 3)
-#define OMAP3430_AUTO_OFF_MASK (1 2)
+#define OMAP3430_PRM_VOLTCTRL_SEL_VMODE(1 4)
+#define OMAP3430_PRM_VOLTCTRL_SEL_OFF (1 3)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_OFF (1 2)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_RET (1 1)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP (1 0)
#define OMAP3430_SETUP_TIME2_MASK (0x 16)
#define OMAP3430_SETUP_TIME1_MASK
Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
On 04/12/2014 06:02 PM, Tony Lindgren wrote:
* Tero Kristo t-kri...@ti.com [140412 02:01]:
On 04/11/2014 02:47 AM, Tony Lindgren wrote:
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
if (core_next_state == PWRDM_POWER_OFF) {
omap3_core_save_context();
omap3_cm_save_context();
I think this implementation is sub-optimal, as it only scales
voltages if core is entering idle state. MPU only idle is possible,
however you do not scale voltages in this case.
Hmm that's same as PWRDM_POWER_RET? That's working too.
Or do you have something else in mind that I'm not aware
of?
No, I mean the case where core_next_state == PWRDM_POWER_ON, but
mpu_next_state = PWRDM_POWER_RET. You could scale MPU voltage in this
case but you don't with this implementation.
@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 100ULL);
}
+void omap3_vc_set_pmic_signaling(int core_next_state)
+{
+ u32 voltctrl;
+
+ voltctrl = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+ OMAP3_PRM_VOLTCTRL_OFFSET);
Just a note here, I am currently working on trying to get rid of all
the direct prm_read/write calls outside PRCM drivers, this and rest
should use voltdm-read/write.
OK, sounds like you already have a patch for that in your
3.14-rc4-cm-prm-driver-wip branch?
Yes, there is a patch for that.
Let's do the fixes first, then it's going to be a lot easier for
us to test the rest of the PRMC changes.
+ /*
+* By default let's use I2C4 signaling for retention idle
+* and sys_off_mode pin signaling for off idle. This way we
+* have sys_clk_req pin go down for retention and both
+* sys_clk_req and sys_off_mode pins will go down for off
+* idle. And we can also scale voltages to zero for off-idle.
+* Note that no actual voltage scaling will happen unless the
+* board specific twl4030 PMIC scripts are loaded.
+*/
+ val = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+OMAP3_PRM_VOLTCTRL_OFFSET);
Why not use the I2C4 signalling for off-mode? This way the voltages
can be scaled to 0.6V even without any board specific scripts.
Using I2C4 works too, we're just missing a place to set
and clear OMAP3430_PRM_VOLTCTRL_SEL_OFF bit currently. Sounds
like eventually we should allow changing it dynamically somewhere.
You can't check the presence of the scripts here?
But for now, SEL_OFF should be enabled as it allows debugging PM
by looking at the sys_clkreq and sys_off_mode pins no matter if
there are board specific scripts or not. The sys_off_mode won't
toggle if things are configured to use I2C4, which is confusing.
You can always measure the voltage rails directly also, but yea, it is
more convenient to just look at the led.
-Tero
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
* Tero Kristo t-kri...@ti.com [140423 00:51]:
On 04/12/2014 06:02 PM, Tony Lindgren wrote:
* Tero Kristo t-kri...@ti.com [140412 02:01]:
On 04/11/2014 02:47 AM, Tony Lindgren wrote:
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
if (core_next_state == PWRDM_POWER_OFF) {
omap3_core_save_context();
omap3_cm_save_context();
I think this implementation is sub-optimal, as it only scales
voltages if core is entering idle state. MPU only idle is possible,
however you do not scale voltages in this case.
Hmm that's same as PWRDM_POWER_RET? That's working too.
Or do you have something else in mind that I'm not aware
of?
No, I mean the case where core_next_state == PWRDM_POWER_ON, but
mpu_next_state = PWRDM_POWER_RET. You could scale MPU voltage in this case
but you don't with this implementation.
OK makes sense to pass both mpu_next_state and core_next_state
to the function then.
@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 100ULL);
}
+void omap3_vc_set_pmic_signaling(int core_next_state)
+{
+ u32 voltctrl;
+
+ voltctrl = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+OMAP3_PRM_VOLTCTRL_OFFSET);
Just a note here, I am currently working on trying to get rid of all
the direct prm_read/write calls outside PRCM drivers, this and rest
should use voltdm-read/write.
OK, sounds like you already have a patch for that in your
3.14-rc4-cm-prm-driver-wip branch?
Yes, there is a patch for that.
OK I'll pick it from there.
Let's do the fixes first, then it's going to be a lot easier for
us to test the rest of the PRMC changes.
+ /*
+ * By default let's use I2C4 signaling for retention idle
+ * and sys_off_mode pin signaling for off idle. This way we
+ * have sys_clk_req pin go down for retention and both
+ * sys_clk_req and sys_off_mode pins will go down for off
+ * idle. And we can also scale voltages to zero for off-idle.
+ * Note that no actual voltage scaling will happen unless the
+ * board specific twl4030 PMIC scripts are loaded.
+ */
+ val = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+ OMAP3_PRM_VOLTCTRL_OFFSET);
Why not use the I2C4 signalling for off-mode? This way the voltages
can be scaled to 0.6V even without any board specific scripts.
Using I2C4 works too, we're just missing a place to set
and clear OMAP3430_PRM_VOLTCTRL_SEL_OFF bit currently. Sounds
like eventually we should allow changing it dynamically somewhere.
You can't check the presence of the scripts here?
I guess we could eventually add something for that :)
But for now, SEL_OFF should be enabled as it allows debugging PM
by looking at the sys_clkreq and sys_off_mode pins no matter if
there are board specific scripts or not. The sys_off_mode won't
toggle if things are configured to use I2C4, which is confusing.
You can always measure the voltage rails directly also, but yea, it is more
convenient to just look at the led.
And works for making sure we don't get new PM kernel regressions
even if twl4030 is not working properly :)
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
* Grazvydas Ignotas nota...@gmail.com [140416 06:58]:
On Wed, Apr 16, 2014 at 1:56 AM, Tony Lindgren t...@atomide.com wrote:
* Grazvydas Ignotas nota...@gmail.com [140414 15:51]:
On Fri, Apr 11, 2014 at 2:47 AM, Tony Lindgren t...@atomide.com wrote:
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
I wonder how is it going to affect latencies, adding stuff to suspend
path hurts things like NAND transfers, which consist of lots of small
blocks with an interrupt after each..
For most part this is the completely idle path, so there should
not be much of hurry. Most devices should already block the deeper
idle states for the devices listed in cm_idlest_per, cm_idlest1_core
and cm_idlest3_core registers. So it should be mostly automatic with
runtime PM.
No idea what happens with GPMC though for NAND transfers :) Might
be worth checking.
What happens there is that the interrupt arrives shortly after the
transfer was issued, but arm_pm_idle() would already be entered and
interrupts disabled, so it then has to go through all those slow
register accesses in omap_sram_idle(), which is just useless work in
such particular case (WFI will just return) and cause interrupt
response latency and loss of throughput. But this is mostly a problem
caused by pwrdm_pre_transition() and pwrdm_post_transition() calls
(they were optimized a bit at one point but later reverted),
core_next_state would probably stay ON and your function wouldn't be
called here, yes.
OK. There may be a way to block idle state transitions with runtime
PM or CPUidle in general to avoid that.
Ideally the NAND driver would use DMA for transferring the data from
the memory mapped buffer with cyclic DMA triggered by the external DMA
request pins sys_ndmareq[0..5] if they are wired. Then the DMA would
keep system from hitting any deeper idle states automatically. But that
might quite a project to debug and implement :)
And I don't know if cyclic DMA is supported for NAND, there may be
various things to check between transferring each block. But in theory
at least onenNAND should be able to use the cyclic DMA transfers.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
On Wed, Apr 16, 2014 at 1:56 AM, Tony Lindgren t...@atomide.com wrote:
* Grazvydas Ignotas nota...@gmail.com [140414 15:51]:
On Fri, Apr 11, 2014 at 2:47 AM, Tony Lindgren t...@atomide.com wrote:
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
I wonder how is it going to affect latencies, adding stuff to suspend
path hurts things like NAND transfers, which consist of lots of small
blocks with an interrupt after each..
For most part this is the completely idle path, so there should
not be much of hurry. Most devices should already block the deeper
idle states for the devices listed in cm_idlest_per, cm_idlest1_core
and cm_idlest3_core registers. So it should be mostly automatic with
runtime PM.
No idea what happens with GPMC though for NAND transfers :) Might
be worth checking.
What happens there is that the interrupt arrives shortly after the
transfer was issued, but arm_pm_idle() would already be entered and
interrupts disabled, so it then has to go through all those slow
register accesses in omap_sram_idle(), which is just useless work in
such particular case (WFI will just return) and cause interrupt
response latency and loss of throughput. But this is mostly a problem
caused by pwrdm_pre_transition() and pwrdm_post_transition() calls
(they were optimized a bit at one point but later reverted),
core_next_state would probably stay ON and your function wouldn't be
called here, yes.
--
GraÅžvydas
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
* Grazvydas Ignotas nota...@gmail.com [140414 15:51]:
On Fri, Apr 11, 2014 at 2:47 AM, Tony Lindgren t...@atomide.com wrote:
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
I wonder how is it going to affect latencies, adding stuff to suspend
path hurts things like NAND transfers, which consist of lots of small
blocks with an interrupt after each..
For most part this is the completely idle path, so there should
not be much of hurry. Most devices should already block the deeper
idle states for the devices listed in cm_idlest_per, cm_idlest1_core
and cm_idlest3_core registers. So it should be mostly automatic with
runtime PM.
No idea what happens with GPMC though for NAND transfers :) Might
be worth checking.
+void omap3_vc_set_pmic_signaling(int core_next_state)
+{
+ u32 voltctrl;
+
+ voltctrl = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+ OMAP3_PRM_VOLTCTRL_OFFSET);
+ switch (core_next_state) {
+ case PWRDM_POWER_OFF:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_RET |
+ OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_OFF;
+ break;
+ case PWRDM_POWER_RET:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_OFF |
+ OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_RET;
+ break;
+ default:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_OFF |
+ OMAP3430_PRM_VOLTCTRL_AUTO_RET);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP;
+ break;
+ }
+ omap2_prm_write_mod_reg(voltctrl, OMAP3430_GR_MOD,
+ OMAP3_PRM_VOLTCTRL_OFFSET);
Could it only write if the value changed? Caching register value would
be great too to avoid slow register accesses.
Yeah sure makes sense, I'll take a look at that. We should not need
to check for voltctrl constantly.
+}
+
+/*
+ * Configure signal polarity for sys_clkreq and sys_off_mode pins
+ * as the default values are wrong and can cause the system to hang
+ * if any twl4030 sccripts are loaded.
s/sccripts/scripts
Will fix.
+ */
+static void __init omap3_vc_init_pmic_signaling(void)
+{
+ u32 val, old;
+
+ val = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+OMAP3_PRM_POLCTRL_OFFSET);
+ old = val;
+
+ if (old OMAP3430_PRM_POLCTRL_CLKREQ_POL)
+ val |= OMAP3430_PRM_POLCTRL_CLKREQ_POL;
Are these 2 lines actually doing anything?
Nope :) It should be if (!(old OMAP3430_PRM_POLCTRL_CLKREQ_POL))...
+ if (old OMAP3430_PRM_POLCTRL_OFFMODE_POL)
+ val = ~OMAP3430_PRM_POLCTRL_OFFMODE_POL;
Why not just clear the bit unconditionally?
Mostly for producing some kind of debug message of what's going on
in case somebody has a PMIC with different polarity. If we agree
that's not needed, then that's not needed.
+ if (val != old) {
+ pr_debug(PM: fixing sys_clkreq and sys_off_mode polarity
0x%x - 0x%x\n,
+old, val);
+ }
+ omap2_prm_write_mod_reg(val, OMAP3430_GR_MOD,
+ OMAP3_PRM_POLCTRL_OFFSET);
Maybe move this write inside the block just above it?
Makes sense.
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
On Fri, Apr 11, 2014 at 2:47 AM, Tony Lindgren t...@atomide.com wrote:
While debugging legacy mode vs device tree booted PM regressions,
I noticed that omap3 is not toggling sys_clkreq and sys_off_mode
pins like it should.
The sys_clkreq and sys_off_mode pins are not toggling because of
the following issues:
1. The default polarity for the sys_off_mode pin is wrong.
OFFMODE_POL needs to be cleared for sys_off_mode to go down when
hitting off-idle, while CLKREQ_POL needs to be set so sys_clkreq
goes down when hitting retention.
2. The values for voltctrl register need to be updated dynamically.
We need to set either the retention idle bits, or off idle bits
in the voltctrl register depending the idle mode we're targeting
to hit.
Let's fix these two issues as otherwise the system will just
hang if any twl4030 PMIC idle scripts are loaded. The only case
where the system does not hang is if only retention idle over I2C4
is configured by the bootloader.
Note that even without the twl4030 PMIC scripts, these fixes will
do the proper signaling of sys_clkreq and sys_off_mode pins, so
the fixes are needed to fix monitoring of PM states with LEDs or
an oscilloscope.
Cc: Kevin Hilman khil...@linaro.org
Cc: Nishanth Menon n...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Tero Kristo t-kri...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/pm34xx.c | 2 +
arch/arm/mach-omap2/prm-regbits-34xx.h | 11 -
arch/arm/mach-omap2/vc.c | 75
+-
arch/arm/mach-omap2/vc.h | 2 +
4 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 87099bb..3119ec2 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -50,6 +50,7 @@
#include sdrc.h
#include sram.h
#include control.h
+#include vc.h
/* pm34xx errata defined in pm.h */
u16 pm34xx_errata;
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
I wonder how is it going to affect latencies, adding stuff to suspend
path hurts things like NAND transfers, which consist of lots of small
blocks with an interrupt after each..
if (core_next_state == PWRDM_POWER_OFF) {
omap3_core_save_context();
omap3_cm_save_context();
diff --git a/arch/arm/mach-omap2/prm-regbits-34xx.h
b/arch/arm/mach-omap2/prm-regbits-34xx.h
index cebad56..106132d 100644
--- a/arch/arm/mach-omap2/prm-regbits-34xx.h
+++ b/arch/arm/mach-omap2/prm-regbits-34xx.h
@@ -123,8 +123,15 @@
#define OMAP3430_GLOBAL_SW_RST_SHIFT 1
#define OMAP3430_GLOBAL_COLD_RST_SHIFT 0
#define OMAP3430_GLOBAL_COLD_RST_MASK (1 0)
-#define OMAP3430_SEL_OFF_MASK (1 3)
-#define OMAP3430_AUTO_OFF_MASK (1 2)
+#define OMAP3430_PRM_VOLTCTRL_SEL_VMODE(1 4)
+#define OMAP3430_PRM_VOLTCTRL_SEL_OFF (1 3)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_OFF (1 2)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_RET (1 1)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP (1 0)
#define OMAP3430_SETUP_TIME2_MASK (0x 16)
#define OMAP3430_SETUP_TIME1_MASK (0x 0)
+#define OMAP3430_PRM_POLCTRL_OFFMODE_POL (1 3)
+#define OMAP3430_PRM_POLCTRL_CLKOUT_POL(1 2)
+#define OMAP3430_PRM_POLCTRL_CLKREQ_POL(1 1)
+#define OMAP3430_PRM_POLCTRL_EXTVOL_POL(1 0)
#endif
diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index 49ac797..3d5ba5d 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 100ULL);
}
+void omap3_vc_set_pmic_signaling(int core_next_state)
+{
+ u32 voltctrl;
+
+ voltctrl = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+ OMAP3_PRM_VOLTCTRL_OFFSET);
+ switch (core_next_state) {
+ case PWRDM_POWER_OFF:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_RET |
+ OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_OFF;
+ break;
+ case PWRDM_POWER_RET:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_OFF |
+ OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_RET;
+ break;
+ default:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_OFF |
+
Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
On 04/11/2014 02:47 AM, Tony Lindgren wrote: While debugging legacy
mode vs device tree booted PM regressions,
I noticed that omap3 is not toggling sys_clkreq and sys_off_mode
pins like it should.
The sys_clkreq and sys_off_mode pins are not toggling because of
the following issues:
1. The default polarity for the sys_off_mode pin is wrong.
OFFMODE_POL needs to be cleared for sys_off_mode to go down when
hitting off-idle, while CLKREQ_POL needs to be set so sys_clkreq
goes down when hitting retention.
2. The values for voltctrl register need to be updated dynamically.
We need to set either the retention idle bits, or off idle bits
in the voltctrl register depending the idle mode we're targeting
to hit.
Let's fix these two issues as otherwise the system will just
hang if any twl4030 PMIC idle scripts are loaded. The only case
where the system does not hang is if only retention idle over I2C4
is configured by the bootloader.
Note that even without the twl4030 PMIC scripts, these fixes will
do the proper signaling of sys_clkreq and sys_off_mode pins, so
the fixes are needed to fix monitoring of PM states with LEDs or
an oscilloscope.
Cc: Kevin Hilman khil...@linaro.org
Cc: Nishanth Menon n...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Tero Kristo t-kri...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/pm34xx.c | 2 +
arch/arm/mach-omap2/prm-regbits-34xx.h | 11 -
arch/arm/mach-omap2/vc.c | 75
+-
arch/arm/mach-omap2/vc.h | 2 +
4 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 87099bb..3119ec2 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -50,6 +50,7 @@
#include sdrc.h
#include sram.h
#include control.h
+#include vc.h
/* pm34xx errata defined in pm.h */
u16 pm34xx_errata;
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
if (core_next_state == PWRDM_POWER_OFF) {
omap3_core_save_context();
omap3_cm_save_context();
I think this implementation is sub-optimal, as it only scales voltages
if core is entering idle state. MPU only idle is possible, however you
do not scale voltages in this case.
diff --git a/arch/arm/mach-omap2/prm-regbits-34xx.h
b/arch/arm/mach-omap2/prm-regbits-34xx.h
index cebad56..106132d 100644
--- a/arch/arm/mach-omap2/prm-regbits-34xx.h
+++ b/arch/arm/mach-omap2/prm-regbits-34xx.h
@@ -123,8 +123,15 @@
#define OMAP3430_GLOBAL_SW_RST_SHIFT 1
#define OMAP3430_GLOBAL_COLD_RST_SHIFT 0
#define OMAP3430_GLOBAL_COLD_RST_MASK(1 0)
-#define OMAP3430_SEL_OFF_MASK (1 3)
-#define OMAP3430_AUTO_OFF_MASK(1 2)
+#define OMAP3430_PRM_VOLTCTRL_SEL_VMODE (1 4)
+#define OMAP3430_PRM_VOLTCTRL_SEL_OFF (1 3)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_OFF(1 2)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_RET(1 1)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP (1 0)
#define OMAP3430_SETUP_TIME2_MASK(0x 16)
#define OMAP3430_SETUP_TIME1_MASK(0x 0)
+#define OMAP3430_PRM_POLCTRL_OFFMODE_POL (1 3)
+#define OMAP3430_PRM_POLCTRL_CLKOUT_POL (1 2)
+#define OMAP3430_PRM_POLCTRL_CLKREQ_POL (1 1)
+#define OMAP3430_PRM_POLCTRL_EXTVOL_POL (1 0)
#endif
diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index 49ac797..3d5ba5d 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 100ULL);
}
+void omap3_vc_set_pmic_signaling(int core_next_state)
+{
+ u32 voltctrl;
+
+ voltctrl = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+OMAP3_PRM_VOLTCTRL_OFFSET);
Just a note here, I am currently working on trying to get rid of all the
direct prm_read/write calls outside PRCM drivers, this and rest should
use voltdm-read/write.
+ switch (core_next_state) {
+ case PWRDM_POWER_OFF:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_RET |
+OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_OFF;
+ break;
+ case PWRDM_POWER_RET:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_OFF |
+OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_RET;
+ break;
+ default:
+ voltctrl =
Re: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
* Tero Kristo t-kri...@ti.com [140412 02:01]:
On 04/11/2014 02:47 AM, Tony Lindgren wrote:
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
if (core_next_state == PWRDM_POWER_OFF) {
omap3_core_save_context();
omap3_cm_save_context();
I think this implementation is sub-optimal, as it only scales
voltages if core is entering idle state. MPU only idle is possible,
however you do not scale voltages in this case.
Hmm that's same as PWRDM_POWER_RET? That's working too.
Or do you have something else in mind that I'm not aware
of?
@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 100ULL);
}
+void omap3_vc_set_pmic_signaling(int core_next_state)
+{
+ u32 voltctrl;
+
+ voltctrl = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+ OMAP3_PRM_VOLTCTRL_OFFSET);
Just a note here, I am currently working on trying to get rid of all
the direct prm_read/write calls outside PRCM drivers, this and rest
should use voltdm-read/write.
OK, sounds like you already have a patch for that in your
3.14-rc4-cm-prm-driver-wip branch?
Let's do the fixes first, then it's going to be a lot easier for
us to test the rest of the PRMC changes.
+ /*
+* By default let's use I2C4 signaling for retention idle
+* and sys_off_mode pin signaling for off idle. This way we
+* have sys_clk_req pin go down for retention and both
+* sys_clk_req and sys_off_mode pins will go down for off
+* idle. And we can also scale voltages to zero for off-idle.
+* Note that no actual voltage scaling will happen unless the
+* board specific twl4030 PMIC scripts are loaded.
+*/
+ val = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+OMAP3_PRM_VOLTCTRL_OFFSET);
Why not use the I2C4 signalling for off-mode? This way the voltages
can be scaled to 0.6V even without any board specific scripts.
Using I2C4 works too, we're just missing a place to set
and clear OMAP3430_PRM_VOLTCTRL_SEL_OFF bit currently. Sounds
like eventually we should allow changing it dynamically somewhere.
But for now, SEL_OFF should be enabled as it allows debugging PM
by looking at the sys_clkreq and sys_off_mode pins no matter if
there are board specific scripts or not. The sys_off_mode won't
toggle if things are configured to use I2C4, which is confusing.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode
While debugging legacy mode vs device tree booted PM regressions,
I noticed that omap3 is not toggling sys_clkreq and sys_off_mode
pins like it should.
The sys_clkreq and sys_off_mode pins are not toggling because of
the following issues:
1. The default polarity for the sys_off_mode pin is wrong.
OFFMODE_POL needs to be cleared for sys_off_mode to go down when
hitting off-idle, while CLKREQ_POL needs to be set so sys_clkreq
goes down when hitting retention.
2. The values for voltctrl register need to be updated dynamically.
We need to set either the retention idle bits, or off idle bits
in the voltctrl register depending the idle mode we're targeting
to hit.
Let's fix these two issues as otherwise the system will just
hang if any twl4030 PMIC idle scripts are loaded. The only case
where the system does not hang is if only retention idle over I2C4
is configured by the bootloader.
Note that even without the twl4030 PMIC scripts, these fixes will
do the proper signaling of sys_clkreq and sys_off_mode pins, so
the fixes are needed to fix monitoring of PM states with LEDs or
an oscilloscope.
Cc: Kevin Hilman khil...@linaro.org
Cc: Nishanth Menon n...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Tero Kristo t-kri...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/pm34xx.c | 2 +
arch/arm/mach-omap2/prm-regbits-34xx.h | 11 -
arch/arm/mach-omap2/vc.c | 75 +-
arch/arm/mach-omap2/vc.h | 2 +
4 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 87099bb..3119ec2 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -50,6 +50,7 @@
#include sdrc.h
#include sram.h
#include control.h
+#include vc.h
/* pm34xx errata defined in pm.h */
u16 pm34xx_errata;
@@ -282,6 +283,7 @@ void omap_sram_idle(void)
/* CORE */
if (core_next_state PWRDM_POWER_ON) {
+ omap3_vc_set_pmic_signaling(core_next_state);
if (core_next_state == PWRDM_POWER_OFF) {
omap3_core_save_context();
omap3_cm_save_context();
diff --git a/arch/arm/mach-omap2/prm-regbits-34xx.h
b/arch/arm/mach-omap2/prm-regbits-34xx.h
index cebad56..106132d 100644
--- a/arch/arm/mach-omap2/prm-regbits-34xx.h
+++ b/arch/arm/mach-omap2/prm-regbits-34xx.h
@@ -123,8 +123,15 @@
#define OMAP3430_GLOBAL_SW_RST_SHIFT 1
#define OMAP3430_GLOBAL_COLD_RST_SHIFT 0
#define OMAP3430_GLOBAL_COLD_RST_MASK (1 0)
-#define OMAP3430_SEL_OFF_MASK (1 3)
-#define OMAP3430_AUTO_OFF_MASK (1 2)
+#define OMAP3430_PRM_VOLTCTRL_SEL_VMODE(1 4)
+#define OMAP3430_PRM_VOLTCTRL_SEL_OFF (1 3)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_OFF (1 2)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_RET (1 1)
+#define OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP (1 0)
#define OMAP3430_SETUP_TIME2_MASK (0x 16)
#define OMAP3430_SETUP_TIME1_MASK (0x 0)
+#define OMAP3430_PRM_POLCTRL_OFFMODE_POL (1 3)
+#define OMAP3430_PRM_POLCTRL_CLKOUT_POL(1 2)
+#define OMAP3430_PRM_POLCTRL_CLKREQ_POL(1 1)
+#define OMAP3430_PRM_POLCTRL_EXTVOL_POL(1 0)
#endif
diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index 49ac797..3d5ba5d 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 100ULL);
}
+void omap3_vc_set_pmic_signaling(int core_next_state)
+{
+ u32 voltctrl;
+
+ voltctrl = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,
+ OMAP3_PRM_VOLTCTRL_OFFSET);
+ switch (core_next_state) {
+ case PWRDM_POWER_OFF:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_RET |
+ OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_OFF;
+ break;
+ case PWRDM_POWER_RET:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_OFF |
+ OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_RET;
+ break;
+ default:
+ voltctrl = ~(OMAP3430_PRM_VOLTCTRL_AUTO_OFF |
+ OMAP3430_PRM_VOLTCTRL_AUTO_RET);
+ voltctrl |= OMAP3430_PRM_VOLTCTRL_AUTO_SLEEP;
+ break;
+ }
+ omap2_prm_write_mod_reg(voltctrl, OMAP3430_GR_MOD,
+ OMAP3_PRM_VOLTCTRL_OFFSET);
+}
+
+/*
+ * Configure signal polarity for sys_clkreq and sys_off_mode pins
+ * as the default
