From mboxrd@z Thu Jan  1 00:00:00 1970
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Thu, 13 Aug 2015 00:35:25 +0300
Subject: [U-Boot] [PATCH v6 5/5] usb: lpc32xx: add host USB driver
In-Reply-To: <4F172219764C784B84C2C1FF44E7DFB10301123F@003FCH1MPN2-041.003f.mgd2.msft.net>
References: <1439208994-19072-4-git-send-email-slemieux.tyco@gmail.com>
 <55CB8891.7000603@mleia.com> <201508122141.32344.marex@denx.de>
 <4F172219764C784B84C2C1FF44E7DFB10301123F@003FCH1MPN2-041.003f.mgd2.msft.net>
Message-ID: <55CBBC1D.1080404@mleia.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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