| Message ID | 1362876391-6801-1-git-send-email-notasas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Gražvydas, On Sun, 10 Mar 2013, Grazvydas Ignotas wrote: > For some unknown reason, allowing hwmod to control MIDLEMODE causes > core_pwrdm to not hit idle states for musb in DM3730 at least. > I've verified that setting any MIDLEMODE value other than "force > standby" before enabling the device causes subsequent suspend > attempts to fail with core_pwrdm not entering idle states, even > if the driver is unloaded and "force standby" is restored before > suspend attempt. Ugh sounds like a broken bootloader/previous OS could easily block full chip idle in this case :-( Does that match your understanding? That, even if the new kernel does everything right in terms of initialization and reset, the PRCM's perception of whether the device is in STANDBY is permanently stuck? > Keeping the register set at force standby (reset value) makes it work > and device still functions properly. musb has driver-controlled > OTG_FORCESTDBY register that controls MSTANDBY signal, so perhaps > MIDLEMODE is not even needed? Note that TI PSP kernels also have > similar workarounds. Would like to get your opinion on a different implementation. For other situations where IP blocks don't work in the standard, expected way, we've defined hwmod flags for those situations, like HWMOD_SWSUP_*, and HWMOD_NO_OCP_AUTOIDLE. The motivation is to affirmatively mark IP blocks that don't work as expected, rather than changing the existing, documented hardware bits, which are ideally autogenerated. So what do you think about adding a hwmod flag, HWMOD_FORCE_MSTDBY, and using that in the hwmod code to program the MSTDBYMODE to FORCE_STANDBY and then skipping all other attempts to write to it? - Paul
On Mon, Mar 11, 2013 at 1:15 AM, Paul Walmsley <paul@pwsan.com> wrote: > Hi Gražvydas, > > On Sun, 10 Mar 2013, Grazvydas Ignotas wrote: > >> For some unknown reason, allowing hwmod to control MIDLEMODE causes >> core_pwrdm to not hit idle states for musb in DM3730 at least. >> I've verified that setting any MIDLEMODE value other than "force >> standby" before enabling the device causes subsequent suspend >> attempts to fail with core_pwrdm not entering idle states, even >> if the driver is unloaded and "force standby" is restored before >> suspend attempt. > > Ugh sounds like a broken bootloader/previous OS could easily block full > chip idle in this case :-( Does that match your understanding? That, even > if the new kernel does everything right in terms of initialization and > reset, the PRCM's perception of whether the device is in STANDBY is > permanently stuck? Soft reset seems to recover from this so there is no problem, but you can't reset before every suspend so a workaround is still needed.. >> Keeping the register set at force standby (reset value) makes it work >> and device still functions properly. musb has driver-controlled >> OTG_FORCESTDBY register that controls MSTANDBY signal, so perhaps >> MIDLEMODE is not even needed? Note that TI PSP kernels also have >> similar workarounds. > > Would like to get your opinion on a different implementation. For other > situations where IP blocks don't work in the standard, expected way, we've > defined hwmod flags for those situations, like HWMOD_SWSUP_*, and > HWMOD_NO_OCP_AUTOIDLE. The motivation is to affirmatively mark IP > blocks that don't work as expected, rather than changing the existing, > documented hardware bits, which are ideally autogenerated. > > So what do you think about adding a hwmod flag, HWMOD_FORCE_MSTDBY, and > using that in the hwmod code to program the MSTDBYMODE to FORCE_STANDBY > and then skipping all other attempts to write to it? Well as long as it works it's good for me, although it'll bloat the code a bit compared to just changing the data. Should I attempt an implementation?
Hello Gražvydas, On Mon, 11 Mar 2013, Grazvydas Ignotas wrote: > Soft reset seems to recover from this so there is no problem, but you > can't reset before every suspend so a workaround is still needed.. OK, that's good. > Well as long as it works it's good for me, although it'll bloat the > code a bit compared to just changing the data. Should I attempt an > implementation? Please do. I'd be shocked if it comes out to more than four or five extra ARM instructions... - Paul
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c index ac7e03e..0388bba 100644 --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c @@ -1666,11 +1666,15 @@ static struct omap_hwmod_class_sysconfig omap3xxx_usbhsotg_sysc = { .rev_offs = 0x0400, .sysc_offs = 0x0404, .syss_offs = 0x0408, - .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE| - SYSC_HAS_ENAWAKEUP | SYSC_HAS_SOFTRESET | - SYSC_HAS_AUTOIDLE), - .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART | - MSTANDBY_FORCE | MSTANDBY_NO | MSTANDBY_SMART), + /* + * musb has MMIDLEMODE, but if we ever enable the device not in force + * idle mode, core_pwrdm does not enter idle states at least on 36xx. + * Note that musb also has OTG_FORCESTDBY register that the driver + * uses to control MSTANDBY signal manually. + */ + .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_ENAWAKEUP | + SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE), + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART), .sysc_fields = &omap_hwmod_sysc_type1, };
For some unknown reason, allowing hwmod to control MIDLEMODE causes core_pwrdm to not hit idle states for musb in DM3730 at least. I've verified that setting any MIDLEMODE value other than "force standby" before enabling the device causes subsequent suspend attempts to fail with core_pwrdm not entering idle states, even if the driver is unloaded and "force standby" is restored before suspend attempt. Keeping the register set at force standby (reset value) makes it work and device still functions properly. musb has driver-controlled OTG_FORCESTDBY register that controls MSTANDBY signal, so perhaps MIDLEMODE is not even needed? Note that TI PSP kernels also have similar workarounds. Signed-off-by: Grazvydas Ignotas <notasas@gmail.com> --- arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)