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

Marek Vasut
2011-09-28T20:44:21Z

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@gmail.com>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Remy Bohmer <linux@bohmer.net>
> Cc: Stefano Babic <sbabic@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