Fixes for omap PM for making omap3 DT only

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 <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
  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(-)

arch/arm/mach-omap2/pm34xx.c +1 -0

index 87099bb..3119ec2 100644
@@ -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;

index cebad56..106132d 100644

arch/arm/mach-omap2/prm-regbits-34xx.h +9 -2

@@ -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			(0xffff << 16)
  #define OMAP3430_SETUP_TIME1_MASK			(0xffff << 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

arch/arm/mach-omap2/vc.c +0 -0

index 49ac797..3d5ba5d 100644

arch/arm/mach-omap2/vc.c +31 -0

+	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 values are wrong and can cause the system to hang
+ * if any twl4030 sccripts are loaded.
+ */
+static void __init omap3_vc_init_pmic_signaling(void)
+{
+	u32 val, old;
+
+	val = omap2_prm_read_mod_reg(OMAP3430_GR_MOD,

arch/arm/mach-omap2/vc.c +6 -0

+			 old, val);
+	}
+	omap2_prm_write_mod_reg(val, OMAP3430_GR_MOD,
+				OMAP3_PRM_POLCTRL_OFFSET);
+
+	/*

arch/arm/mach-omap2/vc.c +1 -0

  static void __init omap3_vc_init_channel(struct voltagedomain *voltdm)
  {
+	omap3_vc_init_pmic_signaling();
  	omap3_set_off_timings(voltdm);
  }
 

arch/arm/mach-omap2/vc.h +2 -0

index 91c8d75..6351f62 100644
@@ -117,6 +117,8 @@ extern struct omap_vc_param omap4_mpu_vc_data;
  extern struct omap_vc_param omap4_iva_vc_data;
  extern struct omap_vc_param omap4_core_vc_data;
 
+void omap3_vc_set_pmic_signaling(int core_next_state);
+
  void omap_vc_init_channel(struct voltagedomain *voltdm);
  int omap_vc_pre_scale(struct voltagedomain *voltdm,
  		      unsigned long target_volt,

On 04/11/2014 02:47 AM, Tony Lindgren wrote:>

arch/arm/mach-omap2/pm34xx.c void omap_sram_idle(void) · line 289

Quoted 8 lines

arch/arm/mach-omap2/pm34xx.c +1 -0

@@ -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.

Quoted 4 lines

arch/arm/mach-omap2/vc.c +0 -0

@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
  	return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 1000000ULL);
  }
 

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 228

Quoted 6 lines

arch/arm/mach-omap2/vc.c +6 -0

+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.

Quoted 6 lines

arch/arm/mach-omap2/vc.c +6 -0

+	/*
+	 * 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.

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 285

Quoted 5 lines

arch/arm/mach-omap2/vc.c +5 -0

+	 * 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.

Quoted 13 lines · Tero Kristo

On 04/11/2014 02:47 AM, Tony Lindgren wrote:>

arch/arm/mach-omap2/pm34xx.c void omap_sram_idle(void) · line 289

Quoted 8 lines

arch/arm/mach-omap2/pm34xx.c +1 -0

@@ -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?

Quoted 5 lines

Quoted 5 lines

arch/arm/mach-omap2/vc.c +0 -0

@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
   	return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 1000000ULL);
   }
 

Quoted 10 lines

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 228

Quoted 6 lines

arch/arm/mach-omap2/vc.c +6 -0

+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.

Quoted 6 lines

Quoted 6 lines

arch/arm/mach-omap2/vc.c +6 -0

+	/*
+	 * 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.

Quoted 8 lines

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 285

Quoted 5 lines

arch/arm/mach-omap2/vc.c +5 -0

+	 * 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.

Quoted 15 lines · Tony Lindgren

Quoted 13 lines · Tero Kristo

On 04/11/2014 02:47 AM, Tony Lindgren wrote:>

arch/arm/mach-omap2/pm34xx.c void omap_sram_idle(void) · line 289

Quoted 8 lines

arch/arm/mach-omap2/pm34xx.c +1 -0

@@ -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.

Quoted 3 lines

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.

Quoted 15 lines

Quoted 14 lines

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 228

Quoted 10 lines

arch/arm/mach-omap2/vc.c +6 -0

@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
  	return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 1000000ULL);
  }
 
+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.

Quoted 2 lines

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.

Quoted 18 lines

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.

Quoted 14 lines

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 285

Quoted 11 lines

arch/arm/mach-omap2/vc.c +11 -0

+	/*
+	 * 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.

Quoted 3 lines

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?

Quoted 4 lines

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.

Quoted 20 lines · Tero Kristo

Quoted 18 lines · Tony Lindgren

Quoted 13 lines · Tero Kristo

On 04/11/2014 02:47 AM, Tony Lindgren wrote:>

Review void omap_sram_idle(void) · line 289

Quoted 8 lines

(quoted diff) +1 -0

@@ -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?

Quoted 3 lines

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.

Quoted 4 lines

Quoted 2 lines

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.

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 <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 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(-)

arch/arm/mach-omap2/pm34xx.c +1 -0

index 87099bb..3119ec2 100644
@@ -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;

arch/arm/mach-omap2/pm34xx.c +0 -0

                if (core_next_state == PWRDM_POWER_OFF) {
                        omap3_core_save_context();
                        omap3_cm_save_context();

arch/arm/mach-omap2/prm-regbits-34xx.h +9 -2

index cebad56..106132d 100644
@@ -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                      (0xffff << 16)
 #define OMAP3430_SETUP_TIME1_MASK                      (0xffff << 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

arch/arm/mach-omap2/vc.c +15 -0

index 49ac797..3d5ba5d 100644
@@ -220,6 +220,78 @@ static inline u32 omap_usec_to_32k(u32 usec)
        return DIV_ROUND_UP_ULL(32768ULL * (u64)usec, 1000000ULL);
 }
 
+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);

arch/arm/mach-omap2/pm34xx.c void omap_sram_idle(void) · line 286

Quoted 5 lines · Tony Lindgren

arch/arm/mach-omap2/pm34xx.c +1 -0

@@ -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..

Quoted 20 lines

arch/arm/mach-omap2/vc.c +20 -0

+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);

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 247

Quoted 5 lines

arch/arm/mach-omap2/vc.c +5 -0

+               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.

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 253

Quoted 6 lines

arch/arm/mach-omap2/vc.c +6 -0

+}
+
+/*
+ * 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

Quoted 5 lines

+ */

arch/arm/mach-omap2/vc.c +4 -0

+static void __init omap3_vc_init_pmic_signaling(void)
+{
+       u32 val, old;
+

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 264

Quoted 6 lines

arch/arm/mach-omap2/vc.c +6 -0

+       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?

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 266

Quoted 2 lines

arch/arm/mach-omap2/vc.c +2 -0

+       if (old & OMAP3430_PRM_POLCTRL_OFFMODE_POL)
+               val &= ~OMAP3430_PRM_POLCTRL_OFFMODE_POL;

Why not just clear the bit unconditionally?

arch/arm/mach-omap2/vc.c static inline u32 omap_usec_to_32k(u32 usec) · line 273

Quoted 6 lines

arch/arm/mach-omap2/vc.c +6 -0

+       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?

Quoted 10 lines · Grazvydas Ignotas

arch/arm/mach-omap2/pm34xx.c void omap_sram_idle(void) · line 286

Quoted 5 lines · Tony Lindgren

arch/arm/mach-omap2/pm34xx.c +1 -0

@@ -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.

Quoted 12 lines · Tony Lindgren

Quoted 10 lines · Grazvydas Ignotas

arch/arm/mach-omap2/pm34xx.c void omap_sram_idle(void) · line 286

Quoted 5 lines · Tony Lindgren

arch/arm/mach-omap2/pm34xx.c +1 -0

@@ -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..

Quoted 8 lines

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.

(quoted diff) +14 -0

 static void omap3_set_off_timings(struct voltagedomain *voltdm)
 {
+	struct omap3_vc_config *c = omap3_vc_timings;
+	u32 tstart, tshut, voltoffset;
+
+	if (c->clksetup)
+		return;
+
+	omap_pm_get_oscillator(&tstart, &tshut);
+	if (tstart == ULONG_MAX) {
+		pr_debug("PM: oscillator start-up time not initialized, using 10ms\n");
+		c->clksetup = omap_usec_to_32k(10000);
+	} else {
+		c->clksetup = omap_usec_to_32k(tstart);
+	}
+

* Tony Lindgren <[email protected]> [140410 16:52]:
And here we're missing a write to clksetup, without that the off idle timings are not correct.. Below is an incremental diff on top of this patch.

We are currently disabling the init of voltage scaling for device tree. With the voltage scaling problems fixed for omap3 in general, there's no need to disable the voltage scaling init for device tree based booting.

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/pm.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

arch/arm/mach-omap2/pm.c +8 -15

index e1b4141..ccefd11 100644
@@ -287,25 +287,21 @@ omap_postcore_initcall(omap2_common_pm_init);
 
 int __init omap2_common_pm_late_init(void)
 {
-       /*
-        * In the case of DT, the PMIC and SR initialization will be done using
-        * a completely different mechanism.
-        * Disable this part if a DT blob is available.
-        */
-       if (!of_have_populated_dt()) {
-
-               /* Init the voltage layer */
-               omap_pmic_late_init();
-               omap_voltage_late_init();
+       if (of_have_populated_dt()) {
+               omap3_twl_init();
+               omap4_twl_init();
+       }
 
-               /* Initialize the voltages */
-               omap3_init_voltages();
-               omap4_init_voltages();
+       /* Init the voltage layer */
+       omap_pmic_late_init();
+       omap_voltage_late_init();
 
-               /* Smartreflex device init */
-               omap_devinit_smartreflex();
+       /* Initialize the voltages */

Hi Tony,

If found this patch in your omap-for-v3.16/pm-signed tag and tested it on top of Linus master + omap 3.16 dt + Tomi fbdev omap. I assumed it's going upstream for 3.16(?).

First of all; is this safe on OMAP4460?
As far as I understand voltage scaling on 4460 has never been mainline, but this code enables voltage scaling for all omap4 parts. More below.

Quoted 47 lines

We are currently disabling the init of voltage scaling for device tree. With the voltage scaling problems fixed for omap3 in general, there's no need to disable the voltage scaling init for device tree based booting.

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/pm.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

arch/arm/mach-omap2/pm.c +8 -15

index e1b4141..ccefd11 100644
@@ -287,25 +287,21 @@ omap_postcore_initcall(omap2_common_pm_init);
 
 int __init omap2_common_pm_late_init(void)
 {
-       /*
-        * In the case of DT, the PMIC and SR initialization will be done using
-        * a completely different mechanism.
-        * Disable this part if a DT blob is available.
-        */
-       if (!of_have_populated_dt()) {
-
-               /* Init the voltage layer */
-               omap_pmic_late_init();
-               omap_voltage_late_init();
+       if (of_have_populated_dt()) {
+               omap3_twl_init();
+               omap4_twl_init();
+       }
 
-               /* Initialize the voltages */
-               omap3_init_voltages();
-               omap4_init_voltages();
+       /* Init the voltage layer */
+       omap_pmic_late_init();
+       omap_voltage_late_init();
 
-               /* Smartreflex device init */
-               omap_devinit_smartreflex();
+       /* Initialize the voltages */

arch/arm/mach-omap2/pm.c omap_postcore_initcall(omap2_common_pm_init);

Quoted 10 lines

arch/arm/mach-omap2/pm.c +4 -2

+       omap3_init_voltages();
+       omap4_init_voltages();
 
-       }
+       /* Smartreflex device init */
+       omap_devinit_smartreflex();
 
        /* cpufreq dummy device instantiation */
        omap_init_cpufreq();
--

In omap4_twl_init() we have:
if (!cpu_is_omap44xx())

    return -ENODEV;

voltdm = voltdm_lookup("mpu"); omap_voltage_register_pmic(voltdm, &omap4_mpu_pmic);

voltdm = voltdm_lookup("iva"); omap_voltage_register_pmic(voltdm, &omap4_iva_pmic);

voltdm = voltdm_lookup("core"); omap_voltage_register_pmic(voltdm, &omap4_core_pmic);

As far as I understand the setup above is only valid for 4430 and not 4460 since it is hook up to the twl in diffent way. External switcher (TPS62361) for mpu rail and the other rails are used differently.

I also get the following messages on boot now:

[ 2.458740] twl: not initialized
[ 2.462127] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.470581] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.479003] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.487457] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.495880] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.504302] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.512725] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1410000 Vs max 1316660
[ 2.521179] omap2_set_init_voltage: unable to find boot up OPP for vdd_mpu
[ 2.528411] omap2_set_init_voltage: unable to set vdd_mpu
[ 2.534088] omap2_set_init_voltage: unable to find boot up OPP for vdd_core
[ 2.541412] omap2_set_init_voltage: unable to set vdd_core
[ 2.547210] omap2_set_init_voltage: unable to find boot up OPP for vdd_iva
[ 2.554443] omap2_set_init_voltage: unable to set vdd_iva

