diff mbox

[PATCHv2,09/12] ARM: OMAP2+: usb_host_fs: add custom setup_preprogram for usb_host_fs (fsusb)

Message ID 20120611004617.20034.21714.stgit@dusk
State New
Headers show

Commit Message

Paul Walmsley June 11, 2012, 12:46 a.m. UTC 
Add a custom setup_preprogram function for the usb_host_fs/fsusb IP
block, and connect it to the OMAP4 FSUSB block.

This is the first of two fixes required to get rid of the boot
warning:

omap_hwmod: usb_host_fs: _wait_target_disable failed

and to allow the module to idle.

It may be necessary to use this reset method for OMAP2xxx SoCs as
well; this is left for a future patch.

Based on a patch originally written by Tero Kristo <t-kristo@ti.com>.

This second version of this patch moves the control functions to
include/linux/platform_data/aess.h at Tony's request:

    http://www.spinics.net/lists/arm-kernel/msg178329.html

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    2 +
 include/linux/platform_data/fsusb.h        |  105 ++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)
 create mode 100644 include/linux/platform_data/fsusb.h

Comments

Tony Lindgren June 11, 2012, 6:34 a.m. UTC | #1 
Hi,

Similar comments to the asess patch on this one below.

* Paul Walmsley <paul@pwsan.com> [120610 17:57]:
> --- /dev/null
> +++ b/include/linux/platform_data/fsusb.h

This would be better as include/linux/platform_data/omap-usb.h.

> +#include <plat/omap_hwmod.h>

This include should not be needed here if the hwmod function is
a static function in the some hwmod*.c file.

> +/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
> +#define HCCOMMANDSTATUS			0x0008
> +
> +/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
> +#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)

I think these already have standard defines in some OHCI header?
Felipe may be able to comment more on this?

