[U-Boot] [PATCH 1/3] usb: ehci-omap: Reset the USB Host OMAP module

[Nikita Kiryanov <nikita@compulab.co.il>]

Hi Roger,

This patch fixes a USB issue we had which was caused by the function
ehci_shutdown() in usb_lowlevel_stop(). The issue was that once "usb
stop" was called, usb devices would no longer be detected by the
"usb start" command. With this patch the devices are detected each
time.

One suggestion though (see below):

On 12/02/2013 03:47 PM, Roger Quadros wrote:
> In commit bb1f327 we removed the UHH reset to fix NFS root (over usb
> ethernet) problems with Beagleboard (3530 ES1.0). However, this
> seems to cause USB detection problems for Pandaboard, about (3/8).
>
> On further investigation, it seems that doing the UHH reset is not
> the cause of the original Beagleboard problem, but in the way the reset
> was done.
>
> This patch adds proper UHH RESET mechanism for OMAP3 and OMAP4/5 based
> on the UHH_REVISION register. This should fix the Beagleboard NFS
> problem as well as the Pandaboard USB detection problem.
>
> Reported-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> CC: Stefan Roese <sr@denx.de>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Signed-off-by: Roger Quadros <rogerq@ti.com>

[...]

> +
> +	switch (rev) {
> +	case OMAP_USBHS_REV1:
> +		/* Wait for soft RESET to complete */
> +		while (!(readl(&uhh->syss) & 0x1)) {
> +			if (timeout > 100) {
> +				printf("%s: RESET timeout\n", __func__);
> +				return -1;
> +			}
> +			udelay(10);
> +			timeout++;
> +		}
> +
> +		/* Set No-Idle, No-Standby */
> +		writel(OMAP_UHH_SYSCONFIG_VAL, &uhh->sysc);
> +		break;
> +
> +	default:	/* Rev. 2 onwards */
> +
> +		udelay(2); /* Need to wait before accessing SYSCONFIG back */
> +
> +		/* Wait for soft RESET to complete */
> +		while ((readl(&uhh->sysc) & 0x1)) {
> +			if (timeout > 100) {
> +				printf("%s: RESET timeout\n", __func__);
> +				return -1;
> +			}
> +			udelay(10);
> +			timeout++;
> +		}
> +
> +		writel(OMAP_UHH_SYSCONFIG_VAL, &uhh->sysc);
> +		break;
> +	}
> +

You have a lot of unnecessart code duplication in this switch statment.
How about:
====
if (rev != OMAP_USBHS_REV1)
	udelay(2); /* Need to wait before accessing SYSCONFIG back */

/* Wait for soft RESET to complete */
while (!omap_uhh_reset_done(rev)) {
	if (timeout > 100) {
		printf("%s: RESET timeout\n", __func__);
		return -1;
	}

	udelay(10);
	timeout++;
}

writel(OMAP_UHH_SYSCONFIG_VAL, &uhh->sysc);
====
With omap_uhh_reset_done() doing the appropriate check for the revision.

Also, why can't we do udelay(2) when rev == OMAP_USBHS_REV1?

-- 
Regards,
Nikita.