[U-Boot] [PATCH v6 5/5] usb: lpc32xx: add host USB driver

[Marek Vasut <marex@denx.de>]

On Wednesday, August 12, 2015 at 07:55:29 PM, Vladimir Zapolskiy wrote:
> Hi Sylvain,
> 
> On 10.08.2015 15:16, slemieux.tyco at gmail.com wrote:
> > From: Sylvain Lemieux <slemieux@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 <slemieux@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.

I'd prefer this to be the way it is, since this function can be extracted
and made into generic code (probably in subsequent patch).

> > +	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.

Yeah.

> > +	return -ETIMEDOUT;
> > +}

[...]