* Re: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
2009-06-24 17:02 ` Pandita, Vikram
@ 2009-06-24 17:38 ` Kevin Hilman
2009-06-24 18:00 ` Kevin Hilman
2009-06-25 6:28 ` Paul Walmsley
2 siblings, 0 replies; 7+ messages in thread
From: Kevin Hilman @ 2009-06-24 17:38 UTC (permalink / raw)
To: Pandita, Vikram; +Cc: linux-omap@vger.kernel.org
"Pandita, Vikram" <vikram.pandita@ti.com> writes:
>>
>>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.
>
> This patch solves MUSB issue, but is not generic for all IP blocks on omap.
>
> ROM code or u-boot could leave the module sysconfig in a non-idle state.
>
> I recommend just like CONFIG_OMAP_RESET_CLOCKS for clocks, same
> could be done generically for all SYSCONFIG registers in some prcm
> code and not driver specific.
Completely agree.
This feature will come as soon as we have omap_hwmod/omapdev in place.
With that, we will have an easy way to model the SYSCONFIG regs of
each modules, and iterate through them.
Kevin
>>
>>Problem reported by Mike Chan <mikechan@google.com>
>>
>>Tested-by: Mike Chan <mikechan@google.com>
>>Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>>---
>>Updates from v1
>>- use ioremap()+__raw_write instead of deprecated omap_write()
>>- Re: defines in omap2430.h, this header is not exported outside
>> drivers/usb, so define it locally yere
>>
>> arch/arm/mach-omap2/usb-musb.c | 31 +++++++++++++++++++++++++++----
>> 1 files changed, 27 insertions(+), 4 deletions(-)
>>
>>diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
>>index d85296d..31adbe6 100644
>>--- a/arch/arm/mach-omap2/usb-musb.c
>>+++ b/arch/arm/mach-omap2/usb-musb.c
>>@@ -26,18 +26,41 @@
>>
>> #include <linux/usb/musb.h>
>>
>>+#include <asm/sizes.h>
>>+
>> #include <mach/hardware.h>
>> #include <mach/irqs.h>
>> #include <mach/mux.h>
>> #include <mach/usb.h>
>>
>>-#define OTG_SYSCONFIG (OMAP34XX_HSUSB_OTG_BASE + 0x404)
>>+#define OTG_SYSCONFIG 0x404
>>+#define OTG_SYSC_SOFTRESET BIT(1)
>
> This is already defined in drivers/usb/musb/omap2430.h
> Could we reuse?
>
> #if defined(CONFIG_ARCH_OMAP2430)
> #define OMAP_HSOTG_BASE (OMAP243X_HS_BASE)
> #elif defined(CONFIG_ARCH_OMAP3430)
> #define OMAP_HSOTG_BASE (OMAP34XX_HSUSB_OTG_BASE)
> #endif
> #define OMAP_HSOTG(offset) (OMAP_HSOTG_BASE + 0x400 + (offset))
> #define OTG_REVISION OMAP_HSOTG(0x0)
> #define OTG_SYSCONFIG OMAP_HSOTG(0x4)
>
>
>>
>> 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;
>>+ void __iomem *otg_base;
>>+
>>+ if (!cpu_is_omap34xx())
>>+ return;
>
> Why is this sys-config reset done only for omap34xx()
> This should be valid for any old 24xx() as well as new 44xx() omaps
>
>>+
>>+ otg_base = ioremap(OMAP34XX_HSUSB_OTG_BASE, SZ_4K);
>>+ if (WARN_ON(!otg_base))
>>+ 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. */
>>+ __raw_writel(OTG_SYSC_SOFTRESET, otg_base + OTG_SYSCONFIG);
>>+
>>+ clk_disable(iclk);
>>+ clk_put(iclk);
>>+ iounmap(otg_base);
>> }
>>
>> #ifdef CONFIG_USB_MUSB_SOC
>>--
>>1.6.3.2
>>
>>--
>>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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
2009-06-24 17:02 ` Pandita, Vikram
2009-06-24 17:38 ` Kevin Hilman
@ 2009-06-24 18:00 ` Kevin Hilman
2009-06-24 18:11 ` Gadiyar, Anand
2009-06-24 19:27 ` Woodruff, Richard
2009-06-25 6:28 ` Paul Walmsley
2 siblings, 2 replies; 7+ messages in thread
From: Kevin Hilman @ 2009-06-24 18:00 UTC (permalink / raw)
To: Pandita, Vikram; +Cc: linux-omap@vger.kernel.org
"Pandita, Vikram" <vikram.pandita@ti.com> writes:
>>-----Original Message-----
>>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Kevin
>>Hilman
>>Sent: Wednesday, June 24, 2009 10:32 AM
>>To: linux-omap@vger.kernel.org
>>Subject: [PATCH v2] 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.
>
> This patch solves MUSB issue, but is not generic for all IP blocks on omap.
>
> ROM code or u-boot could leave the module sysconfig in a non-idle state.
>
> I recommend just like CONFIG_OMAP_RESET_CLOCKS for clocks, same could be done generically for all SYSCONFIG registers in some prcm code and not driver specific.
>
>>
>>Problem reported by Mike Chan <mikechan@google.com>
>>
>>Tested-by: Mike Chan <mikechan@google.com>
>>Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>>---
>>Updates from v1
>>- use ioremap()+__raw_write instead of deprecated omap_write()
>>- Re: defines in omap2430.h, this header is not exported outside
>> drivers/usb, so define it locally yere
>>
>> arch/arm/mach-omap2/usb-musb.c | 31 +++++++++++++++++++++++++++----
>> 1 files changed, 27 insertions(+), 4 deletions(-)
>>
>>diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
>>index d85296d..31adbe6 100644
>>--- a/arch/arm/mach-omap2/usb-musb.c
>>+++ b/arch/arm/mach-omap2/usb-musb.c
>>@@ -26,18 +26,41 @@
>>
>> #include <linux/usb/musb.h>
>>
>>+#include <asm/sizes.h>
>>+
>> #include <mach/hardware.h>
>> #include <mach/irqs.h>
>> #include <mach/mux.h>
>> #include <mach/usb.h>
>>
>>-#define OTG_SYSCONFIG (OMAP34XX_HSUSB_OTG_BASE + 0x404)
>>+#define OTG_SYSCONFIG 0x404
>>+#define OTG_SYSC_SOFTRESET BIT(1)
>
> This is already defined in drivers/usb/musb/omap2430.h
> Could we reuse?
See comment above in the 'Updates from v1' section.
>
> #if defined(CONFIG_ARCH_OMAP2430)
> #define OMAP_HSOTG_BASE (OMAP243X_HS_BASE)
> #elif defined(CONFIG_ARCH_OMAP3430)
> #define OMAP_HSOTG_BASE (OMAP34XX_HSUSB_OTG_BASE)
> #endif
> #define OMAP_HSOTG(offset) (OMAP_HSOTG_BASE + 0x400 + (offset))
> #define OTG_REVISION OMAP_HSOTG(0x0)
> #define OTG_SYSCONFIG OMAP_HSOTG(0x4)
>
>
>>
>> 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;
>>+ void __iomem *otg_base;
>>+
>>+ if (!cpu_is_omap34xx())
>>+ return;
>
> Why is this sys-config reset done only for omap34xx()
> This should be valid for any old 24xx() as well as new 44xx() omaps
You're correct. Good catch.
The force-idle "fix" that this is replacing was only being done on
34xx due to the smart-idle errata. However, this reset should be
done even without any errata.
Thanks for review, will respin.
Kevin
>>+
>>+ otg_base = ioremap(OMAP34XX_HSUSB_OTG_BASE, SZ_4K);
>>+ if (WARN_ON(!otg_base))
>>+ 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. */
>>+ __raw_writel(OTG_SYSC_SOFTRESET, otg_base + OTG_SYSCONFIG);
>>+
>>+ clk_disable(iclk);
>>+ clk_put(iclk);
>>+ iounmap(otg_base);
>> }
>>
>> #ifdef CONFIG_USB_MUSB_SOC
>>--
>>1.6.3.2
>>
>>--
>>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
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
2009-06-24 18:00 ` Kevin Hilman
@ 2009-06-24 18:11 ` Gadiyar, Anand
2009-06-24 19:27 ` Woodruff, Richard
1 sibling, 0 replies; 7+ messages in thread
From: Gadiyar, Anand @ 2009-06-24 18:11 UTC (permalink / raw)
To: Kevin Hilman, Pandita, Vikram; +Cc: linux-omap@vger.kernel.org
<snip>
> >
> >>
> >> 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;
> >>+ void __iomem *otg_base;
> >>+
> >>+ if (!cpu_is_omap34xx())
> >>+ return;
> >
> > Why is this sys-config reset done only for omap34xx()
> > This should be valid for any old 24xx() as well as new 44xx() omaps
>
Minor correction - it's valid only for any old 243x() only. 242x did not
have this controller, IIRC.
> You're correct. Good catch.
>
> The force-idle "fix" that this is replacing was only being done on
> 34xx due to the smart-idle errata. However, this reset should be
> done even without any errata.
>
> Thanks for review, will respin.
>
> Kevin
>
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
2009-06-24 18:00 ` Kevin Hilman
2009-06-24 18:11 ` Gadiyar, Anand
@ 2009-06-24 19:27 ` Woodruff, Richard
1 sibling, 0 replies; 7+ messages in thread
From: Woodruff, Richard @ 2009-06-24 19:27 UTC (permalink / raw)
To: Kevin Hilman, Pandita, Vikram; +Cc: linux-omap@vger.kernel.org
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Kevin Hilman
> > Why is this sys-config reset done only for omap34xx()
> > This should be valid for any old 24xx() as well as new 44xx() omaps
>
> You're correct. Good catch.
>
> The force-idle "fix" that this is replacing was only being done on
> 34xx due to the smart-idle errata. However, this reset should be
> done even without any errata.
Minor note, IIRC the different OTG IP used in OMAP2420 did require a similar force at boot time. That would be a different file if it exists.
Regards,
Richard W.
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
2009-06-24 17:02 ` Pandita, Vikram
2009-06-24 17:38 ` Kevin Hilman
2009-06-24 18:00 ` Kevin Hilman
@ 2009-06-25 6:28 ` Paul Walmsley
2 siblings, 0 replies; 7+ messages in thread
From: Paul Walmsley @ 2009-06-25 6:28 UTC (permalink / raw)
To: Pandita, Vikram; +Cc: Kevin Hilman, linux-omap@vger.kernel.org
On Wed, 24 Jun 2009, Pandita, Vikram wrote:
> >-----Original Message-----
> >From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of 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.
>
> This patch solves MUSB issue, but is not generic for all IP blocks on omap.
>
> ROM code or u-boot could leave the module sysconfig in a non-idle state.
>
> I recommend just like CONFIG_OMAP_RESET_CLOCKS for clocks, same could be
> done generically for all SYSCONFIG registers in some prcm code and not
> driver specific.
I agree. Please contribute to the development of the omap_hwmod patchset,
which does this (among other things).
- Paul
^ permalink raw reply [flat|nested] 7+ messages in thread