[patch] omap: usb: ehci: fix use of hcd->regs

[patch] omap: usb: ehci: fix use of hcd->regs

[Felipe Balbi]

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 <felipe.balbi@nokia.com>
---

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 8f122e5..a67533d 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -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;


-- 
balbi

Re: [patch] omap: usb: ehci: fix use of hcd->regs

[Hiroshi DOYU]

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

> 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 <felipe.balbi@nokia.com>
> ---
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 8f122e5..a67533d 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -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?

>  
>         ehci = hcd_to_ehci(hcd);
>         ehci->caps = hcd->regs;
> 
> 
> -- 
> balbi
> --
> 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

Re: [patch] omap: usb: ehci: fix use of hcd->regs

[Felipe Balbi]

On Fri, Sep 12, 2008 at 03:14:04PM +0300, Hiroshi DOYU wrote:
> From: "ext Felipe Balbi" <felipe.balbi@nokia.com>
> Subject: [patch] omap: usb: ehci: fix use of hcd->regs
> Date: Fri, 12 Sep 2008 15:10:24 +0300
> 
> > 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 <felipe.balbi@nokia.com>
> > ---
> > 
> > diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> > index 8f122e5..a67533d 100644
> > --- a/drivers/usb/host/ehci-omap.c
> > +++ b/drivers/usb/host/ehci-omap.c
> > @@ -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?

Good one. Here's a new version:

===== cut here ====

From: Felipe Balbi <felipe.balbi@nokia.com>
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 <felipe.balbi@nokia.com>
---

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 8f122e5..4cafa79 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -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;


-- 
balbi

Re: [patch] omap: usb: ehci: fix use of hcd->regs

[Hiroshi DOYU]

From: Felipe Balbi <felipe.balbi@nokia.com>
Subject: Re: [patch] omap: usb: ehci: fix use of hcd->regs
Date: Fri, 12 Sep 2008 15:30:10 +0300

> On Fri, Sep 12, 2008 at 03:14:04PM +0300, Hiroshi DOYU wrote:
> > From: "ext Felipe Balbi" <felipe.balbi@nokia.com>
> > Subject: [patch] omap: usb: ehci: fix use of hcd->regs
> > Date: Fri, 12 Sep 2008 15:10:24 +0300
> > 
> > > 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 <felipe.balbi@nokia.com>
> > > ---
> > > 
> > > diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> > > index 8f122e5..a67533d 100644
> > > --- a/drivers/usb/host/ehci-omap.c
> > > +++ b/drivers/usb/host/ehci-omap.c
> > > @@ -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?
> 
> Good one. Here's a new version:
> 
> ===== cut here ====
> 
> From: Felipe Balbi <felipe.balbi@nokia.com>
> 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 <Hiroshi.DOYU@nokia.com>

> Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>

RE: [patch] omap: usb: ehci: fix use of hcd->regs

[Pandita, Vikram]

Felipe
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Hiroshi
> > > >
> > > > diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> > > > index 8f122e5..a67533d 100644
> > > > --- a/drivers/usb/host/ehci-omap.c
> > > > +++ b/drivers/usb/host/ehci-omap.c
> > > > @@ -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);

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.

> > >
> > > Return value check?
> >
> > Good one. Here's a new version:
> >
> > ===== cut here ====
> >
> > From: Felipe Balbi <felipe.balbi@nokia.com>
> > 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 <Hiroshi.DOYU@nokia.com>
> 
> > Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> --
> 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

Re: [patch] omap: usb: ehci: fix use of hcd->regs

[David Brownell]

On Friday 12 September 2008, Felipe Balbi wrote:
> -       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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] omap: usb: ehci: fix use of hcd->regs

[Felipe Balbi]

On Fri, Sep 12, 2008 at 09:32:56AM -0700, David Brownell wrote:
> On Friday 12 September 2008, Felipe Balbi wrote:
> > -       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.

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

Re: [patch] omap: usb: ehci: fix use of hcd->regs

[Felipe Balbi]

