omap: usb: ehci: fix use of hcd->regs

PATCH In review Felipe Balbi 9 participants 39 messages Sep 12, 2008 – Sep 22, 2008

Subsystems (3)

USB USB ehci driver OMAP USB support

lore

Share & tools

lore

Felipe Balbi

author

2008-09-12 12:10 UTC #0 lore raw

PATCH 1/1 omap: usb: ehci: fix use of hcd->regs

hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.

Signed-off-by Felipe Balbi <[email protected]>

---

drivers/usb/host/ehci-omap.c +1 -1

index 8f122e5..a67533d 100644
@@ -438,7 +438,7 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
        hcd->rsrc_start = dev->resource[0].start;
        hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
 
-       hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+       hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
 
        ehci = hcd_to_ehci(hcd);
        ehci->caps = hcd->regs;

Signature · 1 line

balbi

Hiroshi DOYU

2008-09-12 12:14 UTC #1 lore raw

From: "ext Felipe Balbi" <[email protected]>
Subject: [patch] omap: usb: ehci: fix use of hcd->regs
Date: Fri, 12 Sep 2008 15:10:24 +0300

Quoted 11 lines

hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.
Signed-off-by Felipe Balbi <[email protected]>
---
drivers/usb/host/ehci-omap.c +0 -0
index 8f122e5..a67533d 100644

drivers/usb/host/ehci-omap.c static int ehci_hcd_omap_drv_probe(struct platform_device *dev) · line 441

Quoted 5 lines

drivers/usb/host/ehci-omap.c +1 -1

@@ -438,7 +438,7 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
        hcd->rsrc_start = dev->resource[0].start;
        hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
 
-       hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+       hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);

Return value check?

Quoted 11 lines

drivers/usb/host/ehci-omap.c +0 -0

        ehci = hcd_to_ehci(hcd);
        ehci->caps = hcd->regs;

Signature · 6 lines

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

Felipe Balbi

author

2008-09-12 12:30 UTC #2 lore raw

Quoted 18 lines · Hiroshi DOYU

From: "ext Felipe Balbi" <[email protected]>
Subject: [patch] omap: usb: ehci: fix use of hcd->regs
Date: Fri, 12 Sep 2008 15:10:24 +0300
Quoted 14 lines
hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.
Signed-off-by Felipe Balbi <[email protected]>
---
drivers/usb/host/ehci-omap.c +0 -0
index 8f122e5..a67533d 100644
@@ -438,7 +438,7 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
        hcd->rsrc_start = dev->resource[0].start;
        hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
 

Quoted 4 lines

drivers/usb/host/ehci-omap.c static int ehci_hcd_omap_drv_probe(struct platform_device *dev) · line 441
Quoted 2 lines
drivers/usb/host/ehci-omap.c +1 -1
-       hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+       hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
Return value check?

Good one. Here's a new version:

===== cut here ====

From: Felipe Balbi <[email protected]>
Subject: [patch] omap: usb: ehci: fix usb of hcd->regs

hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.

Signed-off-by Felipe Balbi <[email protected]>

---

drivers/usb/host/ehci-omap.c +5 -1

index 8f122e5..4cafa79 100644
@@ -438,7 +438,11 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
        hcd->rsrc_start = dev->resource[0].start;
        hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
 
-       hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+       hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+       if (!hcd->regs) {
+               dev_err(&dev->dev, "ioremap failed\n");
+               return -ENOMEM;
+       }
 
        ehci = hcd_to_ehci(hcd);
        ehci->caps = hcd->regs;

Signature · 1 line

balbi

Hiroshi DOYU

Acked-by

2008-09-12 13:37 UTC #3 lore raw

From: Felipe Balbi <[email protected]>
Subject: Re: [patch] omap: usb: ehci: fix use of hcd->regs
Date: Fri, 12 Sep 2008 15:30:10 +0300

Quoted 26 lines · Hiroshi DOYU

Quoted 18 lines
From: "ext Felipe Balbi" <[email protected]>
Subject: [patch] omap: usb: ehci: fix use of hcd->regs
Date: Fri, 12 Sep 2008 15:10:24 +0300
Quoted 14 lines
hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.
Signed-off-by Felipe Balbi <[email protected]>
---
drivers/usb/host/ehci-omap.c +0 -0
index 8f122e5..a67533d 100644
@@ -438,7 +438,7 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
        hcd->rsrc_start = dev->resource[0].start;
        hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
 
Quoted 4 lines
drivers/usb/host/ehci-omap.c static int ehci_hcd_omap_drv_probe(struct platform_device *dev) · line 441
Quoted 2 lines
drivers/usb/host/ehci-omap.c +1 -1
-       hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+       hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
Return value check?
Good one. Here's a new version:

Quoted 7 lines

===== cut here ====
From: Felipe Balbi <[email protected]>
Subject: [patch] omap: usb: ehci: fix usb of hcd->regs
hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.

Acked-by Hiroshi DOYU <[email protected]>

Quoted 1 line

Signed-off-by Felipe Balbi <[email protected]>

Pandita, Vikram

2008-09-14 18:53 UTC #4 lore raw

Felipe

Quoted 11 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Hiroshi
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
drivers/usb/host/ehci-omap.c +0 -0
index 8f122e5..a67533d 100644
@@ -438,7 +438,7 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
        hcd->rsrc_start = dev->resource[0].start;
        hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
 

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
drivers/usb/host/ehci-omap.c static int ehci_hcd_omap_drv_probe(struct platform_device *dev) · line 441
Quoted 2 lines
drivers/usb/host/ehci-omap.c +1 -1
-       hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+       hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);

I have seen this ioremap() call causes problems on OMAP34xx/24xx as the reg memory is already mapped. Will have to check on the board once.

Quoted 20 lines

Quoted 5 lines
Quoted 2 lines
Return value check?
Good one. Here's a new version:
Quoted 7 lines
===== cut here ====
From: Felipe Balbi <[email protected]>
Subject: [patch] omap: usb: ehci: fix usb of hcd->regs
hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.
Acked-by Hiroshi DOYU <[email protected]>
Quoted 1 line
Signed-off-by Felipe Balbi <[email protected]>
--
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

David Brownell

2008-09-12 16:32 UTC #5 lore raw

In reply to Felipe Balbi

Quoted 2 lines · Felipe Balbi

- hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+ hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);

This is missing its sibling iounmap() ...

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

Felipe Balbi

2008-09-12 17:50 UTC #6 lore raw

Quoted 5 lines · David Brownell

Quoted 2 lines · Felipe Balbi
- hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+ hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
This is missing its sibling iounmap() ...

God... thanks Dave. Good catch. Patch coming soonish.

Signature · 5 lines

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

Felipe Balbi

2008-09-12 18:23 UTC #7 lore raw

Quoted 8 lines · Felipe Balbi

Quoted 5 lines · David Brownell
Quoted 2 lines · Felipe Balbi
- hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+ hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
This is missing its sibling iounmap() ...
God... thanks Dave. Good catch. Patch coming soonish.

Here's the updated version. While at it, I'll send another patch getting rid of some other mistakes during probe.

===== cut here ====

From 818dc8ebd261cdc59670fe1580248ad47f18a1de Mon Sep 17 00:00:00 2001
From: Felipe Balbi <[email protected]>
Date: Fri, 12 Sep 2008 21:07:09 +0300
Subject: [PATCH] omap: usb: ehci: fix usb of hcd->regs

hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.

Signed-off-by Felipe Balbi <[email protected]>

---
 drivers/usb/host/ehci-omap.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

drivers/usb/host/ehci-omap.c +8 -2

index 8f122e5..fff4195 100644
@@ -438,7 +438,11 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
 	hcd->rsrc_start = dev->resource[0].start;
 	hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
 
-	hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+	hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+	if (!hcd->regs) {
+		dev_err(&dev->dev, "ioremap failed\n");
+		return -ENOMEM;
+	}
 
 	ehci = hcd_to_ehci(hcd);
 	ehci->caps = hcd->regs;

@@ -457,8 +461,9 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
 
 	dev_dbg(hcd->self.controller, "ERR: add_hcd");
 	omap_stop_ehc(dev, hcd);
-
+	iounmap(hcd->regs);
 	usb_put_hcd(hcd);
+
 	return retval;
 }

@@ -483,6 +488,7 @@ static int ehci_hcd_omap_drv_remove(struct platform_device *dev)
 
 	dev_dbg(&dev->dev, "ehci_hcd_omap_drv_remove()");
 
+	iounmap(hcd->regs);
 	usb_remove_hcd(hcd);
 	usb_put_hcd(hcd);
 	omap_stop_ehc(dev, hcd);

Signature · 8 lines

1.6.0.1.196.g01914

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

Felipe Balbi

2008-09-12 18:38 UTC #8 lore raw

RFC PATCH 1/1 clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

This one is RFC since I need someone to test it for me. I'm home now without any hw so if anyone could validate if it works I'd be really glad otherwise I'll test it tomorrow at work on my sdp.

===== cut here =====

Quoted 1 line

From 194d26cae0e9093c4b1da4de531f6e5c16495678 Mon Sep 17 00:00:00 2001

From: Felipe Balbi <[email protected]>
Date: Fri, 12 Sep 2008 21:36:42 +0300
Subject: [RFC][PATCH] omap: usb: ehci: clean up to ehci-omap.c

This patch fixes a few mistakes on the ehci-omap driver and cleans up a few style issues. It should be broken into smaller pieces but for now I just wanna get the feeling about the patch from the other guys, so here it goes.

Signed-off-by Felipe Balbi <[email protected]>

---
 drivers/usb/host/ehci-omap.c |  152 ++++++++++++++++++++++--------------------
 1 files changed, 81 insertions(+), 71 deletions(-)

drivers/usb/host/ehci-omap.c +81 ~~-71~~

index fff4195..4f823c5 100644
@@ -5,7 +5,7 @@
  * Tested on OMAP3430 ES2.0 SDP
  *
  * Copyright (C) 2007-2008 Texas Instruments, Inc.
- * 	Author: Vikram Pandita <[email protected]>
+ *	Author: Vikram Pandita <[email protected]>
  *
  * Based on "ehci-fsl.c" and "ehci-au1xxx.c" ehci glue layers
  *

@@ -31,7 +31,6 @@
 
 #include "ehci-omap.h"
 
-
 #ifdef CONFIG_OMAP_EHCI_PHY_MODE
 /* EHCI connected to External PHY */

@@ -42,10 +41,10 @@
 
 /* ISSUE1:
  *      ISP1504 for input clocking mode needs special reset handling
- * 	Hold the PHY in reset by asserting RESET_N signal
- * 	Then start the 60Mhz clock input to PHY
- * 	Release the reset after a delay -
- * 		to get the PHY state machine in working state
+ *	Hold the PHY in reset by asserting RESET_N signal
+ *	Then start the 60Mhz clock input to PHY
+ *	Release the reset after a delay -
+ *		to get the PHY state machine in working state
  */
 #define EXTERNAL_PHY_RESET
 #define	EXT_PHY_RESET_GPIO_PORT1	(57)

@@ -89,10 +88,10 @@ static void omap_usb_utmi_init(struct usb_hcd *hcd, u8 tll_channel_mask)
 
 	/* Use UTMI Ports of TLL */
 	omap_writel((1 << OMAP_UHH_HOSTCONFIG_ULPI_BYPASS_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN_SHIFT)|
-			(0<<OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN_SHIFT),
+			(1 << OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN_SHIFT)|
+			(1 << OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN_SHIFT)|
+			(1 << OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN_SHIFT)|
+			(0 << OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN_SHIFT),
 						OMAP_UHH_HOSTCONFIG);
 	/* Enusre bit is set */
 	while (!(omap_readl(OMAP_UHH_HOSTCONFIG) &

@@ -106,15 +105,15 @@ static void omap_usb_utmi_init(struct usb_hcd *hcd, u8 tll_channel_mask)
 
 		/* Disable AutoIdle */
 		omap_writel(omap_readl(OMAP_TLL_CHANNEL_CONF(i)) &
-			    ~(1<<OMAP_TLL_CHANNEL_CONF_UTMIAUTOIDLE_SHIFT),
+			    ~(1 << OMAP_TLL_CHANNEL_CONF_UTMIAUTOIDLE_SHIFT),
 			    OMAP_TLL_CHANNEL_CONF(i));
 		/* Disable BitStuffing */
 		omap_writel(omap_readl(OMAP_TLL_CHANNEL_CONF(i)) &
-			    ~(1<<OMAP_TLL_CHANNEL_CONF_ULPINOBITSTUFF_SHIFT),
+			    ~(1 << OMAP_TLL_CHANNEL_CONF_ULPINOBITSTUFF_SHIFT),
 			    OMAP_TLL_CHANNEL_CONF(i));
 		/* SDR Mode */
 		omap_writel(omap_readl(OMAP_TLL_CHANNEL_CONF(i)) &
-			    ~(1<<OMAP_TLL_CHANNEL_CONF_ULPIDDRMODE_SHIFT),
+			    ~(1 << OMAP_TLL_CHANNEL_CONF_ULPIDDRMODE_SHIFT),
 			    OMAP_TLL_CHANNEL_CONF(i));
 
 	}

@@ -130,11 +129,11 @@ static void omap_usb_utmi_init(struct usb_hcd *hcd, u8 tll_channel_mask)
 	for (i = 0; i < OMAP_TLL_CHANNEL_COUNT; i++) {
 
 		/* Enable only the channel that is needed */
-		if (!(tll_channel_mask & 1<<i))
+		if (!(tll_channel_mask & 1 << i))
 			continue;
 
 		omap_writel(omap_readl(OMAP_TLL_CHANNEL_CONF(i)) |
-			    (1<<OMAP_TLL_CHANNEL_CONF_CHANEN_SHIFT),
+			    (1 << OMAP_TLL_CHANNEL_CONF_CHANEN_SHIFT),
 			    OMAP_TLL_CHANNEL_CONF(i));
 
 		omap_writeb(0xBE, OMAP_TLL_ULPI_SCRATCH_REGISTER(i));

@@ -262,7 +261,7 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 
 	dev_dbg(hcd->self.controller, "\n TLL RESET DONE\n");
 
-	/* (1<<3) = no idle mode only for initial debugging */
+	/* (1 << 3) = no idle mode only for initial debugging */
 	omap_writel((1 << OMAP_USBTLL_SYSCONFIG_ENAWAKEUP_SHIFT) |
 			(1 << OMAP_USBTLL_SYSCONFIG_SIDLEMODE_SHIFT) |
 			(1 << OMAP_USBTLL_SYSCONFIG_CACTIVITY_SHIFT),

@@ -280,10 +279,10 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 #ifdef CONFIG_OMAP_EHCI_PHY_MODE
 	/* Bypass the TLL module for PHY mode operation */
 	omap_writel((0 << OMAP_UHH_HOSTCONFIG_ULPI_BYPASS_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN_SHIFT)|
-			(0<<OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN_SHIFT),
+			(1 << OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN_SHIFT)|
+			(1 << OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN_SHIFT)|
+			(1 << OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN_SHIFT)|
+			(0 << OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN_SHIFT),
 						OMAP_UHH_HOSTCONFIG);
 	/* Ensure that BYPASS is set */
 	while (omap_readl(OMAP_UHH_HOSTCONFIG) &

@@ -313,7 +312,7 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 	/* Refer ISSUE2: LINK assumes external charge pump */
 
 	/* use Port1 VBUS to charge externally Port2:
-	 * 	So for PHY mode operation use Port2 only
+	 *	So for PHY mode operation use Port2 only
 	 */
 	omap_writel((0xA << EHCI_INSNREG05_ULPI_REGADD_SHIFT) |/* OTG ctrl reg*/
 			(2 << EHCI_INSNREG05_ULPI_OPSEL_SHIFT) |/*   Write */

@@ -323,7 +322,7 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 			EHCI_INSNREG05_ULPI);
 
 	while (!(omap_readl(EHCI_INSNREG05_ULPI) &
-		(1<<EHCI_INSNREG05_ULPI_CONTROL_SHIFT)));
+		(1 << EHCI_INSNREG05_ULPI_CONTROL_SHIFT)));
 
 #endif

@@ -342,16 +341,16 @@ static void omap_stop_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 	dev_dbg(hcd->self.controller, ": stopping TI EHCI USB Controller\n");
 
 	/* Reset OMAP modules for insmod/rmmod to work */
-	omap_writel((1<<1), OMAP_UHH_SYSCONFIG);
-	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1<<0)));
-	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1<<1)));
-	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1<<2)));
+	omap_writel((1 << 1), OMAP_UHH_SYSCONFIG);
+	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1 << 0)));
+	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1 << 1)));
+	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1 << 2)));
 	dev_dbg(hcd->self.controller,
 		"UHH RESET DONE OMAP_UHH_SYSSTATUS %x !!\n",
 			omap_readl(OMAP_UHH_SYSSTATUS));
 
