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