Hi Tom, On Wed, Dec 14, 2011 at 9:30 PM, Tom Rini <tom.r...@gmail.com> wrote: > On Wed, Dec 14, 2011 at 5:09 AM, Govindraj.R <govindraj.r...@ti.com> wrote: >> From: "Govindraj.R" <govindraj.r...@ti.com> > [snip] >> +/* TLL Register Set */ >> +#define OMAP_USBTLL_SYSCONFIG_SIDLEMODE (1 << 3) > > Globally, please fix all #define<tab> to #define<space>. > >> +#define OMAP_USBTLL_SYSCONFIG_AUTOIDLE (1 << 0) >> +#define OMAP_USBTLL_SYSSTATUS_RESETDONE (1 << 0) > > Just use '1' not '(1 << 0)', globally. > > [snip] >> +++ b/drivers/usb/host/ehci-omap.c > [snip] >> + reg = ULPI_FUNC_CTRL_RESET >> + /* FUNCTION_CTRL_SET register */ >> + | (ULPI_SET(ULPI_FUNC_CTRL) << >> EHCI_INSNREG05_ULPI_REGADD_SHIFT) >> + /* Write */ >> + | (2 << EHCI_INSNREG05_ULPI_OPSEL_SHIFT) >> + /* PORTn */ >> + | ((port + 1) << EHCI_INSNREG05_ULPI_PORTSEL_SHIFT) >> + /* start ULPI access*/ >> + | (1 << EHCI_INSNREG05_ULPI_CONTROL_SHIFT); > > Can you please re-write this as: > /* > * What these values are doing... > */ > reg = A | (B << B_SHIFT) | ... > > [snip] >> + /* Wait for ULPI access completion */ >> + while ((readl(&ehci->insreg05_utmi_ulpi) >> + & (1 << EHCI_INSNREG05_ULPI_CONTROL_SHIFT))) >> + if (get_timer(init) > CONFIG_SYS_HZ) { > > Perhaps gmail is getting the indentation wrong here but it seems not > right and the norm is to not start the newline with '&' or '|' or > similar. >
Thanks for the comments will fix and re-post a new version. > [snip] >> + /* >> + * An undocumented "feature" in the OMAP3 EHCI controller, >> + * causes suspended ports to be taken out of suspend when >> + * the USBCMD.Run/Stop bit is cleared (for example when >> + * we do ehci_bus_suspend). >> + * This breaks suspend-resume if the root-hub is allowed >> + * to suspend. Writing 1 to this undocumented register bit >> + * disables this feature and restores normal behavior. >> + */ > > Thank you for clearly commenting on an undocumented "feature"! > > Even 'tho there's changes requested already I've moved this to Remy in > patchwork because I want his comments as the USB maintainer as well. > Thanks! Yes fine, I will CC him the next version, in case if there are no comments until I post v2. -- Thanks, Govindraj.R _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot