* [PATCH] never called printk statement in ide-taskfile.c::wait_drive_not_busy
@ 2007-06-05 2:21 Masatake YAMATO
2007-06-05 17:08 ` Chuck Ebbert
0 siblings, 1 reply; 6+ messages in thread
From: Masatake YAMATO @ 2007-06-05 2:21 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-kernel
Hi,
Patches appended to this mail fixes a bug explained below.
There are two ways to fix the bug. PLEASE CHOOSE BETTER ONE.
Look at wait_drive_not_busy in drivers/ide/ide-taskfile.c:
static u8 wait_drive_not_busy(ide_drive_t *drive)
{
ide_hwif_t *hwif = HWIF(drive);
int retries = 100;
u8 stat;
/*
* Last sector was transfered, wait until drive is ready.
* This can take up to 10 usec, but we will wait max 1 ms
* (drive_cmd_intr() waits that long).
*/
while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
udelay(10);
if (!retries)
printk(KERN_ERR "%s: drive still BUSY!\n", drive->name);
return stat;
}
`printk' is never called because `retries' never holds zero at the
outside of `while' loop: when `retries' holds zero at the while's loop
condition, `retries' will hold -1 at the if condition.
I'm not on this mailing list, so add my address to Cc: when you reply to me.
Signed-off-by: Masatake YAMATO <jet@gyve.org>
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index 30175c7..a74df05 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -246,7 +246,7 @@ static u8 wait_drive_not_busy(ide_drive_t *drive)
* This can take up to 10 usec, but we will wait max 1 ms
* (drive_cmd_intr() waits that long).
*/
- while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
+ while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && --retries)
udelay(10);
if (!retries)
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index 30175c7..5e05311 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -249,7 +249,7 @@ static u8 wait_drive_not_busy(ide_drive_t *drive)
while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
udelay(10);
- if (!retries)
+ if (retries < 0)
printk(KERN_ERR "%s: drive still BUSY!\n", drive->name);
return stat;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] never called printk statement in ide-taskfile.c::wait_drive_not_busy
2007-06-05 2:21 [PATCH] never called printk statement in ide-taskfile.c::wait_drive_not_busy Masatake YAMATO
@ 2007-06-05 17:08 ` Chuck Ebbert
2007-06-05 17:35 ` Masatake YAMATO
0 siblings, 1 reply; 6+ messages in thread
From: Chuck Ebbert @ 2007-06-05 17:08 UTC (permalink / raw)
To: Masatake YAMATO; +Cc: Bartlomiej Zolnierkiewicz, linux-kernel
On 06/04/2007 10:21 PM, Masatake YAMATO wrote:
> diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
> index 30175c7..5e05311 100644
> --- a/drivers/ide/ide-taskfile.c
> +++ b/drivers/ide/ide-taskfile.c
> @@ -249,7 +249,7 @@ static u8 wait_drive_not_busy(ide_drive_t *drive)
> while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
> udelay(10);
>
> - if (!retries)
> + if (retries < 0)
if (stat & BUSY_STAT)
> printk(KERN_ERR "%s: drive still BUSY!\n", drive->name);
>
> return stat;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] never called printk statement in ide-taskfile.c::wait_drive_not_busy
2007-06-05 17:08 ` Chuck Ebbert
@ 2007-06-05 17:35 ` Masatake YAMATO
2007-06-15 22:45 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 6+ messages in thread
From: Masatake YAMATO @ 2007-06-05 17:35 UTC (permalink / raw)
To: cebbert; +Cc: B.Zolnierkiewicz, linux-kernel
> On 06/04/2007 10:21 PM, Masatake YAMATO wrote:
> > diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
> > index 30175c7..5e05311 100644
> > --- a/drivers/ide/ide-taskfile.c
> > +++ b/drivers/ide/ide-taskfile.c
> > @@ -249,7 +249,7 @@ static u8 wait_drive_not_busy(ide_drive_t *drive)
> > while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
> > udelay(10);
> >
> > - if (!retries)
> > + if (retries < 0)
>
> if (stat & BUSY_STAT)
>
> > printk(KERN_ERR "%s: drive still BUSY!\n", drive->name);
> >
Oh, yes.
Giving `retries' both roles: loop counter and condition flag for logging
may not good.
for (retries = 0; retries < 100; retries++)
{
if ((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT)
break;
udelay(10);
}
if (stat & BUSY_STAT)
printk(KERN_ERR "%s: drive still BUSY!\n", drive->name);
Masatake YAMATO
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] never called printk statement in ide-taskfile.c::wait_drive_not_busy
2007-06-05 17:35 ` Masatake YAMATO
@ 2007-06-15 22:45 ` Bartlomiej Zolnierkiewicz
2007-06-17 18:05 ` Masatake YAMATO
0 siblings, 1 reply; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-06-15 22:45 UTC (permalink / raw)
To: Masatake YAMATO; +Cc: cebbert, linux-kernel
Hi,
On Tuesday 05 June 2007, Masatake YAMATO wrote:
> > On 06/04/2007 10:21 PM, Masatake YAMATO wrote:
> > > diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
> > > index 30175c7..5e05311 100644
> > > --- a/drivers/ide/ide-taskfile.c
> > > +++ b/drivers/ide/ide-taskfile.c
> > > @@ -249,7 +249,7 @@ static u8 wait_drive_not_busy(ide_drive_t *drive)
> > > while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
> > > udelay(10);
> > >
> > > - if (!retries)
> > > + if (retries < 0)
> >
> > if (stat & BUSY_STAT)
> >
> > > printk(KERN_ERR "%s: drive still BUSY!\n", drive->name);
> > >
>
> Oh, yes.
> Giving `retries' both roles: loop counter and condition flag for logging
> may not good.
>
>
> for (retries = 0; retries < 100; retries++)
> {
> if ((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT)
> break;
>
> udelay(10);
> }
>
>
> if (stat & BUSY_STAT)
> printk(KERN_ERR "%s: drive still BUSY!\n", drive->name);
Please re-submit this fix in the form of a patch so I can merge it.
Thanks,
Bart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] never called printk statement in ide-taskfile.c::wait_drive_not_busy
2007-06-15 22:45 ` Bartlomiej Zolnierkiewicz
@ 2007-06-17 18:05 ` Masatake YAMATO
2007-06-21 19:56 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 6+ messages in thread
From: Masatake YAMATO @ 2007-06-17 18:05 UTC (permalink / raw)
To: bzolnier; +Cc: cebbert, linux-kernel
> Please re-submit this fix in the form of a patch so I can merge it.
Here it is.
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index 30175c7..aa06daf 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -238,7 +238,7 @@ EXPORT_SYMBOL(task_no_data_intr);
static u8 wait_drive_not_busy(ide_drive_t *drive)
{
ide_hwif_t *hwif = HWIF(drive);
- int retries = 100;
+ int retries;
u8 stat;
/*
@@ -246,10 +246,14 @@ static u8 wait_drive_not_busy(ide_drive_t *drive)
* This can take up to 10 usec, but we will wait max 1 ms
* (drive_cmd_intr() waits that long).
*/
- while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
- udelay(10);
+ for (retries = 0; retries < 100; retries++) {
+ if ((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT)
+ udelay(10);
+ else
+ break;
+ }
- if (!retries)
+ if (stat & BUSY_STAT)
printk(KERN_ERR "%s: drive still BUSY!\n", drive->name);
return stat;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] never called printk statement in ide-taskfile.c::wait_drive_not_busy
2007-06-17 18:05 ` Masatake YAMATO
@ 2007-06-21 19:56 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-06-21 19:56 UTC (permalink / raw)
To: Masatake YAMATO; +Cc: cebbert, linux-kernel
On Sunday 17 June 2007, Masatake YAMATO wrote:
> > Please re-submit this fix in the form of a patch so I can merge it.
>
> Here it is.
Applied but please remember to always include patch description when
sending the improved version of the patch. This time I fixed it manually
to speed up the procedure.
Thanks,
Bart
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-06-21 19:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-05 2:21 [PATCH] never called printk statement in ide-taskfile.c::wait_drive_not_busy Masatake YAMATO
2007-06-05 17:08 ` Chuck Ebbert
2007-06-05 17:35 ` Masatake YAMATO
2007-06-15 22:45 ` Bartlomiej Zolnierkiewicz
2007-06-17 18:05 ` Masatake YAMATO
2007-06-21 19:56 ` Bartlomiej Zolnierkiewicz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox