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

[Marek Vasut <marek.vasut@gmail.com>]

On Tuesday, September 27, 2011 09:24:17 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>
> ---
> Changes for v2:
>     - changed to proper patch
> Changes for v3:
>     - merged other USB patches from u-boot-pxa/efikasb
>     - offset-based access changed to struct-based access
>     - use {clrset,clr,set}bits_le32() calls
>     - CodingStyle and naming cleanup
> Changes for v4:
>      - split into patchset
>      - CodingStyle and naming cleanup
>      - remove endless loops
>      - silence compiler warnings
> Changes for v5:
>        - change order of arguments in ulpi* functions
>        - change type of reg argument
>        - rename offset macro
> Changes for v6:
> 	- rebase on top of u-boot-imx/next
> 	- cleanup of CodingStyle and comments
> 	- use macro machine_is_efikasb()
> 	- introduce header file efika.h
> 
>  board/efikamx/Makefile      |    3 +
>  board/efikamx/efika.h       |    8 +
>  board/efikamx/efikamx-usb.c |  391
> +++++++++++++++++++++++++++++++++++++++++++ board/efikamx/efikamx.c     | 
>   1 +
>  include/configs/efikamx.h   |   16 ++
>  5 files changed, 419 insertions(+), 0 deletions(-)
>  create mode 100644 board/efikamx/efika.h
>  create mode 100644 board/efikamx/efikamx-usb.c
> 

Dear Jana Rapava,

[...]

> diff --git a/board/efikamx/efika.h b/board/efikamx/efika.h
> new file mode 100644
> index 0000000..2134e75
> --- /dev/null
> +++ b/board/efikamx/efika.h
> @@ -0,0 +1,8 @@

Proper header is missing here, with license, author etc.

> +/*
> + * EHCI USB
> + */
> +#ifdef	CONFIG_CMD_USB
> +void setup_iomux_usb(void);
> +#else
> +static inline void setup_iomux_usb(void) { }
> +#endif

[...]

> +u32 ulpi_wait(struct usb_ehci *ehci, u32 ulpi_bit, const char *operation)
> +{
> +	int timeout = ULPI_TIMEOUT;
> +	u32 tmp;
> +
> +	/* Wait for the ulpi_bit to become zero. */
> +	while (--timeout) {
> +		tmp = readl(&ehci->ulpi_viewpoint);
> +		if (!(tmp & ulpi_bit))
> +			break;
> +		WATCHDOG_RESET();
> +	}
> +
> +	if (!timeout) {
> +		printf("ULPI %s timed out\n", operation);
> +		return 0;
> +	}
> +	return tmp;
> +}
> +
> +void ulpi_write(struct usb_ehci *ehci, u32 reg, u32 value)
> +{
> +	if (!(readl(&ehci->ulpi_viewpoint) & ULPI_SS)) {
> +		writel(ULPI_WU, &ehci->ulpi_viewpoint);
> +		ulpi_wait(ehci, ULPI_WU, "wakeup");

This function returns some value, but you're not checking it. It's probably if 
the wait timed out, right ? Why aren't you checking it ?

> +	}
> +
> +	writel(ULPI_RWRUN | ULPI_RWCTRL |
> +		reg << ULPI_ADDR_SHIFT | ulpi_write_mask(value),
> +		&ehci->ulpi_viewpoint);
> +	ulpi_wait(ehci, ULPI_RWRUN, "write");

DTTO

> +}
> +
> +u32 ulpi_read(struct usb_ehci *ehci, u32 reg)
> +{
> +	if (!(readl(&ehci->ulpi_viewpoint) & ULPI_SS)) {
> +		writel(ULPI_WU, &ehci->ulpi_viewpoint);
> +		ulpi_wait(ehci, ULPI_WU, "wakeup");
> +	}
> +
> +	writel(ULPI_RWRUN | reg << ULPI_ADDR_SHIFT, &ehci->ulpi_viewpoint);
> +	return ulpi_read_mask(ulpi_wait(ehci, ULPI_RWRUN, "read"));

This seems wrong ?

Cheers