[U-Boot] [PATCH v9 4/4] Add USB support for Efika

[Marek Vasut]

On Wednesday, September 28, 2011 09:54:41 PM Jana Rapava wrote:
> This commit adds USB support for EfikaMX and EfikaSB.
> 
> Signed-off-by: Jana Rapava <fermata7 at gmail.com>
> Signed-off-by: Marek Vasut <marek.vasut at gmail.com>
> Cc: Remy Bohmer <linux at bohmer.net>
> Cc: Stefano Babic <sbabic at denx.de>

Dear Jana Rapava,

[...]
> +/*
> + * Enable devices connected to USB BUSes.

BUS isn't written in capital letters, it's normal english word.

> + */
> +void efika_usb_enable_devices(void)
> +{

[...]
> +
> +#define ULPI_ADDR_SHIFT		16
> +#define ulpi_write_mask(value)	((value) & 0xff)
> +#define ulpi_read_mask(value)	(((value) >> 8) & 0xff)
> +
> +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32 ulpi_mask)
> +{
> +	int timeout = ULPI_TIMEOUT;
> +	u32 tmp;
> +
> +	writel(ulpi_value, &ehci->ulpi_viewpoint);

Newline

> +	/* Wait for the ulpi_bit to become zero. */

ulpi_mask ? There's no ulpi_bit ... ?

> +	while (--timeout) {
> +		tmp = readl(&ehci->ulpi_viewpoint);
> +		if (!(tmp & ulpi_mask))
> +			break;
> +		WATCHDOG_RESET();
> +	}
> +
> +	return !timeout;
> +}
> +
> +void ulpi_write(struct usb_ehci *ehci, u32 reg, u32 value)
> +{
> +	if (!(readl(&ehci->ulpi_viewpoint) & ULPI_SS)) {
> +		if (ulpi_wait(ehci, ULPI_WU, ULPI_WU))
> +			printf("ULPI wakeup timed out\n");

Exit here if the wait fails.

Also, this seems also like a common pattern, can we abstract it to ulpi_wakeup() 
?

> +	}
> +
> +	if (ulpi_wait(ehci, ULPI_RWRUN | ULPI_RWCTRL |
> +	reg << ULPI_ADDR_SHIFT | ulpi_write_mask(value), ULPI_RWRUN))
> +		printf("ULPI write timed out\n");
> +}
> +
> +u32 ulpi_read(struct usb_ehci *ehci, u32 reg)
> +{
> +	if (!(readl(&ehci->ulpi_viewpoint) & ULPI_SS)) {
> +		if (ulpi_wait(ehci, ULPI_WU, ULPI_WU))
> +			printf("ULPI wakeup timed out\n");
> +	}

Like here ...

int ret = ulpi_wakeup();
if (ret)
	return;

ulpi wakeup being something like
{
if (readl() & ...)
	return 0; // already awake

return ulpi_wait();
}

> +
> +	if (ulpi_wait(ehci, ULPI_RWRUN | reg << ULPI_ADDR_SHIFT, ULPI_RWRUN)) {
> +		printf("ULPI read timed out\n");
> +		return 0;
> +	}
> +	return ulpi_read_mask(readl(&ehci->ulpi_viewpoint));
> +}
> +
> +void ulpi_init(struct usb_ehci *ehci, struct mxc_ulpi_regs *ulpi)
> +{
> +	u32 tmp = 0;
> +	int reg, i;
> +
> +	/* get ID from ULPI immediate registers */

English sentence ... capital letter, dot ... please fix globally!

Nearly there.

BR, MV