-	omap_writel((1<<1), OMAP_USBTLL_SYSCONFIG);
-	while (!(omap_readl(OMAP_USBTLL_SYSSTATUS) & (1<<0)));
+	omap_writel((1 << 1), OMAP_USBTLL_SYSCONFIG);
+	while (!(omap_readl(OMAP_USBTLL_SYSSTATUS) & (1 << 0)));
 	dev_dbg(hcd->self.controller, ":TLL RESEET DONE");
 
 	if (ehci_clocks->usbtll_fck_clk != NULL) {

@@ -412,36 +411,42 @@ static const struct hc_driver ehci_omap_hc_driver;
 static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
 {
 	int retval = 0;
+	int irq = platform_get_irq(dev, 0);
 	struct usb_hcd *hcd;
 	struct ehci_hcd *ehci;
+	struct resource *res;
 
 	dev_dbg(&dev->dev, "ehci_hcd_omap_drv_probe()");
 
-	if (usb_disabled())
-		return -ENODEV;
+	if (usb_disabled()) {
+		retval = -ENODEV;
+		goto err_disabled;
+	}
 
-	if (dev->resource[1].flags != IORESOURCE_IRQ) {
-		dev_dbg(&dev->dev, "resource[1] is not IORESOURCE_IRQ");
+	hcd = usb_create_hcd(&ehci_omap_hc_driver, &dev->dev, dev->dev.bus_id);
+	if (!hcd) {
 		retval = -ENOMEM;
+		goto err_hcd;
 	}
 
-	hcd = usb_create_hcd(&ehci_omap_hc_driver, &dev->dev, dev->dev.bus_id);
-	if (!hcd)
-		return -ENOMEM;
+	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+	if (!res || irq == 0) {
+		retval = -EINVAL;
+		goto err_resource;
+	}
 
 	retval = omap_start_ehc(dev, hcd);
 	if (retval)
-		return retval;
+		goto err_start;
 
-	hcd->rsrc_start = 0;
-	hcd->rsrc_len = 0;
-	hcd->rsrc_start = dev->resource[0].start;
-	hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
+	hcd->rsrc_start = res->start;
+	hcd->rsrc_len = res->end - res->start + 1;
 
 	hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
 	if (!hcd->regs) {
 		dev_err(&dev->dev, "ioremap failed\n");
-		return -ENOMEM;
+		retval = -ENOMEM;
+		goto err_ioremap;
 	}
 
 	ehci = hcd_to_ehci(hcd);

@@ -452,18 +457,36 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
 	ehci->hcs_params = readl(&ehci->caps->hcs_params);
 
 	/* SET 1 micro-frame Interrupt interval */
-	writel(readl(&ehci->regs->command) | (1<<16), &ehci->regs->command);
+	writel(readl(&ehci->regs->command) | (1 << 16), &ehci->regs->command);
 
-	retval = usb_add_hcd(hcd, dev->resource[1].start,
-				IRQF_DISABLED | IRQF_SHARED);
-	if (retval == 0)
-		return retval;
+	retval = usb_add_hcd(hcd, irq, IRQF_DISABLED | IRQF_SHARED);
+	if (retval) {
+		dev_err(&dev->dev, "failed to add hcd\n");
+		goto err_add_hcd;
+	}
 
-	dev_dbg(hcd->self.controller, "ERR: add_hcd");
-	omap_stop_ehc(dev, hcd);
+	return 0;
+
+err_add_hcd:
+	ehci->hcs_params = 0;
+	ehci->regs = NULL;
+	ehci->caps = NULL;
+	ehci = NULL;
 	iounmap(hcd->regs);
+	omap_stop_ehc(dev, hcd);
+
+err_ioremap:
+	hcd->rsrc_start = 0;
+	hcd->rsrc_len = 0;
+
+err_start:
+	usb_remove_hcd(hcd);
 	usb_put_hcd(hcd);
 
+err_hcd:
+err_resource:
+err_disabled:
+	dev_dbg(&dev->dev, "failed to initialized host controller\n");
 	return retval;
 }

@@ -497,18 +520,6 @@ static int ehci_hcd_omap_drv_remove(struct platform_device *dev)
 }
 
 /*-------------------------------------------------------------------------*/
-#ifdef CONFIG_PM
-static int omap_ehci_bus_suspend(struct usb_hcd *hcd)
-{
-	return ehci_bus_suspend(hcd);
-}
-
-static int omap_ehci_bus_resume(struct usb_hcd *hcd)
-{
-	return ehci_bus_resume(hcd);
-}
-#endif
-/*-------------------------------------------------------------------------*/
 
 static const struct hc_driver ehci_omap_hc_driver = {
 	.description = hcd_name,

@@ -548,21 +559,20 @@ static const struct hc_driver ehci_omap_hc_driver = {
 	.hub_status_data = ehci_hub_status_data,
 	.hub_control = ehci_hub_control,
 #ifdef	CONFIG_PM
-	.bus_suspend = omap_ehci_bus_suspend,
-	.bus_resume = omap_ehci_bus_resume,
+	.bus_suspend = ehci_bus_suspend,
+	.bus_resume = ehci_bus_resume,
 #endif
 };
 
 /*-------------------------------------------------------------------------*/
-MODULE_ALIAS("omap-ehci");
+MODULE_ALIAS("plaform:omap-ehci");
 static struct platform_driver ehci_hcd_omap_driver = {
-	.probe = ehci_hcd_omap_drv_probe,
-	.remove = ehci_hcd_omap_drv_remove,
-	.shutdown = usb_hcd_platform_shutdown,
-	/*.suspend      = ehci_hcd_omap_drv_suspend, */
-	/*.resume       = ehci_hcd_omap_drv_resume, */
-	.driver = {
-		.name = "ehci-omap",
-		.bus = &platform_bus_type
+	.probe		= ehci_hcd_omap_drv_probe,
+	.remove		= ehci_hcd_omap_drv_remove,
+	.shutdown	= usb_hcd_platform_shutdown,
+	/*.suspend	= ehci_hcd_omap_drv_suspend, */
+	/*.resume	= ehci_hcd_omap_drv_resume, */
+	.driver		= {
+		.name	= "ehci-omap",
 	}
 };

Signature · 4 lines

1.6.0.1.196.g01914

-- 
balbi

David Brownell

2008-09-12 19:16 UTC #9 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 1 line · Felipe Balbi

/* Enusre bit is set */

My Cortex Mark-I (cerebral, humanoid) treats that as a compile error ... :)

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

Tony Lindgren

2008-09-12 19:22 UTC #10 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 4 lines · David Brownell

Quoted 1 line · Felipe Balbi
/* Enusre bit is set */
My Cortex Mark-I (cerebral, humanoid) treats that as a compile error ... :)

Have you tried preprocessing with squint? :)

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

Felipe Balbi

2008-09-12 19:28 UTC #11 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 7 lines · Tony Lindgren

Quoted 4 lines · David Brownell
Quoted 1 line · Felipe Balbi
/* Enusre bit is set */
My Cortex Mark-I (cerebral, humanoid) treats that as a compile error ... :)
Have you tried preprocessing with squint? :)

ehehe, it's not in my diff, so it wasn't me. :-p
I'll fix it in the final clean up

Signature · 5 lines

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

Felipe Balbi

2008-09-12 19:30 UTC #12 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 11 lines · Felipe Balbi

Quoted 7 lines · Tony Lindgren
Quoted 4 lines · David Brownell
Quoted 1 line · Felipe Balbi
/* Enusre bit is set */
My Cortex Mark-I (cerebral, humanoid) treats that as a compile error ... :)
Have you tried preprocessing with squint? :)
ehehe, it's not in my diff, so it wasn't me. :-p
I'll fix it in the final clean up

btw, when the "virtual clock" structure come from Hiroshi and/or Paul, the clock handling on ehci-omap could get some rewriting as well ;-)

Signature · 5 lines

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

Felipe Balbi

2008-09-12 19:35 UTC #13 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

In reply to Felipe Balbi

Hi Vikram,

Those "issues" mentioned on the ehci-omap driver, are them silicon version related ? I mean, does it happen on all omap3 silicons or doesn't it happen with only versions of them ?

Case the second, we can make the workarounds be applied based on runtime checks instead of #ifdefs. That would look nice.

Also, it looks like PHY and TLL mode will be board specific, which means we can use a platform_data to pass the correct mode to the driver and get rid of those #ifdefs and Kconfig choices as well.

Any comments ?

Signature · 1 line

balbi

Pandita, Vikram

2008-09-14 18:43 UTC #14 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 8 lines

-----Original Message-----
From: Felipe Balbi [mailto:[email protected]]
Sent: Saturday, September 13, 2008 1:06 AM
To: Felipe Balbi
Cc: Tony Lindgren; David Brownell; [email protected]; Linux OMAP Mailing List; Pandita, Vikram
Subject: Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd-

Quoted 7 lines

Quoted 1 line
regs)
Hi Vikram,
Those "issues" mentioned on the ehci-omap driver, are them silicon version related ? I mean, does it happen on all omap3 silicons or doesn't it happen with only versions of them ?

The issues mentioned are _not_ of omap34xx silicon, but of the PHY (ISP1504 in SDP-USB expansion board).