On Fri, Sep 12, 2008 at 08:50:33PM +0300, Felipe Balbi wrote:
> On Fri, Sep 12, 2008 at 09:32:56AM -0700, David Brownell wrote:
> > On Friday 12 September 2008, Felipe Balbi wrote:
> > > -       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 <felipe.balbi@nokia.com>
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 <felipe.balbi@nokia.com>
---
 drivers/usb/host/ehci-omap.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 8f122e5..fff4195 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -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);
-- 
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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Balbi]

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

>From 194d26cae0e9093c4b1da4de531f6e5c16495678 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@nokia.com>
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 <felipe.balbi@nokia.com>
---
 drivers/usb/host/ehci-omap.c |  152 ++++++++++++++++++++++--------------------
 1 files changed, 81 insertions(+), 71 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index fff4195..4f823c5 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -5,7 +5,7 @@
  * Tested on OMAP3430 ES2.0 SDP
  *
  * Copyright (C) 2007-2008 Texas Instruments, Inc.
- * 	Author: Vikram Pandita <vikram.pandita@ti.com>
+ *	Author: Vikram Pandita <vikram.pandita@ti.com>
  *
  * 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",
 	}
 };
-- 
1.6.0.1.196.g01914

-- 
balbi

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

[David Brownell]

On Friday 12 September 2008, Felipe Balbi wrote:
>         /* 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Tony Lindgren]

* David Brownell <david-b@pacbell.net> [080912 12:18]:
> On Friday 12 September 2008, Felipe Balbi wrote:
> >         /* 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Balbi]

On Fri, Sep 12, 2008 at 12:21:59PM -0700, Tony Lindgren wrote:
> * David Brownell <david-b@pacbell.net> [080912 12:18]:
> > On Friday 12 September 2008, Felipe Balbi wrote:
> > >         /* 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

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

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

[Felipe Balbi]

On Fri, Sep 12, 2008 at 10:28:16PM +0300, Felipe Balbi wrote:
> On Fri, Sep 12, 2008 at 12:21:59PM -0700, Tony Lindgren wrote:
> > * David Brownell <david-b@pacbell.net> [080912 12:18]:
> > > On Friday 12 September 2008, Felipe Balbi wrote:
> > > >         /* 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 ;-)

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

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

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

-- 
balbi

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

[Pandita, Vikram]

> -----Original Message-----
> From: Felipe Balbi [mailto:me@felipebalbi.com]
> Sent: Saturday, September 13, 2008 1:06 AM
> To: Felipe Balbi
> Cc: Tony Lindgren; David Brownell; felipe.balbi@nokia.com; 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-
> >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).

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

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

> 
> Any comments ?
> 
> --
> balbi

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

Hi,


On Mon, Sep 15, 2008 at 12:11:46AM +0530, Pandita, Vikram wrote:
> The issues mentioned are _not_ of omap34xx silicon, but of the PHY (ISP1504 in SDP-USB expansion board).

Gotcha

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

-- 
balbi

[-- Attachment #2: 0001-omap-usb-ehci-clean-up-ehci-omap.c.diff --]
[-- Type: text/x-diff, Size: 11278 bytes --]

From bfae28bb4dfa0198bd30584fcfa74682947053eb Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@nokia.com>
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 <felipe.balbi@nokia.com>
---
 drivers/usb/host/ehci-omap.c |  135 +++++++++++++++++-------------------------
 1 files changed, 55 insertions(+), 80 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index fff4195..f1b5ea5 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -5,7 +5,7 @@
  * Tested on OMAP3430 ES2.0 SDP
  *
  * Copyright (C) 2007-2008 Texas Instruments, Inc.
- * 	Author: Vikram Pandita <vikram.pandita@ti.com>
+ *	Author: Vikram Pandita <vikram.pandita@ti.com>
  *
  * 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",
 	}
 };
-- 
1.6.0.1.196.g01914


[-- Attachment #3: 0002-omap-usb-ehci-switch-to-use-gpiolib.diff --]
[-- Type: text/x-diff, Size: 2475 bytes --]

From 14a61159787a5797340445594f57f3921d3ab7e8 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@nokia.com>
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 <felipe.balbi@nokia.com>
---
 drivers/usb/host/ehci-omap.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index f1b5ea5..c73f832 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -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,
-- 
1.6.0.1.196.g01914


[-- Attachment #4: 0003-omap-usb-ehci-fix-usage-of-resources.diff --]
[-- Type: text/x-diff, Size: 3270 bytes --]

From 7e25b8e03aec867002163b8618c126f16682ff52 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@nokia.com>
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 <felipe.balbi@nokia.com>
---
 drivers/usb/host/ehci-omap.c |   62 +++++++++++++++++++++++++++++-------------
 1 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index c73f832..4ac18b2 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -407,36 +407,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 *resource;
 
 	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;
+	resource = platform_get_resource(dev, IORESOURCE_MEM, 0);
+	if (!resource || 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 = resource->start;
+	hcd->rsrc_len = resource->end - resource->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);
@@ -449,16 +455,34 @@ static int ehci_hcd_omap_drv_probe(struct platform_device *dev)
 	/* SET 1 micro-frame Interrupt interval */
 	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;
 }
 
-- 
1.6.0.1.196.g01914


[-- Attachment #5: 0004-omap-usb-ehci-support-final-revision-of-usb-board.diff --]
[-- Type: text/x-diff, Size: 3525 bytes --]

From 775b30a9724fa3274702621b8e595ad35374bf3b Mon Sep 17 00:00:00 2001
From: Vikram Pandita <vikram.pandita@ti.com>
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 <vikram.pandita@ti.com>
Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 drivers/usb/host/ehci-omap.c |   37 ++++++-------------------------------
 1 files changed, 6 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 4ac18b2..f8316d5 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -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;
-- 
1.6.0.1.196.g01914

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

[David Brownell]

On Sunday 14 September 2008, Felipe Balbi wrote:

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

"platform" ...

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

[Felipe Balbi]

On Mon, Sep 15, 2008 at 10:47:14PM -0700, David Brownell wrote:
> On Sunday 14 September 2008, Felipe Balbi wrote:
> 
> > +MODULE_ALIAS("plaform:omap-ehci");
> 
> "platform" ...

thanks. Missed it. It's fixed now.

-- 
balbi

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

[David Brownell]

> 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 <felipe.balbi@nokia.com>

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

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

[Gadiyar, Anand]

> [patch] omap: usb: ehci: fix use of hcd->regs)
> 
> > 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 <felipe.balbi@nokia.com>
> 
> ... 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

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

[David Brownell]

On Monday 15 September 2008, Gadiyar, Anand wrote:
> > 
> > ... 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.

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

[Felipe Balbi]

On Mon, Sep 15, 2008 at 10:48:19PM -0700, David Brownell wrote:
> > 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 <felipe.balbi@nokia.com>
> 
> ... 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.

-- 
balbi

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

[David Brownell]

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

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

[Felipe Balbi]

On Mon, Sep 15, 2008 at 10:49:33PM -0700, David Brownell wrote:
> > +       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
;-)

-- 
balbi

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

[Gadiyar, Anand]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

Hi Felipe, Paul,

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org On Behalf Of Felipe Balbi
> Hi,
> 
> 
> On Mon, Sep 15, 2008 at 12:11:46AM +0530, Pandita, Vikram wrote:
> > The issues mentioned are _not_ of omap34xx silicon, but of 
> > the PHY (ISP1504 in SDP-USB expansion board).
> 
> Gotcha
> 
> > 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.

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

