ide: DMA reporting and validity checking fixes (take 3)

* ide_xfer_verbose() fixups:
  - beautify returned mode names
  - fix PIO5 reporting
  - make it return 'const char *'

* Change printk() level from KERN_DEBUG to KERN_INFO in ide_find_dma_mode().

* Add ide_id_dma_bug() helper based on ide_dma_verbose() to check for invalid
  DMA info in identify block.

* Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().

  As a result DMA won't be tuned or will be disabled after tuning if device
  reports inconsistent info about enabled DMA mode (ide_dma_verbose() does the
  same checks while the IDE device is probed by ide-{cd,disk} device driver).

* Remove no longer needed ide_dma_verbose().

This patch should fix the following problem with out-of-sync IDE messages
reported by Nick Warne:

       hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
       skipping word 93 validity check
        , UDMA(66)

and later debugged by Mark Lord to be caused by:

        ide_dma_verbose()
                printk( ... "2048kB Cache");
        eighty_ninty_three()
                printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
        ide_dma_verbose()
                printk(", UDMA(66)"

Please note that as a result ide-{cd,disk} device drivers won't report the
DMA speed used but this is intended since now DMA mode being used is always
reported by IDE core code.

v2:
* fixes suggested by Randy:
  - use KERN_CONT for printk()-s in ide-{cd,disk}.c
  - don't remove argument name from ide_xfer_verbose() declaration

v3:
* Remove incorrect check for (id->field_valid & 1) from ide_id_dma_bug()
  (spotted by Sergei).

* "XFER SLOW" -> "PIO SLOW" in ide_xfer_verbose() (suggested by Sergei).

* Fix ide_find_dma_mode() to report the correct mode ('mode' after being
  limited by 'req_mode').

Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: Nick Warne <nick@ukfsn.org>
Cc: Mark Lord <lkml@rtr.ca>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 57a5f63..db9abd8 100644 (file)
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -3049,12 +3049,7 @@ int ide_cdrom_probe_capabilities (ide_drive_t *drive)
         else   
                printk(" drive");
 
-       printk(", %dkB Cache", be16_to_cpu(cap.buffer_size));
-
-       if (drive->using_dma)
-               ide_dma_verbose(drive);
-
-       printk("\n");
+       printk(KERN_CONT ", %dkB Cache\n", be16_to_cpu(cap.buffer_size));
 
        return nslots;
 }

diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index 00123d9..b77c849 100644 (file)
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -961,11 +961,8 @@ static void idedisk_setup (ide_drive_t *drive)
        if (id->buf_size)
                printk (" w/%dKiB Cache", id->buf_size/2);
 
-       printk(", CHS=%d/%d/%d", 
-              drive->bios_cyl, drive->bios_head, drive->bios_sect);
-       if (drive->using_dma)
-               ide_dma_verbose(drive);
-       printk("\n");
+       printk(KERN_CONT ", CHS=%d/%d/%d\n",
+                        drive->bios_cyl, drive->bios_head, drive->bios_sect);
 
        /* write cache enabled? */
        if ((id->csfo & 1) || (id->cfs_enable_1 & (1 << 5)))

diff --git a/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c
index 0d795a1..6879aaa 100644 (file)
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -753,10 +753,12 @@ u8 ide_find_dma_mode(ide_drive_t *drive, u8 req_mode)
                        mode = XFER_MW_DMA_1;
        }
 
-       printk(KERN_DEBUG "%s: %s mode selected\n", drive->name,
+       mode = min(mode, req_mode);
+
+       printk(KERN_INFO "%s: %s mode selected\n", drive->name,
                          mode ? ide_xfer_verbose(mode) : "no DMA");
 
-       return min(mode, req_mode);
+       return mode;
 }
 
 EXPORT_SYMBOL_GPL(ide_find_dma_mode);
@@ -772,6 +774,9 @@ static int ide_tune_dma(ide_drive_t *drive)
        if (__ide_dma_bad_drive(drive))
                return 0;
 
+       if (ide_id_dma_bug(drive))
+               return 0;
+
        if (drive->hwif->host_flags & IDE_HFLAG_TRUST_BIOS_FOR_DMA)
                return config_drive_for_dma(drive);
 
@@ -806,58 +811,23 @@ static int ide_dma_check(ide_drive_t *drive)
        return vdma ? 0 : -1;
 }
 
-void ide_dma_verbose(ide_drive_t *drive)
+int ide_id_dma_bug(ide_drive_t *drive)
 {
-       struct hd_driveid *id   = drive->id;
-       ide_hwif_t *hwif        = HWIF(drive);
+       struct hd_driveid *id = drive->id;
 
        if (id->field_valid & 4) {
                if ((id->dma_ultra >> 8) && (id->dma_mword >> 8))
-                       goto bug_dma_off;
-               if (id->dma_ultra & ((id->dma_ultra >> 8) & hwif->ultra_mask)) {
-                       if (((id->dma_ultra >> 11) & 0x1F) &&
-                           eighty_ninty_three(drive)) {
-                               if ((id->dma_ultra >> 15) & 1) {
-                                       printk(", UDMA(mode 7)");
-                               } else if ((id->dma_ultra >> 14) & 1) {
-                                       printk(", UDMA(133)");
-                               } else if ((id->dma_ultra >> 13) & 1) {
-                                       printk(", UDMA(100)");
-                               } else if ((id->dma_ultra >> 12) & 1) {
-                                       printk(", UDMA(66)");
-                               } else if ((id->dma_ultra >> 11) & 1) {
-                                       printk(", UDMA(44)");
-                               } else
-                                       goto mode_two;
-                       } else {
-               mode_two:
-                               if ((id->dma_ultra >> 10) & 1) {
-                                       printk(", UDMA(33)");
-                               } else if ((id->dma_ultra >> 9) & 1) {
-                                       printk(", UDMA(25)");
-                               } else if ((id->dma_ultra >> 8) & 1) {
-                                       printk(", UDMA(16)");
-                               }
-                       }
-               } else {
-                       printk(", (U)DMA");     /* Can be BIOS-enabled! */
-               }
+                       goto err_out;
        } else if (id->field_valid & 2) {
                if ((id->dma_mword >> 8) && (id->dma_1word >> 8))
-                       goto bug_dma_off;
-               printk(", DMA");
-       } else if (id->field_valid & 1) {
-               goto bug_dma_off;
+                       goto err_out;
        }
-       return;
-bug_dma_off:
-       printk(", BUG DMA OFF");
-       hwif->dma_off_quietly(drive);
-       return;
+       return 0;
+err_out:
+       printk(KERN_ERR "%s: bad DMA info in identify block\n", drive->name);
+       return 1;
 }
 
-EXPORT_SYMBOL(ide_dma_verbose);
-
 int ide_set_dma(ide_drive_t *drive)
 {
        ide_hwif_t *hwif = drive->hwif;

diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 5c32561..cef405d 100644 (file)
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -748,6 +748,9 @@ int ide_driveid_update(ide_drive_t *drive)
                drive->id->dma_1word = id->dma_1word;
                /* anything more ? */
                kfree(id);
+
+               if (drive->using_dma && ide_id_dma_bug(drive))
+                       ide_dma_off(drive);
        }
 
        return 1;

diff --git a/drivers/ide/ide-lib.c b/drivers/ide/ide-lib.c
index 1609b86..062d3bc 100644 (file)
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -29,41 +29,44 @@
  *     Add common non I/O op stuff here. Make sure it has proper
  *     kernel-doc function headers or your patch will be rejected
  */
- 
+
+static const char *udma_str[] =
+        { "UDMA/16", "UDMA/25",  "UDMA/33",  "UDMA/44",
+          "UDMA/66", "UDMA/100", "UDMA/133", "UDMA7" };
+static const char *mwdma_str[] =
+       { "MWDMA0", "MWDMA1", "MWDMA2" };
+static const char *swdma_str[] =
+       { "SWDMA0", "SWDMA1", "SWDMA2" };
+static const char *pio_str[] =
+       { "PIO0", "PIO1", "PIO2", "PIO3", "PIO4", "PIO5" };
 
 /**
  *     ide_xfer_verbose        -       return IDE mode names
- *     @xfer_rate: rate to name
+ *     @mode: transfer mode
  *
  *     Returns a constant string giving the name of the mode
  *     requested.
  */
 
-char *ide_xfer_verbose (u8 xfer_rate)
+const char *ide_xfer_verbose(u8 mode)
 {
-        switch(xfer_rate) {
-                case XFER_UDMA_7:      return("UDMA 7");
-                case XFER_UDMA_6:      return("UDMA 6");
-                case XFER_UDMA_5:      return("UDMA 5");
-                case XFER_UDMA_4:      return("UDMA 4");
-                case XFER_UDMA_3:      return("UDMA 3");
-                case XFER_UDMA_2:      return("UDMA 2");
-                case XFER_UDMA_1:      return("UDMA 1");
-                case XFER_UDMA_0:      return("UDMA 0");
-                case XFER_MW_DMA_2:    return("MW DMA 2");
-                case XFER_MW_DMA_1:    return("MW DMA 1");
-                case XFER_MW_DMA_0:    return("MW DMA 0");
-                case XFER_SW_DMA_2:    return("SW DMA 2");
-                case XFER_SW_DMA_1:    return("SW DMA 1");
-                case XFER_SW_DMA_0:    return("SW DMA 0");
-                case XFER_PIO_4:       return("PIO 4");
-                case XFER_PIO_3:       return("PIO 3");
-                case XFER_PIO_2:       return("PIO 2");
-                case XFER_PIO_1:       return("PIO 1");
-                case XFER_PIO_0:       return("PIO 0");
-                case XFER_PIO_SLOW:    return("PIO SLOW");
-                default:               return("XFER ERROR");
-        }
+       const char *s;
+       u8 i = mode & 0xf;
+
+       if (mode >= XFER_UDMA_0 && mode <= XFER_UDMA_7)
+               s = udma_str[i];
+       else if (mode >= XFER_MW_DMA_0 && mode <= XFER_MW_DMA_2)
+               s = mwdma_str[i];
+       else if (mode >= XFER_SW_DMA_0 && mode <= XFER_SW_DMA_2)
+               s = swdma_str[i];
+       else if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
+               s = pio_str[i & 0x7];
+       else if (mode == XFER_PIO_SLOW)
+               s = "PIO SLOW";
+       else
+               s = "XFER ERROR";
+
+       return s;
 }
 
 EXPORT_SYMBOL(ide_xfer_verbose);

diff --git a/include/linux/ide.h b/include/linux/ide.h
index dc75ccb..9a6a41e 100644 (file)
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ int ide_in_drive_list(struct hd_driveid *, const struct drive_list_entry *);
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 int __ide_dma_bad_drive(ide_drive_t *);
+int ide_id_dma_bug(ide_drive_t *);
 
 u8 ide_find_dma_mode(ide_drive_t *, u8);
 
@@ -1264,7 +1265,6 @@ static inline u8 ide_max_dma_mode(ide_drive_t *drive)
 }
 
 void ide_dma_off(ide_drive_t *);
-void ide_dma_verbose(ide_drive_t *);
 int ide_set_dma(ide_drive_t *);
 ide_startstop_t ide_dma_intr(ide_drive_t *);
 
@@ -1287,6 +1287,7 @@ extern void ide_dma_timeout(ide_drive_t *);
 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
 
 #else
+static inline int ide_id_dma_bug(ide_drive_t *drive) { return 0; }
 static inline u8 ide_find_dma_mode(ide_drive_t *drive, u8 speed) { return 0; }
 static inline u8 ide_max_dma_mode(ide_drive_t *drive) { return 0; }
 static inline void ide_dma_off(ide_drive_t *drive) { ; }
@@ -1333,8 +1334,7 @@ static inline void ide_set_hwifdata (ide_hwif_t * hwif, void *data)
        hwif->hwif_data = data;
 }
 
-/* ide-lib.c */
-extern char *ide_xfer_verbose(u8 xfer_rate);
+const char *ide_xfer_verbose(u8 mode);
 extern void ide_toggle_bounce(ide_drive_t *drive, int on);
 extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);