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

[Marek Vasut]

> 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>
> ---
> 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
> Changes for v7:
>               - add proper header to efika.h
>               - include efika.h into efikamx.c
>               - check return values from ulpi_wait()
> Changes for v8:
>     - change the return value of ulpi_wait()
>     - CodingStyle cleanup
>     - add proper header to efikamx-usb.c

Dear Jana Rapava,

[...]

> +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_bit)
> +{
> +    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)
> +        return -1;
> +    else
> +        return tmp;
> +}

Won't 'return !timeout;' be enough?

Also, can you change 'ulpi_bit' to 'ulpi_mask'? That seems more appropriate.

Maybe this can be even changed to ulpi_wait(ehci, value_to_be_written, bit_to_be_polled), then you won't need those writel()s before every write to ulpi anymore.

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


aha here - if(ulpi_wait(ehci, ULPI_RWRUN | ULPI_RWCTRL ..., uLPI_RWRUN))...

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

Still (three revs already) no newline here!!

> +    /* split into vendor and product ID */
> +    debug("Found ULPI TX, ID %04x:%04x\n", tmp >> 16, tmp & 0xffff);

cheers