It doesn't seem too happy on my 4460.

Quoted 5 lines · Joachim Eastwood

Hi Tony,

If found this patch in your omap-for-v3.16/pm-signed tag and tested it on top of Linus master + omap 3.16 dt + Tomi fbdev omap. I assumed it's going upstream for 3.16(?).

Yes.

Quoted 4 lines

First of all; is this safe on OMAP4460?
As far as I understand voltage scaling on 4460 has never been mainline, but this code enables voltage scaling for all omap4 parts. More below.

Sounds like something's not right then. This should just restore the earlier code path we had for legacy booting for omap4.

Quoted 69 lines · Tony Lindgren

Quoted 47 lines

We are currently disabling the init of voltage scaling for device tree. With the voltage scaling problems fixed for omap3 in general, there's no need to disable the voltage scaling init for device tree based booting.

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/pm.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

arch/arm/mach-omap2/pm.c +8 -15

index e1b4141..ccefd11 100644
@@ -287,25 +287,21 @@ omap_postcore_initcall(omap2_common_pm_init);
 
 int __init omap2_common_pm_late_init(void)
 {
-       /*
-        * In the case of DT, the PMIC and SR initialization will be done using
-        * a completely different mechanism.
-        * Disable this part if a DT blob is available.
-        */
-       if (!of_have_populated_dt()) {
-
-               /* Init the voltage layer */
-               omap_pmic_late_init();
-               omap_voltage_late_init();
+       if (of_have_populated_dt()) {
+               omap3_twl_init();
+               omap4_twl_init();
+       }
 
-               /* Initialize the voltages */
-               omap3_init_voltages();
-               omap4_init_voltages();
+       /* Init the voltage layer */
+       omap_pmic_late_init();
+       omap_voltage_late_init();
 
-               /* Smartreflex device init */
-               omap_devinit_smartreflex();
+       /* Initialize the voltages */

arch/arm/mach-omap2/pm.c omap_postcore_initcall(omap2_common_pm_init);

Quoted 10 lines

arch/arm/mach-omap2/pm.c +4 -2

+       omap3_init_voltages();
+       omap4_init_voltages();
 
-       }
+       /* Smartreflex device init */
+       omap_devinit_smartreflex();
 
        /* cpufreq dummy device instantiation */
        omap_init_cpufreq();
--

In omap4_twl_init() we have:
if (!cpu_is_omap44xx())

    return -ENODEV;

voltdm = voltdm_lookup("mpu"); omap_voltage_register_pmic(voltdm, &omap4_mpu_pmic);

voltdm = voltdm_lookup("iva"); omap_voltage_register_pmic(voltdm, &omap4_iva_pmic);

Quoted 6 lines

voltdm = voltdm_lookup("core"); omap_voltage_register_pmic(voltdm, &omap4_core_pmic);

As far as I understand the setup above is only valid for 4430 and not 4460 since it is hook up to the twl in diffent way. External switcher (TPS62361) for mpu rail and the other rails are used differently.

Hmm interesting, I think we had this enabled with legacy booting?

Quoted 23 lines

I also get the following messages on boot now:

[ 2.458740] twl: not initialized
[ 2.462127] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.470581] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.479003] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.487457] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.495880] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.504302] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.512725] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1410000 Vs max 1316660
[ 2.521179] omap2_set_init_voltage: unable to find boot up OPP for vdd_mpu
[ 2.528411] omap2_set_init_voltage: unable to set vdd_mpu
[ 2.534088] omap2_set_init_voltage: unable to find boot up OPP for vdd_core
[ 2.541412] omap2_set_init_voltage: unable to set vdd_core
[ 2.547210] omap2_set_init_voltage: unable to find boot up OPP for vdd_iva
[ 2.554443] omap2_set_init_voltage: unable to set vdd_iva

Quoted 1 line

It doesn't seem too happy on my 4460.

Indeed, we need to fix it. Nishant, any comments on how we should deal with this one?

Quoted 120 lines · Tony Lindgren

Quoted 5 lines · Joachim Eastwood

Hi Tony,

If found this patch in your omap-for-v3.16/pm-signed tag and tested it on top of Linus master + omap 3.16 dt + Tomi fbdev omap. I assumed it's going upstream for 3.16(?).

Yes.

Quoted 4 lines

First of all; is this safe on OMAP4460?
As far as I understand voltage scaling on 4460 has never been mainline, but this code enables voltage scaling for all omap4 parts. More below.

Sounds like something's not right then. This should just restore the earlier code path we had for legacy booting for omap4.

Quoted 69 lines · Tony Lindgren

Quoted 47 lines

We are currently disabling the init of voltage scaling for device tree. With the voltage scaling problems fixed for omap3 in general, there's no need to disable the voltage scaling init for device tree based booting.

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/pm.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

arch/arm/mach-omap2/pm.c +8 -15

index e1b4141..ccefd11 100644
@@ -287,25 +287,21 @@ omap_postcore_initcall(omap2_common_pm_init);
 
 int __init omap2_common_pm_late_init(void)
 {
-       /*
-        * In the case of DT, the PMIC and SR initialization will be done using
-        * a completely different mechanism.
-        * Disable this part if a DT blob is available.
-        */
-       if (!of_have_populated_dt()) {
-
-               /* Init the voltage layer */
-               omap_pmic_late_init();
-               omap_voltage_late_init();
+       if (of_have_populated_dt()) {
+               omap3_twl_init();
+               omap4_twl_init();
+       }
 
-               /* Initialize the voltages */
-               omap3_init_voltages();
-               omap4_init_voltages();
+       /* Init the voltage layer */
+       omap_pmic_late_init();
+       omap_voltage_late_init();
 
-               /* Smartreflex device init */
-               omap_devinit_smartreflex();
+       /* Initialize the voltages */

arch/arm/mach-omap2/pm.c omap_postcore_initcall(omap2_common_pm_init);

Quoted 10 lines

arch/arm/mach-omap2/pm.c +4 -2

+       omap3_init_voltages();
+       omap4_init_voltages();
 
-       }
+       /* Smartreflex device init */
+       omap_devinit_smartreflex();
 
        /* cpufreq dummy device instantiation */
        omap_init_cpufreq();
--

In omap4_twl_init() we have:
if (!cpu_is_omap44xx())

    return -ENODEV;

voltdm = voltdm_lookup("mpu"); omap_voltage_register_pmic(voltdm, &omap4_mpu_pmic);

voltdm = voltdm_lookup("iva"); omap_voltage_register_pmic(voltdm, &omap4_iva_pmic);

Quoted 6 lines

voltdm = voltdm_lookup("core"); omap_voltage_register_pmic(voltdm, &omap4_core_pmic);

As far as I understand the setup above is only valid for 4430 and not 4460 since it is hook up to the twl in diffent way. External switcher (TPS62361) for mpu rail and the other rails are used differently.

Hmm interesting, I think we had this enabled with legacy booting?

Quoted 24 lines

I also get the following messages on boot now:

[ 2.458740] twl: not initialized
[ 2.462127] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.470581] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.479003] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.487457] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.495880] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.504302] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.512725] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1410000 Vs max 1316660
[ 2.521179] omap2_set_init_voltage: unable to find boot up OPP for vdd_mpu
[ 2.528411] omap2_set_init_voltage: unable to set vdd_mpu
[ 2.534088] omap2_set_init_voltage: unable to find boot up OPP for vdd_core
[ 2.541412] omap2_set_init_voltage: unable to set vdd_core
[ 2.547210] omap2_set_init_voltage: unable to find boot up OPP for vdd_iva
[ 2.554443] omap2_set_init_voltage: unable to set vdd_iva

