[PATCH] OMAP3: PM: reset USB OTG module on boot

[PATCH] OMAP3: PM: reset USB OTG module on boot

[Kevin Hilman]

Rather than simply setting force-idle mode on boot, do a reset of the
OTG module.  This really ensures that any bootloader/bootstrap code
that leaves it active will not prevent future retention.  After reset,
OTG module will be in force-idle, force-standby mode.

In addition, ensure that the iclk is enabled before attempting a write
to the module SYSCONFIG register.

Problem reported by Mike Chan <mikechan@google.com>

Tested-by: Mike Chan <mikechan@google.com>
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
If no comments/issues, this will be applied to PM branch and
backported to pm-2.6.29.

 arch/arm/mach-omap2/usb-musb.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index d85296d..85731b8 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -32,12 +32,27 @@
 #include <mach/usb.h>
 
 #define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
+#define OTG_SYSC_SOFTRESET BIT(1)
 
 static void __init usb_musb_pm_init(void)
 {
-	/* Ensure force-idle mode for OTG controller */
-	if (cpu_is_omap34xx())
-		omap_writel(0, OTG_SYSCONFIG);
+	struct clk *iclk;
+
+	if (!cpu_is_omap34xx())
+		return;
+
+	iclk = clk_get(NULL, "hsotgusb_ick");
+	if (WARN_ON(!iclk))
+		return;
+
+	clk_enable(iclk);
+
+	/* Reset OTG controller.  After reset, it will be in
+	 * force-idle, force-standby mode. */
+	omap_writel(OTG_SYSC_SOFTRESET, OTG_SYSCONFIG);
+
+	clk_disable(iclk);
+	clk_put(iclk);
 }
 
 #ifdef CONFIG_USB_MUSB_SOC
-- 
1.6.3.2

Re: [PATCH] OMAP3: PM: reset USB OTG module on boot

[Niilo Minkkinen]

On Wed, 2009-06-24 at 02:27 +0200, ext Kevin Hilman wrote:
> Rather than simply setting force-idle mode on boot, do a reset of the
> OTG module.  This really ensures that any bootloader/bootstrap code
> that leaves it active will not prevent future retention.  After reset,
> OTG module will be in force-idle, force-standby mode.
> 
> In addition, ensure that the iclk is enabled before attempting a write
> to the module SYSCONFIG register.
> 
> Problem reported by Mike Chan <mikechan@google.com>
> 
> Tested-by: Mike Chan <mikechan@google.com>
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> If no comments/issues, this will be applied to PM branch and
> backported to pm-2.6.29.
> 
>  arch/arm/mach-omap2/usb-musb.c |   21 ++++++++++++++++++---
>  1 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> index d85296d..85731b8 100644
> --- a/arch/arm/mach-omap2/usb-musb.c
> +++ b/arch/arm/mach-omap2/usb-musb.c
> @@ -32,12 +32,27 @@
>  #include <mach/usb.h>
>  
>  #define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
> +#define OTG_SYSC_SOFTRESET BIT(1)
>  
>  static void __init usb_musb_pm_init(void)
>  {
> -	/* Ensure force-idle mode for OTG controller */
> -	if (cpu_is_omap34xx())
> -		omap_writel(0, OTG_SYSCONFIG);
> +	struct clk *iclk;
> +
> +	if (!cpu_is_omap34xx())
> +		return;
> +
> +	iclk = clk_get(NULL, "hsotgusb_ick");
> +	if (WARN_ON(!iclk))
> +		return;
> +
> +	clk_enable(iclk);
> +
> +	/* Reset OTG controller.  After reset, it will be in
> +	 * force-idle, force-standby mode. */
> +	omap_writel(OTG_SYSC_SOFTRESET, OTG_SYSCONFIG);

Do you think, this is safe to do w/o waiting reset to be finished
like :  ?

+       omap_writel( SOFTRST, OTG_SYSCONFIG );  /* acquire RESET */
+       start = jiffies;
+       timeout = start + msecs_to_jiffies(10); /* max 10 ms */
+       while (!time_after(jiffies, timeout)) /* wait until RESET OK */
+               if ( omap_readl(OTG_SYSSTATUS) & RESETDONE )
+                       break;

SOFTRST & RESETDONE already defined in omap2430.h

> +
> +	clk_disable(iclk);
> +	clk_put(iclk);
>  }
>  
>  #ifdef CONFIG_USB_MUSB_SOC

