From: der.herr@hofr.at (Nicholas Mc Guire)
To: kernelnewbies@lists.kernelnewbies.org
Subject: udelay vs usleep_range
Date: Sun, 11 Sep 2016 16:33:54 +0000 [thread overview]
Message-ID: <20160911163353.GA9073@osadl.at> (raw)
In-Reply-To: <CAOCi2DEXoxZSmTSAubGuX9m4RDrqt109ed5JKrinWQO=4pky2g@mail.gmail.com>
On Sun, Sep 11, 2016 at 09:28:29PM +0530, Gargi Sharma wrote:
> Hi all,
>
> I ran the checkpatch script over drivers/staging/nvec.c and got the
> following warning
>
> udelay(33);
>
> CHECK: usleep_range is preferred over udelay; see
> Documentation/timers/timers-howto.txt
>
> I checked out the timers-howto.txt and for usleep_range, one has to specify
> the lower and upper limit for the timer. How does one decide what the upper
> limit will be if one is to change the function from udelay to usleep_range?
>
generally you would try to figure out what the rational for the delay is
and what boundary it might have. Eg. the udelay(100) in
drivers/staging/nvec/nvec.c carries the comment:
* We experience less incomplete messages with this delay than without
* it, but we don't know why. Help is appreciated.
indicating that this time might actually not be that prcise
and even if you can just provide a small range say 100 to 110 microseconds
this would help the timer subsystem a lot.
If you can not come up with a proposal based on comments (dont worry about
the time being wrong - you will send your patch to the driver maintainer
for review anyway), then you can always simply send mail to the author
and ask. Actually it would be good if the 33 microseconds would be explained
by some comment rather than leaving these "magic" numbers to speculation.
As this is a set of slow devices (kbd, mouse, power) and any current system
must be asumed to exhibit jitter in the range of atleast 10s of microseconds
you most likely can safely change 33 microseconds to a range of (30,40)
which already can allow significant optimization of runtime timer handling.
So the fix might not only contain a API change to usleep_range() but also
change the values to something meaningful in a macro like
#define I2C_RX_DELAY 30 /* minimum waiting time for I2C command response */
usleep_range(I2C_RX_DELAY, I2C_RX_DELAY + 10)
To find out which list to ask if unsure use:
scripts/get_maintainer.pl drivers/staging/nvec/nvec.c
and check for those marked with "open list" or
git blame -L 630,640 drivers/staging/nvec/nvec.c
(in this case the udelay(33) was on line 634) to ask the
author of that line of code where the udelay(33) comes from.
thx!
hofrat
next prev parent reply other threads:[~2016-09-11 16:33 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-11 15:58 udelay vs usleep_range Gargi Sharma
2016-09-11 16:33 ` Nicholas Mc Guire [this message]
2016-09-11 16:37 ` Peter Senna Tschudin
2016-09-11 16:39 ` Nicholas Mc Guire
2016-09-11 16:48 ` Nicholas Mc Guire
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=20160911163353.GA9073@osadl.at \
--to=der.herr@hofr.at \
--cc=kernelnewbies@lists.kernelnewbies.org \
/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;
as well as URLs for NNTP newsgroup(s).