Subject: [PATCH 02/11] ARM: OMAP3: Fix idle mode signaling for sys_clkreq and sys_off_mode

From: [email-masked] (Tony Lindgren)
Date: Wed, 23 Apr 2014 13:49:26 -0700

* Tero Kristo <[email-masked]> [140423 00:51]:
> On 04/12/2014 06:02 PM, Tony Lindgren wrote:
> >* Tero Kristo <[email-masked]> [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, 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.
> >
> >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