Quoted 3 lines

Case the second, we can make the workarounds be applied based on runtime checks instead of #ifdefs. That would look nice.

Better still is to move the PHY issues to platform/board specific function. Will try to submit a patch on that.

Quoted 4 lines

Also, it looks like PHY and TLL mode will be board specific, which means we can use a platform_data to pass the correct mode to the driver and get rid of those #ifdefs and Kconfig choices as well.

Yes that can be done.

Quoted 5 lines

Any comments ?
--
balbi

Felipe Balbi

2008-09-14 18:58 UTC #15 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Hi,

Quoted 1 line · Vikram

The issues mentioned are _not_ of omap34xx silicon, but of the PHY (ISP1504 in SDP-USB expansion board).

Gotcha

Quoted 2 lines

Better still is to move the PHY issues to platform/board specific function. Will try to submit a patch on that.

sounds good to me ;-)

If you can make your patch on top of the ones I have queued for sending, I'd be glad. Also, if you could test them for me, that would be quite nice since I only have a buggy version of beagleboard with one ehci port.

I'm sending my patches attached for you, there's actually one from you supporting the new version of the expansion board.

Signature · 1 line

balbi

From bfae28bb4dfa0198bd30584fcfa74682947053eb Mon Sep 17 00:00:00 2001
From: Felipe Balbi <[email protected]>
Date: Sat, 13 Sep 2008 18:21:37 +0300
Subject: [PATCH 1/4] omap: usb: ehci: clean up ehci-omap.c

General cleanup for ehci-omap.c

Signed-off-by Felipe Balbi <[email protected]>

---
 drivers/usb/host/ehci-omap.c |  135 +++++++++++++++++-------------------------
 1 files changed, 55 insertions(+), 80 deletions(-)

drivers/usb/host/ehci-omap.c +55 ~~-80~~

index fff4195..f1b5ea5 100644
@@ -5,7 +5,7 @@
  * Tested on OMAP3430 ES2.0 SDP
  *
  * Copyright (C) 2007-2008 Texas Instruments, Inc.
- * 	Author: Vikram Pandita <[email protected]>
+ *	Author: Vikram Pandita <[email protected]>
  *
  * Based on "ehci-fsl.c" and "ehci-au1xxx.c" ehci glue layers
  *

@@ -31,7 +31,6 @@
 
 #include "ehci-omap.h"
 
-
 #ifdef CONFIG_OMAP_EHCI_PHY_MODE
 /* EHCI connected to External PHY */

@@ -42,10 +41,10 @@
 
 /* ISSUE1:
  *      ISP1504 for input clocking mode needs special reset handling
- * 	Hold the PHY in reset by asserting RESET_N signal
- * 	Then start the 60Mhz clock input to PHY
- * 	Release the reset after a delay -
- * 		to get the PHY state machine in working state
+ *	Hold the PHY in reset by asserting RESET_N signal
+ *	Then start the 60Mhz clock input to PHY
+ *	Release the reset after a delay -
+ *		to get the PHY state machine in working state
  */
 #define EXTERNAL_PHY_RESET
 #define	EXT_PHY_RESET_GPIO_PORT1	(57)

@@ -89,16 +88,16 @@ static void omap_usb_utmi_init(struct usb_hcd *hcd, u8 tll_channel_mask)
 
 	/* Use UTMI Ports of TLL */
 	omap_writel((1 << OMAP_UHH_HOSTCONFIG_ULPI_BYPASS_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN_SHIFT)|
-			(0<<OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN_SHIFT),
+			(1 << OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN_SHIFT)|
+			(1 << OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN_SHIFT)|
+			(1 << OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN_SHIFT)|
+			(0 << OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN_SHIFT),
 						OMAP_UHH_HOSTCONFIG);
-	/* Enusre bit is set */
+	/* Ensure bit is set */
 	while (!(omap_readl(OMAP_UHH_HOSTCONFIG) &
 		(1 << OMAP_UHH_HOSTCONFIG_ULPI_BYPASS_SHIFT)));
 
-	dev_dbg(hcd->self.controller, "\nEntered UTMI MODE: success\n");
+	dev_dbg(hcd->self.controller, "Entered UTMI MODE: success\n");
 
 	/* Program the 3 TLL channels upfront */

@@ -106,15 +105,15 @@ static void omap_usb_utmi_init(struct usb_hcd *hcd, u8 tll_channel_mask)
 
 		/* Disable AutoIdle */
 		omap_writel(omap_readl(OMAP_TLL_CHANNEL_CONF(i)) &
-			    ~(1<<OMAP_TLL_CHANNEL_CONF_UTMIAUTOIDLE_SHIFT),
+			    ~(1 << OMAP_TLL_CHANNEL_CONF_UTMIAUTOIDLE_SHIFT),
 			    OMAP_TLL_CHANNEL_CONF(i));
 		/* Disable BitStuffing */
 		omap_writel(omap_readl(OMAP_TLL_CHANNEL_CONF(i)) &
-			    ~(1<<OMAP_TLL_CHANNEL_CONF_ULPINOBITSTUFF_SHIFT),
+			    ~(1 << OMAP_TLL_CHANNEL_CONF_ULPINOBITSTUFF_SHIFT),
 			    OMAP_TLL_CHANNEL_CONF(i));
 		/* SDR Mode */
 		omap_writel(omap_readl(OMAP_TLL_CHANNEL_CONF(i)) &
-			    ~(1<<OMAP_TLL_CHANNEL_CONF_ULPIDDRMODE_SHIFT),
+			    ~(1 << OMAP_TLL_CHANNEL_CONF_ULPIDDRMODE_SHIFT),
 			    OMAP_TLL_CHANNEL_CONF(i));
 
 	}

@@ -130,17 +129,17 @@ static void omap_usb_utmi_init(struct usb_hcd *hcd, u8 tll_channel_mask)
 	for (i = 0; i < OMAP_TLL_CHANNEL_COUNT; i++) {
 
 		/* Enable only the channel that is needed */
-		if (!(tll_channel_mask & 1<<i))
+		if (!(tll_channel_mask & 1 << i))
 			continue;
 
 		omap_writel(omap_readl(OMAP_TLL_CHANNEL_CONF(i)) |
-			    (1<<OMAP_TLL_CHANNEL_CONF_CHANEN_SHIFT),
+			    (1 << OMAP_TLL_CHANNEL_CONF_CHANEN_SHIFT),
 			    OMAP_TLL_CHANNEL_CONF(i));
 
 		omap_writeb(0xBE, OMAP_TLL_ULPI_SCRATCH_REGISTER(i));
-		dev_dbg(hcd->self.controller, "\nULPI_SCRATCH_REG[ch=%d]"
-			"= 0x%02x\n",
-			i+1, omap_readb(OMAP_TLL_ULPI_SCRATCH_REGISTER(i)));
+		dev_dbg(hcd->self.controller, "ULPI_SCRATCH_REG[ch=%d]"
+			"= 0x%02x\n", i + 1,
+			omap_readb(OMAP_TLL_ULPI_SCRATCH_REGISTER(i)));
 	}
 }