It doesn't seem too happy on my 4460.

Quoted 2 lines

Indeed, we need to fix it. Nishant, any comments on how we should deal with this one?

we can add TPS data here for 4460 mpu (panda-ES) if that is interesting to us - given that the common voltage domain/VC/VP stuff so far has gone in no positive direction in our discussions last year.

Quoted 3 lines

we can add TPS data here for 4460 mpu (panda-ES) if that is interesting to us - given that the common voltage domain/VC/VP stuff so far has gone in no positive direction in our discussions last year.

If this means that voltage scaling and 1.5GHz would work for 4460 it would make me very happy :)
But I assume that would bring lots of more code into mach-omap2 which wouldn't be good either. Escaping the old 3.4 kernel would be nice, though.

Quoted 31 lines · Tony Lindgren

Quoted 4 lines · Joachim Eastwood

As far as I understand the setup above is only valid for 4430 and not 4460 since it is hook up to the twl in diffent way. External switcher (TPS62361) for mpu rail and the other rails are used differently.

Hmm interesting, I think we had this enabled with legacy booting?

Quoted 23 lines

I also get the following messages on boot now:

[ 2.458740] twl: not initialized
[ 2.462127] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.470581] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.479003] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.487457] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.495880] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.504302] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1375000 Vs max 1316660
[ 2.512725] twl6030_uv_to_vsel:OUT OF RANGE! non mapped vsel for
1410000 Vs max 1316660
[ 2.521179] omap2_set_init_voltage: unable to find boot up OPP for vdd_mpu
[ 2.528411] omap2_set_init_voltage: unable to set vdd_mpu
[ 2.534088] omap2_set_init_voltage: unable to find boot up OPP for vdd_core
[ 2.541412] omap2_set_init_voltage: unable to set vdd_core
[ 2.547210] omap2_set_init_voltage: unable to find boot up OPP for vdd_iva
[ 2.554443] omap2_set_init_voltage: unable to set vdd_iva

