ide: cable detection fixes (take 2)

Tejun's recent eighty_ninty_three() fix has inspired me to do more thorough
review of the cable detection code...

* print user-friendly warning about limiting the maximum transfer speed
  to UDMA33 (and the reason behind it) when 80-wire cable is not detected,
  also while at it cleanup eighty_ninty_three() a bit

* use eighty_ninty_three() in ide_ata66_check(), this actually fixes 3 bugs:
  - bit 14 (word 93 validity check) == 1 && bit 13 (80-wire cable test) == 1
    were used as 80-wire cable present test for CONFIG_IDEDMA_IVB=n case
    (please see FIXME comment in eighty_ninty_three() for more details)
  - CONFIG_IDEDMA_IVB=y/n cases were interchanged
  - check for SATA devices was missing

* remove private cable warnings from pdc_202xx{old,new} drivers now that core
  code provides this functionality (plus, in pdc202xx_new case the test could
  give false warnings for ATAPI devices because pdc202xx_new driver doesn't
  even support ATAPI DMA)

Cc: Tejun Heo <htejun@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index ed6128f6cd982b225c9a5df7c9391655cd933403..f0be5f665a0e0ae21af516609c0a82c556b4e0cd 100644 (file)
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -571,25 +571,40 @@ EXPORT_SYMBOL(ide_wait_stat);
  */
 u8 eighty_ninty_three (ide_drive_t *drive)
 {
-       if(HWIF(drive)->udma_four == 0)
-               return 0;
+       ide_hwif_t *hwif = drive->hwif;
+       struct hd_driveid *id = drive->id;
+
+       if (hwif->udma_four == 0)
+               goto no_80w;
 
        /* Check for SATA but only if we are ATA5 or higher */
-       if (drive->id->hw_config == 0 && (drive->id->major_rev_num & 0x7FE0))
+       if (id->hw_config == 0 && (id->major_rev_num & 0x7FE0))
                return 1;
-       if (!(drive->id->hw_config & 0x6000))
-               return 0;
-#ifndef CONFIG_IDEDMA_IVB
-       if(!(drive->id->hw_config & 0x4000))
-               return 0;
-#endif /* CONFIG_IDEDMA_IVB */
+
        /*
         * FIXME:
         * - change master/slave IDENTIFY order
         * - force bit13 (80c cable present) check
         *   (unless the slave device is pre-ATA3)
         */
-       return 1;
+#ifndef CONFIG_IDEDMA_IVB
+       if (id->hw_config & 0x4000)
+#else
+       if (id->hw_config & 0x6000)
+#endif
+               return 1;
+
+no_80w:
+       if (drive->udma33_warned == 1)
+               return 0;
+
+       printk(KERN_WARNING "%s: %s side 80-wire cable detection failed, "
+                           "limiting max speed to UDMA33\n",
+                           drive->name, hwif->udma_four ? "drive" : "host");
+
+       drive->udma33_warned = 1;
+
+       return 0;
 }
 
 int ide_ata66_check (ide_drive_t *drive, ide_task_t *args)
@@ -597,23 +612,13 @@ int ide_ata66_check (ide_drive_t *drive, ide_task_t *args)
        if ((args->tfRegister[IDE_COMMAND_OFFSET] == WIN_SETFEATURES) &&
            (args->tfRegister[IDE_SECTOR_OFFSET] > XFER_UDMA_2) &&
            (args->tfRegister[IDE_FEATURE_OFFSET] == SETFEATURES_XFER)) {
-#ifndef CONFIG_IDEDMA_IVB
-               if ((drive->id->hw_config & 0x6000) == 0) {
-#else /* !CONFIG_IDEDMA_IVB */
-               if (((drive->id->hw_config & 0x2000) == 0) ||
-                   ((drive->id->hw_config & 0x4000) == 0)) {
-#endif /* CONFIG_IDEDMA_IVB */
-                       printk("%s: Speed warnings UDMA 3/4/5 is not "
-                               "functional.\n", drive->name);
-                       return 1;
-               }
-               if (!HWIF(drive)->udma_four) {
-                       printk("%s: Speed warnings UDMA 3/4/5 is not "
-                               "functional.\n",
-                               HWIF(drive)->name);
+               if (eighty_ninty_three(drive) == 0) {
+                       printk(KERN_WARNING "%s: UDMA speeds >UDMA33 cannot "
+                                           "be set\n", drive->name);
                        return 1;
                }
        }
+
        return 0;
 }
 

diff --git a/drivers/ide/ide-lib.c b/drivers/ide/ide-lib.c
index 4557fc5a3ea381c4518b18f48142d39380f2d26d..3be3c69383f2e1ec256f0c19a77dfdcfe071e5b1 100644 (file)
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -88,8 +88,15 @@ u8 ide_rate_filter(ide_drive_t *drive, u8 speed)
        if (hwif->udma_filter)
                mask = hwif->udma_filter(drive);
 
-       if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
-               mask &= 0x07;
+       /*
+        * TODO: speed > XFER_UDMA_2 extra check is needed to avoid false
+        * cable warning from eighty_ninty_three(), moving ide_rate_filter()
+        * calls from ->speedproc to core code will make this hack go away
+        */
+       if (speed > XFER_UDMA_2) {
+               if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
+                       mask &= 0x07;
+       }
 
        if (mask)
                mode = fls(mask) - 1 + XFER_UDMA_0;

diff --git a/drivers/ide/pci/pdc202xx_new.c b/drivers/ide/pci/pdc202xx_new.c
index 772ca4007de4d5fc78bab5a2ad91bda7810b90ed..65b1e124edf773731cd21250b03a3780d8674bfb 100644 (file)
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -37,8 +37,6 @@
 #include <asm/pci-bridge.h>
 #endif
 
-#define PDC202_DEBUG_CABLE     0
-
 #undef DEBUG
 
 #ifdef DEBUG
@@ -234,17 +232,8 @@ static int config_chipset_for_dma(ide_drive_t *drive)
 {
        struct hd_driveid *id   = drive->id;
        ide_hwif_t *hwif        = HWIF(drive);
-       u8 ultra_66             = (id->dma_ultra & 0x0078) ? 1 : 0;
-       u8 cable                = pdcnew_cable_detect(hwif);
        u8 speed;
 
-       if (ultra_66 && cable) {
-               printk(KERN_WARNING "Warning: %s channel "
-                      "requires an 80-pin cable for operation.\n",
-                      hwif->channel ? "Secondary" : "Primary");
-               printk(KERN_WARNING "%s reduced to Ultra33 mode.\n", drive->name);
-       }
-
        if (id->capability & 4) {
                /*
                 * Set IORDY_EN & PREFETCH_EN (this seems to have
@@ -547,11 +536,6 @@ static void __devinit init_hwif_pdc202new(ide_hwif_t *hwif)
        if (!noautodma)
                hwif->autodma = 1;
        hwif->drives[0].autodma = hwif->drives[1].autodma = hwif->autodma;
-
-#if PDC202_DEBUG_CABLE
-       printk(KERN_DEBUG "%s: %s-pin cable\n",
-               hwif->name, hwif->udma_four ? "80" : "40");
-#endif /* PDC202_DEBUG_CABLE */
 }
 
 static int __devinit init_setup_pdcnew(struct pci_dev *dev, ide_pci_device_t *d)

diff --git a/drivers/ide/pci/pdc202xx_old.c b/drivers/ide/pci/pdc202xx_old.c
index 207a6191fac774d1cc7ee0b0c8c85203f09c099e..7146fe3f6ba7bdffd0bc9bafaea0afd2e8b8ab64 100644 (file)
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -46,7 +46,6 @@
 #include <asm/io.h>
 #include <asm/irq.h>
 
-#define PDC202_DEBUG_CABLE             0
 #define PDC202XX_DEBUG_DRIVE_INFO      0
 
 static const char *pdc_quirk_drives[] = {
@@ -238,20 +237,7 @@ static int config_chipset_for_dma (ide_drive_t *drive)
        u32 drive_conf          = 0;
        u8 drive_pci            = 0x60 + (drive->dn << 2);
        u8 test1 = 0, test2 = 0, speed = -1;
-       u8 AP = 0, cable = 0;
-
-       u8 ultra_66             = ((id->dma_ultra & 0x0010) ||
-                                  (id->dma_ultra & 0x0008)) ? 1 : 0;
-
-       if (dev->device != PCI_DEVICE_ID_PROMISE_20246)
-               cable = pdc202xx_old_cable_detect(hwif);
-       else
-               ultra_66 = 0;
-
-       if (ultra_66 && cable) {
-               printk(KERN_WARNING "Warning: %s channel requires an 80-pin cable for operation.\n", hwif->channel ? "Secondary":"Primary");
-               printk(KERN_WARNING "%s reduced to Ultra33 mode.\n", drive->name);
-       }
+       u8 AP = 0;
 
        if (dev->device != PCI_DEVICE_ID_PROMISE_20246)
                pdc_old_disable_66MHz_clock(drive->hwif);
@@ -477,10 +463,6 @@ static void __devinit init_hwif_pdc202xx(ide_hwif_t *hwif)
        if (!noautodma)
                hwif->autodma = 1;
        hwif->drives[0].autodma = hwif->drives[1].autodma = hwif->autodma;
-#if PDC202_DEBUG_CABLE
-       printk(KERN_DEBUG "%s: %s-pin cable\n",
-               hwif->name, hwif->udma_four ? "80" : "40");
-#endif /* PDC202_DEBUG_CABLE */        
 }
 
 static void __devinit init_dma_pdc202xx(ide_hwif_t *hwif, unsigned long dmabase)

diff --git a/include/linux/ide.h b/include/linux/ide.h
index 477b8c6be727cb15032d1eb2d98e299bd2029733..ca924b295c2e62580fced8da66aab2f1e9a1b22f 100644 (file)
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -605,6 +605,7 @@ typedef struct ide_drive_s {
        unsigned scsi           : 1;    /* 0=default, 1=ide-scsi emulation */
        unsigned sleeping       : 1;    /* 1=sleeping & sleep field valid */
        unsigned post_reset     : 1;
+       unsigned udma33_warned  : 1;
 
        u8      addressing;     /* 0=28-bit, 1=48-bit, 2=48-bit doing 28-bit */
         u8     quirk_list;     /* considered quirky, set for a specific host */