LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void
@ 2007-02-03 13:53 Bartlomiej Zolnierkiewicz
  2007-02-03 13:53 ` [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void Bartlomiej Zolnierkiewicz
                   ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03 13:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, Bartlomiej Zolnierkiewicz, linux-kernel

[PATCH] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void

* since ide_hwif_t.ide_dma_{host_off,off_quietly} always return '0'
  make these functions void and while at it drop "ide_" prefix
* fix comment for __ide_dma_off_quietly()
* make __ide_dma_{host_off,off_quietly,off}() void and drop "__" prefix

v2:
* while at it rename atiixp_ide_dma_host_off() to atiixp_dma_host_off(),
  sgiioc4_ide_dma_{host_off,off_quietly}() to sgiioc4_dma_{host_off,off_quietly}()
  and sl82c105_ide_dma_off_quietly() to sl82c105_dma_off_quietly()
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/arm/icside.c      |   10 ++++------
 drivers/ide/cris/ide-cris.c   |   14 ++++++--------
 drivers/ide/ide-cd.c          |    6 +++---
 drivers/ide/ide-dma.c         |   39 ++++++++++++++++++---------------------
 drivers/ide/ide-floppy.c      |    8 ++++----
 drivers/ide/ide-io.c          |    2 +-
 drivers/ide/ide-iops.c        |    8 ++++----
 drivers/ide/ide-probe.c       |    2 +-
 drivers/ide/ide-tape.c        |    4 ++--
 drivers/ide/ide.c             |   10 ++++------
 drivers/ide/mips/au1xxx-ide.c |   11 ++++-------
 drivers/ide/pci/atiixp.c      |    6 +++---
 drivers/ide/pci/cs5530.c      |    2 +-
 drivers/ide/pci/it821x.c      |    2 +-
 drivers/ide/pci/sc1200.c      |    6 +++---
 drivers/ide/pci/sgiioc4.c     |   14 +++++---------
 drivers/ide/pci/sl82c105.c    |   13 +++++--------
 drivers/ide/ppc/pmac.c        |    8 +++-----
 include/linux/ide.h           |   12 ++++++------
 19 files changed, 78 insertions(+), 99 deletions(-)

Index: b/drivers/ide/arm/icside.c
===================================================================
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -307,15 +307,13 @@ static int icside_set_speed(ide_drive_t 
 	return on;
 }
 
-static int icside_dma_host_off(ide_drive_t *drive)
+static void icside_dma_host_off(ide_drive_t *drive)
 {
-	return 0;
 }
 
-static int icside_dma_off_quietly(ide_drive_t *drive)
+static void icside_dma_off_quietly(ide_drive_t *drive)
 {
 	drive->using_dma = 0;
-	return icside_dma_host_off(drive);
 }
 
 static int icside_dma_host_on(ide_drive_t *drive)
@@ -494,8 +492,8 @@ static void icside_dma_init(ide_hwif_t *
 	hwif->autodma		= autodma;
 
 	hwif->ide_dma_check	= icside_dma_check;
-	hwif->ide_dma_host_off	= icside_dma_host_off;
-	hwif->ide_dma_off_quietly = icside_dma_off_quietly;
+	hwif->dma_host_off	= icside_dma_host_off;
+	hwif->dma_off_quietly	= icside_dma_off_quietly;
 	hwif->ide_dma_host_on	= icside_dma_host_on;
 	hwif->ide_dma_on	= icside_dma_on;
 	hwif->dma_setup		= icside_dma_setup;
Index: b/drivers/ide/cris/ide-cris.c
===================================================================
--- a/drivers/ide/cris/ide-cris.c
+++ b/drivers/ide/cris/ide-cris.c
@@ -682,9 +682,12 @@ static void cris_ide_input_data (ide_dri
 static void cris_ide_output_data (ide_drive_t *drive, void *, unsigned int);
 static void cris_atapi_input_bytes(ide_drive_t *drive, void *, unsigned int);
 static void cris_atapi_output_bytes(ide_drive_t *drive, void *, unsigned int);
-static int cris_dma_off (ide_drive_t *drive);
 static int cris_dma_on (ide_drive_t *drive);
 
+static void cris_dma_off(ide_drive_t *drive)
+{
+}
+
 static void tune_cris_ide(ide_drive_t *drive, u8 pio)
 {
 	int setup, strobe, hold;
@@ -814,9 +817,9 @@ init_e100_ide (void)
 		hwif->OUTBSYNC = &cris_ide_outbsync;
 		hwif->INB = &cris_ide_inb;
 		hwif->INW = &cris_ide_inw;
-		hwif->ide_dma_host_off = &cris_dma_off;
+		hwif->dma_host_off = &cris_dma_off;
 		hwif->ide_dma_host_on = &cris_dma_on;
-		hwif->ide_dma_off_quietly = &cris_dma_off;
+		hwif->dma_off_quietly = &cris_dma_off;
 		hwif->udma_four = 0;
 		hwif->ultra_mask = cris_ultra_mask;
 		hwif->mwdma_mask = 0x07; /* Multiword DMA 0-2 */
@@ -838,11 +841,6 @@ init_e100_ide (void)
 	cris_ide_set_speed(TYPE_UDMA, ATA_UDMA2_CYC, ATA_UDMA2_DVS, 0);
 }
 
-static int cris_dma_off (ide_drive_t *drive)
-{
-	return 0;
-}
-
 static int cris_dma_on (ide_drive_t *drive)
 {
 	return 0;
Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1104,7 +1104,7 @@ static ide_startstop_t cdrom_read_intr (
 	if (dma) {
 		info->dma = 0;
 		if ((dma_error = HWIF(drive)->ide_dma_end(drive)))
-			__ide_dma_off(drive);
+			ide_dma_off(drive);
 	}
 
 	if (cdrom_decode_status(drive, 0, &stat))
@@ -1700,7 +1700,7 @@ static ide_startstop_t cdrom_newpc_intr(
 	if (dma) {
 		if (dma_error) {
 			printk(KERN_ERR "ide-cd: dma error\n");
-			__ide_dma_off(drive);
+			ide_dma_off(drive);
 			return ide_error(drive, "dma error", stat);
 		}
 
@@ -1826,7 +1826,7 @@ static ide_startstop_t cdrom_write_intr(
 		info->dma = 0;
 		if ((dma_error = HWIF(drive)->ide_dma_end(drive))) {
 			printk(KERN_ERR "ide-cd: write dma error\n");
-			__ide_dma_off(drive);
+			ide_dma_off(drive);
 		}
 	}
 
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -414,61 +414,57 @@ static int dma_timer_expiry (ide_drive_t
 }
 
 /**
- *	__ide_dma_host_off	-	Generic DMA kill
+ *	ide_dma_host_off	-	Generic DMA kill
  *	@drive: drive to control
  *
  *	Perform the generic IDE controller DMA off operation. This
  *	works for most IDE bus mastering controllers
  */
 
-int __ide_dma_host_off (ide_drive_t *drive)
+void ide_dma_host_off(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	u8 unit			= (drive->select.b.unit & 0x01);
 	u8 dma_stat		= hwif->INB(hwif->dma_status);
 
 	hwif->OUTB((dma_stat & ~(1<<(5+unit))), hwif->dma_status);
-	return 0;
 }
 
-EXPORT_SYMBOL(__ide_dma_host_off);
+EXPORT_SYMBOL(ide_dma_host_off);
 
 /**
- *	__ide_dma_host_off_quietly	-	Generic DMA kill
+ *	ide_dma_off_quietly	-	Generic DMA kill
  *	@drive: drive to control
  *
  *	Turn off the current DMA on this IDE controller. 
  */
 
-int __ide_dma_off_quietly (ide_drive_t *drive)
+void ide_dma_off_quietly(ide_drive_t *drive)
 {
 	drive->using_dma = 0;
 	ide_toggle_bounce(drive, 0);
 
-	if (HWIF(drive)->ide_dma_host_off(drive))
-		return 1;
-
-	return 0;
+	drive->hwif->dma_host_off(drive);
 }
 
-EXPORT_SYMBOL(__ide_dma_off_quietly);
+EXPORT_SYMBOL(ide_dma_off_quietly);
 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
 
 /**
- *	__ide_dma_off	-	disable DMA on a device
+ *	ide_dma_off	-	disable DMA on a device
  *	@drive: drive to disable DMA on
  *
  *	Disable IDE DMA for a device on this IDE controller.
  *	Inform the user that DMA has been disabled.
  */
 
-int __ide_dma_off (ide_drive_t *drive)
+void ide_dma_off(ide_drive_t *drive)
 {
 	printk(KERN_INFO "%s: DMA disabled\n", drive->name);
-	return HWIF(drive)->ide_dma_off_quietly(drive);
+	drive->hwif->dma_off_quietly(drive);
 }
 
-EXPORT_SYMBOL(__ide_dma_off);
+EXPORT_SYMBOL(ide_dma_off);
 
 #ifdef CONFIG_BLK_DEV_IDEDMA_PCI
 /**
@@ -758,7 +754,7 @@ void ide_dma_verbose(ide_drive_t *drive)
 	return;
 bug_dma_off:
 	printk(", BUG DMA OFF");
-	hwif->ide_dma_off_quietly(drive);
+	hwif->dma_off_quietly(drive);
 	return;
 }
 
@@ -773,7 +769,8 @@ int ide_set_dma(ide_drive_t *drive)
 
 	switch(rc) {
 	case -1: /* DMA needs to be disabled */
-		return hwif->ide_dma_off_quietly(drive);
+		hwif->dma_off_quietly(drive);
+		return 0;
 	case  0: /* DMA needs to be enabled */
 		return hwif->ide_dma_on(drive);
 	case  1: /* DMA setting cannot be changed */
@@ -937,10 +934,10 @@ void ide_setup_dma (ide_hwif_t *hwif, un
 	if (!(hwif->dma_prdtable))
 		hwif->dma_prdtable	= (hwif->dma_base + 4);
 
-	if (!hwif->ide_dma_off_quietly)
-		hwif->ide_dma_off_quietly = &__ide_dma_off_quietly;
-	if (!hwif->ide_dma_host_off)
-		hwif->ide_dma_host_off = &__ide_dma_host_off;
+	if (!hwif->dma_off_quietly)
+		hwif->dma_off_quietly = &ide_dma_off_quietly;
+	if (!hwif->dma_host_off)
+		hwif->dma_host_off = &ide_dma_host_off;
 	if (!hwif->ide_dma_on)
 		hwif->ide_dma_on = &__ide_dma_on;
 	if (!hwif->ide_dma_host_on)
Index: b/drivers/ide/ide-floppy.c
===================================================================
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -867,7 +867,7 @@ static ide_startstop_t idefloppy_pc_intr
 	if (test_and_clear_bit(PC_DMA_IN_PROGRESS, &pc->flags)) {
 		printk(KERN_ERR "ide-floppy: The floppy wants to issue "
 			"more interrupts in DMA mode\n");
-		(void)__ide_dma_off(drive);
+		ide_dma_off(drive);
 		return ide_do_reset(drive);
 	}
 
@@ -1097,9 +1097,9 @@ static ide_startstop_t idefloppy_issue_p
 	pc->current_position = pc->buffer;
 	bcount.all = min(pc->request_transfer, 63 * 1024);
 
-	if (test_and_clear_bit(PC_DMA_ERROR, &pc->flags)) {
-		(void)__ide_dma_off(drive);
-	}
+	if (test_and_clear_bit(PC_DMA_ERROR, &pc->flags))
+		ide_dma_off(drive);
+
 	feature.all = 0;
 
 	if (test_bit(PC_DMA_RECOMMENDED, &pc->flags) && drive->using_dma)
Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -1351,7 +1351,7 @@ static ide_startstop_t ide_dma_timeout_r
 	 */
 	drive->retry_pio++;
 	drive->state = DMA_PIO_RETRY;
-	(void) hwif->ide_dma_off_quietly(drive);
+	hwif->dma_off_quietly(drive);
 
 	/*
 	 * un-busy drive etc (hwgroup->busy is cleared on return) and
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -753,7 +753,7 @@ int ide_config_drive_speed (ide_drive_t 
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 	if (hwif->ide_dma_check)	 /* check if host supports DMA */
-		hwif->ide_dma_host_off(drive);
+		hwif->dma_host_off(drive);
 #endif
 
 	/*
@@ -832,7 +832,7 @@ int ide_config_drive_speed (ide_drive_t 
 	if (speed >= XFER_SW_DMA_0)
 		hwif->ide_dma_host_on(drive);
 	else if (hwif->ide_dma_check)	/* check if host supports DMA */
-		hwif->ide_dma_off_quietly(drive);
+		hwif->dma_off_quietly(drive);
 #endif
 
 	switch(speed) {
@@ -1042,12 +1042,12 @@ static void check_dma_crc(ide_drive_t *d
 {
 #ifdef CONFIG_BLK_DEV_IDEDMA
 	if (drive->crc_count) {
-		(void) HWIF(drive)->ide_dma_off_quietly(drive);
+		drive->hwif->dma_off_quietly(drive);
 		ide_set_xfer_rate(drive, ide_auto_reduce_xfer(drive));
 		if (drive->current_speed >= XFER_SW_DMA_0)
 			(void) HWIF(drive)->ide_dma_on(drive);
 	} else
-		(void)__ide_dma_off(drive);
+		ide_dma_off(drive);
 #endif
 }
 
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -853,7 +853,7 @@ static void probe_hwif(ide_hwif_t *hwif)
 				 * things, if not checked and cleared.
 				 *   PARANOIA!!!
 				 */
-				hwif->ide_dma_off_quietly(drive);
+				hwif->dma_off_quietly(drive);
 #ifdef CONFIG_IDEDMA_ONLYDISK
 				if (drive->media == ide_disk)
 #endif
Index: b/drivers/ide/ide-tape.c
===================================================================
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1970,7 +1970,7 @@ static ide_startstop_t idetape_pc_intr (
 		printk(KERN_ERR "ide-tape: The tape wants to issue more "
 				"interrupts in DMA mode\n");
 		printk(KERN_ERR "ide-tape: DMA disabled, reverting to PIO\n");
-		(void)__ide_dma_off(drive);
+		ide_dma_off(drive);
 		return ide_do_reset(drive);
 	}
 	/* Get the number of bytes to transfer on this interrupt. */
@@ -2176,7 +2176,7 @@ static ide_startstop_t idetape_issue_pac
 	if (test_and_clear_bit(PC_DMA_ERROR, &pc->flags)) {
 		printk(KERN_WARNING "ide-tape: DMA disabled, "
 				"reverting to PIO\n");
-		(void)__ide_dma_off(drive);
+		ide_dma_off(drive);
 	}
 	if (test_bit(PC_DMA_RECOMMENDED, &pc->flags) && drive->using_dma)
 		dma_ok = !hwif->dma_setup(drive);
Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -506,11 +506,11 @@ static void ide_hwif_restore(ide_hwif_t 
 	hwif->ide_dma_end		= tmp_hwif->ide_dma_end;
 	hwif->ide_dma_check		= tmp_hwif->ide_dma_check;
 	hwif->ide_dma_on		= tmp_hwif->ide_dma_on;
-	hwif->ide_dma_off_quietly	= tmp_hwif->ide_dma_off_quietly;
+	hwif->dma_off_quietly		= tmp_hwif->dma_off_quietly;
 	hwif->ide_dma_test_irq		= tmp_hwif->ide_dma_test_irq;
 	hwif->ide_dma_clear_irq		= tmp_hwif->ide_dma_clear_irq;
 	hwif->ide_dma_host_on		= tmp_hwif->ide_dma_host_on;
-	hwif->ide_dma_host_off		= tmp_hwif->ide_dma_host_off;
+	hwif->dma_host_off		= tmp_hwif->dma_host_off;
 	hwif->ide_dma_lostirq		= tmp_hwif->ide_dma_lostirq;
 	hwif->ide_dma_timeout		= tmp_hwif->ide_dma_timeout;
 
@@ -1138,10 +1138,8 @@ static int set_using_dma (ide_drive_t *d
 		if (ide_set_dma(drive))
 			return -EIO;
 		if (HWIF(drive)->ide_dma_on(drive)) return -EIO;
-	} else {
-		if (__ide_dma_off(drive))
-			return -EIO;
-	}
+	} else
+		ide_dma_off(drive);
 	return 0;
 #else
 	return -EPERM;
Index: b/drivers/ide/mips/au1xxx-ide.c
===================================================================
--- a/drivers/ide/mips/au1xxx-ide.c
+++ b/drivers/ide/mips/au1xxx-ide.c
@@ -449,16 +449,13 @@ static int auide_dma_on(ide_drive_t *dri
 	return auide_dma_host_on(drive);
 }
 
-
-static int auide_dma_host_off(ide_drive_t *drive)
+static void auide_dma_host_off(ide_drive_t *drive)
 {
-	return 0;
 }
 
-static int auide_dma_off_quietly(ide_drive_t *drive)
+static void auide_dma_off_quietly(ide_drive_t *drive)
 {
 	drive->using_dma = 0;
-	return auide_dma_host_off(drive);
 }
 
 static int auide_dma_lostirq(ide_drive_t *drive)
@@ -724,7 +721,7 @@ static int au_ide_probe(struct device *d
 	hwif->speedproc                 = &auide_tune_chipset;
 
 #ifdef CONFIG_BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA
-	hwif->ide_dma_off_quietly       = &auide_dma_off_quietly;
+	hwif->dma_off_quietly		= &auide_dma_off_quietly;
 	hwif->ide_dma_timeout           = &auide_dma_timeout;
 
 	hwif->ide_dma_check             = &auide_dma_check;
@@ -733,7 +730,7 @@ static int au_ide_probe(struct device *d
 	hwif->ide_dma_end               = &auide_dma_end;
 	hwif->dma_setup                 = &auide_dma_setup;
 	hwif->ide_dma_test_irq          = &auide_dma_test_irq;
-	hwif->ide_dma_host_off          = &auide_dma_host_off;
+	hwif->dma_host_off		= &auide_dma_host_off;
 	hwif->ide_dma_host_on           = &auide_dma_host_on;
 	hwif->ide_dma_lostirq           = &auide_dma_lostirq;
 	hwif->ide_dma_on                = &auide_dma_on;
Index: b/drivers/ide/pci/atiixp.c
===================================================================
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -121,7 +121,7 @@ static int atiixp_ide_dma_host_on(ide_dr
 	return __ide_dma_host_on(drive);
 }
 
-static int atiixp_ide_dma_host_off(ide_drive_t *drive)
+static void atiixp_dma_host_off(ide_drive_t *drive)
 {
 	struct pci_dev *dev = drive->hwif->pci_dev;
 	unsigned long flags;
@@ -135,7 +135,7 @@ static int atiixp_ide_dma_host_off(ide_d
 
 	spin_unlock_irqrestore(&atiixp_lock, flags);
 
-	return __ide_dma_host_off(drive);
+	ide_dma_host_off(drive);
 }
 
 /**
@@ -306,7 +306,7 @@ static void __devinit init_hwif_atiixp(i
 		hwif->udma_four = 0;
 
 	hwif->ide_dma_host_on = &atiixp_ide_dma_host_on;
-	hwif->ide_dma_host_off = &atiixp_ide_dma_host_off;
+	hwif->dma_host_off = &atiixp_dma_host_off;
 	hwif->ide_dma_check = &atiixp_dma_check;
 	if (!noautodma)
 		hwif->autodma = 1;
Index: b/drivers/ide/pci/cs5530.c
===================================================================
--- a/drivers/ide/pci/cs5530.c
+++ b/drivers/ide/pci/cs5530.c
@@ -109,7 +109,7 @@ static int cs5530_config_dma (ide_drive_
 	/*
 	 * Default to DMA-off in case we run into trouble here.
 	 */
-	hwif->ide_dma_off_quietly(drive);
+	hwif->dma_off_quietly(drive);
 
 	/*
 	 * The CS5530 specifies that two drives sharing a cable cannot
Index: b/drivers/ide/pci/it821x.c
===================================================================
--- a/drivers/ide/pci/it821x.c
+++ b/drivers/ide/pci/it821x.c
@@ -606,7 +606,7 @@ static void __devinit it821x_fixups(ide_
 				printk(".\n");
 			/* Now the core code will have wrongly decided no DMA
 			   so we need to fix this */
-			hwif->ide_dma_off_quietly(drive);
+			hwif->dma_off_quietly(drive);
 #ifdef CONFIG_IDEDMA_ONLYDISK
 			if (drive->media == ide_disk)
 #endif
Index: b/drivers/ide/pci/sc1200.c
===================================================================
--- a/drivers/ide/pci/sc1200.c
+++ b/drivers/ide/pci/sc1200.c
@@ -161,7 +161,7 @@ static int sc1200_config_dma2 (ide_drive
 	/*
 	 * Default to DMA-off in case we run into trouble here.
 	 */
-	hwif->ide_dma_off_quietly(drive);			/* turn off DMA while we fiddle */
+	hwif->dma_off_quietly(drive);	/* turn off DMA while we fiddle */
 	outb(inb(hwif->dma_base+2)&~(unit?0x40:0x20), hwif->dma_base+2); /* clear DMA_capable bit */
 
 	/*
@@ -439,10 +439,10 @@ static int sc1200_resume (struct pci_dev
 			ide_drive_t *drive = &(hwif->drives[d]);
 			if (drive->present && !__ide_dma_bad_drive(drive)) {
 				int was_using_dma = drive->using_dma;
-				hwif->ide_dma_off_quietly(drive);
+				hwif->dma_off_quietly(drive);
 				sc1200_config_dma(drive);
 				if (!was_using_dma && drive->using_dma) {
-					hwif->ide_dma_off_quietly(drive);
+					hwif->dma_off_quietly(drive);
 				}
 			}
 		}
Index: b/drivers/ide/pci/sgiioc4.c
===================================================================
--- a/drivers/ide/pci/sgiioc4.c
+++ b/drivers/ide/pci/sgiioc4.c
@@ -282,12 +282,11 @@ sgiioc4_ide_dma_on(ide_drive_t * drive)
 	return HWIF(drive)->ide_dma_host_on(drive);
 }
 
-static int
-sgiioc4_ide_dma_off_quietly(ide_drive_t * drive)
+static void sgiioc4_dma_off_quietly(ide_drive_t *drive)
 {
 	drive->using_dma = 0;
 
-	return HWIF(drive)->ide_dma_host_off(drive);
+	drive->hwif->dma_host_off(drive);
 }
 
 static int sgiioc4_ide_dma_check(ide_drive_t *drive)
@@ -317,12 +316,9 @@ sgiioc4_ide_dma_host_on(ide_drive_t * dr
 	return 1;
 }
 
-static int
-sgiioc4_ide_dma_host_off(ide_drive_t * drive)
+static void sgiioc4_dma_host_off(ide_drive_t * drive)
 {
 	sgiioc4_clearirq(drive);
-
-	return 0;
 }
 
 static int
@@ -612,10 +608,10 @@ ide_init_sgiioc4(ide_hwif_t * hwif)
 	hwif->ide_dma_end = &sgiioc4_ide_dma_end;
 	hwif->ide_dma_check = &sgiioc4_ide_dma_check;
 	hwif->ide_dma_on = &sgiioc4_ide_dma_on;
-	hwif->ide_dma_off_quietly = &sgiioc4_ide_dma_off_quietly;
+	hwif->dma_off_quietly = &sgiioc4_dma_off_quietly;
 	hwif->ide_dma_test_irq = &sgiioc4_ide_dma_test_irq;
 	hwif->ide_dma_host_on = &sgiioc4_ide_dma_host_on;
-	hwif->ide_dma_host_off = &sgiioc4_ide_dma_host_off;
+	hwif->dma_host_off = &sgiioc4_dma_host_off;
 	hwif->ide_dma_lostirq = &sgiioc4_ide_dma_lostirq;
 	hwif->ide_dma_timeout = &__ide_dma_timeout;
 
Index: b/drivers/ide/pci/sl82c105.c
===================================================================
--- a/drivers/ide/pci/sl82c105.c
+++ b/drivers/ide/pci/sl82c105.c
@@ -265,19 +265,16 @@ static int sl82c105_ide_dma_on (ide_driv
 	return __ide_dma_on(drive);
 }
 
-static int sl82c105_ide_dma_off_quietly (ide_drive_t *drive)
+static void sl82c105_dma_off_quietly(ide_drive_t *drive)
 {
 	u8 speed = XFER_PIO_0;
-	int rc;
-	
-	DBG(("sl82c105_ide_dma_off_quietly(drive:%s)\n", drive->name));
 
-	rc = __ide_dma_off_quietly(drive);
+	DBG(("sl82c105_dma_off_quietly(drive:%s)\n", drive->name));
+
+	ide_dma_off_quietly(drive);
 	if (drive->pio_speed)
 		speed = drive->pio_speed - XFER_PIO_0;
 	config_for_pio(drive, speed, 0, 1);
-
-	return rc;
 }
 
 /*
@@ -440,7 +437,7 @@ static void __devinit init_hwif_sl82c105
 
 		hwif->ide_dma_check = &sl82c105_check_drive;
 		hwif->ide_dma_on = &sl82c105_ide_dma_on;
-		hwif->ide_dma_off_quietly = &sl82c105_ide_dma_off_quietly;
+		hwif->dma_off_quietly = &sl82c105_dma_off_quietly;
 		hwif->ide_dma_lostirq = &sl82c105_ide_dma_lost_irq;
 		hwif->dma_start = &sl82c105_ide_dma_start;
 		hwif->ide_dma_timeout = &sl82c105_ide_dma_timeout;
Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -1980,10 +1980,8 @@ pmac_ide_dma_test_irq (ide_drive_t *driv
 	return 1;
 }
 
-static int
-pmac_ide_dma_host_off (ide_drive_t *drive)
+static void pmac_ide_dma_host_off(ide_drive_t *drive)
 {
-	return 0;
 }
 
 static int
@@ -2035,7 +2033,7 @@ pmac_ide_setup_dma(pmac_ide_hwif_t *pmif
 		return;
 	}
 
-	hwif->ide_dma_off_quietly = &__ide_dma_off_quietly;
+	hwif->dma_off_quietly = &ide_dma_off_quietly;
 	hwif->ide_dma_on = &__ide_dma_on;
 	hwif->ide_dma_check = &pmac_ide_dma_check;
 	hwif->dma_setup = &pmac_ide_dma_setup;
@@ -2043,7 +2041,7 @@ pmac_ide_setup_dma(pmac_ide_hwif_t *pmif
 	hwif->dma_start = &pmac_ide_dma_start;
 	hwif->ide_dma_end = &pmac_ide_dma_end;
 	hwif->ide_dma_test_irq = &pmac_ide_dma_test_irq;
-	hwif->ide_dma_host_off = &pmac_ide_dma_host_off;
+	hwif->dma_host_off = &pmac_ide_dma_host_off;
 	hwif->ide_dma_host_on = &pmac_ide_dma_host_on;
 	hwif->ide_dma_timeout = &__ide_dma_timeout;
 	hwif->ide_dma_lostirq = &pmac_ide_dma_lostirq;
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -735,11 +735,11 @@ typedef struct hwif_s {
 	int (*ide_dma_end)(ide_drive_t *drive);
 	int (*ide_dma_check)(ide_drive_t *drive);
 	int (*ide_dma_on)(ide_drive_t *drive);
-	int (*ide_dma_off_quietly)(ide_drive_t *drive);
+	void (*dma_off_quietly)(ide_drive_t *drive);
 	int (*ide_dma_test_irq)(ide_drive_t *drive);
 	void (*ide_dma_clear_irq)(ide_drive_t *drive);
 	int (*ide_dma_host_on)(ide_drive_t *drive);
-	int (*ide_dma_host_off)(ide_drive_t *drive);
+	void (*dma_host_off)(ide_drive_t *drive);
 	int (*ide_dma_lostirq)(ide_drive_t *drive);
 	int (*ide_dma_timeout)(ide_drive_t *drive);
 
@@ -1276,7 +1276,7 @@ int ide_in_drive_list(struct hd_driveid 
 int __ide_dma_bad_drive(ide_drive_t *);
 int __ide_dma_good_drive(ide_drive_t *);
 int ide_use_dma(ide_drive_t *);
-int __ide_dma_off(ide_drive_t *);
+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 *);
@@ -1288,8 +1288,8 @@ extern void ide_destroy_dmatable(ide_dri
 extern int ide_release_dma(ide_hwif_t *);
 extern void ide_setup_dma(ide_hwif_t *, unsigned long, unsigned int);
 
-extern int __ide_dma_host_off(ide_drive_t *);
-extern int __ide_dma_off_quietly(ide_drive_t *);
+void ide_dma_host_off(ide_drive_t *);
+void ide_dma_off_quietly(ide_drive_t *);
 extern int __ide_dma_host_on(ide_drive_t *);
 extern int __ide_dma_on(ide_drive_t *);
 extern int __ide_dma_check(ide_drive_t *);
@@ -1302,7 +1302,7 @@ extern int __ide_dma_timeout(ide_drive_t
 
 #else
 static inline int ide_use_dma(ide_drive_t *drive) { return 0; }
-static inline int __ide_dma_off(ide_drive_t *drive) { return 0; }
+static inline void ide_dma_off(ide_drive_t *drive) { ; }
 static inline void ide_dma_verbose(ide_drive_t *drive) { ; }
 static inline int ide_set_dma(ide_drive_t *drive) { return 1; }
 #endif /* CONFIG_BLK_DEV_IDEDMA */

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void
  2007-02-03 13:53 [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
@ 2007-02-03 13:53 ` Bartlomiej Zolnierkiewicz
  2007-02-19 22:20   ` Olaf Hering
  2007-02-03 13:53 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
  2007-02-03 13:53 ` [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode Bartlomiej Zolnierkiewicz
  2 siblings, 1 reply; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03 13:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, Bartlomiej Zolnierkiewicz, linux-kernel

[PATCH] ide: make ide_hwif_t.ide_dma_host_on void

* since ide_hwif_t.ide_dma_host_on is called either when drive->using_dma == 1
  or when return value is discarded make it void, also drop "ide_" prefix
* make __ide_dma_host_on() void and drop "__" prefix

v2:
* while at it rename atiixp_ide_dma_host_on() to atiixp_dma_host_on()
  and sgiioc4_ide_dma_host_on() to sgiioc4_dma_host_on().
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/arm/icside.c      |    8 ++++----
 drivers/ide/cris/ide-cris.c   |    2 +-
 drivers/ide/ide-dma.c         |   17 +++++++----------
 drivers/ide/ide-iops.c        |    2 +-
 drivers/ide/ide.c             |    2 +-
 drivers/ide/mips/au1xxx-ide.c |    8 ++++----
 drivers/ide/pci/atiixp.c      |    6 +++---
 drivers/ide/pci/sgiioc4.c     |   11 +++--------
 drivers/ide/ppc/pmac.c        |    6 ++----
 include/linux/ide.h           |    4 ++--
 10 files changed, 28 insertions(+), 38 deletions(-)

Index: b/drivers/ide/arm/icside.c
===================================================================
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -316,15 +316,15 @@ static void icside_dma_off_quietly(ide_d
 	drive->using_dma = 0;
 }
 
-static int icside_dma_host_on(ide_drive_t *drive)
+static void icside_dma_host_on(ide_drive_t *drive)
 {
-	return 0;
 }
 
 static int icside_dma_on(ide_drive_t *drive)
 {
 	drive->using_dma = 1;
-	return icside_dma_host_on(drive);
+
+	return 0;
 }
 
 static int icside_dma_check(ide_drive_t *drive)
@@ -494,7 +494,7 @@ static void icside_dma_init(ide_hwif_t *
 	hwif->ide_dma_check	= icside_dma_check;
 	hwif->dma_host_off	= icside_dma_host_off;
 	hwif->dma_off_quietly	= icside_dma_off_quietly;
-	hwif->ide_dma_host_on	= icside_dma_host_on;
+	hwif->dma_host_on	= icside_dma_host_on;
 	hwif->ide_dma_on	= icside_dma_on;
 	hwif->dma_setup		= icside_dma_setup;
 	hwif->dma_exec_cmd	= icside_dma_exec_cmd;
Index: b/drivers/ide/cris/ide-cris.c
===================================================================
--- a/drivers/ide/cris/ide-cris.c
+++ b/drivers/ide/cris/ide-cris.c
@@ -818,7 +818,7 @@ init_e100_ide (void)
 		hwif->INB = &cris_ide_inb;
 		hwif->INW = &cris_ide_inw;
 		hwif->dma_host_off = &cris_dma_off;
-		hwif->ide_dma_host_on = &cris_dma_on;
+		hwif->dma_host_on = &cris_dma_on;
 		hwif->dma_off_quietly = &cris_dma_off;
 		hwif->udma_four = 0;
 		hwif->ultra_mask = cris_ultra_mask;
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -468,14 +468,14 @@ EXPORT_SYMBOL(ide_dma_off);
 
 #ifdef CONFIG_BLK_DEV_IDEDMA_PCI
 /**
- *	__ide_dma_host_on	-	Enable DMA on a host
+ *	ide_dma_host_on	-	Enable DMA on a host
  *	@drive: drive to enable for DMA
  *
  *	Enable DMA on an IDE controller following generic bus mastering
  *	IDE controller behaviour
  */
- 
-int __ide_dma_host_on (ide_drive_t *drive)
+
+void ide_dma_host_on(ide_drive_t *drive)
 {
 	if (drive->using_dma) {
 		ide_hwif_t *hwif	= HWIF(drive);
@@ -483,12 +483,10 @@ int __ide_dma_host_on (ide_drive_t *driv
 		u8 dma_stat		= hwif->INB(hwif->dma_status);
 
 		hwif->OUTB((dma_stat|(1<<(5+unit))), hwif->dma_status);
-		return 0;
 	}
-	return 1;
 }
 
-EXPORT_SYMBOL(__ide_dma_host_on);
+EXPORT_SYMBOL(ide_dma_host_on);
 
 /**
  *	__ide_dma_on		-	Enable DMA on a device
@@ -506,8 +504,7 @@ int __ide_dma_on (ide_drive_t *drive)
 	drive->using_dma = 1;
 	ide_toggle_bounce(drive, 1);
 
-	if (HWIF(drive)->ide_dma_host_on(drive))
-		return 1;
+	drive->hwif->dma_host_on(drive);
 
 	return 0;
 }
@@ -940,8 +937,8 @@ void ide_setup_dma (ide_hwif_t *hwif, un
 		hwif->dma_host_off = &ide_dma_host_off;
 	if (!hwif->ide_dma_on)
 		hwif->ide_dma_on = &__ide_dma_on;
-	if (!hwif->ide_dma_host_on)
-		hwif->ide_dma_host_on = &__ide_dma_host_on;
+	if (!hwif->dma_host_on)
+		hwif->dma_host_on = &ide_dma_host_on;
 	if (!hwif->ide_dma_check)
 		hwif->ide_dma_check = &__ide_dma_check;
 	if (!hwif->dma_setup)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -830,7 +830,7 @@ int ide_config_drive_speed (ide_drive_t 
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 	if (speed >= XFER_SW_DMA_0)
-		hwif->ide_dma_host_on(drive);
+		hwif->dma_host_on(drive);
 	else if (hwif->ide_dma_check)	/* check if host supports DMA */
 		hwif->dma_off_quietly(drive);
 #endif
Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -509,7 +509,7 @@ static void ide_hwif_restore(ide_hwif_t 
 	hwif->dma_off_quietly		= tmp_hwif->dma_off_quietly;
 	hwif->ide_dma_test_irq		= tmp_hwif->ide_dma_test_irq;
 	hwif->ide_dma_clear_irq		= tmp_hwif->ide_dma_clear_irq;
-	hwif->ide_dma_host_on		= tmp_hwif->ide_dma_host_on;
+	hwif->dma_host_on		= tmp_hwif->dma_host_on;
 	hwif->dma_host_off		= tmp_hwif->dma_host_off;
 	hwif->ide_dma_lostirq		= tmp_hwif->ide_dma_lostirq;
 	hwif->ide_dma_timeout		= tmp_hwif->ide_dma_timeout;
Index: b/drivers/ide/mips/au1xxx-ide.c
===================================================================
--- a/drivers/ide/mips/au1xxx-ide.c
+++ b/drivers/ide/mips/au1xxx-ide.c
@@ -438,15 +438,15 @@ static int auide_dma_test_irq(ide_drive_
 	return 0;
 }
 
-static int auide_dma_host_on(ide_drive_t *drive)
+static void auide_dma_host_on(ide_drive_t *drive)
 {
-	return 0;
 }
 
 static int auide_dma_on(ide_drive_t *drive)
 {
 	drive->using_dma = 1;
-	return auide_dma_host_on(drive);
+
+	return 0;
 }
 
 static void auide_dma_host_off(ide_drive_t *drive)
@@ -731,7 +731,7 @@ static int au_ide_probe(struct device *d
 	hwif->dma_setup                 = &auide_dma_setup;
 	hwif->ide_dma_test_irq          = &auide_dma_test_irq;
 	hwif->dma_host_off		= &auide_dma_host_off;
-	hwif->ide_dma_host_on           = &auide_dma_host_on;
+	hwif->dma_host_on		= &auide_dma_host_on;
 	hwif->ide_dma_lostirq           = &auide_dma_lostirq;
 	hwif->ide_dma_on                = &auide_dma_on;
 
Index: b/drivers/ide/pci/atiixp.c
===================================================================
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -101,7 +101,7 @@ static u8 atiixp_dma_2_pio(u8 xfer_rate)
 	}
 }
 
-static int atiixp_ide_dma_host_on(ide_drive_t *drive)
+static void atiixp_dma_host_on(ide_drive_t *drive)
 {
 	struct pci_dev *dev = drive->hwif->pci_dev;
 	unsigned long flags;
@@ -118,7 +118,7 @@ static int atiixp_ide_dma_host_on(ide_dr
 
 	spin_unlock_irqrestore(&atiixp_lock, flags);
 
-	return __ide_dma_host_on(drive);
+	ide_dma_host_on(drive);
 }
 
 static void atiixp_dma_host_off(ide_drive_t *drive)
@@ -305,7 +305,7 @@ static void __devinit init_hwif_atiixp(i
 	else
 		hwif->udma_four = 0;
 
-	hwif->ide_dma_host_on = &atiixp_ide_dma_host_on;
+	hwif->dma_host_on = &atiixp_dma_host_on;
 	hwif->dma_host_off = &atiixp_dma_host_off;
 	hwif->ide_dma_check = &atiixp_dma_check;
 	if (!noautodma)
Index: b/drivers/ide/pci/sgiioc4.c
===================================================================
--- a/drivers/ide/pci/sgiioc4.c
+++ b/drivers/ide/pci/sgiioc4.c
@@ -279,7 +279,7 @@ sgiioc4_ide_dma_on(ide_drive_t * drive)
 {
 	drive->using_dma = 1;
 
-	return HWIF(drive)->ide_dma_host_on(drive);
+	return 0;
 }
 
 static void sgiioc4_dma_off_quietly(ide_drive_t *drive)
@@ -307,13 +307,8 @@ sgiioc4_ide_dma_test_irq(ide_drive_t * d
 	return sgiioc4_checkirq(HWIF(drive));
 }
 
-static int
-sgiioc4_ide_dma_host_on(ide_drive_t * drive)
+static void sgiioc4_dma_host_on(ide_drive_t * drive)
 {
-	if (drive->using_dma)
-		return 0;
-
-	return 1;
 }
 
 static void sgiioc4_dma_host_off(ide_drive_t * drive)
@@ -610,7 +605,7 @@ ide_init_sgiioc4(ide_hwif_t * hwif)
 	hwif->ide_dma_on = &sgiioc4_ide_dma_on;
 	hwif->dma_off_quietly = &sgiioc4_dma_off_quietly;
 	hwif->ide_dma_test_irq = &sgiioc4_ide_dma_test_irq;
-	hwif->ide_dma_host_on = &sgiioc4_ide_dma_host_on;
+	hwif->dma_host_on = &sgiioc4_dma_host_on;
 	hwif->dma_host_off = &sgiioc4_dma_host_off;
 	hwif->ide_dma_lostirq = &sgiioc4_ide_dma_lostirq;
 	hwif->ide_dma_timeout = &__ide_dma_timeout;
Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -1984,10 +1984,8 @@ static void pmac_ide_dma_host_off(ide_dr
 {
 }
 
-static int
-pmac_ide_dma_host_on (ide_drive_t *drive)
+static int pmac_ide_dma_host_on(ide_drive_t *drive)
 {
-	return 0;
 }
 
 static int
@@ -2042,7 +2040,7 @@ pmac_ide_setup_dma(pmac_ide_hwif_t *pmif
 	hwif->ide_dma_end = &pmac_ide_dma_end;
 	hwif->ide_dma_test_irq = &pmac_ide_dma_test_irq;
 	hwif->dma_host_off = &pmac_ide_dma_host_off;
-	hwif->ide_dma_host_on = &pmac_ide_dma_host_on;
+	hwif->dma_host_on = &pmac_ide_dma_host_on;
 	hwif->ide_dma_timeout = &__ide_dma_timeout;
 	hwif->ide_dma_lostirq = &pmac_ide_dma_lostirq;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -738,7 +738,7 @@ typedef struct hwif_s {
 	void (*dma_off_quietly)(ide_drive_t *drive);
 	int (*ide_dma_test_irq)(ide_drive_t *drive);
 	void (*ide_dma_clear_irq)(ide_drive_t *drive);
-	int (*ide_dma_host_on)(ide_drive_t *drive);
+	void (*dma_host_on)(ide_drive_t *drive);
 	void (*dma_host_off)(ide_drive_t *drive);
 	int (*ide_dma_lostirq)(ide_drive_t *drive);
 	int (*ide_dma_timeout)(ide_drive_t *drive);
@@ -1290,7 +1290,7 @@ extern void ide_setup_dma(ide_hwif_t *, 
 
 void ide_dma_host_off(ide_drive_t *);
 void ide_dma_off_quietly(ide_drive_t *);
-extern int __ide_dma_host_on(ide_drive_t *);
+void ide_dma_host_on(ide_drive_t *);
 extern int __ide_dma_on(ide_drive_t *);
 extern int __ide_dma_check(ide_drive_t *);
 extern int ide_dma_setup(ide_drive_t *);

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-02-03 13:53 [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
  2007-02-03 13:53 ` [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void Bartlomiej Zolnierkiewicz
@ 2007-02-03 13:53 ` Bartlomiej Zolnierkiewicz
  2007-02-03 13:53 ` [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode Bartlomiej Zolnierkiewicz
  2 siblings, 0 replies; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03 13:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, Bartlomiej Zolnierkiewicz, linux-kernel

[PATCH] ide: fix UDMA/MWDMA/SWDMA masks

* use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
* add udma_mask field to ide_pci_device_t and use it to initialize
  ->ultra_mask in aec62xx, cmd64x, pdc202xx_new and pdc202xx_old drivers
* fix UDMA masks to match with chipset specific *_ratemask()
  (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
   filtering method - done in the next patch)

v2:
* piix: fix cable detection for 82801AA_1 and 82372FB_1
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
* cmd64x: use hwif->cds->udma_mask
  [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
* aec62xx: fix newly introduced bug - check DMA status not command register
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/ide.c              |    3 ---
 drivers/ide/pci/aec62xx.c      |   19 +++++++++++++++++--
 drivers/ide/pci/alim15x3.c     |   13 +++++++++++--
 drivers/ide/pci/cmd64x.c       |   15 ++++++++-------
 drivers/ide/pci/pdc202xx_new.c |    9 ++++++++-
 drivers/ide/pci/pdc202xx_old.c |    7 ++++++-
 drivers/ide/pci/piix.c         |    6 +++++-
 drivers/ide/pci/sis5513.c      |    5 ++++-
 include/linux/ide.h            |    1 +
 9 files changed, 60 insertions(+), 18 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h
 	hwif->bus_state	= BUSSTATE_ON;
 
 	hwif->atapi_dma = 0;		/* disable all atapi dma */ 
-	hwif->ultra_mask = 0x80;	/* disable all ultra */
-	hwif->mwdma_mask = 0x80;	/* disable all mwdma */
-	hwif->swdma_mask = 0x80;	/* disable all swdma */
 
 	init_completion(&hwif->gendev_rel_comp);
 
Index: b/drivers/ide/pci/aec62xx.c
===================================================================
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -270,11 +270,13 @@ static unsigned int __devinit init_chips
 
 static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif)
 {
+	struct pci_dev *dev = hwif->pci_dev;
+
 	hwif->autodma = 0;
 	hwif->tuneproc = &aec62xx_tune_drive;
 	hwif->speedproc = &aec62xx_tune_chipset;
 
-	if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
+	if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
 		hwif->serialized = hwif->channel;
 
 	if (hwif->mate)
@@ -286,7 +288,15 @@ static void __devinit init_hwif_aec62xx(
 		return;
 	}
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
+
+	/* atp865 and atp865r */
+	if (hwif->ultra_mask == 0x3f) {
+		/* check bit 0x10 of DMA status register */
+		if (inb(pci_resource_start(dev, 4) + 2) & 0x10)
+ 			hwif->ultra_mask = 0x7f; /* udma0-6 */
+	}
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
@@ -354,6 +364,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "AEC6260",
 		.init_setup	= init_setup_aec62xx,
@@ -363,6 +374,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= NOAUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "AEC6260R",
 		.init_setup	= init_setup_aec62xx,
@@ -373,6 +385,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= NEVER_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "AEC6X80",
 		.init_setup	= init_setup_aec6x80,
@@ -382,6 +395,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "AEC6X80R",
 		.init_setup	= init_setup_aec6x80,
@@ -392,6 +406,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/alim15x3.c
===================================================================
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a
 
 	hwif->atapi_dma = 1;
 
-	if (m5229_revision > 0x20)
-		hwif->ultra_mask = 0x7f;
+	if (m5229_revision <= 0x20)
+		hwif->ultra_mask = 0x00; /* no udma */
+	else if (m5229_revision < 0xC2)
+		hwif->ultra_mask = 0x07; /* udma0-2 */
+	else if (m5229_revision == 0xC2 || m5229_revision == 0xC3)
+		hwif->ultra_mask = 0x1f; /* udma0-4 */
+	else if (m5229_revision == 0xC4)
+		hwif->ultra_mask = 0x3f; /* udma0-5 */
+	else
+		hwif->ultra_mask = 0x7f; /* udma0-6 */
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/drivers/ide/pci/cmd64x.c
===================================================================
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -690,16 +690,13 @@ static void __devinit init_hwif_cmd64x(i
 
 	hwif->atapi_dma = 1;
 
-	hwif->ultra_mask = 0x3f;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
-	if (dev->device == PCI_DEVICE_ID_CMD_643)
-		hwif->ultra_mask = 0x80;
-	if (dev->device == PCI_DEVICE_ID_CMD_646)
-		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
-	if (dev->device == PCI_DEVICE_ID_CMD_648)
-		hwif->ultra_mask = 0x1f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
+
+	if (dev->device == PCI_DEVICE_ID_CMD_646 && class_rev < 5)
+		hwif->ultra_mask = 0x00;
 
 	hwif->ide_dma_check = &cmd64x_config_drive_for_dma;
 	if (!(hwif->udma_four))
@@ -733,6 +730,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x00, /* no udma */
 	},{	/* 1 */
 		.name		= "CMD646",
 		.init_chipset	= init_chipset_cmd64x,
@@ -741,6 +739,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x00,0x00,0x00}, {0x51,0x80,0x80}},
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 2 */
 		.name		= "CMD648",
 		.init_chipset	= init_chipset_cmd64x,
@@ -748,6 +747,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "CMD649",
 		.init_chipset	= init_chipset_cmd64x,
@@ -755,6 +755,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -545,7 +545,7 @@ static void __devinit init_hwif_pdc202ne
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 
 	hwif->err_stops_fifo = 1;
@@ -621,6 +621,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 1 */
 		.name		= "PDC20269",
 		.init_setup	= init_setup_pdcnew,
@@ -629,6 +630,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 2 */
 		.name		= "PDC20270",
 		.init_setup	= init_setup_pdc20270,
@@ -637,6 +639,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 3 */
 		.name		= "PDC20271",
 		.init_setup	= init_setup_pdcnew,
@@ -645,6 +648,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 4 */
 		.name		= "PDC20275",
 		.init_setup	= init_setup_pdcnew,
@@ -653,6 +657,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 5 */
 		.name		= "PDC20276",
 		.init_setup	= init_setup_pdc20276,
@@ -661,6 +666,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 6 */
 		.name		= "PDC20277",
 		.init_setup	= init_setup_pdcnew,
@@ -669,6 +675,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -478,7 +478,7 @@ static void __devinit init_hwif_pdc202xx
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x3f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 	hwif->atapi_dma = 1;
@@ -587,6 +587,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 16,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "PDC20262",
 		.init_setup	= init_setup_pdc202ata4,
@@ -597,6 +598,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "PDC20263",
 		.init_setup	= init_setup_pdc202ata4,
@@ -607,6 +609,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "PDC20265",
 		.init_setup	= init_setup_pdc20265,
@@ -617,6 +620,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "PDC20267",
 		.init_setup	= init_setup_pdc202xx,
@@ -627,6 +631,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -522,7 +522,7 @@ static void __devinit init_hwif_piix(ide
 		case PCI_DEVICE_ID_INTEL_82371FB_0:
 		case PCI_DEVICE_ID_INTEL_82371FB_1:
 		case PCI_DEVICE_ID_INTEL_82371SB_1:
-			hwif->ultra_mask = 0x80;
+			hwif->ultra_mask = 0x00;
 			break;
 		case PCI_DEVICE_ID_INTEL_82371AB:
 		case PCI_DEVICE_ID_INTEL_82443MX_1:
@@ -530,6 +530,10 @@ static void __devinit init_hwif_piix(ide
 		case PCI_DEVICE_ID_INTEL_82801AB_1:
 			hwif->ultra_mask = 0x07;
 			break;
+		case PCI_DEVICE_ID_INTEL_82801AA_1:
+		case PCI_DEVICE_ID_INTEL_82372FB_1:
+			hwif->ultra_mask = 0x1f;
+			/* fall-through */
 		default:
 			if (!hwif->udma_four)
 				hwif->udma_four = piix_cable_detect(hwif);
Index: b/drivers/ide/pci/sis5513.c
===================================================================
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -858,6 +858,8 @@ static unsigned int __devinit ata66_sis5
 
 static void __devinit init_hwif_sis5513 (ide_hwif_t *hwif)
 {
+	u8 udma_rates[] = { 0x00, 0x00, 0x07, 0x1f, 0x3f, 0x3f, 0x7f, 0x7f };
+
 	hwif->autodma = 0;
 
 	if (!hwif->irq)
@@ -873,7 +875,8 @@ static void __devinit init_hwif_sis5513 
 	}
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x7f;
+
+	hwif->ultra_mask = udma_rates[chipset_family];
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ typedef struct ide_pci_device_s {
 	unsigned int		extra;
 	struct ide_pci_device_s	*next;
 	u8			flags;
+	u8			udma_mask;
 } ide_pci_device_t;
 
 extern int ide_setup_pci_device(struct pci_dev *, ide_pci_device_t *);

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
  2007-02-03 13:53 [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
  2007-02-03 13:53 ` [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void Bartlomiej Zolnierkiewicz
  2007-02-03 13:53 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
@ 2007-02-03 13:53 ` Bartlomiej Zolnierkiewicz
  2007-04-19 19:31   ` Sergei Shtylyov
  2 siblings, 1 reply; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03 13:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, Bartlomiej Zolnierkiewicz, linux-kernel

[PATCH] ide: rework the code for selecting the best DMA transfer mode 

Depends on the "ide: fix UDMA/MWDMA/SWDMA masks" patch.

* add ide_hwif_t.udma_filter hook for filtering UDMA mask
  (use it in alim15x3, hpt366, siimage and serverworks drivers)
* add ide_max_dma_mode() for finding best DMA mode for the device
  (loosely based on some older libata-core.c code)
* convert ide_dma_speed() users to use ide_max_dma_mode()
* make ide_rate_filter() take "ide_drive_t *drive" as an argument instead
  of "u8 mode" and teach it to how to use UDMA mask to do filtering
* use ide_rate_filter() in hpt366 driver
* remove no longer needed ide_dma_speed() and *_ratemask()
* unexport eighty_ninty_three()

v2:
* rename ->filter_udma_mask to ->udma_filter
  [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/arm/icside.c       |    2 
 drivers/ide/cris/ide-cris.c    |    2 
 drivers/ide/ide-dma.c          |   74 ++++++++++++++++++++++++
 drivers/ide/ide-iops.c         |    2 
 drivers/ide/ide-lib.c          |  125 +++++------------------------------------
 drivers/ide/ide.c              |    1 
 drivers/ide/pci/aec62xx.c      |   32 ----------
 drivers/ide/pci/alim15x3.c     |   76 +++++-------------------
 drivers/ide/pci/atiixp.c       |   20 ------
 drivers/ide/pci/cmd64x.c       |   65 ++++-----------------
 drivers/ide/pci/cs5535.c       |   20 ------
 drivers/ide/pci/hpt34x.c       |    9 --
 drivers/ide/pci/hpt366.c       |   67 +++++++++++----------
 drivers/ide/pci/it8213.c       |   20 ------
 drivers/ide/pci/it821x.c       |   20 ------
 drivers/ide/pci/jmicron.c      |   21 ------
 drivers/ide/pci/pdc202xx_new.c |   14 ----
 drivers/ide/pci/pdc202xx_old.c |   27 --------
 drivers/ide/pci/piix.c         |   66 ---------------------
 drivers/ide/pci/serverworks.c  |   31 ++++++----
 drivers/ide/pci/siimage.c      |   45 ++++++--------
 drivers/ide/pci/sis5513.c      |   14 ----
 drivers/ide/pci/slc90e66.c     |   13 ----
 drivers/ide/pci/tc86c001.c     |    9 --
 drivers/ide/pci/triflex.c      |    4 -
 include/linux/ide.h            |   10 +--
 26 files changed, 234 insertions(+), 555 deletions(-)

Index: b/drivers/ide/arm/icside.c
===================================================================
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -347,7 +347,7 @@ static int icside_dma_check(ide_drive_t 
 	 * Enable DMA on any drive that has multiword DMA
 	 */
 	if (id->field_valid & 2) {
-		xfer_mode = ide_dma_speed(drive, 0);
+		xfer_mode = ide_max_dma_mode(drive);
 		goto out;
 	}
 
Index: b/drivers/ide/cris/ide-cris.c
===================================================================
--- a/drivers/ide/cris/ide-cris.c
+++ b/drivers/ide/cris/ide-cris.c
@@ -1006,7 +1006,7 @@ static int cris_ide_build_dmatable (ide_
 
 static int cris_config_drive_for_dma (ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, 1);
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -705,6 +705,80 @@ int ide_use_dma(ide_drive_t *drive)
 
 EXPORT_SYMBOL_GPL(ide_use_dma);
 
+static const u8 xfer_mode_bases[] = {
+	XFER_UDMA_0,
+	XFER_MW_DMA_0,
+	XFER_SW_DMA_0,
+};
+
+static unsigned int ide_get_mode_mask(ide_drive_t *drive, u8 base)
+{
+	struct hd_driveid *id = drive->id;
+	ide_hwif_t *hwif = drive->hwif;
+	unsigned int mask = 0;
+
+	switch(base) {
+	case XFER_UDMA_0:
+		if ((id->field_valid & 4) == 0)
+			break;
+
+		mask = id->dma_ultra & hwif->ultra_mask;
+
+		if (hwif->udma_filter)
+			mask &= hwif->udma_filter(drive);
+
+		if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
+			mask &= 0x07;
+		break;
+	case XFER_MW_DMA_0:
+		mask = id->dma_mword & hwif->mwdma_mask;
+		break;
+	case XFER_SW_DMA_0:
+		mask = id->dma_1word & hwif->swdma_mask;
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	return mask;
+}
+
+/**
+ *	ide_max_dma_mode	-	compute DMA speed
+ *	@drive: IDE device
+ *
+ *	Checks the drive capabilities and returns the speed to use
+ *	for the DMA transfer.  Returns 0 if the drive is incapable
+ *	of DMA transfers.
+ */
+
+u8 ide_max_dma_mode(ide_drive_t *drive)
+{
+	ide_hwif_t *hwif = drive->hwif;
+	unsigned int mask;
+	int x, i;
+	u8 mode = 0;
+
+	if (drive->media != ide_disk && hwif->atapi_dma == 0)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(xfer_mode_bases); i++) {
+		mask = ide_get_mode_mask(drive, xfer_mode_bases[i]);
+		x = fls(mask) - 1;
+		if (x >= 0) {
+			mode = xfer_mode_bases[i] + x;
+			break;
+		}
+	}
+
+	printk(KERN_DEBUG "%s: selected mode 0x%x\n", drive->name, mode);
+
+	return mode;
+}
+
+EXPORT_SYMBOL_GPL(ide_max_dma_mode);
+
 void ide_dma_verbose(ide_drive_t *drive)
 {
 	struct hd_driveid *id	= drive->id;
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -586,8 +586,6 @@ u8 eighty_ninty_three (ide_drive_t *driv
 	return 1;
 }
 
-EXPORT_SYMBOL(eighty_ninty_three);
-
 int ide_ata66_check (ide_drive_t *drive, ide_task_t *args)
 {
 	if ((args->tfRegister[IDE_COMMAND_OFFSET] == WIN_SETFEATURES) &&
Index: b/drivers/ide/ide-lib.c
===================================================================
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -69,123 +69,34 @@ char *ide_xfer_verbose (u8 xfer_rate)
 EXPORT_SYMBOL(ide_xfer_verbose);
 
 /**
- *	ide_dma_speed	-	compute DMA speed
- *	@drive: drive
- *	@mode:	modes available
- *
- *	Checks the drive capabilities and returns the speed to use
- *	for the DMA transfer.  Returns 0 if the drive is incapable
- *	of DMA transfers.
- */
- 
-u8 ide_dma_speed(ide_drive_t *drive, u8 mode)
-{
-	struct hd_driveid *id   = drive->id;
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 ultra_mask, mwdma_mask, swdma_mask;
-	u8 speed = 0;
-
-	if (drive->media != ide_disk && hwif->atapi_dma == 0)
-		return 0;
-
-	/* Capable of UltraDMA modes? */
-	ultra_mask = id->dma_ultra & hwif->ultra_mask;
-
-	if (!(id->field_valid & 4))
-		mode = 0;	/* fallback to MW/SW DMA if no UltraDMA */
-
-	switch (mode) {
-	case 4:
-		if (ultra_mask & 0x40) {
-			speed = XFER_UDMA_6;
-			break;
-		}
-	case 3:
-		if (ultra_mask & 0x20) {
-			speed = XFER_UDMA_5;
-			break;
-		}
-	case 2:
-		if (ultra_mask & 0x10) {
-			speed = XFER_UDMA_4;
-			break;
-		}
-		if (ultra_mask & 0x08) {
-			speed = XFER_UDMA_3;
-			break;
-		}
-	case 1:
-		if (ultra_mask & 0x04) {
-			speed = XFER_UDMA_2;
-			break;
-		}
-		if (ultra_mask & 0x02) {
-			speed = XFER_UDMA_1;
-			break;
-		}
-		if (ultra_mask & 0x01) {
-			speed = XFER_UDMA_0;
-			break;
-		}
-	case 0:
-		mwdma_mask = id->dma_mword & hwif->mwdma_mask;
-
-		if (mwdma_mask & 0x04) {
-			speed = XFER_MW_DMA_2;
-			break;
-		}
-		if (mwdma_mask & 0x02) {
-			speed = XFER_MW_DMA_1;
-			break;
-		}
-		if (mwdma_mask & 0x01) {
-			speed = XFER_MW_DMA_0;
-			break;
-		}
-
-		swdma_mask = id->dma_1word & hwif->swdma_mask;
-
-		if (swdma_mask & 0x04) {
-			speed = XFER_SW_DMA_2;
-			break;
-		}
-		if (swdma_mask & 0x02) {
-			speed = XFER_SW_DMA_1;
-			break;
-		}
-		if (swdma_mask & 0x01) {
-			speed = XFER_SW_DMA_0;
-			break;
-		}
-	}
-
-	return speed;
-}
-EXPORT_SYMBOL(ide_dma_speed);
-
-
-/**
- *	ide_rate_filter		-	return best speed for mode
- *	@mode: modes available
+ *	ide_rate_filter		-	filter transfer mode
+ *	@drive: IDE device
  *	@speed: desired speed
  *
- *	Given the available DMA/UDMA mode this function returns
+ *	Given the available transfer modes this function returns
  *	the best available speed at or below the speed requested.
+ *
+ *	FIXME: filter also PIO/SWDMA/MWDMA modes
  */
 
-u8 ide_rate_filter (u8 mode, u8 speed) 
+u8 ide_rate_filter(ide_drive_t *drive, u8 speed)
 {
 #ifdef CONFIG_BLK_DEV_IDEDMA
-	static u8 speed_max[] = {
-		XFER_MW_DMA_2, XFER_UDMA_2, XFER_UDMA_4,
-		XFER_UDMA_5, XFER_UDMA_6
-	};
+	ide_hwif_t *hwif = drive->hwif;
+	u8 mask = hwif->ultra_mask, mode = XFER_MW_DMA_2;
+
+	if (hwif->udma_filter)
+		mask = hwif->udma_filter(drive);
+
+	if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
+		mask &= 0x07;
+
+	if (mask)
+		mode = fls(mask) - 1 + XFER_UDMA_0;
 
 //	printk("%s: mode 0x%02x, speed 0x%02x\n", __FUNCTION__, mode, speed);
 
-	/* So that we remember to update this if new modes appear */
-	BUG_ON(mode > 4);
-	return min(speed, speed_max[mode]);
+	return min(speed, mode);
 #else /* !CONFIG_BLK_DEV_IDEDMA */
 	return min(speed, (u8)XFER_PIO_4);
 #endif /* CONFIG_BLK_DEV_IDEDMA */
Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -483,6 +483,7 @@ static void ide_hwif_restore(ide_hwif_t 
 
 	hwif->tuneproc			= tmp_hwif->tuneproc;
 	hwif->speedproc			= tmp_hwif->speedproc;
+	hwif->udma_filter		= tmp_hwif->udma_filter;
 	hwif->selectproc		= tmp_hwif->selectproc;
 	hwif->reset_poll		= tmp_hwif->reset_poll;
 	hwif->pre_reset			= tmp_hwif->pre_reset;
Index: b/drivers/ide/pci/aec62xx.c
===================================================================
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -86,38 +86,12 @@ static u8 pci_bus_clock_list_ultra (u8 s
 	return chipset_table->ultra_settings;
 }
 
-static u8 aec62xx_ratemask (ide_drive_t *drive)
-{
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 mode;
-
-	switch(hwif->pci_dev->device) {
-		case PCI_DEVICE_ID_ARTOP_ATP865:
-		case PCI_DEVICE_ID_ARTOP_ATP865R:
-			mode = (inb(hwif->channel ?
-				    hwif->mate->dma_status :
-				    hwif->dma_status) & 0x10) ? 4 : 3;
-			break;
-		case PCI_DEVICE_ID_ARTOP_ATP860:
-		case PCI_DEVICE_ID_ARTOP_ATP860R:
-			mode = 2;
-			break;
-		case PCI_DEVICE_ID_ARTOP_ATP850UF:
-		default:
-			return 1;
-	}
-
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
-}
-
 static int aec6210_tune_chipset (ide_drive_t *drive, u8 xferspeed)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
 	u16 d_conf		= 0;
-	u8 speed	= ide_rate_filter(aec62xx_ratemask(drive), xferspeed);
+	u8 speed		= ide_rate_filter(drive, xferspeed);
 	u8 ultra = 0, ultra_conf = 0;
 	u8 tmp0 = 0, tmp1 = 0, tmp2 = 0;
 	unsigned long flags;
@@ -144,7 +118,7 @@ static int aec6260_tune_chipset (ide_dri
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
-	u8 speed	= ide_rate_filter(aec62xx_ratemask(drive), xferspeed);
+	u8 speed	= ide_rate_filter(drive, xferspeed);
 	u8 unit		= (drive->select.b.unit & 0x01);
 	u8 tmp1 = 0, tmp2 = 0;
 	u8 ultra = 0, drive_conf = 0, ultra_conf = 0;
@@ -182,7 +156,7 @@ static int aec62xx_tune_chipset (ide_dri
 
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, aec62xx_ratemask(drive));	
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!(speed))
 		return 0;
Index: b/drivers/ide/pci/alim15x3.c
===================================================================
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -359,74 +359,31 @@ static void ali15x3_tune_drive (ide_driv
 }
 
 /**
- *	ali15x3_can_ultra	-	check for ultra DMA support
- *	@drive: drive to do the check
+ *	ali_udma_filter		-	compute UDMA mask
+ *	@drive: IDE device
  *
- *	Check the drive and controller revisions. Return 0 if UDMA is
- *	not available, or 1 if UDMA can be used. The actual rules for
- *	the ALi are
+ *	Return available UDMA modes.
+ *
+ *	The actual rules for the ALi are:
  *		No UDMA on revisions <= 0x20
  *		Disk only for revisions < 0xC2
  *		Not WDC drives for revisions < 0xC2
  *
  *	FIXME: WDC ifdef needs to die
  */
- 
-static u8 ali15x3_can_ultra (ide_drive_t *drive)
-{
-#ifndef CONFIG_WDC_ALI15X3
-	struct hd_driveid *id	= drive->id;
-#endif /* CONFIG_WDC_ALI15X3 */
-
-	if (m5229_revision <= 0x20) {
-		return 0;
-	} else if ((m5229_revision < 0xC2) &&
-#ifndef CONFIG_WDC_ALI15X3
-		   ((chip_is_1543c_e && strstr(id->model, "WDC ")) ||
-		    (drive->media!=ide_disk))) {
-#else /* CONFIG_WDC_ALI15X3 */
-		   (drive->media!=ide_disk)) {
-#endif /* CONFIG_WDC_ALI15X3 */
-		return 0;
-	} else {
-		return 1;
-	}
-}
 
-/**
- *	ali15x3_ratemask	-	generate DMA mode list
- *	@drive: drive to compute against
- *
- *	Generate a list of the available DMA modes for the drive. 
- *	FIXME: this function contains lots of bogus masking we can dump
- *
- *	Return the highest available mode (UDMA33, UDMA66, UDMA100,..)
- */
- 
-static u8 ali15x3_ratemask (ide_drive_t *drive)
+static u8 ali_udma_filter(ide_drive_t *drive)
 {
-	u8 mode = 0, can_ultra	= ali15x3_can_ultra(drive);
-
-	if (m5229_revision > 0xC4 && can_ultra) {
-		mode = 4;
-	} else if (m5229_revision == 0xC4 && can_ultra) {
-		mode = 3;
-	} else if (m5229_revision >= 0xC2 && can_ultra) {
-		mode = 2;
-	} else if (can_ultra) {
-		return 1;
-	} else {
-		return 0;
+	if (m5229_revision > 0x20 && m5229_revision < 0xC2) {
+		if (drive->media != ide_disk)
+			return 0;
+#ifndef CONFIG_WDC_ALI15X3
+		if (chip_is_1543c_e && strstr(drive->id->model, "WDC "))
+			return 0;
+#endif
 	}
 
-	/*
-	 *	If the drive sees no suitable cable then UDMA 33
-	 *	is the highest permitted mode
-	 */
-	 
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
+	return drive->hwif->ultra_mask;
 }
 
 /**
@@ -442,7 +399,7 @@ static int ali15x3_tune_chipset (ide_dri
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
-	u8 speed		= ide_rate_filter(ali15x3_ratemask(drive), xferspeed);
+	u8 speed		= ide_rate_filter(drive, xferspeed);
 	u8 speed1		= speed;
 	u8 unit			= (drive->select.b.unit & 0x01);
 	u8 tmpbyte		= 0x00;
@@ -492,7 +449,7 @@ static int ali15x3_tune_chipset (ide_dri
  
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, ali15x3_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!(speed))
 		return 0;
@@ -753,6 +710,7 @@ static void __devinit init_hwif_common_a
 	hwif->autodma = 0;
 	hwif->tuneproc = &ali15x3_tune_drive;
 	hwif->speedproc = &ali15x3_tune_chipset;
+	hwif->udma_filter = &ali_udma_filter;
 
 	/* don't use LBA48 DMA on ALi devices before rev 0xC5 */
 	hwif->no_lba48_dma = (m5229_revision <= 0xC4) ? 1 : 0;
Index: b/drivers/ide/pci/atiixp.c
===================================================================
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -49,22 +49,6 @@ static int save_mdma_mode[4];
 static DEFINE_SPINLOCK(atiixp_lock);
 
 /**
- *	atiixp_ratemask		-	compute rate mask for ATIIXP IDE
- *	@drive: IDE drive to compute for
- *
- *	Returns the available modes for the ATIIXP IDE controller.
- */
-
-static u8 atiixp_ratemask(ide_drive_t *drive)
-{
-	u8 mode = 3;
-
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
-}
-
-/**
  *	atiixp_dma_2_pio		-	return the PIO mode matching DMA
  *	@xfer_rate: transfer speed
  *
@@ -189,7 +173,7 @@ static int atiixp_speedproc(ide_drive_t 
 	u16 tmp16;
 	u8 speed, pio;
 
-	speed = ide_rate_filter(atiixp_ratemask(drive), xferspeed);
+	speed = ide_rate_filter(drive, xferspeed);
 
 	spin_lock_irqsave(&atiixp_lock, flags);
 
@@ -233,7 +217,7 @@ static int atiixp_speedproc(ide_drive_t 
 
 static int atiixp_config_drive_for_dma(ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, atiixp_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
Index: b/drivers/ide/pci/cmd64x.c
===================================================================
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -338,55 +338,6 @@ static void cmd64x_tuneproc (ide_drive_t
 		setup_count, active_count, recovery_count);
 }
 
-static u8 cmd64x_ratemask (ide_drive_t *drive)
-{
-	struct pci_dev *dev	= HWIF(drive)->pci_dev;
-	u8 mode = 0;
-
-	switch(dev->device) {
-		case PCI_DEVICE_ID_CMD_649:
-			mode = 3;
-			break;
-		case PCI_DEVICE_ID_CMD_648:
-			mode = 2;
-			break;
-		case PCI_DEVICE_ID_CMD_643:
-			return 0;
-
-		case PCI_DEVICE_ID_CMD_646:
-		{
-			unsigned int class_rev	= 0;
-			pci_read_config_dword(dev,
-				PCI_CLASS_REVISION, &class_rev);
-			class_rev &= 0xff;
-		/*
-		 * UltraDMA only supported on PCI646U and PCI646U2, which
-		 * correspond to revisions 0x03, 0x05 and 0x07 respectively.
-		 * Actually, although the CMD tech support people won't
-		 * tell me the details, the 0x03 revision cannot support
-		 * UDMA correctly without hardware modifications, and even
-		 * then it only works with Quantum disks due to some
-		 * hold time assumptions in the 646U part which are fixed
-		 * in the 646U2.
-		 *
-		 * So we only do UltraDMA on revision 0x05 and 0x07 chipsets.
-		 */
-			switch(class_rev) {
-				case 0x07:
-				case 0x05:
-					return 1;
-				case 0x03:
-				case 0x01:
-				default:
-					return 0;
-			}
-		}
-	}
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
-}
-
 static void config_cmd64x_chipset_for_pio (ide_drive_t *drive, u8 set_speed)
 {
 	u8 speed	= 0x00;
@@ -412,7 +363,7 @@ static int cmd64x_tune_chipset (ide_driv
 	u8 regU = 0, pciU	= (hwif->channel) ? UDIDETCR1 : UDIDETCR0;
 	u8 regD = 0, pciD	= (hwif->channel) ? BMIDESR1 : BMIDESR0;
 
-	u8 speed	= ide_rate_filter(cmd64x_ratemask(drive), xferspeed);
+	u8 speed = ide_rate_filter(drive, xferspeed);
 
 	if (speed > XFER_PIO_4) {
 		(void) pci_read_config_byte(dev, pciD, &regD);
@@ -459,7 +410,7 @@ static int cmd64x_tune_chipset (ide_driv
 
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed	= ide_dma_speed(drive, cmd64x_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	config_chipset_for_pio(drive, !speed);
 
@@ -695,6 +646,18 @@ static void __devinit init_hwif_cmd64x(i
 
 	hwif->ultra_mask = hwif->cds->udma_mask;
 
+	/*
+	 * UltraDMA only supported on PCI646U and PCI646U2, which
+	 * correspond to revisions 0x03, 0x05 and 0x07 respectively.
+	 * Actually, although the CMD tech support people won't
+	 * tell me the details, the 0x03 revision cannot support
+	 * UDMA correctly without hardware modifications, and even
+	 * then it only works with Quantum disks due to some
+	 * hold time assumptions in the 646U part which are fixed
+	 * in the 646U2.
+	 *
+	 * So we only do UltraDMA on revision 0x05 and 0x07 chipsets.
+	 */
 	if (dev->device == PCI_DEVICE_ID_CMD_646 && class_rev < 5)
 		hwif->ultra_mask = 0x00;
 
Index: b/drivers/ide/pci/cs5535.c
===================================================================
--- a/drivers/ide/pci/cs5535.c
+++ b/drivers/ide/pci/cs5535.c
@@ -127,20 +127,6 @@ static void cs5535_set_speed(ide_drive_t
 	}
 }
 
-static u8 cs5535_ratemask(ide_drive_t *drive)
-{
-	/* eighty93 will return 1 if it's 80core and capable of
-	exceeding udma2, 0 otherwise. we need ratemask to set
-	the max speed and if we can > udma2 then we return 2
-	which selects speed_max as udma4 which is the 5535's max
-	speed, and 1 selects udma2 which is the max for 40c */
-	if (!eighty_ninty_three(drive))
-		return 1;
-
-	return 2;
-}
-
-
 /****
  *	cs5535_set_drive         -     Configure the drive to the new speed
  *	@drive: Drive to set up
@@ -151,7 +137,7 @@ static u8 cs5535_ratemask(ide_drive_t *d
  */
 static int cs5535_set_drive(ide_drive_t *drive, u8 speed)
 {
-	speed = ide_rate_filter(cs5535_ratemask(drive), speed);
+	speed = ide_rate_filter(drive, speed);
 	ide_config_drive_speed(drive, speed);
 	cs5535_set_speed(drive, speed);
 
@@ -180,9 +166,7 @@ static void cs5535_tuneproc(ide_drive_t 
 
 static int cs5535_config_drive_for_dma(ide_drive_t *drive)
 {
-	u8 speed;
-
-	speed = ide_dma_speed(drive, cs5535_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	/* If no DMA speed was available then let dma_check hit pio */
 	if (!speed) {
Index: b/drivers/ide/pci/hpt34x.c
===================================================================
--- a/drivers/ide/pci/hpt34x.c
+++ b/drivers/ide/pci/hpt34x.c
@@ -43,15 +43,10 @@
 
 #define HPT343_DEBUG_DRIVE_INFO		0
 
-static u8 hpt34x_ratemask (ide_drive_t *drive)
-{
-	return 1;
-}
-
 static int hpt34x_tune_chipset (ide_drive_t *drive, u8 xferspeed)
 {
 	struct pci_dev *dev	= HWIF(drive)->pci_dev;
-	u8 speed	= ide_rate_filter(hpt34x_ratemask(drive), xferspeed);
+	u8 speed = ide_rate_filter(drive, xferspeed);
 	u32 reg1= 0, tmp1 = 0, reg2 = 0, tmp2 = 0;
 	u8			hi_speed, lo_speed;
 
@@ -98,7 +93,7 @@ static void hpt34x_tune_drive (ide_drive
 
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, hpt34x_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!(speed))
 		return 0;
Index: b/drivers/ide/pci/hpt366.c
===================================================================
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -513,43 +513,31 @@ static int check_in_drive_list(ide_drive
 	return 0;
 }
 
-static u8 hpt3xx_ratemask(ide_drive_t *drive)
-{
-	struct hpt_info *info	= pci_get_drvdata(HWIF(drive)->pci_dev);
-	u8 mode			= info->max_mode;
-
-	if (!eighty_ninty_three(drive) && mode)
-		mode = min(mode, (u8)1);
-	return mode;
-}
-
 /*
  *	Note for the future; the SATA hpt37x we must set
  *	either PIO or UDMA modes 0,4,5
  */
- 
-static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
+
+static u8 hpt3xx_udma_filter(ide_drive_t *drive)
 {
 	struct hpt_info *info	= pci_get_drvdata(HWIF(drive)->pci_dev);
 	u8 chip_type		= info->chip_type;
-	u8 mode			= hpt3xx_ratemask(drive);
-
-	if (drive->media != ide_disk)
-		return min(speed, (u8)XFER_PIO_4);
+	u8 mode			= info->max_mode;
+	u8 mask;
 
 	switch (mode) {
 		case 0x04:
-			speed = min_t(u8, speed, XFER_UDMA_6);
+			mask = 0x7f;
 			break;
 		case 0x03:
-			speed = min_t(u8, speed, XFER_UDMA_5);
+			mask = 0x3f;
 			if (chip_type >= HPT374)
 				break;
 			if (!check_in_drive_list(drive, bad_ata100_5))
 				goto check_bad_ata33;
 			/* fall thru */
 		case 0x02:
-			speed = min_t(u8, speed, XFER_UDMA_4);
+			mask = 0x1f;
 
 			/*
 			 * CHECK ME, Does this need to be changed to HPT374 ??
@@ -560,13 +548,13 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
 			    !check_in_drive_list(drive, bad_ata66_4))
 				goto check_bad_ata33;
 
-			speed = min_t(u8, speed, XFER_UDMA_3);
+			mask = 0x0f;
 			if (HPT366_ALLOW_ATA66_3 &&
 			    !check_in_drive_list(drive, bad_ata66_3))
 				goto check_bad_ata33;
 			/* fall thru */
 		case 0x01:
-			speed = min_t(u8, speed, XFER_UDMA_2);
+			mask = 0x07;
 
 		check_bad_ata33:
 			if (chip_type >= HPT370A)
@@ -576,10 +564,10 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
 			/* fall thru */
 		case 0x00:
 		default:
-			speed = min_t(u8, speed, XFER_MW_DMA_2);
+			mask = 0x00;
 			break;
 	}
-	return speed;
+	return mask;
 }
 
 static u32 get_speed_setting(u8 speed, struct hpt_info *info)
@@ -607,12 +595,19 @@ static int hpt36x_tune_chipset(ide_drive
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev  *dev	= hwif->pci_dev;
 	struct hpt_info	*info	= pci_get_drvdata(dev);
-	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
+	u8  speed		= ide_rate_filter(drive, xferspeed);
 	u8  itr_addr		= drive->dn ? 0x44 : 0x40;
-	u32 itr_mask		= speed < XFER_MW_DMA_0 ? 0x30070000 :
-				 (speed < XFER_UDMA_0   ? 0xc0070000 : 0xc03800ff);
-	u32 new_itr		= get_speed_setting(speed, info);
 	u32 old_itr		= 0;
+	u32 itr_mask, new_itr;
+
+	/* TODO: move this to ide_rate_filter() [ check ->atapi_dma ] */
+	if (drive->media != ide_disk)
+		speed = min_t(u8, speed, XFER_PIO_4);
+
+	itr_mask = speed < XFER_MW_DMA_0 ? 0x30070000 :
+		  (speed < XFER_UDMA_0   ? 0xc0070000 : 0xc03800ff);
+
+	new_itr = get_speed_setting(speed, info);
 
 	/*
 	 * Disable on-chip PIO FIFO/buffer (and PIO MST mode as well)
@@ -632,12 +627,19 @@ static int hpt37x_tune_chipset(ide_drive
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev  *dev	= hwif->pci_dev;
 	struct hpt_info	*info	= pci_get_drvdata(dev);
-	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
+	u8  speed		= ide_rate_filter(drive, xferspeed);
 	u8  itr_addr		= 0x40 + (drive->dn * 4);
-	u32 itr_mask		= speed < XFER_MW_DMA_0 ? 0x303c0000 :
-				 (speed < XFER_UDMA_0   ? 0xc03c0000 : 0xc1c001ff);
-	u32 new_itr		= get_speed_setting(speed, info);
 	u32 old_itr		= 0;
+	u32 itr_mask, new_itr;
+
+	/* TODO: move this to ide_rate_filter() [ check ->atapi_dma ] */
+	if (drive->media != ide_disk)
+		speed = min_t(u8, speed, XFER_PIO_4);
+
+	itr_mask = speed < XFER_MW_DMA_0 ? 0x303c0000 :
+		  (speed < XFER_UDMA_0   ? 0xc03c0000 : 0xc1c001ff);
+
+	new_itr = get_speed_setting(speed, info);
 
 	pci_read_config_dword(dev, itr_addr, &old_itr);
 	new_itr = (new_itr & ~itr_mask) | (old_itr & itr_mask);
@@ -675,7 +677,7 @@ static void hpt3xx_tune_drive(ide_drive_
  */
 static int config_chipset_for_dma(ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, hpt3xx_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
@@ -1270,6 +1272,7 @@ static void __devinit init_hwif_hpt366(i
 	hwif->intrproc			= &hpt3xx_intrproc;
 	hwif->maskproc			= &hpt3xx_maskproc;
 	hwif->busproc			= &hpt3xx_busproc;
+	hwif->udma_filter		= &hpt3xx_udma_filter;
 
 	/*
 	 * HPT3xxN chips have some complications:
Index: b/drivers/ide/pci/it8213.c
===================================================================
--- a/drivers/ide/pci/it8213.c
+++ b/drivers/ide/pci/it8213.c
@@ -17,22 +17,6 @@
 
 #include <asm/io.h>
 
-/*
- *	it8213_ratemask	-	Compute available modes
- *	@drive: IDE drive
- *
- *	Compute the available speeds for the devices on the interface. This
- *	is all modes to ATA133 clipped by drive cable setup.
- */
-
-static u8 it8213_ratemask (ide_drive_t *drive)
-{
-	u8 mode	= 4;
-	if (!eighty_ninty_three(drive))
-		mode = min_t(u8, mode, 1);
-	return mode;
-}
-
 /**
  *	it8213_dma_2_pio		-	return the PIO mode matching DMA
  *	@xfer_rate: transfer speed
@@ -145,7 +129,7 @@ static int it8213_tune_chipset (ide_driv
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
 	u8 maslave		= 0x40;
-	u8 speed		= ide_rate_filter(it8213_ratemask(drive), xferspeed);
+	u8 speed		= ide_rate_filter(drive, xferspeed);
 	int a_speed		= 3 << (drive->dn * 4);
 	int u_flag		= 1 << drive->dn;
 	int v_flag		= 0x01 << drive->dn;
@@ -222,7 +206,7 @@ static int it8213_tune_chipset (ide_driv
 
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, it8213_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
Index: b/drivers/ide/pci/it821x.c
===================================================================
--- a/drivers/ide/pci/it821x.c
+++ b/drivers/ide/pci/it821x.c
@@ -224,22 +224,6 @@ static void it821x_clock_strategy(ide_dr
 }
 
 /**
- *	it821x_ratemask	-	Compute available modes
- *	@drive: IDE drive
- *
- *	Compute the available speeds for the devices on the interface. This
- *	is all modes to ATA133 clipped by drive cable setup.
- */
-
-static u8 it821x_ratemask (ide_drive_t *drive)
-{
-	u8 mode	= 4;
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
-}
-
-/**
  *	it821x_tuneproc	-	tune a drive
  *	@drive: drive to tune
  *	@mode_wanted: the target operating mode
@@ -448,7 +432,7 @@ static int it821x_tune_chipset (ide_driv
 
 	ide_hwif_t *hwif	= drive->hwif;
 	struct it821x_dev *itdev = ide_get_hwifdata(hwif);
-	u8 speed		= ide_rate_filter(it821x_ratemask(drive), xferspeed);
+	u8 speed		= ide_rate_filter(drive, xferspeed);
 
 	if(!itdev->smart) {
 		switch(speed) {
@@ -496,7 +480,7 @@ static int it821x_tune_chipset (ide_driv
 
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed	= ide_dma_speed(drive, it821x_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (speed) {
 		config_it821x_chipset_for_pio(drive, 0);
Index: b/drivers/ide/pci/jmicron.c
===================================================================
--- a/drivers/ide/pci/jmicron.c
+++ b/drivers/ide/pci/jmicron.c
@@ -22,22 +22,6 @@ typedef enum {
 } port_type;
 
 /**
- *	jmicron_ratemask	-	Compute available modes
- *	@drive: IDE drive
- *
- *	Compute the available speeds for the devices on the interface. This
- *	is all modes to ATA133 clipped by drive cable setup.
- */
-
-static u8 jmicron_ratemask(ide_drive_t *drive)
-{
-	u8 mode	= 4;
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
-}
-
-/**
  *	ata66_jmicron		-	Cable check
  *	@hwif: IDE port
  *
@@ -129,8 +113,7 @@ static void config_jmicron_chipset_for_p
 
 static int jmicron_tune_chipset (ide_drive_t *drive, byte xferspeed)
 {
-
-	u8 speed		= ide_rate_filter(jmicron_ratemask(drive), xferspeed);
+	u8 speed = ide_rate_filter(drive, xferspeed);
 
 	return ide_config_drive_speed(drive, speed);
 }
@@ -145,7 +128,7 @@ static int jmicron_tune_chipset (ide_dri
 
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed	= ide_dma_speed(drive, jmicron_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -82,16 +82,6 @@ static u8 max_dma_rate(struct pci_dev *p
 	return mode;
 }
 
-static u8 pdcnew_ratemask(ide_drive_t *drive)
-{
-	u8 mode = max_dma_rate(HWIF(drive)->pci_dev);
-
-	if (!eighty_ninty_three(drive))
-		mode = min_t(u8, mode, 1);
-
-	return	mode;
-}
-
 /**
  * get_indexed_reg - Get indexed register
  * @hwif: for the port address
@@ -164,7 +154,7 @@ static int pdcnew_tune_chipset(ide_drive
 	u8 adj			= (drive->dn & 1) ? 0x08 : 0x00;
 	int			err;
 
-	speed = ide_rate_filter(pdcnew_ratemask(drive), speed);
+	speed = ide_rate_filter(drive, speed);
 
 	/*
 	 * Issue SETFEATURES_XFER to the drive first. PDC202xx hardware will
@@ -270,7 +260,7 @@ static int config_chipset_for_dma(ide_dr
 		set_indexed_reg(hwif, 0x13 + adj, tmp | 0x03);
 	}
 
-	speed = ide_dma_speed(drive, pdcnew_ratemask(drive));
+	speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -101,35 +101,12 @@ static const char *pdc_quirk_drives[] = 
 #define	MC1		0x02	/* DMA"C" timing */
 #define	MC0		0x01	/* DMA"C" timing */
 
-static u8 pdc202xx_ratemask (ide_drive_t *drive)
-{
-	u8 mode;
-
-	switch(HWIF(drive)->pci_dev->device) {
-		case PCI_DEVICE_ID_PROMISE_20267:
-		case PCI_DEVICE_ID_PROMISE_20265:
-			mode = 3;
-			break;
-		case PCI_DEVICE_ID_PROMISE_20263:
-		case PCI_DEVICE_ID_PROMISE_20262:
-			mode = 2;
-			break;
-		case PCI_DEVICE_ID_PROMISE_20246:
-			return 1;
-		default:
-			return 0;
-	}
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
-}
-
 static int pdc202xx_tune_chipset (ide_drive_t *drive, u8 xferspeed)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
 	u8 drive_pci		= 0x60 + (drive->dn << 2);
-	u8 speed	= ide_rate_filter(pdc202xx_ratemask(drive), xferspeed);
+	u8 speed		= ide_rate_filter(drive, xferspeed);
 
 	u32			drive_conf;
 	u8			AP, BP, CP, DP;
@@ -308,7 +285,7 @@ chipset_is_set:
 	if (drive->media == ide_disk)	/* PREFETCH_EN */
 		pci_write_config_byte(dev, (drive_pci), AP|PREFETCH_EN);
 
-	speed = ide_dma_speed(drive, pdc202xx_ratemask(drive));
+	speed = ide_max_dma_mode(drive);
 
 	if (!(speed)) {
 		/* restore original pci-config space */
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -106,68 +106,6 @@
 static int no_piix_dma;
 
 /**
- *	piix_ratemask		-	compute rate mask for PIIX IDE
- *	@drive: IDE drive to compute for
- *
- *	Returns the available modes for the PIIX IDE controller.
- */
- 
-static u8 piix_ratemask (ide_drive_t *drive)
-{
-	struct pci_dev *dev	= HWIF(drive)->pci_dev;
-	u8 mode;
-
-	switch(dev->device) {
-		case PCI_DEVICE_ID_INTEL_82801EB_1:
-			mode = 3;
-			break;
-		/* UDMA 100 capable */
-		case PCI_DEVICE_ID_INTEL_82801BA_8:
-		case PCI_DEVICE_ID_INTEL_82801BA_9:
-		case PCI_DEVICE_ID_INTEL_82801CA_10:
-		case PCI_DEVICE_ID_INTEL_82801CA_11:
-		case PCI_DEVICE_ID_INTEL_82801E_11:
-		case PCI_DEVICE_ID_INTEL_82801DB_1:
-		case PCI_DEVICE_ID_INTEL_82801DB_10:
-		case PCI_DEVICE_ID_INTEL_82801DB_11:
-		case PCI_DEVICE_ID_INTEL_82801EB_11:
-		case PCI_DEVICE_ID_INTEL_ESB_2:
-		case PCI_DEVICE_ID_INTEL_ICH6_19:
-		case PCI_DEVICE_ID_INTEL_ICH7_21:
-		case PCI_DEVICE_ID_INTEL_ESB2_18:
-		case PCI_DEVICE_ID_INTEL_ICH8_6:
-			mode = 3;
-			break;
-		/* UDMA 66 capable */
-		case PCI_DEVICE_ID_INTEL_82801AA_1:
-		case PCI_DEVICE_ID_INTEL_82372FB_1:
-			mode = 2;
-			break;
-		/* UDMA 33 capable */
-		case PCI_DEVICE_ID_INTEL_82371AB:
-		case PCI_DEVICE_ID_INTEL_82443MX_1:
-		case PCI_DEVICE_ID_INTEL_82451NX:
-		case PCI_DEVICE_ID_INTEL_82801AB_1:
-			return 1;
-		/* Non UDMA capable (MWDMA2) */
-		case PCI_DEVICE_ID_INTEL_82371SB_1:
-		case PCI_DEVICE_ID_INTEL_82371FB_1:
-		case PCI_DEVICE_ID_INTEL_82371FB_0:
-		case PCI_DEVICE_ID_INTEL_82371MX:
-		default:
-			return 0;
-	}
-	
-	/*
-	 *	If we are UDMA66 capable fall back to UDMA33 
-	 *	if the drive cannot see an 80pin cable.
-	 */
-	if (!eighty_ninty_three(drive))
-		mode = min_t(u8, mode, 1);
-	return mode;
-}
-
-/**
  *	piix_dma_2_pio		-	return the PIO mode matching DMA
  *	@xfer_rate: transfer speed
  *
@@ -288,7 +226,7 @@ static int piix_tune_chipset (ide_drive_
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
 	u8 maslave		= hwif->channel ? 0x42 : 0x40;
-	u8 speed		= ide_rate_filter(piix_ratemask(drive), xferspeed);
+	u8 speed		= ide_rate_filter(drive, xferspeed);
 	int a_speed		= 3 << (drive->dn * 4);
 	int u_flag		= 1 << drive->dn;
 	int v_flag		= 0x01 << drive->dn;
@@ -363,7 +301,7 @@ static int piix_tune_chipset (ide_drive_
  
 static int piix_config_drive_for_dma (ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, piix_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	/*
 	 * If no DMA speed was available or the chipset has DMA bugs
Index: b/drivers/ide/pci/serverworks.c
===================================================================
--- a/drivers/ide/pci/serverworks.c
+++ b/drivers/ide/pci/serverworks.c
@@ -65,16 +65,16 @@ static int check_in_drive_lists (ide_dri
 	return 0;
 }
 
-static u8 svwks_ratemask (ide_drive_t *drive)
+static u8 svwks_udma_filter(ide_drive_t *drive)
 {
 	struct pci_dev *dev     = HWIF(drive)->pci_dev;
-	u8 mode = 0;
+	u8 mask = 0;
 
 	if (!svwks_revision)
 		pci_read_config_byte(dev, PCI_REVISION_ID, &svwks_revision);
 
 	if (dev->device == PCI_DEVICE_ID_SERVERWORKS_HT1000IDE)
-		return 2;
+		return 0x1f;
 	if (dev->device == PCI_DEVICE_ID_SERVERWORKS_OSB4IDE) {
 		u32 reg = 0;
 		if (isa_dev)
@@ -86,25 +86,31 @@ static u8 svwks_ratemask (ide_drive_t *d
 		if(drive->media == ide_disk)
 			return 0;
 		/* Check the OSB4 DMA33 enable bit */
-		return ((reg & 0x00004000) == 0x00004000) ? 1 : 0;
+		return ((reg & 0x00004000) == 0x00004000) ? 0x07 : 0;
 	} else if (svwks_revision < SVWKS_CSB5_REVISION_NEW) {
-		return 1;
+		return 0x07;
 	} else if (svwks_revision >= SVWKS_CSB5_REVISION_NEW) {
-		u8 btr = 0;
+		u8 btr = 0, mode;
 		pci_read_config_byte(dev, 0x5A, &btr);
 		mode = btr & 0x3;
-		if (!eighty_ninty_three(drive))
-			mode = min(mode, (u8)1);
+
 		/* If someone decides to do UDMA133 on CSB5 the same
 		   issue will bite so be inclusive */
 		if (mode > 2 && check_in_drive_lists(drive, svwks_bad_ata100))
 			mode = 2;
+
+		switch(mode) {
+		case 2:	 mask = 0x1f; break;
+		case 1:	 mask = 0x07; break;
+		default: mask = 0x00; break;
+		}
 	}
 	if (((dev->device == PCI_DEVICE_ID_SERVERWORKS_CSB6IDE) ||
 	     (dev->device == PCI_DEVICE_ID_SERVERWORKS_CSB6IDE2)) &&
 	    (!(PCI_FUNC(dev->devfn) & 1)))
-		mode = 2;
-	return mode;
+		mask = 0x1f;
+
+	return mask;
 }
 
 static u8 svwks_csb_check (struct pci_dev *dev)
@@ -141,7 +147,7 @@ static int svwks_tune_chipset (ide_drive
 	if (xferspeed == 255)	/* PIO auto-tuning */
 		speed = XFER_PIO_0 + pio;
 	else
-		speed = ide_rate_filter(svwks_ratemask(drive), xferspeed);
+		speed = ide_rate_filter(drive, xferspeed);
 
 	/* If we are about to put a disk into UDMA mode we screwed up.
 	   Our code assumes we never _ever_ do this on an OSB4 */
@@ -304,7 +310,7 @@ static void svwks_tune_drive (ide_drive_
 
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, svwks_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!(speed))
 		speed = XFER_PIO_0 + ide_get_best_pio_mode(drive, 255, 5, NULL);
@@ -500,6 +506,7 @@ static void __devinit init_hwif_svwks (i
 
 	hwif->tuneproc = &svwks_tune_drive;
 	hwif->speedproc = &svwks_tune_chipset;
+	hwif->udma_filter = &svwks_udma_filter;
 
 	hwif->atapi_dma = 1;
 
Index: b/drivers/ide/pci/siimage.c
===================================================================
--- a/drivers/ide/pci/siimage.c
+++ b/drivers/ide/pci/siimage.c
@@ -117,45 +117,41 @@ static inline unsigned long siimage_seld
 }
 
 /**
- *	siimage_ratemask	-	Compute available modes
- *	@drive: IDE drive
+ *	sil_udma_filter		-	compute UDMA mask
+ *	@drive: IDE device
+ *
+ *	Compute the available UDMA speeds for the device on the interface.
  *
- *	Compute the available speeds for the devices on the interface.
  *	For the CMD680 this depends on the clocking mode (scsc), for the
- *	SI3312 SATA controller life is a bit simpler. Enforce UDMA33
- *	as a limit if there is no 80pin cable present.
+ *	SI3112 SATA controller life is a bit simpler.
  */
- 
-static byte siimage_ratemask (ide_drive_t *drive)
+
+static u8 sil_udma_filter(ide_drive_t *drive)
 {
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 mode	= 0, scsc = 0;
+	ide_hwif_t *hwif = drive->hwif;
 	unsigned long base = (unsigned long) hwif->hwif_data;
+	u8 mask = 0, scsc = 0;
 
 	if (hwif->mmio)
 		scsc = hwif->INB(base + 0x4A);
 	else
 		pci_read_config_byte(hwif->pci_dev, 0x8A, &scsc);
 
-	if(is_sata(hwif))
-	{
-		if(strstr(drive->id->model, "Maxtor"))
-			return 3;
-		return 4;
+	if (is_sata(hwif)) {
+		mask = strstr(drive->id->model, "Maxtor") ? 0x3f : 0x7f;
+		goto out;
 	}
-	
+
 	if ((scsc & 0x30) == 0x10)	/* 133 */
-		mode = 4;
+		mask = 0x7f;
 	else if ((scsc & 0x30) == 0x20)	/* 2xPCI */
-		mode = 4;
+		mask = 0x7f;
 	else if ((scsc & 0x30) == 0x00)	/* 100 */
-		mode = 3;
+		mask = 0x3f;
 	else 	/* Disabled ? */
 		BUG();
-
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
+out:
+	return mask;
 }
 
 /**
@@ -306,7 +302,7 @@ static int siimage_tune_chipset (ide_dri
 	ide_hwif_t *hwif	= HWIF(drive);
 	u16 ultra = 0, multi	= 0;
 	u8 mode = 0, unit	= drive->select.b.unit;
-	u8 speed		= ide_rate_filter(siimage_ratemask(drive), xferspeed);
+	u8 speed		= ide_rate_filter(drive, xferspeed);
 	unsigned long base	= (unsigned long)hwif->hwif_data;
 	u8 scsc = 0, addr_mask	= ((hwif->channel) ?
 				    ((hwif->mmio) ? 0xF4 : 0x84) :
@@ -389,7 +385,7 @@ static int siimage_tune_chipset (ide_dri
  
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed	= ide_dma_speed(drive, siimage_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	config_chipset_for_pio(drive, !speed);
 
@@ -991,6 +987,7 @@ static void __devinit init_hwif_siimage(
 	hwif->tuneproc	= &siimage_tuneproc;
 	hwif->reset_poll = &siimage_reset_poll;
 	hwif->pre_reset = &siimage_pre_reset;
+	hwif->udma_filter = &sil_udma_filter;
 
 	if(is_sata(hwif)) {
 		static int first = 1;
Index: b/drivers/ide/pci/sis5513.c
===================================================================
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -428,16 +428,6 @@ static int sis_get_info (char *buffer, c
 }
 #endif /* defined(DISPLAY_SIS_TIMINGS) && defined(CONFIG_PROC_FS) */
 
-static u8 sis5513_ratemask (ide_drive_t *drive)
-{
-	u8 rates[] = { 0, 0, 1, 2, 3, 3, 4, 4 };
-	u8 mode = rates[chipset_family];
-
-	if (!eighty_ninty_three(drive))
-		mode = min(mode, (u8)1);
-	return mode;
-}
-
 /*
  * Configuration functions
  */
@@ -563,7 +553,7 @@ static int sis5513_tune_chipset (ide_dri
 	u8 drive_pci, reg, speed;
 	u32 regdw;
 
-	speed = ide_rate_filter(sis5513_ratemask(drive), xferspeed);
+	speed = ide_rate_filter(drive, xferspeed);
 
 	/* See config_art_rwp_pio for drive pci config registers */
 	drive_pci = 0x40;
@@ -653,7 +643,7 @@ static void sis5513_tune_drive (ide_driv
  */
 static int config_chipset_for_dma (ide_drive_t *drive)
 {
-	u8 speed	= ide_dma_speed(drive, sis5513_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 #ifdef DEBUG
 	printk("SIS5513: config_chipset_for_dma, drive %d, ultra %x\n",
Index: b/drivers/ide/pci/slc90e66.c
===================================================================
--- a/drivers/ide/pci/slc90e66.c
+++ b/drivers/ide/pci/slc90e66.c
@@ -21,15 +21,6 @@
 
 #include <asm/io.h>
 
-static u8 slc90e66_ratemask (ide_drive_t *drive)
-{
-	u8 mode	= 2;
-
-	if (!eighty_ninty_three(drive))
-		mode = min_t(u8, mode, 1);
-	return mode;
-}
-
 static u8 slc90e66_dma_2_pio (u8 xfer_rate) {
 	switch(xfer_rate) {
 		case XFER_UDMA_4:
@@ -119,7 +110,7 @@ static int slc90e66_tune_chipset (ide_dr
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
 	u8 maslave		= hwif->channel ? 0x42 : 0x40;
-	u8 speed	= ide_rate_filter(slc90e66_ratemask(drive), xferspeed);
+	u8 speed		= ide_rate_filter(drive, xferspeed);
 	int sitre = 0, a_speed	= 7 << (drive->dn * 4);
 	int u_speed = 0, u_flag = 1 << drive->dn;
 	u16			reg4042, reg44, reg48, reg4a;
@@ -168,7 +159,7 @@ static int slc90e66_tune_chipset (ide_dr
 
 static int slc90e66_config_drive_for_dma (ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, slc90e66_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
Index: b/drivers/ide/pci/tc86c001.c
===================================================================
--- a/drivers/ide/pci/tc86c001.c
+++ b/drivers/ide/pci/tc86c001.c
@@ -13,18 +13,13 @@
 #include <linux/pci.h>
 #include <linux/ide.h>
 
-static inline u8 tc86c001_ratemask(ide_drive_t *drive)
-{
-	return eighty_ninty_three(drive) ? 2 : 1;
-}
-
 static int tc86c001_tune_chipset(ide_drive_t *drive, u8 speed)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	unsigned long scr_port	= hwif->config_data + (drive->dn ? 0x02 : 0x00);
 	u16 mode, scr		= hwif->INW(scr_port);
 
-	speed = ide_rate_filter(tc86c001_ratemask(drive), speed);
+	speed = ide_rate_filter(drive, speed);
 
 	switch (speed) {
 		case XFER_UDMA_4:	mode = 0x00c0; break;
@@ -174,7 +169,7 @@ static int tc86c001_busproc(ide_drive_t 
 
 static int config_chipset_for_dma(ide_drive_t *drive)
 {
-	u8 speed = ide_dma_speed(drive, tc86c001_ratemask(drive));
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
Index: b/drivers/ide/pci/triflex.c
===================================================================
--- a/drivers/ide/pci/triflex.c
+++ b/drivers/ide/pci/triflex.c
@@ -48,7 +48,7 @@ static int triflex_tune_chipset(ide_driv
 	u16 timing = 0;
 	u32 triflex_timings = 0;
 	u8 unit = (drive->select.b.unit & 0x01);
-	u8 speed = ide_rate_filter(0, xferspeed);
+	u8 speed = ide_rate_filter(drive, xferspeed);
 	
 	pci_read_config_dword(dev, channel_offset, &triflex_timings);
 	
@@ -102,7 +102,7 @@ static void triflex_tune_drive(ide_drive
 
 static int triflex_config_drive_for_dma(ide_drive_t *drive)
 {
-	int speed = ide_dma_speed(drive, 0); /* No ultra speeds */
+	u8 speed = ide_max_dma_mode(drive);
 
 	if (!speed)
 		return 0;
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -717,11 +717,8 @@ typedef struct hwif_s {
 	int	(*quirkproc)(ide_drive_t *);
 	/* driver soft-power interface */
 	int	(*busproc)(ide_drive_t *, int);
-//	/* host rate limiter */
-//	u8	(*ratemask)(ide_drive_t *);
-//	/* device rate limiter */
-//	u8	(*ratefilter)(ide_drive_t *, u8);
 #endif
+	u8 (*udma_filter)(ide_drive_t *);
 
 	void (*ata_input_data)(ide_drive_t *, void *, u32);
 	void (*ata_output_data)(ide_drive_t *, void *, u32);
@@ -1277,6 +1274,7 @@ int ide_in_drive_list(struct hd_driveid 
 int __ide_dma_bad_drive(ide_drive_t *);
 int __ide_dma_good_drive(ide_drive_t *);
 int ide_use_dma(ide_drive_t *);
+u8 ide_max_dma_mode(ide_drive_t *);
 void ide_dma_off(ide_drive_t *);
 void ide_dma_verbose(ide_drive_t *);
 int ide_set_dma(ide_drive_t *);
@@ -1303,6 +1301,7 @@ extern int __ide_dma_timeout(ide_drive_t
 
 #else
 static inline int ide_use_dma(ide_drive_t *drive) { 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) { ; }
 static inline void ide_dma_verbose(ide_drive_t *drive) { ; }
 static inline int ide_set_dma(ide_drive_t *drive) { return 1; }
@@ -1347,8 +1346,7 @@ static inline void ide_set_hwifdata (ide
 }
 
 /* ide-lib.c */
-extern u8 ide_dma_speed(ide_drive_t *drive, u8 mode);
-extern u8 ide_rate_filter(u8 mode, u8 speed); 
+u8 ide_rate_filter(ide_drive_t *, u8);
 extern int ide_dma_enable(ide_drive_t *drive);
 extern char *ide_xfer_verbose(u8 xfer_rate);
 extern void ide_toggle_bounce(ide_drive_t *drive, int on);

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void
  2007-02-03 13:53 ` [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void Bartlomiej Zolnierkiewicz
@ 2007-02-19 22:20   ` Olaf Hering
  2007-02-19 23:21     ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 28+ messages in thread
From: Olaf Hering @ 2007-02-19 22:20 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: Sergei Shtylyov, linux-ide, linux-kernel

On Sat, Feb 03, Bartlomiej Zolnierkiewicz wrote:

> [PATCH] ide: make ide_hwif_t.ide_dma_host_on void

> +++ b/drivers/ide/ppc/pmac.c
> @@ -1984,10 +1984,8 @@ static void pmac_ide_dma_host_off(ide_dr
>  {
>  }
>  
> -static int
> -pmac_ide_dma_host_on (ide_drive_t *drive)
> +static int pmac_ide_dma_host_on(ide_drive_t *drive)
>  {
> -	return 0;
>  }
>  
>  static int

This missed the s/int/void/

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void
  2007-02-19 22:20   ` Olaf Hering
@ 2007-02-19 23:21     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-19 23:21 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Sergei Shtylyov, linux-ide, linux-kernel


On Monday 19 February 2007 23:20, Olaf Hering wrote:
> On Sat, Feb 03, Bartlomiej Zolnierkiewicz wrote:
> 
> > [PATCH] ide: make ide_hwif_t.ide_dma_host_on void
> 
> > +++ b/drivers/ide/ppc/pmac.c
> > @@ -1984,10 +1984,8 @@ static void pmac_ide_dma_host_off(ide_dr
> >  {
> >  }
> >  
> > -static int
> > -pmac_ide_dma_host_on (ide_drive_t *drive)
> > +static int pmac_ide_dma_host_on(ide_drive_t *drive)
> >  {
> > -	return 0;
> >  }
> >  
> >  static int
> 
> This missed the s/int/void/

yes, sorry for that

akpm has fixed this along with other things in pmac driver
and the patch is already in the IDE quilt tree

Bart

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
  2007-02-03 13:53 ` [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode Bartlomiej Zolnierkiewicz
@ 2007-04-19 19:31   ` Sergei Shtylyov
  2007-04-19 19:41     ` Sergei Shtylyov
  0 siblings, 1 reply; 28+ messages in thread
From: Sergei Shtylyov @ 2007-04-19 19:31 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Hello.

Bartlomiej Zolnierkiewicz wrote:
> [PATCH] ide: rework the code for selecting the best DMA transfer mode 

> Depends on the "ide: fix UDMA/MWDMA/SWDMA masks" patch.

   I'm now trying to rewrite hpt366.c to benefit more from these patches...
and alas, this very patch seems to be breaking filtering (at least) in this driver. :-]

> Index: b/drivers/ide/ide-dma.c
> ===================================================================
> --- a/drivers/ide/ide-dma.c
> +++ b/drivers/ide/ide-dma.c
> @@ -705,6 +705,80 @@ int ide_use_dma(ide_drive_t *drive)
>  
>  EXPORT_SYMBOL_GPL(ide_use_dma);
>  
> +static const u8 xfer_mode_bases[] = {
> +	XFER_UDMA_0,
> +	XFER_MW_DMA_0,
> +	XFER_SW_DMA_0,
> +};
> +
> +static unsigned int ide_get_mode_mask(ide_drive_t *drive, u8 base)
> +{
> +	struct hd_driveid *id = drive->id;
> +	ide_hwif_t *hwif = drive->hwif;
> +	unsigned int mask = 0;
> +
> +	switch(base) {
> +	case XFER_UDMA_0:
> +		if ((id->field_valid & 4) == 0)
> +			break;
> +
> +		mask = id->dma_ultra & hwif->ultra_mask;
> +
> +		if (hwif->udma_filter)
> +			mask &= hwif->udma_filter(drive);
> +
> +		if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
> +			mask &= 0x07;

   Note the subtle difference between the old and new behavior: the old driver code was applying UltraDMA filter *after*
the cable type limit, and the new code does it *before*.

> +		break;
> +	case XFER_MW_DMA_0:
> +		mask = id->dma_mword & hwif->mwdma_mask;
> +		break;
> +	case XFER_SW_DMA_0:
> +		mask = id->dma_1word & hwif->swdma_mask;
> +		break;
> +	default:
> +		BUG();
> +		break;
> +	}
> +
> +	return mask;
> +}
> +
> +/**
> + *	ide_max_dma_mode	-	compute DMA speed
> + *	@drive: IDE device
> + *
> + *	Checks the drive capabilities and returns the speed to use
> + *	for the DMA transfer.  Returns 0 if the drive is incapable
> + *	of DMA transfers.
> + */
> +
> +u8 ide_max_dma_mode(ide_drive_t *drive)
> +{
> +	ide_hwif_t *hwif = drive->hwif;
> +	unsigned int mask;
> +	int x, i;
> +	u8 mode = 0;
> +
> +	if (drive->media != ide_disk && hwif->atapi_dma == 0)
> +		return 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(xfer_mode_bases); i++) {
> +		mask = ide_get_mode_mask(drive, xfer_mode_bases[i]);
> +		x = fls(mask) - 1;
> +		if (x >= 0) {
> +			mode = xfer_mode_bases[i] + x;
> +			break;
> +		}
> +	}
> +
> +	printk(KERN_DEBUG "%s: selected mode 0x%x\n", drive->name, mode);
> +
> +	return mode;
> +}
> +
> +EXPORT_SYMBOL_GPL(ide_max_dma_mode);
> +
>  void ide_dma_verbose(ide_drive_t *drive)
>  {
>  	struct hd_driveid *id	= drive->id;
[...]
> Index: b/drivers/ide/pci/hpt366.c
> ===================================================================
> --- a/drivers/ide/pci/hpt366.c
> +++ b/drivers/ide/pci/hpt366.c
> @@ -513,43 +513,31 @@ static int check_in_drive_list(ide_drive
>  	return 0;
>  }
>  
> -static u8 hpt3xx_ratemask(ide_drive_t *drive)
> -{
> -	struct hpt_info *info	= pci_get_drvdata(HWIF(drive)->pci_dev);
> -	u8 mode			= info->max_mode;
> -
> -	if (!eighty_ninty_three(drive) && mode)
> -		mode = min(mode, (u8)1);
> -	return mode;
> -}
> -
>  /*
>   *	Note for the future; the SATA hpt37x we must set
>   *	either PIO or UDMA modes 0,4,5
>   */
> - 
> -static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
> +
> +static u8 hpt3xx_udma_filter(ide_drive_t *drive)
>  {
>  	struct hpt_info *info	= pci_get_drvdata(HWIF(drive)->pci_dev);
>  	u8 chip_type		= info->chip_type;
> -	u8 mode			= hpt3xx_ratemask(drive);
> -
> -	if (drive->media != ide_disk)
> -		return min(speed, (u8)XFER_PIO_4);
> +	u8 mode			= info->max_mode;
> +	u8 mask;
>  
>  	switch (mode) {
>  		case 0x04:
> -			speed = min_t(u8, speed, XFER_UDMA_6);
> +			mask = 0x7f;
>  			break;
>  		case 0x03:
> -			speed = min_t(u8, speed, XFER_UDMA_5);
> +			mask = 0x3f;
>  			if (chip_type >= HPT374)
>  				break;
>  			if (!check_in_drive_list(drive, bad_ata100_5))
>  				goto check_bad_ata33;
>  			/* fall thru */
>  		case 0x02:
> -			speed = min_t(u8, speed, XFER_UDMA_4);
> +			mask = 0x1f;
>  
>  			/*
>  			 * CHECK ME, Does this need to be changed to HPT374 ??
> @@ -560,13 +548,13 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
>  			    !check_in_drive_list(drive, bad_ata66_4))
>  				goto check_bad_ata33;
>  
> -			speed = min_t(u8, speed, XFER_UDMA_3);
> +			mask = 0x0f;
>  			if (HPT366_ALLOW_ATA66_3 &&
>  			    !check_in_drive_list(drive, bad_ata66_3))
>  				goto check_bad_ata33;
>  			/* fall thru */
>  		case 0x01:
> -			speed = min_t(u8, speed, XFER_UDMA_2);
> +			mask = 0x07;
>  
>  		check_bad_ata33:
>  			if (chip_type >= HPT370A)

   This case 0x01 will *never* be hit for HPT370 chip with the new code, therefore the filter won't get applied.

> @@ -576,10 +564,10 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
>  			/* fall thru */
>  		case 0x00:
>  		default:
> -			speed = min_t(u8, speed, XFER_MW_DMA_2);
> +			mask = 0x00;
>  			break;
>  	}
> -	return speed;
> +	return mask;
>  }
[...]
> @@ -1270,6 +1272,7 @@ static void __devinit init_hwif_hpt366(i
>  	hwif->intrproc			= &hpt3xx_intrproc;
>  	hwif->maskproc			= &hpt3xx_maskproc;
>  	hwif->busproc			= &hpt3xx_busproc;
> +	hwif->udma_filter		= &hpt3xx_udma_filter;

   In fact, we only need a filter for HPT36x/370 chips -- I'll address it in my patch.
 
>  	/*
>  	 * HPT3xxN chips have some complications:

WBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
  2007-04-19 19:31   ` Sergei Shtylyov
@ 2007-04-19 19:41     ` Sergei Shtylyov
  2007-04-19 19:46       ` Sergei Shtylyov
  2007-04-20 15:03       ` Sergei Shtylyov
  0 siblings, 2 replies; 28+ messages in thread
From: Sergei Shtylyov @ 2007-04-19 19:41 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
>> [PATCH] ide: rework the code for selecting the best DMA transfer mode 
> 
> 
>> Depends on the "ide: fix UDMA/MWDMA/SWDMA masks" patch.
> 
> 
>   I'm now trying to rewrite hpt366.c to benefit more from these patches...
> and alas, this very patch seems to be breaking filtering (at least) in 
> this driver. :-]
> 
>> Index: b/drivers/ide/ide-dma.c
>> ===================================================================
>> --- a/drivers/ide/ide-dma.c
>> +++ b/drivers/ide/ide-dma.c
>> @@ -705,6 +705,80 @@ int ide_use_dma(ide_drive_t *drive)
>>  
>>  EXPORT_SYMBOL_GPL(ide_use_dma);
>>  
>> +static const u8 xfer_mode_bases[] = {
>> +    XFER_UDMA_0,
>> +    XFER_MW_DMA_0,
>> +    XFER_SW_DMA_0,
>> +};
>> +
>> +static unsigned int ide_get_mode_mask(ide_drive_t *drive, u8 base)
>> +{
>> +    struct hd_driveid *id = drive->id;
>> +    ide_hwif_t *hwif = drive->hwif;
>> +    unsigned int mask = 0;
>> +
>> +    switch(base) {
>> +    case XFER_UDMA_0:
>> +        if ((id->field_valid & 4) == 0)
>> +            break;
>> +
>> +        mask = id->dma_ultra & hwif->ultra_mask;
>> +
>> +        if (hwif->udma_filter)
>> +            mask &= hwif->udma_filter(drive);
>> +
>> +        if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
>> +            mask &= 0x07;
 
>   Note the subtle difference between the old and new behavior: the old 
> driver code was applying UltraDMA filter *after*
> the cable type limit, and the new code does it *before*.

   Was there any particular reason to change that order?

[...]

>> Index: b/drivers/ide/pci/hpt366.c
>> ===================================================================
>> --- a/drivers/ide/pci/hpt366.c
>> +++ b/drivers/ide/pci/hpt366.c
>> @@ -513,43 +513,31 @@ static int check_in_drive_list(ide_drive
>>      return 0;
>>  }
>>  
>> -static u8 hpt3xx_ratemask(ide_drive_t *drive)
>> -{
>> -    struct hpt_info *info    = pci_get_drvdata(HWIF(drive)->pci_dev);
>> -    u8 mode            = info->max_mode;
>> -
>> -    if (!eighty_ninty_three(drive) && mode)
>> -        mode = min(mode, (u8)1);
>> -    return mode;
>> -}
>> -
>>  /*
>>   *    Note for the future; the SATA hpt37x we must set
>>   *    either PIO or UDMA modes 0,4,5
>>   */
>> - -static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
>> +
>> +static u8 hpt3xx_udma_filter(ide_drive_t *drive)
>>  {
>>      struct hpt_info *info    = pci_get_drvdata(HWIF(drive)->pci_dev);
>>      u8 chip_type        = info->chip_type;
>> -    u8 mode            = hpt3xx_ratemask(drive);
>> -
>> -    if (drive->media != ide_disk)
>> -        return min(speed, (u8)XFER_PIO_4);
>> +    u8 mode            = info->max_mode;
>> +    u8 mask;
>>  
>>      switch (mode) {
>>          case 0x04:
>> -            speed = min_t(u8, speed, XFER_UDMA_6);
>> +            mask = 0x7f;
>>              break;
>>          case 0x03:
>> -            speed = min_t(u8, speed, XFER_UDMA_5);
>> +            mask = 0x3f;
>>              if (chip_type >= HPT374)
>>                  break;
>>              if (!check_in_drive_list(drive, bad_ata100_5))
>>                  goto check_bad_ata33;
>>              /* fall thru */
>>          case 0x02:
>> -            speed = min_t(u8, speed, XFER_UDMA_4);
>> +            mask = 0x1f;
>>  
>>              /*
>>               * CHECK ME, Does this need to be changed to HPT374 ??
>> @@ -560,13 +548,13 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
>>                  !check_in_drive_list(drive, bad_ata66_4))
>>                  goto check_bad_ata33;
>>  
>> -            speed = min_t(u8, speed, XFER_UDMA_3);
>> +            mask = 0x0f;
>>              if (HPT366_ALLOW_ATA66_3 &&
>>                  !check_in_drive_list(drive, bad_ata66_3))
>>                  goto check_bad_ata33;
>>              /* fall thru */
>>          case 0x01:
>> -            speed = min_t(u8, speed, XFER_UDMA_2);
>> +            mask = 0x07;
>>  
>>          check_bad_ata33:
>>              if (chip_type >= HPT370A)

>   This case 0x01 will *never* be hit for HPT370 chip with the new code, 
> therefore the filter won't get applied.

   Oh, and for HPT36x chips used with 40c cable too (unless they're artificaially reduced to UltraDMA/33 by the driver #define's).

>> @@ -576,10 +564,10 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
>>              /* fall thru */
>>          case 0x00:
>>          default:
>> -            speed = min_t(u8, speed, XFER_MW_DMA_2);
>> +            mask = 0x00;
>>              break;

   Well, that case 0x00 should never actually happen.

>>      }
>> -    return speed;
>> +    return mask;
>>  }
> 
> [...]
> 
>> @@ -1270,6 +1272,7 @@ static void __devinit init_hwif_hpt366(i
>>      hwif->intrproc            = &hpt3xx_intrproc;
>>      hwif->maskproc            = &hpt3xx_maskproc;
>>      hwif->busproc            = &hpt3xx_busproc;
>> +    hwif->udma_filter        = &hpt3xx_udma_filter;
> 
> 
>   In fact, we only need a filter for HPT36x/370 chips -- I'll address it 
> in my patch.

>>      /*
>>       * HPT3xxN chips have some complications:

MBR, Sergei


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
  2007-04-19 19:41     ` Sergei Shtylyov
@ 2007-04-19 19:46       ` Sergei Shtylyov
  2007-04-20 15:03       ` Sergei Shtylyov
  1 sibling, 0 replies; 28+ messages in thread
From: Sergei Shtylyov @ 2007-04-19 19:46 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello, I wrote:
>> Bartlomiej Zolnierkiewicz wrote:

>>> [PATCH] ide: rework the code for selecting the best DMA transfer mode 

>>> Depends on the "ide: fix UDMA/MWDMA/SWDMA masks" patch.

>>   I'm now trying to rewrite hpt366.c to benefit more from these 
>> patches...
>> and alas, this very patch seems to be breaking filtering (at least) in 
>> this driver. :-]

>>> Index: b/drivers/ide/ide-dma.c
>>> ===================================================================
>>> --- a/drivers/ide/ide-dma.c
>>> +++ b/drivers/ide/ide-dma.c
>>> @@ -705,6 +705,80 @@ int ide_use_dma(ide_drive_t *drive)
>>>  
>>>  EXPORT_SYMBOL_GPL(ide_use_dma);
>>>  
>>> +static const u8 xfer_mode_bases[] = {
>>> +    XFER_UDMA_0,
>>> +    XFER_MW_DMA_0,
>>> +    XFER_SW_DMA_0,
>>> +};
>>> +
>>> +static unsigned int ide_get_mode_mask(ide_drive_t *drive, u8 base)
>>> +{
>>> +    struct hd_driveid *id = drive->id;
>>> +    ide_hwif_t *hwif = drive->hwif;
>>> +    unsigned int mask = 0;
>>> +
>>> +    switch(base) {
>>> +    case XFER_UDMA_0:
>>> +        if ((id->field_valid & 4) == 0)
>>> +            break;
>>> +
>>> +        mask = id->dma_ultra & hwif->ultra_mask;
>>> +
>>> +        if (hwif->udma_filter)
>>> +            mask &= hwif->udma_filter(drive);
>>> +
>>> +        if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
>>> +            mask &= 0x07;
> 
> 
>>   Note the subtle difference between the old and new behavior: the old 
>> driver code was applying UltraDMA filter *after*
>> the cable type limit, and the new code does it *before*.

>   Was there any particular reason to change that order?

   Ah, I see. Cable-reduced mask can't be passed back to driver's filter. This needs changing.

MBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
  2007-04-19 19:41     ` Sergei Shtylyov
  2007-04-19 19:46       ` Sergei Shtylyov
@ 2007-04-20 15:03       ` Sergei Shtylyov
  2007-04-20 18:00         ` Sergei Shtylyov
  1 sibling, 1 reply; 28+ messages in thread
From: Sergei Shtylyov @ 2007-04-20 15:03 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Hello, I wrote:

>>> [PATCH] ide: rework the code for selecting the best DMA transfer mode 

>>> Depends on the "ide: fix UDMA/MWDMA/SWDMA masks" patch.

>>   I'm now trying to rewrite hpt366.c to benefit more from these 
>> patches...
>> and alas, this very patch seems to be breaking filtering (at least) in 
>> this driver. :-]

  Ignore me, I've seemingly misundertood the code. :-<

>>> Index: b/drivers/ide/pci/hpt366.c
>>> ===================================================================
>>> --- a/drivers/ide/pci/hpt366.c
>>> +++ b/drivers/ide/pci/hpt366.c
>>> @@ -513,43 +513,31 @@ static int check_in_drive_list(ide_drive
>>>      return 0;
>>>  }
>>>  
>>> -static u8 hpt3xx_ratemask(ide_drive_t *drive)
>>> -{
>>> -    struct hpt_info *info    = pci_get_drvdata(HWIF(drive)->pci_dev);
>>> -    u8 mode            = info->max_mode;
>>> -
>>> -    if (!eighty_ninty_three(drive) && mode)
>>> -        mode = min(mode, (u8)1);
>>> -    return mode;
>>> -}
>>> -
>>>  /*
>>>   *    Note for the future; the SATA hpt37x we must set
>>>   *    either PIO or UDMA modes 0,4,5
>>>   */
>>> - -static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
>>> +
>>> +static u8 hpt3xx_udma_filter(ide_drive_t *drive)
>>>  {
>>>      struct hpt_info *info    = pci_get_drvdata(HWIF(drive)->pci_dev);
>>>      u8 chip_type        = info->chip_type;
>>> -    u8 mode            = hpt3xx_ratemask(drive);
>>> -
>>> -    if (drive->media != ide_disk)
>>> -        return min(speed, (u8)XFER_PIO_4);
>>> +    u8 mode            = info->max_mode;
>>> +    u8 mask;
>>>  
>>>      switch (mode) {
>>>          case 0x04:
>>> -            speed = min_t(u8, speed, XFER_UDMA_6);
>>> +            mask = 0x7f;
>>>              break;
>>>          case 0x03:
>>> -            speed = min_t(u8, speed, XFER_UDMA_5);
>>> +            mask = 0x3f;
>>>              if (chip_type >= HPT374)
>>>                  break;
>>>              if (!check_in_drive_list(drive, bad_ata100_5))
>>>                  goto check_bad_ata33;
>>>              /* fall thru */
>>>          case 0x02:
>>> -            speed = min_t(u8, speed, XFER_UDMA_4);
>>> +            mask = 0x1f;
>>>  
>>>              /*
>>>               * CHECK ME, Does this need to be changed to HPT374 ??
>>> @@ -560,13 +548,13 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
>>>                  !check_in_drive_list(drive, bad_ata66_4))
>>>                  goto check_bad_ata33;
>>>  
>>> -            speed = min_t(u8, speed, XFER_UDMA_3);
>>> +            mask = 0x0f;
>>>              if (HPT366_ALLOW_ATA66_3 &&
>>>                  !check_in_drive_list(drive, bad_ata66_3))
>>>                  goto check_bad_ata33;
>>>              /* fall thru */
>>>          case 0x01:
>>> -            speed = min_t(u8, speed, XFER_UDMA_2);
>>> +            mask = 0x07;
>>>  
>>>          check_bad_ata33:
>>>              if (chip_type >= HPT370A)

>>   This case 0x01 will *never* be hit for HPT370 chip with the new 
>> code, therefore the filter won't get applied.

>   Oh, and for HPT36x chips used with 40c cable too (unless they're 
> artificaially reduced to UltraDMA/33 by the driver #define's).

   It will still get applied since the code always resorts to looking up the 'bad_ata33' list for HPT36x/370.
I've got a bit muddled in my own code -- not sure if it got much clearer after I'd untangled hpt3xx_ratemask() / hpt3xx_ratefilter() puzzle. :-)

>>> @@ -576,10 +564,10 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
>>>              /* fall thru */
>>>          case 0x00:
>>>          default:
>>> -            speed = min_t(u8, speed, XFER_MW_DMA_2);
>>> +            mask = 0x00;
>>>              break;

>   Well, that case 0x00 should never actually happen.

   Yeah, that's true, 'case 0x00' (and even 'default') labels could have been removed.

MBR, Sergei


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
  2007-04-20 15:03       ` Sergei Shtylyov
@ 2007-04-20 18:00         ` Sergei Shtylyov
  2007-04-23 22:25           ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 28+ messages in thread
From: Sergei Shtylyov @ 2007-04-20 18:00 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Hello, I wrote:

>>>> Index: b/drivers/ide/pci/hpt366.c
>>>> ===================================================================
>>>> --- a/drivers/ide/pci/hpt366.c
>>>> +++ b/drivers/ide/pci/hpt366.c
>>>> @@ -513,43 +513,31 @@ static int check_in_drive_list(ide_drive
>>>>      return 0;
>>>>  }
>>>>  
>>>> -static u8 hpt3xx_ratemask(ide_drive_t *drive)
>>>> -{
>>>> -    struct hpt_info *info    = pci_get_drvdata(HWIF(drive)->pci_dev);
>>>> -    u8 mode            = info->max_mode;
>>>> -
>>>> -    if (!eighty_ninty_three(drive) && mode)
>>>> -        mode = min(mode, (u8)1);
>>>> -    return mode;
>>>> -}
>>>> -
>>>>  /*
>>>>   *    Note for the future; the SATA hpt37x we must set
>>>>   *    either PIO or UDMA modes 0,4,5
>>>>   */
>>>> - -static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
>>>> +
>>>> +static u8 hpt3xx_udma_filter(ide_drive_t *drive)
>>>>  {
>>>>      struct hpt_info *info    = pci_get_drvdata(HWIF(drive)->pci_dev);
>>>>      u8 chip_type        = info->chip_type;
>>>> -    u8 mode            = hpt3xx_ratemask(drive);
>>>> -
>>>> -    if (drive->media != ide_disk)
>>>> -        return min(speed, (u8)XFER_PIO_4);
>>>> +    u8 mode            = info->max_mode;
>>>> +    u8 mask;
>>>>  
>>>>      switch (mode) {
>>>>          case 0x04:
>>>> -            speed = min_t(u8, speed, XFER_UDMA_6);
>>>> +            mask = 0x7f;
>>>>              break;
>>>>          case 0x03:
>>>> -            speed = min_t(u8, speed, XFER_UDMA_5);
>>>> +            mask = 0x3f;
>>>>              if (chip_type >= HPT374)
>>>>                  break;
>>>>              if (!check_in_drive_list(drive, bad_ata100_5))
>>>>                  goto check_bad_ata33;
>>>>              /* fall thru */
>>>>          case 0x02:
>>>> -            speed = min_t(u8, speed, XFER_UDMA_4);
>>>> +            mask = 0x1f;
>>>>  
>>>>              /*
>>>>               * CHECK ME, Does this need to be changed to HPT374 ??
>>>> @@ -560,13 +548,13 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
>>>>                  !check_in_drive_list(drive, bad_ata66_4))
>>>>                  goto check_bad_ata33;
>>>>  
>>>> -            speed = min_t(u8, speed, XFER_UDMA_3);

  Hm, found a buglet in my former filtering rewrite -- the condition in the preceding if stmt should be a reverse one, and speed limitation to XFER_UDMA_3 should have been left under it.  With the current code, XFER_UDMA_2 limitation wouldn't have been applied if the same drive is not in both 'bad_ata66_4' and 'bad_ata66_3' lists -- this, however, actually is not the case since WDC AC310200R drive is in both these lists (maybe I wrote it this way because of this fact :-).

>>>> +            mask = 0x0f;
>>>>              if (HPT366_ALLOW_ATA66_3 &&
>>>>                  !check_in_drive_list(drive, bad_ata66_3))
>>>>                  goto check_bad_ata33;
>>>>              /* fall thru */
>>>>          case 0x01:
>>>> -            speed = min_t(u8, speed, XFER_UDMA_2);
>>>> +            mask = 0x07;
>>>>  
>>>>          check_bad_ata33:
>>>>              if (chip_type >= HPT370A)

>>>   This case 0x01 will *never* be hit for HPT370 chip with the new 
>>> code, therefore the filter won't get applied.

>>   Oh, and for HPT36x chips used with 40c cable too (unless they're 
>> artificaially reduced to UltraDMA/33 by the driver #define's).

>   It will still get applied since the code always resorts to looking up 
> the 'bad_ata33' list for HPT36x/370.
> I've got a bit muddled in my own code -- not sure if it got much clearer 
> after I'd untangled hpt3xx_ratemask() / hpt3xx_ratefilter() puzzle. :-)

  Yeah, I'm definitely having trouble understanding my own code after some months have passed... :-/

MBR, Sergei


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
  2007-04-20 18:00         ` Sergei Shtylyov
@ 2007-04-23 22:25           ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-04-23 22:25 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, linux-kernel


Hi,

On Friday 20 April 2007, Sergei Shtylyov wrote:
> Hello, I wrote:
> 
> >>>> Index: b/drivers/ide/pci/hpt366.c
> >>>> ===================================================================
> >>>> --- a/drivers/ide/pci/hpt366.c
> >>>> +++ b/drivers/ide/pci/hpt366.c
> >>>> @@ -513,43 +513,31 @@ static int check_in_drive_list(ide_drive
> >>>>      return 0;
> >>>>  }
> >>>>  
> >>>> -static u8 hpt3xx_ratemask(ide_drive_t *drive)
> >>>> -{
> >>>> -    struct hpt_info *info    = pci_get_drvdata(HWIF(drive)->pci_dev);
> >>>> -    u8 mode            = info->max_mode;
> >>>> -
> >>>> -    if (!eighty_ninty_three(drive) && mode)
> >>>> -        mode = min(mode, (u8)1);
> >>>> -    return mode;
> >>>> -}
> >>>> -
> >>>>  /*
> >>>>   *    Note for the future; the SATA hpt37x we must set
> >>>>   *    either PIO or UDMA modes 0,4,5
> >>>>   */
> >>>> - -static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
> >>>> +
> >>>> +static u8 hpt3xx_udma_filter(ide_drive_t *drive)
> >>>>  {
> >>>>      struct hpt_info *info    = pci_get_drvdata(HWIF(drive)->pci_dev);
> >>>>      u8 chip_type        = info->chip_type;
> >>>> -    u8 mode            = hpt3xx_ratemask(drive);
> >>>> -
> >>>> -    if (drive->media != ide_disk)
> >>>> -        return min(speed, (u8)XFER_PIO_4);
> >>>> +    u8 mode            = info->max_mode;
> >>>> +    u8 mask;
> >>>>  
> >>>>      switch (mode) {
> >>>>          case 0x04:
> >>>> -            speed = min_t(u8, speed, XFER_UDMA_6);
> >>>> +            mask = 0x7f;
> >>>>              break;
> >>>>          case 0x03:
> >>>> -            speed = min_t(u8, speed, XFER_UDMA_5);
> >>>> +            mask = 0x3f;
> >>>>              if (chip_type >= HPT374)
> >>>>                  break;
> >>>>              if (!check_in_drive_list(drive, bad_ata100_5))
> >>>>                  goto check_bad_ata33;
> >>>>              /* fall thru */
> >>>>          case 0x02:
> >>>> -            speed = min_t(u8, speed, XFER_UDMA_4);
> >>>> +            mask = 0x1f;
> >>>>  
> >>>>              /*
> >>>>               * CHECK ME, Does this need to be changed to HPT374 ??
> >>>> @@ -560,13 +548,13 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
> >>>>                  !check_in_drive_list(drive, bad_ata66_4))
> >>>>                  goto check_bad_ata33;
> >>>>  
> >>>> -            speed = min_t(u8, speed, XFER_UDMA_3);
> 
>   Hm, found a buglet in my former filtering rewrite -- the condition in the preceding if stmt should be a reverse one, and speed limitation to XFER_UDMA_3 should have been left under it.  With the current code, XFER_UDMA_2 limitation wouldn't have been applied if the same drive is not in both 'bad_ata66_4' and 'bad_ata66_3' lists -- this, however, actually is not the case since WDC AC310200R drive is in both these lists (maybe I wrote it this way because of this fact :-).

IIRC I've noticed this during the review of the filtering rewrite
but I though that it was meant to be this way. :)

> >>>> +            mask = 0x0f;
> >>>>              if (HPT366_ALLOW_ATA66_3 &&
> >>>>                  !check_in_drive_list(drive, bad_ata66_3))
> >>>>                  goto check_bad_ata33;
> >>>>              /* fall thru */
> >>>>          case 0x01:
> >>>> -            speed = min_t(u8, speed, XFER_UDMA_2);
> >>>> +            mask = 0x07;
> >>>>  
> >>>>          check_bad_ata33:
> >>>>              if (chip_type >= HPT370A)
> 
> >>>   This case 0x01 will *never* be hit for HPT370 chip with the new 
> >>> code, therefore the filter won't get applied.
> 
> >>   Oh, and for HPT36x chips used with 40c cable too (unless they're 
> >> artificaially reduced to UltraDMA/33 by the driver #define's).
> 
> >   It will still get applied since the code always resorts to looking up 
> > the 'bad_ata33' list for HPT36x/370.
> > I've got a bit muddled in my own code -- not sure if it got much clearer 
> > after I'd untangled hpt3xx_ratemask() / hpt3xx_ratefilter() puzzle. :-)
> 
>   Yeah, I'm definitely having trouble understanding my own code after some months have passed... :-/

The filtering code badly needs more comments/documentation
and it was already true for the old code (before your rewrite).

Bart

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-02-03 17:24 [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
@ 2007-04-22 16:19 ` Sergei Shtylyov
  0 siblings, 0 replies; 28+ messages in thread
From: Sergei Shtylyov @ 2007-04-22 16:19 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Hello.

Bartlomiej Zolnierkiewicz wrote:

> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks

> * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
> * add udma_mask field to ide_pci_device_t and use it to initialize
>   ->ultra_mask in aec62xx, cmd64x, pdc202xx_{new,old} and piix drivers
> * fix UDMA masks to match with chipset specific *_ratemask()
>   (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
>    filtering method - done in the next patch)
> 
> v2:
> * piix: fix cable detection for 82801AA_1 and 82372FB_1
>   [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
> * cmd64x: use hwif->cds->udma_mask
>   [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
> * aec62xx: fix newly introduced bug - check DMA status not command register
>   [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

> v3:
> * piix: use hwif->cds->udma_mask
>   [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

> Index: b/drivers/ide/pci/aec62xx.c
> ===================================================================
> --- a/drivers/ide/pci/aec62xx.c
> +++ b/drivers/ide/pci/aec62xx.c
> @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips
>  
>  static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif)
>  {
> +	struct pci_dev *dev = hwif->pci_dev;
> +
>  	hwif->autodma = 0;
>  	hwif->tuneproc = &aec62xx_tune_drive;
>  	hwif->speedproc = &aec62xx_tune_chipset;
>  
> -	if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
> +	if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
>  		hwif->serialized = hwif->channel;
>  
>  	if (hwif->mate)
> @@ -286,7 +288,15 @@ static void __devinit init_hwif_aec62xx(
>  		return;
>  	}
>  
> -	hwif->ultra_mask = 0x7f;
> +	hwif->ultra_mask = hwif->cds->udma_mask;
> +
> +	/* atp865 and atp865r */
> +	if (hwif->ultra_mask == 0x3f) {
> +		/* check bit 0x10 of DMA status register */
> +		if (inb(pci_resource_start(dev, 4) + 2) & 0x10)
> + 			hwif->ultra_mask = 0x7f; /* udma0-6 */
> +	}
> +
 
   IMO, this asks to be put into init_setup_aec6x80() instead but since I'm already cleaning up that function, I'll move it there myself.

MBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
@ 2007-02-03 17:24 Bartlomiej Zolnierkiewicz
  2007-04-22 16:19 ` Sergei Shtylyov
  0 siblings, 1 reply; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03 17:24 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, Bartlomiej Zolnierkiewicz, linux-kernel

[PATCH] ide: fix UDMA/MWDMA/SWDMA masks

* use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
* add udma_mask field to ide_pci_device_t and use it to initialize
  ->ultra_mask in aec62xx, cmd64x, pdc202xx_{new,old} and piix drivers
* fix UDMA masks to match with chipset specific *_ratemask()
  (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
   filtering method - done in the next patch)

v2:
* piix: fix cable detection for 82801AA_1 and 82372FB_1
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
* cmd64x: use hwif->cds->udma_mask
  [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
* aec62xx: fix newly introduced bug - check DMA status not command register
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

v3:
* piix: use hwif->cds->udma_mask
  [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/ide.c              |    3 -
 drivers/ide/pci/aec62xx.c      |   19 +++++++++-
 drivers/ide/pci/alim15x3.c     |   13 ++++++-
 drivers/ide/pci/cmd64x.c       |   15 ++++----
 drivers/ide/pci/pdc202xx_new.c |    9 ++++-
 drivers/ide/pci/pdc202xx_old.c |    7 +++
 drivers/ide/pci/piix.c         |   73 +++++++++++++++++------------------------
 drivers/ide/pci/sis5513.c      |    5 ++
 include/linux/ide.h            |    1 
 9 files changed, 86 insertions(+), 59 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h
 	hwif->bus_state	= BUSSTATE_ON;
 
 	hwif->atapi_dma = 0;		/* disable all atapi dma */ 
-	hwif->ultra_mask = 0x80;	/* disable all ultra */
-	hwif->mwdma_mask = 0x80;	/* disable all mwdma */
-	hwif->swdma_mask = 0x80;	/* disable all swdma */
 
 	init_completion(&hwif->gendev_rel_comp);
 
Index: b/drivers/ide/pci/aec62xx.c
===================================================================
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -270,11 +270,13 @@ static unsigned int __devinit init_chips
 
 static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif)
 {
+	struct pci_dev *dev = hwif->pci_dev;
+
 	hwif->autodma = 0;
 	hwif->tuneproc = &aec62xx_tune_drive;
 	hwif->speedproc = &aec62xx_tune_chipset;
 
-	if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
+	if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
 		hwif->serialized = hwif->channel;
 
 	if (hwif->mate)
@@ -286,7 +288,15 @@ static void __devinit init_hwif_aec62xx(
 		return;
 	}
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
+
+	/* atp865 and atp865r */
+	if (hwif->ultra_mask == 0x3f) {
+		/* check bit 0x10 of DMA status register */
+		if (inb(pci_resource_start(dev, 4) + 2) & 0x10)
+ 			hwif->ultra_mask = 0x7f; /* udma0-6 */
+	}
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
@@ -354,6 +364,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "AEC6260",
 		.init_setup	= init_setup_aec62xx,
@@ -363,6 +374,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= NOAUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "AEC6260R",
 		.init_setup	= init_setup_aec62xx,
@@ -373,6 +385,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= NEVER_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "AEC6X80",
 		.init_setup	= init_setup_aec6x80,
@@ -382,6 +395,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "AEC6X80R",
 		.init_setup	= init_setup_aec6x80,
@@ -392,6 +406,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/alim15x3.c
===================================================================
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a
 
 	hwif->atapi_dma = 1;
 
-	if (m5229_revision > 0x20)
-		hwif->ultra_mask = 0x7f;
+	if (m5229_revision <= 0x20)
+		hwif->ultra_mask = 0x00; /* no udma */
+	else if (m5229_revision < 0xC2)
+		hwif->ultra_mask = 0x07; /* udma0-2 */
+	else if (m5229_revision == 0xC2 || m5229_revision == 0xC3)
+		hwif->ultra_mask = 0x1f; /* udma0-4 */
+	else if (m5229_revision == 0xC4)
+		hwif->ultra_mask = 0x3f; /* udma0-5 */
+	else
+		hwif->ultra_mask = 0x7f; /* udma0-6 */
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/drivers/ide/pci/cmd64x.c
===================================================================
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -690,16 +690,13 @@ static void __devinit init_hwif_cmd64x(i
 
 	hwif->atapi_dma = 1;
 
-	hwif->ultra_mask = 0x3f;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
-	if (dev->device == PCI_DEVICE_ID_CMD_643)
-		hwif->ultra_mask = 0x80;
-	if (dev->device == PCI_DEVICE_ID_CMD_646)
-		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
-	if (dev->device == PCI_DEVICE_ID_CMD_648)
-		hwif->ultra_mask = 0x1f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
+
+	if (dev->device == PCI_DEVICE_ID_CMD_646 && class_rev < 5)
+		hwif->ultra_mask = 0x00;
 
 	hwif->ide_dma_check = &cmd64x_config_drive_for_dma;
 	if (!(hwif->udma_four))
@@ -733,6 +730,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x00, /* no udma */
 	},{	/* 1 */
 		.name		= "CMD646",
 		.init_chipset	= init_chipset_cmd64x,
@@ -741,6 +739,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x00,0x00,0x00}, {0x51,0x80,0x80}},
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 2 */
 		.name		= "CMD648",
 		.init_chipset	= init_chipset_cmd64x,
@@ -748,6 +747,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "CMD649",
 		.init_chipset	= init_chipset_cmd64x,
@@ -755,6 +755,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -545,7 +545,7 @@ static void __devinit init_hwif_pdc202ne
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 
 	hwif->err_stops_fifo = 1;
@@ -621,6 +621,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 1 */
 		.name		= "PDC20269",
 		.init_setup	= init_setup_pdcnew,
@@ -629,6 +630,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 2 */
 		.name		= "PDC20270",
 		.init_setup	= init_setup_pdc20270,
@@ -637,6 +639,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 3 */
 		.name		= "PDC20271",
 		.init_setup	= init_setup_pdcnew,
@@ -645,6 +648,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 4 */
 		.name		= "PDC20275",
 		.init_setup	= init_setup_pdcnew,
@@ -653,6 +657,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 5 */
 		.name		= "PDC20276",
 		.init_setup	= init_setup_pdc20276,
@@ -661,6 +666,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 6 */
 		.name		= "PDC20277",
 		.init_setup	= init_setup_pdcnew,
@@ -669,6 +675,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -478,7 +478,7 @@ static void __devinit init_hwif_pdc202xx
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x3f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 	hwif->atapi_dma = 1;
@@ -587,6 +587,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 16,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "PDC20262",
 		.init_setup	= init_setup_pdc202ata4,
@@ -597,6 +598,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "PDC20263",
 		.init_setup	= init_setup_pdc202ata4,
@@ -607,6 +609,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "PDC20265",
 		.init_setup	= init_setup_pdc20265,
@@ -617,6 +620,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "PDC20267",
 		.init_setup	= init_setup_pdc202xx,
@@ -627,6 +631,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -514,26 +514,14 @@ static void __devinit init_hwif_piix(ide
 		hwif->ide_dma_clear_irq = &piix_dma_clear_irq;
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x3f;
+
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x06;
 	hwif->swdma_mask = 0x04;
 
-	switch(hwif->pci_dev->device) {
-		case PCI_DEVICE_ID_INTEL_82371FB_0:
-		case PCI_DEVICE_ID_INTEL_82371FB_1:
-		case PCI_DEVICE_ID_INTEL_82371SB_1:
-			hwif->ultra_mask = 0x80;
-			break;
-		case PCI_DEVICE_ID_INTEL_82371AB:
-		case PCI_DEVICE_ID_INTEL_82443MX_1:
-		case PCI_DEVICE_ID_INTEL_82451NX:
-		case PCI_DEVICE_ID_INTEL_82801AB_1:
-			hwif->ultra_mask = 0x07;
-			break;
-		default:
-			if (!hwif->udma_four)
-				hwif->udma_four = piix_cable_detect(hwif);
-			break;
+	if (hwif->ultra_mask & 0x78) {
+		if (!hwif->udma_four)
+			hwif->udma_four = piix_cable_detect(hwif);
 	}
 
 	if (no_piix_dma)
@@ -547,7 +535,7 @@ static void __devinit init_hwif_piix(ide
 	hwif->drives[0].autodma = hwif->autodma;
 }
 
-#define DECLARE_PIIX_DEV(name_str) \
+#define DECLARE_PIIX_DEV(name_str, udma) \
 	{						\
 		.name		= name_str,		\
 		.init_chipset	= init_chipset_piix,	\
@@ -556,11 +544,12 @@ static void __devinit init_hwif_piix(ide
 		.autodma	= AUTODMA,		\
 		.enablebits	= {{0x41,0x80,0x80}, {0x43,0x80,0x80}}, \
 		.bootable	= ON_BOARD,		\
+		.udma_mask	= udma,			\
 	}
 
 static ide_pci_device_t piix_pci_info[] __devinitdata = {
-	/*  0 */ DECLARE_PIIX_DEV("PIIXa"),
-	/*  1 */ DECLARE_PIIX_DEV("PIIXb"),
+	/*  0 */ DECLARE_PIIX_DEV("PIIXa", 0x00),	/* no udma */
+	/*  1 */ DECLARE_PIIX_DEV("PIIXb", 0x00),	/* no udma */
 
 	/*  2 */
 	{	/*
@@ -577,28 +566,28 @@ static ide_pci_device_t piix_pci_info[] 
 		.flags		= IDEPCI_FLAG_ISA_PORTS
 	},
 
-	/*  3 */ DECLARE_PIIX_DEV("PIIX3"),
-	/*  4 */ DECLARE_PIIX_DEV("PIIX4"),
-	/*  5 */ DECLARE_PIIX_DEV("ICH0"),
-	/*  6 */ DECLARE_PIIX_DEV("PIIX4"),
-	/*  7 */ DECLARE_PIIX_DEV("ICH"),
-	/*  8 */ DECLARE_PIIX_DEV("PIIX4"),
-	/*  9 */ DECLARE_PIIX_DEV("PIIX4"),
-	/* 10 */ DECLARE_PIIX_DEV("ICH2"),
-	/* 11 */ DECLARE_PIIX_DEV("ICH2M"),
-	/* 12 */ DECLARE_PIIX_DEV("ICH3M"),
-	/* 13 */ DECLARE_PIIX_DEV("ICH3"),
-	/* 14 */ DECLARE_PIIX_DEV("ICH4"),
-	/* 15 */ DECLARE_PIIX_DEV("ICH5"),
-	/* 16 */ DECLARE_PIIX_DEV("C-ICH"),
-	/* 17 */ DECLARE_PIIX_DEV("ICH4"),
-	/* 18 */ DECLARE_PIIX_DEV("ICH5-SATA"),
-	/* 19 */ DECLARE_PIIX_DEV("ICH5"),
-	/* 20 */ DECLARE_PIIX_DEV("ICH6"),
-	/* 21 */ DECLARE_PIIX_DEV("ICH7"),
-	/* 22 */ DECLARE_PIIX_DEV("ICH4"),
-	/* 23 */ DECLARE_PIIX_DEV("ESB2"),
-	/* 24 */ DECLARE_PIIX_DEV("ICH8M"),
+	/*  3 */ DECLARE_PIIX_DEV("PIIX3", 0x00),	/* no udma */
+	/*  4 */ DECLARE_PIIX_DEV("PIIX4", 0x07),	/* udma0-2 */
+	/*  5 */ DECLARE_PIIX_DEV("ICH0",  0x07),	/* udma0-2 */
+	/*  6 */ DECLARE_PIIX_DEV("PIIX4", 0x07),	/* udma0-2 */
+	/*  7 */ DECLARE_PIIX_DEV("ICH",   0x1f),	/* udma0-4 */
+	/*  8 */ DECLARE_PIIX_DEV("PIIX4", 0x1f),	/* udma0-4 */
+	/*  9 */ DECLARE_PIIX_DEV("PIIX4", 0x07),	/* udma0-2 */
+	/* 10 */ DECLARE_PIIX_DEV("ICH2",  0x3f),	/* udma0-5 */
+	/* 11 */ DECLARE_PIIX_DEV("ICH2M", 0x3f),	/* udma0-5 */
+	/* 12 */ DECLARE_PIIX_DEV("ICH3M", 0x3f),	/* udma0-5 */
+	/* 13 */ DECLARE_PIIX_DEV("ICH3",  0x3f),	/* udma0-5 */
+	/* 14 */ DECLARE_PIIX_DEV("ICH4",  0x3f),	/* udma0-5 */
+	/* 15 */ DECLARE_PIIX_DEV("ICH5",  0x3f),	/* udma0-5 */
+	/* 16 */ DECLARE_PIIX_DEV("C-ICH", 0x3f),	/* udma0-5 */
+	/* 17 */ DECLARE_PIIX_DEV("ICH4",  0x3f),	/* udma0-5 */
+	/* 18 */ DECLARE_PIIX_DEV("ICH5-SATA", 0x3f),	/* udma0-5 */
+	/* 19 */ DECLARE_PIIX_DEV("ICH5",  0x3f),	/* udma0-5 */
+	/* 20 */ DECLARE_PIIX_DEV("ICH6",  0x3f),	/* udma0-5 */
+	/* 21 */ DECLARE_PIIX_DEV("ICH7",  0x3f),	/* udma0-5 */
+	/* 22 */ DECLARE_PIIX_DEV("ICH4",  0x3f),	/* udma0-5 */
+	/* 23 */ DECLARE_PIIX_DEV("ESB2",  0x3f),	/* udma0-5 */
+	/* 24 */ DECLARE_PIIX_DEV("ICH8M", 0x3f),	/* udma0-5 */
 };
 
 /**
Index: b/drivers/ide/pci/sis5513.c
===================================================================
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -858,6 +858,8 @@ static unsigned int __devinit ata66_sis5
 
 static void __devinit init_hwif_sis5513 (ide_hwif_t *hwif)
 {
+	u8 udma_rates[] = { 0x00, 0x00, 0x07, 0x1f, 0x3f, 0x3f, 0x7f, 0x7f };
+
 	hwif->autodma = 0;
 
 	if (!hwif->irq)
@@ -873,7 +875,8 @@ static void __devinit init_hwif_sis5513 
 	}
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x7f;
+
+	hwif->ultra_mask = udma_rates[chipset_family];
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ typedef struct ide_pci_device_s {
 	unsigned int		extra;
 	struct ide_pci_device_s	*next;
 	u8			flags;
+	u8			udma_mask;
 } ide_pci_device_t;
 
 extern int ide_setup_pci_device(struct pci_dev *, ide_pci_device_t *);

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-02-03  0:39       ` Bartlomiej Zolnierkiewicz
@ 2007-02-03 16:30         ` Sergei Shtylyov
  0 siblings, 0 replies; 28+ messages in thread
From: Sergei Shtylyov @ 2007-02-03 16:30 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>Index: b/drivers/ide/pci/cmd64x.c
>>>===================================================================
>>>--- a/drivers/ide/pci/cmd64x.c
>>>+++ b/drivers/ide/pci/cmd64x.c
>>>@@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
>>>      hwif->swdma_mask = 0x07;
>>>
>>>      if (dev->device == PCI_DEVICE_ID_CMD_643)
>>>-             hwif->ultra_mask = 0x80;
>>>+             hwif->ultra_mask = 0x00;
>>>      if (dev->device == PCI_DEVICE_ID_CMD_646)
>>>-             hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
>>>+             hwif->ultra_mask =
>>>+                     (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
>>>      if (dev->device == PCI_DEVICE_ID_CMD_648)
>>>              hwif->ultra_mask = 0x1f;

>>   Hm, well, this doesn't look consistent with the changes in other drivers.
>>This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...
>>   You'd only have to check for PCI-646 revisions < 5 then...

> reworked

    Thanks. :-)

>>>Index: b/drivers/ide/pci/piix.c
>>>===================================================================
>>>--- a/drivers/ide/pci/piix.c
>>>+++ b/drivers/ide/pci/piix.c
>>>              default:
>>>                      if (!hwif->udma_four)
>>>                              hwif->udma_four = piix_cable_detect(hwif);

>>   This one also certainly asks for explicit hwif->cds->ultra_mask
>>initializers... Thus almost all of this switch statement could go away...
  	
> Alas doing it now would make the nice DECLARE_PIIX_DEV() macro go away

    Why? Could add another argument to that macro...

> (=> a lot of duplicated code)... could be done in the future...

    Yes, of course.

> Thanks,
> Bart

MBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
       [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
@ 2007-02-03  0:50       ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03  0:50 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, linux-kernel


Sergei Shtylyov wrote:
> 
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
>> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks
> 
>> * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
>> * add udma_mask field to ide_pci_device_t and use it to initialize
>>   ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
>> * fix UDMA masks to match with chipset specific *_ratemask()
>>   (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
>>    filtering method - done in the next patch)
> 
>    More issues with aec62xx.c driver, found after looking at the next
> patch...
> 
>> Index: b/drivers/ide/pci/aec62xx.c
>> ===================================================================
>> --- a/drivers/ide/pci/aec62xx.c
>> +++ b/drivers/ide/pci/aec62xx.c
>> @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips
>>
>>  static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif)
>>  {
>> +     struct pci_dev *dev = hwif->pci_dev;
>> +
>>       hwif->autodma = 0;
>>       hwif->tuneproc = &aec62xx_tune_drive;
>>       hwif->speedproc = &aec62xx_tune_chipset;
>>
>> -     if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
>> +     if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
>>               hwif->serialized = hwif->channel;
>>
>>       if (hwif->mate)
>> @@ -286,7 +288,16 @@ static void __devinit init_hwif_aec62xx(
>>               return;
>>       }
>>
>> -     hwif->ultra_mask = 0x7f;
>> +     hwif->ultra_mask = hwif->cds->udma_mask;
>> +
>> +     /* atp865 and atp865r */
>> +     if (hwif->ultra_mask == 0x3f) {
>> +             unsigned long io = pci_resource_start(dev, 4);
>> +
>> +             if (inb(io) & 0x10)
>> +                     hwif->ultra_mask = 0x7f; /* udma0-6 */
>> +     }
>> +
> 
>    Looks like another intruduced buglet: you're reading DMA command, but
> aec62xx_ratemask() was reading DMA status originally for this bit.

Doh, fixed.  Thanks for catching this.

>>       hwif->mwdma_mask = 0x07;
>>       hwif->swdma_mask = 0x07;
> 
>    Hm, caught another nit: this driver doesn't actually support single-word
> DMA modes... :-)

added to TODO

>> Index: b/drivers/ide/pci/alim15x3.c
>> ===================================================================
>> --- a/drivers/ide/pci/alim15x3.c
>> +++ b/drivers/ide/pci/alim15x3.c
>> @@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a
>>
>>       hwif->atapi_dma = 1;
>>
>> -     if (m5229_revision > 0x20)
>> -             hwif->ultra_mask = 0x7f;
>> +     if (m5229_revision <= 0x20)
>> +             hwif->ultra_mask = 0x00; /* no udma */
>> +     else if (m5229_revision < 0xC2)
>> +             hwif->ultra_mask = 0x07; /* udma0-2 */
>> +     else if (m5229_revision == 0xC2 || m5229_revision == 0xC3)
>> +             hwif->ultra_mask = 0x1f; /* udma0-4 */
>> +     else if (m5229_revision == 0xC4)
>> +             hwif->ultra_mask = 0x3f; /* udma0-5 */
>> +     else
>> +             hwif->ultra_mask = 0x7f; /* udma0-6 */
>> +
>>       hwif->mwdma_mask = 0x07;
>>       hwif->swdma_mask = 0x07;
> 
>    Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver...
> And PIO setting via speedproc() method is broken -- it passes to tuneproc()
> method mode number not biased by -XFER_PIO_0 beforehand.  Will cook up some
> patch, maybe... :-/

Please do, I added this to IDE TODO to not forget about the issue...

Thanks,
Bart


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
       [not found]     ` <58cb370e0702021606v4efa6143lf060e6aab9782c35@mail.gmail.com>
@ 2007-02-03  0:39       ` Bartlomiej Zolnierkiewicz
  2007-02-03 16:30         ` Sergei Shtylyov
  0 siblings, 1 reply; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03  0:39 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, linux-kernel


Hi,

Sergei Shtylyov wrote:
> 
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
>> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks
> 
>> * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
>> * add udma_mask field to ide_pci_device_t and use it to initialize
>>   ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
>> * fix UDMA masks to match with chipset specific *_ratemask()
>>   (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
>>    filtering method - done in the next patch)
> 
>    More nit picking (-:
> 
>> Index: b/drivers/ide/pci/cmd64x.c
>> ===================================================================
>> --- a/drivers/ide/pci/cmd64x.c
>> +++ b/drivers/ide/pci/cmd64x.c
>> @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
>>       hwif->swdma_mask = 0x07;
>>
>>       if (dev->device == PCI_DEVICE_ID_CMD_643)
>> -             hwif->ultra_mask = 0x80;
>> +             hwif->ultra_mask = 0x00;
>>       if (dev->device == PCI_DEVICE_ID_CMD_646)
>> -             hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
>> +             hwif->ultra_mask =
>> +                     (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
>>       if (dev->device == PCI_DEVICE_ID_CMD_648)
>>               hwif->ultra_mask = 0x1f;
> 
>    Hm, well, this doesn't look consistent with the changes in other drivers.
> This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...
>    You'd only have to check for PCI-646 revisions < 5 then...

reworked

>> Index: b/drivers/ide/pci/piix.c
>> ===================================================================
>> --- a/drivers/ide/pci/piix.c
>> +++ b/drivers/ide/pci/piix.c
>> @@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide
>>               case PCI_DEVICE_ID_INTEL_82371FB_0:
>>               case PCI_DEVICE_ID_INTEL_82371FB_1:
>>               case PCI_DEVICE_ID_INTEL_82371SB_1:
>> -                     hwif->ultra_mask = 0x80;
>> +                     hwif->ultra_mask = 0x00;
>>                       break;
>>               case PCI_DEVICE_ID_INTEL_82371AB:
>>               case PCI_DEVICE_ID_INTEL_82443MX_1:
>> @@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide
>>               case PCI_DEVICE_ID_INTEL_82801AB_1:
>>                       hwif->ultra_mask = 0x07;
>>                       break;
>> +             case PCI_DEVICE_ID_INTEL_82801AA_1:
>> +             case PCI_DEVICE_ID_INTEL_82372FB_1:
>> +                     hwif->ultra_mask = 0x1f;
>> +                     break;
> 
>    Alas, I'm afraid this part is wrong!
>    At least, the cable detection should work for 82801AA the same way as for
> the 82801Bx and newer chips, if Intel's datasheet is to be trusted... I think
> we should fall thru here.

yes (extra "break:" shouldn't be there), fixed

>>               default:
>>                       if (!hwif->udma_four)
>>                               hwif->udma_four = piix_cable_detect(hwif);
> 
>    This one also certainly asks for explicit hwif->cds->ultra_mask
> initializers... Thus almost all of this switch statement could go away...
	
Alas doing it now would make the nice DECLARE_PIIX_DEV() macro go away
(=> a lot of duplicated code)... could be done in the future...

Thanks,
Bart


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-31 20:38       ` Sergei Shtylyov
@ 2007-01-31 23:24         ` Alan
  0 siblings, 0 replies; 28+ messages in thread
From: Alan @ 2007-01-31 23:24 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

> should be slightly overclocking PIO modes 0/1 (ULi docs don't shed much light 
> on how it should be calculated)... Well, this seems fixed in libata drivers.

The libata code for tuning is based upon the BIOS programmers guide. That
seemed to be the best coverage of a minimal selection. It's also got a
big "confidential" stamp on it so I can't pass it on.

Alan

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 18:46     ` Alan
  2007-01-22 20:28       ` Sergei Shtylyov
@ 2007-01-31 20:38       ` Sergei Shtylyov
  2007-01-31 23:24         ` Alan
  1 sibling, 1 reply; 28+ messages in thread
From: Sergei Shtylyov @ 2007-01-31 20:38 UTC (permalink / raw)
  To: Alan; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello.

Alan wrote:

>>    Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 

> Thats long been broken. Should be correct in the libata driver

    I've looked thru the specs and it seemed to me that ULi hardware is much 
broken PIO wise: their max active time is 8 cycles even on taskfile access 
which gives 240 ns while standard requeires 290 ns for modes 0 thru 2...

    I've also noted that the tuneproc() method in both cmd64x.c and alim15x3.c 
seems to misdo recovery calculation, taking address setup into account -- that 
should be slightly overclocking PIO modes 0/1 (ULi docs don't shed much light 
on how it should be calculated)... Well, this seems fixed in libata drivers.

> Alan

MBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-23 14:51           ` Sergei Shtylyov
@ 2007-01-25 15:40             ` Sergei Shtylyov
  0 siblings, 0 replies; 28+ messages in thread
From: Sergei Shtylyov @ 2007-01-25 15:40 UTC (permalink / raw)
  To: Alan; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello.

Sergei Shtylyov wrote:

>> Not a suprise to be honest. I fixed some of the ALi stuff when I did it
>> and I think that was pushed back into drivers/ide. The CMD64x hasn't had
>> much love really.

>    Another buglet found by random glancing at this driver:

> /**
>  *      cmd648_dma_stop -       DMA stop callback
>  *      @qc: Command in progress
>  *
>  *      DMA has completed.
>  */

> static void cmd648_bmdma_stop(struct ata_queued_cmd *qc)
> {
>         struct ata_port *ap = qc->ap;
>         struct pci_dev *pdev = to_pci_dev(ap->host->dev);
>         u8 dma_intr;
>         int dma_reg = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0;
>         int dma_mask = ap->port_no ? ARTTIM2 : CFR;
> 
>         ata_bmdma_stop(qc);
> 
>         pci_read_config_byte(pdev, dma_reg, &dma_intr);
>         pci_write_config_byte(pdev, dma_reg, dma_intr | dma_mask);
> }

>    dma_reg and dma_mask initializers must have been swapped since 
> ARTTIM2 and CFR are regster names.  So, the code reads/writes 
> semi-random regs...

    BTW, on PCI0646U2 and later chips, the interrupt status (it's not really 
DMA interrupt status but a latched INTRQ signal not "coupled" with DMA logic, 
according to the datasheets) can be read from MRDMODE reg. which is accessible 
in I/O space at BMIDE base + 1 which is certainly faster.  That's what 
drivers/ide/cmd64x.c is doing in its test_dma_irq() method (however, it's 
doign this on PCI0643 and early revs of PCI0646 which don't have these bits).
The driver's dma_end() method is acting really strange: it checks if the cjip 
is PCI-648/9 and reads the PCI config space to clear those interrupt bits 
while these chips have them in I/O mapped MRDMODE; OTOH, it ignores these bits 
on earlier chips which have them in oonfig. space only (CFR/ARTTIM23 regs)... 
go figure.  I'm going to clean this up but don't heve the h/w handy... :-/

>> Wouldn't mind the older 64x (not 640) data sheets if they are sharable.

>    Sent what I had on this machine.  Will looks for newer revision of 
> PCJ0646U2 spec elsewhere...

    Sent rev. 1.3... Hopefully gkernel.sourceforge.net/specs/ will be updated.

MBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 21:31         ` Alan
  2007-01-22 22:39           ` Jeff Garzik
@ 2007-01-23 14:51           ` Sergei Shtylyov
  2007-01-25 15:40             ` Sergei Shtylyov
  1 sibling, 1 reply; 28+ messages in thread
From: Sergei Shtylyov @ 2007-01-23 14:51 UTC (permalink / raw)
  To: Alan; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello.

Alan wrote:
>>>>   Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 

>>>Thats long been broken. Should be correct in the libata driver

>>    Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code 
>>from the IDE driver.  No way it could be working.  You may check against the 
>>PC64x datasheets (if you have them -- if you don't I think I may share) and 
>>see for yourself -- it's abolutely idiotic.

    I.e. MWDMA2 should be working due to the way the driver is written (it 
sets up PIO4 timings when auto-tuning DMA) but not the other modes since 
speedproc() method is brain-damaged in this part.

> Not a suprise to be honest. I fixed some of the ALi stuff when I did it
> and I think that was pushed back into drivers/ide. The CMD64x hasn't had
> much love really.

    Another buglet found by random glancing at this driver:

/**
  *      cmd648_dma_stop -       DMA stop callback
  *      @qc: Command in progress
  *
  *      DMA has completed.
  */

static void cmd648_bmdma_stop(struct ata_queued_cmd *qc)
{
         struct ata_port *ap = qc->ap;
         struct pci_dev *pdev = to_pci_dev(ap->host->dev);
         u8 dma_intr;
         int dma_reg = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0;
         int dma_mask = ap->port_no ? ARTTIM2 : CFR;

         ata_bmdma_stop(qc);

         pci_read_config_byte(pdev, dma_reg, &dma_intr);
         pci_write_config_byte(pdev, dma_reg, dma_intr | dma_mask);
}

    dma_reg and dma_mask initializers must have been swapped since ARTTIM2 and 
CFR are regster names.  So, the code reads/writes semi-random regs...

> Wouldn't mind the older 64x (not 640) data sheets if they are sharable.

    Sent what I had on this machine.  Will looks for newer revision of 
PCJ0646U2 spec elsewhere...

MBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 21:31         ` Alan
@ 2007-01-22 22:39           ` Jeff Garzik
  2007-01-23 14:51           ` Sergei Shtylyov
  1 sibling, 0 replies; 28+ messages in thread
From: Jeff Garzik @ 2007-01-22 22:39 UTC (permalink / raw)
  To: Alan; +Cc: Sergei Shtylyov, Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Alan wrote:
> Wouldn't mind the older 64x (not 640) data sheets if they are sharable.


If they are sharable, I would love to archive them at

	http://gkernel.sourceforge.net/specs/

Regards,

	Jeff



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 20:28       ` Sergei Shtylyov
@ 2007-01-22 21:31         ` Alan
  2007-01-22 22:39           ` Jeff Garzik
  2007-01-23 14:51           ` Sergei Shtylyov
  0 siblings, 2 replies; 28+ messages in thread
From: Alan @ 2007-01-22 21:31 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

> >>    Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 
> 
> > Thats long been broken. Should be correct in the libata driver
> 
>     Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code 
> from the IDE driver.  No way it could be working.  You may check against the 
> PC64x datasheets (if you have them -- if you don't I think I may share) and 
> see for yourself -- it's abolutely idiotic.

Not a suprise to be honest. I fixed some of the ALi stuff when I did it
and I think that was pushed back into drivers/ide. The CMD64x hasn't had
much love really.

Wouldn't mind the older 64x (not 640) data sheets if they are sharable.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 18:46     ` Alan
@ 2007-01-22 20:28       ` Sergei Shtylyov
  2007-01-22 21:31         ` Alan
  2007-01-31 20:38       ` Sergei Shtylyov
  1 sibling, 1 reply; 28+ messages in thread
From: Sergei Shtylyov @ 2007-01-22 20:28 UTC (permalink / raw)
  To: Alan; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello.

Alan wrote:

>>    Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 

> Thats long been broken. Should be correct in the libata driver

    Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code 
from the IDE driver.  No way it could be working.  You may check against the 
PC64x datasheets (if you have them -- if you don't I think I may share) and 
see for yourself -- it's abolutely idiotic.

> Alan

WBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 18:17   ` Sergei Shtylyov
@ 2007-01-22 18:46     ` Alan
  2007-01-22 20:28       ` Sergei Shtylyov
  2007-01-31 20:38       ` Sergei Shtylyov
       [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
  1 sibling, 2 replies; 28+ messages in thread
From: Alan @ 2007-01-22 18:46 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

On Mon, 22 Jan 2007 21:17:33 +0300
>     Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 

Thats long been broken. Should be correct in the libata driver

Alan

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-19  0:32 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
  2007-01-22 16:19   ` Sergei Shtylyov
@ 2007-01-22 18:17   ` Sergei Shtylyov
  2007-01-22 18:46     ` Alan
       [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
  1 sibling, 2 replies; 28+ messages in thread
From: Sergei Shtylyov @ 2007-01-22 18:17 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Hello.

Bartlomiej Zolnierkiewicz wrote:

> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks

> * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
> * add udma_mask field to ide_pci_device_t and use it to initialize
>   ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
> * fix UDMA masks to match with chipset specific *_ratemask()
>   (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
>    filtering method - done in the next patch)

    More issues with aec62xx.c driver, found after looking at the next patch...

> Index: b/drivers/ide/pci/aec62xx.c
> ===================================================================
> --- a/drivers/ide/pci/aec62xx.c
> +++ b/drivers/ide/pci/aec62xx.c
> @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips
>  
>  static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif)
>  {
> +	struct pci_dev *dev = hwif->pci_dev;
> +
>  	hwif->autodma = 0;
>  	hwif->tuneproc = &aec62xx_tune_drive;
>  	hwif->speedproc = &aec62xx_tune_chipset;
>  
> -	if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
> +	if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
>  		hwif->serialized = hwif->channel;
>  
>  	if (hwif->mate)
> @@ -286,7 +288,16 @@ static void __devinit init_hwif_aec62xx(
>  		return;
>  	}
>  
> -	hwif->ultra_mask = 0x7f;
> +	hwif->ultra_mask = hwif->cds->udma_mask;
> +
> +	/* atp865 and atp865r */
> +	if (hwif->ultra_mask == 0x3f) {
> +		unsigned long io = pci_resource_start(dev, 4);
> +
> +		if (inb(io) & 0x10)
> + 			hwif->ultra_mask = 0x7f; /* udma0-6 */
> +	}
> +

    Looks like another intruduced buglet: you're reading DMA command, but 
aec62xx_ratemask() was reading DMA status originally for this bit.

>  	hwif->mwdma_mask = 0x07;
>  	hwif->swdma_mask = 0x07;

    Hm, caught another nit: this driver doesn't actually support single-word 
DMA modes... :-)

> Index: b/drivers/ide/pci/alim15x3.c
> ===================================================================
> --- a/drivers/ide/pci/alim15x3.c
> +++ b/drivers/ide/pci/alim15x3.c
> @@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a
>  
>  	hwif->atapi_dma = 1;
>  
> -	if (m5229_revision > 0x20)
> -		hwif->ultra_mask = 0x7f;
> +	if (m5229_revision <= 0x20)
> +		hwif->ultra_mask = 0x00; /* no udma */
> +	else if (m5229_revision < 0xC2)
> +		hwif->ultra_mask = 0x07; /* udma0-2 */
> +	else if (m5229_revision == 0xC2 || m5229_revision == 0xC3)
> +		hwif->ultra_mask = 0x1f; /* udma0-4 */
> +	else if (m5229_revision == 0xC4)
> +		hwif->ultra_mask = 0x3f; /* udma0-5 */
> +	else
> +		hwif->ultra_mask = 0x7f; /* udma0-6 */
> +
>  	hwif->mwdma_mask = 0x07;
>  	hwif->swdma_mask = 0x07;

    Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 
And PIO setting via speedproc() method is broken -- it passes to tuneproc() 
method mode number not biased by -XFER_PIO_0 beforehand.  Will cook up some 
patch, maybe... :-/

MBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-19  0:32 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
@ 2007-01-22 16:19   ` Sergei Shtylyov
       [not found]     ` <58cb370e0702021606v4efa6143lf060e6aab9782c35@mail.gmail.com>
  2007-01-22 18:17   ` Sergei Shtylyov
  1 sibling, 1 reply; 28+ messages in thread
From: Sergei Shtylyov @ 2007-01-22 16:19 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Hello.

Bartlomiej Zolnierkiewicz wrote:

> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks

> * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
> * add udma_mask field to ide_pci_device_t and use it to initialize
>   ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
> * fix UDMA masks to match with chipset specific *_ratemask()
>   (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
>    filtering method - done in the next patch)

    More nit picking (-:

> Index: b/drivers/ide/pci/cmd64x.c
> ===================================================================
> --- a/drivers/ide/pci/cmd64x.c
> +++ b/drivers/ide/pci/cmd64x.c
> @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
>  	hwif->swdma_mask = 0x07;
>  
>  	if (dev->device == PCI_DEVICE_ID_CMD_643)
> -		hwif->ultra_mask = 0x80;
> +		hwif->ultra_mask = 0x00;
>  	if (dev->device == PCI_DEVICE_ID_CMD_646)
> -		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
> +		hwif->ultra_mask =
> +			(class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
>  	if (dev->device == PCI_DEVICE_ID_CMD_648)
>  		hwif->ultra_mask = 0x1f;

    Hm, well, this doesn't look consistent with the changes in other drivers.
This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...
    You'd only have to check for PCI-646 revisions < 5 then...

> Index: b/drivers/ide/pci/piix.c
> ===================================================================
> --- a/drivers/ide/pci/piix.c
> +++ b/drivers/ide/pci/piix.c
> @@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide
>  		case PCI_DEVICE_ID_INTEL_82371FB_0:
>  		case PCI_DEVICE_ID_INTEL_82371FB_1:
>  		case PCI_DEVICE_ID_INTEL_82371SB_1:
> -			hwif->ultra_mask = 0x80;
> +			hwif->ultra_mask = 0x00;
>  			break;
>  		case PCI_DEVICE_ID_INTEL_82371AB:
>  		case PCI_DEVICE_ID_INTEL_82443MX_1:
> @@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide
>  		case PCI_DEVICE_ID_INTEL_82801AB_1:
>  			hwif->ultra_mask = 0x07;
>  			break;
> +		case PCI_DEVICE_ID_INTEL_82801AA_1:
> +		case PCI_DEVICE_ID_INTEL_82372FB_1:
> +			hwif->ultra_mask = 0x1f;
> +			break;

    Alas, I'm afraid this part is wrong!
    At least, the cable detection should work for 82801AA the same way as for 
the 82801Bx and newer chips, if Intel's datasheet is to be trusted... I think 
we should fall thru here.

>  		default:
>  			if (!hwif->udma_four)
>  				hwif->udma_four = piix_cable_detect(hwif);

    This one also certainly asks for explicit hwif->cds->ultra_mask 
initializers... Thus almost all of this switch statement could go away...

MBR, Sergei

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-19  0:30 [PATCH 0/15] IDE quilt tree updated Bartlomiej Zolnierkiewicz
@ 2007-01-19  0:32 ` Bartlomiej Zolnierkiewicz
  2007-01-22 16:19   ` Sergei Shtylyov
  2007-01-22 18:17   ` Sergei Shtylyov
  0 siblings, 2 replies; 28+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-01-19  0:32 UTC (permalink / raw)
  To: linux-ide; +Cc: Bartlomiej Zolnierkiewicz, linux-kernel

[PATCH] ide: fix UDMA/MWDMA/SWDMA masks

* use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
* add udma_mask field to ide_pci_device_t and use it to initialize
  ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
* fix UDMA masks to match with chipset specific *_ratemask()
  (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
   filtering method - done in the next patch)

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/ide.c              |    3 ---
 drivers/ide/pci/aec62xx.c      |   20 ++++++++++++++++++--
 drivers/ide/pci/alim15x3.c     |   13 +++++++++++--
 drivers/ide/pci/cmd64x.c       |    5 +++--
 drivers/ide/pci/pdc202xx_new.c |    9 ++++++++-
 drivers/ide/pci/pdc202xx_old.c |    7 ++++++-
 drivers/ide/pci/piix.c         |    6 +++++-
 drivers/ide/pci/sis5513.c      |    5 ++++-
 include/linux/ide.h            |    1 +
 9 files changed, 56 insertions(+), 13 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h
 	hwif->bus_state	= BUSSTATE_ON;
 
 	hwif->atapi_dma = 0;		/* disable all atapi dma */ 
-	hwif->ultra_mask = 0x80;	/* disable all ultra */
-	hwif->mwdma_mask = 0x80;	/* disable all mwdma */
-	hwif->swdma_mask = 0x80;	/* disable all swdma */
 
 	init_completion(&hwif->gendev_rel_comp);
 
Index: b/drivers/ide/pci/aec62xx.c
===================================================================
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -270,11 +270,13 @@ static unsigned int __devinit init_chips
 
 static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif)
 {
+	struct pci_dev *dev = hwif->pci_dev;
+
 	hwif->autodma = 0;
 	hwif->tuneproc = &aec62xx_tune_drive;
 	hwif->speedproc = &aec62xx_tune_chipset;
 
-	if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
+	if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
 		hwif->serialized = hwif->channel;
 
 	if (hwif->mate)
@@ -286,7 +288,16 @@ static void __devinit init_hwif_aec62xx(
 		return;
 	}
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
+
+	/* atp865 and atp865r */
+	if (hwif->ultra_mask == 0x3f) {
+		unsigned long io = pci_resource_start(dev, 4);
+
+		if (inb(io) & 0x10)
+ 			hwif->ultra_mask = 0x7f; /* udma0-6 */
+	}
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
@@ -354,6 +365,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "AEC6260",
 		.init_setup	= init_setup_aec62xx,
@@ -363,6 +375,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= NOAUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "AEC6260R",
 		.init_setup	= init_setup_aec62xx,
@@ -373,6 +386,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= NEVER_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "AEC6X80",
 		.init_setup	= init_setup_aec6x80,
@@ -382,6 +396,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "AEC6X80R",
 		.init_setup	= init_setup_aec6x80,
@@ -392,6 +407,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/alim15x3.c
===================================================================
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a
 
 	hwif->atapi_dma = 1;
 
-	if (m5229_revision > 0x20)
-		hwif->ultra_mask = 0x7f;
+	if (m5229_revision <= 0x20)
+		hwif->ultra_mask = 0x00; /* no udma */
+	else if (m5229_revision < 0xC2)
+		hwif->ultra_mask = 0x07; /* udma0-2 */
+	else if (m5229_revision == 0xC2 || m5229_revision == 0xC3)
+		hwif->ultra_mask = 0x1f; /* udma0-4 */
+	else if (m5229_revision == 0xC4)
+		hwif->ultra_mask = 0x3f; /* udma0-5 */
+	else
+		hwif->ultra_mask = 0x7f; /* udma0-6 */
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/drivers/ide/pci/cmd64x.c
===================================================================
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
 	hwif->swdma_mask = 0x07;
 
 	if (dev->device == PCI_DEVICE_ID_CMD_643)
-		hwif->ultra_mask = 0x80;
+		hwif->ultra_mask = 0x00;
 	if (dev->device == PCI_DEVICE_ID_CMD_646)
-		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
+		hwif->ultra_mask =
+			(class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
 	if (dev->device == PCI_DEVICE_ID_CMD_648)
 		hwif->ultra_mask = 0x1f;
 
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -545,7 +545,7 @@ static void __devinit init_hwif_pdc202ne
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 
 	hwif->err_stops_fifo = 1;
@@ -621,6 +621,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 1 */
 		.name		= "PDC20269",
 		.init_setup	= init_setup_pdcnew,
@@ -629,6 +630,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 2 */
 		.name		= "PDC20270",
 		.init_setup	= init_setup_pdc20270,
@@ -637,6 +639,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 3 */
 		.name		= "PDC20271",
 		.init_setup	= init_setup_pdcnew,
@@ -645,6 +648,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 4 */
 		.name		= "PDC20275",
 		.init_setup	= init_setup_pdcnew,
@@ -653,6 +657,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 5 */
 		.name		= "PDC20276",
 		.init_setup	= init_setup_pdc20276,
@@ -661,6 +666,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 6 */
 		.name		= "PDC20277",
 		.init_setup	= init_setup_pdcnew,
@@ -669,6 +675,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -488,7 +488,7 @@ static void __devinit init_hwif_pdc202xx
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x3f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 	hwif->atapi_dma = 1;
@@ -597,6 +597,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 16,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "PDC20262",
 		.init_setup	= init_setup_pdc202ata4,
@@ -607,6 +608,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "PDC20263",
 		.init_setup	= init_setup_pdc202ata4,
@@ -617,6 +619,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "PDC20265",
 		.init_setup	= init_setup_pdc20265,
@@ -627,6 +630,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "PDC20267",
 		.init_setup	= init_setup_pdc202xx,
@@ -637,6 +641,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide
 		case PCI_DEVICE_ID_INTEL_82371FB_0:
 		case PCI_DEVICE_ID_INTEL_82371FB_1:
 		case PCI_DEVICE_ID_INTEL_82371SB_1:
-			hwif->ultra_mask = 0x80;
+			hwif->ultra_mask = 0x00;
 			break;
 		case PCI_DEVICE_ID_INTEL_82371AB:
 		case PCI_DEVICE_ID_INTEL_82443MX_1:
@@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide
 		case PCI_DEVICE_ID_INTEL_82801AB_1:
 			hwif->ultra_mask = 0x07;
 			break;
+		case PCI_DEVICE_ID_INTEL_82801AA_1:
+		case PCI_DEVICE_ID_INTEL_82372FB_1:
+			hwif->ultra_mask = 0x1f;
+			break;
 		default:
 			if (!hwif->udma_four)
 				hwif->udma_four = piix_cable_detect(hwif);
Index: b/drivers/ide/pci/sis5513.c
===================================================================
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -858,6 +858,8 @@ static unsigned int __devinit ata66_sis5
 
 static void __devinit init_hwif_sis5513 (ide_hwif_t *hwif)
 {
+	u8 udma_rates[] = { 0x00, 0x00, 0x07, 0x1f, 0x3f, 0x3f, 0x7f, 0x7f };
+
 	hwif->autodma = 0;
 
 	if (!hwif->irq)
@@ -873,7 +875,8 @@ static void __devinit init_hwif_sis5513 
 	}
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x7f;
+
+	hwif->ultra_mask = udma_rates[chipset_family];
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ typedef struct ide_pci_device_s {
 	unsigned int		extra;
 	struct ide_pci_device_s	*next;
 	u8			flags;
+	u8			udma_mask;
 } ide_pci_device_t;
 
 extern int ide_setup_pci_device(struct pci_dev *, ide_pci_device_t *);

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2007-04-23 22:30 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-03 13:53 [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
2007-02-03 13:53 ` [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void Bartlomiej Zolnierkiewicz
2007-02-19 22:20   ` Olaf Hering
2007-02-19 23:21     ` Bartlomiej Zolnierkiewicz
2007-02-03 13:53 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-02-03 13:53 ` [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode Bartlomiej Zolnierkiewicz
2007-04-19 19:31   ` Sergei Shtylyov
2007-04-19 19:41     ` Sergei Shtylyov
2007-04-19 19:46       ` Sergei Shtylyov
2007-04-20 15:03       ` Sergei Shtylyov
2007-04-20 18:00         ` Sergei Shtylyov
2007-04-23 22:25           ` Bartlomiej Zolnierkiewicz
  -- strict thread matches above, loose matches on Subject: below --
2007-02-03 17:24 [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-04-22 16:19 ` Sergei Shtylyov
2007-01-19  0:30 [PATCH 0/15] IDE quilt tree updated Bartlomiej Zolnierkiewicz
2007-01-19  0:32 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-01-22 16:19   ` Sergei Shtylyov
     [not found]     ` <58cb370e0702021606v4efa6143lf060e6aab9782c35@mail.gmail.com>
2007-02-03  0:39       ` Bartlomiej Zolnierkiewicz
2007-02-03 16:30         ` Sergei Shtylyov
2007-01-22 18:17   ` Sergei Shtylyov
2007-01-22 18:46     ` Alan
2007-01-22 20:28       ` Sergei Shtylyov
2007-01-22 21:31         ` Alan
2007-01-22 22:39           ` Jeff Garzik
2007-01-23 14:51           ` Sergei Shtylyov
2007-01-25 15:40             ` Sergei Shtylyov
2007-01-31 20:38       ` Sergei Shtylyov
2007-01-31 23:24         ` Alan
     [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
2007-02-03  0:50       ` Bartlomiej Zolnierkiewicz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).