[U-Boot] [PATCH v5] ulpi: add generic ULPI functionality

[Simon Glass]

Hi,

On Sun, Nov 27, 2011 at 4:19 PM, Jana Rapava <fermata7 at gmail.com> wrote:
> Add generic functions for reading, writing and setting bits in ULPI registers.
>
> Signed-off-by: Jana Rapava <fermata7 at gmail.com>
> Cc: Remy Bohmer <linux at bohmer.net>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Igor Grinberg <grinberg at compulab.co.il>
> Cc: Wolfgang Grandegger <wg at denx.de>
> Cc: Simon Glass <sjg at chromium.org>
> ---
> Changes for v2:
>      - make code EHCI-independent
>      - use udelay() in waiting loop
>      - mark static functions as static
>      - naming changes
> Changes for v3:
>       - merge with patch ulpi: add generic ULPI support header file
>       - rewrite ULPI interface in more functionality-oriented way
> Changes for v4:
>       - add error-checking
>       - add waiting for completion into ulpi_reset() function
> Changes for v5:
>        - CodingStyle changes
>        - add comments
>        - simplify implemenation of the ULPI interface functions

I think this is a lot cleaner, thank you.

>
>  Makefile                         |    1 +
>  drivers/usb/ulpi/Makefile        |   45 +++++++
>  drivers/usb/ulpi/ulpi-viewport.c |  114 ++++++++++++++++++
>  drivers/usb/ulpi/ulpi.c          |  158 +++++++++++++++++++++++++
>  include/usb/ulpi.h               |  238 ++++++++++++++++++++++++++++++++++++++
>  5 files changed, 556 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/usb/ulpi/Makefile
>  create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
>  create mode 100644 drivers/usb/ulpi/ulpi.c
>  create mode 100644 include/usb/ulpi.h
>
> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
> new file mode 100644
> index 0000000..c4dc408
> --- /dev/null
> +++ b/drivers/usb/ulpi/ulpi.c
> @@ -0,0 +1,158 @@
[snip]
> +int ulpi_drive_vbus(u32 ulpi_viewport,
> +       int ext_power_supply, int use_ext_indicator)
> +{
> +       int flags = ULPI_OTG_DRVVBUS |
> +               (ext_power_supply) ? ULPI_OTG_DRVVBUS_EXT : 0 |
> +               (use_ext_indicator) ? ULPI_OTG_EXTVBUSIND : 0;

You have a few places where some people might prefer brackets, but not
important. This one I think really should be:

 int flags = ULPI_OTG_DRVVBUS |
               (ext_power_supply ? ULPI_OTG_DRVVBUS_EXT : 0) |
               (use_ext_indicator ? ULPI_OTG_EXTVBUSIND : 0);

or perhaps if you like:

int flags = ULPI_OTG_DRVVBUS;

if (ext_power_supply)
   flags |= ULPI_OTG_DRVVBUS_EXT;
if (use_ext_indicator)
   flags |= ULPI_OTG_EXTVBUSIND;

> +
> +       return ulpi_write(ulpi_viewport, &ulpi->otg_ctrl_set, flags);
> +}
> +
> +/*

Otherwise:

Acked-by: Simon Glass <sjg at chromium.org>

Regards,
Simon