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