All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it
@ 2009-08-13 20:46 Jon Hunter
  2009-08-13 22:20 ` Kevin Hilman
  0 siblings, 1 reply; 8+ messages in thread
From: Jon Hunter @ 2009-08-13 20:46 UTC (permalink / raw)
  To: linux-omap; +Cc: Jon Hunter

From: Jon Hunter <jon-hunter@ti.com>

This is version 2 of this patch. I have incorporated the changes 
recommended by Kevin Hilman.

Depending on the OMAP3 boot mode the MUSB peripheral may or may not be
accessible when the kernel starts.

The OMAP3 can boot from several devices including USB. The sequence of the
devices the OMAP will attempt to boot from is configured via the sys_boot
pins on the device. If USB is one of the devices the OMAP boot ROM attempts
to boot from on power-on, then the interface clock to the MUSB peripheral
will be enabled by the boot ROM and the MUSB peripheral will be accessible
when the kernel boots. However, if USB is not one of the devices OMAP
attempts to boot from, then the interface clock is not enabled and the MUSB
peripheral will not be accessible on start-up.

If the MUSB peripheral is not accessible when the kernel boots, then the
kernel will crash when attempting to access the OTG_SYSCONFIG in the
function musb_platform_init(). The actual cause of the crash is the write
to the OTG_SYSCONFIG register in the function usb_musb_pm_init() to reset
the MUSB peripheral which occurs prior to calling musb_platform_init().
The function usb_musb_pm_init() does not check to see if the interface
clock for the MUSB peripheral is enabled before writing to MUSB register.
This write access does not generate a data-abort at this point, but because
this write does not complete all future accesses to the MUSB controller will
generate data-aborts regardless of whether the interface clock has been
enabled at a later stage. The only way I have found to recover from this is
resetting the device. My understanding is that the interconnect works in
this way to prevent a bad access locking up the system.

This patch ensures the interface clock for the MUSB in the function
usb_musb_pm_init() is enabled. This patch also ensures that the interface
clock is disabled after the reset is complete. My reasoning for always
disabling the clock rather than maintaing its state is:

1). If the MUSB peripheral is not being used then the interface clock should
be disabled.
2). The musb_set_clock() function uses a static variable called "clk_on" to
determine if the MUSB interface clock is on or off. On boot-up clk_on will
be 0 and so this function assumes that the clock is off by default which
may not be the case in the current code.

I have also added a while-loop to wait for the reset of the MUSB module to
complete before this function exits and the interface clock it disabled.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/usb-musb.c |   34 +++++++++++++++++++++++++++++++---
 1 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 3efa19c..247f653 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -35,10 +35,20 @@
 
 #define OTG_SYSCONFIG	   0x404
 #define OTG_SYSC_SOFTRESET BIT(1)
+#define OTG_SYSSTATUS     0x408
+#define OTG_SYSS_RESETDONE BIT(0)
+
+static struct platform_device dummy_pdev = {
+	.dev = {
+		.bus = &platform_bus_type,
+	},
+};
 
 static void __init usb_musb_pm_init(void)
 {
 	void __iomem *otg_base;
+	struct clk *otg_clk;
+	struct device *dev = &dummy_pdev.dev;
 
 	if (!cpu_is_omap34xx())
 		return;
@@ -47,9 +57,27 @@ static void __init usb_musb_pm_init(void)
 	if (WARN_ON(!otg_base))
 		return;
 
-	/* Reset OTG controller.  After reset, it will be in
-	 * force-idle, force-standby mode. */
-	__raw_writel(OTG_SYSC_SOFTRESET, otg_base + OTG_SYSCONFIG);
+	dev_set_name(dev, "musb_hdrc");
+	otg_clk = clk_get(dev, "ick");
+
+	if (otg_clk && clk_enable(otg_clk)) {
+		printk(KERN_WARNING
+			"%s: Unable to enable clocks for MUSB, "
+			"cannot reset.\n",  __func__);
+	} else {
+		/* Reset OTG controller. After reset, it will be in
+		 * force-idle, force-standby mode. */
+		__raw_writel(OTG_SYSC_SOFTRESET, otg_base + OTG_SYSCONFIG);
+
+		while (!(OTG_SYSS_RESETDONE &
+					__raw_readl(otg_base + OTG_SYSSTATUS)))
+			cpu_relax();
+	}
+
+	if (otg_clk) {
+		clk_disable(otg_clk);
+		clk_put(otg_clk);
+	}
 
 	iounmap(otg_base);
 }
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it
  2009-08-13 20:46 [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it Jon Hunter
@ 2009-08-13 22:20 ` Kevin Hilman
  2009-08-13 22:52   ` Kevin Hilman
  2009-08-14 17:25   ` Jon Hunter
  0 siblings, 2 replies; 8+ messages in thread