Re: [PATCH] OMAP3: PM: reset USB OTG module on boot

[Kevin Hilman]

Niilo Minkkinen <ext-niilo.1.minkkinen@nokia.com> writes:

> On Wed, 2009-06-24 at 02:27 +0200, ext Kevin Hilman wrote:
>> Rather than simply setting force-idle mode on boot, do a reset of the
>> OTG module.  This really ensures that any bootloader/bootstrap code
>> that leaves it active will not prevent future retention.  After reset,
>> OTG module will be in force-idle, force-standby mode.
>> 
>> In addition, ensure that the iclk is enabled before attempting a write
>> to the module SYSCONFIG register.
>> 
>> Problem reported by Mike Chan <mikechan@google.com>
>> 
>> Tested-by: Mike Chan <mikechan@google.com>
>> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>> ---
>> If no comments/issues, this will be applied to PM branch and
>> backported to pm-2.6.29.
>> 
>>  arch/arm/mach-omap2/usb-musb.c |   21 ++++++++++++++++++---
>>  1 files changed, 18 insertions(+), 3 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
>> index d85296d..85731b8 100644
>> --- a/arch/arm/mach-omap2/usb-musb.c
>> +++ b/arch/arm/mach-omap2/usb-musb.c
>> @@ -32,12 +32,27 @@
>>  #include <mach/usb.h>
>>  
>>  #define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
>> +#define OTG_SYSC_SOFTRESET BIT(1)
>>  
>>  static void __init usb_musb_pm_init(void)
>>  {
>> -	/* Ensure force-idle mode for OTG controller */
>> -	if (cpu_is_omap34xx())
>> -		omap_writel(0, OTG_SYSCONFIG);
>> +	struct clk *iclk;
>> +
>> +	if (!cpu_is_omap34xx())
>> +		return;
>> +
>> +	iclk = clk_get(NULL, "hsotgusb_ick");
>> +	if (WARN_ON(!iclk))
>> +		return;
>> +
>> +	clk_enable(iclk);
>> +
>> +	/* Reset OTG controller.  After reset, it will be in
>> +	 * force-idle, force-standby mode. */
>> +	omap_writel(OTG_SYSC_SOFTRESET, OTG_SYSCONFIG);
>
> Do you think, this is safe to do w/o waiting reset to be finished
> like :  ?
>
> +       omap_writel( SOFTRST, OTG_SYSCONFIG );  /* acquire RESET */
> +       start = jiffies;
> +       timeout = start + msecs_to_jiffies(10); /* max 10 ms */
> +       while (!time_after(jiffies, timeout)) /* wait until RESET OK */
> +               if ( omap_readl(OTG_SYSSTATUS) & RESETDONE )
> +                       break;

I thought about waiting for reset, but I decided that it wasn't
necessary to wait since I wasn't going to write any other values
after.  Why hold up the boot process when nothing else will be
writing?

Kevin

> SOFTRST & RESETDONE already defined in omap2430.h
>
>> +
>> +	clk_disable(iclk);
>> +	clk_put(iclk);
>>  }
>>  
>>  #ifdef CONFIG_USB_MUSB_SOC

Re: [PATCH] OMAP3: PM: reset USB OTG module on boot

[Niilo Minkkinen]

On Wed, 2009-06-24 at 15:21 +0200, ext Kevin Hilman wrote:
> Niilo Minkkinen <ext-niilo.1.minkkinen@nokia.com> writes:
> 
> > On Wed, 2009-06-24 at 02:27 +0200, ext Kevin Hilman wrote:
> >> Rather than simply setting force-idle mode on boot, do a reset of the
> >> OTG module.  This really ensures that any bootloader/bootstrap code
> >> that leaves it active will not prevent future retention.  After reset,
> >> OTG module will be in force-idle, force-standby mode.
> >> 
> >> In addition, ensure that the iclk is enabled before attempting a write
> >> to the module SYSCONFIG register.
> >> 
> >> Problem reported by Mike Chan <mikechan@google.com>
> >> 
> >> Tested-by: Mike Chan <mikechan@google.com>
> >> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> >> ---
> >> If no comments/issues, this will be applied to PM branch and
> >> backported to pm-2.6.29.
> >> 
> >>  arch/arm/mach-omap2/usb-musb.c |   21 ++++++++++++++++++---
> >>  1 files changed, 18 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> >> index d85296d..85731b8 100644
> >> --- a/arch/arm/mach-omap2/usb-musb.c
> >> +++ b/arch/arm/mach-omap2/usb-musb.c
> >> @@ -32,12 +32,27 @@
> >>  #include <mach/usb.h>
> >>  
> >>  #define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
> >> +#define OTG_SYSC_SOFTRESET BIT(1)
> >>  
> >>  static void __init usb_musb_pm_init(void)
> >>  {
> >> -	/* Ensure force-idle mode for OTG controller */
> >> -	if (cpu_is_omap34xx())
> >> -		omap_writel(0, OTG_SYSCONFIG);
> >> +	struct clk *iclk;
> >> +
> >> +	if (!cpu_is_omap34xx())
> >> +		return;
> >> +
> >> +	iclk = clk_get(NULL, "hsotgusb_ick");
> >> +	if (WARN_ON(!iclk))
> >> +		return;
> >> +
> >> +	clk_enable(iclk);
> >> +
> >> +	/* Reset OTG controller.  After reset, it will be in
> >> +	 * force-idle, force-standby mode. */
> >> +	omap_writel(OTG_SYSC_SOFTRESET, OTG_SYSCONFIG);
> >
> > Do you think, this is safe to do w/o waiting reset to be finished
> > like :  ?
> >
> > +       omap_writel( SOFTRST, OTG_SYSCONFIG );  /* acquire RESET */
> > +       start = jiffies;
> > +       timeout = start + msecs_to_jiffies(10); /* max 10 ms */
> > +       while (!time_after(jiffies, timeout)) /* wait until RESET OK */
> > +               if ( omap_readl(OTG_SYSSTATUS) & RESETDONE )
> > +                       break;
> 
> I thought about waiting for reset, but I decided that it wasn't
> necessary to wait since I wasn't going to write any other values
> after.  Why hold up the boot process when nothing else will be
> writing?
> 
> Kevin
> 

Sure you are right.
Thing I don't know, how near is the next access to musb ...
In my tests, this wait hasn't take even 1 ms (in units of jiffies), so I
don't know duration of it.
My comment anyhow can be think as a face-up.

-niilo-

> > SOFTRST & RESETDONE already defined in omap2430.h
> >
> >> +
> >> +	clk_disable(iclk);
> >> +	clk_put(iclk);
> >>  }
> >>  
> >>  #ifdef CONFIG_USB_MUSB_SOC

Re: [PATCH] OMAP3: PM: reset USB OTG module on boot

[Kevin Hilman]

Niilo Minkkinen <ext-niilo.1.minkkinen@nokia.com> writes:

> On Wed, 2009-06-24 at 15:21 +0200, ext Kevin Hilman wrote:
>> Niilo Minkkinen <ext-niilo.1.minkkinen@nokia.com> writes:
>> 
>> > On Wed, 2009-06-24 at 02:27 +0200, ext Kevin Hilman wrote:
>> >> Rather than simply setting force-idle mode on boot, do a reset of the
>> >> OTG module.  This really ensures that any bootloader/bootstrap code
>> >> that leaves it active will not prevent future retention.  After reset,
>> >> OTG module will be in force-idle, force-standby mode.
>> >> 
>> >> In addition, ensure that the iclk is enabled before attempting a write
>> >> to the module SYSCONFIG register.
>> >> 
>> >> Problem reported by Mike Chan <mikechan@google.com>
>> >> 
>> >> Tested-by: Mike Chan <mikechan@google.com>
>> >> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>> >> ---
>> >> If no comments/issues, this will be applied to PM branch and
>> >> backported to pm-2.6.29.
>> >> 
>> >>  arch/arm/mach-omap2/usb-musb.c |   21 ++++++++++++++++++---
>> >>  1 files changed, 18 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
>> >> index d85296d..85731b8 100644
>> >> --- a/arch/arm/mach-omap2/usb-musb.c
>> >> +++ b/arch/arm/mach-omap2/usb-musb.c
>> >> @@ -32,12 +32,27 @@
>> >>  #include <mach/usb.h>
>> >>  
>> >>  #define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
>> >> +#define OTG_SYSC_SOFTRESET BIT(1)
>> >>  
>> >>  static void __init usb_musb_pm_init(void)
>> >>  {
>> >> -	/* Ensure force-idle mode for OTG controller */
>> >> -	if (cpu_is_omap34xx())
>> >> -		omap_writel(0, OTG_SYSCONFIG);
>> >> +	struct clk *iclk;
>> >> +
>> >> +	if (!cpu_is_omap34xx())
>> >> +		return;
>> >> +
>> >> +	iclk = clk_get(NULL, "hsotgusb_ick");
>> >> +	if (WARN_ON(!iclk))
>> >> +		return;
>> >> +
>> >> +	clk_enable(iclk);
>> >> +
>> >> +	/* Reset OTG controller.  After reset, it will be in
>> >> +	 * force-idle, force-standby mode. */
>> >> +	omap_writel(OTG_SYSC_SOFTRESET, OTG_SYSCONFIG);
>> >
>> > Do you think, this is safe to do w/o waiting reset to be finished
>> > like :  ?
>> >
>> > +       omap_writel( SOFTRST, OTG_SYSCONFIG );  /* acquire RESET */
>> > +       start = jiffies;
>> > +       timeout = start + msecs_to_jiffies(10); /* max 10 ms */
>> > +       while (!time_after(jiffies, timeout)) /* wait until RESET OK */
>> > +               if ( omap_readl(OTG_SYSSTATUS) & RESETDONE )
>> > +                       break;
>> 
>> I thought about waiting for reset, but I decided that it wasn't
>> necessary to wait since I wasn't going to write any other values
>> after.  Why hold up the boot process when nothing else will be
>> writing?
>> 
>> Kevin
>> 
>
> Sure you are right.
> Thing I don't know, how near is the next access to musb ...

Next access of MUSB isn't until the driver starts, and even then,
there should be a clk_get(), clk_enable() before any other access to
the MUSB regs.

Kevin

> In my tests, this wait hasn't take even 1 ms (in units of jiffies), so I
> don't know duration of it.
> My comment anyhow can be think as a face-up.
>
> -niilo-
>
>> > SOFTRST & RESETDONE already defined in omap2430.h
>> >
>> >> +
>> >> +	clk_disable(iclk);
>> >> +	clk_put(iclk);
>> >>  }
>> >>  
>> >>  #ifdef CONFIG_USB_MUSB_SOC
>
> --
> 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

Re: [PATCH] OMAP3: PM: reset USB OTG module on boot

[Niilo Minkkinen]

On Wed, 2009-06-24 at 16:07 +0200, ext Kevin Hilman wrote:
> Niilo Minkkinen <ext-niilo.1.minkkinen@nokia.com> writes:
> 
> > On Wed, 2009-06-24 at 15:21 +0200, ext Kevin Hilman wrote:
> >> Niilo Minkkinen <ext-niilo.1.minkkinen@nokia.com> writes:
> >> 
> >> > On Wed, 2009-06-24 at 02:27 +0200, ext Kevin Hilman wrote:
> >> >> Rather than simply setting force-idle mode on boot, do a reset of the
> >> >> OTG module.  This really ensures that any bootloader/bootstrap code
> >> >> that leaves it active will not prevent future retention.  After reset,
> >> >> OTG module will be in force-idle, force-standby mode.
> >> >> 
> >> >> In addition, ensure that the iclk is enabled before attempting a write
> >> >> to the module SYSCONFIG register.
> >> >> 
> >> >> Problem reported by Mike Chan <mikechan@google.com>
> >> >> 
> >> >> Tested-by: Mike Chan <mikechan@google.com>
> >> >> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> >> >> ---
> >> >> If no comments/issues, this will be applied to PM branch and
> >> >> backported to pm-2.6.29.
> >> >> 
> >> >>  arch/arm/mach-omap2/usb-musb.c |   21 ++++++++++++++++++---
> >> >>  1 files changed, 18 insertions(+), 3 deletions(-)
> >> >> 
> >> >> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> >> >> index d85296d..85731b8 100644
> >> >> --- a/arch/arm/mach-omap2/usb-musb.c
> >> >> +++ b/arch/arm/mach-omap2/usb-musb.c
> >> >> @@ -32,12 +32,27 @@
> >> >>  #include <mach/usb.h>
> >> >>  
> >> >>  #define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
> >> >> +#define OTG_SYSC_SOFTRESET BIT(1)
> >> >>  
> >> >>  static void __init usb_musb_pm_init(void)
> >> >>  {
> >> >> -	/* Ensure force-idle mode for OTG controller */
> >> >> -	if (cpu_is_omap34xx())
> >> >> -		omap_writel(0, OTG_SYSCONFIG);
> >> >> +	struct clk *iclk;
> >> >> +
> >> >> +	if (!cpu_is_omap34xx())
> >> >> +		return;
> >> >> +
> >> >> +	iclk = clk_get(NULL, "hsotgusb_ick");
> >> >> +	if (WARN_ON(!iclk))
> >> >> +		return;
> >> >> +
> >> >> +	clk_enable(iclk);
> >> >> +
> >> >> +	/* Reset OTG controller.  After reset, it will be in
> >> >> +	 * force-idle, force-standby mode. */
> >> >> +	omap_writel(OTG_SYSC_SOFTRESET, OTG_SYSCONFIG);
> >> >
> >> > Do you think, this is safe to do w/o waiting reset to be finished
> >> > like :  ?
> >> >
> >> > +       omap_writel( SOFTRST, OTG_SYSCONFIG );  /* acquire RESET */
> >> > +       start = jiffies;
> >> > +       timeout = start + msecs_to_jiffies(10); /* max 10 ms */
> >> > +       while (!time_after(jiffies, timeout)) /* wait until RESET OK */
> >> > +               if ( omap_readl(OTG_SYSSTATUS) & RESETDONE )
> >> > +                       break;
> >> 
> >> I thought about waiting for reset, but I decided that it wasn't
> >> necessary to wait since I wasn't going to write any other values
> >> after.  Why hold up the boot process when nothing else will be
> >> writing?
> >> 
> >> Kevin
> >> 
> >
> > Sure you are right.
> > Thing I don't know, how near is the next access to musb ...
> 
> Next access of MUSB isn't until the driver starts, and even then,
> there should be a clk_get(), clk_enable() before any other access to
> the MUSB regs.
> 
> Kevin
> 

Ok. This clears things.
Sorry for consuming bandwidth.

-niilo-

> > In my tests, this wait hasn't take even 1 ms (in units of jiffies), so I
> > don't know duration of it.
> > My comment anyhow can be think as a face-up.
> >
> > -niilo-
> >
> >> > SOFTRST & RESETDONE already defined in omap2430.h
> >> >
> >> >> +
> >> >> +	clk_disable(iclk);
> >> >> +	clk_put(iclk);
> >> >>  }
> >> >>  
> >> >>  #ifdef CONFIG_USB_MUSB_SOC
> >
> > --
> > 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