RE: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
On Wed, 24 Jun 2009, Pandita, Vikram wrote: > >-Original Message- > >From: linux-omap-ow...@vger.kernel.org > >[mailto:linux-omap-ow...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- > ow...@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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
> > > >> > >> 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
"Pandita, Vikram" writes: >>-Original Message- >>From: linux-omap-ow...@vger.kernel.org >>[mailto:linux-omap-ow...@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 >> >>Tested-by: Mike Chan >>Signed-off-by: Kevin Hilman >>--- >>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 >> >>+#include >>+ >> #include >> #include >> #include >> #include >> >>-#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_REVISIONOMAP_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 majord...@vger.kernel.org >>More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
"Pandita, Vikram" 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 >> >>Tested-by: Mike Chan >>Signed-off-by: Kevin Hilman >>--- >>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 >> >>+#include >>+ >> #include >> #include >> #include >> #include >> >>-#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_REVISIONOMAP_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 majord...@vger.kernel.org >>More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] OMAP3: PM: reset USB OTG module on boot
>-Original Message- >From: linux-omap-ow...@vger.kernel.org >[mailto:linux-omap-ow...@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 > >Tested-by: Mike Chan >Signed-off-by: Kevin Hilman >--- >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 > >+#include >+ > #include > #include > #include > #include > >-#define OTG_SYSCONFIG (OMAP34XX_HSUSB_OTG_BASE + 0x404) >+#define OTG_SYSCONFIG0x404 >+#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_REVISIONOMAP_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 majord...@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[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. Problem reported by Mike Chan Tested-by: Mike Chan Signed-off-by: Kevin Hilman --- 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 +#include + #include #include #include #include -#define OTG_SYSCONFIG (OMAP34XX_HSUSB_OTG_BASE + 0x404) +#define OTG_SYSCONFIG 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; + void __iomem *otg_base; + + if (!cpu_is_omap34xx()) + return; + + 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html