From: Kevin Hilman @ 2009-08-13 22:20 UTC (permalink / raw)
  To: Jon Hunter; +Cc: linux-omap

Jon Hunter <jon-hunter@ti.com> writes:

> This is version 2 of this patch. I have incorporated the changes 
> recommended by Kevin Hilman.
>
> Depending on the OMAP3 boot mode the MUSB peripheral may or may not be
> accessible when the kernel starts.
>
> The OMAP3 can boot from several devices including USB. The sequence of the
> devices the OMAP will attempt to boot from is configured via the sys_boot
> pins on the device. If USB is one of the devices the OMAP boot ROM attempts
> to boot from on power-on, then the interface clock to the MUSB peripheral
> will be enabled by the boot ROM and the MUSB peripheral will be accessible
> when the kernel boots. However, if USB is not one of the devices OMAP
> attempts to boot from, then the interface clock is not enabled and the MUSB
> peripheral will not be accessible on start-up.
>
> If the MUSB peripheral is not accessible when the kernel boots, then the
> kernel will crash when attempting to access the OTG_SYSCONFIG in the
> function musb_platform_init(). The actual cause of the crash is the write
> to the OTG_SYSCONFIG register in the function usb_musb_pm_init() to reset
> the MUSB peripheral which occurs prior to calling musb_platform_init().
> The function usb_musb_pm_init() does not check to see if the interface
> clock for the MUSB peripheral is enabled before writing to MUSB register.
> This write access does not generate a data-abort at this point, but because
> this write does not complete all future accesses to the MUSB controller will
> generate data-aborts regardless of whether the interface clock has been
> enabled at a later stage. The only way I have found to recover from this is
> resetting the device. My understanding is that the interconnect works in
> this way to prevent a bad access locking up the system.
>
> This patch ensures the interface clock for the MUSB in the function
> usb_musb_pm_init() is enabled. This patch also ensures that the interface
> clock is disabled after the reset is complete. My reasoning for always
> disabling the clock rather than maintaing its state is:
>
> 1). If the MUSB peripheral is not being used then the interface clock should
> be disabled.
> 2). The musb_set_clock() function uses a static variable called "clk_on" to
> determine if the MUSB interface clock is on or off. On boot-up clk_on will
> be 0 and so this function assumes that the clock is off by default which
> may not be the case in the current code.
>
> I have also added a while-loop to wait for the reset of the MUSB module to
> complete before this function exits and the interface clock it disabled.
>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---

Jon, you're raising the bar and spoiling us with such descriptive
changelogs.  If everyone was as thorough as you, the world would be a
merrier place. :)

Thanks, pushing to PM branch and pm-2.6.29.

Kevin

>  arch/arm/mach-omap2/usb-musb.c |   34 +++++++++++++++++++++++++++++++---
>  1 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> index 3efa19c..247f653 100644
> --- a/arch/arm/mach-omap2/usb-musb.c
> +++ b/arch/arm/mach-omap2/usb-musb.c
> @@ -35,10 +35,20 @@
>  
>  #define OTG_SYSCONFIG	   0x404
>  #define OTG_SYSC_SOFTRESET BIT(1)
> +#define OTG_SYSSTATUS     0x408
> +#define OTG_SYSS_RESETDONE BIT(0)
> +
> +static struct platform_device dummy_pdev = {
> +	.dev = {
> +		.bus = &platform_bus_type,
> +	},
> +};
>  
>  static void __init usb_musb_pm_init(void)
>  {
>  	void __iomem *otg_base;
> +	struct clk *otg_clk;
> +	struct device *dev = &dummy_pdev.dev;
>  
>  	if (!cpu_is_omap34xx())
>  		return;
> @@ -47,9 +57,27 @@ static void __init usb_musb_pm_init(void)
>  	if (WARN_ON(!otg_base))
>  		return;
>  
> -	/* Reset OTG controller.  After reset, it will be in
> -	 * force-idle, force-standby mode. */
> -	__raw_writel(OTG_SYSC_SOFTRESET, otg_base + OTG_SYSCONFIG);
> +	dev_set_name(dev, "musb_hdrc");
> +	otg_clk = clk_get(dev, "ick");
> +
> +	if (otg_clk && clk_enable(otg_clk)) {
> +		printk(KERN_WARNING
> +			"%s: Unable to enable clocks for MUSB, "
> +			"cannot reset.\n",  __func__);
> +	} else {
> +		/* Reset OTG controller. After reset, it will be in
> +		 * force-idle, force-standby mode. */
> +		__raw_writel(OTG_SYSC_SOFTRESET, otg_base + OTG_SYSCONFIG);
> +
> +		while (!(OTG_SYSS_RESETDONE &
> +					__raw_readl(otg_base + OTG_SYSSTATUS)))
> +			cpu_relax();
> +	}
> +
> +	if (otg_clk) {
> +		clk_disable(otg_clk);
> +		clk_put(otg_clk);
> +	}
>  
>  	iounmap(otg_base);
>  }
> -- 
> 1.6.0.4
>
> --
> 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] 8+ messages in thread

* Re: [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it
  2009-08-13 22:20 ` Kevin Hilman
@ 2009-08-13 22:52   ` Kevin Hilman
  2009-08-14 17:25   ` Jon Hunter
  1 sibling, 0 replies; 8+ messages in thread
From: Kevin Hilman @ 2009-08-13 22:52 UTC (permalink / raw)
  To: Jon Hunter; +Cc: linux-omap

Kevin Hilman <khilman@deeprootsystems.com> writes:

> Jon Hunter <jon-hunter@ti.com> writes:
>
>> This is version 2 of this patch. I have incorporated the changes 
>> recommended by Kevin Hilman.
>>
[...]

>
> Thanks, pushing to PM branch and pm-2.6.29.
>

Just FYI in case you cant find this patch in the PM branch.  While
rebasing the PM branch, I grouped this patch along with hwmod and the
MMC reset removal so you wont set it on the tip of the PM branch.

Kevin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it
  2009-08-13 22:20 ` Kevin Hilman
  2009-08-13 22:52   ` Kevin Hilman
@ 2009-08-14 17:25   ` Jon Hunter
  2009-08-14 18:11     ` Kevin Hilman
  1 sibling, 1 reply; 8+ messages in thread
From: Jon Hunter @ 2009-08-14 17:25 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: linux-omap


Kevin Hilman wrote:
> Jon, you're raising the bar and spoiling us with such descriptive
> changelogs.  If everyone was as thorough as you, the world would be a
> merrier place. :)
> 
> Thanks, pushing to PM branch and pm-2.6.29.
> 
> Kevin

No problem. You are welcome. Just an FYI, but for 2.6.29, I was doing 
some testing this morning and I found that I needed to make the 
following change to the patch to make it work for 2.6.29. You are 
probably aware already as this makes the patch similar to your original 
patch for mmc-reset.

-	dev_set_name(dev, "musb_hdrc");
-	otg_clk = clk_get(dev, "ick");
+	otg_clk = clk_get(dev, "hsotgusb_ick");

Cheers
Jon


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it
  2009-08-14 17:25   ` Jon Hunter
@ 2009-08-14 18:11     ` Kevin Hilman
  2009-08-17  8:13       ` Tony Lindgren
  0 siblings, 1 reply; 8+ messages in thread
From: Kevin Hilman @ 2009-08-14 18:11 UTC (permalink / raw)
  To: Jon Hunter; +Cc: linux-omap

Jon Hunter <jon-hunter@ti.com> writes:

> Kevin Hilman wrote:
>> Jon, you're raising the bar and spoiling us with such descriptive
>> changelogs.  If everyone was as thorough as you, the world would be a
>> merrier place. :)
>>
>> Thanks, pushing to PM branch and pm-2.6.29.
>>
>> Kevin
>
> No problem. You are welcome. Just an FYI, but for 2.6.29, I was doing
> some testing this morning and I found that I needed to make the
> following change to the patch to make it work for 2.6.29. You are
> probably aware already as this makes the patch similar to your
> original patch for mmc-reset.
>
> -	dev_set_name(dev, "musb_hdrc");
> -	otg_clk = clk_get(dev, "ick");
> +	otg_clk = clk_get(dev, "hsotgusb_ick");

Ah yes, I thought of that when I first saw the patch but forgot to do
that on pm-2.6.29.

Thanks, pushing to pm-2.6.29.

Kevin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it
  2009-08-14 18:11     ` Kevin Hilman
@ 2009-08-17  8:13       ` Tony Lindgren
  2009-08-18  9:04         ` Kevin Hilman
  2009-08-18  9:07         ` Kevin Hilman
  0 siblings, 2 replies; 8+ messages in thread
From: Tony Lindgren @ 2009-08-17  8:13 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: Jon Hunter, linux-omap

* Kevin Hilman <khilman@deeprootsystems.com> [090814 21:11]:
> Jon Hunter <jon-hunter@ti.com> writes:
> 
> > Kevin Hilman wrote:
> >> Jon, you're raising the bar and spoiling us with such descriptive
> >> changelogs.  If everyone was as thorough as you, the world would be a
> >> merrier place. :)
> >>
> >> Thanks, pushing to PM branch and pm-2.6.29.
> >>
> >> Kevin
> >
> > No problem. You are welcome. Just an FYI, but for 2.6.29, I was doing
> > some testing this morning and I found that I needed to make the
> > following change to the patch to make it work for 2.6.29. You are
> > probably aware already as this makes the patch similar to your
> > original patch for mmc-reset.
> >
> > -	dev_set_name(dev, "musb_hdrc");
> > -	otg_clk = clk_get(dev, "ick");
> > +	otg_clk = clk_get(dev, "hsotgusb_ick");
> 
> Ah yes, I thought of that when I first saw the patch but forgot to do
> that on pm-2.6.29.
> 
> Thanks, pushing to pm-2.6.29.

Where's the dependency with this patch to the PM branch? If there is 
a dependency, can it be removed for mainline?

To me it looks like we should send a patch against the mainline tree via
linux-usb list.

Regards,

Tony



> 
> Kevin
> --
> 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] 8+ messages in thread

* Re: [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it
  2009-08-17  8:13       ` Tony Lindgren
@ 2009-08-18  9:04         ` Kevin Hilman
  2009-08-18  9:07         ` Kevin Hilman
  1 sibling, 0 replies; 8+ messages in thread
From: Kevin Hilman @ 2009-08-18  9:04 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Jon Hunter, linux-omap

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@deeprootsystems.com> [090814 21:11]:
>> Jon Hunter <jon-hunter@ti.com> writes:
>> 
>> > Kevin Hilman wrote:
>> >> Jon, you're raising the bar and spoiling us with such descriptive
>> >> changelogs.  If everyone was as thorough as you, the world would be a
>> >> merrier place. :)
>> >>
>> >> Thanks, pushing to PM branch and pm-2.6.29.
>> >>
>> >> Kevin
>> >
>> > No problem. You are welcome. Just an FYI, but for 2.6.29, I was doing
>> > some testing this morning and I found that I needed to make the
>> > following change to the patch to make it work for 2.6.29. You are
>> > probably aware already as this makes the patch similar to your
>> > original patch for mmc-reset.
>> >
>> > -	dev_set_name(dev, "musb_hdrc");
>> > -	otg_clk = clk_get(dev, "ick");
>> > +	otg_clk = clk_get(dev, "hsotgusb_ick");
>> 
>> Ah yes, I thought of that when I first saw the patch but forgot to do
>> that on pm-2.6.29.
>> 
>> Thanks, pushing to pm-2.6.29.
>
> Where's the dependency with this patch to the PM branch? If there is 
> a dependency, can it be removed for mainline?
>
> To me it looks like we should send a patch against the mainline tree via
> linux-usb list.

No, this is a temporary patch until OTG support is added to omap_hwmod.

Kevin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it
  2009-08-17  8:13       ` Tony Lindgren
  2009-08-18  9:04         ` Kevin Hilman
@ 2009-08-18  9:07         ` Kevin Hilman
  1 sibling, 0 replies; 8+ messages in thread
From: Kevin Hilman @ 2009-08-18  9:07 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Jon Hunter, linux-omap

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@deeprootsystems.com> [090814 21:11]:
>> Jon Hunter <jon-hunter@ti.com> writes:
>> 
>> > Kevin Hilman wrote:
>> >> Jon, you're raising the bar and spoiling us with such descriptive
>> >> changelogs.  If everyone was as thorough as you, the world would be a
>> >> merrier place. :)
>> >>
>> >> Thanks, pushing to PM branch and pm-2.6.29.
>> >>
>> >> Kevin
>> >
>> > No problem. You are welcome. Just an FYI, but for 2.6.29, I was doing
>> > some testing this morning and I found that I needed to make the
>> > following change to the patch to make it work for 2.6.29. You are
>> > probably aware already as this makes the patch similar to your
>> > original patch for mmc-reset.
>> >
>> > -	dev_set_name(dev, "musb_hdrc");
>> > -	otg_clk = clk_get(dev, "ick");
>> > +	otg_clk = clk_get(dev, "hsotgusb_ick");
>> 
>> Ah yes, I thought of that when I first saw the patch but forgot to do
>> that on pm-2.6.29.
>> 
>> Thanks, pushing to pm-2.6.29.
>
> Where's the dependency with this patch to the PM branch? If there is 
> a dependency, can it be removed for mainline?
>
> To me it looks like we should send a patch against the mainline tree via
> linux-usb list.

No, this is a temporary patch until OTG support is added to omap_hwmod.

Kevin

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-08-18  9:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-13 20:46 [PATCHv2] OMAP3: PM: Ensure MUSB is accessible before we attempt to reset it Jon Hunter
2009-08-13 22:20 ` Kevin Hilman
2009-08-13 22:52   ` Kevin Hilman
2009-08-14 17:25   ` Jon Hunter
2009-08-14 18:11     ` Kevin Hilman
2009-08-17  8:13       ` Tony Lindgren
2009-08-18  9:04         ` Kevin Hilman
2009-08-18  9:07         ` Kevin Hilman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.