Quoted 4 lines

Quoted 1 line

It doesn't seem too happy on my 4460.

Indeed, we need to fix it. Nishant, any comments on how we should deal with this one?

we can add TPS data here for 4460 mpu (panda-ES) if that is interesting to us - given that the common voltage domain/VC/VP stuff so far has gone in no positive direction in our discussions last year.

On N900 there are nice LEDs that show the state of the sys_clkreq and sys_off_mode pins.

Hi,

Review

Quoted 6 lines · Tony Lindgren

(quoted diff) +6 -0

+        leds {
+                compatible = "gpio-leds";
+                heartbeat {
+                        label = "debug::sleep";
+                        gpios = <&gpio6 2 GPIO_ACTIVE_HIGH>;  /* gpio162 */
+                        linux,default-trigger = "default-on";

Just some very minor nits, spaces should be converted to TABs here for indent, and patch title should say "keyboard" instead of "keybaord".

Acked-by Aaro Koskinen <[email protected]>

I noticed a regression where the omap sys_clkreq signal will never trigger for omap3 when booted with device tree while it triggers when booted in legacy mode. This means voltage scaling does not do anything when booted with device tree.

Turns out the reason is we fail to initialize the SmartReflex enable bit in twl4030 with the following error:

twl: not initialized

And that happens because we are wrongly tinkering with the twl4030 registers in arch/arm/mach-omap2/omap_twl.c before the driver is initialized. Looking at the the SmartReflex bit enable code in omap_twl.c, we need to always set it.

So let's fix the issue by always enabling the twl4030 SmartReflex bit in the drivers/mfd/twl-core.c probe, and drop the related code in omap_twl.c.

Note that we still have some twl4030 tinkering left in omap_twl.c for the twl6030 case, but that's a different patch.

Cc Kevin Hilman <[email protected]>

Cc Lee Jones <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Samuel Ortiz <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/omap_twl.c | 60 ------------------------------------------
 drivers/mfd/twl-core.c         | 15 +++++++++++
 2 files changed, 15 insertions(+), 60 deletions(-)

Quoted 29 lines

I noticed a regression where the omap sys_clkreq signal will never trigger for omap3 when booted with device tree while it triggers when booted in legacy mode. This means voltage scaling does not do anything when booted with device tree.

Turns out the reason is we fail to initialize the SmartReflex enable bit in twl4030 with the following error:

twl: not initialized

And that happens because we are wrongly tinkering with the twl4030 registers in arch/arm/mach-omap2/omap_twl.c before the driver is initialized. Looking at the the SmartReflex bit enable code in omap_twl.c, we need to always set it.

So let's fix the issue by always enabling the twl4030 SmartReflex bit in the drivers/mfd/twl-core.c probe, and drop the related code in omap_twl.c.

Note that we still have some twl4030 tinkering left in omap_twl.c for the twl6030 case, but that's a different patch.

Cc Kevin Hilman <[email protected]>

Cc Lee Jones <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Samuel Ortiz <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

Quoted 4 lines

---
 arch/arm/mach-omap2/omap_twl.c | 60 ------------------------------------------
 drivers/mfd/twl-core.c         | 15 +++++++++++
 2 files changed, 15 insertions(+), 60 deletions(-)

Patch looks okay to me, and removes lots of code which is nice to see. How do you want to handle this patch? How about if I set up an MFD-OMAP immutable branch for us to use leading up to the v3.16 merge window?

Acked-by Lee Jones <[email protected]>

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Hi Tony,

Sorry for the delay - I've been snowed under recently.

As requested, you patch based on v3.14 ( for some reason :) ).

The following changes since commit 455c6fdbd219161bd09b1165f11699d6d73de11c:

Linux 3.14 (2014-03-30 20:40:15 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-omap-3.16

for you to fetch changes up to a613b739b8c08eab811e677810045cc0522fc3e6:

mfd: twl-core: Fix idle mode signaling for omaps when booted with device tree (2014-04-23 15:31:05 +0100)

----------------------------------------------------------------
Immutable branch between MFD and ARM OMAP due for v3.16 merge-window.

----------------------------------------------------------------
Tony Lindgren (1):
mfd: twl-core: Fix idle mode signaling for omaps when booted with device tree

 arch/arm/mach-omap2/omap_twl.c | 60 ------------------------------------------------------------
 drivers/mfd/twl-core.c         | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 60 deletions(-)

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Since we set up device wake-up interrupts as pinctrl-single interrupts, we now must use the standard request_irq and related functions to manage them.

If the pin interrupts are enabled for some pins at boot, the wake-up events can show up as constantly pending at least on omaps and will hang the system unless the related device driver clears the event at the device.

To fix this, let's clear the interrupt flags during init, and print out a warning so the board maintainers can update their drivers to do proper request_irq for the driver specific wake-up events.

Cc Haojian Zhuang <[email protected]>

Cc Linus Walleij <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

Quoted 9 lines · Tony Lindgren

Since we set up device wake-up interrupts as pinctrl-single interrupts, we now must use the standard request_irq and related functions to manage them.

If the pin interrupts are enabled for some pins at boot, the wake-up events can show up as constantly pending at least on omaps and will hang the system unless the related device driver clears the event at the device.

Quoted 8 lines

To fix this, let's clear the interrupt flags during init, and print out a warning so the board maintainers can update their drivers to do proper request_irq for the driver specific wake-up events.

Cc Haojian Zhuang <[email protected]>

Cc Linus Walleij <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

Looks clean.

Acked-by Linus Walleij <[email protected]>

Shall I apply this patch or will you funnel it through ARM SoC due to deps?

Quoted 2 lines

Shall I apply this patch or will you funnel it through ARM SoC due to deps?

No deps except boards hanging without it.. Please feel free to take this one, prererrably as a fix for the -rc series if no objections.

As we're planning to make omap3 device tree only soon, I was poking around and noticed that PM is not working properly. As we're planning to drop about 20k lines of code, I just had to try to fix this so we know what is going on and don't have to go back. I was pretty bummed out to find that we've had non-working PM code in mainline for really long time.

Anyways, I got the voltage scaling and N900 debug leds working, so with those we can notice any future regressions immediately :)

These are against v3.14, then you might want to also apply the following two patches:

[PATCH] of/platform: Fix no irq domain found errors when populating interrupts
https://lkml.org/lkml/2014/4/10/620

[PATCH] serial: omap: Fix missing pm_runtime_resume handling by simplifying code
http://www.spinics.net/lists/linux-omap/msg104782.html

Note that for the actual voltage scaling to happen, the twl4030 PMIC scripts are also needed. I have some uncleaned patches to load those based on the compatible flag, will post those separately. This series alone fixes the idle state signaling to the PMIC, so we can monitor sys_clkreq and sys_off_idle pins properly.

Please review, comment and test,

I'm on vacation from tomorrow until next week's weekend, but I will have a look the week after next (if still needed).

I have not yet read much of TWL/OMAP PM documentation, but I wonder if it makes sense to move the powerscripting stuff into the DTS file somehow. On a first look the added N900 tables in drivers/mfd/twl4030-power.c seem like a step back to me.