[-- Attachment #2: 0005-omap-usb-ehci-dpll5-infinite-loop.patch --]
[-- Type: application/octet-stream, Size: 860 bytes --]

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(+)

Index: linux-omap-2.6/drivers/usb/host/ehci-omap.c
===================================================================
--- linux-omap-2.6.orig/drivers/usb/host/ehci-omap.c	2008-09-18 14:59:34.000000000 +0530
+++ linux-omap-2.6/drivers/usb/host/ehci-omap.c	2008-09-18 15:00:09.737475758 +0530
@@ -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 */
 
 

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

[Paul Walmsley]

Hello Anand,

On Thu, 18 Sep 2008, Gadiyar, Anand wrote:

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

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

Yes, will send patches for this soon.


- Paul

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

[Felipe Balbi]

On Mon, Sep 15, 2008 at 12:11:46AM +0530, Vikram Pandita wrote:
> 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.
-- 
balbi

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

[Gadiyar, Anand]

> From: linux-omap-owner@vger.kernel.org On Behalf Of Felipe Balbi
> 
> On Mon, Sep 15, 2008 at 12:11:46AM +0530, Vikram Pandita wrote:
> > 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>

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

[Felipe Balbi]

On Mon, Sep 22, 2008 at 08:51:29PM +0530, ext Gadiyar, Anand wrote:
> > From: linux-omap-owner@vger.kernel.org On Behalf Of Felipe Balbi
> > 
> > On Mon, Sep 15, 2008 at 12:11:46AM +0530, Vikram Pandita wrote:
> > > 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.

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

-- 
balbi

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

[Gadiyar, Anand]

> On Mon, Sep 22, 2008 at 08:51:29PM +0530, ext Gadiyar, Anand wrote:
> > > From: linux-omap-owner@vger.kernel.org On Behalf Of Felipe Balbi
> > > 
> > > On Mon, Sep 15, 2008 at 12:11:46AM +0530, Vikram Pandita wrote:
> > > > 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.
> 
> 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

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

[Felipe Balbi]

On Mon, Sep 22, 2008 at 09:00:26PM +0530, ext Gadiyar, Anand wrote:
> 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.

-- 
balbi

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

[Gadiyar, Anand]

> On Behalf Of Felipe Balbi [me@felipebalbi.com]
> On Fri, Sep 12, 2008 at 12:21:59PM -0700, Tony Lindgren wrote:
> > * David Brownell <david-b@pacbell.net> [080912 12:18]:
> > > On Friday 12 September 2008, Felipe Balbi wrote:
> > > >         /* 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

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

[David Brownell]

On Friday 12 September 2008, Gadiyar, Anand wrote:
> 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.


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


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


> 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

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

[Gadiyar, Anand]

> From: David Brownell [david-b@pacbell.net]
> On Friday 12 September 2008, Gadiyar, Anand wrote:
> > 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...

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


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

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

[David Brownell]

On Friday 12 September 2008, Gadiyar, Anand wrote:
> 
> > 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.
 

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

[Felipe Balbi]

On Sat, Sep 13, 2008 at 01:31:12AM +0530, Gadiyar, Anand wrote:
> 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 ?

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

-- 
balbi

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

[Gadiyar, Anand]

> From: Felipe Balbi [me@felipebalbi.com]
> On Sat, Sep 13, 2008 at 01:31:12AM +0530, Gadiyar, Anand wrote:
> > 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 ?
> 
> > 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

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

[Steve Sakoman]

On Fri, Sep 12, 2008 at 2:06 PM, Gadiyar, Anand <gadiyar@ti.com> wrote:
>> From: Felipe Balbi [me@felipebalbi.com]
>> On Sat, Sep 13, 2008 at 01:31:12AM +0530, Gadiyar, Anand wrote:
>> > 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.
>
> 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

Re: [patch] omap: usb: ehci: fix use of hcd->regs

[Tony Lindgren]

* Felipe Balbi <me@felipebalbi.com> [080912 11:23]:
> On Fri, Sep 12, 2008 at 08:50:33PM +0300, Felipe Balbi wrote:
> > On Fri, Sep 12, 2008 at 09:32:56AM -0700, David Brownell wrote:
> > > On Friday 12 September 2008, Felipe Balbi wrote:
> > > > -       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 <felipe.balbi@nokia.com>
> 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

> 
> Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> ---
>  drivers/usb/host/ehci-omap.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 8f122e5..fff4195 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -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);
> -- 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html