[PATCH] ide: ide_port_wait_ready() fix

October 11th, 2011 - 01:20 pm ET by Bartlomiej Zolnierkiewicz | Report spam

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: ide_port_wait_ready() fix

Fix for commit a20b2a4 ("ide: skip probe if there are no devices on
the port (v2)"). We must check for slave device before failing.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

drivers/ide/ide-probe.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Index: b/drivers/ide/ide-probe.c
a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -598,7 +598,7 @@ static int ide_port_wait_ready(ide_hwif_
{
const struct ide_tp_ops *tp_ops = hwif->tp_ops;
ide_drive_t *drive;
- int i, rc;
+ int i, rc, prev_rc = 0;

printk(KERN_DEBUG "Probing IDE interface %s...", hwif->name);

@@ -623,8 +623,10 @@ static int ide_port_wait_ready(ide_hwif_
tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS);
mdelay(2);
rc = ide_wait_not_busy(hwif, 35000);
- if (rc)
+ if (prev_rc && rc)
goto out;
+ prev_rc = rc;
+ rc = 0;
} else
printk(KERN_DEBUG "%s: ide_wait_not_busy() skipped",
drive->name);
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Follow the discussion Replies

4 replies

Make a reply

Replies

#1 David Miller

October 11th, 2011 - 03:20 pm ET | Report spam

From: Bartlomiej Zolnierkiewicz
Date: Tue, 11 Oct 2011 19:13:18 +0200

From: Bartlomiej Zolnierkiewicz
Subject: [PATCH] ide: ide_port_wait_ready() fix

Fix for commit a20b2a4 ("ide: skip probe if there are no devices on
the port (v2)"). We must check for slave device before failing.

Signed-off-by: Bartlomiej Zolnierkiewicz

This will mishandle the case where there is no slave in the device
list.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Reply to this message

#2 Bartlomiej Zolnierkiewicz

October 12th, 2011 - 11:10 am ET | Report spam

David Miller wrote:

From: Bartlomiej Zolnierkiewicz
Date: Tue, 11 Oct 2011 19:13:18 +0200

> From: Bartlomiej Zolnierkiewicz
> Subject: [PATCH] ide: ide_port_wait_ready() fix
>
> Fix for commit a20b2a4 ("ide: skip probe if there are no devices on
> the port (v2)"). We must check for slave device before failing.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz

This will mishandle the case where there is no slave in the device
list.

I don't see it:

@ -598,7 +598,7 @@ static int ide_port_wait_ready(ide_hwif_
{
const struct ide_tp_ops *tp_ops = hwif->tp_ops;
ide_drive_t *drive;
- int i, rc;
+ int i, rc, prev_rc = 0;

printk(KERN_DEBUG "Probing IDE interface %s...", hwif->name);

@@ -623,8 +623,10 @@ static int ide_port_wait_ready(ide_hwif_
tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS);
mdelay(2);
rc = ide_wait_not_busy(hwif, 35000);
- if (rc)
+ if (prev_rc && rc)
goto out;
+ prev_rc = rc;
+ rc = 0;
} else
printk(KERN_DEBUG "%s: ide_wait_not_busy() skipped",
drive->name);

If there is no slave device but there is a master device the code falls-through
and returns a success.

The patch fixes regression introduced in commit a20b2a4 as some esoteric
setups return ide_wait_not_busy() -ENODEV error on master device while there
is slave device present in the system.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Reply to this message

#3 David Miller

October 12th, 2011 - 03:10 pm ET | Report spam

From: Bartlomiej Zolnierkiewicz
Date: Wed, 12 Oct 2011 16:59:55 +0200

David Miller wrote:

From: Bartlomiej Zolnierkiewicz
Date: Tue, 11 Oct 2011 19:13:18 +0200

> From: Bartlomiej Zolnierkiewicz
> Subject: [PATCH] ide: ide_port_wait_ready() fix
>
> Fix for commit a20b2a4 ("ide: skip probe if there are no devices on
> the port (v2)"). We must check for slave device before failing.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz

This will mishandle the case where there is no slave in the device
list.

I don't see it:

@ -598,7 +598,7 @@ static int ide_port_wait_ready(ide_hwif_
{
const struct ide_tp_ops *tp_ops = hwif->tp_ops;
ide_drive_t *drive;
- int i, rc;
+ int i, rc, prev_rc = 0;

printk(KERN_DEBUG "Probing IDE interface %s...", hwif->name);

@@ -623,8 +623,10 @@ static int ide_port_wait_ready(ide_hwif_
tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS);
mdelay(2);
rc = ide_wait_not_busy(hwif, 35000);
- if (rc)
+ if (prev_rc && rc)
goto out;
+ prev_rc = rc;
+ rc = 0;
} else
printk(KERN_DEBUG "%s: ide_wait_not_busy() skipped",
drive->name);

If there is no slave device but there is a master device the code falls-through
and returns a success.

That's not what we want, if there is only a master device and no slave device
in the list this loop is iterating over we want to return the error code
in "rc", not zero.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Reply to this message

#4 Bartlomiej Zolnierkiewicz

October 13th, 2011 - 06:50 am ET | Report spam

David Miller wrote:

From: Bartlomiej Zolnierkiewicz
Date: Wed, 12 Oct 2011 16:59:55 +0200

> David Miller wrote:
>
>> From: Bartlomiej Zolnierkiewicz
>> Date: Tue, 11 Oct 2011 19:13:18 +0200
>>
>> > From: Bartlomiej Zolnierkiewicz
>> > Subject: [PATCH] ide: ide_port_wait_ready() fix
>> >
>> > Fix for commit a20b2a4 ("ide: skip probe if there are no devices on
>> > the port (v2)"). We must check for slave device before failing.
>> >
>> > Signed-off-by: Bartlomiej Zolnierkiewicz
>>
>> This will mishandle the case where there is no slave in the device
>> list.
>
> I don't see it:
>
> @ -598,7 +598,7 @@ static int ide_port_wait_ready(ide_hwif_
> {
> const struct ide_tp_ops *tp_ops = hwif->tp_ops;
> ide_drive_t *drive;
> - int i, rc;
> + int i, rc, prev_rc = 0;
>
> printk(KERN_DEBUG "Probing IDE interface %s...", hwif->name);
>
> @@ -623,8 +623,10 @@ static int ide_port_wait_ready(ide_hwif_
> tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS);
> mdelay(2);
> rc = ide_wait_not_busy(hwif, 35000);
> - if (rc)
> + if (prev_rc && rc)
> goto out;
> + prev_rc = rc;
> + rc = 0;
> } else
> printk(KERN_DEBUG "%s: ide_wait_not_busy() skipped",
> drive->name);
>
> If there is no slave device but there is a master device the code falls-through
> and returns a success.

That's not what we want, if there is only a master device and no slave device
in the list this loop is iterating over we want to return the error code
in "rc", not zero.

No, we want to return zero (success) since at least once device was found
(otherwise we fail probe on some esoteric setups returning -ENODEV from
ide_wait_not_busy() for master device).

This is how this function worked before commit a20b2a4 if you want something
else okay but it needs to work with aforementioned setups.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Follow the discussion Replies

Reply to this message

Create a new topic Replies

Make a reply

Make your own search

[PATCH] ide: ide_port_wait_ready() fix

Similar topics

Replies