Hi Vladimir and Marek,
> -----Original Message-----
> From: Vladimir Zapolskiy [mailto:v...@mleia.com]
> Sent: 12-Aug-15 1:55 PM
>
> Hi Sylvain,
>
> On 10.08.2015 15:16, slemieux.t...@gmail.com wrote:
> > From: Sylvain Lemieux <slemi...@tycoint.com>
> >
> > Incorporate USB driver from legacy LPCLinux NXP BSP.
> > The files taken from the legacy patch are:
> > - lpc32xx USB driver
> > - lpc3250 header file USB registers definition.
> >
> > The legacy driver was updated and clean-up as part of the integration with
> > the latest u-boot.
> >
> > Signed-off-by: Sylvain Lemieux <slemi...@tycoint.com>
> > ---
[...]
> > +
> > +static int wait_for_bit(void *reg, const u32 mask, bool set)
> > +{
>
> (set == false) argument is not in use, and hence there is a piece of
> dead code in the function.
>
See comment in other response; I prefer to keep this way.
It will make it easier to introduce a generic function.
> > + u32 val;
> > + unsigned long start = get_timer(0);
> > +
> > + while (1) {
> > + val = readl(reg);
> > + if (!set)
> > + val = ~val;
> > +
> > + if ((val & mask) == mask)
> > + return 0;
> > +
> > + if (get_timer(start) > CONFIG_SYS_HZ)
> > + break;
> > +
> > + udelay(1);
> > + }
> > +
> > + debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n",
> > + __func__, reg, mask, set);
>
> I would recommend on error path always to display this message to a user.
>
OK
[...]
> > +
> > + /* enable I2C clock in OTG block if it isn't */
> > + if ((readl(&otg->otg_clk_sts) & OTG_CLK_I2C_EN) != OTG_CLK_I2C_EN) {
>
> if (!(readl(&otg->otg_clk_sts) & OTG_CLK_I2C_EN))
>
> > + writel(OTG_CLK_I2C_EN, &otg->otg_clk_ctrl);
> > +
> > + ret = wait_for_bit(&otg->otg_clk_sts, OTG_CLK_I2C_EN, 1);
> > + if (ret)
> > + return ret;
>
> Or actually the embracing check may be dropped and
> writel(OTG_CLK_I2C_EN) / wait_for_bit() are done unconditionally.
I will change to always write the register and perform the wait operation.
>
> > + }
> > +
> > + /* Configure ISP1301 */
> > + isp1301_configure();
>
> Not sure if there are any LPC32xx boards with USB host/device, but
> without ISP1301 phy. Let it be here until such a board emerges, if Marek
> does not object.
>
> In general I wonder, won't drivers/usb/phy/ be a better place for this
> driver or bigger part of it?
The Embedded Artists LPC3250 OEM Board is having the USB with the ISP1301;
http://www.embeddedartists.com/products/kits/lpc3250_kit.php
The legacy NXP LPC32xx BSP included board support files for it.
[...]
> >
>
> Feel free to add my
>
> Tested-by: Vladimir Zapolskiy <v...@mleia.com>
>
> --
> With best wishes,
> Vladimir
________________________________
This e-mail contains privileged and confidential information intended for the
use of the addressees named above. If you are not the intended recipient of
this e-mail, you are hereby notified that you must not disseminate, copy or
take any action in respect of any information contained in it. If you have
received this e-mail in error, please notify the sender immediately by e-mail
and immediately destroy this e-mail and its attachments.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