> +static int fsusb_reset_host_controller(const char *name, void __iomem *base)
> +{
> +	int i;
> +
> +	writel(HCCOMMANDSTATUS_HCR_MASK, base + HCCOMMANDSTATUS);
> +
> +	for (i = 0; i < MAX_FSUSB_HCR_TIME; i++) {
> +		if (!(readl(base + HCCOMMANDSTATUS) & HCCOMMANDSTATUS_HCR_MASK))
> +			break;
> +		udelay(1);
> +	}
> +
> +	if (i == MAX_FSUSB_HCR_TIME) {
> +		pr_warn("%s: %s: host controller reset failed (waited %d usec)\n",
> +			__func__, name, MAX_FSUSB_HCR_TIME);
> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}

This should be "static inline int fsusb_reset_host_controller" as it's
in a header.

> +static int __maybe_unused hwmod_fsusb_preprogram(struct omap_hwmod *oh)
> +{
> +	void __iomem *va;
> +
> +	va = omap_hwmod_get_mpu_rt_va(oh);
> +	if (!va)
> +		return -EINVAL;
> +
> +	fsusb_reset_host_controller(oh->name, va);
> +
> +	return 0;
> +}

And this too should be a static function in some hwmod*.c file.

Regards,

Tony
Felipe Balbi June 11, 2012, 7:13 a.m. UTC | #2 
On Sun, Jun 10, 2012 at 11:34:17PM -0700, Tony Lindgren wrote:
> Hi,
> 
> Similar comments to the asess patch on this one below.
> 
> * Paul Walmsley <paul@pwsan.com> [120610 17:57]:
> > --- /dev/null
> > +++ b/include/linux/platform_data/fsusb.h
> 
> This would be better as include/linux/platform_data/omap-usb.h.
> 
> > +#include <plat/omap_hwmod.h>
> 
> This include should not be needed here if the hwmod function is
> a static function in the some hwmod*.c file.
> 
> > +/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
> > +#define HCCOMMANDSTATUS			0x0008
> > +
> > +/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
> > +#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)
> 
> I think these already have standard defines in some OHCI header?
> Felipe may be able to comment more on this?

Well, yeah... but it's defined on drivers/usb/host/ohci.h and it's
actually a structure:

| /*
|  * This is the structure of the OHCI controller's memory mapped I/O region.
|  * You must use readl() and writel() (in <asm/io.h>) to access these fields!!
|  * Layout is in section 7 (and appendix B) of the spec.
|  */
| struct ohci_regs {
| 	/* control and status registers (section 7.1) */
| 	__hc32	revision;
| 	__hc32	control;
| 	__hc32	cmdstatus;
| 	__hc32	intrstatus;
| 	__hc32	intrenable;
| 	__hc32	intrdisable;
| 
| 	/* memory pointers (section 7.2) */
| 	__hc32	hcca;
| 	__hc32	ed_periodcurrent;
| 	__hc32	ed_controlhead;
| 	__hc32	ed_controlcurrent;
| 	__hc32	ed_bulkhead;
| 	__hc32	ed_bulkcurrent;
| 	__hc32	donehead;
| 
| 	/* frame counters (section 7.3) */
| 	__hc32	fminterval;
| 	__hc32	fmremaining;
| 	__hc32	fmnumber;
| 	__hc32	periodicstart;
| 	__hc32	lsthresh;
| 
| 	/* Root hub ports (section 7.4) */
| 	struct	ohci_roothub_regs {
| 		__hc32	a;
| 		__hc32	b;
| 		__hc32	status;
| #define MAX_ROOT_PORTS	15	/* maximum OHCI root hub ports (RH_A_NDP) */
| 		__hc32	portstatus [MAX_ROOT_PORTS];
| 	} roothub;
| 
| 	/* and optional "legacy support" registers (appendix B) at 0x0100 */
| 
| } __attribute__ ((aligned(32)));

[...]

| /*
|  * HcCommandStatus (cmdstatus) register masks
|  */
| #define OHCI_HCR	(1 << 0)	/* host controller reset */
| #define OHCI_CLF	(1 << 1)	/* control list filled */
| #define OHCI_BLF	(1 << 2)	/* bulk list filled */
| #define OHCI_OCR	(1 << 3)	/* ownership change request */
| #define OHCI_SOC	(3 << 16)	/* scheduling overrun count */

> > +static int fsusb_reset_host_controller(const char *name, void __iomem *base)
> > +{
> > +	int i;
> > +
> > +	writel(HCCOMMANDSTATUS_HCR_MASK, base + HCCOMMANDSTATUS);
> > +
> > +	for (i = 0; i < MAX_FSUSB_HCR_TIME; i++) {
> > +		if (!(readl(base + HCCOMMANDSTATUS) & HCCOMMANDSTATUS_HCR_MASK))
> > +			break;
> > +		udelay(1);
> > +	}
> > +
> > +	if (i == MAX_FSUSB_HCR_TIME) {
> > +		pr_warn("%s: %s: host controller reset failed (waited %d usec)\n",
> > +			__func__, name, MAX_FSUSB_HCR_TIME);
> > +		return -EBUSY;
> > +	}
> > +
> > +	return 0;
> > +}
> 
> This should be "static inline int fsusb_reset_host_controller" as it's
> in a header.

why is it even in a header ? Only hwmod_fsusb_preprogram() will use it
anyway.
Tony Lindgren June 11, 2012, 7:41 a.m. UTC | #3 
* Felipe Balbi <balbi@ti.com> [120611 00:19]:
> On Sun, Jun 10, 2012 at 11:34:17PM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > Similar comments to the asess patch on this one below.
> > 
> > * Paul Walmsley <paul@pwsan.com> [120610 17:57]:
> > > --- /dev/null
> > > +++ b/include/linux/platform_data/fsusb.h
> > 
> > This would be better as include/linux/platform_data/omap-usb.h.
> > 
> > > +#include <plat/omap_hwmod.h>
> > 
> > This include should not be needed here if the hwmod function is
> > a static function in the some hwmod*.c file.
> > 
> > > +/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
> > > +#define HCCOMMANDSTATUS			0x0008
> > > +
> > > +/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
> > > +#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)
> > 
> > I think these already have standard defines in some OHCI header?
> > Felipe may be able to comment more on this?
> 
> Well, yeah... but it's defined on drivers/usb/host/ohci.h and it's
> actually a structure:

OK I guess then the define is OK.

> > This should be "static inline int fsusb_reset_host_controller" as it's
> > in a header.
> 
> why is it even in a header ? Only hwmod_fsusb_preprogram() will use it
> anyway.

Well ideally it would be something that any OHCI driver can use for
it's reset, and whatever bus code can also call to reset and idle
for the unused devices that don't have the driver compiled in. Got
any better suggestions where to place it? I could be a generic
ohci_reset function in some header?

I don't want to have driver specific code under arch/arm/*omap*/* as
the bus level code should not need to know anything about the driver
specific registers.

Regards,

Tony
Felipe Balbi June 11, 2012, 7:48 a.m. UTC | #4 
Hi,

On Mon, Jun 11, 2012 at 12:41:33AM -0700, Tony Lindgren wrote:
> > > This should be "static inline int fsusb_reset_host_controller" as it's
> > > in a header.
> > 
> > why is it even in a header ? Only hwmod_fsusb_preprogram() will use it
> > anyway.
> 
> Well ideally it would be something that any OHCI driver can use for
> it's reset, and whatever bus code can also call to reset and idle
> for the unused devices that don't have the driver compiled in. Got
> any better suggestions where to place it? I could be a generic
> ohci_reset function in some header?

maybe create <linux/usb/ohci.h> ? Then, while doing that, move register
definitions to this new header too... Something like:

$ mv drivers/usb/host/ohci.h include/linux/usb/
$ sed -i 's/\"ohci.h\"/<linux/usb/ohci.h>/g' $(git ls-files)

then add your ohci_reset() function...

> I don't want to have driver specific code under arch/arm/*omap*/* as
> the bus level code should not need to know anything about the driver
> specific registers.

I see.
Tony Lindgren June 11, 2012, 8:03 a.m. UTC | #5 
* Felipe Balbi <balbi@ti.com> [120611 00:55]:
> Hi,
> 
> On Mon, Jun 11, 2012 at 12:41:33AM -0700, Tony Lindgren wrote:
> > > > This should be "static inline int fsusb_reset_host_controller" as it's
> > > > in a header.
> > > 
> > > why is it even in a header ? Only hwmod_fsusb_preprogram() will use it
> > > anyway.
> > 
> > Well ideally it would be something that any OHCI driver can use for
> > it's reset, and whatever bus code can also call to reset and idle
> > for the unused devices that don't have the driver compiled in. Got
> > any better suggestions where to place it? I could be a generic
> > ohci_reset function in some header?
> 
> maybe create <linux/usb/ohci.h> ? Then, while doing that, move register
> definitions to this new header too... Something like:
> 
> $ mv drivers/usb/host/ohci.h include/linux/usb/
> $ sed -i 's/\"ohci.h\"/<linux/usb/ohci.h>/g' $(git ls-files)
> 
> then add your ohci_reset() function...

Hmm but then again it's pointless to export the all the ohci registers
as that will lead to misuse outside drivers/usb.. Sounds like just
defining the reset register is safest option here.
 
> > I don't want to have driver specific code under arch/arm/*omap*/* as
> > the bus level code should not need to know anything about the driver
> > specific registers.
> 
> I see.

Regards,

Tony
Felipe Balbi June 11, 2012, 9:12 a.m. UTC | #6 
On Mon, Jun 11, 2012 at 01:03:42AM -0700, Tony Lindgren wrote:
> * Felipe Balbi <balbi@ti.com> [120611 00:55]:
> > Hi,
> > 
> > On Mon, Jun 11, 2012 at 12:41:33AM -0700, Tony Lindgren wrote:
> > > > > This should be "static inline int fsusb_reset_host_controller" as it's
> > > > > in a header.
> > > > 
> > > > why is it even in a header ? Only hwmod_fsusb_preprogram() will use it
> > > > anyway.
> > > 
> > > Well ideally it would be something that any OHCI driver can use for
> > > it's reset, and whatever bus code can also call to reset and idle
> > > for the unused devices that don't have the driver compiled in. Got
> > > any better suggestions where to place it? I could be a generic
> > > ohci_reset function in some header?
> > 
> > maybe create <linux/usb/ohci.h> ? Then, while doing that, move register
> > definitions to this new header too... Something like:
> > 
> > $ mv drivers/usb/host/ohci.h include/linux/usb/
> > $ sed -i 's/\"ohci.h\"/<linux/usb/ohci.h>/g' $(git ls-files)
> > 
> > then add your ohci_reset() function...
> 
> Hmm but then again it's pointless to export the all the ohci registers
> as that will lead to misuse outside drivers/usb.. Sounds like just
> defining the reset register is safest option here.

could be as well, but looks like we need to create <linux/usb/ohci.h>
anyway...
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 037424f..b8b28be 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -21,6 +21,7 @@ 
 #include <linux/io.h>
 
 #include <linux/platform_data/aess.h>
+#include <linux/platform_data/fsusb.h>
 
 #include <plat/omap_hwmod.h>
 #include <plat/cpu.h>
@@ -3314,6 +3315,7 @@  static struct omap_hwmod_class_sysconfig omap44xx_usb_host_fs_sysc = {
 static struct omap_hwmod_class omap44xx_usb_host_fs_hwmod_class = {
 	.name	= "usb_host_fs",
 	.sysc	= &omap44xx_usb_host_fs_sysc,
+	.setup_preprogram	= hwmod_fsusb_preprogram,
 };
 
 /* usb_host_fs */
diff --git a/include/linux/platform_data/fsusb.h b/include/linux/platform_data/fsusb.h
new file mode 100644
index 0000000..d1e08e0
--- /dev/null
+++ b/include/linux/platform_data/fsusb.h
@@ -0,0 +1,105 @@ 
+/*
+ * FSUSB IP block integration
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ * Paul Walmsley
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+#ifndef __LINUX_PLATFORM_DATA_FSUSB_H__
+#define __LINUX_PLATFORM_DATA_FSUSB_H__
+
+#include <linux/kernel.h>
+#include <linux/delay.h>
+
+#include <plat/omap_hwmod.h>
+
+ /*
+  * MAX_MODULE_SOFTRESET_TIME: maximum time in microseconds to wait for
+  * an IP block to finish an OCP SOFTRESET.
+  */
+#define MAX_MODULE_SOFTRESET_TIME	10000
+
+ /*
+  * MAX_FSUSB_HCR_TIME: maximum time in microseconds to wait for the
+  * FSUSB block to complete its Host Controller Reset.  This 30 µs
+  * timeout comes from ohci-hcd.c:ohci_run().
+  */
+#define MAX_FSUSB_HCR_TIME		30
+
+/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
+#define HCCOMMANDSTATUS			0x0008
+
+/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
+#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)
+
+/**
+ * fsusb_reset_host_controller - execute an OHCI host controller reset
+ * @name: string that uniquely identifies this on-chip instance of the IP block
+ * @base: base virtual address of the FSUSB IP block
+ *
+ * Run a Host Controller reset on the FSUSB IP block.  At the
+ * conclusion of this reset, the controller will be in UsbSuspend
+ * mode.  This differs from the OCP softreset, which leaves the
+ * controller in the UsbReset mode.  (The FSUSB must be in UsbSuspend
+ * mode to indicate idle to the OMAP PRCM.)  Returns 0 upon success
+ * or -EBUSY if the reset timed out.
+ */
+static int fsusb_reset_host_controller(const char *name, void __iomem *base)
+{
+	int i;
+
+	writel(HCCOMMANDSTATUS_HCR_MASK, base + HCCOMMANDSTATUS);
+
+	for (i = 0; i < MAX_FSUSB_HCR_TIME; i++) {
+		if (!(readl(base + HCCOMMANDSTATUS) & HCCOMMANDSTATUS_HCR_MASK))
+			break;
+		udelay(1);
+	}
+
+	if (i == MAX_FSUSB_HCR_TIME) {
+		pr_warn("%s: %s: host controller reset failed (waited %d usec)\n",
+			__func__, name, MAX_FSUSB_HCR_TIME);
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+/**
+ * hwmod_fsusb_preprogram - place the FSUSB into UsbSuspend state
+ * @oh: struct omap_hwmod *
+ *
+ * Run a Host Controller Reset on the FSUSB.  This will place the host
+ * controller into UsbSuspend state, which will cause the FSUSB
+ * indicate that it is idle to the OMAP PRCM.  This is intended to run
+ * _after_ the OCP softreset, which can be handled via the standard
+ * function.  Passes along the return value of
+ * fsusb_reset_host_controller().
+ */
+static int __maybe_unused hwmod_fsusb_preprogram(struct omap_hwmod *oh)
+{
+	void __iomem *va;
+
+	va = omap_hwmod_get_mpu_rt_va(oh);
+	if (!va)
+		return -EINVAL;
+
+	fsusb_reset_host_controller(oh->name, va);
+
+	return 0;
+}
+
+#endif /* __LINUX_PLATFORM_DATA_FSUSB_H__ */