RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

Girish S G
2009-10-21T15:25:34Z

> -----Original Message-----
> From: Gopinath, Thara [mailto:thara@ti.com]
> Sent: Tuesday, October 20, 2009 11:37 PM
> To: Kevin Hilman; Tero.Kristo@nokia.com
> Cc: Ghongdemath, Girish; Woodruff, Richard; linux-omap@vger.kernel.org; jouni.hogander@nokia.com
> Subject: RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller
> >> OMAP3_PRM_VOLTCTRL_OFFSET);
> >> }
> >>+ /* Re-enable interrupt controller autoidle */
> >>+ omap_writel(OMAP3430_AUTOIDLE, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
>
> Autoidle is being enabled inside the if (core_next_state < PWRDM_POWER_ON). This is a bug because it
> is disabled irrespective of core pwr domain state. So if Core domain is on , this code will end up
> not enabling INTC autoidle during resume.

It's outside "if (core_next_state < *)" check in restore path. I see this patch is dependent on some previous set of patches.

-Girish

>
> >>-----Original Message-----
> >>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Kevin
> >>Hilman
> >>Sent: Tuesday, October 20, 2009 10:02 PM
> >>To: Tero.Kristo@nokia.com
> >>Cc: Ghongdemath, Girish; Woodruff, Richard; linux-omap@vger.kernel.org; jouni.hogander@nokia.com
> >>Subject: Re: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller
> >>
> >><Tero.Kristo@nokia.com> writes:
> >>
> >>[...]
> >>
> >>>>>
> >>>>> Anyway, I guess the optimization would look something like this:
> >>>>>
> >>>>> diff --git a/arch/arm/mach-omap2/pm34xx.c
> >>>>b/arch/arm/mach-omap2/pm34xx.c
> >>>>> index 210a806..7a98321 100644
> >>>>> --- a/arch/arm/mach-omap2/pm34xx.c
> >>>>> +++ b/arch/arm/mach-omap2/pm34xx.c
> >>>>> @@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
> >>>>> OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
> >>>>> OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
> >>>>>
> >>>>> +#define INTC_SYSCONFIG 0x10
> >>>>> +
> >>>>> struct power_state {
> >>>>> struct powerdomain *pwrdm;
> >>>>> u32 next_state;
> >>>>> @@ -505,6 +507,12 @@ void omap_sram_idle(void)
> >>>>> prm_set_mod_reg_bits(OMAP3430_EN_IO,
> >>>>WKUP_MOD, PM_WKEN);
> >>>>> omap3_enable_io_chain();
> >>>>> }
> >>>>> + /*
> >>>>> + * Disable INTC autoidle as it can cause interrupt controller
> >>>>> + * to enter unknown state with right combination of
> >>>>sleep / wakeup
> >>>>> + * transitions
> >>>>> + */
> >>>>> + omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
> >>>>
> >>>>Except omap_write* functions are deprecated.
> >>>
> >>> I see, will use __raw_writel here then.
> >>>
> >>>>
> >>>>I'd rather see a call into the interrupt code. Something like
> >>>>omap_intc_prepare_idle()
> >>>
> >>> Can do these changes to add the interface yes. Though name would be omap3_intc_prepare_idle() /
> >>...resume_idle() as this is only needed for OMAP3.
> >>
> >>OK.
> >>
> >>>>
> >>>>Also, isn't this only needed if CORE != ON?
> >>>
> >>> I understood from the problem description that this is needed always when we are entering WFI,
> >>because we might have L3/L4 sleep/run transitions there even if CORE is on.
> >>>
> >>
> >>OK.
> >>
> >>Kevin
> >>--
> >>To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> >>the body of a message to majordomo@vger.kernel.org
> >>More majordomo info at http://vger.kernel.org/majordomo-info.html