[PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[Grazvydas Ignotas]

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

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,
 };
 
-- 
1.7.9.5

[PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[Grazvydas Ignotas]

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

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,
 };
 
-- 
1.7.9.5

Re: [PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1747 bytes --]

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

[PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[Paul Walmsley]

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

Re: [PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[Grazvydas Ignotas]

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?


-- 
Gražvydas
--
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

[PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[Grazvydas Ignotas]

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?


-- 
Gra?vydas

Re: [PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 505 bytes --]

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

[PATCH] ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb

[Paul Walmsley]

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