From: Finn Thain <fthain@telegraphics.com.au>
To: Ondrej Zary <linux@rainbow-software.org>
Cc: Christoph Hellwig <hch@infradead.org>,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] NCR5380: Use probe_irq_*() for IRQ probing
Date: Thu, 3 Nov 2016 13:16:42 +1100 (AEDT) [thread overview]
Message-ID: <alpine.LNX.2.00.1611022056350.12392@nippy.intranet> (raw)
In-Reply-To: <201611020900.31899.linux@rainbow-software.org>
On Wed, 2 Nov 2016, Ondrej Zary wrote:
> On Wednesday 02 November 2016, Finn Thain wrote:
> > On Mon, 31 Oct 2016, Ondrej Zary wrote:
> >
> > > + NCR5380_write(SELECT_ENABLE_REG, hostdata->id_mask);
> > > + NCR5380_write(OUTPUT_DATA_REG, hostdata->id_mask);
> > > + NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE | ICR_ASSERT_DATA | ICR_ASSERT_SEL);
> > > +
> > > + usleep_range(1000, 20000);
> >
> > Again, msleep(1) would be more appropriate here.
>
> WARNING: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt
> #164: FILE: drivers/scsi/g_NCR5380.c:88:
> + msleep(1);
>
That link is almost 10 years old. I wonder if it is still accurate.
Anyway, I take it that you chose usleep_range() so as to make the
checkpatch warning go away. Why not use checkpatch.conf for that, or just
ignore it? The upper bound for the delay is unimportant and so the warning
isn't applicable.
Since the lower bound is some unknown number of microseconds, I think
msleep(1) nicely expresses the two constraints. Whereas,
usleep_range(1000, 20000) misrepresents them.
We really don't need three significant figures. If you want precision,
surely you would have to measure the time it takes for the IRQ to fire,
and derive a worst case (lower bound) from the measurements.
--
next prev parent reply other threads:[~2016-11-03 2:16 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-31 20:18 [PATCH 0/6] (g_)NCR5380: Improve IRQ probing and some fixes Ondrej Zary
2016-10-31 20:18 ` [PATCH 1/6] NCR5380: Use probe_irq_*() for IRQ probing Ondrej Zary
2016-11-02 7:45 ` Finn Thain
2016-11-02 8:00 ` Ondrej Zary
2016-11-03 2:16 ` Finn Thain [this message]
2016-10-31 20:18 ` [PATCH 2/6] g_NCR5380: Test the IRQ before accepting it Ondrej Zary
2016-11-02 7:45 ` Finn Thain
2016-11-02 19:16 ` Ondrej Zary
2016-11-03 2:17 ` Finn Thain
2016-10-31 20:18 ` [PATCH 3/6] g_NCR5380: Check for chip presence before calling NCR5380_init() Ondrej Zary
2016-11-02 7:46 ` Finn Thain
2016-10-31 20:18 ` [PATCH 4/6] g_NCR5380: Add IRQ auto-configuration for HP C2502 Ondrej Zary
2016-11-02 7:46 ` Finn Thain
2016-11-02 8:29 ` Ondrej Zary
2016-11-03 2:17 ` Finn Thain
2016-11-03 8:00 ` Ondrej Zary
2016-11-04 3:00 ` Finn Thain
2016-10-31 20:18 ` [PATCH 5/6] g_NCR5380: Autoprobe IRQ by default Ondrej Zary
2016-11-02 7:47 ` Finn Thain
2016-12-03 0:39 ` Finn Thain
2016-10-31 20:18 ` [PATCH 6/6] g_NCR5380: Fix release region in error handling Ondrej Zary
2016-11-02 7:49 ` Finn Thain
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=alpine.LNX.2.00.1611022056350.12392@nippy.intranet \
--to=fthain@telegraphics.com.au \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=linux@rainbow-software.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).