Re: [PATCH 46/64] ide: add drive->pio_mode field

From: Sergei Shtylyov
Date: Thu Jan 21 2010 - 12:50:17 EST


Hello.

Bartlomiej Zolnierkiewicz wrote:

On Monday 18 January 2010 08:28:22 pm Sergei Shtylyov wrote:
Bartlomiej Zolnierkiewicz wrote:

From: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
Subject: [PATCH] ide: add drive->pio_mode field

Add pio_mode field to ide_drive_t matching pio_mode field used in
struct ata_device.

The validity of the field is restricted to ->set_pio_mode method
only currently in IDE subsystem.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
---
drivers/ide/ide-devsets.c | 2 ++
drivers/ide/ide-probe.c | 2 ++
drivers/ide/ide-xfer-mode.c | 3 +++
include/linux/ide.h | 1 +
4 files changed, 8 insertions(+)

Index: b/drivers/ide/ide-devsets.c
===================================================================
--- a/drivers/ide/ide-devsets.c
+++ b/drivers/ide/ide-devsets.c
@@ -105,6 +105,8 @@ static int set_pio_mode(ide_drive_t *dri
return -ENOSYS;
if (set_pio_mode_abuse(drive->hwif, arg)) {
+ drive->pio_mode = arg + XFER_PIO_0;
+
Er, didn't understand this... these 'arg' values are not really PIO modes, no?

In the usual case they are

But after set_pio_mode_abuse() returned non-zero we know it's not usual case, no?

and a few 'special' ->set_pio_mode implementations
will extract the desired 'arg' value just fine by doing '- XFER_PIO_0'..

I don't understand why we should change drive->pio_mode if we know it's not a PIO mode but "abuse value". Perhaps you're dropping the mode argument from set_pio_mode() in some of the next patches? Doesn't seem a good idea, given that this "abuse case" still exist...

--
Bartlomiej Zolnierkiewicz

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/