[PATCH 01/18] ide: add ->read_sff_dma_status method

[PATCH 01/18] ide: add ->read_sff_dma_status method

[Bartlomiej Zolnierkiewicz]

Add ->read_sff_dma_status method for reading DMA Status register
and use it instead of ->INB.

While at it:

* Use inb() directly in ns87415.c::ns87415_dma_end().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-dma.c      |   12 ++++++------
 drivers/ide/ide-iops.c     |   10 ++++++++++
 drivers/ide/pci/ns87415.c  |   13 ++++++++++---
 drivers/ide/pci/scc_pata.c |    7 +++++++
 include/linux/ide.h        |    2 ++
 5 files changed, 35 insertions(+), 9 deletions(-)

Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -334,7 +334,7 @@ static int config_drive_for_dma (ide_dri
 static int dma_timer_expiry (ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
-	u8 dma_stat		= hwif->INB(hwif->dma_status);
+	u8 dma_stat		= hwif->read_sff_dma_status(hwif);
 
 	printk(KERN_WARNING "%s: dma_timer_expiry: dma status == 0x%02x\n",
 		drive->name, dma_stat);
@@ -369,7 +369,7 @@ void ide_dma_host_set(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);
+	u8 dma_stat		= hwif->read_sff_dma_status(hwif);
 
 	if (on)
 		dma_stat |= (1 << (5 + unit));
@@ -472,8 +472,8 @@ int ide_dma_setup(ide_drive_t *drive)
 	/* specify r/w */
 	hwif->OUTB(reading, hwif->dma_command);
 
-	/* read dma_status for INTR & ERROR flags */
-	dma_stat = hwif->INB(hwif->dma_status);
+	/* read DMA status for INTR & ERROR flags */
+	dma_stat = hwif->read_sff_dma_status(hwif);
 
 	/* clear INTR & ERROR flags */
 	hwif->OUTB(dma_stat|6, hwif->dma_status);
@@ -520,7 +520,7 @@ int __ide_dma_end (ide_drive_t *drive)
 	/* stop DMA */
 	hwif->OUTB(dma_cmd&~1, hwif->dma_command);
 	/* get DMA status */
-	dma_stat = hwif->INB(hwif->dma_status);
+	dma_stat = hwif->read_sff_dma_status(hwif);
 	/* clear the INTR & ERROR bits */
 	hwif->OUTB(dma_stat|6, hwif->dma_status);
 	/* purge DMA mappings */
@@ -537,7 +537,7 @@ EXPORT_SYMBOL(__ide_dma_end);
 int ide_dma_test_irq(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
-	u8 dma_stat		= hwif->INB(hwif->dma_status);
+	u8 dma_stat		= hwif->read_sff_dma_status(hwif);
 
 	/* return 1 if INTR asserted */
 	if ((dma_stat & 4) == 4)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -103,6 +103,14 @@ void SELECT_MASK(ide_drive_t *drive, int
 		port_ops->maskproc(drive, mask);
 }
 
+static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
+{
+	if (hwif->host_flags & IDE_HFLAG_MMIO)
+		return readb((void __iomem *)hwif->dma_status);
+	else
+		return inb(hwif->dma_status);
+}
+
 static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
 {
 	ide_hwif_t *hwif = drive->hwif;
@@ -323,6 +331,8 @@ static void ata_output_data(ide_drive_t 
 
 void default_hwif_transport(ide_hwif_t *hwif)
 {
+	hwif->read_sff_dma_status = ide_read_sff_dma_status;
+
 	hwif->tf_load	  = ide_tf_load;
 	hwif->tf_read	  = ide_tf_read;
 
Index: b/drivers/ide/pci/ns87415.c
===================================================================
--- a/drivers/ide/pci/ns87415.c
+++ b/drivers/ide/pci/ns87415.c
@@ -63,6 +63,11 @@ static u8 superio_ide_inb (unsigned long
 	return inb(port);
 }
 
+static u8 superio_read_sff_dma_status(ide_hwif_t *hwif)
+{
+	return superio_ide_inb(hwif->dma_status);
+}
+
 static void superio_tf_read(ide_drive_t *drive, ide_task_t *task)
 {
 	struct ide_io_ports *io_ports = &drive->hwif->io_ports;
@@ -122,6 +127,8 @@ static void __devinit superio_ide_init_i
 	tmp = superio_ide_inb(superio_ide_dma_status[port]);
 	outb(tmp | 0x66, superio_ide_dma_status[port]);
 
+	hwif->read_sff_dma_status = superio_read_sff_dma_status;
+
 	hwif->tf_read = superio_tf_read;
 
 	/* We need to override inb to workaround a SuperIO errata */
@@ -200,13 +207,13 @@ static int ns87415_dma_end(ide_drive_t *
 	u8 dma_stat = 0, dma_cmd = 0;
 
 	drive->waiting_for_dma = 0;
-	dma_stat = hwif->INB(hwif->dma_status);
+	dma_stat = hwif->read_sff_dma_status(hwif);
 	/* get dma command mode */
-	dma_cmd = hwif->INB(hwif->dma_command);
+	dma_cmd = inb(hwif->dma_command);
 	/* stop DMA */
 	outb(dma_cmd & ~1, hwif->dma_command);
 	/* from ERRATA: clear the INTR & ERROR bits */
-	dma_cmd = hwif->INB(hwif->dma_command);
+	dma_cmd = inb(hwif->dma_command);
 	outb(dma_cmd | 6, hwif->dma_command);
 	/* and free any DMA resources */
 	ide_destroy_dmatable(drive);
Index: b/drivers/ide/pci/scc_pata.c
===================================================================
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -126,6 +126,11 @@ static u8 scc_ide_inb(unsigned long port
 	return (u8)data;
 }
 
+static u8 scc_read_sff_dma_status(ide_hwif_t *hwif)
+{
+	return (u8)in_be32((void *)hwif->dma_status);
+}
+
 static void scc_ide_insw(unsigned long port, void *addr, u32 count)
 {
 	u16 *ptr = (u16 *)addr;
@@ -774,6 +779,8 @@ static void __devinit init_mmio_iops_scc
 
 	ide_set_hwifdata(hwif, ports);
 
+	hwif->read_sff_dma_status = scc_read_sff_dma_status;
+
 	hwif->tf_load = scc_tf_load;
 	hwif->tf_read = scc_tf_read;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -468,6 +468,8 @@ typedef struct hwif_s {
 	const struct ide_port_ops	*port_ops;
 	const struct ide_dma_ops	*dma_ops;
 
+	u8 (*read_sff_dma_status)(struct hwif_s *);
+
 	void (*tf_load)(ide_drive_t *, struct ide_task_s *);
 	void (*tf_read)(ide_drive_t *, struct ide_task_s *);
 

[PATCH 02/18] ide: use I/O ops directly in ide-dma.c

[Bartlomiej Zolnierkiewicz]

Use I/O ops directly in ide_dma_host_set(), ide_dma_setup(),
ide_dma_start() and __ide_dma_end().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-dma.c |   59 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 14 deletions(-)

Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -376,7 +376,10 @@ void ide_dma_host_set(ide_drive_t *drive
 	else
 		dma_stat &= ~(1 << (5 + unit));
 
-	hwif->OUTB(dma_stat, hwif->dma_status);
+	if (hwif->host_flags & IDE_HFLAG_MMIO)
+		writeb(dma_stat, (void __iomem *)hwif->dma_status);
+	else
+		outb(dma_stat, hwif->dma_status);
 }
 
 EXPORT_SYMBOL_GPL(ide_dma_host_set);
@@ -449,6 +452,7 @@ int ide_dma_setup(ide_drive_t *drive)
 	ide_hwif_t *hwif = drive->hwif;
 	struct request *rq = HWGROUP(drive)->rq;
 	unsigned int reading;
+	u8 mmio = (hwif->host_flags & IDE_HFLAG_MMIO) ? 1 : 0;
 	u8 dma_stat;
 
 	if (rq_data_dir(rq))
@@ -470,13 +474,20 @@ int ide_dma_setup(ide_drive_t *drive)
 		outl(hwif->dmatable_dma, hwif->dma_base + ATA_DMA_TABLE_OFS);
 
 	/* specify r/w */
-	hwif->OUTB(reading, hwif->dma_command);
+	if (mmio)
+		writeb(reading, (void __iomem *)hwif->dma_command);
+	else
+		outb(reading, hwif->dma_command);
 
 	/* read DMA status for INTR & ERROR flags */
 	dma_stat = hwif->read_sff_dma_status(hwif);
 
 	/* clear INTR & ERROR flags */
-	hwif->OUTB(dma_stat|6, hwif->dma_status);
+	if (mmio)
+		writeb(dma_stat | 6, (void __iomem *)hwif->dma_status);
+	else
+		outb(dma_stat | 6, hwif->dma_status);
+
 	drive->waiting_for_dma = 1;
 	return 0;
 }
@@ -492,16 +503,23 @@ EXPORT_SYMBOL_GPL(ide_dma_exec_cmd);
 
 void ide_dma_start(ide_drive_t *drive)
 {
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 dma_cmd		= hwif->INB(hwif->dma_command);
+	ide_hwif_t *hwif = drive->hwif;
+	u8 dma_cmd;
 
 	/* Note that this is done *after* the cmd has
 	 * been issued to the drive, as per the BM-IDE spec.
 	 * The Promise Ultra33 doesn't work correctly when
 	 * we do this part before issuing the drive cmd.
 	 */
-	/* start DMA */
-	hwif->OUTB(dma_cmd|1, hwif->dma_command);
+	if (hwif->host_flags & IDE_HFLAG_MMIO) {
+		dma_cmd = readb((void __iomem *)hwif->dma_command);
+		/* start DMA */
+		writeb(dma_cmd | 1, (void __iomem *)hwif->dma_command);
+	} else {
+		dma_cmd = inb(hwif->dma_command);
+		outb(dma_cmd | 1, hwif->dma_command);
+	}
+
 	hwif->dma = 1;
 	wmb();
 }
@@ -511,18 +529,31 @@ EXPORT_SYMBOL_GPL(ide_dma_start);
 /* returns 1 on error, 0 otherwise */
 int __ide_dma_end (ide_drive_t *drive)
 {
-	ide_hwif_t *hwif	= HWIF(drive);
+	ide_hwif_t *hwif = drive->hwif;
+	u8 mmio = (hwif->host_flags & IDE_HFLAG_MMIO) ? 1 : 0;
 	u8 dma_stat = 0, dma_cmd = 0;
 
 	drive->waiting_for_dma = 0;
-	/* get dma_command mode */
-	dma_cmd = hwif->INB(hwif->dma_command);
-	/* stop DMA */
-	hwif->OUTB(dma_cmd&~1, hwif->dma_command);
+
+	if (mmio) {
+		/* get DMA command mode */
+		dma_cmd = readb((void __iomem *)hwif->dma_command);
+		/* stop DMA */
+		writeb(dma_cmd & ~1, (void __iomem *)hwif->dma_command);
+	} else {
+		dma_cmd = inb(hwif->dma_command);
+		outb(dma_cmd & ~1, hwif->dma_command);
+	}
+
 	/* get DMA status */
 	dma_stat = hwif->read_sff_dma_status(hwif);
-	/* clear the INTR & ERROR bits */
-	hwif->OUTB(dma_stat|6, hwif->dma_status);
+
+	if (mmio)
+		/* clear the INTR & ERROR bits */
+		writeb(dma_stat | 6, (void __iomem *)hwif->dma_status);
+	else
+		outb(dma_stat | 6, hwif->dma_status);
+
 	/* purge DMA mappings */
 	ide_destroy_dmatable(drive);
 	/* verify good DMA status */

[PATCH 03/18] ide: remove ->dma_{status,command} fields from ide_hwif_t

[Bartlomiej Zolnierkiewicz]

* Use ->dma_base + offset instead of ->dma_{status,command}
  and remove no longer needed ->dma_{status,command}.

While at it:

* Use ATA_DMA_* defines.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-dma.c          |   42 ++++++++++++++++++++---------------------
 drivers/ide/ide-iops.c         |    4 +--
 drivers/ide/pci/cmd64x.c       |   12 +++++------
 drivers/ide/pci/hpt366.c       |   10 ++++-----
 drivers/ide/pci/ns87415.c      |   14 ++++++-------
 drivers/ide/pci/pdc202xx_old.c |    2 -
 drivers/ide/pci/piix.c         |    4 +--
 drivers/ide/pci/scc_pata.c     |   38 +++++++++++++++++--------------------
 drivers/ide/pci/siimage.c      |    4 +--
 drivers/ide/pci/sl82c105.c     |    4 +--
 drivers/ide/pci/tc86c001.c     |   13 +++++++-----
 include/linux/ide.h            |    2 -
 12 files changed, 74 insertions(+), 75 deletions(-)

Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -377,9 +377,10 @@ void ide_dma_host_set(ide_drive_t *drive
 		dma_stat &= ~(1 << (5 + unit));
 
 	if (hwif->host_flags & IDE_HFLAG_MMIO)
-		writeb(dma_stat, (void __iomem *)hwif->dma_status);
+		writeb(dma_stat,
+		       (void __iomem *)(hwif->dma_base + ATA_DMA_STATUS));
 	else
-		outb(dma_stat, hwif->dma_status);
+		outb(dma_stat, hwif->dma_base + ATA_DMA_STATUS);
 }
 
 EXPORT_SYMBOL_GPL(ide_dma_host_set);
@@ -475,18 +476,19 @@ int ide_dma_setup(ide_drive_t *drive)
 
 	/* specify r/w */
 	if (mmio)
-		writeb(reading, (void __iomem *)hwif->dma_command);
+		writeb(reading, (void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
 	else
-		outb(reading, hwif->dma_command);
+		outb(reading, hwif->dma_base + ATA_DMA_CMD);
 
 	/* read DMA status for INTR & ERROR flags */
 	dma_stat = hwif->read_sff_dma_status(hwif);
 
 	/* clear INTR & ERROR flags */
 	if (mmio)
-		writeb(dma_stat | 6, (void __iomem *)hwif->dma_status);
+		writeb(dma_stat | 6,
+		       (void __iomem *)(hwif->dma_base + ATA_DMA_STATUS));
 	else
-		outb(dma_stat | 6, hwif->dma_status);
+		outb(dma_stat | 6, hwif->dma_base + ATA_DMA_STATUS);
 
 	drive->waiting_for_dma = 1;
 	return 0;
@@ -512,12 +514,13 @@ void ide_dma_start(ide_drive_t *drive)
 	 * we do this part before issuing the drive cmd.
 	 */
 	if (hwif->host_flags & IDE_HFLAG_MMIO) {
-		dma_cmd = readb((void __iomem *)hwif->dma_command);
+		dma_cmd = readb((void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
 		/* start DMA */
-		writeb(dma_cmd | 1, (void __iomem *)hwif->dma_command);
+		writeb(dma_cmd | 1,
+		       (void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
 	} else {
-		dma_cmd = inb(hwif->dma_command);
-		outb(dma_cmd | 1, hwif->dma_command);
+		dma_cmd = inb(hwif->dma_base + ATA_DMA_CMD);
+		outb(dma_cmd | 1, hwif->dma_base + ATA_DMA_CMD);
 	}
 
 	hwif->dma = 1;
@@ -537,12 +540,13 @@ int __ide_dma_end (ide_drive_t *drive)
 
 	if (mmio) {
 		/* get DMA command mode */
-		dma_cmd = readb((void __iomem *)hwif->dma_command);
+		dma_cmd = readb((void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
 		/* stop DMA */
-		writeb(dma_cmd & ~1, (void __iomem *)hwif->dma_command);
+		writeb(dma_cmd & ~1,
+		       (void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
 	} else {
-		dma_cmd = inb(hwif->dma_command);
-		outb(dma_cmd & ~1, hwif->dma_command);
+		dma_cmd = inb(hwif->dma_base + ATA_DMA_CMD);
+		outb(dma_cmd & ~1, hwif->dma_base + ATA_DMA_CMD);
 	}
 
 	/* get DMA status */
@@ -550,9 +554,10 @@ int __ide_dma_end (ide_drive_t *drive)
 
 	if (mmio)
 		/* clear the INTR & ERROR bits */
-		writeb(dma_stat | 6, (void __iomem *)hwif->dma_status);
+		writeb(dma_stat | 6,
+		       (void __iomem *)(hwif->dma_base + ATA_DMA_STATUS));
 	else
-		outb(dma_stat | 6, hwif->dma_status);
+		outb(dma_stat | 6, hwif->dma_base + ATA_DMA_STATUS);
 
 	/* purge DMA mappings */
 	ide_destroy_dmatable(drive);
@@ -888,11 +893,6 @@ void ide_setup_dma(ide_hwif_t *hwif, uns
 {
 	hwif->dma_base = base;
 
-	if (!hwif->dma_command)
-		hwif->dma_command	= hwif->dma_base + 0;
-	if (!hwif->dma_status)
-		hwif->dma_status	= hwif->dma_base + 2;
-
 	hwif->dma_ops = &sff_dma_ops;
 }
 
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -106,9 +106,9 @@ void SELECT_MASK(ide_drive_t *drive, int
 static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
 {
 	if (hwif->host_flags & IDE_HFLAG_MMIO)
-		return readb((void __iomem *)hwif->dma_status);
+		return readb((void __iomem *)(hwif->dma_base + ATA_DMA_STATUS));
 	else
-		return inb(hwif->dma_status);
+		return inb(hwif->dma_base + ATA_DMA_STATUS);
 }
 
 static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
Index: b/drivers/ide/pci/cmd64x.c
===================================================================
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -262,7 +262,7 @@ static int cmd648_dma_test_irq(ide_drive
 	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
 	u8 irq_mask		= hwif->channel ? MRDMODE_INTR_CH1 :
 						  MRDMODE_INTR_CH0;
-	u8 dma_stat		= inb(hwif->dma_status);
+	u8 dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 	u8 mrdmode		= inb(base + 1);
 
 #ifdef DEBUG
@@ -286,7 +286,7 @@ static int cmd64x_dma_test_irq(ide_drive
 	int irq_reg		= hwif->channel ? ARTTIM23 : CFR;
 	u8  irq_mask		= hwif->channel ? ARTTIM23_INTR_CH1 :
 						  CFR_INTR_CH0;
-	u8  dma_stat		= inb(hwif->dma_status);
+	u8  dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 	u8  irq_stat		= 0;
 
 	(void) pci_read_config_byte(dev, irq_reg, &irq_stat);
@@ -317,13 +317,13 @@ static int cmd646_1_dma_end(ide_drive_t 
 
 	drive->waiting_for_dma = 0;
 	/* get DMA status */
-	dma_stat = inb(hwif->dma_status);
+	dma_stat = inb(hwif->dma_base + ATA_DMA_STATUS);
 	/* read DMA command state */
-	dma_cmd = inb(hwif->dma_command);
+	dma_cmd = inb(hwif->dma_base + ATA_DMA_CMD);
 	/* stop DMA */
-	outb(dma_cmd & ~1, hwif->dma_command);
+	outb(dma_cmd & ~1, hwif->dma_base + ATA_DMA_CMD);
 	/* clear the INTR & ERROR bits */
-	outb(dma_stat | 6, hwif->dma_status);
+	outb(dma_stat | 6, hwif->dma_base + ATA_DMA_STATUS);
 	/* and free any DMA resources */
 	ide_destroy_dmatable(drive);
 	/* verify good DMA status */
Index: b/drivers/ide/pci/hpt366.c
===================================================================
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -801,9 +801,9 @@ static void hpt370_irq_timeout(ide_drive
 	printk(KERN_DEBUG "%s: %d bytes in FIFO\n", drive->name, bfifo & 0x1ff);
 
 	/* get DMA command mode */
-	dma_cmd = inb(hwif->dma_command);
+	dma_cmd = inb(hwif->dma_base + ATA_DMA_CMD);
 	/* stop DMA */
-	outb(dma_cmd & ~0x1, hwif->dma_command);
+	outb(dma_cmd & ~0x1, hwif->dma_base + ATA_DMA_CMD);
 	hpt370_clear_engine(drive);
 }
 
@@ -818,12 +818,12 @@ static void hpt370_dma_start(ide_drive_t
 static int hpt370_dma_end(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
-	u8  dma_stat		= inb(hwif->dma_status);
+	u8  dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 
 	if (dma_stat & 0x01) {
 		/* wait a little */
 		udelay(20);
-		dma_stat = inb(hwif->dma_status);
+		dma_stat = inb(hwif->dma_base + ATA_DMA_STATUS);
 		if (dma_stat & 0x01)
 			hpt370_irq_timeout(drive);
 	}
@@ -850,7 +850,7 @@ static int hpt374_dma_test_irq(ide_drive
 		return 0;
 	}
 
-	dma_stat = inb(hwif->dma_status);
+	dma_stat = inb(hwif->dma_base + ATA_DMA_STATUS);
 	/* return 1 if INTR asserted */
 	if (dma_stat & 4)
 		return 1;
Index: b/drivers/ide/pci/ns87415.c
===================================================================
--- a/drivers/ide/pci/ns87415.c
+++ b/drivers/ide/pci/ns87415.c
@@ -65,7 +65,7 @@ static u8 superio_ide_inb (unsigned long
 
 static u8 superio_read_sff_dma_status(ide_hwif_t *hwif)
 {
-	return superio_ide_inb(hwif->dma_status);
+	return superio_ide_inb(hwif->dma_base + ATA_DMA_STATUS);
 }
 
 static void superio_tf_read(ide_drive_t *drive, ide_task_t *task)
@@ -208,13 +208,13 @@ static int ns87415_dma_end(ide_drive_t *
 
 	drive->waiting_for_dma = 0;
 	dma_stat = hwif->read_sff_dma_status(hwif);
-	/* get dma command mode */
-	dma_cmd = inb(hwif->dma_command);
+	/* get DMA command mode */
+	dma_cmd = inb(hwif->dma_base + ATA_DMA_CMD);
 	/* stop DMA */
-	outb(dma_cmd & ~1, hwif->dma_command);
+	outb(dma_cmd & ~1, hwif->dma_base + ATA_DMA_CMD);
 	/* from ERRATA: clear the INTR & ERROR bits */
-	dma_cmd = inb(hwif->dma_command);
-	outb(dma_cmd | 6, hwif->dma_command);
+	dma_cmd = inb(hwif->dma_base + ATA_DMA_CMD);
+	outb(dma_cmd | 6, hwif->dma_base + ATA_DMA_CMD);
 	/* and free any DMA resources */
 	ide_destroy_dmatable(drive);
 	/* verify good DMA status */
@@ -302,7 +302,7 @@ static void __devinit init_hwif_ns87415 
 	if (!hwif->dma_base)
 		return;
 
-	outb(0x60, hwif->dma_status);
+	outb(0x60, hwif->dma_base + ATA_DMA_STATUS);
 }
 
 static const struct ide_port_ops ns87415_port_ops = {
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -206,7 +206,7 @@ static int pdc202xx_dma_test_irq(ide_dri
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	unsigned long high_16	= hwif->extra_base - 16;
-	u8 dma_stat		= inb(hwif->dma_status);
+	u8 dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 	u8 sc1d			= inb(high_16 + 0x001d);
 
 	if (hwif->channel) {
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -227,9 +227,9 @@ static void piix_dma_clear_irq(ide_drive
 	u8 dma_stat;
 
 	/* clear the INTR & ERROR bits */
-	dma_stat = inb(hwif->dma_status);
+	dma_stat = inb(hwif->dma_base + ATA_DMA_STATUS);
 	/* Should we force the bit as well ? */
-	outb(dma_stat, hwif->dma_status);
+	outb(dma_stat, hwif->dma_base + ATA_DMA_STATUS);
 }
 
 struct ich_laptop {
Index: b/drivers/ide/pci/scc_pata.c
===================================================================
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -128,7 +128,7 @@ static u8 scc_ide_inb(unsigned long port
 
 static u8 scc_read_sff_dma_status(ide_hwif_t *hwif)
 {
-	return (u8)in_be32((void *)hwif->dma_status);
+	return (u8)in_be32((void *)(hwif->dma_base + 4));
 }
 
 static void scc_ide_insw(unsigned long port, void *addr, u32 count)
@@ -266,14 +266,14 @@ static void scc_dma_host_set(ide_drive_t
 {
 	ide_hwif_t *hwif = drive->hwif;
 	u8 unit = (drive->select.b.unit & 0x01);
-	u8 dma_stat = scc_ide_inb(hwif->dma_status);
+	u8 dma_stat = scc_ide_inb(hwif->dma_base + 4);
 
 	if (on)
 		dma_stat |= (1 << (5 + unit));
 	else
 		dma_stat &= ~(1 << (5 + unit));
 
-	scc_ide_outb(dma_stat, hwif->dma_status);
+	scc_ide_outb(dma_stat, hwif->dma_base + 4);
 }
 
 /**
@@ -309,13 +309,13 @@ static int scc_dma_setup(ide_drive_t *dr
 	out_be32((void __iomem *)(hwif->dma_base + 8), hwif->dmatable_dma);
 
 	/* specify r/w */
-	out_be32((void __iomem *)hwif->dma_command, reading);
+	out_be32((void __iomem *)hwif->dma_base, reading);
 
-	/* read dma_status for INTR & ERROR flags */
-	dma_stat = in_be32((void __iomem *)hwif->dma_status);
+	/* read DMA status for INTR & ERROR flags */
+	dma_stat = in_be32((void __iomem *)(hwif->dma_base + 4));
 
 	/* clear INTR & ERROR flags */
-	out_be32((void __iomem *)hwif->dma_status, dma_stat|6);
+	out_be32((void __iomem *)(hwif->dma_base + 4), dma_stat | 6);
 	drive->waiting_for_dma = 1;
 	return 0;
 }
@@ -323,10 +323,10 @@ static int scc_dma_setup(ide_drive_t *dr
 static void scc_dma_start(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
-	u8 dma_cmd = scc_ide_inb(hwif->dma_command);
+	u8 dma_cmd = scc_ide_inb(hwif->dma_base);
 
 	/* start DMA */
-	scc_ide_outb(dma_cmd | 1, hwif->dma_command);
+	scc_ide_outb(dma_cmd | 1, hwif->dma_base);
 	hwif->dma = 1;
 	wmb();
 }
@@ -338,13 +338,13 @@ static int __scc_dma_end(ide_drive_t *dr
 
 	drive->waiting_for_dma = 0;
 	/* get DMA command mode */
-	dma_cmd = scc_ide_inb(hwif->dma_command);
+	dma_cmd = scc_ide_inb(hwif->dma_base);
 	/* stop DMA */
-	scc_ide_outb(dma_cmd & ~1, hwif->dma_command);
+	scc_ide_outb(dma_cmd & ~1, hwif->dma_base);
 	/* get DMA status */
-	dma_stat = scc_ide_inb(hwif->dma_status);
+	dma_stat = scc_ide_inb(hwif->dma_base + 4);
 	/* clear the INTR & ERROR bits */
-	scc_ide_outb(dma_stat | 6, hwif->dma_status);
+	scc_ide_outb(dma_stat | 6, hwif->dma_base + 4);
 	/* purge DMA mappings */
 	ide_destroy_dmatable(drive);
 	/* verify good DMA status */
@@ -364,6 +364,7 @@ static int __scc_dma_end(ide_drive_t *dr
 static int scc_dma_end(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = HWIF(drive);
+	void __iomem *dma_base = (void __iomem *)hwif->dma_base;
 	unsigned long intsts_port = hwif->dma_base + 0x014;
 	u32 reg;
 	int dma_stat, data_loss = 0;
@@ -402,7 +403,7 @@ static int scc_dma_end(ide_drive_t *driv
 			printk(KERN_WARNING "%s: SERROR\n", SCC_PATA_NAME);
 			out_be32((void __iomem *)intsts_port, INTSTS_SERROR|INTSTS_BMSINT);
 
-			out_be32((void __iomem *)hwif->dma_command, in_be32((void __iomem *)hwif->dma_command) & ~QCHCD_IOS_SS);
+			out_be32(dma_base, in_be32(dma_base) & ~QCHCD_IOS_SS);
 			continue;
 		}
 
@@ -417,7 +418,7 @@ static int scc_dma_end(ide_drive_t *driv
 
 			out_be32((void __iomem *)intsts_port, INTSTS_PRERR|INTSTS_BMSINT);
 
-			out_be32((void __iomem *)hwif->dma_command, in_be32((void __iomem *)hwif->dma_command) & ~QCHCD_IOS_SS);
+			out_be32(dma_base, in_be32(dma_base) & ~QCHCD_IOS_SS);
 			continue;
 		}
 
@@ -425,12 +426,12 @@ static int scc_dma_end(ide_drive_t *driv
 			printk(KERN_WARNING "%s: Response Error\n", SCC_PATA_NAME);
 			out_be32((void __iomem *)intsts_port, INTSTS_RERR|INTSTS_BMSINT);
 
-			out_be32((void __iomem *)hwif->dma_command, in_be32((void __iomem *)hwif->dma_command) & ~QCHCD_IOS_SS);
+			out_be32(dma_base, in_be32(dma_base) & ~QCHCD_IOS_SS);
 			continue;
 		}
 
 		if (reg & INTSTS_ICERR) {
-			out_be32((void __iomem *)hwif->dma_command, in_be32((void __iomem *)hwif->dma_command) & ~QCHCD_IOS_SS);
+			out_be32(dma_base, in_be32(dma_base) & ~QCHCD_IOS_SS);
 
 			printk(KERN_WARNING "%s: Illegal Configuration\n", SCC_PATA_NAME);
 			out_be32((void __iomem *)intsts_port, INTSTS_ICERR|INTSTS_BMSINT);
@@ -833,9 +834,6 @@ static void __devinit init_hwif_scc(ide_
 
 	ports->hwif = hwif;
 
-	hwif->dma_command = hwif->dma_base;
-	hwif->dma_status = hwif->dma_base + 0x04;
-
 	/* PTERADD */
 	out_be32((void __iomem *)(hwif->dma_base + 0x018), hwif->dmatable_dma);
 
Index: b/drivers/ide/pci/siimage.c
===================================================================
--- a/drivers/ide/pci/siimage.c
+++ b/drivers/ide/pci/siimage.c
@@ -334,7 +334,7 @@ static int siimage_io_dma_test_irq(ide_d
 	unsigned long addr	= siimage_selreg(hwif, 1);
 
 	/* return 1 if INTR asserted */
-	if (hwif->INB(hwif->dma_status) & 4)
+	if (inb(hwif->dma_base + ATA_DMA_STATUS) & 4)
 		return 1;
 
 	/* return 1 if Device INTR asserted */
@@ -382,7 +382,7 @@ static int siimage_mmio_dma_test_irq(ide
 	}
 
 	/* return 1 if INTR asserted */
-	if (readb((void __iomem *)hwif->dma_status) & 0x04)
+	if (readb((void __iomem *)(hwif->dma_base + ATA_DMA_STATUS)) & 4)
 		return 1;
 
 	/* return 1 if Device INTR asserted */
Index: b/drivers/ide/pci/sl82c105.c
===================================================================
--- a/drivers/ide/pci/sl82c105.c
+++ b/drivers/ide/pci/sl82c105.c
@@ -157,9 +157,9 @@ static void sl82c105_dma_lost_irq(ide_dr
 	 * Was DMA enabled?  If so, disable it - we're resetting the
 	 * host.  The IDE layer will be handling the drive for us.
 	 */
-	dma_cmd = inb(hwif->dma_command);
+	dma_cmd = inb(hwif->dma_base + ATA_DMA_CMD);
 	if (dma_cmd & 1) {
-		outb(dma_cmd & ~1, hwif->dma_command);
+		outb(dma_cmd & ~1, hwif->dma_base + ATA_DMA_CMD);
 		printk("sl82c105: DMA was enabled\n");
 	}
 
Index: b/drivers/ide/pci/tc86c001.c
===================================================================
--- a/drivers/ide/pci/tc86c001.c
+++ b/drivers/ide/pci/tc86c001.c
@@ -63,7 +63,7 @@ static int tc86c001_timer_expiry(ide_dri
 	ide_hwif_t *hwif	= HWIF(drive);
 	ide_expiry_t *expiry	= ide_get_hwifdata(hwif);
 	ide_hwgroup_t *hwgroup	= HWGROUP(drive);
-	u8 dma_stat		= inb(hwif->dma_status);
+	u8 dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 
 	/* Restore a higher level driver's expiry handler first. */
 	hwgroup->expiry	= expiry;
@@ -71,21 +71,24 @@ static int tc86c001_timer_expiry(ide_dri
 	if ((dma_stat & 5) == 1) {	/* DMA active and no interrupt */
 		unsigned long sc_base	= hwif->config_data;
 		unsigned long twcr_port	= sc_base + (drive->dn ? 0x06 : 0x04);
-		u8 dma_cmd		= inb(hwif->dma_command);
+		u8 dma_cmd		= inb(hwif->dma_base + ATA_DMA_CMD);
 
 		printk(KERN_WARNING "%s: DMA interrupt possibly stuck, "
 		       "attempting recovery...\n", drive->name);
 
 		/* Stop DMA */
-		outb(dma_cmd & ~0x01, hwif->dma_command);
+		outb(dma_cmd & ~0x01, hwif->dma_base + ATA_DMA_CMD);
 
 		/* Setup the dummy DMA transfer */
 		outw(0, sc_base + 0x0a);	/* Sector Count */
 		outw(0, twcr_port);	/* Transfer Word Count 1 or 2 */
 
 		/* Start the dummy DMA transfer */
-		outb(0x00, hwif->dma_command); /* clear R_OR_WCTR for write */
-		outb(0x01, hwif->dma_command); /* set START_STOPBM */
+
+		/* clear R_OR_WCTR for write */
+		outb(0x00, hwif->dma_base + ATA_DMA_CMD);
+		/* set START_STOPBM */
+		outb(0x01, hwif->dma_base + ATA_DMA_CMD);
 
 		/*
 		 * If an interrupt was pending, it should come thru shortly.
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -505,8 +505,6 @@ typedef struct hwif_s {
 	int		irq;		/* our irq number */
 
 	unsigned long	dma_base;	/* base addr for dma ports */
-	unsigned long	dma_command;	/* dma command register */
-	unsigned long	dma_status;	/* dma status register */
 
 	unsigned long	config_data;	/* for use by chipset-specific code */
 	unsigned long	select_data;	/* for use by chipset-specific code */

[PATCH 04/18] ide: remove ide_setup_dma()

[Bartlomiej Zolnierkiewicz]

Export sff_dma_ops and then remove ide_setup_dma().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/arm/palm_bk3710.c |    7 +++----
 drivers/ide/ide-dma.c         |   12 ++----------
 drivers/ide/pci/alim15x3.c    |    4 +++-
 drivers/ide/pci/hpt366.c      |    4 +++-
 drivers/ide/setup-pci.c       |    4 +++-
 include/linux/ide.h           |    2 +-
 6 files changed, 15 insertions(+), 18 deletions(-)

Index: b/drivers/ide/arm/palm_bk3710.c
===================================================================
--- a/drivers/ide/arm/palm_bk3710.c
+++ b/drivers/ide/arm/palm_bk3710.c
@@ -318,15 +318,14 @@ static u8 __devinit palm_bk3710_cable_de
 static int __devinit palm_bk3710_init_dma(ide_hwif_t *hwif,
 					  const struct ide_port_info *d)
 {
-	unsigned long base =
-		hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
-
 	printk(KERN_INFO "    %s: MMIO-DMA\n", hwif->name);
 
 	if (ide_allocate_dma_engine(hwif))
 		return -1;
 
-	ide_setup_dma(hwif, base);
+	hwif->dma_base = hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
+
+	hwif->dma_ops = &sff_dma_ops;
 
 	return 0;
 }
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -878,7 +878,7 @@ int ide_allocate_dma_engine(ide_hwif_t *
 }
 EXPORT_SYMBOL_GPL(ide_allocate_dma_engine);
 
-static const struct ide_dma_ops sff_dma_ops = {
+const struct ide_dma_ops sff_dma_ops = {
 	.dma_host_set		= ide_dma_host_set,
 	.dma_setup		= ide_dma_setup,
 	.dma_exec_cmd		= ide_dma_exec_cmd,
@@ -888,13 +888,5 @@ static const struct ide_dma_ops sff_dma_
 	.dma_timeout		= ide_dma_timeout,
 	.dma_lost_irq		= ide_dma_lost_irq,
 };
-
-void ide_setup_dma(ide_hwif_t *hwif, unsigned long base)
-{
-	hwif->dma_base = base;
-
-	hwif->dma_ops = &sff_dma_ops;
-}
-
-EXPORT_SYMBOL_GPL(ide_setup_dma);
+EXPORT_SYMBOL_GPL(sff_dma_ops);
 #endif /* CONFIG_BLK_DEV_IDEDMA_SFF */
Index: b/drivers/ide/pci/alim15x3.c
===================================================================
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -483,7 +483,9 @@ static int __devinit init_dma_ali15x3(id
 	if (ide_allocate_dma_engine(hwif))
 		return -1;
 
-	ide_setup_dma(hwif, base);
+	hwif->dma_base = base;
+
+	hwif->dma_ops = &sff_dma_ops;
 
 	return 0;
 }
Index: b/drivers/ide/pci/hpt366.c
===================================================================
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -1346,7 +1346,9 @@ static int __devinit init_dma_hpt366(ide
 	if (ide_allocate_dma_engine(hwif))
 		return -1;
 
-	ide_setup_dma(hwif, base);
+	hwif->dma_base = base;
+
+	hwif->dma_ops = &sff_dma_ops;
 
 	return 0;
 }
Index: b/drivers/ide/setup-pci.c
===================================================================
--- a/drivers/ide/setup-pci.c
+++ b/drivers/ide/setup-pci.c
@@ -376,7 +376,9 @@ int ide_hwif_setup_dma(ide_hwif_t *hwif,
 		if (ide_allocate_dma_engine(hwif))
 			return -1;
 
-		ide_setup_dma(hwif, base);
+		hwif->dma_base = base;
+
+		hwif->dma_ops = &sff_dma_ops;
 	}
 
 	return 0;
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -993,6 +993,7 @@ void ide_setup_pci_noise(struct pci_dev 
 #ifdef CONFIG_BLK_DEV_IDEDMA_PCI
 int ide_pci_set_master(struct pci_dev *, const char *);
 unsigned long ide_pci_dma_base(ide_hwif_t *, const struct ide_port_info *);
+extern const struct ide_dma_ops sff_dma_ops;
 int ide_hwif_setup_dma(ide_hwif_t *, const struct ide_port_info *);
 #else
 static inline int ide_hwif_setup_dma(ide_hwif_t *hwif,
@@ -1150,7 +1151,6 @@ void ide_destroy_dmatable(ide_drive_t *)
 extern int ide_build_dmatable(ide_drive_t *, struct request *);
 int ide_allocate_dma_engine(ide_hwif_t *);
 void ide_release_dma_engine(ide_hwif_t *);
-void ide_setup_dma(ide_hwif_t *, unsigned long);
 
 void ide_dma_host_set(ide_drive_t *, int);
 extern int ide_dma_setup(ide_drive_t *);

[PATCH 05/18] ide: factor out simplex handling from ide_pci_dma_base()

[Bartlomiej Zolnierkiewicz]

* Factor out simplex handling from ide_pci_dma_base() to
  ide_pci_check_simplex().

* Set hwif->dma_base early in ->init_dma method / ide_hwif_setup_dma()
  and reset it in ide_init_port() if DMA initialization fails.

* Use ->read_sff_dma_status instead of ->INB in ide_pci_dma_base().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-probe.c    |    1 +
 drivers/ide/pci/alim15x3.c |   12 +++++++++---
 drivers/ide/pci/hpt366.c   |   12 +++++++++---
 drivers/ide/setup-pci.c    |   35 +++++++++++++++++++++++------------
 include/linux/ide.h        |    1 +
 5 files changed, 43 insertions(+), 18 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1369,6 +1369,7 @@ static void ide_init_port(ide_hwif_t *hw
 
 		if (rc < 0) {
 			printk(KERN_INFO "%s: DMA disabled\n", hwif->name);
+			hwif->dma_base = 0;
 			hwif->swdma_mask = 0;
 			hwif->mwdma_mask = 0;
 			hwif->ultra_mask = 0;
Index: b/drivers/ide/pci/alim15x3.c
===================================================================
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -471,7 +471,15 @@ static int __devinit init_dma_ali15x3(id
 	struct pci_dev *dev = to_pci_dev(hwif->dev);
 	unsigned long base = ide_pci_dma_base(hwif, d);
 
-	if (base == 0 || ide_pci_set_master(dev, d->name) < 0)
+	if (base == 0)
+		return -1;
+
+	hwif->dma_base = base;
+
+	if (ide_pci_check_simplex(hwif, d) < 0)
+		return -1;
+
+	if (ide_pci_set_master(dev, d->name) < 0)
 		return -1;
 
 	if (!hwif->channel)
@@ -483,8 +491,6 @@ static int __devinit init_dma_ali15x3(id
 	if (ide_allocate_dma_engine(hwif))
 		return -1;
 
-	hwif->dma_base = base;
-
 	hwif->dma_ops = &sff_dma_ops;
 
 	return 0;
Index: b/drivers/ide/pci/hpt366.c
===================================================================
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -1320,7 +1320,15 @@ static int __devinit init_dma_hpt366(ide
 	unsigned long flags, base = ide_pci_dma_base(hwif, d);
 	u8 dma_old, dma_new, masterdma = 0, slavedma = 0;
 
-	if (base == 0 || ide_pci_set_master(dev, d->name) < 0)
+	if (base == 0)
+		return -1;
+
+	hwif->dma_base = base;
+
+	if (ide_pci_check_simplex(hwif, d) < 0)
+		return -1;
+
+	if (ide_pci_set_master(dev, d->name) < 0)
 		return -1;
 
 	dma_old = inb(base + 2);
@@ -1346,8 +1354,6 @@ static int __devinit init_dma_hpt366(ide
 	if (ide_allocate_dma_engine(hwif))
 		return -1;
 
-	hwif->dma_base = base;
-
 	hwif->dma_ops = &sff_dma_ops;
 
 	return 0;
Index: b/drivers/ide/setup-pci.c
===================================================================
--- a/drivers/ide/setup-pci.c
+++ b/drivers/ide/setup-pci.c
@@ -72,15 +72,12 @@ static void ide_pci_clear_simplex(unsign
  *	@d: IDE port info
  *
  *	Fetch the DMA Bus-Master-I/O-Base-Address (BMIBA) from PCI space.
- *	Where a device has a partner that is already in DMA mode we check
- *	and enforce IDE simplex rules.
  */
 
 unsigned long ide_pci_dma_base(ide_hwif_t *hwif, const struct ide_port_info *d)
 {
 	struct pci_dev *dev = to_pci_dev(hwif->dev);
 	unsigned long dma_base = 0;
-	u8 dma_stat = 0;
 
 	if (hwif->host_flags & IDE_HFLAG_MMIO)
 		return hwif->dma_base;
@@ -101,11 +98,19 @@ unsigned long ide_pci_dma_base(ide_hwif_
 	if (hwif->channel)
 		dma_base += 8;
 
-	if (d->host_flags & IDE_HFLAG_CS5520)
+	return dma_base;
+}
+EXPORT_SYMBOL_GPL(ide_pci_dma_base);
+
+int ide_pci_check_simplex(ide_hwif_t *hwif, const struct ide_port_info *d)
+{
+	u8 dma_stat;
+
+	if (d->host_flags & (IDE_HFLAG_MMIO | IDE_HFLAG_CS5520))
 		goto out;
 
 	if (d->host_flags & IDE_HFLAG_CLEAR_SIMPLEX) {
-		ide_pci_clear_simplex(dma_base, d->name);
+		ide_pci_clear_simplex(hwif->dma_base, d->name);
 		goto out;
 	}
 
@@ -119,15 +124,15 @@ unsigned long ide_pci_dma_base(ide_hwif_
 	 * we tune the drive then try to grab DMA ownership if we want to be
 	 * the DMA end.  This has to be become dynamic to handle hot-plug.
 	 */
-	dma_stat = hwif->INB(dma_base + 2);
+	dma_stat = hwif->read_sff_dma_status(hwif);
 	if ((dma_stat & 0x80) && hwif->mate && hwif->mate->dma_base) {
 		printk(KERN_INFO "%s: simplex device: DMA disabled\n", d->name);
-		dma_base = 0;
+		return -1;
 	}
 out:
-	return dma_base;
+	return 0;
 }
-EXPORT_SYMBOL_GPL(ide_pci_dma_base);
+EXPORT_SYMBOL_GPL(ide_pci_check_simplex);
 
 /*
  * Set up BM-DMA capability (PnP BIOS should have done this)
@@ -362,7 +367,15 @@ int ide_hwif_setup_dma(ide_hwif_t *hwif,
 	     (dev->class & 0x80))) {
 		unsigned long base = ide_pci_dma_base(hwif, d);
 
-		if (base == 0 || ide_pci_set_master(dev, d->name) < 0)
+		if (base == 0)
+			return -1;
+
+		hwif->dma_base = base;
+
+		if (ide_pci_check_simplex(hwif, d) < 0)
+			return -1;
+
+		if (ide_pci_set_master(dev, d->name) < 0)
 			return -1;
 
 		if (hwif->host_flags & IDE_HFLAG_MMIO)
@@ -376,8 +389,6 @@ int ide_hwif_setup_dma(ide_hwif_t *hwif,
 		if (ide_allocate_dma_engine(hwif))
 			return -1;
 
-		hwif->dma_base = base;
-
 		hwif->dma_ops = &sff_dma_ops;
 	}
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -994,6 +994,7 @@ void ide_setup_pci_noise(struct pci_dev 
 int ide_pci_set_master(struct pci_dev *, const char *);
 unsigned long ide_pci_dma_base(ide_hwif_t *, const struct ide_port_info *);
 extern const struct ide_dma_ops sff_dma_ops;
+int ide_pci_check_simplex(ide_hwif_t *, const struct ide_port_info *);
 int ide_hwif_setup_dma(ide_hwif_t *, const struct ide_port_info *);
 #else
 static inline int ide_hwif_setup_dma(ide_hwif_t *hwif,

[PATCH 06/18] ide: add ->exec_command method

[Bartlomiej Zolnierkiewicz]

Add ->exec_command method for writing ATA Command register
and use it instead of ->OUTBSYNC.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-io.c       |    3 +--
 drivers/ide/ide-iops.c     |   19 ++++++++++++++-----
 drivers/ide/ide-probe.c    |    6 +++---
 drivers/ide/ide-taskfile.c |    2 +-
 drivers/ide/pci/scc_pata.c |    9 +++++++++
 drivers/ide/ppc/pmac.c     |    9 +++++++++
 drivers/scsi/ide-scsi.c    |    3 +--
 include/linux/ide.h        |    3 ++-
 8 files changed, 40 insertions(+), 14 deletions(-)

Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -437,8 +437,7 @@ static ide_startstop_t ide_atapi_error(i
 
 	if (ide_read_status(drive) & (BUSY_STAT | DRQ_STAT))
 		/* force an abort */
-		hwif->OUTBSYNC(hwif, WIN_IDLEIMMEDIATE,
-			       hwif->io_ports.command_addr);
+		hwif->exec_command(hwif, WIN_IDLEIMMEDIATE);
 
 	if (rq->errors >= ERROR_MAX) {
 		ide_kill_rq(drive, rq);
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -103,6 +103,14 @@ void SELECT_MASK(ide_drive_t *drive, int
 		port_ops->maskproc(drive, mask);
 }
 
+static void ide_exec_command(ide_hwif_t *hwif, u8 cmd)
+{
+	if (hwif->host_flags & IDE_HFLAG_MMIO)
+		writeb(cmd, (void __iomem *)hwif->io_ports.command_addr);
+	else
+		outb(cmd, hwif->io_ports.command_addr);
+}
+
 static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
 {
 	if (hwif->host_flags & IDE_HFLAG_MMIO)
@@ -331,6 +339,7 @@ static void ata_output_data(ide_drive_t 
 
 void default_hwif_transport(ide_hwif_t *hwif)
 {
+	hwif->exec_command	  = ide_exec_command;
 	hwif->read_sff_dma_status = ide_read_sff_dma_status;
 
 	hwif->tf_load	  = ide_tf_load;
@@ -696,7 +705,7 @@ int ide_driveid_update(ide_drive_t *driv
 	SELECT_MASK(drive, 1);
 	ide_set_irq(drive, 1);
 	msleep(50);
-	hwif->OUTBSYNC(hwif, WIN_IDENTIFY, hwif->io_ports.command_addr);
+	hwif->exec_command(hwif, WIN_IDENTIFY);
 	timeout = jiffies + WAIT_WORSTCASE;
 	do {
 		if (time_after(jiffies, timeout)) {
@@ -783,7 +792,7 @@ int ide_config_drive_speed(ide_drive_t *
 	ide_set_irq(drive, 0);
 	hwif->OUTB(speed, io_ports->nsect_addr);
 	hwif->OUTB(SETFEATURES_XFER, io_ports->feature_addr);
-	hwif->OUTBSYNC(hwif, WIN_SETFEATURES, io_ports->command_addr);
+	hwif->exec_command(hwif, WIN_SETFEATURES);
 	if (drive->quirk_list == 2)
 		ide_set_irq(drive, 1);
 
@@ -891,7 +900,7 @@ void ide_execute_command(ide_drive_t *dr
 
 	spin_lock_irqsave(&ide_lock, flags);
 	__ide_set_handler(drive, handler, timeout, expiry);
-	hwif->OUTBSYNC(hwif, cmd, hwif->io_ports.command_addr);
+	hwif->exec_command(hwif, cmd);
 	/*
 	 * Drive takes 400nS to respond, we must avoid the IRQ being
 	 * serviced before that.
@@ -909,7 +918,7 @@ void ide_execute_pkt_cmd(ide_drive_t *dr
 	unsigned long flags;
 
 	spin_lock_irqsave(&ide_lock, flags);
-	hwif->OUTBSYNC(hwif, WIN_PACKETCMD, hwif->io_ports.command_addr);
+	hwif->exec_command(hwif, WIN_PACKETCMD);
 	ndelay(400);
 	spin_unlock_irqrestore(&ide_lock, flags);
 }
@@ -1104,7 +1113,7 @@ static ide_startstop_t do_reset1 (ide_dr
 		pre_reset(drive);
 		SELECT_DRIVE(drive);
 		udelay (20);
-		hwif->OUTBSYNC(hwif, WIN_SRST, io_ports->command_addr);
+		hwif->exec_command(hwif, WIN_SRST);
 		ndelay(400);
 		hwgroup->poll_timeout = jiffies + WAIT_WORSTCASE;
 		hwgroup->polling = 1;
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -295,7 +295,7 @@ static int actual_try_to_identify (ide_d
 		hwif->OUTB(0, io_ports->feature_addr);
 
 	/* ask drive for ID */
-	hwif->OUTBSYNC(hwif, cmd, hwif->io_ports.command_addr);
+	hwif->exec_command(hwif, cmd);
 
 	timeout = ((cmd == WIN_IDENTIFY) ? WAIT_WORSTCASE : WAIT_PIDENTIFY) / 2;
 	timeout += jiffies;
@@ -482,7 +482,7 @@ static int do_probe (ide_drive_t *drive,
 			msleep(50);
 			SELECT_DRIVE(drive);
 			msleep(50);
-			hwif->OUTBSYNC(hwif, WIN_SRST, io_ports->command_addr);
+			hwif->exec_command(hwif, WIN_SRST);
 			(void)ide_busy_sleep(hwif);
 			rc = try_to_identify(drive, cmd);
 		}
@@ -518,7 +518,7 @@ static void enable_nest (ide_drive_t *dr
 	printk("%s: enabling %s -- ", hwif->name, drive->id->model);
 	SELECT_DRIVE(drive);
 	msleep(50);
-	hwif->OUTBSYNC(hwif, EXABYTE_ENABLE_NEST, hwif->io_ports.command_addr);
+	hwif->exec_command(hwif, EXABYTE_ENABLE_NEST);
 
 	if (ide_busy_sleep(hwif)) {
 		printk(KERN_CONT "failed (timeout)\n");
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -87,7 +87,7 @@ ide_startstop_t do_rw_taskfile (ide_driv
 	switch (task->data_phase) {
 	case TASKFILE_MULTI_OUT:
 	case TASKFILE_OUT:
-		hwif->OUTBSYNC(hwif, tf->command, hwif->io_ports.command_addr);
+		hwif->exec_command(hwif, tf->command);
 		ndelay(400);	/* FIXME */
 		return pre_task_out_intr(drive, task->rq);
 	case TASKFILE_MULTI_IN:
Index: b/drivers/ide/pci/scc_pata.c
===================================================================
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -126,6 +126,14 @@ static u8 scc_ide_inb(unsigned long port
 	return (u8)data;
 }
 
+static void scc_exec_command(ide_hwif_t *hwif, u8 cmd)
+{
+	out_be32((void *)hwif->io_ports.command_addr, cmd);
+	eieio();
+	in_be32((void *)(hwif->dma_base + 0x01c));
+	eieio();
+}
+
 static u8 scc_read_sff_dma_status(ide_hwif_t *hwif)
 {
 	return (u8)in_be32((void *)(hwif->dma_base + 4));
@@ -780,6 +788,7 @@ static void __devinit init_mmio_iops_scc
 
 	ide_set_hwifdata(hwif, ports);
 
+	hwif->exec_command	  = scc_exec_command;
 	hwif->read_sff_dma_status = scc_read_sff_dma_status;
 
 	hwif->tf_load = scc_tf_load;
Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -495,6 +495,13 @@ static void pmac_outbsync(ide_hwif_t *hw
 				     + IDE_TIMING_CONFIG));
 }
 
+static void pmac_exec_command(ide_hwif_t *hwif, u8 cmd)
+{
+	writeb(cmd, (void __iomem *)hwif->io_ports.command_addr);
+	(void)readl((void __iomem *)(hwif->io_ports.data_addr
+				     + IDE_TIMING_CONFIG));
+}
+
 /*
  * Old tuning functions (called on hdparm -p), sets up drive PIO timings
  */
@@ -1086,6 +1093,8 @@ static int __devinit pmac_ide_setup_devi
 	if (hwif == NULL)
 		return -ENOENT;
 
+	hwif->exec_command = pmac_exec_command;
+
 	/* Setup MMIO ops */
 	default_hwif_mmiops(hwif);
        	hwif->OUTBSYNC = pmac_outbsync;
Index: b/drivers/scsi/ide-scsi.c
===================================================================
--- a/drivers/scsi/ide-scsi.c
+++ b/drivers/scsi/ide-scsi.c
@@ -248,8 +248,7 @@ idescsi_atapi_error(ide_drive_t *drive, 
 
 	if (ide_read_status(drive) & (BUSY_STAT | DRQ_STAT))
 		/* force an abort */
-		hwif->OUTBSYNC(hwif, WIN_IDLEIMMEDIATE,
-			       hwif->io_ports.command_addr);
+		hwif->exec_command(hwif, WIN_IDLEIMMEDIATE);
 
 	rq->errors++;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -468,7 +468,8 @@ typedef struct hwif_s {
 	const struct ide_port_ops	*port_ops;
 	const struct ide_dma_ops	*dma_ops;
 
-	u8 (*read_sff_dma_status)(struct hwif_s *);
+	void	(*exec_command)(struct hwif_s *, u8);
+	u8	(*read_sff_dma_status)(struct hwif_s *);
 
 	void (*tf_load)(ide_drive_t *, struct ide_task_s *);
 	void (*tf_read)(ide_drive_t *, struct ide_task_s *);

[PATCH 07/18] ide: add ->read_status method

[Bartlomiej Zolnierkiewicz]

* Remove ide_read_status() inline helper.

* Add ->read_status method for reading ATA Status register
  and use it instead of ->INB.

While at it:

* Don't use HWGROUP() macro.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/arm/icside.c   |    4 +++-
 drivers/ide/ide-atapi.c    |    2 +-
 drivers/ide/ide-cd.c       |   12 ++++++++----
 drivers/ide/ide-dma.c      |    5 +++--
 drivers/ide/ide-floppy.c   |    3 ++-
 drivers/ide/ide-io.c       |   19 ++++++++++---------
 drivers/ide/ide-iops.c     |   33 ++++++++++++++++++++++-----------
 drivers/ide/ide-probe.c    |   22 +++++++++++-----------
 drivers/ide/ide-tape.c     |    6 ++++--
 drivers/ide/ide-taskfile.c |   23 ++++++++++++++---------
 drivers/ide/pci/ns87415.c  |    8 +++++++-
 drivers/ide/pci/scc_pata.c |    6 ++++++
 drivers/ide/pci/sgiioc4.c  |   19 ++++++++++---------
 drivers/scsi/ide-scsi.c    |    2 +-
 include/linux/ide.h        |    8 +-------
 15 files changed, 103 insertions(+), 69 deletions(-)

Index: b/drivers/ide/arm/icside.c
===================================================================
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -375,12 +375,14 @@ static int icside_dma_test_irq(ide_drive
 
 static void icside_dma_timeout(ide_drive_t *drive)
 {
+	ide_hwif_t *hwif = drive->hwif;
+
 	printk(KERN_ERR "%s: DMA timeout occurred: ", drive->name);
 
 	if (icside_dma_test_irq(drive))
 		return;
 
-	ide_dump_status(drive, "DMA timeout", ide_read_status(drive));
+	ide_dump_status(drive, "DMA timeout", hwif->read_status(hwif));
 
 	icside_dma_end(drive);
 }
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -35,7 +35,7 @@ ide_startstop_t ide_pc_intr(ide_drive_t 
 	}
 
 	/* Clear the interrupt */
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (pc->flags & PC_FLAG_DMA_IN_PROGRESS) {
 		if (hwif->dma_ops->dma_end(drive) ||
Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -280,11 +280,12 @@ static void ide_dump_status_no_sense(ide
  */
 static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret)
 {
-	struct request *rq = HWGROUP(drive)->rq;
+	ide_hwif_t *hwif = drive->hwif;
+	struct request *rq = hwif->hwgroup->rq;
 	int stat, err, sense_key;
 
 	/* check for errors */
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (stat_ret)
 		*stat_ret = stat;
@@ -606,6 +607,8 @@ static ide_startstop_t cdrom_transfer_pa
 static int ide_cd_check_ireason(ide_drive_t *drive, struct request *rq,
 				int len, int ireason, int rw)
 {
+	ide_hwif_t *hwif = drive->hwif;
+
 	/*
 	 * ireason == 0: the drive wants to receive data from us
 	 * ireason == 2: the drive is expecting to transfer data to us
@@ -624,7 +627,7 @@ static int ide_cd_check_ireason(ide_driv
 		 * Some drives (ASUS) seem to tell us that status info is
 		 * available.  Just get it and ignore.
 		 */
-		(void)ide_read_status(drive);
+		(void)hwif->read_status(hwif);
 		return 0;
 	} else {
 		/* drive wants a command packet, or invalid ireason... */
@@ -1188,8 +1191,9 @@ static ide_startstop_t ide_cd_do_request
 
 	if (blk_fs_request(rq)) {
 		if (info->cd_flags & IDE_CD_FLAG_SEEKING) {
+			ide_hwif_t *hwif = drive->hwif;
 			unsigned long elapsed = jiffies - info->start_seek;
-			int stat = ide_read_status(drive);
+			int stat = hwif->read_status(hwif);
 
 			if ((stat & SEEK_STAT) != SEEK_STAT) {
 				if (elapsed < IDECD_SEEK_TIMEOUT) {
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -100,10 +100,11 @@ static const struct drive_list_entry dri
  
 ide_startstop_t ide_dma_intr (ide_drive_t *drive)
 {
+	ide_hwif_t *hwif = drive->hwif;
 	u8 stat = 0, dma_stat = 0;
 
-	dma_stat = drive->hwif->dma_ops->dma_end(drive);
-	stat = ide_read_status(drive);
+	dma_stat = hwif->dma_ops->dma_end(drive);
+	stat = hwif->read_status(hwif);
 
 	if (OK_STAT(stat,DRIVE_READY,drive->bad_wstat|DRQ_STAT)) {
 		if (!dma_stat) {
Index: b/drivers/ide/ide-floppy.c
===================================================================
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -949,11 +949,12 @@ static int idefloppy_get_format_progress
 
 		/* Else assume format_unit has finished, and we're at 0x10000 */
 	} else {
+		ide_hwif_t *hwif = drive->hwif;
 		unsigned long flags;
 		u8 stat;
 
 		local_irq_save(flags);
-		stat = ide_read_status(drive);
+		stat = hwif->read_status(hwif);
 		local_irq_restore(flags);
 
 		progress_indication = ((stat & SEEK_STAT) == 0) ? 0 : 0x10000;
Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -381,8 +381,7 @@ static ide_startstop_t ide_ata_error(ide
 		if (err == ABRT_ERR) {
 			if (drive->select.b.lba &&
 			    /* some newer drives don't support WIN_SPECIFY */
-			    hwif->INB(hwif->io_ports.command_addr) ==
-				WIN_SPECIFY)
+			    hwif->read_status(hwif) == WIN_SPECIFY)
 				return ide_stopped;
 		} else if ((err & BAD_CRC) == BAD_CRC) {
 			/* UDMA crc error, just retry the operation */
@@ -408,7 +407,7 @@ static ide_startstop_t ide_ata_error(ide
 		return ide_stopped;
 	}
 
-	if (ide_read_status(drive) & (BUSY_STAT | DRQ_STAT))
+	if (hwif->read_status(hwif) & (BUSY_STAT | DRQ_STAT))
 		rq->errors |= ERROR_RESET;
 
 	if ((rq->errors & ERROR_RESET) == ERROR_RESET) {
@@ -435,7 +434,7 @@ static ide_startstop_t ide_atapi_error(i
 		/* add decoding error stuff */
 	}
 
-	if (ide_read_status(drive) & (BUSY_STAT | DRQ_STAT))
+	if (hwif->read_status(hwif) & (BUSY_STAT | DRQ_STAT))
 		/* force an abort */
 		hwif->exec_command(hwif, WIN_IDLEIMMEDIATE);
 
@@ -760,7 +759,8 @@ static ide_startstop_t execute_drive_cmd
 #ifdef DEBUG
  	printk("%s: DRIVE_CMD (null)\n", drive->name);
 #endif
-	ide_end_drive_cmd(drive, ide_read_status(drive), ide_read_error(drive));
+	ide_end_drive_cmd(drive, hwif->read_status(hwif),
+			  ide_read_error(drive));
 
  	return ide_stopped;
 }
@@ -1169,7 +1169,7 @@ static ide_startstop_t ide_dma_timeout_r
 		printk(KERN_WARNING "%s: DMA timeout error\n", drive->name);
 		(void)hwif->dma_ops->dma_end(drive);
 		ret = ide_error(drive, "dma timeout error",
-				ide_read_status(drive));
+				hwif->read_status(hwif));
 	} else {
 		printk(KERN_WARNING "%s: DMA timeout retry\n", drive->name);
 		hwif->dma_ops->dma_timeout(drive);
@@ -1294,7 +1294,7 @@ void ide_timer_expiry (unsigned long dat
 				} else
 					startstop =
 					ide_error(drive, "irq timeout",
-						  ide_read_status(drive));
+						  hwif->read_status(hwif));
 			}
 			drive->service_time = jiffies - drive->service_start;
 			spin_lock_irq(&ide_lock);
@@ -1350,7 +1350,8 @@ static void unexpected_intr (int irq, id
 	 */
 	do {
 		if (hwif->irq == irq) {
-			stat = hwif->INB(hwif->io_ports.status_addr);
+			stat = hwif->read_status(hwif);
+
 			if (!OK_STAT(stat, READY_STAT, BAD_STAT)) {
 				/* Try to not flood the console with msgs */
 				static unsigned long last_msgtime, count;
@@ -1440,7 +1441,7 @@ irqreturn_t ide_intr (int irq, void *dev
 			 * Whack the status register, just in case
 			 * we have a leftover pending IRQ.
 			 */
-			(void) hwif->INB(hwif->io_ports.status_addr);
+			(void)hwif->read_status(hwif);
 #endif /* CONFIG_BLK_DEV_IDEPCI */
 		}
 		spin_unlock_irqrestore(&ide_lock, flags);
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -111,6 +111,14 @@ static void ide_exec_command(ide_hwif_t 
 		outb(cmd, hwif->io_ports.command_addr);
 }
 
+static u8 ide_read_status(ide_hwif_t *hwif)
+{
+	if (hwif->host_flags & IDE_HFLAG_MMIO)
+		return readb((void __iomem *)hwif->io_ports.status_addr);
+	else
+		return inb(hwif->io_ports.status_addr);
+}
+
 static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
 {
 	if (hwif->host_flags & IDE_HFLAG_MMIO)
@@ -340,6 +348,7 @@ static void ata_output_data(ide_drive_t 
 void default_hwif_transport(ide_hwif_t *hwif)
 {
 	hwif->exec_command	  = ide_exec_command;
+	hwif->read_status	  = ide_read_status;
 	hwif->read_sff_dma_status = ide_read_sff_dma_status;
 
 	hwif->tf_load	  = ide_tf_load;
@@ -505,7 +514,7 @@ int drive_is_ready (ide_drive_t *drive)
 		stat = ide_read_altstatus(drive);
 	else
 		/* Note: this may clear a pending IRQ!! */
-		stat = ide_read_status(drive);
+		stat = hwif->read_status(hwif);
 
 	if (stat & BUSY_STAT)
 		/* drive busy:  definitely not interrupting */
@@ -530,24 +539,25 @@ EXPORT_SYMBOL(drive_is_ready);
  */
 static int __ide_wait_stat(ide_drive_t *drive, u8 good, u8 bad, unsigned long timeout, u8 *rstat)
 {
+	ide_hwif_t *hwif = drive->hwif;
 	unsigned long flags;
 	int i;
 	u8 stat;
 
 	udelay(1);	/* spec allows drive 400ns to assert "BUSY" */
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (stat & BUSY_STAT) {
 		local_irq_set(flags);
 		timeout += jiffies;
-		while ((stat = ide_read_status(drive)) & BUSY_STAT) {
+		while ((stat = hwif->read_status(hwif)) & BUSY_STAT) {
 			if (time_after(jiffies, timeout)) {
 				/*
 				 * One last read after the timeout in case
 				 * heavy interrupt load made us not make any
 				 * progress during the timeout..
 				 */
-				stat = ide_read_status(drive);
+				stat = hwif->read_status(hwif);
 				if (!(stat & BUSY_STAT))
 					break;
 
@@ -567,7 +577,7 @@ static int __ide_wait_stat(ide_drive_t *
 	 */
 	for (i = 0; i < 10; i++) {
 		udelay(1);
-		stat = ide_read_status(drive);
+		stat = hwif->read_status(hwif);
 
 		if (OK_STAT(stat, good, bad)) {
 			*rstat = stat;
@@ -718,7 +728,7 @@ int ide_driveid_update(ide_drive_t *driv
 	} while (stat & BUSY_STAT);
 
 	msleep(50);	/* wait for IRQ and DRQ_STAT */
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (!OK_STAT(stat, DRQ_STAT, BAD_R_STAT)) {
 		SELECT_MASK(drive, 0);
@@ -733,7 +743,7 @@ int ide_driveid_update(ide_drive_t *driv
 		return 0;
 	}
 	hwif->input_data(drive, NULL, id, SECTOR_SIZE);
-	(void)ide_read_status(drive);	/* clear drive IRQ */
+	(void)hwif->read_status(hwif);	/* clear drive IRQ */
 	local_irq_enable();
 	local_irq_restore(flags);
 	ide_fix_driveid(id);
@@ -935,12 +945,13 @@ static ide_startstop_t do_reset1 (ide_dr
  */
 static ide_startstop_t atapi_reset_pollfunc (ide_drive_t *drive)
 {
-	ide_hwgroup_t *hwgroup	= HWGROUP(drive);
+	ide_hwif_t *hwif = drive->hwif;
+	ide_hwgroup_t *hwgroup = hwif->hwgroup;
 	u8 stat;
 
 	SELECT_DRIVE(drive);
 	udelay (10);
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (OK_STAT(stat, 0, BUSY_STAT))
 		printk("%s: ATAPI reset complete\n", drive->name);
@@ -984,7 +995,7 @@ static ide_startstop_t reset_pollfunc (i
 		}
 	}
 
-	tmp = ide_read_status(drive);
+	tmp = hwif->read_status(hwif);
 
 	if (!OK_STAT(tmp, 0, BUSY_STAT)) {
 		if (time_before(jiffies, hwgroup->poll_timeout)) {
@@ -1196,7 +1207,7 @@ int ide_wait_not_busy(ide_hwif_t *hwif, 
 		 * about locking issues (2.5 work ?).
 		 */
 		mdelay(1);
-		stat = hwif->INB(hwif->io_ports.status_addr);
+		stat = hwif->read_status(hwif);
 		if ((stat & BUSY_STAT) == 0)
 			return 0;
 		/*
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -276,7 +276,7 @@ static int actual_try_to_identify (ide_d
 
 	if (io_ports->ctl_addr) {
 		a = ide_read_altstatus(drive);
-		s = ide_read_status(drive);
+		s = hwif->read_status(hwif);
 		if ((a ^ s) & ~INDEX_STAT)
 			/* ancient Seagate drives, broken interfaces */
 			printk(KERN_INFO "%s: probing with STATUS(0x%02x) "
@@ -307,12 +307,12 @@ static int actual_try_to_identify (ide_d
 		/* give drive a breather */
 		msleep(50);
 		s = use_altstatus ? ide_read_altstatus(drive)
-				  : ide_read_status(drive);
+				  : hwif->read_status(hwif);
 	} while (s & BUSY_STAT);
 
 	/* wait for IRQ and DRQ_STAT */
 	msleep(50);
-	s = ide_read_status(drive);
+	s = hwif->read_status(hwif);
 
 	if (OK_STAT(s, DRQ_STAT, BAD_R_STAT)) {
 		unsigned long flags;
@@ -324,7 +324,7 @@ static int actual_try_to_identify (ide_d
 		/* drive responded with ID */
 		rc = 0;
 		/* clear drive IRQ */
-		(void)ide_read_status(drive);
+		(void)hwif->read_status(hwif);
 		local_irq_restore(flags);
 	} else {
 		/* drive refused ID */
@@ -371,7 +371,7 @@ static int try_to_identify (ide_drive_t 
 
 		ide_set_irq(drive, 0);
 		/* clear drive IRQ */
-		(void)ide_read_status(drive);
+		(void)hwif->read_status(hwif);
 		udelay(5);
 		irq = probe_irq_off(cookie);
 		if (!hwif->irq) {
@@ -396,7 +396,7 @@ static int ide_busy_sleep(ide_hwif_t *hw
 
 	do {
 		msleep(50);
-		stat = hwif->INB(hwif->io_ports.status_addr);
+		stat = hwif->read_status(hwif);
 		if ((stat & BUSY_STAT) == 0)
 			return 0;
 	} while (time_before(jiffies, timeout));
@@ -461,7 +461,7 @@ static int do_probe (ide_drive_t *drive,
 		return 3;
 	}
 
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (OK_STAT(stat, READY_STAT, BUSY_STAT) ||
 	    drive->present || cmd == WIN_PIDENTIFY) {
@@ -471,7 +471,7 @@ static int do_probe (ide_drive_t *drive,
 			rc = try_to_identify(drive,cmd);
 		}
 
-		stat = ide_read_status(drive);
+		stat = hwif->read_status(hwif);
 
 		if (stat == (BUSY_STAT | READY_STAT))
 			return 4;
@@ -488,7 +488,7 @@ static int do_probe (ide_drive_t *drive,
 		}
 
 		/* ensure drive IRQ is clear */
-		stat = ide_read_status(drive);
+		stat = hwif->read_status(hwif);
 
 		if (rc == 1)
 			printk(KERN_ERR "%s: no response (status = 0x%02x)\n",
@@ -502,7 +502,7 @@ static int do_probe (ide_drive_t *drive,
 		SELECT_DRIVE(&hwif->drives[0]);
 		msleep(50);
 		/* ensure drive irq is clear */
-		(void)ide_read_status(drive);
+		(void)hwif->read_status(hwif);
 	}
 	return rc;
 }
@@ -527,7 +527,7 @@ static void enable_nest (ide_drive_t *dr
 
 	msleep(50);
 
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (!OK_STAT(stat, 0, BAD_STAT))
 		printk(KERN_CONT "failed (status = 0x%02x)\n", stat);
Index: b/drivers/ide/ide-tape.c
===================================================================
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -927,11 +927,12 @@ static void idetape_create_mode_sense_cm
 
 static ide_startstop_t idetape_media_access_finished(ide_drive_t *drive)
 {
+	ide_hwif_t *hwif = drive->hwif;
 	idetape_tape_t *tape = drive->driver_data;
 	struct ide_atapi_pc *pc = tape->pc;
 	u8 stat;
 
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (stat & SEEK_STAT) {
 		if (stat & ERR_STAT) {
@@ -980,6 +981,7 @@ static void ide_tape_create_rw_cmd(ideta
 static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 					  struct request *rq, sector_t block)
 {
+	ide_hwif_t *hwif = drive->hwif;
 	idetape_tape_t *tape = drive->driver_data;
 	struct ide_atapi_pc *pc = NULL;
 	struct request *postponed_rq = tape->postponed_rq;
@@ -1017,7 +1019,7 @@ static ide_startstop_t idetape_do_reques
 	 * If the tape is still busy, postpone our request and service
 	 * the other device meanwhile.
 	 */
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (!drive->dsc_overlap && !(rq->cmd[0] & REQ_IDETAPE_PC2))
 		set_bit(IDETAPE_FLAG_IGNORE_DSC, &tape->flags);
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -124,7 +124,8 @@ EXPORT_SYMBOL_GPL(do_rw_taskfile);
  */
 static ide_startstop_t set_multmode_intr(ide_drive_t *drive)
 {
-	u8 stat = ide_read_status(drive);
+	ide_hwif_t *hwif = drive->hwif;
+	u8 stat = hwif->read_status(hwif);
 
 	if (OK_STAT(stat, READY_STAT, BAD_STAT))
 		drive->mult_count = drive->mult_req;
@@ -141,10 +142,11 @@ static ide_startstop_t set_multmode_intr
  */
 static ide_startstop_t set_geometry_intr(ide_drive_t *drive)
 {
+	ide_hwif_t *hwif = drive->hwif;
 	int retries = 5;
 	u8 stat;
 
-	while (((stat = ide_read_status(drive)) & BUSY_STAT) && retries--)
+	while (((stat = hwif->read_status(hwif)) & BUSY_STAT) && retries--)
 		udelay(10);
 
 	if (OK_STAT(stat, READY_STAT, BAD_STAT))
@@ -162,7 +164,8 @@ static ide_startstop_t set_geometry_intr
  */
 static ide_startstop_t recal_intr(ide_drive_t *drive)
 {
-	u8 stat = ide_read_status(drive);
+	ide_hwif_t *hwif = drive->hwif;
+	u8 stat = hwif->read_status(hwif);
 
 	if (!OK_STAT(stat, READY_STAT, BAD_STAT))
 		return ide_error(drive, "recal_intr", stat);
@@ -174,11 +177,12 @@ static ide_startstop_t recal_intr(ide_dr
  */
 static ide_startstop_t task_no_data_intr(ide_drive_t *drive)
 {
-	ide_task_t *args	= HWGROUP(drive)->rq->special;
+	ide_hwif_t *hwif = drive->hwif;
+	ide_task_t *args = hwif->hwgroup->rq->special;
 	u8 stat;
 
 	local_irq_enable_in_hardirq();
-	stat = ide_read_status(drive);
+	stat = hwif->read_status(hwif);
 
 	if (!OK_STAT(stat, READY_STAT, BAD_STAT))
 		return ide_error(drive, "task_no_data_intr", stat);
@@ -192,6 +196,7 @@ static ide_startstop_t task_no_data_intr
 
 static u8 wait_drive_not_busy(ide_drive_t *drive)
 {
+	ide_hwif_t *hwif = drive->hwif;
 	int retries;
 	u8 stat;
 
@@ -200,7 +205,7 @@ static u8 wait_drive_not_busy(ide_drive_
 	 * take up to 6 ms on some ATAPI devices, so we will wait max 10 ms.
 	 */
 	for (retries = 0; retries < 1000; retries++) {
-		stat = ide_read_status(drive);
+		stat = hwif->read_status(hwif);
 
 		if (stat & BUSY_STAT)
 			udelay(10);
@@ -383,8 +388,8 @@ static ide_startstop_t task_in_unexpecte
 static ide_startstop_t task_in_intr(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
-	struct request *rq = HWGROUP(drive)->rq;
-	u8 stat = ide_read_status(drive);
+	struct request *rq = hwif->hwgroup->rq;
+	u8 stat = hwif->read_status(hwif);
 
 	/* Error? */
 	if (stat & ERR_STAT)
@@ -418,7 +423,7 @@ static ide_startstop_t task_out_intr (id
 {
 	ide_hwif_t *hwif = drive->hwif;
 	struct request *rq = HWGROUP(drive)->rq;
-	u8 stat = ide_read_status(drive);
+	u8 stat = hwif->read_status(hwif);
 
 	if (!OK_STAT(stat, DRIVE_READY, drive->bad_wstat))
 		return task_error(drive, rq, __func__, stat);
Index: b/drivers/ide/pci/ns87415.c
===================================================================
--- a/drivers/ide/pci/ns87415.c
+++ b/drivers/ide/pci/ns87415.c
@@ -63,6 +63,11 @@ static u8 superio_ide_inb (unsigned long
 	return inb(port);
 }
 
+static u8 superio_read_status(ide_hwif_t *hwif)
+{
+	return superio_ide_inb(hwif->io_ports.status_addr);
+}
+
 static u8 superio_read_sff_dma_status(ide_hwif_t *hwif)
 {
 	return superio_ide_inb(hwif->dma_base + ATA_DMA_STATUS);
@@ -127,6 +132,7 @@ static void __devinit superio_ide_init_i
 	tmp = superio_ide_inb(superio_ide_dma_status[port]);
 	outb(tmp | 0x66, superio_ide_dma_status[port]);
 
+	hwif->read_status	  = superio_read_status;
 	hwif->read_sff_dma_status = superio_read_sff_dma_status;
 
 	hwif->tf_read = superio_tf_read;
@@ -287,7 +293,7 @@ static void __devinit init_hwif_ns87415 
 		outb(8, hwif->io_ports.ctl_addr);
 		do {
 			udelay(50);
-			stat = hwif->INB(hwif->io_ports.status_addr);
+			stat = hwif->read_status(hwif);
                 	if (stat == 0xff)
                         	break;
         	} while ((stat & BUSY_STAT) && --timeout);
Index: b/drivers/ide/pci/scc_pata.c
===================================================================
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -134,6 +134,11 @@ static void scc_exec_command(ide_hwif_t 
 	eieio();
 }
 
+static u8 scc_read_status(ide_hwif_t *hwif)
+{
+	return (u8)in_be32((void *)hwif->io_ports.status_addr);
+}
+
 static u8 scc_read_sff_dma_status(ide_hwif_t *hwif)
 {
 	return (u8)in_be32((void *)(hwif->dma_base + 4));
@@ -789,6 +794,7 @@ static void __devinit init_mmio_iops_scc
 	ide_set_hwifdata(hwif, ports);
 
 	hwif->exec_command	  = scc_exec_command;
+	hwif->read_status	  = scc_read_status;
 	hwif->read_sff_dma_status = scc_read_sff_dma_status;
 
 	hwif->tf_load = scc_tf_load;
Index: b/drivers/ide/pci/sgiioc4.c
===================================================================
--- a/drivers/ide/pci/sgiioc4.c
+++ b/drivers/ide/pci/sgiioc4.c
@@ -127,7 +127,7 @@ sgiioc4_checkirq(ide_hwif_t * hwif)
 	return 0;
 }
 
-static u8 sgiioc4_INB(unsigned long);
+static u8 sgiioc4_read_status(ide_hwif_t *);
 
 static int
 sgiioc4_clearirq(ide_drive_t * drive)
@@ -141,18 +141,19 @@ sgiioc4_clearirq(ide_drive_t * drive)
 	intr_reg = readl((void __iomem *)other_ir);
 	if (intr_reg & 0x03) { /* Valid IOC4-IDE interrupt */
 		/*
-		 * Using sgiioc4_INB to read the Status register has a side
-		 * effect of clearing the interrupt.  The first read should
+		 * Using sgiioc4_read_status to read the Status register has a
+		 * side effect of clearing the interrupt.  The first read should
 		 * clear it if it is set.  The second read should return
 		 * a "clear" status if it got cleared.  If not, then spin
 		 * for a bit trying to clear it.
 		 */
-		u8 stat = sgiioc4_INB(io_ports->status_addr);
+		u8 stat = sgiioc4_read_status(hwif);
 		int count = 0;
-		stat = sgiioc4_INB(io_ports->status_addr);
+
+		stat = sgiioc4_read_status(hwif);
 		while ((stat & 0x80) && (count++ < 100)) {
 			udelay(1);
-			stat = sgiioc4_INB(io_ports->status_addr);
+			stat = sgiioc4_read_status(hwif);
 		}
 
 		if (intr_reg & 0x02) {
@@ -304,9 +305,9 @@ sgiioc4_dma_lost_irq(ide_drive_t * drive
 	ide_dma_lost_irq(drive);
 }
 
-static u8
-sgiioc4_INB(unsigned long port)
+static u8 sgiioc4_read_status(ide_hwif_t *hwif)
 {
+	unsigned long port = hwif->io_ports.status_addr;
 	u8 reg = (u8) readb((void __iomem *) port);
 
 	if ((port & 0xFFF) == 0x11C) {	/* Status register of IOC4 */
@@ -628,7 +629,7 @@ sgiioc4_ide_setup_pci_device(struct pci_
 	/* Initializing chipset IRQ Registers */
 	writel(0x03, (void __iomem *)(irqport + IOC4_INTR_SET * 4));
 
-	hwif->INB = &sgiioc4_INB;
+	hwif->read_status = sgiioc4_read_status;
 
 	idx[0] = hwif->index;
 
Index: b/drivers/scsi/ide-scsi.c
===================================================================
--- a/drivers/scsi/ide-scsi.c
+++ b/drivers/scsi/ide-scsi.c
@@ -246,7 +246,7 @@ idescsi_atapi_error(ide_drive_t *drive, 
 {
 	ide_hwif_t *hwif = drive->hwif;
 
-	if (ide_read_status(drive) & (BUSY_STAT | DRQ_STAT))
+	if (hwif->read_status(hwif) & (BUSY_STAT | DRQ_STAT))
 		/* force an abort */
 		hwif->exec_command(hwif, WIN_IDLEIMMEDIATE);
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -469,6 +469,7 @@ typedef struct hwif_s {
 	const struct ide_dma_ops	*dma_ops;
 
 	void	(*exec_command)(struct hwif_s *, u8);
+	u8	(*read_status)(struct hwif_s *);
 	u8	(*read_sff_dma_status)(struct hwif_s *);
 
 	void (*tf_load)(ide_drive_t *, struct ide_task_s *);
@@ -1348,13 +1349,6 @@ static inline void ide_set_irq(ide_drive
 		       hwif->io_ports.ctl_addr);
 }
 
-static inline u8 ide_read_status(ide_drive_t *drive)
-{
-	ide_hwif_t *hwif = drive->hwif;
-
-	return hwif->INB(hwif->io_ports.status_addr);
-}
-
 static inline u8 ide_read_altstatus(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;

[PATCH 08/18] ide: add ->read_altstatus method

[Bartlomiej Zolnierkiewicz]

* Remove ide_read_altstatus() inline helper.

* Add ->read_altstatus method for reading ATA Alternate Status
  register and use it instead of ->INB.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-iops.c     |   13 +++++++++++--
 drivers/ide/ide-probe.c    |    4 ++--
 drivers/ide/pci/scc_pata.c |    6 ++++++
 include/linux/ide.h        |    8 +-------
 4 files changed, 20 insertions(+), 11 deletions(-)

Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -119,6 +119,14 @@ static u8 ide_read_status(ide_hwif_t *hw
 		return inb(hwif->io_ports.status_addr);
 }
 
+static u8 ide_read_altstatus(ide_hwif_t *hwif)
+{
+	if (hwif->host_flags & IDE_HFLAG_MMIO)
+		return readb((void __iomem *)hwif->io_ports.ctl_addr);
+	else
+		return inb(hwif->io_ports.ctl_addr);
+}
+
 static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
 {
 	if (hwif->host_flags & IDE_HFLAG_MMIO)
@@ -349,6 +357,7 @@ void default_hwif_transport(ide_hwif_t *
 {
 	hwif->exec_command	  = ide_exec_command;
 	hwif->read_status	  = ide_read_status;
+	hwif->read_altstatus	  = ide_read_altstatus;
 	hwif->read_sff_dma_status = ide_read_sff_dma_status;
 
 	hwif->tf_load	  = ide_tf_load;
@@ -511,7 +520,7 @@ int drive_is_ready (ide_drive_t *drive)
 	 * about possible isa-pnp and pci-pnp issues yet.
 	 */
 	if (hwif->io_ports.ctl_addr)
-		stat = ide_read_altstatus(drive);
+		stat = hwif->read_altstatus(hwif);
 	else
 		/* Note: this may clear a pending IRQ!! */
 		stat = hwif->read_status(hwif);
@@ -724,7 +733,7 @@ int ide_driveid_update(ide_drive_t *driv
 		}
 
 		msleep(50);	/* give drive a breather */
-		stat = ide_read_altstatus(drive);
+		stat = hwif->read_altstatus(hwif);
 	} while (stat & BUSY_STAT);
 
 	msleep(50);	/* wait for IRQ and DRQ_STAT */
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -275,7 +275,7 @@ static int actual_try_to_identify (ide_d
 	msleep(50);
 
 	if (io_ports->ctl_addr) {
-		a = ide_read_altstatus(drive);
+		a = hwif->read_altstatus(hwif);
 		s = hwif->read_status(hwif);
 		if ((a ^ s) & ~INDEX_STAT)
 			/* ancient Seagate drives, broken interfaces */
@@ -306,7 +306,7 @@ static int actual_try_to_identify (ide_d
 		}
 		/* give drive a breather */
 		msleep(50);
-		s = use_altstatus ? ide_read_altstatus(drive)
+		s = use_altstatus ? hwif->read_altstatus(hwif)
 				  : hwif->read_status(hwif);
 	} while (s & BUSY_STAT);
 
Index: b/drivers/ide/pci/scc_pata.c
===================================================================
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -139,6 +139,11 @@ static u8 scc_read_status(ide_hwif_t *hw
 	return (u8)in_be32((void *)hwif->io_ports.status_addr);
 }
 
+static u8 scc_read_altstatus(ide_hwif_t *hwif)
+{
+	return (u8)in_be32((void *)hwif->io_ports.ctl_addr);
+}
+
 static u8 scc_read_sff_dma_status(ide_hwif_t *hwif)
 {
 	return (u8)in_be32((void *)(hwif->dma_base + 4));
@@ -795,6 +800,7 @@ static void __devinit init_mmio_iops_scc
 
 	hwif->exec_command	  = scc_exec_command;
 	hwif->read_status	  = scc_read_status;
+	hwif->read_altstatus	  = scc_read_altstatus;
 	hwif->read_sff_dma_status = scc_read_sff_dma_status;
 
 	hwif->tf_load = scc_tf_load;
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -470,6 +470,7 @@ typedef struct hwif_s {
 
 	void	(*exec_command)(struct hwif_s *, u8);
 	u8	(*read_status)(struct hwif_s *);
+	u8	(*read_altstatus)(struct hwif_s *);
 	u8	(*read_sff_dma_status)(struct hwif_s *);
 
 	void (*tf_load)(ide_drive_t *, struct ide_task_s *);
@@ -1349,13 +1350,6 @@ static inline void ide_set_irq(ide_drive
 		       hwif->io_ports.ctl_addr);
 }
 
-static inline u8 ide_read_altstatus(ide_drive_t *drive)
-{
-	ide_hwif_t *hwif = drive->hwif;
-
-	return hwif->INB(hwif->io_ports.ctl_addr);
-}
-
 static inline u8 ide_read_error(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;

[PATCH 09/18] ide: add ->set_irq method

[Bartlomiej Zolnierkiewicz]

Add ->set_irq method for setting nIEN bit of ATA Device Control
register and use it instead of ide_set_irq().

While at it:

* Use ->set_irq in init_irq() and do_reset1().

* Don't use HWIF() macro in ide_check_pm_state().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-io.c       |   14 ++++++++------
 drivers/ide/ide-iops.c     |   37 +++++++++++++++++++++++++++----------
 drivers/ide/ide-probe.c    |    9 ++++-----
 drivers/ide/ide-taskfile.c |    2 +-
 drivers/ide/pci/scc_pata.c |   19 +++++++++++++++++++
 drivers/ide/ppc/pmac.c     |   17 +++++++++++++++++
 include/linux/ide.h        |   10 ++--------
 7 files changed, 78 insertions(+), 30 deletions(-)

Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -783,16 +783,17 @@ static void ide_check_pm_state(ide_drive
 		 * the bus may be broken enough to walk on our toes at this
 		 * point.
 		 */
+		ide_hwif_t *hwif = drive->hwif;
 		int rc;
 #ifdef DEBUG_PM
 		printk("%s: Wakeup request inited, waiting for !BSY...\n", drive->name);
 #endif
-		rc = ide_wait_not_busy(HWIF(drive), 35000);
+		rc = ide_wait_not_busy(hwif, 35000);
 		if (rc)
 			printk(KERN_WARNING "%s: bus not ready on wakeup\n", drive->name);
 		SELECT_DRIVE(drive);
-		ide_set_irq(drive, 1);
-		rc = ide_wait_not_busy(HWIF(drive), 100000);
+		hwif->set_irq(hwif, 1);
+		rc = ide_wait_not_busy(hwif, 100000);
 		if (rc)
 			printk(KERN_WARNING "%s: drive not ready on wakeup\n", drive->name);
 	}
@@ -1069,7 +1070,7 @@ static void ide_do_request (ide_hwgroup_
 			 * quirk_list may not like intr setups/cleanups
 			 */
 			if (drive->quirk_list != 1)
-				ide_set_irq(drive, 0);
+				hwif->set_irq(hwif, 0);
 		}
 		hwgroup->hwif = hwif;
 		hwgroup->drive = drive;
@@ -1547,6 +1548,7 @@ EXPORT_SYMBOL(ide_do_drive_cmd);
 
 void ide_pktcmd_tf_load(ide_drive_t *drive, u32 tf_flags, u16 bcount, u8 dma)
 {
+	ide_hwif_t *hwif = drive->hwif;
 	ide_task_t task;
 
 	memset(&task, 0, sizeof(task));
@@ -1557,9 +1559,9 @@ void ide_pktcmd_tf_load(ide_drive_t *dri
 	task.tf.lbah    = (bcount >> 8) & 0xff;
 
 	ide_tf_dump(drive->name, &task.tf);
-	ide_set_irq(drive, 1);
+	hwif->set_irq(hwif, 1);
 	SELECT_MASK(drive, 0);
-	drive->hwif->tf_load(drive, &task);
+	hwif->tf_load(drive, &task);
 }
 
 EXPORT_SYMBOL_GPL(ide_pktcmd_tf_load);
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -135,6 +135,23 @@ static u8 ide_read_sff_dma_status(ide_hw
 		return inb(hwif->dma_base + ATA_DMA_STATUS);
 }
 
+static void ide_set_irq(ide_hwif_t *hwif, int on)
+{
+	u8 ctl = ATA_DEVCTL_OBS;
+
+	if (on == 4) { /* hack for SRST */
+		ctl |= 4;
+		on &= ~4;
+	}
+
+	ctl |= on ? 0 : 2;
+
+	if (hwif->host_flags & IDE_HFLAG_MMIO)
+		writeb(ctl, (void __iomem *)hwif->io_ports.ctl_addr);
+	else
+		outb(ctl, hwif->io_ports.ctl_addr);
+}
+
 static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
 {
 	ide_hwif_t *hwif = drive->hwif;
@@ -360,6 +377,8 @@ void default_hwif_transport(ide_hwif_t *
 	hwif->read_altstatus	  = ide_read_altstatus;
 	hwif->read_sff_dma_status = ide_read_sff_dma_status;
 
+	hwif->set_irq	  = ide_set_irq;
+
 	hwif->tf_load	  = ide_tf_load;
 	hwif->tf_read	  = ide_tf_read;
 
@@ -722,7 +741,7 @@ int ide_driveid_update(ide_drive_t *driv
 	 */
 
 	SELECT_MASK(drive, 1);
-	ide_set_irq(drive, 1);
+	hwif->set_irq(hwif, 1);
 	msleep(50);
 	hwif->exec_command(hwif, WIN_IDENTIFY);
 	timeout = jiffies + WAIT_WORSTCASE;
@@ -808,12 +827,12 @@ int ide_config_drive_speed(ide_drive_t *
 	SELECT_DRIVE(drive);
 	SELECT_MASK(drive, 0);
 	udelay(1);
-	ide_set_irq(drive, 0);
+	hwif->set_irq(hwif, 0);
 	hwif->OUTB(speed, io_ports->nsect_addr);
 	hwif->OUTB(SETFEATURES_XFER, io_ports->feature_addr);
 	hwif->exec_command(hwif, WIN_SETFEATURES);
 	if (drive->quirk_list == 2)
-		ide_set_irq(drive, 1);
+		hwif->set_irq(hwif, 1);
 
 	error = __ide_wait_stat(drive, drive->ready_stat,
 				BUSY_STAT|DRQ_STAT|ERR_STAT,
@@ -1116,7 +1135,6 @@ static ide_startstop_t do_reset1 (ide_dr
 	ide_hwgroup_t *hwgroup;
 	struct ide_io_ports *io_ports;
 	const struct ide_port_ops *port_ops;
-	u8 ctl;
 
 	spin_lock_irqsave(&ide_lock, flags);
 	hwif = HWIF(drive);
@@ -1162,16 +1180,15 @@ static ide_startstop_t do_reset1 (ide_dr
 	 * immediate interrupt due to the edge transition it produces.
 	 * This single interrupt gives us a "fast poll" for drives that
 	 * recover from reset very quickly, saving us the first 50ms wait time.
+	 *
+	 * TODO: add ->softreset method and stop abusing ->set_irq
 	 */
 	/* set SRST and nIEN */
-	hwif->OUTBSYNC(hwif, ATA_DEVCTL_OBS | 6, io_ports->ctl_addr);
+	hwif->set_irq(hwif, 4);
 	/* more than enough time */
 	udelay(10);
-	if (drive->quirk_list == 2)
-		ctl = ATA_DEVCTL_OBS;		/* clear SRST and nIEN */
-	else
-		ctl = ATA_DEVCTL_OBS | 2;	/* clear SRST, leave nIEN */
-	hwif->OUTBSYNC(hwif, ctl, io_ports->ctl_addr);
+	/* clear SRST, leave nIEN (unless device is on the quirk list) */
+	hwif->set_irq(hwif, drive->quirk_list == 2);
 	/* more than enough time */
 	udelay(10);
 	hwgroup->poll_timeout = jiffies + WAIT_WORSTCASE;
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -361,7 +361,7 @@ static int try_to_identify (ide_drive_t 
 			autoprobe = 1;
 			cookie = probe_irq_on();
 		}
-		ide_set_irq(drive, autoprobe);
+		hwif->set_irq(hwif, autoprobe);
 	}
 
 	retval = actual_try_to_identify(drive, cmd);
@@ -369,7 +369,7 @@ static int try_to_identify (ide_drive_t 
 	if (autoprobe) {
 		int irq;
 
-		ide_set_irq(drive, 0);
+		hwif->set_irq(hwif, 0);
 		/* clear drive IRQ */
 		(void)hwif->read_status(hwif);
 		udelay(5);
@@ -711,7 +711,7 @@ static int ide_port_wait_ready(ide_hwif_
 		/* Ignore disks that we will not probe for later. */
 		if (!drive->noprobe || drive->present) {
 			SELECT_DRIVE(drive);
-			ide_set_irq(drive, 1);
+			hwif->set_irq(hwif, 1);
 			mdelay(2);
 			rc = ide_wait_not_busy(hwif, 35000);
 			if (rc)
@@ -1068,8 +1068,7 @@ static int init_irq (ide_hwif_t *hwif)
 			sa = IRQF_SHARED;
 
 		if (io_ports->ctl_addr)
-			/* clear nIEN */
-			hwif->OUTBSYNC(hwif, ATA_DEVCTL_OBS, io_ports->ctl_addr);
+			hwif->set_irq(hwif, 1);
 
 		if (request_irq(hwif->irq,&ide_intr,sa,hwif->name,hwgroup))
 	       		goto out_unlink;
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -79,7 +79,7 @@ ide_startstop_t do_rw_taskfile (ide_driv
 
 	if ((task->tf_flags & IDE_TFLAG_DMA_PIO_FALLBACK) == 0) {
 		ide_tf_dump(drive->name, tf);
-		ide_set_irq(drive, 1);
+		hwif->set_irq(hwif, 1);
 		SELECT_MASK(drive, 0);
 		hwif->tf_load(drive, task);
 	}
Index: b/drivers/ide/pci/scc_pata.c
===================================================================
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -149,6 +149,23 @@ static u8 scc_read_sff_dma_status(ide_hw
 	return (u8)in_be32((void *)(hwif->dma_base + 4));
 }
 
+static void scc_set_irq(ide_hwif_t *hwif, int on)
+{
+	u8 ctl = ATA_DEVCTL_OBS;
+
+	if (on == 4) { /* hack for SRST */
+		ctl |= 4;
+		on &= ~4;
+	}
+
+	ctl |= on ? 0 : 2;
+
+	out_be32((void *)hwif->io_ports.ctl_addr, ctl);
+	eieio();
+	in_be32((void *)(hwif->dma_base + 0x01c));
+	eieio();
+}
+
 static void scc_ide_insw(unsigned long port, void *addr, u32 count)
 {
 	u16 *ptr = (u16 *)addr;
@@ -803,6 +820,8 @@ static void __devinit init_mmio_iops_scc
 	hwif->read_altstatus	  = scc_read_altstatus;
 	hwif->read_sff_dma_status = scc_read_sff_dma_status;
 
+	hwif->set_irq = scc_set_irq;
+
 	hwif->tf_load = scc_tf_load;
 	hwif->tf_read = scc_tf_read;
 
Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -502,6 +502,22 @@ static void pmac_exec_command(ide_hwif_t
 				     + IDE_TIMING_CONFIG));
 }
 
+static void pmac_set_irq(ide_hwif_t *hwif, int on)
+{
+	u8 ctl = ATA_DEVCTL_OBS;
+
+	if (on == 4) { /* hack for SRST */
+		ctl |= 4;
+		on &= ~4;
+	}
+
+	ctl |= on ? 0 : 2;
+
+	writeb(ctl, (void __iomem *)hwif->io_ports.ctl_addr);
+	(void)readl((void __iomem *)(hwif->io_ports.data_addr
+				     + IDE_TIMING_CONFIG));
+}
+
 /*
  * Old tuning functions (called on hdparm -p), sets up drive PIO timings
  */
@@ -1094,6 +1110,7 @@ static int __devinit pmac_ide_setup_devi
 		return -ENOENT;
 
 	hwif->exec_command = pmac_exec_command;
+	hwif->set_irq	   = pmac_set_irq;
 
 	/* Setup MMIO ops */
 	default_hwif_mmiops(hwif);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -473,6 +473,8 @@ typedef struct hwif_s {
 	u8	(*read_altstatus)(struct hwif_s *);
 	u8	(*read_sff_dma_status)(struct hwif_s *);
 
+	void	(*set_irq)(struct hwif_s *, int);
+
 	void (*tf_load)(ide_drive_t *, struct ide_task_s *);
 	void (*tf_read)(ide_drive_t *, struct ide_task_s *);
 
@@ -1342,14 +1344,6 @@ static inline ide_drive_t *ide_get_paire
 	return &hwif->drives[(drive->dn ^ 1) & 1];
 }
 
-static inline void ide_set_irq(ide_drive_t *drive, int on)
-{
-	ide_hwif_t *hwif = drive->hwif;
-
-	hwif->OUTBSYNC(hwif, ATA_DEVCTL_OBS | (on ? 0 : 2),
-		       hwif->io_ports.ctl_addr);
-}
-
 static inline u8 ide_read_error(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;

[PATCH 10/18] ide: change order of register access in ide_config_drive_speed()

[Bartlomiej Zolnierkiewicz]

Write ATA Feature register before ATA Sector Count register as
a preparation to use ->tf_load in ide_config_drive_speed().

This change shouldn't affect anything (just an usual paranoia).

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-iops.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -828,8 +828,8 @@ int ide_config_drive_speed(ide_drive_t *
 	SELECT_MASK(drive, 0);
 	udelay(1);
 	hwif->set_irq(hwif, 0);
-	hwif->OUTB(speed, io_ports->nsect_addr);
 	hwif->OUTB(SETFEATURES_XFER, io_ports->feature_addr);
+	hwif->OUTB(speed, io_ports->nsect_addr);
 	hwif->exec_command(hwif, WIN_SETFEATURES);
 	if (drive->quirk_list == 2)
 		hwif->set_irq(hwif, 1);

[PATCH 11/18] ide: use ->tf_load in ide_config_drive_speed()

[Bartlomiej Zolnierkiewicz]

Convert ide_config_drive_speed() to use ->tf_load instead of ->OUTB.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-iops.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -792,9 +792,9 @@ int ide_driveid_update(ide_drive_t *driv
 int ide_config_drive_speed(ide_drive_t *drive, u8 speed)
 {
 	ide_hwif_t *hwif = drive->hwif;
-	struct ide_io_ports *io_ports = &hwif->io_ports;
 	int error = 0;
 	u8 stat;
+	ide_task_t task;
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 	if (hwif->dma_ops)	/* check if host supports DMA */
@@ -828,9 +828,16 @@ int ide_config_drive_speed(ide_drive_t *
 	SELECT_MASK(drive, 0);
 	udelay(1);
 	hwif->set_irq(hwif, 0);
-	hwif->OUTB(SETFEATURES_XFER, io_ports->feature_addr);
-	hwif->OUTB(speed, io_ports->nsect_addr);
+
+	memset(&task, 0, sizeof(task));
+	task.tf_flags = IDE_TFLAG_OUT_FEATURE | IDE_TFLAG_OUT_NSECT;
+	task.tf.feature = SETFEATURES_XFER;
+	task.tf.nsect   = speed;
+
+	hwif->tf_load(drive, &task);
+
 	hwif->exec_command(hwif, WIN_SETFEATURES);
+
 	if (drive->quirk_list == 2)
 		hwif->set_irq(hwif, 1);
 

[PATCH 12/18] ide: use ->tf_load in actual_try_to_identify()

[Bartlomiej Zolnierkiewicz]

Convert actual_try_to_identify() to use ->tf_load instead of ->OUTB.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-probe.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -290,9 +290,15 @@ static int actual_try_to_identify (ide_d
 	/* set features register for atapi
 	 * identify command to be sure of reply
 	 */
-	if ((cmd == WIN_PIDENTIFY))
-		/* disable dma & overlap */
-		hwif->OUTB(0, io_ports->feature_addr);
+	if (cmd == WIN_PIDENTIFY) {
+		ide_task_t task;
+
+		memset(&task, 0, sizeof(task));
+		/* disable DMA & overlap */
+		task.tf_flags = IDE_TFLAG_OUT_FEATURE;
+
+		drive->hwif->tf_load(drive, &task);
+	}
 
 	/* ask drive for ID */
 	hwif->exec_command(hwif, cmd);

[PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Bartlomiej Zolnierkiewicz]

Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-iops.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
 	const struct ide_port_ops *port_ops = hwif->port_ops;
+	ide_task_t task;
 
 	if (port_ops && port_ops->selectproc)
 		port_ops->selectproc(drive);
 
-	hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
+	memset(&task, 0, sizeof(task));
+	task.tf_flags = IDE_TFLAG_OUT_DEVICE;
+
+	drive->hwif->tf_load(drive, &task);
 }
 
 void SELECT_MASK(ide_drive_t *drive, int mask)

[PATCH 14/18] ide: use ->tf_read in ide_read_error()

[Bartlomiej Zolnierkiewicz]

* Add IDE_TFLAG_IN_FEATURE taskfile flag for reading Feature
  register and handle it in ->tf_read.

* Convert ide_read_error() to use ->tf_read instead of ->INB,
  then uninline and export it.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/h8300/ide-h8300.c |    2 ++
 drivers/ide/ide-iops.c        |   15 +++++++++++++++
 drivers/ide/pci/ns87415.c     |    2 ++
 drivers/ide/pci/scc_pata.c    |    2 ++
 include/linux/ide.h           |   10 +++-------
 5 files changed, 24 insertions(+), 7 deletions(-)

Index: b/drivers/ide/h8300/ide-h8300.c
===================================================================
--- a/drivers/ide/h8300/ide-h8300.c
+++ b/drivers/ide/h8300/ide-h8300.c
@@ -100,6 +100,8 @@ static void h8300_tf_read(ide_drive_t *d
 	/* be sure we're looking at the low order bits */
 	outb(ATA_DEVCTL_OBS & ~0x80, io_ports->ctl_addr);
 
+	if (task->tf_flags & IDE_TFLAG_IN_FEATURE)
+		tf->feature = inb(io_ports->feature_addr);
 	if (task->tf_flags & IDE_TFLAG_IN_NSECT)
 		tf->nsect  = inb(io_ports->nsect_addr);
 	if (task->tf_flags & IDE_TFLAG_IN_LBAL)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -241,6 +241,8 @@ static void ide_tf_read(ide_drive_t *dri
 	/* be sure we're looking at the low order bits */
 	tf_outb(ATA_DEVCTL_OBS & ~0x80, io_ports->ctl_addr);
 
+	if (task->tf_flags & IDE_TFLAG_IN_FEATURE)
+		tf->feature = tf_inb(io_ports->feature_addr);
 	if (task->tf_flags & IDE_TFLAG_IN_NSECT)
 		tf->nsect  = tf_inb(io_ports->nsect_addr);
 	if (task->tf_flags & IDE_TFLAG_IN_LBAL)
@@ -390,6 +392,19 @@ void default_hwif_transport(ide_hwif_t *
 	hwif->output_data = ata_output_data;
 }
 
+u8 ide_read_error(ide_drive_t *drive)
+{
+	ide_task_t task;
+
+	memset(&task, 0, sizeof(task));
+	task.tf_flags = IDE_TFLAG_IN_FEATURE;
+
+	drive->hwif->tf_read(drive, &task);
+
+	return task.tf.error;
+}
+EXPORT_SYMBOL_GPL(ide_read_error);
+
 void ide_fix_driveid (struct hd_driveid *id)
 {
 #ifndef __LITTLE_ENDIAN
Index: b/drivers/ide/pci/ns87415.c
===================================================================
--- a/drivers/ide/pci/ns87415.c
+++ b/drivers/ide/pci/ns87415.c
@@ -88,6 +88,8 @@ static void superio_tf_read(ide_drive_t 
 	/* be sure we're looking at the low order bits */
 	outb(ATA_DEVCTL_OBS & ~0x80, io_ports->ctl_addr);
 
+	if (task->tf_flags & IDE_TFLAG_IN_FEATURE)
+		tf->feature = inb(io_ports->feature_addr);
 	if (task->tf_flags & IDE_TFLAG_IN_NSECT)
 		tf->nsect  = inb(io_ports->nsect_addr);
 	if (task->tf_flags & IDE_TFLAG_IN_LBAL)
Index: b/drivers/ide/pci/scc_pata.c
===================================================================
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -742,6 +742,8 @@ static void scc_tf_read(ide_drive_t *dri
 	/* be sure we're looking at the low order bits */
 	scc_ide_outb(ATA_DEVCTL_OBS & ~0x80, io_ports->ctl_addr);
 
+	if (task->tf_flags & IDE_TFLAG_IN_FEATURE)
+		tf->feature = scc_ide_inb(io_ports->feature_addr);
 	if (task->tf_flags & IDE_TFLAG_IN_NSECT)
 		tf->nsect  = scc_ide_inb(io_ports->nsect_addr);
 	if (task->tf_flags & IDE_TFLAG_IN_LBAL)
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -875,6 +875,7 @@ enum {
 	IDE_TFLAG_IN_HOB		= IDE_TFLAG_IN_HOB_FEATURE |
 					  IDE_TFLAG_IN_HOB_NSECT |
 					  IDE_TFLAG_IN_HOB_LBA,
+	IDE_TFLAG_IN_FEATURE		= (1 << 1),
 	IDE_TFLAG_IN_NSECT		= (1 << 25),
 	IDE_TFLAG_IN_LBAL		= (1 << 26),
 	IDE_TFLAG_IN_LBAM		= (1 << 27),
@@ -942,6 +943,8 @@ void ide_tf_dump(const char *, struct id
 extern void SELECT_DRIVE(ide_drive_t *);
 void SELECT_MASK(ide_drive_t *, int);
 
+u8 ide_read_error(ide_drive_t *);
+
 extern int drive_is_ready(ide_drive_t *);
 
 void ide_pktcmd_tf_load(ide_drive_t *, u32, u16, u8);
@@ -1343,11 +1346,4 @@ static inline ide_drive_t *ide_get_paire
 
 	return &hwif->drives[(drive->dn ^ 1) & 1];
 }
-
-static inline u8 ide_read_error(ide_drive_t *drive)
-{
-	ide_hwif_t *hwif = drive->hwif;
-
-	return hwif->INB(hwif->io_ports.error_addr);
-}
 #endif /* _IDE_H */

[PATCH 15/18] ide: add ide_read_device() helper

[Bartlomiej Zolnierkiewicz]

Add ide_read_device() helper and convert do_probe() to use it
instead of ->INB.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-probe.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -410,6 +410,18 @@ static int ide_busy_sleep(ide_hwif_t *hw
 	return 1;
 }
 
+static u8 ide_read_device(ide_drive_t *drive)
+{
+	ide_task_t task;
+
+	memset(&task, 0, sizeof(task));
+	task.tf_flags = IDE_TFLAG_IN_DEVICE;
+
+	drive->hwif->tf_read(drive, &task);
+
+	return task.tf.device;
+}
+
 /**
  *	do_probe		-	probe an IDE device
  *	@drive: drive to probe
@@ -434,7 +446,6 @@ static int ide_busy_sleep(ide_hwif_t *hw
 static int do_probe (ide_drive_t *drive, u8 cmd)
 {
 	ide_hwif_t *hwif = HWIF(drive);
-	struct ide_io_ports *io_ports = &hwif->io_ports;
 	int rc;
 	u8 stat;
 
@@ -455,8 +466,8 @@ static int do_probe (ide_drive_t *drive,
 	msleep(50);
 	SELECT_DRIVE(drive);
 	msleep(50);
-	if (hwif->INB(io_ports->device_addr) != drive->select.all &&
-	    !drive->present) {
+
+	if (ide_read_device(drive) != drive->select.all && !drive->present) {
 		if (drive->select.b.unit != 0) {
 			/* exit with drive0 selected */
 			SELECT_DRIVE(&hwif->drives[0]);

[PATCH 16/18] ide: add ide_read_ireason() helper

[Bartlomiej Zolnierkiewicz]

Add ide_read_ireason() helper and use instead of ->INB for reading
ATAPI Interrupt Reason register.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-atapi.c |   17 ++++++++++++++---
 drivers/ide/ide-iops.c  |   13 +++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -183,16 +183,27 @@ cmd_finished:
 }
 EXPORT_SYMBOL_GPL(ide_pc_intr);
 
+static u8 ide_read_ireason(ide_drive_t *drive)
+{
+	ide_task_t task;
+
+	memset(&task, 0, sizeof(task));
+	task.tf_flags = IDE_TFLAG_IN_NSECT;
+
+	drive->hwif->tf_read(drive, &task);
+
+	return task.tf.nsect & 3;
+}
+
 static u8 ide_wait_ireason(ide_drive_t *drive, u8 ireason)
 {
-	ide_hwif_t *hwif = drive->hwif;
 	int retries = 100;
 
 	while (retries-- && ((ireason & CD) == 0 || (ireason & IO))) {
 		printk(KERN_ERR "%s: (IO,CoD != (0,1) while issuing "
 				"a packet command, retrying\n", drive->name);
 		udelay(100);
-		ireason = hwif->INB(hwif->io_ports.nsect_addr);
+		ireason = ide_read_ireason(drive);
 		if (retries == 0) {
 			printk(KERN_ERR "%s: (IO,CoD != (0,1) while issuing "
 					"a packet command, ignoring\n",
@@ -219,7 +230,7 @@ ide_startstop_t ide_transfer_pc(ide_driv
 		return startstop;
 	}
 
-	ireason = hwif->INB(hwif->io_ports.nsect_addr);
+	ireason = ide_read_ireason(drive);
 	if (drive->media == ide_tape && !drive->scsi)
 		ireason = ide_wait_ireason(drive, ireason);
 

[PATCH 17/18] ide: add ide_read_bcount_and_ireason() helper

[Bartlomiej Zolnierkiewicz]

Add ide_read_bcount_and_ireason() helper and use it instead of ->INB
in {cdrom_newpc,ide_pc}_intr().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-atapi.c |    6 ++----
 drivers/ide/ide-cd.c    |   12 ++++--------
 drivers/ide/ide-iops.c  |   15 +++++++++++++++
 include/linux/ide.h     |    1 +
 4 files changed, 22 insertions(+), 12 deletions(-)

Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -107,11 +107,9 @@ cmd_finished:
 		ide_dma_off(drive);
 		return ide_do_reset(drive);
 	}
-	/* Get the number of bytes to transfer on this interrupt. */
-	bcount = (hwif->INB(hwif->io_ports.lbah_addr) << 8) |
-		  hwif->INB(hwif->io_ports.lbam_addr);
 
-	ireason = hwif->INB(hwif->io_ports.nsect_addr);
+	/* Get the number of bytes to transfer on this interrupt. */
+	ide_read_bcount_and_ireason(drive, &bcount, &ireason);
 
 	if (ireason & CD) {
 		printk(KERN_ERR "%s: CoD != 0 in %s\n", drive->name, __func__);
Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -923,10 +923,11 @@ static ide_startstop_t cdrom_newpc_intr(
 	struct request *rq = HWGROUP(drive)->rq;
 	xfer_func_t *xferfunc;
 	ide_expiry_t *expiry = NULL;
-	int dma_error = 0, dma, stat, ireason, len, thislen, uptodate = 0;
+	int dma_error = 0, dma, stat, thislen, uptodate = 0;
 	int write = (rq_data_dir(rq) == WRITE) ? 1 : 0;
 	unsigned int timeout;
-	u8 lowcyl, highcyl;
+	u16 len;
+	u8 ireason;
 
 	/* check for errors */
 	dma = info->dma;
@@ -954,12 +955,7 @@ static ide_startstop_t cdrom_newpc_intr(
 		goto end_request;
 	}
 
-	/* ok we fall to pio :/ */
-	ireason = hwif->INB(hwif->io_ports.nsect_addr) & 0x3;
-	lowcyl  = hwif->INB(hwif->io_ports.lbam_addr);
-	highcyl = hwif->INB(hwif->io_ports.lbah_addr);
-
-	len = lowcyl + (256 * highcyl);
+	ide_read_bcount_and_ireason(drive, &len, &ireason);
 
 	thislen = blk_fs_request(rq) ? len : rq->data_len;
 	if (thislen > len)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -405,6 +405,21 @@ u8 ide_read_error(ide_drive_t *drive)
 }
 EXPORT_SYMBOL_GPL(ide_read_error);
 
+void ide_read_bcount_and_ireason(ide_drive_t *drive, u16 *bcount, u8 *ireason)
+{
+	ide_task_t task;
+
+	memset(&task, 0, sizeof(task));
+	task.tf_flags = IDE_TFLAG_IN_LBAH | IDE_TFLAG_IN_LBAM |
+			IDE_TFLAG_IN_NSECT;
+
+	drive->hwif->tf_read(drive, &task);
+
+	*bcount = (task.tf.lbah << 8) | task.tf.lbam;
+	*ireason = task.tf.nsect & 3;
+}
+EXPORT_SYMBOL_GPL(ide_read_bcount_and_ireason);
+
 void ide_fix_driveid (struct hd_driveid *id)
 {
 #ifndef __LITTLE_ENDIAN
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -944,6 +944,7 @@ extern void SELECT_DRIVE(ide_drive_t *);
 void SELECT_MASK(ide_drive_t *, int);
 
 u8 ide_read_error(ide_drive_t *);
+void ide_read_bcount_and_ireason(ide_drive_t *, u16 *, u8 *);
 
 extern int drive_is_ready(ide_drive_t *);
 

[PATCH 18/18] ide: remove ->INB, ->OUTB and ->OUTBSYNC methods

[Bartlomiej Zolnierkiewicz]

* Remove no longer needed ->INB, ->OUTB and ->OUTBSYNC methods.

Then:

* Remove no longer used default_hwif_[mm]iops() and ide_[mm_]outbsync().

* Cleanup SuperIO handling in ns87415.c.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/arm/icside.c          |    5 ----
 drivers/ide/arm/palm_bk3710.c     |    2 -
 drivers/ide/arm/rapide.c          |    2 -
 drivers/ide/h8300/ide-h8300.c     |    2 -
 drivers/ide/ide-iops.c            |   28 -----------------------
 drivers/ide/ide.c                 |    1 
 drivers/ide/legacy/ide_platform.c |    4 ---
 drivers/ide/mips/swarm.c          |    3 --
 drivers/ide/pci/ns87415.c         |   46 ++++++++++----------------------------
 drivers/ide/pci/scc_pata.c        |   12 ---------
 drivers/ide/pci/sgiioc4.c         |    3 --
 drivers/ide/pci/siimage.c         |    2 -
 drivers/ide/ppc/pmac.c            |   13 ----------
 include/linux/ide.h               |    7 -----
 14 files changed, 15 insertions(+), 115 deletions(-)

Index: b/drivers/ide/arm/icside.c
===================================================================
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -469,8 +469,6 @@ icside_register_v5(struct icside_state *
 	if (!hwif)
 		return -ENODEV;
 
-	default_hwif_mmiops(hwif);
-
 	state->hwif[0] = hwif;
 
 	ecard_set_drvdata(ec, state);
@@ -547,14 +545,11 @@ icside_register_v6(struct icside_state *
 		return -ENODEV;
 
 	hwif->chipset = ide_acorn;
-	default_hwif_mmiops(hwif);
 
 	idx[0] = hwif->index;
 
 	mate = ide_find_port();
 	if (mate) {
-		default_hwif_mmiops(mate);
-
 		hws[1] = &hw[1];
 		idx[1] = mate->index;
 	}
Index: b/drivers/ide/arm/palm_bk3710.c
===================================================================
--- a/drivers/ide/arm/palm_bk3710.c
+++ b/drivers/ide/arm/palm_bk3710.c
@@ -401,8 +401,6 @@ static int __devinit palm_bk3710_probe(s
 
 	i = hwif->index;
 
-	default_hwif_mmiops(hwif);
-
 	idx[0] = i;
 
 	ide_device_add(idx, &palm_bk3710_port_info, hws);
Index: b/drivers/ide/arm/rapide.c
===================================================================
--- a/drivers/ide/arm/rapide.c
+++ b/drivers/ide/arm/rapide.c
@@ -59,8 +59,6 @@ rapide_probe(struct expansion_card *ec, 
 		goto release;
 	}
 
-	default_hwif_mmiops(hwif);
-
 	idx[0] = hwif->index;
 
 	ide_device_add(idx, &rapide_port_info, hws);
Index: b/drivers/ide/h8300/ide-h8300.c
===================================================================
--- a/drivers/ide/h8300/ide-h8300.c
+++ b/drivers/ide/h8300/ide-h8300.c
@@ -171,8 +171,6 @@ static inline void hw_setup(hw_regs_t *h
 
 static inline void hwif_setup(ide_hwif_t *hwif)
 {
-	default_hwif_iops(hwif);
-
 	hwif->tf_load = h8300_tf_load;
 	hwif->tf_read = h8300_tf_read;
 
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -42,18 +42,6 @@ static void ide_outb (u8 val, unsigned l
 	outb(val, port);
 }
 
-static void ide_outbsync(ide_hwif_t *hwif, u8 addr, unsigned long port)
-{
-	outb(addr, port);
-}
-
-void default_hwif_iops (ide_hwif_t *hwif)
-{
-	hwif->OUTB	= ide_outb;
-	hwif->OUTBSYNC	= ide_outbsync;
-	hwif->INB	= ide_inb;
-}
-
 /*
  *	MMIO operations, typically used for SATA controllers
  */
@@ -68,22 +56,6 @@ static void ide_mm_outb (u8 value, unsig
 	writeb(value, (void __iomem *) port);
 }
 
-static void ide_mm_outbsync(ide_hwif_t *hwif, u8 value, unsigned long port)
-{
-	writeb(value, (void __iomem *) port);
-}
-
-void default_hwif_mmiops (ide_hwif_t *hwif)
-{
-	hwif->OUTB	= ide_mm_outb;
-	/* Most systems will need to override OUTBSYNC, alas however
-	   this one is controller specific! */
-	hwif->OUTBSYNC	= ide_mm_outbsync;
-	hwif->INB	= ide_mm_inb;
-}
-
-EXPORT_SYMBOL(default_hwif_mmiops);
-
 void SELECT_DRIVE (ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -101,7 +101,6 @@ void ide_init_port_data(ide_hwif_t *hwif
 
 	init_completion(&hwif->gendev_rel_comp);
 
-	default_hwif_iops(hwif);
 	default_hwif_transport(hwif);
 
 	ide_port_init_devices_data(hwif);
Index: b/drivers/ide/legacy/ide_platform.c
===================================================================
--- a/drivers/ide/legacy/ide_platform.c
+++ b/drivers/ide/legacy/ide_platform.c
@@ -103,10 +103,8 @@ static int __devinit plat_ide_probe(stru
 	plat_ide_setup_ports(&hw, base, alt_base, pdata, res_irq->start);
 	hw.dev = &pdev->dev;
 
-	if (mmio) {
+	if (mmio)
 		d.host_flags |= IDE_HFLAG_MMIO;
-		default_hwif_mmiops(hwif);
-	}
 
 	idx[0] = hwif->index;
 
Index: b/drivers/ide/mips/swarm.c
===================================================================
--- a/drivers/ide/mips/swarm.c
+++ b/drivers/ide/mips/swarm.c
@@ -120,9 +120,6 @@ static int __devinit swarm_ide_probe(str
 	if (hwif == NULL)
 		goto err;
 
-	/* Setup MMIO ops. */
-	default_hwif_mmiops(hwif);
-
 	idx[0] = hwif->index;
 
 	ide_device_add(idx, &swarm_port_info, hws);
Index: b/drivers/ide/pci/ns87415.c
===================================================================
--- a/drivers/ide/pci/ns87415.c
+++ b/drivers/ide/pci/ns87415.c
@@ -28,10 +28,6 @@
  */
 #include <asm/superio.h>
 
-static unsigned long superio_ide_status[2];
-static unsigned long superio_ide_select[2];
-static unsigned long superio_ide_dma_status[2];
-
 #define SUPERIO_IDE_MAX_RETRIES 25
 
 /* Because of a defect in Super I/O, all reads of the PCI DMA status 
@@ -40,27 +36,18 @@ static unsigned long superio_ide_dma_sta
  */
 static u8 superio_ide_inb (unsigned long port)
 {
-	if (port == superio_ide_status[0] ||
-	    port == superio_ide_status[1] ||
-	    port == superio_ide_select[0] ||
-	    port == superio_ide_select[1] ||
-	    port == superio_ide_dma_status[0] ||
-	    port == superio_ide_dma_status[1]) {
-		u8 tmp;
-		int retries = SUPERIO_IDE_MAX_RETRIES;
-
-		/* printk(" [ reading port 0x%x with retry ] ", port); */
+	u8 tmp;
+	int retries = SUPERIO_IDE_MAX_RETRIES;
 
-		do {
-			tmp = inb(port);
-			if (tmp == 0)
-				udelay(50);
-		} while (tmp == 0 && retries-- > 0);
+	/* printk(" [ reading port 0x%x with retry ] ", port); */
 
-		return tmp;
-	}
+	do {
+		tmp = inb(port);
+		if (tmp == 0)
+			udelay(50);
+	} while (tmp == 0 && retries-- > 0);
 
-	return inb(port);
+	return tmp;
 }
 
 static u8 superio_read_status(ide_hwif_t *hwif)
@@ -120,27 +107,20 @@ static void superio_tf_read(ide_drive_t 
 static void __devinit superio_ide_init_iops (struct hwif_s *hwif)
 {
 	struct pci_dev *pdev = to_pci_dev(hwif->dev);
-	u32 base, dmabase;
+	u32 dma_stat;
 	u8 port = hwif->channel, tmp;
 
-	base = pci_resource_start(pdev, port * 2) & ~3;
-	dmabase = pci_resource_start(pdev, 4) & ~3;
-
-	superio_ide_status[port] = base + 7;
-	superio_ide_select[port] = base + 6;
-	superio_ide_dma_status[port] = dmabase + (!port ? 2 : 0xa);
+	dma_stat = (pci_resource_start(pdev, 4) & ~3) + (!port ? 2 : 0xa);
 
 	/* Clear error/interrupt, enable dma */
-	tmp = superio_ide_inb(superio_ide_dma_status[port]);
-	outb(tmp | 0x66, superio_ide_dma_status[port]);
+	tmp = superio_ide_inb(dma_stat);
+	outb(tmp | 0x66, dma_stat);
 
 	hwif->read_status	  = superio_read_status;
 	hwif->read_sff_dma_status = superio_read_sff_dma_status;
 
 	hwif->tf_read = superio_tf_read;
 
-	/* We need to override inb to workaround a SuperIO errata */
-	hwif->INB = superio_ide_inb;
 }
 
 static void __devinit init_iops_ns87415(ide_hwif_t *hwif)
Index: b/drivers/ide/pci/scc_pata.c
===================================================================
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -188,14 +188,6 @@ static void scc_ide_outb(u8 addr, unsign
 	out_be32((void*)port, addr);
 }
 
-static void scc_ide_outbsync(ide_hwif_t *hwif, u8 addr, unsigned long port)
-{
-	out_be32((void*)port, addr);
-	eieio();
-	in_be32((void*)(hwif->dma_base + 0x01c));
-	eieio();
-}
-
 static void
 scc_ide_outsw(unsigned long port, void *addr, u32 count)
 {
@@ -830,10 +822,6 @@ static void __devinit init_mmio_iops_scc
 	hwif->input_data  = scc_input_data;
 	hwif->output_data = scc_output_data;
 
-	hwif->INB = scc_ide_inb;
-	hwif->OUTB = scc_ide_outb;
-	hwif->OUTBSYNC = scc_ide_outbsync;
-
 	hwif->dma_base = dma_base;
 	hwif->config_data = ports->ctl;
 }
Index: b/drivers/ide/pci/sgiioc4.c
===================================================================
--- a/drivers/ide/pci/sgiioc4.c
+++ b/drivers/ide/pci/sgiioc4.c
@@ -623,9 +623,6 @@ sgiioc4_ide_setup_pci_device(struct pci_
 	if (hwif == NULL)
 		goto err;
 
-	/* The IOC4 uses MMIO rather than Port IO. */
-	default_hwif_mmiops(hwif);
-
 	/* Initializing chipset IRQ Registers */
 	writel(0x03, (void __iomem *)(irqport + IOC4_INTR_SET * 4));
 
Index: b/drivers/ide/pci/siimage.c
===================================================================
--- a/drivers/ide/pci/siimage.c
+++ b/drivers/ide/pci/siimage.c
@@ -602,7 +602,7 @@ static void __devinit init_mmio_iops_sii
 	 *	Fill in the basic hwif bits
 	 */
 	hwif->host_flags |= IDE_HFLAG_MMIO;
-	default_hwif_mmiops(hwif);
+
 	hwif->hwif_data	= addr;
 
 	/*
Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -486,15 +486,6 @@ pmac_ide_do_update_timings(ide_drive_t *
 		pmac_ide_selectproc(drive);
 }
 
-static void pmac_outbsync(ide_hwif_t *hwif, u8 value, unsigned long port)
-{
-	u32 tmp;
-	
-	writeb(value, (void __iomem *) port);
-	tmp = readl((void __iomem *)(hwif->io_ports.data_addr
-				     + IDE_TIMING_CONFIG));
-}
-
 static void pmac_exec_command(ide_hwif_t *hwif, u8 cmd)
 {
 	writeb(cmd, (void __iomem *)hwif->io_ports.command_addr);
@@ -1112,10 +1103,6 @@ static int __devinit pmac_ide_setup_devi
 	hwif->exec_command = pmac_exec_command;
 	hwif->set_irq	   = pmac_set_irq;
 
-	/* Setup MMIO ops */
-	default_hwif_mmiops(hwif);
-       	hwif->OUTBSYNC = pmac_outbsync;
-
 	idx[0] = hwif->index;
 
 	ide_device_add(idx, &d, hws);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -483,11 +483,6 @@ typedef struct hwif_s {
 
 	void (*ide_dma_clear_irq)(ide_drive_t *drive);
 
-	void (*OUTB)(u8 addr, unsigned long port);
-	void (*OUTBSYNC)(struct hwif_s *hwif, u8 addr, unsigned long port);
-
-	u8  (*INB)(unsigned long port);
-
 	/* dma physical region descriptor table (cpu view) */
 	unsigned int	*dmatable_cpu;
 	/* dma physical region descriptor table (dma view) */
@@ -1013,8 +1008,6 @@ static inline int ide_hwif_setup_dma(ide
 }
 #endif
 
-extern void default_hwif_iops(ide_hwif_t *);
-extern void default_hwif_mmiops(ide_hwif_t *);
 extern void default_hwif_transport(ide_hwif_t *);
 
 typedef struct ide_pci_enablebit_s {

Re: [PATCH 04/18] ide: remove ide_setup_dma()

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:
> Export sff_dma_ops and then remove ide_setup_dma().
>
> There should be no functional changes caused by this patch.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>
>   
[...]
> Index: b/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- a/drivers/ide/arm/palm_bk3710.c
> +++ b/drivers/ide/arm/palm_bk3710.c
> @@ -318,15 +318,14 @@ static u8 __devinit palm_bk3710_cable_de
>  static int __devinit palm_bk3710_init_dma(ide_hwif_t *hwif,
>  					  const struct ide_port_info *d)
>  {
> -	unsigned long base =
> -		hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
> -
>  	printk(KERN_INFO "    %s: MMIO-DMA\n", hwif->name);
>  
>  	if (ide_allocate_dma_engine(hwif))
>  		return -1;
>  
> -	ide_setup_dma(hwif, base);
> +	hwif->dma_base = hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
>   

   That's er... too complex. Actually, 'base' has the value for 
'dma_base' at that moment...

WBR, Sergei

Re: [PATCH 12/18] ide: use ->tf_load in actual_try_to_identify()

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:
> Convert actual_try_to_identify() to use ->tf_load instead of ->OUTB.
>
> There should be no functional changes caused by this patch.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>   
[...]
> Index: b/drivers/ide/ide-probe.c
> ===================================================================
> --- a/drivers/ide/ide-probe.c
> +++ b/drivers/ide/ide-probe.c
> @@ -290,9 +290,15 @@ static int actual_try_to_identify (ide_d
>  	/* set features register for atapi
>  	 * identify command to be sure of reply
>  	 */
> -	if ((cmd == WIN_PIDENTIFY))
> -		/* disable dma & overlap */
> -		hwif->OUTB(0, io_ports->feature_addr);
> +	if (cmd == WIN_PIDENTIFY) {
> +		ide_task_t task;
> +
> +		memset(&task, 0, sizeof(task));
> +		/* disable DMA & overlap */
> +		task.tf_flags = IDE_TFLAG_OUT_FEATURE;
> +
> +		drive->hwif->tf_load(drive, &task);
> +	}
>   

   Eww, this is bulky but should be a part of your plan to switch to 
tf_load() method...

WBR, Sergei

Re: [PATCH 04/18] ide: remove ide_setup_dma()

[Bartlomiej Zolnierkiewicz]

On Saturday 21 June 2008, Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> > Export sff_dma_ops and then remove ide_setup_dma().
> >
> > There should be no functional changes caused by this patch.
> >
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >
> >   
> [...]
> > Index: b/drivers/ide/arm/palm_bk3710.c
> > ===================================================================
> > --- a/drivers/ide/arm/palm_bk3710.c
> > +++ b/drivers/ide/arm/palm_bk3710.c
> > @@ -318,15 +318,14 @@ static u8 __devinit palm_bk3710_cable_de
> >  static int __devinit palm_bk3710_init_dma(ide_hwif_t *hwif,
> >  					  const struct ide_port_info *d)
> >  {
> > -	unsigned long base =
> > -		hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
> > -
> >  	printk(KERN_INFO "    %s: MMIO-DMA\n", hwif->name);
> >  
> >  	if (ide_allocate_dma_engine(hwif))
> >  		return -1;
> >  
> > -	ide_setup_dma(hwif, base);
> > +	hwif->dma_base = hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
> >   
> 
>    That's er... too complex. Actually, 'base' has the value for 
> 'dma_base' at that moment...

Well, the above place is the only user of 'base' so isn't the variable
kind of superfluous?

Thanks,
Bart

Re: [PATCH 12/18] ide: use ->tf_load in actual_try_to_identify()

[Bartlomiej Zolnierkiewicz]

On Saturday 21 June 2008, Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> > Convert actual_try_to_identify() to use ->tf_load instead of ->OUTB.
> >
> > There should be no functional changes caused by this patch.
> >
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >   
> [...]
> > Index: b/drivers/ide/ide-probe.c
> > ===================================================================
> > --- a/drivers/ide/ide-probe.c
> > +++ b/drivers/ide/ide-probe.c
> > @@ -290,9 +290,15 @@ static int actual_try_to_identify (ide_d
> >  	/* set features register for atapi
> >  	 * identify command to be sure of reply
> >  	 */
> > -	if ((cmd == WIN_PIDENTIFY))
> > -		/* disable dma & overlap */
> > -		hwif->OUTB(0, io_ports->feature_addr);
> > +	if (cmd == WIN_PIDENTIFY) {
> > +		ide_task_t task;
> > +
> > +		memset(&task, 0, sizeof(task));
> > +		/* disable DMA & overlap */
> > +		task.tf_flags = IDE_TFLAG_OUT_FEATURE;
> > +
> > +		drive->hwif->tf_load(drive, &task);
> > +	}
> >   
> 
>    Eww, this is bulky but should be a part of your plan to switch to 
> tf_load() method...

Yes on both matters:

- It is a part of The Grand Plan. ;)

- I also don't like the small increase of complexity but these changes
  make the core code completely independent of the hardware I/O registers
  (allowing any taskfile transport method).

PS Thanks for reviewing these patches.

Bart

Re: [PATCH 04/18] ide: remove ide_setup_dma()

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:
> On Saturday 21 June 2008, Sergei Shtylyov wrote:
>   
>> Hello.
>>
>> Bartlomiej Zolnierkiewicz wrote:
>>     
>>> Export sff_dma_ops and then remove ide_setup_dma().
>>>
>>> There should be no functional changes caused by this patch.
>>>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>>
>>>       

Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

>>> Index: b/drivers/ide/arm/palm_bk3710.c
>>> ===================================================================
>>> --- a/drivers/ide/arm/palm_bk3710.c
>>> +++ b/drivers/ide/arm/palm_bk3710.c
>>> @@ -318,15 +318,14 @@ static u8 __devinit palm_bk3710_cable_de
>>>  static int __devinit palm_bk3710_init_dma(ide_hwif_t *hwif,
>>>  					  const struct ide_port_info *d)
>>>  {
>>> -	unsigned long base =
>>> -		hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
>>> -
>>>  	printk(KERN_INFO "    %s: MMIO-DMA\n", hwif->name);
>>>  
>>>  	if (ide_allocate_dma_engine(hwif))
>>>  		return -1;
>>>  
>>> -	ide_setup_dma(hwif, base);
>>> +	hwif->dma_base = hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
>>>   
>>>       
>>    That's er... too complex. Actually, 'base' has the value for 
>> 'dma_base' at that moment...
>>     
>
> Well, the above place is the only user of 'base' so isn't the variable
> kind of superfluous?
>   

   Oops, sorry. I somehow thought this change was in other context (was 
looking at the older version of driver recently).

> Thanks,
> Bart
>   

MBR, Sergei

Re: [PATCH 04/18] ide: remove ide_setup_dma()

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

> Export sff_dma_ops and then remove ide_setup_dma().

> There should be no functional changes caused by this patch.

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

> Index: b/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- a/drivers/ide/arm/palm_bk3710.c
> +++ b/drivers/ide/arm/palm_bk3710.c
> @@ -318,15 +318,14 @@ static u8 __devinit palm_bk3710_cable_de
>  static int __devinit palm_bk3710_init_dma(ide_hwif_t *hwif,
>  					  const struct ide_port_info *d)
>  {
> -	unsigned long base =
> -		hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
> -
>  	printk(KERN_INFO "    %s: MMIO-DMA\n", hwif->name);
>  
>  	if (ide_allocate_dma_engine(hwif))
>  		return -1;
>  
> -	ide_setup_dma(hwif, base);
> +	hwif->dma_base = hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
> +
> +	hwif->dma_ops = &sff_dma_ops;
>  
>  	return 0;
>  }

    Unfortunately, this patch broke this driver:

drivers/ide/arm/palm_bk3710.c:327: error: ‘sff_dma_ops’ undeclared (first use 
in this function). The declatation of sff_dma_ops should've been surrounded by:

#ifdef CONFIG_BLK_DEV_IDEDMA_SFF

not by:

#ifdef CONFIG_BLK_DEV_IDEDMA_PCI

the same as its definition is in ide-dma.c...

MBR, Sergei

Re: [PATCH 04/18] ide: remove ide_setup_dma()

[Sergei Shtylyov]

Hi, I wrote:

>> Export sff_dma_ops and then remove ide_setup_dma().

>> There should be no functional changes caused by this patch.

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

>> Index: b/drivers/ide/arm/palm_bk3710.c
>> ===================================================================
>> --- a/drivers/ide/arm/palm_bk3710.c
>> +++ b/drivers/ide/arm/palm_bk3710.c
>> @@ -318,15 +318,14 @@ static u8 __devinit palm_bk3710_cable_de
>>  static int __devinit palm_bk3710_init_dma(ide_hwif_t *hwif,
>>                        const struct ide_port_info *d)
>>  {
>> -    unsigned long base =
>> -        hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
>> -
>>      printk(KERN_INFO "    %s: MMIO-DMA\n", hwif->name);
>>  
>>      if (ide_allocate_dma_engine(hwif))
>>          return -1;
>>  
>> -    ide_setup_dma(hwif, base);
>> +    hwif->dma_base = hwif->io_ports.data_addr - 
>> IDE_PALM_ATA_PRI_REG_OFFSET;
>> +
>> +    hwif->dma_ops = &sff_dma_ops;
>>  
>>      return 0;
>>  }

>    Unfortunately, this patch broke this driver:

> drivers/ide/arm/palm_bk3710.c:327: error: ‘sff_dma_ops’ undeclared 
> (first use in this function). The declatation of sff_dma_ops should've 
> been surrounded by:

> #ifdef CONFIG_BLK_DEV_IDEDMA_SFF

> not by:

> #ifdef CONFIG_BLK_DEV_IDEDMA_PCI

> the same as its definition is in ide-dma.c...

    OK, Kevin Hilman said he's going to post a patch for this issue...

MBR, Sergei

Re: [PATCH 01/18] ide: add ->read_sff_dma_status method

[Sergei Shtylyov]

Bartlomiej Zolnierkiewicz wrote:

> Add ->read_sff_dma_status method for reading DMA Status register
> and use it instead of ->INB.

> While at it:

> * Use inb() directly in ns87415.c::ns87415_dma_end().

> There should be no functional changes caused by this patch.

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

> Index: b/drivers/ide/ide-iops.c
> ===================================================================
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -103,6 +103,14 @@ void SELECT_MASK(ide_drive_t *drive, int
>  		port_ops->maskproc(drive, mask);
>  }
>  
> +static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
> +{
> +	if (hwif->host_flags & IDE_HFLAG_MMIO)
> +		return readb((void __iomem *)hwif->dma_status);
> +	else
> +		return inb(hwif->dma_status);
> +}
> +

    It's probably too late to object now (and I've been busy before) but I 
don't understand how reading DMA status ended up here ISO ide-dma.c...

>  static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
>  {
>  	ide_hwif_t *hwif = drive->hwif;
> @@ -323,6 +331,8 @@ static void ata_output_data(ide_drive_t 
>  
>  void default_hwif_transport(ide_hwif_t *hwif)
>  {
> +	hwif->read_sff_dma_status = ide_read_sff_dma_status;
> +
>  	hwif->tf_load	  = ide_tf_load;
>  	hwif->tf_read	  = ide_tf_read;

    I also didn't understand the motivation behind putting this method 
together with the transport operations... IMO, DMA programming interface 
hardly has anything to do with transporting the data over IDE bus.

MBR, Sergei

Re: [PATCH 01/18] ide: add ->read_sff_dma_status method

[Bartlomiej Zolnierkiewicz]

On Wednesday 03 September 2008, Sergei Shtylyov wrote:
> Bartlomiej Zolnierkiewicz wrote:
> 
> > Add ->read_sff_dma_status method for reading DMA Status register
> > and use it instead of ->INB.
> 
> > While at it:
> 
> > * Use inb() directly in ns87415.c::ns87415_dma_end().
> 
> > There should be no functional changes caused by this patch.
> 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 
> > Index: b/drivers/ide/ide-iops.c
> > ===================================================================
> > --- a/drivers/ide/ide-iops.c
> > +++ b/drivers/ide/ide-iops.c
> > @@ -103,6 +103,14 @@ void SELECT_MASK(ide_drive_t *drive, int
> >  		port_ops->maskproc(drive, mask);
> >  }
> >  
> > +static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
> > +{
> > +	if (hwif->host_flags & IDE_HFLAG_MMIO)
> > +		return readb((void __iomem *)hwif->dma_status);
> > +	else
> > +		return inb(hwif->dma_status);
> > +}
> > +
> 
>     It's probably too late to object now (and I've been busy before) but I 
> don't understand how reading DMA status ended up here ISO ide-dma.c...
> 
> >  static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
> >  {
> >  	ide_hwif_t *hwif = drive->hwif;
> > @@ -323,6 +331,8 @@ static void ata_output_data(ide_drive_t 
> >  
> >  void default_hwif_transport(ide_hwif_t *hwif)
> >  {
> > +	hwif->read_sff_dma_status = ide_read_sff_dma_status;
> > +
> >  	hwif->tf_load	  = ide_tf_load;
> >  	hwif->tf_read	  = ide_tf_read;
> 
>     I also didn't understand the motivation behind putting this method 
> together with the transport operations... IMO, DMA programming interface 
> hardly has anything to do with transporting the data over IDE bus.

The motivation was that hwif->dma_ops is not available yet when
->read_sff_dma_status is used in ide_pci_check_simplex().

However I agree that it should somehow find its way into ->dma_ops
(as usual patches are stongly preffered :).

Thanks,
Bart

Re: [PATCH 01/18] ide: add ->read_sff_dma_status method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>> static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
>>> {
>>> 	ide_hwif_t *hwif = drive->hwif;
>>>@@ -323,6 +331,8 @@ static void ata_output_data(ide_drive_t 
>>> 
>>> void default_hwif_transport(ide_hwif_t *hwif)
>>> {
>>>+	hwif->read_sff_dma_status = ide_read_sff_dma_status;
>>>+
>>> 	hwif->tf_load	  = ide_tf_load;
>>> 	hwif->tf_read	  = ide_tf_read;
>>
>>    I also didn't understand the motivation behind putting this method 
>>together with the transport operations... IMO, DMA programming interface 
>>hardly has anything to do with transporting the data over IDE bus.

> The motivation was that hwif->dma_ops is not available yet when
> ->read_sff_dma_status is used in ide_pci_check_simplex().

> However I agree that it should somehow find its way into ->dma_ops
> (as usual patches are stongly preffered :).

    Unless I'm missing something changing the place where hwif->dma_ops is 
initialized to sff_dma_ops (along the lines it was changed for hwif->dma_base) 
seems pretty trivial, so I wonder why you didn't do it in the same patch...
    Hm, I guess I'm seeing a real issue with this patch...

> Thanks,
> Bart

MBR, Sergei

Re: [PATCH 01/18] ide: add ->read_sff_dma_status method

[Sergei Shtylyov]

Hello, I wrote:

>    Hm, I guess I'm seeing a real issue

    Nevermind, it's just ide_read_sff_dma_status() used for hwif->tp_ops-> 
read_sff_dma_status() method in the driver where it clearly doesn't make 
sense.  But it seems it never gets called in these cases anyway (only adds a 
confusion :-).

> with this patch...

    Not with this patch actually, with the one that introduced 'struct tp_ops'.

>> Thanks,
>> Bart

MBR, Sergei

Re: [PATCH 01/18] ide: add ->read_sff_dma_status method

[Bartlomiej Zolnierkiewicz]

On Sunday 07 September 2008, Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> >>> static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
> >>> {
> >>> 	ide_hwif_t *hwif = drive->hwif;
> >>>@@ -323,6 +331,8 @@ static void ata_output_data(ide_drive_t 
> >>> 
> >>> void default_hwif_transport(ide_hwif_t *hwif)
> >>> {
> >>>+	hwif->read_sff_dma_status = ide_read_sff_dma_status;
> >>>+
> >>> 	hwif->tf_load	  = ide_tf_load;
> >>> 	hwif->tf_read	  = ide_tf_read;
> >>
> >>    I also didn't understand the motivation behind putting this method 
> >>together with the transport operations... IMO, DMA programming interface 
> >>hardly has anything to do with transporting the data over IDE bus.
> 
> > The motivation was that hwif->dma_ops is not available yet when
> > ->read_sff_dma_status is used in ide_pci_check_simplex().
> 
> > However I agree that it should somehow find its way into ->dma_ops
> > (as usual patches are stongly preffered :).
> 
>     Unless I'm missing something changing the place where hwif->dma_ops is 
> initialized to sff_dma_ops (along the lines it was changed for hwif->dma_base) 
> seems pretty trivial, so I wonder why you didn't do it in the same patch...

Indeed, it should be trivial now, one just needs to be careful to:

* move 'if (d->dma_ops) ...' from ide_init_port() into
  ->init_dma/ide_hwif_setup_dma()

* unset ->dma_ops on ->init_dma/ide_hwif_setup_dma() failures

I guess I overlooked it ATM of making the patch (or the code evolved
greatly in the meantime)...

[ It is really time consuming and difficult to recall the every small
  detail of every patch after few months (the patch was posted 10 weeks
  ago and merged 6 weeks ago)...  The most efficient way of handling
  such issues upon discovery is with sending patches... ]

Thanks,
Bart

Re: [PATCH 01/18] ide: add ->read_sff_dma_status method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>>> static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
>>>>> {
>>>>> 	ide_hwif_t *hwif = drive->hwif;
>>>>> @@ -323,6 +331,8 @@ static void ata_output_data(ide_drive_t 
>>>>>
>>>>> void default_hwif_transport(ide_hwif_t *hwif)
>>>>> {
>>>>> +	hwif->read_sff_dma_status = ide_read_sff_dma_status;
>>>>> +
>>>>> 	hwif->tf_load	  = ide_tf_load;
>>>>> 	hwif->tf_read	  = ide_tf_read;
>>>>>           
>>>>    I also didn't understand the motivation behind putting this method 
>>>> together with the transport operations... IMO, DMA programming interface 
>>>> hardly has anything to do with transporting the data over IDE bus.
>>>>         
>>> The motivation was that hwif->dma_ops is not available yet when
>>> ->read_sff_dma_status is used in ide_pci_check_simplex().
>>>       
>>> However I agree that it should somehow find its way into ->dma_ops
>>> (as usual patches are stongly preffered :).
>>>       
>>     Unless I'm missing something changing the place where hwif->dma_ops is 
>> initialized to sff_dma_ops (along the lines it was changed for hwif->dma_base) 
>> seems pretty trivial, so I wonder why you didn't do it in the same patch...
>>     

   Ah, I forgot for a moment that there were two patches and it would 
have make no sense to do that in the patch that factored out 
ide_pci_check_simplex()... And then tre was a patch introducing 'struct 
ide_tp_ops' which incorporated the read_sff_dma_status() method.

> Indeed, it should be trivial now, one just needs to be careful to:
>
> * move 'if (d->dma_ops) ...' from ide_init_port() into
>   ->init_dma/ide_hwif_setup_dma()
>
> * unset ->dma_ops on ->init_dma/ide_hwif_setup_dma() failures
>   

   Sure.

> I guess I overlooked it ATM of making the patch (or the code evolved
> greatly in the meantime)...
>   

   I think I understand now: it's sticking read_sff_dma_status() method 
into 'struct ide_tp_ops' that was a wrong move that's worth undoing (by 
putting it where it really belongs).

> [ It is really time consuming and difficult to recall the every small
>   detail of every patch after few months (the patch was posted 10 weeks
>   

   Heh, as if it wasn't time consuming to untange that after a few 
months (when I'm suposed to spend time elsewhere :-)...

>   ago and merged 6 weeks ago)...  The most efficient way of handling
>   such issues upon discovery is with sending patches... ]
>   

    Sigh, I'll see what I can do in my currently very limieted time...

> Thanks,
> Bart
>   

MBR, Sergei

Re: [PATCH 02/18] ide: use I/O ops directly in ide-dma.c

[Sergei Shtylyov]

Bartlomiej Zolnierkiewicz wrote:

> Use I/O ops directly in ide_dma_host_set(), ide_dma_setup(),
> ide_dma_start() and __ide_dma_end().

> There should be no functional changes caused by this patch.

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
[...]
> Index: b/drivers/ide/ide-dma.c
> ===================================================================
> --- a/drivers/ide/ide-dma.c
> +++ b/drivers/ide/ide-dma.c
> @@ -376,7 +376,10 @@ void ide_dma_host_set(ide_drive_t *drive
>  	else
>  		dma_stat &= ~(1 << (5 + unit));
>  
> -	hwif->OUTB(dma_stat, hwif->dma_status);
> +	if (hwif->host_flags & IDE_HFLAG_MMIO)
> +		writeb(dma_stat, (void __iomem *)hwif->dma_status);
> +	else
> +		outb(dma_stat, hwif->dma_status);
>  }
>  
>  EXPORT_SYMBOL_GPL(ide_dma_host_set);
> @@ -470,13 +474,20 @@ int ide_dma_setup(ide_drive_t *drive)
[...]
>  	/* read DMA status for INTR & ERROR flags */
>  	dma_stat = hwif->read_sff_dma_status(hwif);
>  
>  	/* clear INTR & ERROR flags */
> -	hwif->OUTB(dma_stat|6, hwif->dma_status);
> +	if (mmio)
> +		writeb(dma_stat | 6, (void __iomem *)hwif->dma_status);
> +	else
> +		outb(dma_stat | 6, hwif->dma_status);
> +
>  	drive->waiting_for_dma = 1;
>  	return 0;
>  }
> @@ -511,18 +529,31 @@ EXPORT_SYMBOL_GPL(ide_dma_start);
[...]
>  	/* get DMA status */
>  	dma_stat = hwif->read_sff_dma_status(hwif);
> -	/* clear the INTR & ERROR bits */
> -	hwif->OUTB(dma_stat|6, hwif->dma_status);
> +
> +	if (mmio)
> +		/* clear the INTR & ERROR bits */
> +		writeb(dma_stat | 6, (void __iomem *)hwif->dma_status);
> +	else
> +		outb(dma_stat | 6, hwif->dma_status);
> +

    Seems like the above 3 sequences are asking to be factored out into 
ide_dma_write_status(); with the latter two possibly factored out into 
ide_dma_clear_status(). I'm making a notch if you don't mind (or beat me to it 
:-)...

MBR, Sergei

Re: [PATCH 09/18] ide: add ->set_irq method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

> Add ->set_irq method for setting nIEN bit of ATA Device Control
> register and use it instead of ide_set_irq().
>
> While at it:
>
> * Use ->set_irq in init_irq() and do_reset1().
>
> * Don't use HWIF() macro in ide_check_pm_state().
>
> There should be no functional changes caused by this patch.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>   
[...]
> Index: b/drivers/ide/ide-iops.c
> ===================================================================
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -135,6 +135,23 @@ static u8 ide_read_sff_dma_status(ide_hw
>  		return inb(hwif->dma_base + ATA_DMA_STATUS);
>  }
>  
> +static void ide_set_irq(ide_hwif_t *hwif, int on)
> +{
> +	u8 ctl = ATA_DEVCTL_OBS;
> +
> +	if (on == 4) { /* hack for SRST */
> +		ctl |= 4;
> +		on &= ~4;
> +	}
> +
> +	ctl |= on ? 0 : 2;
> +
> +	if (hwif->host_flags & IDE_HFLAG_MMIO)
> +		writeb(ctl, (void __iomem *)hwif->io_ports.ctl_addr);
> +	else
> +		outb(ctl, hwif->io_ports.ctl_addr);
> +}
> +
>  static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
>  {
>  	ide_hwif_t *hwif = drive->hwif;
>   
[...]
> @@ -1162,16 +1180,15 @@ static ide_startstop_t do_reset1 (ide_dr
>  	 * immediate interrupt due to the edge transition it produces.
>  	 * This single interrupt gives us a "fast poll" for drives that
>  	 * recover from reset very quickly, saving us the first 50ms wait time.
> +	 *
> +	 * TODO: add ->softreset method and stop abusing ->set_irq
>  	 */
>  	/* set SRST and nIEN */
> -	hwif->OUTBSYNC(hwif, ATA_DEVCTL_OBS | 6, io_ports->ctl_addr);
> +	hwif->set_irq(hwif, 4);
>  	/* more than enough time */
>  	udelay(10);

   It seems to me that the whole idea of having 2 separate methods for 
writing the single device control register is a wrong idea.
I'm suggesting to convert set_irq() method to write_devctrl() method and 
pass the register value directly to it.

MBR, Sergei

Re: [PATCH 09/18] ide: add ->set_irq method

[Bartlomiej Zolnierkiewicz]

On Wednesday 15 October 2008, Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> > Add ->set_irq method for setting nIEN bit of ATA Device Control
> > register and use it instead of ide_set_irq().
> >
> > While at it:
> >
> > * Use ->set_irq in init_irq() and do_reset1().
> >
> > * Don't use HWIF() macro in ide_check_pm_state().
> >
> > There should be no functional changes caused by this patch.
> >
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >   
> [...]
> > Index: b/drivers/ide/ide-iops.c
> > ===================================================================
> > --- a/drivers/ide/ide-iops.c
> > +++ b/drivers/ide/ide-iops.c
> > @@ -135,6 +135,23 @@ static u8 ide_read_sff_dma_status(ide_hw
> >  		return inb(hwif->dma_base + ATA_DMA_STATUS);
> >  }
> >  
> > +static void ide_set_irq(ide_hwif_t *hwif, int on)
> > +{
> > +	u8 ctl = ATA_DEVCTL_OBS;
> > +
> > +	if (on == 4) { /* hack for SRST */
> > +		ctl |= 4;
> > +		on &= ~4;
> > +	}
> > +
> > +	ctl |= on ? 0 : 2;
> > +
> > +	if (hwif->host_flags & IDE_HFLAG_MMIO)
> > +		writeb(ctl, (void __iomem *)hwif->io_ports.ctl_addr);
> > +	else
> > +		outb(ctl, hwif->io_ports.ctl_addr);
> > +}
> > +
> >  static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
> >  {
> >  	ide_hwif_t *hwif = drive->hwif;
> >   
> [...]
> > @@ -1162,16 +1180,15 @@ static ide_startstop_t do_reset1 (ide_dr
> >  	 * immediate interrupt due to the edge transition it produces.
> >  	 * This single interrupt gives us a "fast poll" for drives that
> >  	 * recover from reset very quickly, saving us the first 50ms wait time.
> > +	 *
> > +	 * TODO: add ->softreset method and stop abusing ->set_irq
> >  	 */
> >  	/* set SRST and nIEN */
> > -	hwif->OUTBSYNC(hwif, ATA_DEVCTL_OBS | 6, io_ports->ctl_addr);
> > +	hwif->set_irq(hwif, 4);
> >  	/* more than enough time */
> >  	udelay(10);
> 
>    It seems to me that the whole idea of having 2 separate methods for 
> writing the single device control register is a wrong idea.
> I'm suggesting to convert set_irq() method to write_devctrl() method and 
> pass the register value directly to it.

Makes sense.  Care to make a patch?

Re: [PATCH 09/18] ide: add ->set_irq method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>> Add ->set_irq method for setting nIEN bit of ATA Device Control
>>> register and use it instead of ide_set_irq().
>>>
>>> While at it:
>>>
>>> * Use ->set_irq in init_irq() and do_reset1().
>>>
>>> * Don't use HWIF() macro in ide_check_pm_state().
>>>
>>> There should be no functional changes caused by this patch.
>>>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>>   
>>>       
>> [...]
>>     
>>> Index: b/drivers/ide/ide-iops.c
>>> ===================================================================
>>> --- a/drivers/ide/ide-iops.c
>>> +++ b/drivers/ide/ide-iops.c
>>> @@ -135,6 +135,23 @@ static u8 ide_read_sff_dma_status(ide_hw
>>>  		return inb(hwif->dma_base + ATA_DMA_STATUS);
>>>  }
>>>  
>>> +static void ide_set_irq(ide_hwif_t *hwif, int on)
>>> +{
>>> +	u8 ctl = ATA_DEVCTL_OBS;
>>> +
>>> +	if (on == 4) { /* hack for SRST */
>>> +		ctl |= 4;
>>> +		on &= ~4;
>>> +	}
>>> +
>>> +	ctl |= on ? 0 : 2;
>>> +
>>> +	if (hwif->host_flags & IDE_HFLAG_MMIO)
>>> +		writeb(ctl, (void __iomem *)hwif->io_ports.ctl_addr);
>>> +	else
>>> +		outb(ctl, hwif->io_ports.ctl_addr);
>>> +}
>>> +
>>>  static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
>>>  {
>>>  	ide_hwif_t *hwif = drive->hwif;
>>>   
>>>       
>> [...]
>>     
>>> @@ -1162,16 +1180,15 @@ static ide_startstop_t do_reset1 (ide_dr
>>>  	 * immediate interrupt due to the edge transition it produces.
>>>  	 * This single interrupt gives us a "fast poll" for drives that
>>>  	 * recover from reset very quickly, saving us the first 50ms wait time.
>>> +	 *
>>> +	 * TODO: add ->softreset method and stop abusing ->set_irq
>>>  	 */
>>>  	/* set SRST and nIEN */
>>> -	hwif->OUTBSYNC(hwif, ATA_DEVCTL_OBS | 6, io_ports->ctl_addr);
>>> +	hwif->set_irq(hwif, 4);
>>>  	/* more than enough time */
>>>  	udelay(10);
>>>       
>>    It seems to me that the whole idea of having 2 separate methods for 
>> writing the single device control register is a wrong idea.
>> I'm suggesting to convert set_irq() method to write_devctrl() method and 
>> pass the register value directly to it.
>>     

   Perhaps with the method itself forcing the obsolete bit 3 set like 
it's done now...

> Makes sense.  Care to make a patch?
>   

   I would if I had the time. Can only make another notch for now...

MBR, Sergei

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Sergei Shtylyov]

Bartlomiej Zolnierkiewicz wrote:

> Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.

> There should be no functional changes caused by this patch.

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

> Index: b/drivers/ide/ide-iops.c
> ===================================================================
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
>  {
>  	ide_hwif_t *hwif = drive->hwif;
>  	const struct ide_port_ops *port_ops = hwif->port_ops;
> +	ide_task_t task;
>  
>  	if (port_ops && port_ops->selectproc)
>  		port_ops->selectproc(drive);
>  
> -	hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
> +	memset(&task, 0, sizeof(task));
> +	task.tf_flags = IDE_TFLAG_OUT_DEVICE;
> +
> +	drive->hwif->tf_load(drive, &task);

    This actually doesn't seem like a bright idea to me, considering that this 
gets called when starting every request. How will you look at me adding the 
transport method for writing this register? :-)

MBR, Sergei

Re: [PATCH 14/18] ide: use ->tf_read in ide_read_error()

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

> * Add IDE_TFLAG_IN_FEATURE taskfile flag for reading Feature
>   register and handle it in ->tf_read.
>   

  Doesn't seem like a good name -- you can't read the features register. 
I should've commented to this patch earlier, of course...

MBR, Sergei

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Sergei Shtylyov]

Hello, I wrote:

>> Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.
>> There should be no functional changes caused by this patch.
>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>> Index: b/drivers/ide/ide-iops.c
>> ===================================================================
>> --- a/drivers/ide/ide-iops.c
>> +++ b/drivers/ide/ide-iops.c
>> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
>>  {
>>      ide_hwif_t *hwif = drive->hwif;
>>      const struct ide_port_ops *port_ops = hwif->port_ops;
>> +    ide_task_t task;
>>  
>>      if (port_ops && port_ops->selectproc)
>>          port_ops->selectproc(drive);
>>  
>> -    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
>> +    memset(&task, 0, sizeof(task));
>> +    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
>> +
>> +    drive->hwif->tf_load(drive, &task);
>
>    This actually doesn't seem like a bright idea to me, considering 
> that this gets called when starting every request. How will you look 
> at me adding the transport method for writing this register? :-)

   OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
consistent... well, I'm not excited about the whole idea conversion to 
tf_{load|read}() -- it's not clear what exactly this bought us.

MBR, Sergei

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Sergei Shtylyov]

Hello, I wrote:

>>> There should be no functional changes caused by this patch.
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>> Index: b/drivers/ide/ide-iops.c
>>> ===================================================================
>>> --- a/drivers/ide/ide-iops.c
>>> +++ b/drivers/ide/ide-iops.c
>>> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
>>>  {
>>>      ide_hwif_t *hwif = drive->hwif;
>>>      const struct ide_port_ops *port_ops = hwif->port_ops;
>>> +    ide_task_t task;
>>>  
>>>      if (port_ops && port_ops->selectproc)
>>>          port_ops->selectproc(drive);
>>>  
>>> -    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
>>> +    memset(&task, 0, sizeof(task));
>>> +    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
>>> +
>>> +    drive->hwif->tf_load(drive, &task);
>>
>>    This actually doesn't seem like a bright idea to me, considering 
>> that this gets called when starting every request. How will you look 
>> at me adding the transport method for writing this register? :-)
> Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.
>
>   OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
> consistent... well, I'm not excited about the whole idea conversion to 
> tf_{load|read}() -- it's not clear what exactly this bought us.

   We at least could have saved on memset() -- tf_load() method ignores 
fields other than tf_flags anyway...

MBR, Sergei

Re: [PATCH 14/18] ide: use ->tf_read in ide_read_error()

[Sergei Shtylyov]

Hello, I wrote:

>> * Add IDE_TFLAG_IN_FEATURE taskfile flag for reading Feature
>>   register and handle it in ->tf_read.
>>   
>
>  Doesn't seem like a good name -- you can't read the features 
> register. I should've commented to this patch earlier, of course...

   Moreover, the error register shoudn't be affeceted by the HOB bit, so 
this flag doesn't make sense.

MBR, Sergei

Re: [PATCH 14/18] ide: use ->tf_read in ide_read_error()

[Sergei Shtylyov]

Hello, I wrote:

>>>   register and handle it in ->tf_read.
>>>   
>>
>>  Doesn't seem like a good name -- you can't read the features 
>> register. I should've commented to this patch earlier, of course...
> * Add IDE_TFLAG_IN_FEATURE taskfile flag for reading Feature
>
>   Moreover, the error register shoudn't be affeceted by the HOB bit, 
> so this flag doesn't make sense.

   I meant IDE_TFLAG_IN_HOB_FEATURE, of course.

MBR, Sergei

Re: [PATCH 14/18] ide: use ->tf_read in ide_read_error()

[Bartlomiej Zolnierkiewicz]

On Monday 16 February 2009, Sergei Shtylyov wrote:
> Hello, I wrote:
> 
> >>>   register and handle it in ->tf_read.
> >>>   
> >>
> >>  Doesn't seem like a good name -- you can't read the features 
> >> register. I should've commented to this patch earlier, of course...

[ Yeah... preferably in June 2008 when this patch was posted... ]

> > * Add IDE_TFLAG_IN_FEATURE taskfile flag for reading Feature

Trivial s/IN_*FEATURE/IN_*ERROR/ should do the job.

I will fix it later unless you beat me to it.

> >   Moreover, the error register shoudn't be affeceted by the HOB bit, 
> > so this flag doesn't make sense.
> 
>    I meant IDE_TFLAG_IN_HOB_FEATURE, of course.

Once again blame the certain ioctl (you know which one)... :)

Thanks,
Bart

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Bartlomiej Zolnierkiewicz]

On Monday 16 February 2009, Sergei Shtylyov wrote:
> Hello, I wrote:
> 
> >>> There should be no functional changes caused by this patch.
> >>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >>> Index: b/drivers/ide/ide-iops.c
> >>> ===================================================================
> >>> --- a/drivers/ide/ide-iops.c
> >>> +++ b/drivers/ide/ide-iops.c
> >>> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
> >>>  {
> >>>      ide_hwif_t *hwif = drive->hwif;
> >>>      const struct ide_port_ops *port_ops = hwif->port_ops;
> >>> +    ide_task_t task;
> >>>  
> >>>      if (port_ops && port_ops->selectproc)
> >>>          port_ops->selectproc(drive);
> >>>  
> >>> -    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
> >>> +    memset(&task, 0, sizeof(task));
> >>> +    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
> >>> +
> >>> +    drive->hwif->tf_load(drive, &task);
> >>
> >>    This actually doesn't seem like a bright idea to me, considering 
> >> that this gets called when starting every request. How will you look 
> >> at me adding the transport method for writing this register? :-)

Please check profiles first -- it might not be worth it. [1]

> > Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.
> >
> >   OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
> > consistent... well, I'm not excited about the whole idea conversion to 
> > tf_{load|read}() -- it's not clear what exactly this bought us.

This was explained some months ago already, so just to recall -- it was
a part of a bigger work removing duplicated code and allowing abstraction
of the ATA logic.

Anyway this is not set in a stone so if you have proposal of a better
approach please come forward with it.

>    We at least could have saved on memset() -- tf_load() method ignores 
> fields other than tf_flags anyway...

Unless it is huge performance win (unlikely) this is not a good idea
as it would be a maintainance nightmare.

->tf_load does only use cmd->tf_flags today but it might change one day
and nobody will remember to audit all users that they pass a valid cmd...

Thanks,
Bart

[1] coincidentally on the past Saturday I woke up with a bright idea
of doing some profiling of IDE code... I thought this would be a fun...

# readprofile -m System.map | grep ide_
    30 ide_intr                                   0.0714
     1 ide_map_sg                                 0.0112
     1 ide_complete_rq                            0.0152
    78 do_ide_request                             0.0643
     1 __ide_wait_stat                            0.0059
    51 ide_execute_command                        0.5100
     4 ide_set_handler                            0.0635
     7 ide_outb                                   1.1667
   308 ide_mm_inb                                44.0000
...

This was on ICH4 PATA controller...  Fun indeed, ain't it?

[ For non-ATA folks: it is _impossible_ for ide_mm_inb to be used
  on the above controller. ]

I still have to check what will happen if I would change the order
of following assignments in ide_tf_read():

...
        if (mmio) {
                tf_outb = ide_mm_outb;
                tf_inb  = ide_mm_inb;
        } else {
                tf_outb = ide_outb;
                tf_inb  = ide_inb;
        }
...

However I instead I went ahead and tried to run oprofile to get me some
more trushworthy results:

# opreport -l vmlinux
CPU: Pentium M (P6 core), speed 600 MHz (estimated)
Counted CPU_CLK_UNHALTED events (clocks processor is not halted, and not in a thermal trip) with a unit mask of 0x00 (No unit mask) count 100000
Segmentation fault

After this, testing some older kernel versions and assuring that user-space
packages are at their latest distro versions I had enough of fun...

Re: [PATCH 14/18] ide: use ->tf_read in ide_read_error()

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>>>   register and handle it in ->tf_read.
>>>>>   
>>>>>           
>>>>  Doesn't seem like a good name -- you can't read the features 
>>>> register. I should've commented to this patch earlier, of course...
>>>>         
>
> [ Yeah... preferably in June 2008 when this patch was posted... ]
>   

   I was too busy acquainting myself with the wonderful world of ARM and 
MUSB back then... :-/

>>> * Add IDE_TFLAG_IN_FEATURE taskfile flag for reading Feature
>>>       
>
> Trivial s/IN_*FEATURE/IN_*ERROR/ should do the job.
>
> I will fix it later unless you beat me to it.
>   

   Well, if I'm going to get my hands in the transport methods anyway, I 
think I'll address it.

>>>   Moreover, the error register shoudn't be affeceted by the HOB bit, 
>>> so this flag doesn't make sense.
>>>       
>>    I meant IDE_TFLAG_IN_HOB_FEATURE, of course.
>>     
>
> Once again blame the certain ioctl (you know which one)... :)
>   

   Oh, horror... and I know that it wasn't completely ungrounded since 
both ATA/PI-6 adn -7 have words about reading the features register 
(depending on HOB). At least ATA/PI-8 got rid of this. Anyway, I think 
we can safely get rid of this flag and just return the same value in 
'features' and 'hob_features'. Besides, union ide_reg_valid_s also has 
such interesting fields as 'select_hob' and 'control_hob' but they seem 
to be ignored altogether (as long as 'select') if I don't mistake...

> Thanks,
> Bart
>   

MBR, Sergei

Re: [PATCH 14/18] ide: use ->tf_read in ide_read_error()

[Sergei Shtylyov]

Hello, I wrote:

>>>>       
>>>    I meant IDE_TFLAG_IN_HOB_FEATURE, of course.
>>>     
>>
>> Once again blame the certain ioctl (you know which one)... :)
>>   
>   Moreover, the error register shoudn't be affeceted by the HOB bit, 
> so this flag doesn't make sense.
>
>   Oh, horror... and I know that it wasn't completely ungrounded since 
> both ATA/PI-6 adn -7 have words about reading the features register 
> (depending on HOB). At least ATA/PI-8 got rid of this. Anyway, I think 
> we can safely get rid of this flag and just return the same value in 
> 'features' and 'hob_features'.

  Although, after thinking a bit more, it doesn't seem worth the trouble 
-- so the rename should suffice...

MBR, Sergei

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:
> On Monday 16 February 2009, Sergei Shtylyov wrote:
>   
>> Hello, I wrote:
>>
>>     
>>>>> There should be no functional changes caused by this patch.
>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>>>> Index: b/drivers/ide/ide-iops.c
>>>>> ===================================================================
>>>>> --- a/drivers/ide/ide-iops.c
>>>>> +++ b/drivers/ide/ide-iops.c
>>>>> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
>>>>>  {
>>>>>      ide_hwif_t *hwif = drive->hwif;
>>>>>      const struct ide_port_ops *port_ops = hwif->port_ops;
>>>>> +    ide_task_t task;
>>>>>  
>>>>>      if (port_ops && port_ops->selectproc)
>>>>>          port_ops->selectproc(drive);
>>>>>  
>>>>> -    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
>>>>> +    memset(&task, 0, sizeof(task));
>>>>> +    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
>>>>> +
>>>>> +    drive->hwif->tf_load(drive, &task);
>>>>>           
>>>>    This actually doesn't seem like a bright idea to me, considering 
>>>> that this gets called when starting every request. How will you look 
>>>> at me adding the transport method for writing this register? :-)
>>>>         
>
> Please check profiles first -- it might not be worth it. [1]
>
>   
>>> Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.
>>>
>>>   OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
>>> consistent... well, I'm not excited about the whole idea conversion to 
>>> tf_{load|read}() -- it's not clear what exactly this bought us.
>>>       
>
> This was explained some months ago already, so just to recall -- it was
> a part of a bigger work removing duplicated code and allowing abstraction
> of the ATA logic.
>
> Anyway this is not set in a stone so if you have proposal of a better
> approach please come forward with it.
>   

   Er... I think that the previous IN()/OUT() methods were better. Note 
that we ended up using the local version of them in the dafault 
ide_tf_{load}read}() anyway -- as Alan has pointed out it might be worth 
splitting those into I/O and memory space versions... although given 
general slowness of the I/O accesses, this is probably not going to win 
much speed-wise.

>>    We at least could have saved on memset() -- tf_load() method ignores 
>> fields other than tf_flags anyway...
>>     
>
> Unless it is huge performance win (unlikely) this is not a good idea as it would be a maintainance nightmare.
>   
> ->tf_load does only use cmd->tf_flags today but it might change one day
> and nobody will remember to audit all users that they pass a valid cmd...
>   

   It's just quite unbearable to see (especially for a long time 
assembly coder) how a single register write is turning into *that*.
So, it still seems worth risking... :-)

> Thanks,
> Bart
>   

MBR, Sergei

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Sergei Shtylyov]

Hello, I wrote:

>>> There should be no functional changes caused by this patch.
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>> Index: b/drivers/ide/ide-iops.c
>>> ===================================================================
>>> --- a/drivers/ide/ide-iops.c
>>> +++ b/drivers/ide/ide-iops.c
>>> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
>>>  {
>>>      ide_hwif_t *hwif = drive->hwif;
>>>      const struct ide_port_ops *port_ops = hwif->port_ops;
>>> +    ide_task_t task;
>>>  
>>>      if (port_ops && port_ops->selectproc)
>>>          port_ops->selectproc(drive);
>>>  
>>> -    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
>>> +    memset(&task, 0, sizeof(task));
>>> +    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
>>> +
>>> +    drive->hwif->tf_load(drive, &task);
>>
>>    This actually doesn't seem like a bright idea to me, considering 
>> that this gets called when starting every request. How will you look 
>> at me adding the transport method for writing this register? :-)
> Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.
>
>   OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
> consistent...

   ... which didn't prevent libata from having such backdoor though, in 
the form of dev_select() method.

MBR, Sergei

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Bartlomiej Zolnierkiewicz]

On Tuesday 17 February 2009, Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> > On Monday 16 February 2009, Sergei Shtylyov wrote:
> >   
> >> Hello, I wrote:
> >>
> >>     
> >>>>> There should be no functional changes caused by this patch.
> >>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >>>>> Index: b/drivers/ide/ide-iops.c
> >>>>> ===================================================================
> >>>>> --- a/drivers/ide/ide-iops.c
> >>>>> +++ b/drivers/ide/ide-iops.c
> >>>>> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
> >>>>>  {
> >>>>>      ide_hwif_t *hwif = drive->hwif;
> >>>>>      const struct ide_port_ops *port_ops = hwif->port_ops;
> >>>>> +    ide_task_t task;
> >>>>>  
> >>>>>      if (port_ops && port_ops->selectproc)
> >>>>>          port_ops->selectproc(drive);
> >>>>>  
> >>>>> -    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
> >>>>> +    memset(&task, 0, sizeof(task));
> >>>>> +    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
> >>>>> +
> >>>>> +    drive->hwif->tf_load(drive, &task);
> >>>>>           
> >>>>    This actually doesn't seem like a bright idea to me, considering 
> >>>> that this gets called when starting every request. How will you look 
> >>>> at me adding the transport method for writing this register? :-)
> >>>>         
> >
> > Please check profiles first -- it might not be worth it. [1]
> >
> >   
> >>> Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.
> >>>
> >>>   OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
> >>> consistent... well, I'm not excited about the whole idea conversion to 
> >>> tf_{load|read}() -- it's not clear what exactly this bought us.
> >>>       
> >
> > This was explained some months ago already, so just to recall -- it was
> > a part of a bigger work removing duplicated code and allowing abstraction
> > of the ATA logic.
> >
> > Anyway this is not set in a stone so if you have proposal of a better
> > approach please come forward with it.
> >   
> 
>    Er... I think that the previous IN()/OUT() methods were better. Note 
> that we ended up using the local version of them in the dafault 
> ide_tf_{load}read}() anyway -- as Alan has pointed out it might be worth 

During ide_tf_{load,read}() addition I was a bit too optimistic about
the possibility of the quick io{read,write}* conversion later...

> splitting those into I/O and memory space versions... although given 
> general slowness of the I/O accesses, this is probably not going to win 
> much speed-wise.

Maybe it would be worth to add ->tf_{inb,outb} to struct ide_tp_ops
and convert default tp_ops to use them...  OTOH we should reinvestigate
the io{read,write}*() way first (maybe things have improved there)...

> >>    We at least could have saved on memset() -- tf_load() method ignores 
> >> fields other than tf_flags anyway...
> >>     
> >
> > Unless it is huge performance win (unlikely) this is not a good idea as it would be a maintainance nightmare.
> >   
> > ->tf_load does only use cmd->tf_flags today but it might change one day
> > and nobody will remember to audit all users that they pass a valid cmd...
> >   
> 
>    It's just quite unbearable to see (especially for a long time 
> assembly coder) how a single register write is turning into *that*.
> So, it still seems worth risking... :-)

I see your point here.  If SELECT_DRIVE() is performance sensitive we
may just add another struct ide_tp_ops method for it...

Thanks,
Bart

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Sergei Shtylyov]

Hello, I wrote:

>>>> There should be no functional changes caused by this patch.
>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>>> Index: b/drivers/ide/ide-iops.c
>>>> ===================================================================
>>>> --- a/drivers/ide/ide-iops.c
>>>> +++ b/drivers/ide/ide-iops.c
>>>> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
>>>>  {
>>>>      ide_hwif_t *hwif = drive->hwif;
>>>>      const struct ide_port_ops *port_ops = hwif->port_ops;
>>>> +    ide_task_t task;
>>>>  
>>>>      if (port_ops && port_ops->selectproc)
>>>>          port_ops->selectproc(drive);
>>>>  
>>>> -    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
>>>> +    memset(&task, 0, sizeof(task));
>>>> +    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
>>>> +
>>>> +    drive->hwif->tf_load(drive, &task);

>>>    This actually doesn't seem like a bright idea to me, considering 
>>> that this gets called when starting every request. How will you look 
>>> at me adding the transport method for writing this register? :-)

>> Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.

>>   OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
>> consistent...

>   ... which didn't prevent libata from having such backdoor though, in 
> the form of dev_select() method.

    Oh, and I forgot about the IDE core's infamous selectproc() method which 
could easily be taught to also write the device/head register and moved to 
'struct ide_tp_ops'.

MBR, Sergei

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Sergei Shtylyov]

Bartlomiej Zolnierkiewicz wrote:

>>>>>>>There should be no functional changes caused by this patch.
>>>>>>>Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>>>>>>Index: b/drivers/ide/ide-iops.c
>>>>>>>===================================================================
>>>>>>>--- a/drivers/ide/ide-iops.c
>>>>>>>+++ b/drivers/ide/ide-iops.c
>>>>>>>@@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
>>>>>>> {
>>>>>>>     ide_hwif_t *hwif = drive->hwif;
>>>>>>>     const struct ide_port_ops *port_ops = hwif->port_ops;
>>>>>>>+    ide_task_t task;
>>>>>>> 
>>>>>>>     if (port_ops && port_ops->selectproc)
>>>>>>>         port_ops->selectproc(drive);
>>>>>>> 
>>>>>>>-    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
>>>>>>>+    memset(&task, 0, sizeof(task));
>>>>>>>+    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
>>>>>>>+
>>>>>>>+    drive->hwif->tf_load(drive, &task);

>>>>>>   This actually doesn't seem like a bright idea to me, considering 
>>>>>>that this gets called when starting every request. How will you look 
>>>>>>at me adding the transport method for writing this register? :-)

>>>Please check profiles first -- it might not be worth it. [1]

>>>>>Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.

>>>>>  OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
>>>>>consistent... well, I'm not excited about the whole idea conversion to 
>>>>>tf_{load|read}() -- it's not clear what exactly this bought us.

>>>This was explained some months ago already, so just to recall -- it was
>>>a part of a bigger work removing duplicated code and allowing abstraction
>>>of the ATA logic.

>>>Anyway this is not set in a stone so if you have proposal of a better
>>>approach please come forward with it.

>>   Er... I think that the previous IN()/OUT() methods were better. Note 
>>that we ended up using the local version of them in the dafault 
>>ide_tf_{load}read}() anyway -- as Alan has pointed out it might be worth 

> During ide_tf_{load,read}() addition I was a bit too optimistic about
> the possibility of the quick io{read,write}* conversion later...

>>splitting those into I/O and memory space versions... although given 
>>general slowness of the I/O accesses, this is probably not going to win 
>>much speed-wise.

> Maybe it would be worth to add ->tf_{inb,outb} to struct ide_tp_ops
> and convert default tp_ops to use them...  OTOH we should reinvestigate
> the io{read,write}*() way first (maybe things have improved there)...

    Yes, let's not be hasty here...

>>>>   We at least could have saved on memset() -- tf_load() method ignores 
>>>>fields other than tf_flags anyway...

>>>Unless it is huge performance win (unlikely) this is not a good idea as it would be a maintainance nightmare.

>>>->tf_load does only use cmd->tf_flags today but it might change one day
>>>and nobody will remember to audit all users that they pass a valid cmd...

>>   It's just quite unbearable to see (especially for a long time 
>>assembly coder) how a single register write is turning into *that*.
>>So, it still seems worth risking... :-)

> I see your point here.  If SELECT_DRIVE() is performance sensitive we
> may just add another struct ide_tp_ops method for it...

    Or we may finally teach selectproc() to also do that, turning it into 
analog of libata's dev_select() method.

> Thanks,
> Bart

MBR, Sergei

Re: [PATCH 13/18] ide: use ->tf_load in SELECT_DRIVE()

[Sergei Shtylyov]

Hello, I wrote:

>>>>>>>> There should be no functional changes caused by this patch.
>>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>>>>>>> Index: b/drivers/ide/ide-iops.c
>>>>>>>> ===================================================================
>>>>>>>> --- a/drivers/ide/ide-iops.c
>>>>>>>> +++ b/drivers/ide/ide-iops.c
>>>>>>>> @@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
>>>>>>>> {
>>>>>>>>     ide_hwif_t *hwif = drive->hwif;
>>>>>>>>     const struct ide_port_ops *port_ops = hwif->port_ops;
>>>>>>>> +    ide_task_t task;

>>>>>>>>     if (port_ops && port_ops->selectproc)
>>>>>>>>         port_ops->selectproc(drive);
>>>>>>>>
>>>>>>>> -    hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
>>>>>>>> +    memset(&task, 0, sizeof(task));
>>>>>>>> +    task.tf_flags = IDE_TFLAG_OUT_DEVICE;
>>>>>>>> +
>>>>>>>> +    drive->hwif->tf_load(drive, &task);

>>>>>>>   This actually doesn't seem like a bright idea to me, 
>>>>>>> considering that this gets called when starting every request. 
>>>>>>> How will you look at me adding the transport method for writing 
>>>>>>> this register? :-)

>>>> Please check profiles first -- it might not be worth it. [1]

>>>>>> Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.

>>>>>>  OTOH, adding such a "backdoor" to the taskfile doesn't seem very 
>>>>>> consistent... well, I'm not excited about the whole idea 
>>>>>> conversion to tf_{load|read}() -- it's not clear what exactly this 
>>>>>> bought us.

>>>> This was explained some months ago already, so just to recall -- it was
>>>> a part of a bigger work removing duplicated code and allowing 
>>>> abstraction
>>>> of the ATA logic.

>>>> Anyway this is not set in a stone so if you have proposal of a better
>>>> approach please come forward with it.

>>>   Er... I think that the previous IN()/OUT() methods were better. 
>>> Note that we ended up using the local version of them in the dafault 
>>> ide_tf_{load}read}() anyway -- as Alan has pointed out it might be worth 

>> During ide_tf_{load,read}() addition I was a bit too optimistic about
>> the possibility of the quick io{read,write}* conversion later...

>>> splitting those into I/O and memory space versions... although given 
>>> general slowness of the I/O accesses, this is probably not going to 
>>> win much speed-wise.

>> Maybe it would be worth to add ->tf_{inb,outb} to struct ide_tp_ops
>> and convert default tp_ops to use them...  OTOH we should reinvestigate
>> the io{read,write}*() way first (maybe things have improved there)...

>    Yes, let's not be hasty here...

    What I certainly don't like is how tf_load/read() handle LBA48: there's 
much of the code duplication going on. I'll think what can be done about it 
but it may not be easy to tackle... it looks like 'struct ide_taskfile' needs 
to be reorganized to include 2 6-byte sub-structures.

>> Thanks,
>> Bart

MBR, Sergei