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.
[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!
--
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
