[PATCH] atp870u: Split long udelay()

[PATCH] atp870u: Split long udelay()

[Ben Hutchings]

udelay() is supposed to be limited to 1 ms, and will generate a
__bad_udelay() on ARM for constant arguments > 2000.  Split
udelay(0x800) into mdelay(2); udelay(48).  (I suspect that msleep(3)
would work but I do not know how critical the timing is here.)

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/scsi/atp870u.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index b137e56..bf0ecda 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -1171,7 +1171,8 @@ wait_io1:
 	outw(val, tmport);
 	outb(2, 0x80);
 TCM_SYNC:
-	udelay(0x800);
+	mdelay(2);
+	udelay(48);
 	if ((inb(tmport) & 0x80) == 0x00) {	/* bsy ? */
 		outw(0, tmport--);
 		outb(0, tmport);
-- 
1.6.6.2

Re: [PATCH] atp870u: Split long udelay()

[James Bottomley]

On Fri, 2010-02-19 at 01:56 +0000, Ben Hutchings wrote:
> udelay() is supposed to be limited to 1 ms, and will generate a
> __bad_udelay() on ARM for constant arguments > 2000.  Split
> udelay(0x800) into mdelay(2); udelay(48).  (I suspect that msleep(3)
> would work but I do not know how critical the timing is here.)

So no to this one.  Either ARM is right and udelay > 2000 is wrong
(which actually sounds correct given how long this will eat CPU for) so
the driver needs fixing, or ARM is wrong and the warning needs fixing.

Splitting the delay to defeat the warning while retaining the behaviour
it was trying to warn about is the wrong thing to do.

James

Re: [PATCH] atp870u: Split long udelay()

[Ben Hutchings]

On Fri, Feb 19, 2010 at 09:28:19AM -0600, James Bottomley wrote:
> On Fri, 2010-02-19 at 01:56 +0000, Ben Hutchings wrote:
> > udelay() is supposed to be limited to 1 ms, and will generate a
> > __bad_udelay() on ARM for constant arguments > 2000.  Split
> > udelay(0x800) into mdelay(2); udelay(48).  (I suspect that msleep(3)
> > would work but I do not know how critical the timing is here.)
> 
> So no to this one.  Either ARM is right and udelay > 2000 is wrong
> (which actually sounds correct given how long this will eat CPU for) so
> the driver needs fixing, or ARM is wrong and the warning needs fixing.
> 
> Splitting the delay to defeat the warning while retaining the behaviour
> it was trying to warn about is the wrong thing to do.
 
AIUI the restrictions on udelay() are there because the delay is likely
to be inaccurate for large arguments.  If it was actually wrong to wait
for over a millisecond then mdelay() would not exist.

Ben.

-- 
Ben Hutchings
The two most common things in the universe are hydrogen and stupidity.

Re: [PATCH] atp870u: Split long udelay()

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

On Fri, 2010-02-19 at 16:44 +0000, Ben Hutchings wrote:
> On Fri, Feb 19, 2010 at 09:28:19AM -0600, James Bottomley wrote:
> > On Fri, 2010-02-19 at 01:56 +0000, Ben Hutchings wrote:
> > > udelay() is supposed to be limited to 1 ms, and will generate a
> > > __bad_udelay() on ARM for constant arguments > 2000.  Split
> > > udelay(0x800) into mdelay(2); udelay(48).  (I suspect that msleep(3)
> > > would work but I do not know how critical the timing is here.)
> > 
> > So no to this one.  Either ARM is right and udelay > 2000 is wrong
> > (which actually sounds correct given how long this will eat CPU for) so
> > the driver needs fixing, or ARM is wrong and the warning needs fixing.
> > 
> > Splitting the delay to defeat the warning while retaining the behaviour
> > it was trying to warn about is the wrong thing to do.
>  
> AIUI the restrictions on udelay() are there because the delay is likely
> to be inaccurate for large arguments.  If it was actually wrong to wait
> for over a millisecond then mdelay() would not exist.

Please could you respond to the above?

Ben.

-- 
Ben Hutchings
Horngren's Observation:
                   Among economists, the real world is often a special case.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]