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

[Jana Rapava]

2011/11/27 Simon Glass <sjg at chromium.org>

> > +static int ulpi_wait(u32 ulpi_viewport, u32 ulpi_value, u32 ulpi_mask)
>
>
> How about a comment as to what this function does, params and return
> values?
>
> Also perhaps if you pass the operation to this function it can print
> the error message (perhaps with debug()) rather than ever calling
> having to do it? Or at least decide whether messages should be in
> bottom-level functions or in top-level - and perhaps considering using
> debug() instead of printf() at lower levels.
>
> > +{
> > +       int timeout = CONFIG_USB_ULPI_TIMEOUT;
> > +       u32 tmp;
> > +
> > +       writel(ulpi_value, ulpi_viewport);
>
> Why do the write here? Should it be done in the write call below? At
> the very least you should rename this function.
>
>
This function issues ULPI wakeup or read-write request, writes the request
into ULPI viewport register and waits until ULPI interface signalizes
completion
of this request.
With error messages is the situation a bit difficult, because this function
only knows
whether the request was for wakeup or for read-write.
 I think more specific error messages  are more helpful, so I decided to
put them into
functions which knows what we've attempted to
do(ulpi_wakeup()/ulpi_read()/ulpi-write()).
 If you really think that debug() is more appropriate in these functions, I
can change this,
but Igor doesn't consider this a reason for this patch not to be accepted,
and I agree with him.

And I'll rename this function into ulpi_request(), ulpi_wait() is really a
bit deceiveing.
Thanks for your comments.
Regards,
Jana Rapava