Hello > -----Original Message----- > From: Uwe Kleine-König [mailto:u.kleine-koenig@xxxxxxxxxxxxxx] > Sent: Thursday, October 28, 2010 2:27 AM > To: Sascha Hauer; Daniel Mack; Valentin Longchamp; Nguyen Dinh-R00091; > Wolfram Sang; Eric Bénard > Cc: Matthieu CASTET; linux-usb@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; > Alan Stern > Subject: Re: ehci-mxc broken [Was: Re: [PATCH] ehci tdi : let's > tdi_resetset host mode] > > Hello, > > just extending the audience in case someone with more knowledge about > the usb driver wants to help. > > Best regards > Uwe > > On Wed, Oct 27, 2010 at 04:26:23PM -0400, Alan Stern wrote: > > On Wed, 27 Oct 2010, Uwe Kleine-König wrote: > > > > > On Wed, Oct 27, 2010 at 02:50:57PM -0400, Alan Stern wrote: > > > > On Wed, 27 Oct 2010, Uwe Kleine-König wrote: > > > > > > > > > This patch (i.e. 65fd42724aee31018b0bb53f4cb04971423be664) broke > the build on > > > > > mxc using the mx51_defconfig: > > > > > > > > > > CC drivers/usb/host/ehci-hcd.o > > > > > In file included from drivers/usb/host/ehci-hcd.c:1166: > > > > > drivers/usb/host/ehci-mxc.c: In function > 'ehci_mxc_drv_probe': > > > > > drivers/usb/host/ehci-mxc.c:192: error: 'ehci' undeclared > (first use in this function) > > > > > drivers/usb/host/ehci-mxc.c:192: error: (Each undeclared > identifier is reported only once > > > > > drivers/usb/host/ehci-mxc.c:192: error: for each function it > appears in.) > > > > > drivers/usb/host/ehci-mxc.c:117: warning: unused variable > 'temp' > > > > > make[3]: *** [drivers/usb/host/ehci-hcd.o] Error 1 > > > > > make[2]: *** [drivers/usb/host/ehci-hcd.o] Error 2 > > > > > make[1]: *** [sub-make] Error 2 > > > > > make: *** [all] Error 2 > > > > > > > > > > It's not obvious for me how to fix it up. Does anyone care to > help me? > > > > > > > > It looks that entire "set up the PORTSCx register" section near > line > > > > 192 belongs inside ehci_mxc_setup() (near the end) instead of > > > > ehci_mxc_drv_probe(). > > > > > > > > The unused "temp" variable can simply be removed. > > > This is only compile tested: > > > > > > diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci- > mxc.c > > > index ac9c4d7..70edb11 100644 > > > --- a/drivers/usb/host/ehci-mxc.c > > > +++ b/drivers/usb/host/ehci-mxc.c > > > @@ -36,6 +36,8 @@ struct ehci_mxc_priv { > > > static int ehci_mxc_setup(struct usb_hcd *hcd) > > > { > > > struct ehci_hcd *ehci = hcd_to_ehci(hcd); > > > + struct device *dev = hcd->self.controller; > > > + struct mxc_usbh_platform_data *pdata = dev_get_platdata(dev); > > > int retval; > > > > > > /* EHCI registers start at offset 0x100 */ > > > @@ -64,6 +66,11 @@ static int ehci_mxc_setup(struct usb_hcd *hcd) > > > ehci_reset(ehci); > > > > > > ehci_port_power(ehci, 0); > > > + > > > + /* set up the PORTSCx register */ > > > + ehci_writel(ehci, pdata->portsc, &ehci->regs->port_status[0]); > > > + mdelay(10); > > > + > > > return 0; > > > } > > > > > > @@ -114,7 +121,7 @@ static int ehci_mxc_drv_probe(struct > platform_device *pdev) > > > struct mxc_usbh_platform_data *pdata = pdev->dev.platform_data; > > > struct usb_hcd *hcd; > > > struct resource *res; > > > - int irq, ret, temp; > > > + int irq, ret; > > > struct ehci_mxc_priv *priv; > > > struct device *dev = &pdev->dev; > > > > > > @@ -188,10 +195,6 @@ static int ehci_mxc_drv_probe(struct > platform_device *pdev) > > > clk_enable(priv->ahbclk); > > > } > > > > > > - /* set up the PORTSCx register */ > > > - ehci_writel(ehci, pdata->portsc, &ehci->regs->port_status[0]); > > > - mdelay(10); > > > - > > > /* setup specific usb hw */ > > > ret = mxc_initialize_usb_hw(pdev->id, pdata->flags); > > > if (ret < 0) > > > > > > If I don't hear anything about this being stupid I will try to get > that > > > tested tomorrow on real hardware. I tried a similar fix on my local machine. I can see that the USB host driver was loaded correctly, but it seemed that the Host1 port is not operating in host mode. On the MX51 Babbage HW, it was not able to detect the 7-port hub on-board. The OTG port seems to be operating correctly with "otg_mode=host". > > > > It's not clear whether the mdelay() call should be moved, and it sure > > looks like it should be changed to msleep(). > > > > Also, I don't know where in that function the PORTSCx write really > > belongs. Somebody who is familiar with the hardware will have to > > answer these questions. > > > > Alan Stern > > > > > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | http://www.pengutronix.de/ > | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
