Hi Marek, Thanks for you review.
On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut <marek.va...@gmail.com> wrote: >> From: "Govindraj.R" <govindraj.r...@ti.com> >> >> Clean up added ehci-omap.c and make it generic for re-use across >> soc having same ehci ip block. Also pass the modes to be configured >> and configure the ports accordingly. All usb layers are not cache >> aligned till then keep cache off for usb ops as ehci will use >> internally dma for all usb ops. >> >> * Add a generic common header ehci-omap.h having common ip block >> data and reg shifts. >> * Rename and modify ehci-omap3 to ehci.h retain only conflicting >> sysc reg shifts remove others and move to common header file. > > Don't reimplement the ulpi stuff ... there's already some ulpi stuff in uboot > that needs fixing, so fix it and use it. > I am not implementing any ulpi stuff I am just configuring OMAP on soc usb host controller (ehci). All the configuration stuff is OMAP specific things which are done in ehci-omap.c file stuffs done are like soft-reset, port mode to be used and putting port in no -idle mode(omap specific pm implementation) etc. >> >> Signed-off-by: Govindraj.R <govindraj.r...@ti.com> >> --- >> arch/arm/include/asm/arch-omap3/ehci.h | 55 ++++++ >> arch/arm/include/asm/arch-omap3/ehci_omap3.h | 58 ------- >> arch/arm/include/asm/arch-omap4/ehci.h | 49 ++++++ >> arch/arm/include/asm/ehci-omap.h | 147 +++++++++++++++++ >> drivers/usb/host/ehci-omap.c | 228 [...] >> + >> +#ifdef CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS >> +#define OMAP_HS_USB_PORTS CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS >> +#else >> +#define OMAP_HS_USB_PORTS 3 >> +#endif >> + >> +#define is_ehci_phy_mode(x) (x == OMAP_EHCI_PORT_MODE_PHY) >> +#define is_ehci_tll_mode(x) (x == OMAP_EHCI_PORT_MODE_TLL) >> +#define is_ehci_hsic_mode(x) (x == OMAP_EHCI_PORT_MODE_HSIC) > > This is unsafe ... use ((x) == ...) Okay, will correct this. > >> + >> +/* Values of UHH_REVISION - Note: these are not given in the TRM */ >> +#define OMAP_USBHS_REV1 0x00000010 /* > OMAP3 */ >> +#define OMAP_USBHS_REV2 0x50700100 /* > OMAP4 */ >> + [...] >> +/* ULPI */ >> +#define ULPI_SET(a) (a + 1) >> +#define ULPI_CLR(a) (a + 2) > > ULPI ... use generic stuff Actually this for omap specific configuration done with omap reg map. EHCI register INSNREG05_ULPI needs to be configured if we are in ulpi-phy mode same is done here. > >> +#define ULPI_FUNC_CTRL 0x04 >> +#define ULPI_FUNC_CTRL_RESET (1 << 5) >> + >> +struct omap_usbhs_board_data { >> + enum usbhs_omap_port_mode port_mode[OMAP_HS_USB_PORTS]; >> +}; >> + >> +struct omap_usbtll { >> + u32 rev; /* 0x00 */ >> + u32 hwinfo; /* 0x04 */ >> + u8 pad1[0x8]; > > reserved1[] instead of pad1[], fix globally yes fine, will correct this. [..] >> >> +struct omap_uhh *const uhh = (struct omap_uhh *)OMAP_UHH_BASE; >> +struct omap_usbtll *const usbtll = (struct omap_usbtll *)OMAP_USBTLL_BASE; >> +struct omap_ehci *const ehci = (struct omap_ehci *)OMAP_EHCI_BASE; > > static > yes correct, will change this. >> + >> +static int omap_uhh_reset(void) >> +{ [...] >> + /* perform TLL soft reset, and wait until reset is complete */ >> + writel(OMAP_USBTLL_SYSCONFIG_SOFTRESET, &usbtll->sysc); >> + >> + /* Wait for TLL reset to complete */ >> + while (!(readl(&usbtll->syss) & OMAP_USBTLL_SYSSTATUS_RESETDONE)) > > Add timeout, fix globally Sorry I didn't get you here. The function uses a timeout value init and then same init value to used to poll for CONFIG_SYS_HZ time for reset to be done else prints timeout failure. >> + if (get_timer(init) > CONFIG_SYS_HZ) { >> + debug("OMAP EHCI error: timeout resetting TLL\n"); >> + return -EL3RST; >> + } >> + [...] >> + /* setup ULPI bypass and burst configurations */ >> + reg |= (OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN | >> + OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN | >> + OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN); >> + reg &= ~OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN; > > clrsetbits_le32 ? yes can be used. -- Thanks, Govindraj.R _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot