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
