OMAP3: PM: reset USB OTG module on boot

PATCH Superseded Kevin Hilman 2 participants 6 messages Jun 24, 2009

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP USB support

lore

Share & tools

lore

Kevin Hilman

author

2009-06-24 00:27 UTC #0 lore raw

PATCH 1/1 OMAP3: PM: reset USB OTG module on boot

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 <[email protected]>

Tested-by Mike Chan <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

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

arch/arm/mach-omap2/usb-musb.c +18 -3

index d85296d..85731b8 100644
@@ -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

Signature · 1 line

1.6.3.2

Niilo Minkkinen

2009-06-24 06:57 UTC #1 lore raw

Quoted 38 lines · ext 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 <[email protected]>
Tested-by Mike Chan <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
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(-)
arch/arm/mach-omap2/usb-musb.c +5 -3
index d85296d..85731b8 100644
@@ -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;

arch/arm/mach-omap2/usb-musb.c line 52

Quoted 10 lines

arch/arm/mach-omap2/usb-musb.c +10 -0

+
+	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

Quoted 6 lines

arch/arm/mach-omap2/usb-musb.c +3 -0

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

Kevin Hilman

author

2009-06-24 13:21 UTC #2 lore raw

Quoted 53 lines · Niilo Minkkinen

Quoted 38 lines · ext 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 <[email protected]>
Tested-by Mike Chan <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
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(-)
arch/arm/mach-omap2/usb-musb.c +5 -3
index d85296d..85731b8 100644
@@ -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;
arch/arm/mach-omap2/usb-musb.c line 52
Quoted 10 lines
arch/arm/mach-omap2/usb-musb.c +10 -0
+
+	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 : ?

Review

Quoted 6 lines

(quoted diff) +6 -0

+       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

Quoted 8 lines

SOFTRST & RESETDONE already defined in omap2430.h
Quoted 6 lines
arch/arm/mach-omap2/usb-musb.c +3 -0
+
+	clk_disable(iclk);
+	clk_put(iclk);
 }
 
 #ifdef CONFIG_USB_MUSB_SOC

Niilo Minkkinen

2009-06-24 13:54 UTC #3 lore raw

Quoted 62 lines · ext Kevin Hilman

Quoted 53 lines · Niilo Minkkinen
Quoted 38 lines · ext 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 <[email protected]>
Tested-by Mike Chan <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
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(-)
arch/arm/mach-omap2/usb-musb.c +5 -3
index d85296d..85731b8 100644
@@ -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;
arch/arm/mach-omap2/usb-musb.c line 52
Quoted 10 lines
arch/arm/mach-omap2/usb-musb.c +10 -0
+
+	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 : ?
Review
Quoted 6 lines
(quoted diff) +6 -0
+       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;

Quoted 7 lines

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-

Quoted 8 lines

Quoted 8 lines
SOFTRST & RESETDONE already defined in omap2430.h
Quoted 6 lines
arch/arm/mach-omap2/usb-musb.c +3 -0
+
+	clk_disable(iclk);
+	clk_put(iclk);
 }
 
 #ifdef CONFIG_USB_MUSB_SOC

Kevin Hilman

author

2009-06-24 14:07 UTC #4 lore raw

Quoted 68 lines · Niilo Minkkinen

Quoted 67 lines · ext Kevin Hilman
Quoted 53 lines · Niilo Minkkinen
Quoted 38 lines · ext 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 <[email protected]>
Tested-by Mike Chan <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
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(-)
arch/arm/mach-omap2/usb-musb.c +5 -3
index d85296d..85731b8 100644
@@ -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;
arch/arm/mach-omap2/usb-musb.c line 52
Quoted 10 lines
arch/arm/mach-omap2/usb-musb.c +10 -0
+
+	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 : ?
Review
Quoted 6 lines
(quoted diff) +6 -0
+       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?

Quoted 5 lines

Quoted 2 lines
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

Quoted 19 lines

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-
Quoted 8 lines
Quoted 8 lines
SOFTRST & RESETDONE already defined in omap2430.h
arch/arm/mach-omap2/usb-musb.c line 58
Quoted 6 lines
arch/arm/mach-omap2/usb-musb.c +3 -0
+
+	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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Niilo Minkkinen

2009-06-24 14:12 UTC #5 lore raw

Quoted 73 lines · ext Kevin Hilman

Quoted 71 lines · Niilo Minkkinen
Quoted 69 lines · ext Kevin Hilman
Quoted 53 lines · Niilo Minkkinen
Quoted 38 lines · ext 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 <[email protected]>
Tested-by Mike Chan <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
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(-)
arch/arm/mach-omap2/usb-musb.c +5 -3
index d85296d..85731b8 100644
@@ -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;
arch/arm/mach-omap2/usb-musb.c line 52
Quoted 10 lines
arch/arm/mach-omap2/usb-musb.c +10 -0
+
+	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 : ?
Review
Quoted 6 lines
(quoted diff) +6 -0
+       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

Quoted 9 lines

Quoted 2 lines
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-

Quoted 19 lines

Quoted 19 lines
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-
Quoted 8 lines
Quoted 8 lines
SOFTRST & RESETDONE already defined in omap2430.h
arch/arm/mach-omap2/usb-musb.c line 58
Quoted 6 lines
arch/arm/mach-omap2/usb-musb.c +3 -0
+
+	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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html