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

Jana Rapava fermata7 at gmail.com
Sun Nov 27 23:37:34 CET 2011


2011/11/27 Igor Grinberg <grinberg at compulab.co.il>

> >
> >> +{
> >> +       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.
>
>
> If you move this writel() from here, then you need to add
> it to every function calling this one (it is not just the write below).
> It can be done, but again can we do this later?
>
>
If you would really want, I can do it later, but I think that rename this
function
and document it properly is better way.

>
> >
> >> +}
> >> +
> >> +static int ulpi_wakeup(u32 ulpi_viewport)
> >> +{
> >> +       if (readl(ulpi_viewport) & ULPI_SS)
> >> +               return 0; /* already awake */
> >> +
> >> +       return ulpi_wait(ulpi_viewport, ULPI_WU, ULPI_WU);
> >> +}
> >> +
> >> +u32 ulpi_write(u32 ulpi_viewport, u32 reg, u32 value)
> >
> > It seems that every caller requires a cast for the second parameter.
> > Perhaps you could avoid this if you make the reg parameter u8 *
> > instead?
>
> This is what I would call a "no go"...
> Jana, can you, please deal with this?
> And if you do, then you can deal with *most* of other comments as well.
>
>
Ok, I'll do so.
Regards,
Jana Rapava


More information about the U-Boot mailing list