public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Vladimir Zapolskiy <vz@mleia.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v6 5/5] usb: lpc32xx: add host USB driver
Date: Thu, 13 Aug 2015 00:35:25 +0300	[thread overview]
Message-ID: <55CBBC1D.1080404@mleia.com> (raw)
In-Reply-To: <4F172219764C784B84C2C1FF44E7DFB10301123F@003FCH1MPN2-041.003f.mgd2.msft.net>

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

  reply	other threads:[~2015-08-12 21:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-10 12:16 [U-Boot] [PATCH v6 5/5] usb: lpc32xx: add host USB driver slemieux.tyco at gmail.com
2015-08-10 12:23 ` Marek Vasut
2015-08-10 13:15   ` LEMIEUX, SYLVAIN
2015-08-12 17:55 ` Vladimir Zapolskiy
2015-08-12 19:41   ` Marek Vasut
2015-08-12 19:47     ` LEMIEUX, SYLVAIN
2015-08-12 21:35       ` Vladimir Zapolskiy [this message]
2015-08-12 22:10         ` Marek Vasut
2015-08-12 20:00   ` LEMIEUX, SYLVAIN
2015-08-13  0:04     ` Vladimir Zapolskiy
2015-08-13 11:55       ` LEMIEUX, SYLVAIN
2015-08-13 15:03         ` Vladimir Zapolskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55CBBC1D.1080404@mleia.com \
    --to=vz@mleia.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox