* [PATCH][pata] ide: cable detection fixes
@ 2007-02-11 22:24 Bartlomiej Zolnierkiewicz
2007-02-11 23:08 ` Alan
0 siblings, 1 reply; 3+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-11 22:24 UTC (permalink / raw)
To: linux-ide; +Cc: Tejun Heo
[PATCH] ide: cable detection fixes
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 13 (word 93 validity check) == 1 and bit 12 (80-wire cable test) == 0
configuration was incorrectly treated as a 80-wire cable present
[ for CONFIG_IDEDMA_IVB=n ]
- CONFIG_IDEDMA_IVB=y/n cases were interchanged
- check for SATA devices were 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)
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-iops.c | 51 +++++++++++++++++++++--------------------
drivers/ide/ide-lib.c | 11 +++++++-
drivers/ide/pci/pdc202xx_new.c | 16 ------------
drivers/ide/pci/pdc202xx_old.c | 20 ----------------
include/linux/ide.h | 1
5 files changed, 38 insertions(+), 61 deletions(-)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -571,21 +571,34 @@ 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 */
- if (!(drive->id->hw_config & 0x2000))
+ if ((id->hw_config & 0x6000) == 0x6000)
+#else
+ if (id->hw_config & 0x2000)
+#endif
+ return 1;
+
+no_80w:
+ if (drive->udma33_warned == 1)
return 0;
- return 1;
+
+ 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)
@@ -593,23 +606,13 @@ int ide_ata66_check (ide_drive_t *drive,
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;
}
Index: b/drivers/ide/ide-lib.c
===================================================================
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -88,8 +88,15 @@ u8 ide_rate_filter(ide_drive_t *drive, u
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;
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- 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_dr
{
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 (drive->media != ide_disk)
return 0;
@@ -548,11 +537,6 @@ static void __devinit init_hwif_pdc202ne
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)
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- 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_d
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
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)
Index: b/include/linux/ide.h
===================================================================
--- 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 */
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH][pata] ide: cable detection fixes
2007-02-11 22:24 [PATCH][pata] ide: cable detection fixes Bartlomiej Zolnierkiewicz
@ 2007-02-11 23:08 ` Alan
2007-02-12 0:37 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 3+ messages in thread
From: Alan @ 2007-02-11 23:08 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, Tejun Heo
> * use eighty_ninty_three() in ide_ata66_check(), this actually fixes 3 bugs:
> - bit 13 (word 93 validity check) == 1 and bit 12 (80-wire cable test) == 0
> configuration was incorrectly treated as a 80-wire cable present
I think you mean bit 14 and bit 13. And for validity you must check bits
14/15 together as 0x01. Code seems to have the bits right not the
comment, but doesn't check bit 15.
You also change the IVB behaviour from assuming drives would set either
the correct bit and/or the valid bit, to drives must always set the
correct bit but not neccessarily valid. It would be nice to know if that
is intentional as the original IVB hack is murky early ATA standard
history and I am not sure what the drive id error in question actually
was ?
Alan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][pata] ide: cable detection fixes
2007-02-11 23:08 ` Alan
@ 2007-02-12 0:37 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 3+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-12 0:37 UTC (permalink / raw)
To: Alan; +Cc: linux-ide, Tejun Heo
On Monday 12 February 2007 00:08, Alan wrote:
> > * use eighty_ninty_three() in ide_ata66_check(), this actually fixes 3 bugs:
> > - bit 13 (word 93 validity check) == 1 and bit 12 (80-wire cable test) == 0
> > configuration was incorrectly treated as a 80-wire cable present
>
> I think you mean bit 14 and bit 13. And for validity you must check bits
Yes, thanks for catching this, fixed now.
[ "fix drive side 80c cable check" patch description also needed fixing ]
> 14/15 together as 0x01. Code seems to have the bits right not the
> comment, but doesn't check bit 15.
>
> You also change the IVB behaviour from assuming drives would set either
> the correct bit and/or the valid bit, to drives must always set the
> correct bit but not neccessarily valid. It would be nice to know if that
> is intentional as the original IVB hack is murky early ATA standard
For ide_ata66_check() IVB=y/n cases were interchanged when compared to
eighty_ninty_three():
* for IVB=n (the default setting) the old code required bit 14 or bit 13
to be set which is what eighty_ninty_three() required for IVB=y before
Tejun's patch
* for IVB=y (IVB hack on) the old code required both bits 14 and 13 to be set,
which is what eighty_ninty_three() requires for IVB=n after Tejun's patch
(before Tejun's patch only bit 14 was required to be set)
The old code just couldn't work for IVB, this probably has gone unnoticed
for years because ide_ata66_check() is only used for user space originated
transfer mode change requests.
> history and I am not sure what the drive id error in question actually
> was ?
I can't be sure what was the original intent of IVB hack but I have seen
a few cases when IVB hack was needed for Quantum disks which had only bit 13
set and bits 15/14 cleared.
[ it is also worth to note here that ATA4 doesn't define word 93 and
that the pre-ATA5 proposal for the cable detection came from Quantum ]
Bart
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-02-12 0:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-11 22:24 [PATCH][pata] ide: cable detection fixes Bartlomiej Zolnierkiewicz
2007-02-11 23:08 ` Alan
2007-02-12 0:37 ` Bartlomiej Zolnierkiewicz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.