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

[Vladimir Zapolskiy]

On 12.08.2015 22:47, LEMIEUX, SYLVAIN wrote:
> Hi Vladimir,
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: 12-Aug-15 3:42 PM
>>
>> 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 at 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 at 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).
>>
> 
> Vladimir, are you OK with keeping the code as-is?
> It will make it easier when we attempt to create a generic
> "wait_for_bit()" function.

Okay, if maintainer says, let it remain :)

>>>> + 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;
>>>> +}
>>
>> [...]
>