Re: [PATCH][RFC] OMAP3: PM: Adding OSWR support
* Thara Gopinath [090827 09:54]: > This patch adds OSWR support for MPU/CORE domains in CPUidle. > Two new C states are being added to the existing C states > which makes the new states look like below. Please explain what OSWR means, the patch description should be clear to everybody. I guess you mean Open SWitch Retention? 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][RFC] OMAP3: PM: Adding OSWR support
OSWR stands for Open Switch With Retention. This is one of the target power states where logic is lost for all the modules in the power domain except for the ones with Built in retention Flip Flops. This is the main difference between OFF and OSWR mode. Because of this feature, wake up latency for OSWR is lesser than OFF mode but more than CSWR (Closed Switch With Retention - where module logic retained). Vishwa -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Tony Lindgren Sent: Friday, August 28, 2009 9:37 PM To: Gopinath, Thara Cc: linux-omap@vger.kernel.org Subject: Re: [PATCH][RFC] OMAP3: PM: Adding OSWR support * Thara Gopinath [090827 09:54]: > This patch adds OSWR support for MPU/CORE domains in CPUidle. > Two new C states are being added to the existing C states > which makes the new states look like below. Please explain what OSWR means, the patch description should be clear to everybody. I guess you mean Open SWitch Retention? 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 -- 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][RFC] OMAP3: PM: Adding OSWR support
> -Original Message- > From: linux-omap-ow...@vger.kernel.org > [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of > Sripathy, Vishwanath > Subject: RE: [PATCH][RFC] OMAP3: PM: Adding OSWR support > > OSWR stands for Open Switch With Retention. > This is one of the target power states where logic is lost for all the > modules in the power domain > except for the ones with Built in retention Flip Flops. This is the main > difference between OFF and So, this's supported for CORE, PER and MPU? -Girish -- 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][RFC] OMAP3: PM: Adding OSWR support
Sripathy, Vishwanath wrote: OSWR stands for Open Switch With Retention. Not to split hairs, but the OMAP3430 Technical Reference Manual states that OSwR means Open Switch Retention. So no "with". This W always bother me too! Cheers Jon -- 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][RFC] OMAP3: PM: Adding OSWR support
Girish > >-Original Message- >From: Ghongdemath, Girish >Sent: Wednesday, September 02, 2009 5:03 AM >To: Sripathy, Vishwanath; 'Tony Lindgren'; Gopinath, Thara >Cc: linux-omap@vger.kernel.org >Subject: RE: [PATCH][RFC] OMAP3: PM: Adding OSWR support > > > >> -Original Message- >> From: linux-omap-ow...@vger.kernel.org >> [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of >> Sripathy, Vishwanath >> Subject: RE: [PATCH][RFC] OMAP3: PM: Adding OSWR support >> >> OSWR stands for Open Switch With Retention. >> This is one of the target power states where logic is lost for all the >> modules in the power domain >> except for the ones with Built in retention Flip Flops. This is the main >> difference between OFF and > >So, this's supported for CORE, PER and MPU? > >-Girish OSWR is supported for only for Core and MPU domains (in SW). PER will be in OFF state when Core is in OSWR. -- 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][RFC] OMAP3: PM: Adding OSWR support
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- > ow...@vger.kernel.org] On Behalf Of Hunter, Jon > Not to split hairs, but the OMAP3430 Technical Reference Manual states > that OSwR means Open Switch Retention. So no "with". This W always > bother me too! No bother with W, its part of switch. Underlying spec's have always defined it as following: OSWR stands for Open SWitch Retention CSWR stands for Closed SWitch Retention Regards, Richard W. -- 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][RFC] OMAP3: PM: Adding OSWR support
Thara Gopinath writes: > This patch adds OSWR support for MPU/CORE domains in CPUidle. > Two new C states are being added to the existing C states > which makes the new states look like below. > > C1 - MPU WFI + Core active > C2 - MPU WFI + Core inactive > C3 - MPU CSWR + Core inactive > C4 - MPU OFF + Core inactive > C5 - MPU CSWR + Core CSWR > C6 - MPU OFF + Core CSWR > C7 - MPU OSWR + CORE OSWR (New State) > C8 - MPU OFF + CORE OSWR (New State) > C9 - MPU OFF + CORE OFF > > To enable this feature echo 1 > /sys/powwer/enable_oswr_ret > To disable this feature echo 0 > /sys/poweer/enable_oswr_ret > > This patch depends on http://patchwork.kernel.org/patch/44290/ and > is validated on latest PM branch on OMAP3430 SDP. > > Signed-off-by: Thara Gopinath Thara, This is a large and complex feature which should probably be broken up into bits for cleaner review/merge. First, the Changelog should summarize the OSWR feature and how it is different from current retention mode (CSWR) as well as the motivation for the feature. You might consider breaking it up as follows: - CPUidle max state change - UART prepare idle changes - powerdomain API additions: pwrdm_read_next_*_state() - context save/restore API changes to take target state - base OSWR logic in idle path - 'enable_oswr_ret' option - DPLL4 autoidle hack Some general comments: Can you move the /sys/power/enable_oswr_ret to /debug/pm_debug. I'll be moving the /sys/power/enable_off_mode there as well. Also, I think the name 'enable_oswr' is better than 'enable_oswr_ret'. The 'enable_oswr_ret' feature needs some work. The way it is done will work, but the CPUidle state accounting will not be correct as the target C-state will not be the same as the one actually entered. Sanjeev Premi was working on a way to use the C-state .valid flags to do something like this. Then both, enable_off_mode and enable_oswr can simply set/clear the .valid field of the C-state and the CPUidle enter hook can check the C-state valid flags. More comments below... > --- > arch/arm/mach-omap2/control.c | 20 +++ > arch/arm/mach-omap2/cpuidle34xx.c | 154 > +++-- > arch/arm/mach-omap2/pm-debug.c|3 + > arch/arm/mach-omap2/pm.c | 16 +++- > arch/arm/mach-omap2/pm.h |1 + > arch/arm/mach-omap2/pm34xx.c | 119 +++- > arch/arm/mach-omap2/powerdomain.c | 89 ++- > arch/arm/mach-omap2/powerdomains34xx.h|2 + > arch/arm/mach-omap2/serial.c | 17 +-- > arch/arm/mach-omap2/sleep34xx.S |7 +- > arch/arm/plat-omap/include/mach/control.h |1 + > arch/arm/plat-omap/include/mach/powerdomain.h |4 + > arch/arm/plat-omap/include/mach/serial.h |2 +- > include/linux/cpuidle.h |2 +- > 14 files changed, 379 insertions(+), 58 deletions(-) > > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c > index c9407c0..dc90207 100644 > --- a/arch/arm/mach-omap2/control.c > +++ b/arch/arm/mach-omap2/control.c > @@ -331,6 +331,26 @@ void omap3_save_scratchpad_contents(void) > sizeof(sdrc_block_contents), &arm_context_addr, 4); > } > > +void omap3_scratchpad_dpll4autoidle(int enable) > +{ > + void * __iomem scratchpad_address; > + struct omap3_scratchpad_prcm_block prcm_block_contents; > + > + scratchpad_address = OMAP2_IO_ADDRESS(OMAP343X_SCRATCHPAD); > + > + memcpy_fromio(&prcm_block_contents, scratchpad_address + 0x2C, > + sizeof(prcm_block_contents)); > + if (enable) > + prcm_block_contents.cm_autoidle_pll |= > + (1 << OMAP3430_AUTO_PERIPH_DPLL_SHIFT); > + else > + prcm_block_contents.cm_autoidle_pll &= > + ~OMAP3430_AUTO_PERIPH_DPLL_MASK; > + > + memcpy_toio(scratchpad_address + 0x2C, &prcm_block_contents, > + sizeof(prcm_block_contents)); > +} > + Do you need to read then write the whole block just to change one value? > void omap3_control_save_context(void) > { > control_context.sysconfig = omap_ctrl_readl(OMAP2_CONTROL_SYSCONFIG); > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c > b/arch/arm/mach-omap2/cpuidle34xx.c > index 7bbec90..a5b811b 100644 > --- a/arch/arm/mach-omap2/cpuidle34xx.c > +++ b/arch/arm/mach-omap2/cpuidle34xx.c > @@ -36,14 +36,17 @@ > > #ifdef CONFIG_CPU_IDLE > > -#define OMAP3_MAX_STATES 7 > +#define OMAP3_MAX_STATES 9 > #define OMAP3_STATE_C1 0 /* C1 - MPU WFI + Core active */ > #define OMAP3_STATE_C2 1 /* C2 - MPU WFI + Core inactive */ > #define OMAP3_STATE_C3 2 /* C3 - MPU CSWR + Core inactive */ > -#define OMAP3_STATE_C4 3 /* C4 - MPU OFF + Core iactive */ > -#define OMAP3_STATE_C5 4 /* C5 - MPU RET + Core RET */ > -#define OMAP3_STATE_C6 5 /* C6 - MPU OFF + Core RET */ >