Commit 3ab7efe8e2cbcca2d401b43cfcc2fa9a7dac2299

Authored by Bartlomiej Zolnierkiewicz
1 parent e11b9035a4

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>

Showing 6 changed files with 53 additions and 85 deletions Side-by-side Diff

drivers/ide/ide-cd.c
... ... @@ -3049,12 +3049,7 @@
3049 3049 else
3050 3050 printk(" drive");
3051 3051  
3052   - printk(", %dkB Cache", be16_to_cpu(cap.buffer_size));
3053   -
3054   - if (drive->using_dma)
3055   - ide_dma_verbose(drive);
3056   -
3057   - printk("\n");
  3052 + printk(KERN_CONT ", %dkB Cache\n", be16_to_cpu(cap.buffer_size));
3058 3053  
3059 3054 return nslots;
3060 3055 }
drivers/ide/ide-disk.c
... ... @@ -961,11 +961,8 @@
961 961 if (id->buf_size)
962 962 printk (" w/%dKiB Cache", id->buf_size/2);
963 963  
964   - printk(", CHS=%d/%d/%d",
965   - drive->bios_cyl, drive->bios_head, drive->bios_sect);
966   - if (drive->using_dma)
967   - ide_dma_verbose(drive);
968   - printk("\n");
  964 + printk(KERN_CONT ", CHS=%d/%d/%d\n",
  965 + drive->bios_cyl, drive->bios_head, drive->bios_sect);
969 966  
970 967 /* write cache enabled? */
971 968 if ((id->csfo & 1) || (id->cfs_enable_1 & (1 << 5)))
drivers/ide/ide-dma.c
... ... @@ -753,10 +753,12 @@
753 753 mode = XFER_MW_DMA_1;
754 754 }
755 755  
756   - printk(KERN_DEBUG "%s: %s mode selected\n", drive->name,
  756 + mode = min(mode, req_mode);
  757 +
  758 + printk(KERN_INFO "%s: %s mode selected\n", drive->name,
757 759 mode ? ide_xfer_verbose(mode) : "no DMA");
758 760  
759   - return min(mode, req_mode);
  761 + return mode;
760 762 }
761 763  
762 764 EXPORT_SYMBOL_GPL(ide_find_dma_mode);
... ... @@ -772,6 +774,9 @@
772 774 if (__ide_dma_bad_drive(drive))
773 775 return 0;
774 776  
  777 + if (ide_id_dma_bug(drive))
  778 + return 0;
  779 +
775 780 if (drive->hwif->host_flags & IDE_HFLAG_TRUST_BIOS_FOR_DMA)
776 781 return config_drive_for_dma(drive);
777 782  
778 783  
779 784  
780 785  
781 786  
782 787  
... ... @@ -806,57 +811,22 @@
806 811 return vdma ? 0 : -1;
807 812 }
808 813  
809   -void ide_dma_verbose(ide_drive_t *drive)
  814 +int ide_id_dma_bug(ide_drive_t *drive)
810 815 {
811   - struct hd_driveid *id = drive->id;
812   - ide_hwif_t *hwif = HWIF(drive);
  816 + struct hd_driveid *id = drive->id;
813 817  
814 818 if (id->field_valid & 4) {
815 819 if ((id->dma_ultra >> 8) && (id->dma_mword >> 8))
816   - goto bug_dma_off;
817   - if (id->dma_ultra & ((id->dma_ultra >> 8) & hwif->ultra_mask)) {
818   - if (((id->dma_ultra >> 11) & 0x1F) &&
819   - eighty_ninty_three(drive)) {
820   - if ((id->dma_ultra >> 15) & 1) {
821   - printk(", UDMA(mode 7)");
822   - } else if ((id->dma_ultra >> 14) & 1) {
823   - printk(", UDMA(133)");
824   - } else if ((id->dma_ultra >> 13) & 1) {
825   - printk(", UDMA(100)");
826   - } else if ((id->dma_ultra >> 12) & 1) {
827   - printk(", UDMA(66)");
828   - } else if ((id->dma_ultra >> 11) & 1) {
829   - printk(", UDMA(44)");
830   - } else
831   - goto mode_two;
832   - } else {
833   - mode_two:
834   - if ((id->dma_ultra >> 10) & 1) {
835   - printk(", UDMA(33)");
836   - } else if ((id->dma_ultra >> 9) & 1) {
837   - printk(", UDMA(25)");
838   - } else if ((id->dma_ultra >> 8) & 1) {
839   - printk(", UDMA(16)");
840   - }
841   - }
842   - } else {
843   - printk(", (U)DMA"); /* Can be BIOS-enabled! */
844   - }
  820 + goto err_out;
845 821 } else if (id->field_valid & 2) {
846 822 if ((id->dma_mword >> 8) && (id->dma_1word >> 8))
847   - goto bug_dma_off;
848   - printk(", DMA");
849   - } else if (id->field_valid & 1) {
850   - goto bug_dma_off;
  823 + goto err_out;
851 824 }
852   - return;
853   -bug_dma_off:
854   - printk(", BUG DMA OFF");
855   - hwif->dma_off_quietly(drive);
856   - return;
  825 + return 0;
  826 +err_out:
  827 + printk(KERN_ERR "%s: bad DMA info in identify block\n", drive->name);
  828 + return 1;
857 829 }
858   -
859   -EXPORT_SYMBOL(ide_dma_verbose);
860 830  
861 831 int ide_set_dma(ide_drive_t *drive)
862 832 {
drivers/ide/ide-iops.c
... ... @@ -748,6 +748,9 @@
748 748 drive->id->dma_1word = id->dma_1word;
749 749 /* anything more ? */
750 750 kfree(id);
  751 +
  752 + if (drive->using_dma && ide_id_dma_bug(drive))
  753 + ide_dma_off(drive);
751 754 }
752 755  
753 756 return 1;
drivers/ide/ide-lib.c
... ... @@ -29,41 +29,44 @@
29 29 * Add common non I/O op stuff here. Make sure it has proper
30 30 * kernel-doc function headers or your patch will be rejected
31 31 */
32   -
33 32  
  33 +static const char *udma_str[] =
  34 + { "UDMA/16", "UDMA/25", "UDMA/33", "UDMA/44",
  35 + "UDMA/66", "UDMA/100", "UDMA/133", "UDMA7" };
  36 +static const char *mwdma_str[] =
  37 + { "MWDMA0", "MWDMA1", "MWDMA2" };
  38 +static const char *swdma_str[] =
  39 + { "SWDMA0", "SWDMA1", "SWDMA2" };
  40 +static const char *pio_str[] =
  41 + { "PIO0", "PIO1", "PIO2", "PIO3", "PIO4", "PIO5" };
  42 +
34 43 /**
35 44 * ide_xfer_verbose - return IDE mode names
36   - * @xfer_rate: rate to name
  45 + * @mode: transfer mode
37 46 *
38 47 * Returns a constant string giving the name of the mode
39 48 * requested.
40 49 */
41 50  
42   -char *ide_xfer_verbose (u8 xfer_rate)
  51 +const char *ide_xfer_verbose(u8 mode)
43 52 {
44   - switch(xfer_rate) {
45   - case XFER_UDMA_7: return("UDMA 7");
46   - case XFER_UDMA_6: return("UDMA 6");
47   - case XFER_UDMA_5: return("UDMA 5");
48   - case XFER_UDMA_4: return("UDMA 4");
49   - case XFER_UDMA_3: return("UDMA 3");
50   - case XFER_UDMA_2: return("UDMA 2");
51   - case XFER_UDMA_1: return("UDMA 1");
52   - case XFER_UDMA_0: return("UDMA 0");
53   - case XFER_MW_DMA_2: return("MW DMA 2");
54   - case XFER_MW_DMA_1: return("MW DMA 1");
55   - case XFER_MW_DMA_0: return("MW DMA 0");
56   - case XFER_SW_DMA_2: return("SW DMA 2");
57   - case XFER_SW_DMA_1: return("SW DMA 1");
58   - case XFER_SW_DMA_0: return("SW DMA 0");
59   - case XFER_PIO_4: return("PIO 4");
60   - case XFER_PIO_3: return("PIO 3");
61   - case XFER_PIO_2: return("PIO 2");
62   - case XFER_PIO_1: return("PIO 1");
63   - case XFER_PIO_0: return("PIO 0");
64   - case XFER_PIO_SLOW: return("PIO SLOW");
65   - default: return("XFER ERROR");
66   - }
  53 + const char *s;
  54 + u8 i = mode & 0xf;
  55 +
  56 + if (mode >= XFER_UDMA_0 && mode <= XFER_UDMA_7)
  57 + s = udma_str[i];
  58 + else if (mode >= XFER_MW_DMA_0 && mode <= XFER_MW_DMA_2)
  59 + s = mwdma_str[i];
  60 + else if (mode >= XFER_SW_DMA_0 && mode <= XFER_SW_DMA_2)
  61 + s = swdma_str[i];
  62 + else if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
  63 + s = pio_str[i & 0x7];
  64 + else if (mode == XFER_PIO_SLOW)
  65 + s = "PIO SLOW";
  66 + else
  67 + s = "XFER ERROR";
  68 +
  69 + return s;
67 70 }
68 71  
69 72 EXPORT_SYMBOL(ide_xfer_verbose);
... ... @@ -1255,6 +1255,7 @@
1255 1255  
1256 1256 #ifdef CONFIG_BLK_DEV_IDEDMA
1257 1257 int __ide_dma_bad_drive(ide_drive_t *);
  1258 +int ide_id_dma_bug(ide_drive_t *);
1258 1259  
1259 1260 u8 ide_find_dma_mode(ide_drive_t *, u8);
1260 1261  
... ... @@ -1264,7 +1265,6 @@
1264 1265 }
1265 1266  
1266 1267 void ide_dma_off(ide_drive_t *);
1267   -void ide_dma_verbose(ide_drive_t *);
1268 1268 int ide_set_dma(ide_drive_t *);
1269 1269 ide_startstop_t ide_dma_intr(ide_drive_t *);
1270 1270  
... ... @@ -1287,6 +1287,7 @@
1287 1287 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
1288 1288  
1289 1289 #else
  1290 +static inline int ide_id_dma_bug(ide_drive_t *drive) { return 0; }
1290 1291 static inline u8 ide_find_dma_mode(ide_drive_t *drive, u8 speed) { return 0; }
1291 1292 static inline u8 ide_max_dma_mode(ide_drive_t *drive) { return 0; }
1292 1293 static inline void ide_dma_off(ide_drive_t *drive) { ; }
... ... @@ -1333,8 +1334,7 @@
1333 1334 hwif->hwif_data = data;
1334 1335 }
1335 1336  
1336   -/* ide-lib.c */
1337   -extern char *ide_xfer_verbose(u8 xfer_rate);
  1337 +const char *ide_xfer_verbose(u8 mode);
1338 1338 extern void ide_toggle_bounce(ide_drive_t *drive, int on);
1339 1339 extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
1340 1340