[U-Boot] [PATCH 3/5] imx: Add support for USB EHCI on imx25

[Marek Vasut]

On Thursday, June 30, 2011 05:48:54 PM Stefano Babic wrote:
> On 06/30/2011 11:57 AM, Matthias Weisser wrote:
> > Adding support for USB host on imx25 using the internal PHY
> > 
> > Signed-off-by: Matthias Weisser <weisserm at arcor.de>
> > ---
> 
> Hi Matthias,
> 
> > +#ifdef CONFIG_MX25
> > +#define MX25_USB_CTRL_IP_PUE_DOWN_BIT	(1<<6)
> > +#define MX25_USB_CTRL_HSTD_BIT		(1<<5)
> > +#define MX25_USB_CTRL_USBTE_BIT		(1<<4)
> > +#define MX25_USB_CTRL_OCPOL_OTG_BIT	(1<<3)
> > +#endif
> > +
> > +#ifdef CONFIG_MX31
> > 
> >  #define MX31_OTG_SIC_SHIFT	29
> >  #define MX31_OTG_SIC_MASK	(0x3 << MX31_OTG_SIC_SHIFT)
> >  #define MX31_OTG_PM_BIT		(1 << 24)
> > 
> > @@ -42,10 +50,19 @@
> > 
> >  #define MX31_H1_SIC_MASK	(0x3 << MX31_H1_SIC_SHIFT)
> >  #define MX31_H1_PM_BIT		(1 << 8)
> >  #define MX31_H1_DT_BIT		(1 << 4)
> > 
> > +#endif
> 
> Can we try to get rid of nasty #ifdef ? I am sure, we would have in
> future support for at least MX5, and if we add a special handling for
> each SOC this file will become unreadable.

... we would have ... if the developer won't hang himself first ;-)

Otherwise, we should probably unify this IMX_USB_BASE stuff indeed. Maybe even 
compute USBCTRL_OTGBASE_OFFSET on the fly (as imx51 has different offset there).
> 
> It seems to me that in most cases we can do it using the same names.
> 
> >  	/* Take USB2 */
> > 
> > +#ifdef CONFIG_MX25
> > +	ehci = (struct usb_ehci *)(IMX_USB_BASE +
> > +#else
> > 
> >  	ehci = (struct usb_ehci *)(MX31_OTG_BASE_ADDR +
> > 
> > +#endif
> 
> For example, here we can have a CONFIG_MXB_USB_BASE (or something like
> this), without adding #ifdef. I do not know if we can get completely get
> rid of them, but it is worth to try.
> 
> Best regards,
> Stefano Babic