Re: ehci-mxc broken [Was: Re: [PATCH] ehci tdi : let's tdi_reset set host mode]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux