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

Marek Vasut
2011-09-27T19:47:13Z

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