| Message ID | 1309427865-17531-4-git-send-email-weisserm@arcor.de |
|---|---|
| State | Changes Requested |
| Headers | show |
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@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. 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
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@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
diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c index 6af35ab..eca430f 100644 --- a/drivers/usb/host/ehci-mxc.c +++ b/drivers/usb/host/ehci-mxc.c @@ -29,6 +29,14 @@ #define USBCTRL_OTGBASE_OFFSET 0x600 +#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 static int mxc_set_usbcontrol(int port, unsigned int flags) { unsigned int v; + +#ifdef CONFIG_MX25 + v = MX25_USB_CTRL_IP_PUE_DOWN_BIT | MX25_USB_CTRL_HSTD_BIT | + MX25_USB_CTRL_USBTE_BIT | MX25_USB_CTRL_OCPOL_OTG_BIT; + + writel(v, IMX_USB_BASE + USBCTRL_OTGBASE_OFFSET); +#endif + #ifdef CONFIG_MX31 v = readl(MX31_OTG_BASE_ADDR + USBCTRL_OTGBASE_OFFSET); @@ -94,27 +111,34 @@ static int mxc_set_usbcontrol(int port, unsigned int flags) int ehci_hcd_init(void) { - u32 tmp; struct usb_ehci *ehci; +#ifdef CONFIG_MX31 + u32 tmp; struct clock_control_regs *sc_regs = (struct clock_control_regs *)CCM_BASE; tmp = __raw_readl(&sc_regs->ccmr); __raw_writel(__raw_readl(&sc_regs->ccmr) | (1 << 9), &sc_regs->ccmr) ; +#endif udelay(80); /* Take USB2 */ +#ifdef CONFIG_MX25 + ehci = (struct usb_ehci *)(IMX_USB_BASE + +#else ehci = (struct usb_ehci *)(MX31_OTG_BASE_ADDR + +#endif (0x200 * CONFIG_MXC_USB_PORT)); hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength); hcor = (struct ehci_hcor *)((uint32_t) hccr + HC_LENGTH(ehci_readl(&hccr->cr_capbase))); setbits_le32(&ehci->usbmode, CM_HOST); +#ifdef CONFIG_MX31 setbits_le32(&ehci->control, USB_EN); __raw_writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc); - +#endif mxc_set_usbcontrol(CONFIG_MXC_USB_PORT, CONFIG_MXC_USB_FLAGS); udelay(10000);
Adding support for USB host on imx25 using the internal PHY Signed-off-by: Matthias Weisser <weisserm@arcor.de> --- drivers/usb/host/ehci-mxc.c | 28 ++++++++++++++++++++++++++-- 1 files changed, 26 insertions(+), 2 deletions(-)