2011/11/27 Igor Grinberg <grinb...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