@@ -150,7 +149,7 @@ static void omap_usb_utmi_init(struct usb_hcd *hcd, u8 tll_channel_mask)
 
 
 /* omap_start_ehc
- * 	- Start the TI USBHOST controller
+ *	- Start the TI USBHOST controller
  */
 static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 {

@@ -180,8 +179,7 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 
 	while (!(cm_read_mod_reg(PLL_MOD, CM_IDLEST2) &
 				OMAP3430ES2_ST_PERIPH2_CLK_MASK))
-		dev_dbg(hcd->self.controller,
-			"idlest2 = 0x%x\n",
+		dev_dbg(hcd->self.controller, "idlest2 = 0x%x\n",
 			cm_read_mod_reg(PLL_MOD, CM_IDLEST2));
 	/* End DPLL5 programming */

@@ -260,9 +258,9 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 	while (!(omap_readl(OMAP_USBTLL_SYSSTATUS) &
 		(1 << OMAP_USBTLL_SYSSTATUS_RESETDONE_SHIFT)));
 
-	dev_dbg(hcd->self.controller, "\n TLL RESET DONE\n");
+	dev_dbg(hcd->self.controller, "TLL RESET DONE\n");
 
-	/* (1<<3) = no idle mode only for initial debugging */
+	/* (1 << 3) = no idle mode only for initial debugging */
 	omap_writel((1 << OMAP_USBTLL_SYSCONFIG_ENAWAKEUP_SHIFT) |
 			(1 << OMAP_USBTLL_SYSCONFIG_SIDLEMODE_SHIFT) |
 			(1 << OMAP_USBTLL_SYSCONFIG_CACTIVITY_SHIFT),

@@ -280,10 +278,10 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 #ifdef CONFIG_OMAP_EHCI_PHY_MODE
 	/* Bypass the TLL module for PHY mode operation */
 	omap_writel((0 << OMAP_UHH_HOSTCONFIG_ULPI_BYPASS_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN_SHIFT)|
-			(1<<OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN_SHIFT)|
-			(0<<OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN_SHIFT),
+			(1 << OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN_SHIFT)|
+			(1 << OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN_SHIFT)|
+			(1 << OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN_SHIFT)|
+			(0 << OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN_SHIFT),
 						OMAP_UHH_HOSTCONFIG);
 	/* Ensure that BYPASS is set */
 	while (omap_readl(OMAP_UHH_HOSTCONFIG) &

@@ -313,7 +311,7 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 	/* Refer ISSUE2: LINK assumes external charge pump */
 
 	/* use Port1 VBUS to charge externally Port2:
-	 * 	So for PHY mode operation use Port2 only
+	 *	So for PHY mode operation use Port2 only
 	 */
 	omap_writel((0xA << EHCI_INSNREG05_ULPI_REGADD_SHIFT) |/* OTG ctrl reg*/
 			(2 << EHCI_INSNREG05_ULPI_OPSEL_SHIFT) |/*   Write */

@@ -323,7 +321,7 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 			EHCI_INSNREG05_ULPI);
 
 	while (!(omap_readl(EHCI_INSNREG05_ULPI) &
-		(1<<EHCI_INSNREG05_ULPI_CONTROL_SHIFT)));
+		(1 << EHCI_INSNREG05_ULPI_CONTROL_SHIFT)));
 
 #endif

@@ -342,17 +340,16 @@ static void omap_stop_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 	dev_dbg(hcd->self.controller, ": stopping TI EHCI USB Controller\n");
 
 	/* Reset OMAP modules for insmod/rmmod to work */
-	omap_writel((1<<1), OMAP_UHH_SYSCONFIG);
-	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1<<0)));
-	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1<<1)));
-	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1<<2)));
-	dev_dbg(hcd->self.controller,
-		"UHH RESET DONE OMAP_UHH_SYSSTATUS %x !!\n",
+	omap_writel((1 << 1), OMAP_UHH_SYSCONFIG);
+	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1 << 0)));
+	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1 << 1)));
+	while (!(omap_readl(OMAP_UHH_SYSSTATUS) & (1 << 2)));
+	dev_dbg(hcd->self.controller, "UHH RESET DONE OMAP_UHH_SYSSTATUS %x\n",
 			omap_readl(OMAP_UHH_SYSSTATUS));
 
-	omap_writel((1<<1), OMAP_USBTLL_SYSCONFIG);
-	while (!(omap_readl(OMAP_USBTLL_SYSSTATUS) & (1<<0)));
-	dev_dbg(hcd->self.controller, ":TLL RESEET DONE");
+	omap_writel((1 << 1), OMAP_USBTLL_SYSCONFIG);
+	while (!(omap_readl(OMAP_USBTLL_SYSSTATUS) & (1 << 0)));
+	dev_dbg(hcd->self.controller, ": TLL RESEET DONE");
 
 	if (ehci_clocks->usbtll_fck_clk != NULL) {
 		clk_disable(ehci_clocks->usbtll_fck_clk);

@@ -452,7 +449,7 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
 	ehci->hcs_params = readl(&ehci->caps->hcs_params);
 
 	/* SET 1 micro-frame Interrupt interval */
-	writel(readl(&ehci->regs->command) | (1<<16), &ehci->regs->command);
+	writel(readl(&ehci->regs->command) | (1 << 16), &ehci->regs->command);
 
 	retval = usb_add_hcd(hcd, dev->resource[1].start,
 				IRQF_DISABLED | IRQF_SHARED);

@@ -497,18 +494,6 @@ static int ehci_hcd_omap_drv_remove(struct platform_device *dev)
 }
 
 /*-------------------------------------------------------------------------*/
-#ifdef CONFIG_PM
-static int omap_ehci_bus_suspend(struct usb_hcd *hcd)
-{
-	return ehci_bus_suspend(hcd);
-}
-
-static int omap_ehci_bus_resume(struct usb_hcd *hcd)
-{
-	return ehci_bus_resume(hcd);
-}
-#endif
-/*-------------------------------------------------------------------------*/
 
 static const struct hc_driver ehci_omap_hc_driver = {
 	.description = hcd_name,

@@ -516,53 +501,43 @@ static const struct hc_driver ehci_omap_hc_driver = {
 	.hcd_priv_size = sizeof(struct ehci_hcd)
 				+ sizeof(struct ehci_omap_clock_defs),
 
-	/*
-	 * generic hardware linkage
-	 */
+	/* generic hardware linkage */
 	.irq = ehci_irq,
 	.flags = HCD_MEMORY | HCD_USB2,
 
-	/*
-	 * basic lifecycle operations
-	 */
+	/* basic lifecycle operations */
 	.reset = ehci_init,
 	.start = ehci_run,
 	.stop = ehci_stop,
 	.shutdown = ehci_shutdown,
 
-	/*
-	 * managing i/o requests and associated device resources
-	 */
+	/* managing i/o requests and associated device resources */
 	.urb_enqueue = ehci_urb_enqueue,
 	.urb_dequeue = ehci_urb_dequeue,
 	.endpoint_disable = ehci_endpoint_disable,
 
-	/*
-	 * scheduling support
-	 */
+	/* scheduling support */
 	.get_frame_number = ehci_get_frame,
 
-	/*
-	 * root hub support
-	 */
+	/* root hub support */
 	.hub_status_data = ehci_hub_status_data,
 	.hub_control = ehci_hub_control,
 #ifdef	CONFIG_PM
-	.bus_suspend = omap_ehci_bus_suspend,
-	.bus_resume = omap_ehci_bus_resume,
+	.bus_suspend = ehci_bus_suspend,
+	.bus_resume = ehci_bus_resume,
 #endif
 };
 
-/*-------------------------------------------------------------------------*/
-MODULE_ALIAS("omap-ehci");
+MODULE_AUTHOR("Texas Instruments, Inc.");
+MODULE_ALIAS("plaform:omap-ehci");
+
 static struct platform_driver ehci_hcd_omap_driver = {
-	.probe = ehci_hcd_omap_drv_probe,
-	.remove = ehci_hcd_omap_drv_remove,
-	.shutdown = usb_hcd_platform_shutdown,
-	/*.suspend      = ehci_hcd_omap_drv_suspend, */
-	/*.resume       = ehci_hcd_omap_drv_resume, */
-	.driver = {
-		.name = "ehci-omap",
-		.bus = &platform_bus_type
+	.probe		= ehci_hcd_omap_drv_probe,
+	.remove		= ehci_hcd_omap_drv_remove,
+	.shutdown	= usb_hcd_platform_shutdown,
+	/*.suspend	= ehci_hcd_omap_drv_suspend, */
+	/*.resume	= ehci_hcd_omap_drv_resume, */
+	.driver		= {
+		.name	= "ehci-omap",
 	}
 };

Signature · 1 line

1.6.0.1.196.g01914

From 14a61159787a5797340445594f57f3921d3ab7e8 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <[email protected]>
Date: Sat, 13 Sep 2008 18:14:00 +0300
Subject: [PATCH 2/4] omap: usb: ehci: switch to use gpiolib

Quit using omap-specific gpio handling and start using gpiolib for ehci-omap.c

Signed-off-by Felipe Balbi <[email protected]>

---
 drivers/usb/host/ehci-omap.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

drivers/usb/host/ehci-omap.c +9 ~~-11~~

index f1b5ea5..c73f832 100644
@@ -27,7 +27,7 @@
 
 #include <linux/platform_device.h>
 #include <linux/clk.h>
-#include <mach/gpio.h>
+#include <linux/gpio.h>
 
 #include "ehci-omap.h"

@@ -222,12 +222,10 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 
 #ifdef EXTERNAL_PHY_RESET
 	/* Refer: ISSUE1 */
-	omap_request_gpio(EXT_PHY_RESET_GPIO_PORT1);
-	omap_set_gpio_direction(EXT_PHY_RESET_GPIO_PORT1, 0);
-	omap_request_gpio(EXT_PHY_RESET_GPIO_PORT2);
-	omap_set_gpio_direction(EXT_PHY_RESET_GPIO_PORT2, 0);
-	omap_set_gpio_dataout(EXT_PHY_RESET_GPIO_PORT1, 0);
-	omap_set_gpio_dataout(EXT_PHY_RESET_GPIO_PORT2, 0);
+	gpio_request(EXT_PHY_RESET_GPIO_PORT1, "reset_port1");
+	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT1, 0);
+	gpio_request(EXT_PHY_RESET_GPIO_PORT2, "reset_port2");
+	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT2, 0);
 	/* Hold the PHY in RESET for enough time till DIR is high */
 	udelay(EXT_PHY_RESET_DELAY);
 #endif

