[PATCH 6/15] atiixp/jmicron/triflex: fix PIO fallback

Previous thread: Re: SATA exceptions with 2.6.20-rc5 by Robert Hancock on Thursday, January 18, 2007 - 5:09 pm. (2 messages)

Next thread: none
From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:30 pm

Hi,

I've just updated IDE quilt tree:
	http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/patches/


New patches:

* IDE driver for Delkin/Lexar/ASKA/Workbit/etc. CardBus CF adapters
  (Mark Lord <mlord@pobox.com>)

* ACPI support for IDE devices
  (Hannes Reinecke <hare@suse.de>)

* ide: unregister ide-pnp driver on unload
  (Tejun Heo <htejun@gmail.com>)

* via82cxxx/pata_via: correct PCI_DEVICE_ID_VIA_SATA_EIDE ID and add support
  for CX700 and 8237S
  (Josepch Chan <josephchan@via.com.tw>)

* rework of the code selecting the best DMA transfer mode (~500 LOCs less)
  (me)

* some misc fixes/cleanups (me)

diffstat:
 68 files changed, 3310 insertions(+), 2495 deletions(-)


I'm sending only new patches for review/comments.

If you would like to see the full quilt series (or to get combined patch)
against 2.6.20-rc5, they are available here:

       http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/releases/

Thanks,
Bart
-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:31 pm

[PATCH] ACPI support for IDE devices

This patch implements ACPI integration for generic IDE devices.
The ACPI spec mandates that some methods are called during suspend and
resume. And consequently there most modern Laptops cannot resume
properly without it.

According to the spec, we should call '_GTM' (Get Timing) upon suspend
to store the current IDE adapter settings.
Upon resume we should call '_STM' (Set Timing) to initialize the
adapter with the stored settings; afterwards '_GTF' (Get Taskfile)
should be called which returns a buffer with some IDE initialisation
commands. Those commands should be passed to the drive.

There are two module params which control the behaviour of this patch:

'ide=noacpi'
	Do not call any ACPI methods (Disables any ACPI method calls)
'ide=acpigtf'
	Enable execution of _GTF methods upon resume.
	Has no effect if 'ide=noacpi' is set.
'ide=acpionboot'
	Enable execution of ACPI methods during boot.
	This might be required on some machines if 'ide=acpigtf' is
	selected as some machines modify the _GTF information
	depending on the drive identification passed down with _STM.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/Kconfig     |    7 
 drivers/ide/Makefile    |    1 
 drivers/ide/ide-acpi.c  |  696 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/ide/ide-probe.c |    3 
 drivers/ide/ide.c       |   36 ++
 include/linux/ide.h     |   27 +
 6 files changed, 770 insertions(+)

Index: b/drivers/ide/Kconfig
===================================================================
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -271,6 +271,13 @@ config BLK_DEV_IDESCSI
 	  If both this SCSI emulation and native ATAPI support are compiled
 	  into the kernel, the native support will be used.
 
+config BLK_DEV_IDEACPI
+	bool "IDE ACPI support"
+	depends on ACPI
+	---help---
+	  Implement ACPI support for generic IDE devices. On modern
+	  ...
From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:31 pm

[PATCH] via82cxxx/pata_via: correct PCI_DEVICE_ID_VIA_SATA_EIDE ID and add support for CX700 and 8237S

This patch:
* Corrects the wrong device ID of PCI_DEVICE_ID_VIA_SATA_EIDE
  from 0x0581 to 0x5324.
* Adds VIA CX700 and VT8237S support in drivers/ide/pci/via82cxxx.c
* Adds VIA VT8237S support in drivers/ata/pata_via.c

Signed-off-by: Josepch Chan <josephchan@via.com.tw>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ata/pata_via.c      |    1 +
 drivers/ide/pci/via82cxxx.c |    3 +++
 include/linux/pci_ids.h     |    3 ++-
 3 files changed, 6 insertions(+), 1 deletion(-)

Index: b/drivers/ata/pata_via.c
===================================================================
--- a/drivers/ata/pata_via.c
+++ b/drivers/ata/pata_via.c
@@ -95,6 +95,7 @@ static const struct via_isa_bridge {
 	u8 rev_max;
 	u16 flags;
 } via_isa_bridges[] = {
+	{ "vt8237s",	PCI_DEVICE_ID_VIA_8237S,    0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
 	{ "vt8251",	PCI_DEVICE_ID_VIA_8251,     0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
 	{ "cx700",	PCI_DEVICE_ID_VIA_CX700,    0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
 	{ "vt6410",	PCI_DEVICE_ID_VIA_6410,     0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST | VIA_NO_ENABLES},
Index: b/drivers/ide/pci/via82cxxx.c
===================================================================
--- a/drivers/ide/pci/via82cxxx.c
+++ b/drivers/ide/pci/via82cxxx.c
@@ -78,6 +78,8 @@ static struct via_isa_bridge {
 	u8 rev_max;
 	u16 flags;
 } via_isa_bridges[] = {
+	{ "cx7000",	PCI_DEVICE_ID_VIA_CX700,     0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
+	{ "vt8237s",	PCI_DEVICE_ID_VIA_8237S,    0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
 	{ "vt6410",	PCI_DEVICE_ID_VIA_6410,     0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
 	{ "vt8251",	PCI_DEVICE_ID_VIA_8251,     0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
 	{ "vt8237",	PCI_DEVICE_ID_VIA_8237,     0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
@@ -504,6 +506,7 @@ static struct pci_device_id via_pci_tbl[
 	{ ...
From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:31 pm

[PATCH] it8213: fix build and ->ultra_mask

* PCI_DEVICE_ID_ITE_8213 is only defined in -mm kernels,
  so just use PCI Device ID (0x8213) directly
* fix ->ultra_mask to indicate UDMA6 support

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

---
 drivers/ide/pci/it8213.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: b/drivers/ide/pci/it8213.c
===================================================================
--- a/drivers/ide/pci/it8213.c
+++ b/drivers/ide/pci/it8213.c
@@ -282,7 +282,7 @@ static void __devinit init_hwif_it8213(i
 		return;
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x3f;
+	hwif->ultra_mask = 0x7f;
 	hwif->mwdma_mask = 0x06;
 	hwif->swdma_mask = 0x04;
 
@@ -338,7 +338,7 @@ static int __devinit it8213_init_one(str
 
 
 static struct pci_device_id it8213_pci_tbl[] = {
-	{ PCI_VENDOR_ID_ITE, PCI_DEVICE_ID_ITE_8213,  PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+	{ PCI_VENDOR_ID_ITE, 0x8213, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
 	{ 0, },
 };
 
-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:31 pm

[PATCH] ide: convert ide_hwif_t.mmio into flag

All users of ->mmio == 1 are gone so convert ->mmio into flag.

Noticed by Alan Cox.

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

---
 drivers/ide/arm/icside.c      |    2 +-
 drivers/ide/arm/rapide.c      |    2 +-
 drivers/ide/cris/ide-cris.c   |    2 +-
 drivers/ide/h8300/ide-h8300.c |    2 +-
 drivers/ide/ide-dma.c         |    8 ++++----
 drivers/ide/ide.c             |    5 ++---
 drivers/ide/legacy/buddha.c   |    2 +-
 drivers/ide/legacy/gayle.c    |    2 +-
 drivers/ide/legacy/macide.c   |    2 +-
 drivers/ide/legacy/q40ide.c   |    2 +-
 drivers/ide/mips/au1xxx-ide.c |    3 ++-
 drivers/ide/mips/swarm.c      |    2 +-
 drivers/ide/pci/sgiioc4.c     |    2 +-
 drivers/ide/pci/siimage.c     |    3 ++-
 drivers/ide/ppc/pmac.c        |    2 +-
 include/linux/ide.h           |    2 +-
 16 files changed, 22 insertions(+), 21 deletions(-)

Index: b/drivers/ide/arm/icside.c
===================================================================
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -556,7 +556,7 @@ icside_setup(void __iomem *base, struct 
 		 * Ensure we're using MMIO
 		 */
 		default_hwif_mmiops(hwif);
-		hwif->mmio = 2;
+		hwif->mmio = 1;
 
 		for (i = IDE_DATA_OFFSET; i <= IDE_STATUS_OFFSET; i++) {
 			hwif->hw.io_ports[i] = port;
Index: b/drivers/ide/arm/rapide.c
===================================================================
--- a/drivers/ide/arm/rapide.c
+++ b/drivers/ide/arm/rapide.c
@@ -46,7 +46,7 @@ rapide_locate_hwif(void __iomem *base, v
 	hwif->hw.io_ports[IDE_CONTROL_OFFSET] = (unsigned long)ctrl;
 	hwif->io_ports[IDE_CONTROL_OFFSET] = (unsigned long)ctrl;
 	hwif->hw.irq = hwif->irq = irq;
-	hwif->mmio = 2;
+	hwif->mmio = 1;
 	default_hwif_mmiops(hwif);
 
 	return hwif;
Index: b/drivers/ide/cris/ide-cris.c
===================================================================
--- a/drivers/ide/cris/ide-cris.c
+++ b/drivers/ide/cris/ide-cris.c
@@ ...
From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:31 pm

[PATCH] hpt34x: hpt34x_tune_chipset() (->speedproc) fix

* remember to clear reg2 bits for the current device before setting mode
* remove no longer needed hpt34x_clear_chipset()

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

---
 drivers/ide/pci/hpt34x.c |   17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

Index: b/drivers/ide/pci/hpt34x.c
===================================================================
--- a/drivers/ide/pci/hpt34x.c
+++ b/drivers/ide/pci/hpt34x.c
@@ -48,19 +48,6 @@ static u8 hpt34x_ratemask (ide_drive_t *
 	return 1;
 }
 
-static void hpt34x_clear_chipset (ide_drive_t *drive)
-{
-	struct pci_dev *dev	= HWIF(drive)->pci_dev;
-	u32 reg1 = 0, tmp1 = 0, reg2 = 0, tmp2 = 0;
-
-	pci_read_config_dword(dev, 0x44, &reg1);
-	pci_read_config_dword(dev, 0x48, &reg2);
-	tmp1 = ((0x00 << (3*drive->dn)) | (reg1 & ~(7 << (3*drive->dn))));
-	tmp2 = (reg2 & ~(0x11 << drive->dn));
-	pci_write_config_dword(dev, 0x44, tmp1);
-	pci_write_config_dword(dev, 0x48, tmp2);
-}
-
 static int hpt34x_tune_chipset (ide_drive_t *drive, u8 xferspeed)
 {
 	struct pci_dev *dev	= HWIF(drive)->pci_dev;
@@ -81,7 +68,7 @@ static int hpt34x_tune_chipset (ide_driv
 	pci_read_config_dword(dev, 0x44, &reg1);
 	pci_read_config_dword(dev, 0x48, &reg2);
 	tmp1 = ((lo_speed << (3*drive->dn)) | (reg1 & ~(7 << (3*drive->dn))));
-	tmp2 = ((hi_speed << drive->dn) | reg2);
+	tmp2 = ((hi_speed << drive->dn) | (reg2 & ~(0x11 << drive->dn)));
 	pci_write_config_dword(dev, 0x44, tmp1);
 	pci_write_config_dword(dev, 0x48, tmp2);
 
@@ -99,7 +86,6 @@ static int hpt34x_tune_chipset (ide_driv
 static void hpt34x_tune_drive (ide_drive_t *drive, u8 pio)
 {
 	pio = ide_get_best_pio_mode(drive, pio, 5, NULL);
-	hpt34x_clear_chipset(drive);
 	(void) hpt34x_tune_chipset(drive, (XFER_PIO_0 + pio));
 }
 
@@ -117,7 +103,6 @@ static int config_chipset_for_dma (ide_d
 	if (!(speed))
 		return 0;
 
-	hpt34x_clear_chipset(drive);
 	(void) hpt34x_tune_chipset(drive, ...
From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:31 pm

[PATCH] atiixp/jmicron/triflex: fix PIO fallback

* atiixp: if DMA can't be used atiixp_config_drive_for_dma() should return 0,
  atiixp_dma_check() will tune the correct PIO mode anyway

* jmicron: if DMA can't be used config_chipset_for_dma() should return 0,
  micron_config_drive_for_dma() will tune the correct PIO mode anyway

  config_jmicron_chipset_for_pio(drive, !speed) doesn't program
  device transfer mode for speed != 0 (only wastes some CPU cycles
  on ide_get_best_pio_mode() call) so remove it

* triflex: if DMA can't be used triflex_config_drive_for_dma() should return 0,
  triflex_config_drive_xfer_rate() will tune correct PIO mode anyway

Above changes also fix (theoretical) issue when ->speedproc fails to set
device transfer mode (i.e. when ide_config_drive_speed() fails to program it)
but one of DMA transfer modes is already enabled on the device by the BIOS.
In such scenario ide_dma_enable() will incorrectly return true statement
and ->ide_dma_check will try to enable DMA on the device.

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

---
 drivers/ide/pci/atiixp.c  |    7 ++-----
 drivers/ide/pci/jmicron.c |    4 +++-
 drivers/ide/pci/triflex.c |    8 +++-----
 3 files changed, 8 insertions(+), 11 deletions(-)

Index: b/drivers/ide/pci/atiixp.c
===================================================================
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -235,11 +235,8 @@ static int atiixp_config_drive_for_dma(i
 {
 	u8 speed = ide_dma_speed(drive, atiixp_ratemask(drive));
 
-	/* If no DMA speed was available then disable DMA and use PIO. */
-	if (!speed) {
-		u8 tspeed = ide_get_best_pio_mode(drive, 255, 5, NULL);
-		speed = atiixp_dma_2_pio(XFER_PIO_0 + tspeed) + XFER_PIO_0;
-	}
+	if (!speed)
+		return 0;
 
 	(void) atiixp_speedproc(drive, speed);
 	return ide_dma_enable(drive);
Index: b/drivers/ide/pci/jmicron.c
===================================================================
--- ...
From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:31 pm

[PATCH] piix: cleanup

* disable DMA masks if no_piix_dma is set and remove now
  not needed no_piix_dma_check from piix_config_drive_for_dma()
* there is no need to read register 0x55 in init_hwif_piix()
* move cable detection code to piix_cable_detect()
* remove unreachable 82371MX code from init_hwif_piix()

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

---
 drivers/ide/pci/piix.c |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -369,7 +369,7 @@ static int piix_config_drive_for_dma (id
 	 * If no DMA speed was available or the chipset has DMA bugs
 	 * then disable DMA and use PIO
 	 */
-	if (!speed || no_piix_dma)
+	if (!speed)
 		return 0;
 
 	(void) piix_tune_chipset(drive, speed);
@@ -444,6 +444,16 @@ static unsigned int __devinit init_chips
 	return 0;
 }
 
+static int __devinit piix_cable_detect(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev = hwif->pci_dev;
+	u8 reg54h = 0, mask = hwif->channel ? 0xc0 : 0x30;
+
+	pci_read_config_byte(dev, 0x54, &reg54h);
+
+	return (reg54h & mask) ? 1 : 0;
+}
+
 /**
  *	init_hwif_piix		-	fill in the hwif for the PIIX
  *	@hwif: IDE interface
@@ -454,9 +464,6 @@ static unsigned int __devinit init_chips
 
 static void __devinit init_hwif_piix(ide_hwif_t *hwif)
 {
-	u8 reg54h = 0, reg55h = 0, ata66 = 0;
-	u8 mask = hwif->channel ? 0xc0 : 0x30;
-
 #ifndef CONFIG_IA64
 	if (!hwif->irq)
 		hwif->irq = hwif->channel ? 15 : 14;
@@ -486,9 +493,6 @@ static void __devinit init_hwif_piix(ide
 	hwif->swdma_mask = 0x04;
 
 	switch(hwif->pci_dev->device) {
-		case PCI_DEVICE_ID_INTEL_82371MX:
-			hwif->mwdma_mask = 0x80;
-			hwif->swdma_mask = 0x80;
 		case PCI_DEVICE_ID_INTEL_82371FB_0:
 		case PCI_DEVICE_ID_INTEL_82371FB_1:
 		case PCI_DEVICE_ID_INTEL_82371SB_1:
@@ -501,14 +505,14 @@ static void __devinit ...
From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:31 pm

[PATCH] ide: disable DMA in ->ide_dma_check for "no IORDY" case

If DMA is unsupported ->ide_dma_check should disable DMA.

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

---
 drivers/ide/pci/aec62xx.c      |    8 +++-----
 drivers/ide/pci/atiixp.c       |    5 ++---
 drivers/ide/pci/cmd64x.c       |    8 +++-----
 drivers/ide/pci/cs5535.c       |    5 ++---
 drivers/ide/pci/hpt34x.c       |    8 +++-----
 drivers/ide/pci/hpt366.c       |    8 +++-----
 drivers/ide/pci/pdc202xx_new.c |    8 +++-----
 drivers/ide/pci/pdc202xx_old.c |    8 +++-----
 drivers/ide/pci/piix.c         |    5 ++---
 drivers/ide/pci/serverworks.c  |    9 +++------
 drivers/ide/pci/siimage.c      |    8 +++-----
 drivers/ide/pci/sis5513.c      |    8 +++-----
 drivers/ide/pci/slc90e66.c     |    5 ++---
 drivers/ide/pci/tc86c001.c     |    8 +++-----
 14 files changed, 38 insertions(+), 63 deletions(-)

Index: b/drivers/ide/pci/aec62xx.c
===================================================================
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -214,12 +214,10 @@ static int aec62xx_config_drive_xfer_rat
 	if (ide_use_dma(drive) && config_chipset_for_dma(drive))
 		return hwif->ide_dma_on(drive);
 
-	if (ide_use_fast_pio(drive)) {
+	if (ide_use_fast_pio(drive))
 		aec62xx_tune_drive(drive, 5);
-		return hwif->ide_dma_off_quietly(drive);
-	}
-	/* IORDY not supported */
-	return 0;
+
+	return hwif->ide_dma_off_quietly(drive);
 }
 
 static int aec62xx_irq_timeout (ide_drive_t *drive)
Index: b/drivers/ide/pci/atiixp.c
===================================================================
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -264,10 +264,9 @@ static int atiixp_dma_check(ide_drive_t 
 		tspeed = ide_get_best_pio_mode(drive, 255, 5, NULL);
 		speed = atiixp_dma_2_pio(XFER_PIO_0 + tspeed) + XFER_PIO_0;
 		hwif->speedproc(drive, speed);
-		return hwif->ide_dma_off_quietly(drive);
 	}
-	/* IORDY not supported */
-	return ...
From: Sergei Shtylyov
Date: Friday, January 19, 2007 - 9:26 am

Hello.


    I've looked thru the code, and found more issues with the PIO fallback 

    This function looks like it's working (thouugh having the wrong limit of 
PIO5 on auto-tuning) but is unnecassary complex.
    Heh, the driver is certainly a rip-off form hpt366.c. What a doubtful 

    It's simply stupid to convert PIO mode to PIO mode. The whole idea is 

    Well, well, the tuneproc() method can't ahndle auto-tunuing here (255)...
And it also doesn't set up drive's own speed. The code seem to be another 



    This driver's tuneproc() method always auto-tunes here instead of setting 


     Ugh, PIO fallback effectively always tries to set mode 4 here (thanks 
it's not 5). Mess.

MBR, Sergei
-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:32 pm

[PATCH] sgiioc4: fix sgiioc4_ide_dma_check() to enable/disable DMA properly

* use sgiioc4_ide_dma_{on,off_quietly}() instead of changing
  drive->using_dma directly
* fix warning message
* add FIXME

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

---
 drivers/ide/pci/sgiioc4.c |   26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

Index: b/drivers/ide/pci/sgiioc4.c
===================================================================
--- a/drivers/ide/pci/sgiioc4.c
+++ b/drivers/ide/pci/sgiioc4.c
@@ -275,21 +275,6 @@ sgiioc4_ide_dma_end(ide_drive_t * drive)
 }
 
 static int
-sgiioc4_ide_dma_check(ide_drive_t * drive)
-{
-	if (ide_config_drive_speed(drive, XFER_MW_DMA_2) != 0) {
-		printk(KERN_INFO
-		       "Couldnot set %s in Multimode-2 DMA mode | "
-			   "Drive %s using PIO instead\n",
-		       drive->name, drive->name);
-		drive->using_dma = 0;
-	} else
-		drive->using_dma = 1;
-
-	return 0;
-}
-
-static int
 sgiioc4_ide_dma_on(ide_drive_t * drive)
 {
 	drive->using_dma = 1;
@@ -305,6 +290,17 @@ sgiioc4_ide_dma_off_quietly(ide_drive_t 
 	return HWIF(drive)->ide_dma_host_off(drive);
 }
 
+static int sgiioc4_ide_dma_check(ide_drive_t *drive)
+{
+	/* FIXME: check for available DMA modes */
+	if (ide_config_drive_speed(drive, XFER_MW_DMA_2) != 0) {
+		printk(KERN_WARNING "%s: couldn't set MWDMA2 mode, "
+				    "using PIO instead\n", drive->name);
+		return sgiioc4_ide_dma_off_quietly(drive);
+	} else
+		return sgiioc4_ide_dma_on(drive);
+}
+
 /* returns 1 if dma irq issued, 0 otherwise */
 static int
 sgiioc4_ide_dma_test_irq(ide_drive_t * drive)
-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:32 pm

[PATCH] ide: add ide_set_dma() helper

* add ide_set_dma() helper and make ide_hwif_t.ide_dma_check return
  -1 when DMA needs to be disabled (== need to call ->ide_dma_off_quietly)
   0 when DMA needs to be enabled  (== need to call ->ide_dma_on)
   1 when DMA setting shouldn't be changed
* fix IDE code to use ide_set_dma() instead if using ->ide_dma_check directly

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

---
 drivers/ide/arm/icside.c       |    5 +----
 drivers/ide/cris/ide-cris.c    |    6 ++----
 drivers/ide/ide-dma.c          |   37 ++++++++++++++++++++++++++++++-------
 drivers/ide/ide-io.c           |    2 +-
 drivers/ide/ide-probe.c        |    2 +-
 drivers/ide/ide.c              |    3 ++-
 drivers/ide/mips/au1xxx-ide.c  |    4 ++--
 drivers/ide/pci/aec62xx.c      |    6 ++----
 drivers/ide/pci/alim15x3.c     |   11 +++++------
 drivers/ide/pci/amd74xx.c      |    5 +++--
 drivers/ide/pci/atiixp.c       |    7 +++----
 drivers/ide/pci/cmd64x.c       |    6 ++----
 drivers/ide/pci/cs5520.c       |    5 ++---
 drivers/ide/pci/cs5530.c       |    5 +----
 drivers/ide/pci/cs5535.c       |    5 ++---
 drivers/ide/pci/hpt34x.c       |    8 +++-----
 drivers/ide/pci/hpt366.c       |    6 ++----
 drivers/ide/pci/it8213.c       |   14 ++++++--------
 drivers/ide/pci/it821x.c       |   12 +++++-------
 drivers/ide/pci/jmicron.c      |   10 ++++------
 drivers/ide/pci/ns87415.c      |    3 ++-
 drivers/ide/pci/pdc202xx_new.c |    8 +++-----
 drivers/ide/pci/pdc202xx_old.c |    8 +++-----
 drivers/ide/pci/piix.c         |   10 ++++------
 drivers/ide/pci/sc1200.c       |    5 +----
 drivers/ide/pci/serverworks.c  |    6 ++----
 drivers/ide/pci/sgiioc4.c      |    4 ++--
 drivers/ide/pci/siimage.c      |    6 ++----
 drivers/ide/pci/sis5513.c      |    6 ++----
 drivers/ide/pci/sl82c105.c     |    6 +++---
 drivers/ide/pci/slc90e66.c     |   10 ++++------
 drivers/ide/pci/tc86c001.c     |    6 ++----
 drivers/ide/pci/triflex.c      |    9 ...
From: Sergei Shtylyov
Date: Saturday, January 20, 2007 - 1:22 pm

Hello again. :-)




    Ugh, this code looks like it's asking to be converted into calling 


    That must be the famous VDMA thing... :-)
    I wonder if it *actually* works on HPT36x/37x which seem to have support 

    The 2nd argument of that funtion is useless -- it basically does nothing 



    I have no idea why that function is so huge in this driver, i.e. why all 

    Ugh. This driver claims the full MW/SW DMA support while actually only 

    This also asks to be converted into ide_use_dma() call. The patch is 

    The same promise about tuneproc() in this driver... :-)

MBR, Sergei
-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:32 pm

[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

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    |   14 ++++++--------
 drivers/ide/ppc/pmac.c        |    8 +++-----
 include/linux/ide.h           |   12 ++++++------
 19 files changed, 79 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 ...
From: Sergei Shtylyov
Date: Saturday, January 20, 2007 - 1:56 pm

Hello again. :-)





   Ugh, this forces PIO4 on fallback... and dma_on() doesn't select any modes 




    The patch to fix those two functions is also cooking...

MBR, Sergei

-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:32 pm

[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

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 @@ ...
From: Sergei Shtylyov
Date: Saturday, January 20, 2007 - 1:46 pm

Hello again. :-)





    Unrelated note: not sure why this default value needs explicit assignemnt...

MBR, Sergei
-

From: Sergei Shtylyov
Date: Monday, March 26, 2007 - 10:19 am

Hello.


    BTW, it would also make sense to make hwif->ide_dma_timeout() and 
hwif->ide_dma_lostirq void too (and possibly drop the ide_ prefix). Their 
results are *explicitly* ignored.

MBR, Sergei
-

From: Sergei Shtylyov
Date: Friday, April 20, 2007 - 1:36 pm

I've started preparing the patches and found out that aec62xx has completely bogus ide_dma_timeout() -- the same as ide_dma_lostirq() and it doesn't even call __ide_dma_timeout()... :-/
   Don't know whether to deal with this in a separate patch...

MBR, Sergei
-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:32 pm

[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 == ...
From: Sergei Shtylyov
Date: Monday, January 22, 2007 - 9:19 am

Hello.



    Hm, well, this doesn't look consistent with the changes in other drivers.
This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...

    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 

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

MBR, Sergei
-

From: Sergei Shtylyov
Date: Monday, January 22, 2007 - 11:17 am

Hello.



    Looks like another intruduced buglet: you're reading DMA command, but 

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

    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
-

From: Alan
Date: Monday, January 22, 2007 - 11:46 am

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

Alan
-

From: Sergei Shtylyov
Date: Monday, January 22, 2007 - 1:28 pm

Hello.


    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 

WBR, Sergei
-

From: Alan
Date: Monday, January 22, 2007 - 2:31 pm

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.

-

From: Jeff Garzik
Date: Monday, January 22, 2007 - 3:39 pm

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

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

Regards,

	Jeff


-

From: Sergei Shtylyov
Date: Tuesday, January 23, 2007 - 7:51 am

Hello.


    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 

    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 

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

MBR, Sergei
-

From: Sergei Shtylyov
Date: Thursday, January 25, 2007 - 8:40 am

Hello.


    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)... 

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

MBR, Sergei
-

From: Sergei Shtylyov
Date: Wednesday, January 31, 2007 - 1:38 pm

Hello.


    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 

MBR, Sergei
-

From: Alan
Date: Wednesday, January 31, 2007 - 4:24 pm

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
-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:32 pm

[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.filter_udma_mask 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()

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       |   66 ++++-----------------
 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    ...
From: Sergei Shtylyov
Date: Monday, January 22, 2007 - 12:48 pm

Hello.



    Erm, maybe a shorter method name like udma_filter would go with the others 

    I didn't quite like the array/loop approach but well, that's my taste (I'd



    Erm, I see.  This driver will need some redesign because 'struct hpt_info' 
was fitted for the old rate filtering model.  Looks like the 'max_mode' field 

    When I think about it, it seems quite stupid to set a PIO mode instead of 
a requested DMA one.  So, this is actually a questionable piece of code in 
this driver...

    Well, I must note the sorrow fact that both the IDE susbsytem as a whole 
doesn't keep track of PIO/DMA modes separately (having only current_mode) and 
the many drivers also fail to handle the timing registers shared by PIO/DMA 

    Well, I liked this code where it was. But anyway, it's going to be 



    Hm, what was the problem with setting the proper masks based on PCI device 
ID in the init. code?

    Most probably this is doable at init. time also...

MBR, Sergei

PS: I understand that the intent wasn not to make this rewrite optimal but
do it quick and with least affect.  And I certainly may handle hpt366.c 
redesign... :-)

-

From: Bartlomiej Zolnierkiewicz
Date: Thursday, January 18, 2007 - 5:32 pm

[PATCH] ide: add ide_tune_dma() helper

After reworking the code responsible for selecting the best DMA
transfer mode it is now possible to add generic ide_tune_dma() helper.

Convert some IDE PCI host drivers to use it (the ones left need more work).

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

---
 drivers/ide/ide-dma.c      |   20 ++++++++++++++++++++
 drivers/ide/pci/aec62xx.c  |   13 +------------
 drivers/ide/pci/atiixp.c   |   22 +---------------------
 drivers/ide/pci/cs5535.c   |   15 +--------------
 drivers/ide/pci/hpt34x.c   |   20 +-------------------
 drivers/ide/pci/hpt366.c   |   20 +-------------------
 drivers/ide/pci/it8213.c   |   21 +--------------------
 drivers/ide/pci/jmicron.c  |   21 +--------------------
 drivers/ide/pci/piix.c     |   26 +-------------------------
 drivers/ide/pci/sis5513.c  |   21 +--------------------
 drivers/ide/pci/slc90e66.c |   13 +------------
 drivers/ide/pci/tc86c001.c |   13 +------------
 drivers/ide/pci/triflex.c  |   13 +------------
 include/linux/ide.h        |    2 ++
 14 files changed, 34 insertions(+), 206 deletions(-)

Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -779,6 +779,26 @@ u8 ide_max_dma_mode(ide_drive_t *drive)
 
 EXPORT_SYMBOL_GPL(ide_max_dma_mode);
 
+int ide_tune_dma(ide_drive_t *drive)
+{
+	u8 speed;
+
+	/* TODO: use only ide_max_dma_mode() */
+	if (!ide_use_dma(drive))
+		return 0;
+
+	speed = ide_max_dma_mode(drive);
+
+	if (!speed)
+		return 0;
+
+	drive->hwif->speedproc(drive, speed);
+
+	return ide_dma_enable(drive);
+}
+
+EXPORT_SYMBOL_GPL(ide_tune_dma);
+
 void ide_dma_verbose(ide_drive_t *drive)
 {
 	struct hd_driveid *id	= drive->id;
Index: b/drivers/ide/pci/aec62xx.c
===================================================================
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -154,17 +154,6 @@ static int ...
Previous thread: Re: SATA exceptions with 2.6.20-rc5 by Robert Hancock on Thursday, January 18, 2007 - 5:09 pm. (2 messages)

Next thread: none