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