@@ -303,8 +301,8 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 	 * Hold the PHY in RESET for enough time till PHY is settled and ready
 	 */
 	udelay(EXT_PHY_RESET_DELAY);
-	omap_set_gpio_dataout(EXT_PHY_RESET_GPIO_PORT1, 1);
-	omap_set_gpio_dataout(EXT_PHY_RESET_GPIO_PORT2, 1);
+	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT1, 1);
+	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT2, 1);
 #endif
 
 #ifdef VBUS_INTERNAL_CHARGEPUMP_HACK

@@ -383,8 +381,8 @@ static void omap_stop_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 
 
 #ifdef EXTERNAL_PHY_RESET
-	omap_free_gpio(EXT_PHY_RESET_GPIO_PORT1);
-	omap_free_gpio(EXT_PHY_RESET_GPIO_PORT2);
+	gpio_free(EXT_PHY_RESET_GPIO_PORT1);
+	gpio_free(EXT_PHY_RESET_GPIO_PORT2);
 #endif
 
 	dev_dbg(hcd->self.controller,

Signature · 1 line

1.6.0.1.196.g01914

From 7e25b8e03aec867002163b8618c126f16682ff52 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <[email protected]>
Date: Sat, 13 Sep 2008 18:25:00 +0300
Subject: [PATCH 3/4] omap: usb: ehci: fix usage of resources

use platform_get_irq() and platform_get_resource instead of trying to access struct resource * directly.

Signed-off-by Felipe Balbi <[email protected]>

---
 drivers/usb/host/ehci-omap.c |   62 +++++++++++++++++++++++++++++-------------
 1 files changed, 43 insertions(+), 19 deletions(-)

drivers/usb/host/ehci-omap.c +43 ~~-19~~

95 lines not on this page · raw message

Signature · 1 line

1.6.0.1.196.g01914

From 775b30a9724fa3274702621b8e595ad35374bf3b Mon Sep 17 00:00:00 2001
From: Vikram Pandita <[email protected]>
Date: Sat, 13 Sep 2008 18:31:04 +0300
Subject: [PATCH 4/4] omap: usb: ehci: support final revision of usb board

Code changes for final rev of EHCI/OHCI USB expansion board 750-2099-001(C) with hw bug fixes to support ISP1504 PHY in input clocking mode.

The board has 12-pin ULPI Port1 and Port2 EHCI pins connected to ISP1504's. Port3 is connected to ISP1301 for connection to OHCI.

This is the final USB expansion board that will be released and so the bug fixes are critical. Changes tested on SDP3430.

Signed-off-by Vikram Pandita <[email protected]>

Signed-off-by Felipe Balbi <[email protected]>

---
 drivers/usb/host/ehci-omap.c |   37 ++++++-------------------------------
 1 files changed, 6 insertions(+), 31 deletions(-)

drivers/usb/host/ehci-omap.c +6 ~~-31~~

index 4ac18b2..f8316d5 100644
@@ -34,7 +34,7 @@
 #ifdef CONFIG_OMAP_EHCI_PHY_MODE
 /* EHCI connected to External PHY */
 
-/* External USB connectivity board: 750-2083-001
+/* External USB connectivity board: 750-2099-001(C)
  * Connected to OMAP3430 SDP
  * The board has Port1 and Port2 connected to ISP1504 in 12-pin ULPI mode
  */

@@ -49,14 +49,7 @@
 #define EXTERNAL_PHY_RESET
 #define	EXT_PHY_RESET_GPIO_PORT1	(57)
 #define	EXT_PHY_RESET_GPIO_PORT2	(61)
-#define	EXT_PHY_RESET_DELAY		(10)
-
-/* ISSUE2:
- * USBHOST supports External charge pump PHYs only
- * Use the VBUS from Port1 to power VBUS of Port2 externally
- * So use Port2 as the working ULPI port
- */
-#define VBUS_INTERNAL_CHARGEPUMP_HACK
+#define	EXT_PHY_RESET_DELAY		(500)
 
 #endif /* CONFIG_OMAP_EHCI_PHY_MODE */

@@ -223,9 +216,9 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 #ifdef EXTERNAL_PHY_RESET
 	/* Refer: ISSUE1 */
 	gpio_request(EXT_PHY_RESET_GPIO_PORT1, "reset_port1");
-	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT1, 0);
+	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT1, 1);
 	gpio_request(EXT_PHY_RESET_GPIO_PORT2, "reset_port2");
-	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT2, 0);
+	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT2, 1);
 	/* Hold the PHY in RESET for enough time till DIR is high */
 	udelay(EXT_PHY_RESET_DELAY);
 #endif

@@ -301,26 +294,8 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 	 * Hold the PHY in RESET for enough time till PHY is settled and ready
 	 */
 	udelay(EXT_PHY_RESET_DELAY);
-	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT1, 1);
-	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT2, 1);
-#endif
-
-#ifdef VBUS_INTERNAL_CHARGEPUMP_HACK
-	/* Refer ISSUE2: LINK assumes external charge pump */
-
-	/* use Port1 VBUS to charge externally Port2:
-	 *	So for PHY mode operation use Port2 only
-	 */
-	omap_writel((0xA << EHCI_INSNREG05_ULPI_REGADD_SHIFT) |/* OTG ctrl reg*/
-			(2 << EHCI_INSNREG05_ULPI_OPSEL_SHIFT) |/*   Write */
-			(1 << EHCI_INSNREG05_ULPI_PORTSEL_SHIFT) |/* Port1 */
-			(1 << EHCI_INSNREG05_ULPI_CONTROL_SHIFT) |/* Start */
-			(0x26),
-			EHCI_INSNREG05_ULPI);
-
-	while (!(omap_readl(EHCI_INSNREG05_ULPI) &
-		(1 << EHCI_INSNREG05_ULPI_CONTROL_SHIFT)));
-
+	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT1, 0);
+	gpio_direction_output(EXT_PHY_RESET_GPIO_PORT2, 0);
 #endif
 
 	return 0;

Signature · 1 line

1.6.0.1.196.g01914

David Brownell

2008-09-16 05:53 UTC #16 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 1 line · Felipe Balbi

+MODULE_ALIAS("plaform:omap-ehci");

"platform" ...

Felipe Balbi

2008-09-16 07:27 UTC #17 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 5 lines · David Brownell

Quoted 1 line · Felipe Balbi
+MODULE_ALIAS("plaform:omap-ehci");
"platform" ...

thanks. Missed it. It's fixed now.

Signature · 1 line

balbi

David Brownell

2008-09-16 05:55 UTC #18 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

In reply to Felipe Balbi

Quoted 6 lines

Subject: [PATCH 2/4] omap: usb: ehci: switch to use gpiolib
Quit using omap-specific gpio handling and start using gpiolib for ehci-omap.c
Signed-off-by Felipe Balbi <[email protected]>

... yes, but also move all the pinmuxing to arch/arm/plat-omap/something

Gadiyar, Anand

2008-09-16 06:05 UTC #19 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 11 lines

[patch] omap: usb: ehci: fix use of hcd->regs)
Quoted 6 lines
Subject: [PATCH 2/4] omap: usb: ehci: switch to use gpiolib
Quit using omap-specific gpio handling and start using gpiolib for ehci-omap.c
Signed-off-by Felipe Balbi <[email protected]>
... yes, but also move all the pinmuxing to arch/arm/plat-omap/something

Pin-muxing is done in the board file. Is there a problem with that?

- Anand

David Brownell

2008-09-16 06:35 UTC #20 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 5 lines · Anand

Quoted 3 lines
... yes, but also move all the pinmuxing to arch/arm/plat-omap/something
Pin-muxing is done in the board file. Is there a problem with that?

Sorry, I was too loose. I meant "pin setup", which is a superset of pin muxing. The functions in the second patch should be wrapped up in board-specific code.

Felipe Balbi

2008-09-16 07:29 UTC #21 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

In reply to David Brownell

Quoted 8 lines · David Brownell

Quoted 6 lines
Subject: [PATCH 2/4] omap: usb: ehci: switch to use gpiolib
Quit using omap-specific gpio handling and start using gpiolib for ehci-omap.c
Signed-off-by Felipe Balbi <[email protected]>
... yes, but also move all the pinmuxing to arch/arm/plat-omap/something

usb_ehci_init() also needs to be changed. For now I'm only dealing with the driver itself, board init code will be dealt with on later patches.

Signature · 1 line

balbi

David Brownell

2008-09-16 05:56 UTC #22 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

In reply to Felipe Balbi

Quoted 1 line

+ dev_dbg(&dev->dev, "failed to initialized host controller\n");

... and include the status code, to help identify *WHY* it failed (this is re #3 ... should have sent three separate emails, with inlined patches)

Felipe Balbi

author

2008-09-16 08:29 UTC #23 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 5 lines · David Brownell

Quoted 1 line
+ dev_dbg(&dev->dev, "failed to initialized host controller\n");
... and include the status code, to help identify *WHY* it failed (this is re #3 ... should have sent three separate emails, with inlined patches)

I didn't really to sent them for review. Was just trying to be sure Vikram had my patches before he starts working on that driver, but sure; when they're ready, I'll send them properly. Thanks for reviewing so far ;-)

Signature · 1 line

balbi

Gadiyar, Anand

2008-09-18 09:35 UTC #24 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

In reply to Felipe Balbi

Hi Felipe, Paul,

Quoted 16 lines

-----Original Message-----
From: [email protected] On Behalf Of Felipe Balbi Hi,
Quoted 2 lines · Vikram
The issues mentioned are _not_ of omap34xx silicon, but of the PHY (ISP1504 in SDP-USB expansion board).
Gotcha
Quoted 2 lines
Better still is to move the PHY issues to platform/board specific function. Will try to submit a patch on that.
sounds good to me ;-)

Quoted 7 lines

If you can make your patch on top of the ones I have queued for sending, I'd be glad. Also, if you could test them for me, that would be quite nice since I only have a buggy version of beagleboard with one ehci port.
I'm sending my patches attached for you, there's actually one from you supporting the new version of the expansion board.

Your patches apply fine, but when you insmod the ehci module, we go into an infinite loop. Looks like some error reading a bit in CM_IDLEST2.

Attached patch removes the infinite loop for now and at least enumeration can then happen. Ideally the clock framework should be taking care of enabling DPLL5 when the corresponding clocks are enabled, but this does not happen today.

@ Paul,
Would it be possible for you to look at this please?

Regards,
Anand

Remove infinite loop in DPLL 5 programming. Ideally, DPLL5 should be taken care of by the clock framework.

---
 drivers/usb/host/ehci-omap.c |    2 ++
 1 files changed, 2 insertions(+)

drivers/usb/host/ehci-omap.c +2 -0

Index: linux-omap-2.6/drivers/usb/host/ehci-omap.c
===================================================================
@@ -170,10 +170,12 @@ static int omap_start_ehc(struct platfor
 			(7 << OMAP3430ES2_EN_PERIPH2_DPLL_SHIFT),
 			PLL_MOD, OMAP3430ES2_CM_CLKEN2);
 
+#if 0
 	while (!(cm_read_mod_reg(PLL_MOD, CM_IDLEST2) &
 				OMAP3430ES2_ST_PERIPH2_CLK_MASK))
 		dev_dbg(hcd->self.controller, "idlest2 = 0x%x\n",
 			cm_read_mod_reg(PLL_MOD, CM_IDLEST2));
+#endif
 	/* End DPLL5 programming */

Paul Walmsley

2008-09-21 23:28 UTC #25 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Hello Anand,

Quoted 2 lines · Anand

Ideally the clock framework should be taking care of enabling DPLL5 when the corresponding clocks are enabled, but this does not happen today.

It will do that now; but what it will not do is set DPLL5's rate correctly.

Quoted 2 lines

@ Paul,
Would it be possible for you to look at this please?

Yes, will send patches for this soon.

- Paul

Felipe Balbi

author

2008-09-22 15:17 UTC #26 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

In reply to Pandita, Vikram

Quoted 2 lines · Vikram Pandita

Better still is to move the PHY issues to platform/board specific function. Will try to submit a patch on that.

Any updates on this ?? Could you test the patches I sent you ? Would be nice to have them applied if they're ok.

Signature · 1 line

balbi

Gadiyar, Anand

2008-09-22 15:21 UTC #27 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 8 lines

From: [email protected] On Behalf Of Felipe Balbi
Quoted 2 lines · Vikram Pandita
Better still is to move the PHY issues to platform/board specific function. Will try to submit a patch on that.
Any updates on this ?? Could you test the patches I sent you ? Would be nice to have them applied if they're ok.

I tested the patches for you. I guess you missed my mail. It compiles and runs fine, but needs an indefinite while loop removed - haven't looked into why it fails there, but EHCI seems to work without that check.

<http://marc.info/?l=linux-omap&m=122173056311024&w=2>

Felipe Balbi

author

2008-09-22 15:27 UTC #28 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 6 lines · Anand

Quoted 6 lines
From: [email protected] On Behalf Of Felipe Balbi
Quoted 2 lines · Vikram Pandita
Better still is to move the PHY issues to platform/board specific function. Will try to submit a patch on that.

Quoted 6 lines

Quoted 2 lines
Any updates on this ?? Could you test the patches I sent you ? Would be nice to have them applied if they're ok.
I tested the patches for you. I guess you missed my mail. It compiles and runs fine, but needs an indefinite while loop removed - haven't looked into why it fails there, but EHCI seems to work without that check.

Hmm.. but that seems to be another issue, right ? Wasn't that clock-related ??

Signature · 1 line

balbi

Gadiyar, Anand

2008-09-22 15:30 UTC #29 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 7 lines · Anand

Quoted 6 lines
Quoted 6 lines
From: [email protected] On Behalf Of Felipe Balbi
Quoted 2 lines · Vikram Pandita
Better still is to move the PHY issues to platform/board specific function. Will try to submit a patch on that.

Quoted 9 lines

Quoted 6 lines
Quoted 2 lines
Any updates on this ?? Could you test the patches I sent you ? Would be nice to have them applied if they're ok.
I tested the patches for you. I guess you missed my mail. It compiles and runs fine, but needs an indefinite while loop removed - haven't looked into why it fails there, but EHCI seems to work without that check.
Hmm.. but that seems to be another issue, right ? Wasn't that clock-related ??

Yes, it was related to DPLL5 I think. But with your patches plus that one section of code commented out, EHCI was still working. So nothing broken yet.

- Anand

Felipe Balbi

author

2008-09-22 15:32 UTC #30 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 2 lines · Anand

Yes, it was related to DPLL5 I think. But with your patches plus that one section of code commented out, EHCI was still working. So nothing broken yet.

Cool, so let's wait a bit more for clock updates, then we apply those, maybe that's the way to go. I'm also waiting the virtual clock structures so we can clean up the clock handling on the ehci driver.

After that the driver could possibly be sent to mainline for integration.

Signature · 1 line

balbi

Gadiyar, Anand

2008-09-12 20:01 UTC #31 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

In reply to Felipe Balbi

Quoted 4 lines

On Behalf Of Felipe Balbi [[email protected]]
Quoted 2 lines · Tony Lindgren
Quoted 1 line · David Brownell
On Friday 12 September 2008, Felipe Balbi wrote:

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
        /* Enusre bit is set */
My Cortex Mark-I (cerebral, humanoid) treats that as a compile error ... :)
Have you tried preprocessing with squint? :)
ehehe, it's not in my diff, so it wasn't me. :-p
I'll fix it in the final clean up

Was planning on cleaning up this driver and sending to linux-omap as Tony suggested, but I'm afraid I won't be able to look at it for a while.

So if you're going to be doing this anyway, then could you please meld in Vikram's other patch from [1] as well?

By the way, how do you propose to test this driver? Do you have the expansion board for the SDP, or some other hardware that can be used?

Also, while we're discussing this driver, a few points to be resolved:

1. TLL vs PHY mode needs to be set somewhere. Me thinks this information ought to come from board-specific data and the driver would need to set things up accordingly. What would be a good way to do this?

2. The OHCI driver (not yet in linux-omap and not in a shape to go ought at the moment either) would need to touch a lot of common registers during init/exit/re-init. Any ideas where to put this code such that we can have the EHCI driver, or the OHCI driver, or both loaded and unloaded at will and still working without interfering with each other.

3. For OHCI, the transceiver used on the expansion board happens to be an ISP1301 for which there currently exists a driver (drivers/i2c/chips/isp1301_omap.c). Since this is meant to be an OTG transceiver, the driver does otg_set_transceiver.
But MUSB is the OTG controller and has it's own transceiver... and we can't have two of those, right? :) Ideas welcome.

Regards,
Anand

[1] http://marc.info/?l=linux-omap&m=121483462926647&w=2

David Brownell

2008-09-12 20:30 UTC #32 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 3 lines · Anand

1. TLL vs PHY mode needs to be set somewhere. Me thinks this information ought to come from board-specific data and the driver would need to set things up accordingly. What would be a good way to do this?

Define an ehci_omap_platform_data struct and use it for the stuff that shouldn't be in mach-omap2 ... I seem to not understand why there would be a question here.

Quoted 5 lines

2. The OHCI driver (not yet in linux-omap and not in a shape to go ought at the moment either) would need to touch a lot of common registers during init/exit/re-init. Any ideas where to put this code such that we can have the EHCI driver, or the OHCI driver, or both loaded and unloaded at will and still working without interfering with each other.

Sounds like code that can live in mach-omap2 and be called when the drivers start or stop.

Quoted 3 lines

3. For OHCI, the transceiver used on the expansion board happens to be an ISP1301 for which there currently exists a driver (drivers/i2c/chips/isp1301_omap.c). Since this is meant to be an OTG transceiver, the driver does otg_set_transceiver.

And does OMAP2 have the OMAP1 OTG controller? If not, then you can't use that driver.

Quoted 2 lines

But MUSB is the OTG controller and has it's own transceiver... and we can't have two of those, right? :) Ideas welcome.

You'll need a new driver; since it's host only, it would only need to be a little stub. The interface shouldn't matter much; just make it general enough that other hardware could use it. (Nothing OMAP-specific, etc.)

- Dave

Gadiyar, Anand

2008-09-12 20:55 UTC #33 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 9 lines

From: David Brownell [[email protected]]
Quoted 3 lines · Anand
1. TLL vs PHY mode needs to be set somewhere. Me thinks this information ought to come from board-specific data and the driver would need to set things up accordingly. What would be a good way to do this?
Define an ehci_omap_platform_data struct and use it for the stuff that shouldn't be in mach-omap2 ... I seem to not understand why there would be a question here.

Lack of experience. I'm got miles to go...

Quoted 9 lines

Quoted 6 lines
2. The OHCI driver (not yet in linux-omap and not in a shape to go ought at the moment either) would need to touch a lot of common registers during init/exit/re-init. Any ideas where to put this code such that we can have the EHCI driver, or the OHCI driver, or both loaded and unloaded at will and still working without interfering with each other.
Sounds like code that can live in mach-omap2 and be called when the drivers start or stop.

Yup. Then a lot of the code in that ehci-omap.c file would need to be moved there to avoid duplicating code. Will do that when I add the OHCI bus-glue.

Quoted 8 lines

Quoted 3 lines
3. For OHCI, the transceiver used on the expansion board happens to be an ISP1301 for which there currently exists a driver (drivers/i2c/chips/isp1301_omap.c). Since this is meant to be an OTG transceiver, the driver does otg_set_transceiver.
And does OMAP2 have the OMAP1 OTG controller? If not, then you can't use that driver.

Quoted 8 lines

Quoted 2 lines
But MUSB is the OTG controller and has it's own transceiver... and we can't have two of those, right? :) Ideas welcome.
You'll need a new driver; since it's host only, it would only need to be a little stub. The interface shouldn't matter much; just make it general enough that other hardware could use it. (Nothing OMAP-specific, etc.)

Okay. Will do. I'll fix the OHCI code too and send it in, but it'll take a while.

Thanks for the advice.

- Anand

David Brownell

2008-09-13 00:42 UTC #34 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 8 lines · Anand

Quoted 3 lines
Sounds like code that can live in mach-omap2 and be called when the drivers start or stop.
Yup. Then a lot of the code in that ehci-omap.c file would need to be moved there to avoid duplicating code. Will do that when I add the OHCI bus-glue.

OK. To clarify: it could also be argued that such stuff should be in some bit of drivers/usb/host code ... but from my previous exposure to OMAP USB stuff, I assume this kind of code will be a lot more SOC-sensitive (revisions etc) than we like to see in the USB tree. The setup bits (how to mux pins and configure the transceiver) clearly belongs in mach-omap2/usb-SOMETHING.c and if anything clearly doesn't belong there, it can move.

Felipe Balbi

2008-09-12 20:38 UTC #35 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

In reply to Gadiyar, Anand

Quoted 5 lines · Anand

Was planning on cleaning up this driver and sending to linux-omap as Tony suggested, but I'm afraid I won't be able to look at it for a while.
So if you're going to be doing this anyway, then could you please meld in Vikram's other patch from [1] as well?

Do you think we should just drop support for the older versions of the expansion board ?

Quoted 2 lines

By the way, how do you propose to test this driver? Do you have the expansion board for the SDP, or some other hardware that can be used?

I have one version of beagle with one ehci port ;-)

Signature · 1 line

balbi

Gadiyar, Anand

2008-09-12 21:06 UTC #36 lore raw

RE: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 8 lines

From: Felipe Balbi [[email protected]]
Quoted 5 lines · Anand
Was planning on cleaning up this driver and sending to linux-omap as Tony suggested, but I'm afraid I won't be able to look at it for a while.
So if you're going to be doing this anyway, then could you please meld in Vikram's other patch from [1] as well?

Quoted 8 lines

Do you think we should just drop support for the older versions of the expansion board ?
Quoted 2 lines
By the way, how do you propose to test this driver? Do you have the expansion board for the SDP, or some other hardware that can be used?
I have one version of beagle with one ehci port ;-)

There were only two major versions of the board - and I don't think there are many of either version floating around. I'm not sure how many people have the earlier one and how many have the later. I don't think you'll need to support the older board.

You could just keep the later one by folding vikram's patch into this. Anyone who wants the older board would simply need to reverse-apply the patch.

The beagle should do. I guess you have a Rev A. ISTR there were some issues with data transfers, but you should be able to get enumeration at least.

- Anand

Steve Sakoman

2008-09-12 22:18 UTC #37 lore raw

Re: [rfc] [patch] clean up to ehci-omap (Was: Re: [patch] omap: usb: ehci: fix use of hcd->regs)

Quoted 8 lines · Anand

Quoted 7 lines
From: Felipe Balbi [[email protected]]
Quoted 2 lines · Anand
By the way, how do you propose to test this driver? Do you have the expansion board for the SDP, or some other hardware that can be used?
I have one version of beagle with one ehci port ;-)

Quoted 7 lines

There were only two major versions of the board - and I don't think there are many of either version floating around. I'm not sure how many people have the earlier one and how many have the later. I don't think you'll need to support the older board.
The beagle should do. I guess you have a Rev A. ISTR there were some issues with data transfers, but you should be able to get enumeration at least.

The Overo also has an EHCI port and should be available "Real Soon Now". Like the TI Beagle folks, we're working hard to get a good, reliable EHCI implementation. It turns out to be a rather difficult task.

Steve

Tony Lindgren

2008-09-12 23:54 UTC #38 lore raw

In reply to Felipe Balbi

Quoted 15 lines · Felipe Balbi

Quoted 8 lines · Felipe Balbi
Quoted 5 lines · David Brownell
Quoted 2 lines · Felipe Balbi
- hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+ hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
This is missing its sibling iounmap() ...
God... thanks Dave. Good catch. Patch coming soonish.
Here's the updated version. While at it, I'll send another patch getting rid of some other mistakes during probe.
===== cut here ====

Quoted 7 lines

From 818dc8ebd261cdc59670fe1580248ad47f18a1de Mon Sep 17 00:00:00 2001
From: Felipe Balbi <[email protected]>
Date: Fri, 12 Sep 2008 21:07:09 +0300
Subject: [PATCH] omap: usb: ehci: fix usb of hcd->regs
hcd->regs should be initialized by ioremaping hcd->rsrc_start and hcd->rsrc_len. Fix it for ehci-omap.c.

Pushing this today.

Tony

Quoted 51 lines

Signed-off-by Felipe Balbi <[email protected]>

---
 drivers/usb/host/ehci-omap.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

drivers/usb/host/ehci-omap.c +8 -2

index 8f122e5..fff4195 100644
@@ -438,7 +438,11 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
 	hcd->rsrc_start = dev->resource[0].start;
 	hcd->rsrc_len = dev->resource[0].end - dev->resource[0].start + 1;
 
-	hcd->regs = (void __iomem *) (int) IO_ADDRESS(hcd->rsrc_start);
+	hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+	if (!hcd->regs) {
+		dev_err(&dev->dev, "ioremap failed\n");
+		return -ENOMEM;
+	}
 
 	ehci = hcd_to_ehci(hcd);
 	ehci->caps = hcd->regs;

@@ -457,8 +461,9 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
 
 	dev_dbg(hcd->self.controller, "ERR: add_hcd");
 	omap_stop_ehc(dev, hcd);
-
+	iounmap(hcd->regs);
 	usb_put_hcd(hcd);
+
 	return retval;
 }

@@ -483,6 +488,7 @@ static int ehci_hcd_omap_drv_remove(struct platform_device *dev)
 
 	dev_dbg(&dev->dev, "ehci_hcd_omap_drv_remove()");
 
+	iounmap(hcd->regs);
 	usb_remove_hcd(hcd);
 	usb_put_hcd(hcd);
 	omap_stop_ehc(dev, hcd);

Signature · 8 lines

1.6.0.1.196.g01914

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

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