[PATCH] (pata-2.6 fix queue) sl82c105: rework PIO support

[PATCH] (pata-2.6 fix queue) sl82c105: rework PIO support

[Sergei Shtylyov]

Get rid of the 'pio_speed' member of 'ide_drive_t' that was only used by this
driver by storing the PIO mode timings in the 'drive_data' instead -- this
allows us to greatly simplify the process  of "reloading" of the chip's timing
register and do in right in the ide_dma_off_quietly() and to turn the former
config_for_pio() into the tuneproc() method.

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

---
This patch has also been actually tested at last. :-)

 drivers/ide/pci/sl82c105.c |   90 +++++++++++++++++----------------------------
 include/linux/ide.h        |    1 
 2 files changed, 34 insertions(+), 57 deletions(-)

Index: linux-2.6/drivers/ide/pci/sl82c105.c
===================================================================
--- linux-2.6.orig/drivers/ide/pci/sl82c105.c
+++ linux-2.6/drivers/ide/pci/sl82c105.c
@@ -77,42 +77,43 @@ static unsigned int get_timing_sl82c105(
 /*
  * Configure the drive and chipset for PIO
  */
-static void config_for_pio(ide_drive_t *drive, int pio, int report, int chipset_only)
+static void sl82c105_tune_drive(ide_drive_t *drive, u8 pio)
 {
-	ide_hwif_t *hwif = HWIF(drive);
-	struct pci_dev *dev = hwif->pci_dev;
+	ide_hwif_t *hwif	= HWIF(drive);
+	struct pci_dev *dev	= hwif->pci_dev;
+	int reg			= 0x44 + drive->dn * 4;
 	ide_pio_data_t p;
-	u16 drv_ctrl = 0x909;
-	unsigned int xfer_mode, reg;
+	u16 drv_ctrl;
+	u8  xfer_mode;
 
-	DBG(("config_for_pio(drive:%s, pio:%d, report:%d, chipset_only:%d)\n",
-		drive->name, pio, report, chipset_only));
+	DBG(("sl82c105_tune_drive(drive:%s, pio:%u)\n", drive->name, pio));
 		
-	reg = (hwif->channel ? 0x4c : 0x44) + (drive->select.b.unit ? 4 : 0);
-
-	pio = ide_get_best_pio_mode(drive, pio, 5, &p);
+	xfer_mode = ide_get_best_pio_mode(drive, pio, 5, &p) + XFER_PIO_0;
 
-	xfer_mode = XFER_PIO_0 + pio;
+	if (ide_config_drive_speed(drive, xfer_mode))
+		return;
 
-	if (chipset_only || ide_config_drive_speed(drive, xfer_mode) == 0) {
-		drv_ctrl = get_timing_sl82c105(&p);
-		drive->pio_speed = xfer_mode;
-	} else
-		drive->pio_speed = XFER_PIO_0;
+	drive->drive_data = drv_ctrl = get_timing_sl82c105(&p);
 
-	if (drive->using_dma == 0) {
+	if (!drive->using_dma) {
 		/*
 		 * If we are actually using MW DMA, then we can not
 		 * reprogram the interface drive control register.
 		 */
-		pci_write_config_word(dev, reg, drv_ctrl);
-		pci_read_config_word(dev, reg, &drv_ctrl);
+		pci_write_config_word(dev, reg,  drv_ctrl);
+		pci_read_config_word (dev, reg, &drv_ctrl);
 
-		if (report) {
-			printk("%s: selected %s (%dns) (%04X)\n", drive->name,
-			       ide_xfer_verbose(xfer_mode), p.cycle_time, drv_ctrl);
-		}
 	}
+
+	printk(KERN_DEBUG "%s: selected %s (%dns) (%04X)\n", drive->name,
+	       ide_xfer_verbose(xfer_mode), p.cycle_time, drv_ctrl);
+
+	/*
+	 * We support 32-bit I/O on this interface, and it
+	 * doesn't have problems with interrupts.
+	 */
+	drive->io_32bit = 1;
+	drive->unmask	= 1;
 }
 
 /*
@@ -248,17 +249,14 @@ static int sl82c105_ide_dma_on (ide_driv
 
 static int sl82c105_ide_dma_off_quietly (ide_drive_t *drive)
 {
-	u8 speed = XFER_PIO_0;
-	int rc;
-	
+	struct pci_dev *dev	= HWIF(drive)->pci_dev;
+	int reg 		= 0x44 + drive->dn * 4;
+
 	DBG(("sl82c105_ide_dma_off_quietly(drive:%s)\n", drive->name));
 
-	rc = __ide_dma_off_quietly(drive);
-	if (drive->pio_speed)
-		speed = drive->pio_speed - XFER_PIO_0;
-	config_for_pio(drive, speed, 0, 1);
+	pci_write_config_word(dev, reg, drive->drive_data);
 
-	return rc;
+	return __ide_dma_off_quietly(drive);
 }
 
 /*
@@ -304,24 +302,6 @@ static void sl82c105_resetproc(ide_drive
 }
 	
 /*
- * We only deal with PIO mode here - DMA mode 'using_dma' is not
- * initialised at the point that this function is called.
- */
-static void tune_sl82c105(ide_drive_t *drive, u8 pio)
-{
-	DBG(("tune_sl82c105(drive:%s)\n", drive->name));
-
-	config_for_pio(drive, pio, 1, 0);
-
-	/*
-	 * We support 32-bit I/O on this interface, and it
-	 * doesn't have problems with interrupts.
-	 */
-	drive->io_32bit = 1;
-	drive->unmask = 1;
-}
-
-/*
  * Return the revision of the Winbond bridge
  * which this function is part of.
  */
@@ -385,19 +365,17 @@ static void __devinit init_hwif_sl82c105
 
 	DBG(("init_hwif_sl82c105(hwif: ide%d)\n", hwif->index));
 
-	hwif->tuneproc = tune_sl82c105;
-	hwif->selectproc = sl82c105_selectproc;
-	hwif->resetproc = sl82c105_resetproc;
+	hwif->tuneproc		= sl82c105_tune_drive;
+	hwif->selectproc	= sl82c105_selectproc;
+	hwif->resetproc 	= sl82c105_resetproc;
 
 	/*
 	 * Default to PIO 0 for fallback unless tuned otherwise.
 	 * We always autotune PIO,  this is done before DMA is checked,
 	 * so there's no risk of accidentally disabling DMA
 	 */
-	hwif->drives[0].pio_speed = XFER_PIO_0;
-	hwif->drives[0].autotune = 1;
-	hwif->drives[1].pio_speed = XFER_PIO_0;
-	hwif->drives[1].autotune = 1;
+	hwif->drives[0].drive_data = hwif->drives[1].drive_data = 0x0909;
+	hwif->drives[0].autotune   = hwif->drives[1].autotune   = 1;
 
 	if (!hwif->dma_base)
 		return;
Index: linux-2.6/include/linux/ide.h
===================================================================
--- linux-2.6.orig/include/linux/ide.h
+++ linux-2.6/include/linux/ide.h
@@ -613,7 +613,6 @@ typedef struct ide_drive_s {
 
         u8	quirk_list;	/* considered quirky, set for a specific host */
         u8	init_speed;	/* transfer rate set at boot */
-        u8	pio_speed;      /* unused by core, used by some drivers for fallback from DMA */
         u8	current_speed;	/* current transfer rate set */
         u8	dn;		/* now wide spread use */
         u8	wcache;		/* status of write cache */

Re: [PATCH] (pata-2.6 fix queue) sl82c105: rework PIO support

[Bartlomiej Zolnierkiewicz]

On Tuesday 13 February 2007 23:01, Sergei Shtylyov wrote:
> Get rid of the 'pio_speed' member of 'ide_drive_t' that was only used by this
> driver by storing the PIO mode timings in the 'drive_data' instead -- this
> allows us to greatly simplify the process  of "reloading" of the chip's timing
> register and do in right in the ide_dma_off_quietly() and to turn the former
> config_for_pio() into the tuneproc() method.
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> ---
> This patch has also been actually tested at last. :-)

applied

[PATCH pata-2.6] sl82c105: rework PIO support (take 2)

[Sergei Shtylyov]

Get rid of the 'pio_speed' member of 'ide_drive_t' that was only used by this
driver by storing the PIO mode timings in the 'drive_data' instead -- this
allows us to greatly  simplify the process of "reloading" of the chip's timing
register and do it right in sl82c150_dma_off_quietly() and to get rid of two
extra arguments to config_for_pio() -- which got renamed to sl82c105_tune_pio()
and now returns a PIO mode selected, with ide_config_drive_speed() call moved
into the tuneproc() method, now called sl82c105_tune_drive() with the code to
set drive's 'io_32bit' and 'unmask' flags in its turn moved to its proper place
in the init_hwif() method.
Also, while at it, rename get_timing_sl82c105() into get_pio_timings() and get
rid of the code in it clamping cycle counts to 32 which was both incorrect and
never executed anyway...

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
The patch has been actually tested at last. :-)

 drivers/ide/pci/sl82c105.c |   99 ++++++++++++++++++---------------------------
 include/linux/ide.h        |    1 
 2 files changed, 41 insertions(+), 59 deletions(-)

Index: linux-2.6/drivers/ide/pci/sl82c105.c
===================================================================
--- linux-2.6.orig/drivers/ide/pci/sl82c105.c
+++ linux-2.6/drivers/ide/pci/sl82c105.c
@@ -11,6 +11,8 @@
  * Merge in Russell's HW workarounds, fix various problems
  * with the timing registers setup.
  *  -- Benjamin Herrenschmidt (01/11/03) benh@kernel.crashing.org
+ *
+ * Copyright (C) 2006-2007 MontaVista Software, Inc. <source@mvista.com>
  */
 
 #include <linux/types.h>
@@ -47,25 +49,19 @@
 #define CTRL_P0EN       (1 << 0)
 
 /*
- * Convert a PIO mode and cycle time to the required on/off
- * times for the interface.  This has protection against run-away
- * timings.
+ * Convert a PIO mode and cycle time to the required on/off times
+ * for the interface.  This has protection against runaway timings.
  */
-static unsigned int get_timing_sl82c105(ide_pio_data_t *p)
+static unsigned int get_pio_timings(ide_pio_data_t *p)
 {
-	unsigned int cmd_on;
-	unsigned int cmd_off;
+	unsigned int cmd_on, cmd_off;
 
-	cmd_on = (ide_pio_timings[p->pio_mode].active_time + 29) / 30;
+	cmd_on  = (ide_pio_timings[p->pio_mode].active_time + 29) / 30;
 	cmd_off = (p->cycle_time - 30 * cmd_on + 29) / 30;
 
-	if (cmd_on > 32)
-		cmd_on = 32;
 	if (cmd_on == 0)
 		cmd_on = 1;
 
-	if (cmd_off > 32)
-		cmd_off = 32;
 	if (cmd_off == 0)
 		cmd_off = 1;
 
@@ -73,44 +69,34 @@ static unsigned int get_timing_sl82c105(
 }
 
 /*
- * Configure the drive and chipset for PIO
+ * Configure the chipset for PIO mode.
  */
-static void config_for_pio(ide_drive_t *drive, int pio, int report, int chipset_only)
+static u8 sl82c105_tune_pio(ide_drive_t *drive, u8 pio)
 {
-	ide_hwif_t *hwif = HWIF(drive);
-	struct pci_dev *dev = hwif->pci_dev;
+	struct pci_dev *dev	= HWIF(drive)->pci_dev;
+	int reg			= 0x44 + drive->dn * 4;
 	ide_pio_data_t p;
-	u16 drv_ctrl = 0x909;
-	unsigned int xfer_mode, reg;
+	u16 drv_ctrl;
 
-	DBG(("config_for_pio(drive:%s, pio:%d, report:%d, chipset_only:%d)\n",
-		drive->name, pio, report, chipset_only));
-		
-	reg = (hwif->channel ? 0x4c : 0x44) + (drive->select.b.unit ? 4 : 0);
+	DBG(("sl82c105_tune_pio(drive:%s, pio:%u)\n", drive->name, pio));
 
 	pio = ide_get_best_pio_mode(drive, pio, 5, &p);
 
-	xfer_mode = XFER_PIO_0 + pio;
+	drive->drive_data = drv_ctrl = get_pio_timings(&p);
 
-	if (chipset_only || ide_config_drive_speed(drive, xfer_mode) == 0) {
-		drv_ctrl = get_timing_sl82c105(&p);
-		drive->pio_speed = xfer_mode;
-	} else
-		drive->pio_speed = XFER_PIO_0;
-
-	if (drive->using_dma == 0) {
+	if (!drive->using_dma) {
 		/*
 		 * If we are actually using MW DMA, then we can not
 		 * reprogram the interface drive control register.
 		 */
-		pci_write_config_word(dev, reg, drv_ctrl);
-		pci_read_config_word(dev, reg, &drv_ctrl);
-
-		if (report) {
-			printk("%s: selected %s (%dns) (%04X)\n", drive->name,
-			       ide_xfer_verbose(xfer_mode), p.cycle_time, drv_ctrl);
-		}
+		pci_write_config_word(dev, reg,  drv_ctrl);
+		pci_read_config_word (dev, reg, &drv_ctrl);
 	}
+
+	printk(KERN_DEBUG "%s: selected %s (%dns) (%04X)\n", drive->name,
+	       ide_xfer_verbose(pio + XFER_PIO_0), p.cycle_time, drv_ctrl);
+
+	return pio;
 }
 
 /*
@@ -267,14 +253,14 @@ static int sl82c105_ide_dma_on (ide_driv
 
 static void sl82c105_dma_off_quietly(ide_drive_t *drive)
 {
-	u8 speed = XFER_PIO_0;
+	struct pci_dev *dev	= HWIF(drive)->pci_dev;
+	int reg 		= 0x44 + drive->dn * 4;
 
 	DBG(("sl82c105_dma_off_quietly(drive:%s)\n", drive->name));
 
+	pci_write_config_word(dev, reg, drive->drive_data);
+
 	ide_dma_off_quietly(drive);
-	if (drive->pio_speed)
-		speed = drive->pio_speed - XFER_PIO_0;
-	config_for_pio(drive, speed, 0, 1);
 }
 
 /*
@@ -323,18 +309,12 @@ static void sl82c105_resetproc(ide_drive
  * We only deal with PIO mode here - DMA mode 'using_dma' is not
  * initialised at the point that this function is called.
  */
-static void tune_sl82c105(ide_drive_t *drive, u8 pio)
+static void sl82c105_tune_drive(ide_drive_t *drive, u8 pio)
 {
-	DBG(("tune_sl82c105(drive:%s)\n", drive->name));
+	DBG(("sl82c105_tune_drive(drive:%s, pio:%u)\n", drive->name, pio));
 
-	config_for_pio(drive, pio, 1, 0);
-
-	/*
-	 * We support 32-bit I/O on this interface, and it
-	 * doesn't have problems with interrupts.
-	 */
-	drive->io_32bit = 1;
-	drive->unmask = 1;
+	pio = sl82c105_tune_pio(drive, pio);
+	(void) ide_config_drive_speed(drive, XFER_PIO_0 + pio);
 }
 
 /*
@@ -401,19 +381,22 @@ static void __devinit init_hwif_sl82c105
 
 	DBG(("init_hwif_sl82c105(hwif: ide%d)\n", hwif->index));
 
-	hwif->tuneproc = tune_sl82c105;
-	hwif->selectproc = sl82c105_selectproc;
-	hwif->resetproc = sl82c105_resetproc;
+	hwif->tuneproc		= &sl82c105_tune_drive;
+	hwif->selectproc	= &sl82c105_selectproc;
+	hwif->resetproc 	= &sl82c105_resetproc;
+
+	/*
+	 * We support 32-bit I/O on this interface, and
+	 * it doesn't have problems with interrupts.
+	 */
+	hwif->drives[0].io_32bit = hwif->drives[1].io_32bit = 1;
+	hwif->drives[0].unmask   = hwif->drives[1].unmask   = 1;
 
 	/*
-	 * Default to PIO 0 for fallback unless tuned otherwise.
 	 * We always autotune PIO,  this is done before DMA is checked,
 	 * so there's no risk of accidentally disabling DMA
 	 */
-	hwif->drives[0].pio_speed = XFER_PIO_0;
-	hwif->drives[0].autotune = 1;
-	hwif->drives[1].pio_speed = XFER_PIO_0;
-	hwif->drives[1].autotune = 1;
+	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
 	hwif->atapi_dma = 0;
 	hwif->mwdma_mask = 0;
Index: linux-2.6/include/linux/ide.h
===================================================================
--- linux-2.6.orig/include/linux/ide.h
+++ linux-2.6/include/linux/ide.h
@@ -613,7 +613,6 @@ typedef struct ide_drive_s {
 
         u8	quirk_list;	/* considered quirky, set for a specific host */
         u8	init_speed;	/* transfer rate set at boot */
-        u8	pio_speed;      /* unused by core, used by some drivers for fallback from DMA */
         u8	current_speed;	/* current transfer rate set */
         u8	dn;		/* now wide spread use */
         u8	wcache;		/* status of write cache */

Re: [PATCH pata-2.6] sl82c105: rework PIO support (take 2)

[Sergei Shtylyov]

Hello, I wrote:

> Index: linux-2.6/include/linux/ide.h
> ===================================================================
> --- linux-2.6.orig/include/linux/ide.h
> +++ linux-2.6/include/linux/ide.h
> @@ -613,7 +613,6 @@ typedef struct ide_drive_s {
>  
>          u8	quirk_list;	/* considered quirky, set for a specific host */
>          u8	init_speed;	/* transfer rate set at boot */
> -        u8	pio_speed;      /* unused by core, used by some drivers for fallback from DMA */
>          u8	current_speed;	/* current transfer rate set */
>          u8	dn;		/* now wide spread use */
>          u8	wcache;		/* status of write cache */

    Oops, forgot to pull the recent kernel before sending out, so this part 
won't apply bacause of the desired_speed nuisance. Do I need to resend?

MBR, Sergei

Re: [PATCH pata-2.6] sl82c105: rework PIO support (take 2)

[Bartlomiej Zolnierkiewicz]

On Wednesday 28 March 2007, Sergei Shtylyov wrote:
> Hello, I wrote:
> 
> > Index: linux-2.6/include/linux/ide.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/ide.h
> > +++ linux-2.6/include/linux/ide.h
> > @@ -613,7 +613,6 @@ typedef struct ide_drive_s {
> >  
> >          u8	quirk_list;	/* considered quirky, set for a specific host */
> >          u8	init_speed;	/* transfer rate set at boot */
> > -        u8	pio_speed;      /* unused by core, used by some drivers for fallback from DMA */
> >          u8	current_speed;	/* current transfer rate set */
> >          u8	dn;		/* now wide spread use */
> >          u8	wcache;		/* status of write cache */
> 
>     Oops, forgot to pull the recent kernel before sending out, so this part 
> won't apply bacause of the desired_speed nuisance. Do I need to resend?

I was mentally prepared to fix it manually to save time on both sides but...

Hunk #1 succeeded at 613 with fuzz 2.

;)

Re: [PATCH pata-2.6] sl82c105: rework PIO support (take 2)

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>Index: linux-2.6/include/linux/ide.h
>>>===================================================================
>>>--- linux-2.6.orig/include/linux/ide.h
>>>+++ linux-2.6/include/linux/ide.h
>>>@@ -613,7 +613,6 @@ typedef struct ide_drive_s {
>>> 
>>>         u8	quirk_list;	/* considered quirky, set for a specific host */
>>>         u8	init_speed;	/* transfer rate set at boot */
>>>-        u8	pio_speed;      /* unused by core, used by some drivers for fallback from DMA */
>>>         u8	current_speed;	/* current transfer rate set */
>>>         u8	dn;		/* now wide spread use */
>>>         u8	wcache;		/* status of write cache */
>>
>>    Oops, forgot to pull the recent kernel before sending out, so this part 
>>won't apply bacause of the desired_speed nuisance. Do I need to resend?

> I was mentally prepared to fix it manually to save time on both sides but...

> Hunk #1 succeeded at 613 with fuzz 2.

    Probably you're using quilt newer that 0.42 then? :-)

> ;)

MBR, Sergei

Re: [PATCH pata-2.6] sl82c105: rework PIO support (take 2)

[Bartlomiej Zolnierkiewicz]

On Wednesday 28 March 2007, Sergei Shtylyov wrote:
> Get rid of the 'pio_speed' member of 'ide_drive_t' that was only used by this
> driver by storing the PIO mode timings in the 'drive_data' instead -- this
> allows us to greatly  simplify the process of "reloading" of the chip's timing
> register and do it right in sl82c150_dma_off_quietly() and to get rid of two
> extra arguments to config_for_pio() -- which got renamed to sl82c105_tune_pio()
> and now returns a PIO mode selected, with ide_config_drive_speed() call moved
> into the tuneproc() method, now called sl82c105_tune_drive() with the code to
> set drive's 'io_32bit' and 'unmask' flags in its turn moved to its proper place
> in the init_hwif() method.
> Also, while at it, rename get_timing_sl82c105() into get_pio_timings() and get
> rid of the code in it clamping cycle counts to 32 which was both incorrect and
> never executed anyway...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

applied

[PATCH] sl82c105: add printk() logging facility

[Sergei Shtylyov]

Add missing printk() logging facility in sl82c105_dma_lost_irq().

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

---
The patch is atop of the ide-2.6.git 'for-next' branch...

 drivers/ide/sl82c105.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

Index: ide-2.6/drivers/ide/sl82c105.c
===================================================================
--- ide-2.6.orig/drivers/ide/sl82c105.c
+++ ide-2.6/drivers/ide/sl82c105.c
@@ -10,7 +10,7 @@
  * with the timing registers setup.
  *  -- Benjamin Herrenschmidt (01/11/03) benh@kernel.crashing.org
  *
- * Copyright (C) 2006-2007 MontaVista Software, Inc. <source@mvista.com>
+ * Copyright (C) 2006-2007,2009 MontaVista Software, Inc. <source@mvista.com>
  * Copyright (C)      2007 Bartlomiej Zolnierkiewicz
  */
 
@@ -146,14 +146,15 @@ static void sl82c105_dma_lost_irq(ide_dr
 	u32 val, mask		= hwif->channel ? CTRL_IDE_IRQB : CTRL_IDE_IRQA;
 	u8 dma_cmd;
 
-	printk("sl82c105: lost IRQ, resetting host\n");
+	printk(KERN_WARNING "sl82c105: lost IRQ, resetting host\n");
 
 	/*
 	 * Check the raw interrupt from the drive.
 	 */
 	pci_read_config_dword(dev, 0x40, &val);
 	if (val & mask)
-		printk("sl82c105: drive was requesting IRQ, but host lost it\n");
+		printk(KERN_INFO "sl82c105: drive was requesting IRQ, "
+		       "but host lost it\n");
 
 	/*
 	 * Was DMA enabled?  If so, disable it - we're resetting the
@@ -162,7 +163,7 @@ static void sl82c105_dma_lost_irq(ide_dr
 	dma_cmd = inb(hwif->dma_base + ATA_DMA_CMD);
 	if (dma_cmd & 1) {
 		outb(dma_cmd & ~1, hwif->dma_base + ATA_DMA_CMD);
-		printk("sl82c105: DMA was enabled\n");
+		printk(KERN_INFO "sl82c105: DMA was enabled\n");
 	}
 
 	sl82c105_reset_host(dev);

Re: [PATCH] sl82c105: add printk() logging facility

[Bartlomiej Zolnierkiewicz]

On Monday 08 June 2009 19:46:19 Sergei Shtylyov wrote:
> Add missing printk() logging facility in sl82c105_dma_lost_irq().
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

applied

[PATCH 1/9] cmd64x: implement clear_irq() method

[Sergei Shtylyov]

Convert the driver's two dma_end() methods into clear_irq() methods -- the
driver will now use the standard dma_end() method implementation, ide_dma_end().

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/cmd64x.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

Index: ide-2.6/drivers/ide/cmd64x.c
===================================================================
--- ide-2.6.orig/drivers/ide/cmd64x.c
+++ ide-2.6/drivers/ide/cmd64x.c
@@ -7,7 +7,7 @@
  * Copyright (C) 1998		David S. Miller (davem@redhat.com)
  *
  * Copyright (C) 1999-2002	Andre Hedrick <andre@linux-ide.org>
- * Copyright (C) 2007		MontaVista Software, Inc. <source@mvista.com>
+ * Copyright (C) 2007,2009	MontaVista Software, Inc. <source@mvista.com>
  */
 
 #include <linux/module.h>
@@ -226,11 +226,10 @@ static void cmd64x_set_dma_mode(ide_driv
 		(void) pci_write_config_byte(dev, pciU, regU);
 }
 
-static int cmd648_dma_end(ide_drive_t *drive)
+static void cmd648_clear_irq(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= drive->hwif;
 	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
-	int err			= ide_dma_end(drive);
 	u8  irq_mask		= hwif->channel ? MRDMODE_INTR_CH1 :
 						  MRDMODE_INTR_CH0;
 	u8  mrdmode		= inb(base + 1);
@@ -238,11 +237,9 @@ static int cmd648_dma_end(ide_drive_t *d
 	/* clear the interrupt bit */
 	outb((mrdmode & ~(MRDMODE_INTR_CH0 | MRDMODE_INTR_CH1)) | irq_mask,
 	     base + 1);
-
-	return err;
 }
 
-static int cmd64x_dma_end(ide_drive_t *drive)
+static void cmd64x_clear_irq(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= drive->hwif;
 	struct pci_dev *dev	= to_pci_dev(hwif->dev);
@@ -250,13 +247,10 @@ static int cmd64x_dma_end(ide_drive_t *d
 	u8  irq_mask		= hwif->channel ? ARTTIM23_INTR_CH1 :
 						  CFR_INTR_CH0;
 	u8  irq_stat		= 0;
-	int err			= ide_dma_end(drive);
 
 	(void) pci_read_config_byte(dev, irq_reg, &irq_stat);
 	/* clear the interrupt bit */
 	(void) pci_write_config_byte(dev, irq_reg, irq_stat | irq_mask);
-
-	return err;
 }
 
 static int cmd648_dma_test_irq(ide_drive_t *drive)
@@ -370,6 +364,14 @@ static u8 cmd64x_cable_detect(ide_hwif_t
 static const struct ide_port_ops cmd64x_port_ops = {
 	.set_pio_mode		= cmd64x_set_pio_mode,
 	.set_dma_mode		= cmd64x_set_dma_mode,
+	.clear_irq		= cmd64x_clear_irq,
+	.cable_detect		= cmd64x_cable_detect,
+};
+
+static const struct ide_port_ops cmd648_port_ops = {
+	.set_pio_mode		= cmd64x_set_pio_mode,
+	.set_dma_mode		= cmd64x_set_dma_mode,
+	.clear_irq		= cmd648_clear_irq,
 	.cable_detect		= cmd64x_cable_detect,
 };
 
@@ -377,7 +379,7 @@ static const struct ide_dma_ops cmd64x_d
 	.dma_host_set		= ide_dma_host_set,
 	.dma_setup		= ide_dma_setup,
 	.dma_start		= ide_dma_start,
-	.dma_end		= cmd64x_dma_end,
+	.dma_end		= ide_dma_end,
 	.dma_test_irq		= cmd64x_dma_test_irq,
 	.dma_lost_irq		= ide_dma_lost_irq,
 	.dma_timer_expiry	= ide_dma_sff_timer_expiry,
@@ -399,7 +401,7 @@ static const struct ide_dma_ops cmd648_d
 	.dma_host_set		= ide_dma_host_set,
 	.dma_setup		= ide_dma_setup,
 	.dma_start		= ide_dma_start,
-	.dma_end		= cmd648_dma_end,
+	.dma_end		= ide_dma_end,
 	.dma_test_irq		= cmd648_dma_test_irq,
 	.dma_lost_irq		= ide_dma_lost_irq,
 	.dma_timer_expiry	= ide_dma_sff_timer_expiry,
@@ -423,7 +425,7 @@ static const struct ide_port_info cmd64x
 		.name		= DRV_NAME,
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
-		.port_ops	= &cmd64x_port_ops,
+		.port_ops	= &cmd648_port_ops,
 		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_SERIALIZE |
 				  IDE_HFLAG_ABUSE_PREFETCH,
@@ -435,7 +437,7 @@ static const struct ide_port_info cmd64x
 		.name		= DRV_NAME,
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
-		.port_ops	= &cmd64x_port_ops,
+		.port_ops	= &cmd648_port_ops,
 		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
@@ -446,7 +448,7 @@ static const struct ide_port_info cmd64x
 		.name		= DRV_NAME,
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
-		.port_ops	= &cmd64x_port_ops,
+		.port_ops	= &cmd648_port_ops,
 		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
@@ -484,6 +486,7 @@ static int __devinit cmd64x_init_one(str
 			 */
 			if (dev->revision < 3) {
 				d.enablebits[0].reg = 0;
+				d.port_ops = &cmd64x_port_ops;
 				if (dev->revision == 1)
 					d.dma_ops = &cmd646_rev1_dma_ops;
 				else

Re: [PATCH 1/9] cmd64x: implement clear_irq() method

[Bartlomiej Zolnierkiewicz]

Hi Sergei,

This is a very nice series and I would like to apply it immediately
but unfortunately there is quite a few issues to be addressed first..

On Wednesday 10 June 2009 20:44:45 Sergei Shtylyov wrote:
> Convert the driver's two dma_end() methods into clear_irq() methods -- the
> driver will now use the standard dma_end() method implementation, ide_dma_end().
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> ---
> The patch is atop of ide-2.6.git 'for-next' branch.
> 
>  drivers/ide/cmd64x.c |   31 +++++++++++++++++--------------
>  1 files changed, 17 insertions(+), 14 deletions(-)

[...]

> @@ -226,11 +226,10 @@ static void cmd64x_set_dma_mode(ide_driv
>  		(void) pci_write_config_byte(dev, pciU, regU);
>  }
>  
> -static int cmd648_dma_end(ide_drive_t *drive)
> +static void cmd648_clear_irq(ide_drive_t *drive)
>  {
>  	ide_hwif_t *hwif	= drive->hwif;
>  	unsigned long base	= hwif->dma_base - (hwif->channel * 8);

Don't we need to check whether hwif->dma_base is valid now?

Also shouldn't this patch be done after fixing core code first?
[IOW this should be patch #5.5]

Re: [PATCH 1/9] cmd64x: implement clear_irq() method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

> Hi Sergei,
>
> This is a very nice series 

   Oh, I'm flattened. :-)

> and I would like to apply it immediately
> but unfortunately there is quite a few issues to be addressed first..
>   

   Sigh, it's a pity -- I'm now on vacation and won't be available since 
this Sunday... I should have posted it earlier of course. Will have to 
try to address your comments tomorrow then.

>> Convert the driver's two dma_end() methods into clear_irq() methods -- the
>> driver will now use the standard dma_end() method implementation, ide_dma_end().
>>
>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>
>> ---
>> The patch is atop of ide-2.6.git 'for-next' branch.
>>
>>  drivers/ide/cmd64x.c |   31 +++++++++++++++++--------------
>>  1 files changed, 17 insertions(+), 14 deletions(-)
>>     
>
> [...]
>   
>> @@ -226,11 +226,10 @@ static void cmd64x_set_dma_mode(ide_driv
>>  		(void) pci_write_config_byte(dev, pciU, regU);
>>  }
>>  
>> -static int cmd648_dma_end(ide_drive_t *drive)
>> +static void cmd648_clear_irq(ide_drive_t *drive)
>>  {
>>  	ide_hwif_t *hwif	= drive->hwif;
>>  	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
>>     
>
> Don't we need to check whether hwif->dma_base is valid now?
>   

   You're right, I have managed to overlook this. I'll change this to 
pci_resource_start() call instead...

> Also shouldn't this patch be done after fixing core code first?
>   

   Fixing what exactly, ide_timer_expiry()?

> [IOW this should be patch #5.5]
>   

   I don't see how this is connected to the fix but I can reorder if 
you'd prefer it this way.

MBR, Sergei

Re: [PATCH 1/9] cmd64x: implement clear_irq() method

[Bartlomiej Zolnierkiewicz]

On Friday 12 June 2009 20:39:51 Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> > Hi Sergei,
> >
> > This is a very nice series 
> 
>    Oh, I'm flattened. :-)
> 
> > and I would like to apply it immediately
> > but unfortunately there is quite a few issues to be addressed first..
> >   
> 
>    Sigh, it's a pity -- I'm now on vacation and won't be available since 
> this Sunday... I should have posted it earlier of course. Will have to 
> try to address your comments tomorrow then.
> 
> >> Convert the driver's two dma_end() methods into clear_irq() methods -- the
> >> driver will now use the standard dma_end() method implementation, ide_dma_end().
> >>
> >> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> >>
> >> ---
> >> The patch is atop of ide-2.6.git 'for-next' branch.
> >>
> >>  drivers/ide/cmd64x.c |   31 +++++++++++++++++--------------
> >>  1 files changed, 17 insertions(+), 14 deletions(-)
> >>     
> >
> > [...]
> >   
> >> @@ -226,11 +226,10 @@ static void cmd64x_set_dma_mode(ide_driv
> >>  		(void) pci_write_config_byte(dev, pciU, regU);
> >>  }
> >>  
> >> -static int cmd648_dma_end(ide_drive_t *drive)
> >> +static void cmd648_clear_irq(ide_drive_t *drive)
> >>  {
> >>  	ide_hwif_t *hwif	= drive->hwif;
> >>  	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
> >>     
> >
> > Don't we need to check whether hwif->dma_base is valid now?
> >   
> 
>    You're right, I have managed to overlook this. I'll change this to 
> pci_resource_start() call instead...

Currently this driver should operate fine without BAR4 set so even if
this is pci_resource_start(), the return value still needs to be checked
against 0 -- it is the reliability/maintainability issue.

> > Also shouldn't this patch be done after fixing core code first?
> >   
> 
>    Fixing what exactly, ide_timer_expiry()?

Yes.  Currently IRQ is cleared in ide_timer_expiry()'s code-path:

 -> hwif->handler [ ide_dma_intr() ]
     -> dma_ops->dma_end [ cmd64{x,8}_dma_end() ]

After this patch it won't be.

Re: [PATCH 1/9] cmd64x: implement clear_irq() method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>> Convert the driver's two dma_end() methods into clear_irq() methods -- the
>>>> driver will now use the standard dma_end() method implementation, ide_dma_end().
>>>>
>>>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>>>
>>>> ---
>>>> The patch is atop of ide-2.6.git 'for-next' branch.
>>>>
>>>>  drivers/ide/cmd64x.c |   31 +++++++++++++++++--------------
>>>>  1 files changed, 17 insertions(+), 14 deletions(-)
>>>>     
>>>>         
>>> [...]
>>>   
>>>       
>>>> @@ -226,11 +226,10 @@ static void cmd64x_set_dma_mode(ide_driv
>>>>  		(void) pci_write_config_byte(dev, pciU, regU);
>>>>  }
>>>>  
>>>> -static int cmd648_dma_end(ide_drive_t *drive)
>>>> +static void cmd648_clear_irq(ide_drive_t *drive)
>>>>  {
>>>>  	ide_hwif_t *hwif	= drive->hwif;
>>>>  	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
>>>>     
>>>>         
>>> Don't we need to check whether hwif->dma_base is valid now?
>>>   
>>>       
>>    You're right, I have managed to overlook this. I'll change this to 
>> pci_resource_start() call instead...
>>     
>
> Currently this driver should operate fine without BAR4 set so even if
> this is pci_resource_start(), the return value still needs to be checked
> against 0 -- it is the reliability/maintainability issue.
>   

   Nay, the PCI device just must not be enabled in this case: all BARs 
must be allocated, period -- that's what the PCI specs say, IIRC. 
Besides, 0 doesn't generally mean "unassigned" in the PCI world.
I know, I know, we consider it unallocated but that's not correct -- 
only PCI 2.2 (IIRC) used to have words about 0 meaning "unassigned". 
Note that we don't check pci_resource_start() result for 0 in the 
drivers that do call it, like hpt366.c... I'm seeing the checks in 
pdc202xx_*.c but they seem pretty useless -- this just must not happen. 
So, I don't agree here...

>>> Also shouldn't this patch be done after fixing core code first?  
>>>       
>>    Fixing what exactly, ide_timer_expiry()?
>>     
>
> Yes.  Currently IRQ is cleared in ide_timer_expiry()'s code-path:
>
>  -> hwif->handler [ ide_dma_intr() ]
>      -> dma_ops->dma_end [ cmd64{x,8}_dma_end() ]
>
> After this patch it won't be.
>   

   Ah, that's a correct spotting...

MBR, Sergei

Re: [PATCH 1/9] cmd64x: implement clear_irq() method

[Bartlomiej Zolnierkiewicz]

On Friday 12 June 2009 21:13:41 Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> >>>> Convert the driver's two dma_end() methods into clear_irq() methods -- the
> >>>> driver will now use the standard dma_end() method implementation, ide_dma_end().
> >>>>
> >>>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> >>>>
> >>>> ---
> >>>> The patch is atop of ide-2.6.git 'for-next' branch.
> >>>>
> >>>>  drivers/ide/cmd64x.c |   31 +++++++++++++++++--------------
> >>>>  1 files changed, 17 insertions(+), 14 deletions(-)
> >>>>     
> >>>>         
> >>> [...]
> >>>   
> >>>       
> >>>> @@ -226,11 +226,10 @@ static void cmd64x_set_dma_mode(ide_driv
> >>>>  		(void) pci_write_config_byte(dev, pciU, regU);
> >>>>  }
> >>>>  
> >>>> -static int cmd648_dma_end(ide_drive_t *drive)
> >>>> +static void cmd648_clear_irq(ide_drive_t *drive)
> >>>>  {
> >>>>  	ide_hwif_t *hwif	= drive->hwif;
> >>>>  	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
> >>>>     
> >>>>         
> >>> Don't we need to check whether hwif->dma_base is valid now?
> >>>   
> >>>       
> >>    You're right, I have managed to overlook this. I'll change this to 
> >> pci_resource_start() call instead...
> >>     
> >
> > Currently this driver should operate fine without BAR4 set so even if
> > this is pci_resource_start(), the return value still needs to be checked
> > against 0 -- it is the reliability/maintainability issue.
> >   
> 
>    Nay, the PCI device just must not be enabled in this case: all BARs 
> must be allocated, period -- that's what the PCI specs say, IIRC. 
> Besides, 0 doesn't generally mean "unassigned" in the PCI world.
> I know, I know, we consider it unallocated but that's not correct -- 
> only PCI 2.2 (IIRC) used to have words about 0 meaning "unassigned". 

What matters here is our Linux PCI stack implementation not the spec,
and historically it allowed use of not fully setup (because of some
weird/broken BIOSes) PCI devices -- though this may no longer be the
case..

> Note that we don't check pci_resource_start() result for 0 in the 
> drivers that do call it, like hpt366.c... I'm seeing the checks in 
> pdc202xx_*.c but they seem pretty useless -- this just must not happen. 

Care to remove them while we are at it?

Re: [PATCH 1/9] cmd64x: implement clear_irq() method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>>>> Convert the driver's two dma_end() methods into clear_irq() methods -- the
>>>>>> driver will now use the standard dma_end() method implementation, ide_dma_end().
>>>>>>
>>>>>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>>>>>
>>>>>> ---
>>>>>> The patch is atop of ide-2.6.git 'for-next' branch.
>>>>>>
>>>>>>  drivers/ide/cmd64x.c |   31 +++++++++++++++++--------------
>>>>>>  1 files changed, 17 insertions(+), 14 deletions(-)
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> [...]
>>>>>   
>>>>>       
>>>>>           
>>>>>> @@ -226,11 +226,10 @@ static void cmd64x_set_dma_mode(ide_driv
>>>>>>  		(void) pci_write_config_byte(dev, pciU, regU);
>>>>>>  }
>>>>>>  
>>>>>> -static int cmd648_dma_end(ide_drive_t *drive)
>>>>>> +static void cmd648_clear_irq(ide_drive_t *drive)
>>>>>>  {
>>>>>>  	ide_hwif_t *hwif	= drive->hwif;
>>>>>>  	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> Don't we need to check whether hwif->dma_base is valid now?
>>>>>   
>>>>>       
>>>>>           
>>>>    You're right, I have managed to overlook this. I'll change this to 
>>>> pci_resource_start() call instead...
>>>>     
>>>>         
>>> Currently this driver should operate fine without BAR4 set so even if
>>> this is pci_resource_start(), the return value still needs to be checked
>>> against 0 -- it is the reliability/maintainability issue.
>>>   
>>>       
>>    Nay, the PCI device just must not be enabled in this case: all BARs 
>> must be allocated, period -- that's what the PCI specs say, IIRC. 
>> Besides, 0 doesn't generally mean "unassigned" in the PCI world.
>> I know, I know, we consider it unallocated but that's not correct -- 
>> only PCI 2.2 (IIRC) used to have words about 0 meaning "unassigned". 
>>     
>
> What matters here is our Linux PCI stack implementation not the spec,
> and historically it allowed use of not fully setup (because of some
> weird/broken BIOSes) PCI devices -- though this may no longer be the
> case..
>   

  Ah, and I know one such PowerPC platform (doesn't set BAR5 of some 
chip I can't remember right now, perhaps it was SiI 680)...

>> Note that we don't check pci_resource_start() result for 0 in the 
>> drivers that do call it, like hpt366.c... I'm seeing the checks in 
>> pdc202xx_*.c but they seem pretty useless -- this just must not happen. 
>>     
>
> Care to remove them while we are at it?
>   

   No, my time is very limited now and they don't hurt. :-)

MBR, Sergei

[PATCH 2/9] ide: call clear_irq() method in ide_timer_expiry()

[Sergei Shtylyov]

Now the clear_irq() method is called only from ide_intr() but ide_timer_expiry()
also should call this method in case when drive_is_ready() succeeds...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/ide-io.c |    3 +++
 1 files changed, 3 insertions(+)

Index: ide-2.6/drivers/ide/ide-io.c
===================================================================
--- ide-2.6.orig/drivers/ide/ide-io.c
+++ ide-2.6/drivers/ide/ide-io.c
@@ -669,6 +669,9 @@ void ide_timer_expiry (unsigned long dat
 				hwif->dma_ops->dma_lost_irq(drive);
 			if (hwif->ack_intr)
 				hwif->ack_intr(hwif);
+			if (hwif->port_ops && hwif->port_ops->clear_irq)
+				hwif->port_ops->clear_irq(drive);
+
 			printk(KERN_WARNING "%s: lost interrupt\n",
 				drive->name);
 			startstop = handler(drive);

[PATCH 3/9] ide: move IRQ clearing from ack_intr() method to clear_irq() method

[Sergei Shtylyov]

There are now two methods that clear the port interrupt: ack_intr() method,
implemented only on M680x0 machines, that is called at the start of ide_intr(),
and clear_irq() method, that is called somewhat later in this function.  In
order to stop this duplication, delegate the task of clearing the interrupt
to clear_irq() method, only leaving to ack_intr() the task of testing for the
port interrupt.  This involves moving clear_irq() method call in ide_intr()
closer to the beginning of the function and removing ack_intr() method call
in ide_timer_expiry(), now becoming useless...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/gayle.c  |   23 +++++++++++------------
 drivers/ide/ide-io.c |   11 ++++-------
 drivers/ide/macide.c |   18 ++++++++++++++----
 3 files changed, 29 insertions(+), 23 deletions(-)

Index: ide-2.6/drivers/ide/gayle.c
===================================================================
--- ide-2.6.orig/drivers/ide/gayle.c
+++ ide-2.6/drivers/ide/gayle.c
@@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
      *  Check and acknowledge the interrupt status
      */
 
-static int gayle_ack_intr_a4000(ide_hwif_t *hwif)
+static int gayle_ack_intr(ide_hwif_t *hwif)
 {
     unsigned char ch;
 
@@ -76,16 +76,12 @@ static int gayle_ack_intr_a4000(ide_hwif
     return 1;
 }
 
-static int gayle_ack_intr_a1200(ide_hwif_t *hwif)
+static void gayle_a1200_clear_irq(ide_drive_t *drive)
 {
-    unsigned char ch;
+    ide_hwif_t *hwif = drive->hwif;
 
-    ch = z_readb(hwif->io_ports.irq_addr);
-    if (!(ch & GAYLE_IRQ_IDE))
-	return 0;
     (void)z_readb(hwif->io_ports.status_addr);
     z_writeb(0x7c, hwif->io_ports.irq_addr);
-    return 1;
 }
 
 static void __init gayle_setup_ports(struct ide_hw *hw, unsigned long base,
@@ -108,6 +104,10 @@ static void __init gayle_setup_ports(str
 	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops gayle_a1200_port_ops = {
+	.clear_irq		= gayle_a1200_clear_irq,
+};
+
 static const struct ide_port_info gayle_port_info = {
 	.host_flags		= IDE_HFLAG_MMIO | IDE_HFLAG_SERIALIZE |
 				  IDE_HFLAG_NO_DMA,
@@ -123,9 +123,9 @@ static int __init gayle_init(void)
 {
     unsigned long phys_base, res_start, res_n;
     unsigned long base, ctrlport, irqport;
-    ide_ack_intr_t *ack_intr;
     int a4000, i, rc;
     struct ide_hw hw[GAYLE_NUM_HWIFS], *hws[GAYLE_NUM_HWIFS];
+    struct ide_port_info d = gayle_port_info;
 
     if (!MACH_IS_AMIGA)
 	return -ENODEV;
@@ -148,11 +148,10 @@ found:
 	if (a4000) {
 	    phys_base = GAYLE_BASE_4000;
 	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_4000);
-	    ack_intr = gayle_ack_intr_a4000;
 	} else {
 	    phys_base = GAYLE_BASE_1200;
 	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_1200);
-	    ack_intr = gayle_ack_intr_a1200;
+	    d.port_ops = gayle_a1200_port_ops;
 	}
 
 	res_start = ((unsigned long)phys_base) & ~(GAYLE_NEXT_PORT-1);
@@ -165,12 +164,12 @@ found:
 	base = (unsigned long)ZTWO_VADDR(phys_base + i * GAYLE_NEXT_PORT);
 	ctrlport = GAYLE_HAS_CONTROL_REG ? (base + GAYLE_CONTROL) : 0;
 
-	gayle_setup_ports(&hw[i], base, ctrlport, irqport, ack_intr);
+	gayle_setup_ports(&hw[i], base, ctrlport, irqport, gayle_ack_intr);
 
 	hws[i] = &hw[i];
     }
 
-    rc = ide_host_add(&gayle_port_info, hws, i, NULL);
+    rc = ide_host_add(&d, hws, i, NULL);
     if (rc)
 	release_mem_region(res_start, res_n);
 
Index: ide-2.6/drivers/ide/ide-io.c
===================================================================
--- ide-2.6.orig/drivers/ide/ide-io.c
+++ ide-2.6/drivers/ide/ide-io.c
@@ -667,8 +667,6 @@ void ide_timer_expiry (unsigned long dat
 		} else if (drive_is_ready(drive)) {
 			if (drive->waiting_for_dma)
 				hwif->dma_ops->dma_lost_irq(drive);
-			if (hwif->ack_intr)
-				hwif->ack_intr(hwif);
 			if (hwif->port_ops && hwif->port_ops->clear_irq)
 				hwif->port_ops->clear_irq(drive);
 
@@ -791,6 +789,10 @@ irqreturn_t ide_intr (int irq, void *dev
 		goto out;
 
 	handler = hwif->handler;
+	drive	= hwif->cur_dev;
+
+	if (hwif->port_ops && hwif->port_ops->clear_irq)
+		hwif->port_ops->clear_irq(drive);
 
 	if (handler == NULL || hwif->polling) {
 		/*
@@ -821,8 +823,6 @@ irqreturn_t ide_intr (int irq, void *dev
 		goto out;
 	}
 
-	drive = hwif->cur_dev;
-
 	if (!drive_is_ready(drive))
 		/*
 		 * This happens regularly when we share a PCI IRQ with
@@ -839,9 +839,6 @@ irqreturn_t ide_intr (int irq, void *dev
 	del_timer(&hwif->timer);
 	spin_unlock(&hwif->lock);
 
-	if (hwif->port_ops && hwif->port_ops->clear_irq)
-		hwif->port_ops->clear_irq(drive);
-
 	if (drive->dev_flags & IDE_DFLAG_UNMASK)
 		local_irq_enable_in_hardirq();
 
Index: ide-2.6/drivers/ide/macide.c
===================================================================
--- ide-2.6.orig/drivers/ide/macide.c
+++ ide-2.6/drivers/ide/macide.c
@@ -55,13 +55,16 @@ volatile unsigned char *ide_ifr = (unsig
 
 int macide_ack_intr(ide_hwif_t* hwif)
 {
-	if (*ide_ifr & 0x20) {
-		*ide_ifr &= ~0x20;
+	if (*ide_ifr & 0x20)
 		return 1;
-	}
 	return 0;
 }
 
+static int macide_clear_irq(ide_drive_t *drive)
+{
+	*ide_ifr &= ~0x20;
+}
+
 static void __init macide_setup_ports(struct ide_hw *hw, unsigned long base,
 				      int irq, ide_ack_intr_t *ack_intr)
 {
@@ -78,7 +81,12 @@ static void __init macide_setup_ports(st
 	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops macide_port_ops = {
+	.clear_irq		= macide_clear_irq,
+};
+
 static const struct ide_port_info macide_port_info = {
+	.port_ops		= &macide_port_ops,
 	.host_flags		= IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA,
 	.irq_flags		= IRQF_SHARED,
 	.chipset		= ide_generic,
@@ -97,6 +105,7 @@ static int __init macide_init(void)
 	unsigned long base;
 	int irq;
 	struct ide_hw hw, *hws[] = { &hw };
+	struct ide_port_info d = macide_port_info;
 
 	if (!MACH_IS_MAC)
 		return -ENODEV;
@@ -115,6 +124,7 @@ static int __init macide_init(void)
 	case MAC_IDE_BABOON:
 		base = BABOON_BASE;
 		ack_intr = NULL;
+		d.port_ops = NULL;
 		irq = IRQ_BABOON_1;
 		break;
 	default:
@@ -126,7 +136,7 @@ static int __init macide_init(void)
 
 	macide_setup_ports(&hw, base, irq, ack_intr);
 
-	return ide_host_add(&macide_port_info, hws, 1, NULL);
+	return ide_host_add(&d, hws, 1, NULL);
 }
 
 module_init(macide_init);

Re: [PATCH 3/9] ide: move IRQ clearing from ack_intr() method to clear_irq() method

[Bartlomiej Zolnierkiewicz]

On Wednesday 10 June 2009 20:50:02 Sergei Shtylyov wrote:
> There are now two methods that clear the port interrupt: ack_intr() method,
> implemented only on M680x0 machines, that is called at the start of ide_intr(),
> and clear_irq() method, that is called somewhat later in this function.  In
> order to stop this duplication, delegate the task of clearing the interrupt
> to clear_irq() method, only leaving to ack_intr() the task of testing for the
> port interrupt.  This involves moving clear_irq() method call in ide_intr()
> closer to the beginning of the function and removing ack_intr() method call
> in ide_timer_expiry(), now becoming useless...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> ---
> The patch is atop of ide-2.6.git 'for-next' branch.
> 
>  drivers/ide/gayle.c  |   23 +++++++++++------------
>  drivers/ide/ide-io.c |   11 ++++-------
>  drivers/ide/macide.c |   18 ++++++++++++++----
>  3 files changed, 29 insertions(+), 23 deletions(-)
> 
> Index: ide-2.6/drivers/ide/gayle.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/gayle.c
> +++ ide-2.6/drivers/ide/gayle.c
> @@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
>       *  Check and acknowledge the interrupt status
>       */
>  
> -static int gayle_ack_intr_a4000(ide_hwif_t *hwif)
> +static int gayle_ack_intr(ide_hwif_t *hwif)
>  {
>      unsigned char ch;
>  
> @@ -76,16 +76,12 @@ static int gayle_ack_intr_a4000(ide_hwif
>      return 1;
>  }
>  
> -static int gayle_ack_intr_a1200(ide_hwif_t *hwif)
> +static void gayle_a1200_clear_irq(ide_drive_t *drive)
>  {
> -    unsigned char ch;
> +    ide_hwif_t *hwif = drive->hwif;
>  
> -    ch = z_readb(hwif->io_ports.irq_addr);
> -    if (!(ch & GAYLE_IRQ_IDE))
> -	return 0;
>      (void)z_readb(hwif->io_ports.status_addr);
>      z_writeb(0x7c, hwif->io_ports.irq_addr);
> -    return 1;
>  }

buddha.c needs a similar treatment

> --- ide-2.6.orig/drivers/ide/ide-io.c
> +++ ide-2.6/drivers/ide/ide-io.c

> @@ -791,6 +789,10 @@ irqreturn_t ide_intr (int irq, void *dev
>  		goto out;
>  
>  	handler = hwif->handler;
> +	drive	= hwif->cur_dev;
> +
> +	if (hwif->port_ops && hwif->port_ops->clear_irq)
> +		hwif->port_ops->clear_irq(drive);

We need to check for valid ->handler before using ->cur_dev
(it may contain a stale value otherwise).
 
Moreover I somehow miss the point of moving ->clear_irq call here
(it should be done after we know that it is really our IRQ).

>  	if (handler == NULL || hwif->polling) {
>  		/*
> @@ -821,8 +823,6 @@ irqreturn_t ide_intr (int irq, void *dev
>  		goto out;
>  	}
>  
> -	drive = hwif->cur_dev;
> -
>  	if (!drive_is_ready(drive))
>  		/*
>  		 * This happens regularly when we share a PCI IRQ with
> @@ -839,9 +839,6 @@ irqreturn_t ide_intr (int irq, void *dev
>  	del_timer(&hwif->timer);
>  	spin_unlock(&hwif->lock);
>  
> -	if (hwif->port_ops && hwif->port_ops->clear_irq)
> -		hwif->port_ops->clear_irq(drive);
> -
>  	if (drive->dev_flags & IDE_DFLAG_UNMASK)
>  		local_irq_enable_in_hardirq();
>  
> Index: ide-2.6/drivers/ide/macide.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/macide.c
> +++ ide-2.6/drivers/ide/macide.c
> @@ -55,13 +55,16 @@ volatile unsigned char *ide_ifr = (unsig
>  
>  int macide_ack_intr(ide_hwif_t* hwif)
>  {
> -	if (*ide_ifr & 0x20) {
> -		*ide_ifr &= ~0x20;
> +	if (*ide_ifr & 0x20)
>  		return 1;
> -	}
>  	return 0;
>  }
>  
> +static int macide_clear_irq(ide_drive_t *drive)
> +{
> +	*ide_ifr &= ~0x20;
> +}

s/int/void/

also please cc: m68k list on patches #3-5

Re: [PATCH 3/9] ide: move IRQ clearing from ack_intr() method to clear_irq() method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>> There are now two methods that clear the port interrupt: ack_intr() method,
>> implemented only on M680x0 machines, that is called at the start of ide_intr(),
>> and clear_irq() method, that is called somewhat later in this function.  In
>> order to stop this duplication, delegate the task of clearing the interrupt
>> to clear_irq() method, only leaving to ack_intr() the task of testing for the
>> port interrupt.  This involves moving clear_irq() method call in ide_intr()
>> closer to the beginning of the function and removing ack_intr() method call
>> in ide_timer_expiry(), now becoming useless...
>>
>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>
>> ---
>> The patch is atop of ide-2.6.git 'for-next' branch.
>>
>>  drivers/ide/gayle.c  |   23 +++++++++++------------
>>  drivers/ide/ide-io.c |   11 ++++-------
>>  drivers/ide/macide.c |   18 ++++++++++++++----
>>  3 files changed, 29 insertions(+), 23 deletions(-)
>>
>> Index: ide-2.6/drivers/ide/gayle.c
>> ===================================================================
>> --- ide-2.6.orig/drivers/ide/gayle.c
>> +++ ide-2.6/drivers/ide/gayle.c
>> @@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
>>       *  Check and acknowledge the interrupt status
>>       */
>>  
>> -static int gayle_ack_intr_a4000(ide_hwif_t *hwif)
>> +static int gayle_ack_intr(ide_hwif_t *hwif)
>>  {
>>      unsigned char ch;
>>  
>> @@ -76,16 +76,12 @@ static int gayle_ack_intr_a4000(ide_hwif
>>      return 1;
>>  }
>>  
>> -static int gayle_ack_intr_a1200(ide_hwif_t *hwif)
>> +static void gayle_a1200_clear_irq(ide_drive_t *drive)
>>  {
>> -    unsigned char ch;
>> +    ide_hwif_t *hwif = drive->hwif;
>>  
>> -    ch = z_readb(hwif->io_ports.irq_addr);
>> -    if (!(ch & GAYLE_IRQ_IDE))
>> -	return 0;
>>      (void)z_readb(hwif->io_ports.status_addr);
>>      z_writeb(0x7c, hwif->io_ports.irq_addr);
>> -    return 1;
>>  }
>>     
>
> buddha.c needs a similar treatment
>   

   Do you mean this fragment of xsurf_ack_intr()?

    /* X-Surf needs a 0 written to IRQ register to ensure ISA bit A11 stays at 0 */
    z_writeb(0, hwif->io_ports.irq_addr);


   I felt doubtful about it and decided to leave it as is.

>> --- ide-2.6.orig/drivers/ide/ide-io.c
>> +++ ide-2.6/drivers/ide/ide-io.c
>>     
>
>   
>> @@ -791,6 +789,10 @@ irqreturn_t ide_intr (int irq, void *dev
>>  		goto out;
>>  
>>  	handler = hwif->handler;
>> +	drive	= hwif->cur_dev;
>> +
>> +	if (hwif->port_ops && hwif->port_ops->clear_irq)
>> +		hwif->port_ops->clear_irq(drive);
>>     
>
> We need to check for valid ->handler before using ->cur_dev
> (it may contain a stale value otherwise).
>   

   Hm...

> Moreover I somehow miss the point of moving ->clear_irq call here
>   

   I moved it here because ack_intr() was clearing the interrupt at 
exactly *this* point.

> (it should be done after we know that it is really our IRQ).
>   

   I don't think that really matters much...

>>  	if (handler == NULL || hwif->polling) {
>>  		/*
>> @@ -821,8 +823,6 @@ irqreturn_t ide_intr (int irq, void *dev
>>  		goto out;
>>  	}
>>  
>> -	drive = hwif->cur_dev;
>> -
>>  	if (!drive_is_ready(drive))
>>  		/*
>>  		 * This happens regularly when we share a PCI IRQ with
>> @@ -839,9 +839,6 @@ irqreturn_t ide_intr (int irq, void *dev
>>  	del_timer(&hwif->timer);
>>  	spin_unlock(&hwif->lock);
>>  
>> -	if (hwif->port_ops && hwif->port_ops->clear_irq)
>> -		hwif->port_ops->clear_irq(drive);
>> -
>>  	if (drive->dev_flags & IDE_DFLAG_UNMASK)
>>  		local_irq_enable_in_hardirq();
>>  
>> Index: ide-2.6/drivers/ide/macide.c
>> ===================================================================
>> --- ide-2.6.orig/drivers/ide/macide.c
>> +++ ide-2.6/drivers/ide/macide.c
>> @@ -55,13 +55,16 @@ volatile unsigned char *ide_ifr = (unsig
>>  
>>  int macide_ack_intr(ide_hwif_t* hwif)
>>  {
>> -	if (*ide_ifr & 0x20) {
>> -		*ide_ifr &= ~0x20;
>> +	if (*ide_ifr & 0x20)
>>  		return 1;
>> -	}
>>  	return 0;
>>  }
>>  
>> +static int macide_clear_irq(ide_drive_t *drive)
>> +{
>> +	*ide_ifr &= ~0x20;
>> +}
>>     
>
> s/int/void/
>   

   Oops.

> also please cc: m68k list on patches #3-5
>   

   OK.

MBR, Sergei

Re: [PATCH 3/9] ide: move IRQ clearing from ack_intr() method to clear_irq() method

[Bartlomiej Zolnierkiewicz]

On Friday 12 June 2009 20:24:58 Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> >> There are now two methods that clear the port interrupt: ack_intr() method,
> >> implemented only on M680x0 machines, that is called at the start of ide_intr(),
> >> and clear_irq() method, that is called somewhat later in this function.  In
> >> order to stop this duplication, delegate the task of clearing the interrupt
> >> to clear_irq() method, only leaving to ack_intr() the task of testing for the
> >> port interrupt.  This involves moving clear_irq() method call in ide_intr()
> >> closer to the beginning of the function and removing ack_intr() method call
> >> in ide_timer_expiry(), now becoming useless...
> >>
> >> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> >>
> >> ---
> >> The patch is atop of ide-2.6.git 'for-next' branch.
> >>
> >>  drivers/ide/gayle.c  |   23 +++++++++++------------
> >>  drivers/ide/ide-io.c |   11 ++++-------
> >>  drivers/ide/macide.c |   18 ++++++++++++++----
> >>  3 files changed, 29 insertions(+), 23 deletions(-)
> >>
> >> Index: ide-2.6/drivers/ide/gayle.c
> >> ===================================================================
> >> --- ide-2.6.orig/drivers/ide/gayle.c
> >> +++ ide-2.6/drivers/ide/gayle.c
> >> @@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
> >>       *  Check and acknowledge the interrupt status
> >>       */
> >>  
> >> -static int gayle_ack_intr_a4000(ide_hwif_t *hwif)
> >> +static int gayle_ack_intr(ide_hwif_t *hwif)
> >>  {
> >>      unsigned char ch;
> >>  
> >> @@ -76,16 +76,12 @@ static int gayle_ack_intr_a4000(ide_hwif
> >>      return 1;
> >>  }
> >>  
> >> -static int gayle_ack_intr_a1200(ide_hwif_t *hwif)
> >> +static void gayle_a1200_clear_irq(ide_drive_t *drive)
> >>  {
> >> -    unsigned char ch;
> >> +    ide_hwif_t *hwif = drive->hwif;
> >>  
> >> -    ch = z_readb(hwif->io_ports.irq_addr);
> >> -    if (!(ch & GAYLE_IRQ_IDE))
> >> -	return 0;
> >>      (void)z_readb(hwif->io_ports.status_addr);
> >>      z_writeb(0x7c, hwif->io_ports.irq_addr);
> >> -    return 1;
> >>  }
> >>     
> >
> > buddha.c needs a similar treatment
> >   
> 
>    Do you mean this fragment of xsurf_ack_intr()?

Yes.

>     /* X-Surf needs a 0 written to IRQ register to ensure ISA bit A11 stays at 0 */
>     z_writeb(0, hwif->io_ports.irq_addr);
> 
> 
>    I felt doubtful about it and decided to leave it as is.
> 
> >> --- ide-2.6.orig/drivers/ide/ide-io.c
> >> +++ ide-2.6/drivers/ide/ide-io.c
> >>     
> >
> >   
> >> @@ -791,6 +789,10 @@ irqreturn_t ide_intr (int irq, void *dev
> >>  		goto out;
> >>  
> >>  	handler = hwif->handler;
> >> +	drive	= hwif->cur_dev;
> >> +
> >> +	if (hwif->port_ops && hwif->port_ops->clear_irq)
> >> +		hwif->port_ops->clear_irq(drive);
> >>     
> >
> > We need to check for valid ->handler before using ->cur_dev
> > (it may contain a stale value otherwise).
> >   
> 
>    Hm...
> 
> > Moreover I somehow miss the point of moving ->clear_irq call here
> >   
> 
>    I moved it here because ack_intr() was clearing the interrupt at 
> exactly *this* point.

ack_intr() is a m68k-specific thing -- please take a look at things from
this perspective.

> > (it should be done after we know that it is really our IRQ).
> >   
> 
>    I don't think that really matters much...

We can have ->clear_irq without ->test_irq (i.e. piix) and thus end up
incorrectly clearing IRQ in case of shared IRQs..

Re: [PATCH 3/9] ide: move IRQ clearing from ack_intr() method to clear_irq() method

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>> There are now two methods that clear the port interrupt: ack_intr() method,
>>>> implemented only on M680x0 machines, that is called at the start of ide_intr(),
>>>> and clear_irq() method, that is called somewhat later in this function.  In
>>>> order to stop this duplication, delegate the task of clearing the interrupt
>>>> to clear_irq() method, only leaving to ack_intr() the task of testing for the
>>>> port interrupt.  This involves moving clear_irq() method call in ide_intr()
>>>> closer to the beginning of the function and removing ack_intr() method call
>>>> in ide_timer_expiry(), now becoming useless...
>>>>
>>>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>>>
>>>> ---
>>>> The patch is atop of ide-2.6.git 'for-next' branch.
>>>>
>>>>  drivers/ide/gayle.c  |   23 +++++++++++------------
>>>>  drivers/ide/ide-io.c |   11 ++++-------
>>>>  drivers/ide/macide.c |   18 ++++++++++++++----
>>>>  3 files changed, 29 insertions(+), 23 deletions(-)
>>>>
>>>> Index: ide-2.6/drivers/ide/gayle.c
>>>> ===================================================================
>>>> --- ide-2.6.orig/drivers/ide/gayle.c
>>>> +++ ide-2.6/drivers/ide/gayle.c
>>>> @@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
>>>>       *  Check and acknowledge the interrupt status
>>>>       */
>>>>  
>>>> -static int gayle_ack_intr_a4000(ide_hwif_t *hwif)
>>>> +static int gayle_ack_intr(ide_hwif_t *hwif)
>>>>  {
>>>>      unsigned char ch;
>>>>  
>>>> @@ -76,16 +76,12 @@ static int gayle_ack_intr_a4000(ide_hwif
>>>>      return 1;
>>>>  }
>>>>  
>>>> -static int gayle_ack_intr_a1200(ide_hwif_t *hwif)
>>>> +static void gayle_a1200_clear_irq(ide_drive_t *drive)
>>>>  {
>>>> -    unsigned char ch;
>>>> +    ide_hwif_t *hwif = drive->hwif;
>>>>  
>>>> -    ch = z_readb(hwif->io_ports.irq_addr);
>>>> -    if (!(ch & GAYLE_IRQ_IDE))
>>>> -	return 0;
>>>>      (void)z_readb(hwif->io_ports.status_addr);
>>>>      z_writeb(0x7c, hwif->io_ports.irq_addr);
>>>> -    return 1;
>>>>  }
>>>>     
>>>>         
>>> buddha.c needs a similar treatment
>>>   
>>>       
>>    Do you mean this fragment of xsurf_ack_intr()?
>>     
>
> Yes.
>   

  OK, I'm taking your word for it.

>>     /* X-Surf needs a 0 written to IRQ register to ensure ISA bit A11 stays at 0 */
>>     z_writeb(0, hwif->io_ports.irq_addr);
>>     

   Ah, that's "A11", not "All"

>>
>>    I felt doubtful about it and decided to leave it as is.
>>
>>     
>>>> --- ide-2.6.orig/drivers/ide/ide-io.c
>>>> +++ ide-2.6/drivers/ide/ide-io.c
>>>>     
>>>>         
>>>   
>>>       
>>>> @@ -791,6 +789,10 @@ irqreturn_t ide_intr (int irq, void *dev
>>>>  		goto out;
>>>>  
>>>>  	handler = hwif->handler;
>>>> +	drive	= hwif->cur_dev;
>>>> +
>>>> +	if (hwif->port_ops && hwif->port_ops->clear_irq)
>>>> +		hwif->port_ops->clear_irq(drive);
>>>>     
>>>>         
>>> We need to check for valid ->handler before using ->cur_dev
>>> (it may contain a stale value otherwise).
>>>   
>>>       
>>    Hm...
>>     

   Do we really care about the valid 'drive' here? Ah, we do, for the 
sake of piix.c's implementation of clear_irq(); otherwise, we really 
don't...

>>> Moreover I somehow miss the point of moving ->clear_irq call here
>>>   
>>>       
>>    I moved it here because ack_intr() was clearing the interrupt at 
>> exactly *this* point.
>>     
>
> ack_intr() is a m68k-specific thing -- please take a look at things from
> this perspective.
>   

   I'm not sure what you mean by "this perspective". I've been looked at 
it from the more common perspective from which clearing IRQ earlier also 
made sense. When the handler is missing, we always "whack" the status 
register to clear the interrupt, so it seemed consistent to also clear 
it in the controller before doing this.

>>> (it should be done after we know that it is really our IRQ). 
>>>       
>>    I don't think that really matters much...
>>     
>
> We can have ->clear_irq without ->test_irq (i.e. piix) and thus end up
> incorrectly clearing IRQ in case of shared IRQs..
>   

   I doubt that you really can clear IRQ "incorrectly"... speaking of 
existing clear_irq() implementation, I should have added test_irq() 
handler to piix.c...

MBR, Sergei

Re: [PATCH 3/9] ide: move IRQ clearing from ack_intr() method to clear_irq() method

[Sergei Shtylyov]

Hello, I wrote:

>>>>> There are now two methods that clear the port interrupt: 
>>>>> ack_intr() method,
>>>>> implemented only on M680x0 machines, that is called at the start 
>>>>> of ide_intr(),
>>>>> and clear_irq() method, that is called somewhat later in this 
>>>>> function.  In
>>>>> order to stop this duplication, delegate the task of clearing the 
>>>>> interrupt
>>>>> to clear_irq() method, only leaving to ack_intr() the task of 
>>>>> testing for the
>>>>> port interrupt.  This involves moving clear_irq() method call in 
>>>>> ide_intr()
>>>>> closer to the beginning of the function and removing ack_intr() 
>>>>> method call
>>>>> in ide_timer_expiry(), now becoming useless...
>>>>>
>>>>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>>>>
>>>>> ---
>>>>> The patch is atop of ide-2.6.git 'for-next' branch.
>>>>>
>>>>>  drivers/ide/gayle.c  |   23 +++++++++++------------
>>>>>  drivers/ide/ide-io.c |   11 ++++-------
>>>>>  drivers/ide/macide.c |   18 ++++++++++++++----
>>>>>  3 files changed, 29 insertions(+), 23 deletions(-)
>>>>>
>>>>> Index: ide-2.6/drivers/ide/gayle.c
>>>>> ===================================================================
>>>>> --- ide-2.6.orig/drivers/ide/gayle.c
>>>>> +++ ide-2.6/drivers/ide/gayle.c
>>>>> @@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
>>>>>       *  Check and acknowledge the interrupt status
>>>>>       */
>>>>>  
>>>>> -static int gayle_ack_intr_a4000(ide_hwif_t *hwif)
>>>>> +static int gayle_ack_intr(ide_hwif_t *hwif)
>>>>>  {
>>>>>      unsigned char ch;
>>>>>  
>>>>> @@ -76,16 +76,12 @@ static int gayle_ack_intr_a4000(ide_hwif
>>>>>      return 1;
>>>>>  }
>>>>>  
>>>>> -static int gayle_ack_intr_a1200(ide_hwif_t *hwif)
>>>>> +static void gayle_a1200_clear_irq(ide_drive_t *drive)
>>>>>  {
>>>>> -    unsigned char ch;
>>>>> +    ide_hwif_t *hwif = drive->hwif;
>>>>>  
>>>>> -    ch = z_readb(hwif->io_ports.irq_addr);
>>>>> -    if (!(ch & GAYLE_IRQ_IDE))
>>>>> -    return 0;
>>>>>      (void)z_readb(hwif->io_ports.status_addr);
>>>>>      z_writeb(0x7c, hwif->io_ports.irq_addr);
>>>>> -    return 1;
>>>>>  }
>>>>>             
>>>> buddha.c needs a similar treatment
>>>>         
>>>    Do you mean this fragment of xsurf_ack_intr()?
>>>     
>>
>> Yes.
>>   
>
>  OK, I'm taking your word for it.
>
>>>     /* X-Surf needs a 0 written to IRQ register to ensure ISA bit 
>>> A11 stays at 0 */
>>>     z_writeb(0, hwif->io_ports.irq_addr);
>>>     
>
>   Ah, that's "A11", not "All"

   Haven't finished writing... if this is really here only to clear A11, 
I somewhat doubt that I need to modify this...

MBR, Sergei

Re: [PATCH 3/9] ide: move IRQ clearing from ack_intr() method to clear_irq() method

[Bartlomiej Zolnierkiewicz]

On Friday 12 June 2009 21:07:34 Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> >>>> There are now two methods that clear the port interrupt: ack_intr() method,
> >>>> implemented only on M680x0 machines, that is called at the start of ide_intr(),
> >>>> and clear_irq() method, that is called somewhat later in this function.  In
> >>>> order to stop this duplication, delegate the task of clearing the interrupt
> >>>> to clear_irq() method, only leaving to ack_intr() the task of testing for the
> >>>> port interrupt.  This involves moving clear_irq() method call in ide_intr()
> >>>> closer to the beginning of the function and removing ack_intr() method call
> >>>> in ide_timer_expiry(), now becoming useless...
> >>>>
> >>>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> >>>>
> >>>> ---
> >>>> The patch is atop of ide-2.6.git 'for-next' branch.
> >>>>
> >>>>  drivers/ide/gayle.c  |   23 +++++++++++------------
> >>>>  drivers/ide/ide-io.c |   11 ++++-------
> >>>>  drivers/ide/macide.c |   18 ++++++++++++++----
> >>>>  3 files changed, 29 insertions(+), 23 deletions(-)
> >>>>
> >>>> Index: ide-2.6/drivers/ide/gayle.c
> >>>> ===================================================================
> >>>> --- ide-2.6.orig/drivers/ide/gayle.c
> >>>> +++ ide-2.6/drivers/ide/gayle.c
> >>>> @@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
> >>>>       *  Check and acknowledge the interrupt status
> >>>>       */
> >>>>  
> >>>> -static int gayle_ack_intr_a4000(ide_hwif_t *hwif)
> >>>> +static int gayle_ack_intr(ide_hwif_t *hwif)
> >>>>  {
> >>>>      unsigned char ch;
> >>>>  
> >>>> @@ -76,16 +76,12 @@ static int gayle_ack_intr_a4000(ide_hwif
> >>>>      return 1;
> >>>>  }
> >>>>  
> >>>> -static int gayle_ack_intr_a1200(ide_hwif_t *hwif)
> >>>> +static void gayle_a1200_clear_irq(ide_drive_t *drive)
> >>>>  {
> >>>> -    unsigned char ch;
> >>>> +    ide_hwif_t *hwif = drive->hwif;
> >>>>  
> >>>> -    ch = z_readb(hwif->io_ports.irq_addr);
> >>>> -    if (!(ch & GAYLE_IRQ_IDE))
> >>>> -	return 0;
> >>>>      (void)z_readb(hwif->io_ports.status_addr);
> >>>>      z_writeb(0x7c, hwif->io_ports.irq_addr);
> >>>> -    return 1;
> >>>>  }
> >>>>     
> >>>>         
> >>> buddha.c needs a similar treatment
> >>>   
> >>>       
> >>    Do you mean this fragment of xsurf_ack_intr()?
> >>     
> >
> > Yes.
> >   
> 
>   OK, I'm taking your word for it.

Don't take my word on it, jut cc: m68k guys. :)

> >>     /* X-Surf needs a 0 written to IRQ register to ensure ISA bit A11 stays at 0 */
> >>     z_writeb(0, hwif->io_ports.irq_addr);
> >>     
> 
>    Ah, that's "A11", not "All"

lol

> >>
> >>    I felt doubtful about it and decided to leave it as is.
> >>
> >>     
> >>>> --- ide-2.6.orig/drivers/ide/ide-io.c
> >>>> +++ ide-2.6/drivers/ide/ide-io.c
> >>>>     
> >>>>         
> >>>   
> >>>       
> >>>> @@ -791,6 +789,10 @@ irqreturn_t ide_intr (int irq, void *dev
> >>>>  		goto out;
> >>>>  
> >>>>  	handler = hwif->handler;
> >>>> +	drive	= hwif->cur_dev;
> >>>> +
> >>>> +	if (hwif->port_ops && hwif->port_ops->clear_irq)
> >>>> +		hwif->port_ops->clear_irq(drive);
> >>>>     
> >>>>         
> >>> We need to check for valid ->handler before using ->cur_dev
> >>> (it may contain a stale value otherwise).
> >>>   
> >>>       
> >>    Hm...
> >>     
> 
>    Do we really care about the valid 'drive' here? Ah, we do, for the 
> sake of piix.c's implementation of clear_irq(); otherwise, we really 
> don't...

We want a good and maintainable design/code.  Not WorksForMeToday one.

> >>> Moreover I somehow miss the point of moving ->clear_irq call here
> >>>   
> >>>       
> >>    I moved it here because ack_intr() was clearing the interrupt at 
> >> exactly *this* point.
> >>     
> >
> > ack_intr() is a m68k-specific thing -- please take a look at things from
> > this perspective.
> >   
> 
>    I'm not sure what you mean by "this perspective". I've been looked at 
> it from the more common perspective from which clearing IRQ earlier also 
> made sense. When the handler is missing, we always "whack" the status 
> register to clear the interrupt, so it seemed consistent to also clear 
> it in the controller before doing this.
> 
> >>> (it should be done after we know that it is really our IRQ). 
> >>>       
> >>    I don't think that really matters much...
> >>     
> >
> > We can have ->clear_irq without ->test_irq (i.e. piix) and thus end up
> > incorrectly clearing IRQ in case of shared IRQs..
> >   
> 
>    I doubt that you really can clear IRQ "incorrectly"... speaking of 
> existing clear_irq() implementation, I should have added test_irq() 
> handler to piix.c...

Depends on the low-level hardware implementation.

[PATCH 4/9] ide: move ack_intr() method into 'struct ide_port_ops'

[Sergei Shtylyov]

Move the ack_intr() method into 'struct ide_port_ops', also renaming it to
test_irq() while at it...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/buddha.c |   27 ++++++++++++++++-----------
 drivers/ide/gayle.c  |   14 +++++++++-----
 drivers/ide/ide-io.c |    3 ++-
 drivers/ide/macide.c |   12 ++++--------
 include/linux/ide.h  |   10 +---------
 5 files changed, 32 insertions(+), 34 deletions(-)

Index: ide-2.6/drivers/ide/buddha.c
===================================================================
--- ide-2.6.orig/drivers/ide/buddha.c
+++ ide-2.6/drivers/ide/buddha.c
@@ -99,7 +99,7 @@ static const char *buddha_board_name[] =
      *  Check and acknowledge the interrupt status
      */
 
-static int buddha_ack_intr(ide_hwif_t *hwif)
+static int buddha_test_irq(ide_hwif_t *hwif)
 {
     unsigned char ch;
 
@@ -109,7 +109,7 @@ static int buddha_ack_intr(ide_hwif_t *h
     return 1;
 }
 
-static int xsurf_ack_intr(ide_hwif_t *hwif)
+static int xsurf_test_irq(ide_hwif_t *hwif)
 {
     unsigned char ch;
 
@@ -122,8 +122,7 @@ static int xsurf_ack_intr(ide_hwif_t *hw
 }
 
 static void __init buddha_setup_ports(struct ide_hw *hw, unsigned long base,
-				      unsigned long ctl, unsigned long irq_port,
-				      ide_ack_intr_t *ack_intr)
+				      unsigned long ctl, unsigned long irq_port)
 {
 	int i;
 
@@ -138,9 +137,16 @@ static void __init buddha_setup_ports(st
 	hw->io_ports.irq_addr = irq_port;
 
 	hw->irq = IRQ_AMIGA_PORTS;
-	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops buddha_port_ops = {
+	.test_irq		= buddha_test_irq,
+};
+
+static const struct ide_port_ops xsurf_port_ops = {
+	.test_irq		= xsurf_test_irq,
+};
+
 static const struct ide_port_info buddha_port_info = {
 	.host_flags		= IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA,
 	.irq_flags		= IRQF_SHARED,
@@ -157,6 +163,7 @@ static int __init buddha_init(void)
 	u_long buddha_board = 0;
 	BuddhaType type;
 	int buddha_num_hwifs, i;
+	struct ide_port_info d = buddha_port_info;
 
 	while ((z = zorro_find_device(ZORRO_WILDCARD, z))) {
 		unsigned long board;
@@ -203,28 +210,26 @@ fail_base2:
 
 		for (i = 0; i < buddha_num_hwifs; i++) {
 			unsigned long base, ctl, irq_port;
-			ide_ack_intr_t *ack_intr;
 
 			if (type != BOARD_XSURF) {
 				base = buddha_board + buddha_bases[i];
 				ctl = base + BUDDHA_CONTROL;
 				irq_port = buddha_board + buddha_irqports[i];
-				ack_intr = buddha_ack_intr;
+				d.port_ops = buddha_port_ops;
 			} else {
 				base = buddha_board + xsurf_bases[i];
 				/* X-Surf has no CS1* (Control/AltStat) */
 				ctl = 0;
 				irq_port = buddha_board + xsurf_irqports[i];
-				ack_intr = xsurf_ack_intr;
+				d.port_ops = xsurf_port_ops;
 			}
 
-			buddha_setup_ports(&hw[i], base, ctl, irq_port,
-					   ack_intr);
+			buddha_setup_ports(&hw[i], base, ctl, irq_port);
 
 			hws[i] = &hw[i];
 		}
 
-		ide_host_add(&buddha_port_info, hws, i, NULL);
+		ide_host_add(&d, hws, i, NULL);
 	}
 
 	return 0;
Index: ide-2.6/drivers/ide/gayle.c
===================================================================
--- ide-2.6.orig/drivers/ide/gayle.c
+++ ide-2.6/drivers/ide/gayle.c
@@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
      *  Check and acknowledge the interrupt status
      */
 
-static int gayle_ack_intr(ide_hwif_t *hwif)
+static int gayle_test_irq(ide_hwif_t *hwif)
 {
     unsigned char ch;
 
@@ -85,8 +85,7 @@ static void gayle_a1200_clear_irq(ide_dr
 }
 
 static void __init gayle_setup_ports(struct ide_hw *hw, unsigned long base,
-				     unsigned long ctl, unsigned long irq_port,
-				     ide_ack_intr_t *ack_intr)
+				     unsigned long ctl, unsigned long irq_port)
 {
 	int i;
 
@@ -101,11 +100,15 @@ static void __init gayle_setup_ports(str
 	hw->io_ports.irq_addr = irq_port;
 
 	hw->irq = IRQ_AMIGA_PORTS;
-	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops gayle_a4000_port_ops = {
+	.test_irq		= gayle_test_irq,
+};
+
 static const struct ide_port_ops gayle_a1200_port_ops = {
 	.clear_irq		= gayle_a1200_clear_irq,
+	.test_irq		= gayle_test_irq,
 };
 
 static const struct ide_port_info gayle_port_info = {
@@ -148,6 +151,7 @@ found:
 	if (a4000) {
 	    phys_base = GAYLE_BASE_4000;
 	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_4000);
+	    d.port_ops = gayle_a4000_port_ops;
 	} else {
 	    phys_base = GAYLE_BASE_1200;
 	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_1200);
@@ -164,7 +168,7 @@ found:
 	base = (unsigned long)ZTWO_VADDR(phys_base + i * GAYLE_NEXT_PORT);
 	ctrlport = GAYLE_HAS_CONTROL_REG ? (base + GAYLE_CONTROL) : 0;
 
-	gayle_setup_ports(&hw[i], base, ctrlport, irqport, gayle_ack_intr);
+	gayle_setup_ports(&hw[i], base, ctrlport, irqport);
 
 	hws[i] = &hw[i];
     }
Index: ide-2.6/drivers/ide/ide-io.c
===================================================================
--- ide-2.6.orig/drivers/ide/ide-io.c
+++ ide-2.6/drivers/ide/ide-io.c
@@ -789,7 +789,8 @@ irqreturn_t ide_intr (int irq, void *dev
 
 	spin_lock_irqsave(&hwif->lock, flags);
 
-	if (hwif->ack_intr && hwif->ack_intr(hwif) == 0)
+	if (hwif->port_ops && hwif->port_ops->test_irq &&
+	    hwif->port_ops->test_irq(hwif) == 0)
 		goto out;
 
 	handler = hwif->handler;
Index: ide-2.6/drivers/ide/macide.c
===================================================================
--- ide-2.6.orig/drivers/ide/macide.c
+++ ide-2.6/drivers/ide/macide.c
@@ -53,7 +53,7 @@
 
 volatile unsigned char *ide_ifr = (unsigned char *) (IDE_BASE + IDE_IFR);
 
-int macide_ack_intr(ide_hwif_t* hwif)
+int macide_test_irq(ide_hwif_t *hwif)
 {
 	if (*ide_ifr & 0x20)
 		return 1;
@@ -66,7 +66,7 @@ static int macide_clear_irq(ide_drive_t 
 }
 
 static void __init macide_setup_ports(struct ide_hw *hw, unsigned long base,
-				      int irq, ide_ack_intr_t *ack_intr)
+				      int irq)
 {
 	int i;
 
@@ -78,11 +78,11 @@ static void __init macide_setup_ports(st
 	hw->io_ports.ctl_addr = base + IDE_CONTROL;
 
 	hw->irq = irq;
-	hw->ack_intr = ack_intr;
 }
 
 static const struct ide_port_ops macide_port_ops = {
 	.clear_irq		= macide_clear_irq,
+	.test_irq		= macide_test_irq,
 };
 
 static const struct ide_port_info macide_port_info = {
@@ -101,7 +101,6 @@ static const char *mac_ide_name[] =
 
 static int __init macide_init(void)
 {
-	ide_ack_intr_t *ack_intr;
 	unsigned long base;
 	int irq;
 	struct ide_hw hw, *hws[] = { &hw };
@@ -113,17 +112,14 @@ static int __init macide_init(void)
 	switch (macintosh_config->ide_type) {
 	case MAC_IDE_QUADRA:
 		base = IDE_BASE;
-		ack_intr = macide_ack_intr;
 		irq = IRQ_NUBUS_F;
 		break;
 	case MAC_IDE_PB:
 		base = IDE_BASE;
-		ack_intr = macide_ack_intr;
 		irq = IRQ_NUBUS_C;
 		break;
 	case MAC_IDE_BABOON:
 		base = BABOON_BASE;
-		ack_intr = NULL;
 		d.port_ops = NULL;
 		irq = IRQ_BABOON_1;
 		break;
@@ -134,7 +130,7 @@ static int __init macide_init(void)
 	printk(KERN_INFO "ide: Macintosh %s IDE controller\n",
 			 mac_ide_name[macintosh_config->ide_type - 1]);
 
-	macide_setup_ports(&hw, base, irq, ack_intr);
+	macide_setup_ports(&hw, base, irq);
 
 	return ide_host_add(&d, hws, 1, NULL);
 }
Index: ide-2.6/include/linux/ide.h
===================================================================
--- ide-2.6.orig/include/linux/ide.h
+++ ide-2.6/include/linux/ide.h
@@ -157,12 +157,6 @@ enum {
 #define REQ_UNPARK_HEADS	0x23
 
 /*
- * Check for an interrupt and acknowledge the interrupt status
- */
-struct hwif_s;
-typedef int (ide_ack_intr_t)(struct hwif_s *);
-
-/*
  * hwif_chipset_t is used to keep track of the specific hardware
  * chipset used by each IDE interface, if known.
  */
@@ -185,7 +179,6 @@ struct ide_hw {
 	};
 
 	int		irq;			/* our irq number */
-	ide_ack_intr_t	*ack_intr;		/* acknowledge interrupt */
 	struct device	*dev, *parent;
 	unsigned long	config;
 };
@@ -649,6 +642,7 @@ struct ide_port_ops {
 	void	(*maskproc)(ide_drive_t *, int);
 	void	(*quirkproc)(ide_drive_t *);
 	void	(*clear_irq)(ide_drive_t *);
+	int	(*test_irq)(struct hwif_s *);
 
 	u8	(*mdma_filter)(ide_drive_t *);
 	u8	(*udma_filter)(ide_drive_t *);
@@ -708,8 +702,6 @@ typedef struct hwif_s {
 
 	struct device *dev;
 
-	ide_ack_intr_t *ack_intr;
-
 	void (*rw_disk)(ide_drive_t *, struct request *);
 
 	const struct ide_tp_ops		*tp_ops;

Re: [PATCH 4/9] ide: move ack_intr() method into 'struct ide_port_ops'

[Bartlomiej Zolnierkiewicz]

On Wednesday 10 June 2009 20:51:16 Sergei Shtylyov wrote:
> Move the ack_intr() method into 'struct ide_port_ops', also renaming it to
> test_irq() while at it...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> ---
> The patch is atop of ide-2.6.git 'for-next' branch.
> 
>  drivers/ide/buddha.c |   27 ++++++++++++++++-----------
>  drivers/ide/gayle.c  |   14 +++++++++-----
>  drivers/ide/ide-io.c |    3 ++-
>  drivers/ide/macide.c |   12 ++++--------
>  include/linux/ide.h  |   10 +---------
>  5 files changed, 32 insertions(+), 34 deletions(-)

Seems to be missing changes to ide-probe.c (!), falconide.c and q40ide.c.

Re: [PATCH 4/9] ide: move ack_intr() method into 'struct ide_port_ops'

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>> Move the ack_intr() method into 'struct ide_port_ops', also renaming it to
>> test_irq() while at it...
>>
>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>
>> ---
>> The patch is atop of ide-2.6.git 'for-next' branch.
>>
>>  drivers/ide/buddha.c |   27 ++++++++++++++++-----------
>>  drivers/ide/gayle.c  |   14 +++++++++-----
>>  drivers/ide/ide-io.c |    3 ++-
>>  drivers/ide/macide.c |   12 ++++--------
>>  include/linux/ide.h  |   10 +---------
>>  5 files changed, 32 insertions(+), 34 deletions(-)
>>     
>
> Seems to be missing changes to ide-probe.c (!),

   Hm... not sure how I've managed to miss it. :-<

>  falconide.c and q40ide.c.
>   

   Yet I was sure that I'd touched every driver with ack_intr()... :-/
   Working on one patch for several days doesn't help. :-(

WBR, Sergei

[PATCH 5/9] cmd640: implement test_irq() method

[Sergei Shtylyov]

Implement test_irq() method, adding the drive 2/3 interrupt bit definition.

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

---
This is based on PCI0640B datasheet.
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/cmd640.c |   13 +++++++++++++
 1 files changed, 13 insertions(+)

Index: ide-2.6/drivers/ide/cmd640.c
===================================================================
--- ide-2.6.orig/drivers/ide/cmd640.c
+++ ide-2.6/drivers/ide/cmd640.c
@@ -153,6 +153,7 @@ static int cmd640_vlb;
 #define ARTTIM23	0x57
 #define   ARTTIM23_DIS_RA2	0x04
 #define   ARTTIM23_DIS_RA3	0x08
+#define   ARTTIM23_IDE23INTR	0x10
 #define DRWTIM23	0x58
 #define BRST		0x59
 
@@ -629,12 +630,24 @@ static void cmd640_init_dev(ide_drive_t 
 #endif /* CONFIG_BLK_DEV_CMD640_ENHANCED */
 }
 
+static int cmd640_test_irq(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	int irq_reg		= hwif->channel ? ARTTIM23 : CFR;
+	u8  irq_stat, irq_mask	= hwif->channel ? ARTTIM23_IDE23INTR :
+						  CFR_IDE01INTR;
+
+	pci_read_config_byte(dev, irq_reg, &irq_stat);
+
+	return (irq_stat & irq_mask) ? 1 : 0;
+}
 
 static const struct ide_port_ops cmd640_port_ops = {
 	.init_dev		= cmd640_init_dev,
 #ifdef CONFIG_BLK_DEV_CMD640_ENHANCED
 	.set_pio_mode		= cmd640_set_pio_mode,
 #endif
+	.test_irq		= cmd640_test_irq,
 };
 
 static int pci_conf1(void)

[PATCH 6/9] cmd64x: implement test_irq() method

[Sergei Shtylyov]

Convert the driver's two dma_test_irq() methods into test_irq() methods. The
driver will now use the standard dma_test_irq() method implementation which
allows to remove 'cmd54x_dma_ops' and 'cmd648_dma_ops' that become identical
to 'sff_dma_ops'...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/cmd64x.c |   69 ++++++++-------------------------------------------
 1 files changed, 12 insertions(+), 57 deletions(-)

Index: ide-2.6/drivers/ide/cmd64x.c
===================================================================
--- ide-2.6.orig/drivers/ide/cmd64x.c
+++ ide-2.6/drivers/ide/cmd64x.c
@@ -253,53 +253,34 @@ static void cmd64x_clear_irq(ide_drive_t
 	(void) pci_write_config_byte(dev, irq_reg, irq_stat | irq_mask);
 }
 
-static int cmd648_dma_test_irq(ide_drive_t *drive)
+static int cmd648_test_irq(ide_hwif_t *hwif)
 {
-	ide_hwif_t *hwif	= drive->hwif;
-	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	unsigned long base	= pci_resource_start(dev, 4);
 	u8 irq_mask		= hwif->channel ? MRDMODE_INTR_CH1 :
 						  MRDMODE_INTR_CH0;
-	u8 dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 	u8 mrdmode		= inb(base + 1);
 
-#ifdef DEBUG
-	printk("%s: dma_stat: 0x%02x mrdmode: 0x%02x irq_mask: 0x%02x\n",
-	       drive->name, dma_stat, mrdmode, irq_mask);
-#endif
-	if (!(mrdmode & irq_mask))
-		return 0;
-
-	/* return 1 if INTR asserted */
-	if (dma_stat & 4)
-		return 1;
+	pr_debug("%s: mrdmode: 0x%02x irq_mask: 0x%02x\n",
+		 hwif->name, mrdmode, irq_mask);
 
-	return 0;
+	return (mrdmode & irq_mask) ? 1 : 0;
 }
 
-static int cmd64x_dma_test_irq(ide_drive_t *drive)
+static int cmd64x_test_irq(ide_hwif_t *hwif)
 {
-	ide_hwif_t *hwif	= drive->hwif;
 	struct pci_dev *dev	= to_pci_dev(hwif->dev);
 	int irq_reg		= hwif->channel ? ARTTIM23 : CFR;
 	u8  irq_mask		= hwif->channel ? ARTTIM23_INTR_CH1 :
 						  CFR_INTR_CH0;
-	u8  dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 	u8  irq_stat		= 0;
 
 	(void) pci_read_config_byte(dev, irq_reg, &irq_stat);
 
-#ifdef DEBUG
-	printk("%s: dma_stat: 0x%02x irq_stat: 0x%02x irq_mask: 0x%02x\n",
-	       drive->name, dma_stat, irq_stat, irq_mask);
-#endif
-	if (!(irq_stat & irq_mask))
-		return 0;
-
-	/* return 1 if INTR asserted */
-	if (dma_stat & 4)
-		return 1;
+	pr_debug("%s: irq_stat: 0x%02x irq_mask: 0x%02x\n",
+		 hwif->name, irq_stat, irq_mask);
 
-	return 0;
+	return (irq_stat & irq_mask) ? 1 : 0;
 }
 
 /*
@@ -365,6 +346,7 @@ static const struct ide_port_ops cmd64x_
 	.set_pio_mode		= cmd64x_set_pio_mode,
 	.set_dma_mode		= cmd64x_set_dma_mode,
 	.clear_irq		= cmd64x_clear_irq,
+	.test_irq		= cmd64x_test_irq,
 	.cable_detect		= cmd64x_cable_detect,
 };
 
@@ -372,20 +354,10 @@ static const struct ide_port_ops cmd648_
 	.set_pio_mode		= cmd64x_set_pio_mode,
 	.set_dma_mode		= cmd64x_set_dma_mode,
 	.clear_irq		= cmd648_clear_irq,
+	.test_irq		= cmd648_test_irq,
 	.cable_detect		= cmd64x_cable_detect,
 };
 
-static const struct ide_dma_ops cmd64x_dma_ops = {
-	.dma_host_set		= ide_dma_host_set,
-	.dma_setup		= ide_dma_setup,
-	.dma_start		= ide_dma_start,
-	.dma_end		= ide_dma_end,
-	.dma_test_irq		= cmd64x_dma_test_irq,
-	.dma_lost_irq		= ide_dma_lost_irq,
-	.dma_timer_expiry	= ide_dma_sff_timer_expiry,
-	.dma_sff_read_status	= ide_dma_sff_read_status,
-};
-
 static const struct ide_dma_ops cmd646_rev1_dma_ops = {
 	.dma_host_set		= ide_dma_host_set,
 	.dma_setup		= ide_dma_setup,
@@ -397,24 +369,12 @@ static const struct ide_dma_ops cmd646_r
 	.dma_sff_read_status	= ide_dma_sff_read_status,
 };
 
-static const struct ide_dma_ops cmd648_dma_ops = {
-	.dma_host_set		= ide_dma_host_set,
-	.dma_setup		= ide_dma_setup,
-	.dma_start		= ide_dma_start,
-	.dma_end		= ide_dma_end,
-	.dma_test_irq		= cmd648_dma_test_irq,
-	.dma_lost_irq		= ide_dma_lost_irq,
-	.dma_timer_expiry	= ide_dma_sff_timer_expiry,
-	.dma_sff_read_status	= ide_dma_sff_read_status,
-};
-
 static const struct ide_port_info cmd64x_chipsets[] __devinitdata = {
 	{	/* 0: CMD643 */
 		.name		= DRV_NAME,
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x00,0x00,0x00}, {0x51,0x08,0x08}},
 		.port_ops	= &cmd64x_port_ops,
-		.dma_ops	= &cmd64x_dma_ops,
 		.host_flags	= IDE_HFLAG_CLEAR_SIMPLEX |
 				  IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
@@ -426,7 +386,6 @@ static const struct ide_port_info cmd64x
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
 		.port_ops	= &cmd648_port_ops,
-		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_SERIALIZE |
 				  IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
@@ -438,7 +397,6 @@ static const struct ide_port_info cmd64x
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
 		.port_ops	= &cmd648_port_ops,
-		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
 		.mwdma_mask	= ATA_MWDMA2,
@@ -449,7 +407,6 @@ static const struct ide_port_info cmd64x
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
 		.port_ops	= &cmd648_port_ops,
-		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
 		.mwdma_mask	= ATA_MWDMA2,
@@ -489,8 +446,6 @@ static int __devinit cmd64x_init_one(str
 				d.port_ops = &cmd64x_port_ops;
 				if (dev->revision == 1)
 					d.dma_ops = &cmd646_rev1_dma_ops;
-				else
-					d.dma_ops = &cmd64x_dma_ops;
 			}
 		}
 	}

[PATCH 7/9] pdc202xx_old: implement test_irq() method

[Sergei Shtylyov]

Implement test_irq() method based on the driver's former dma_test_irq() method.

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

---
The patch is atop of ide-2.6.git 'for-next' branch.
Bart, it wouldn't hurt to check this against the documentation...

 drivers/ide/pdc202xx_old.c |   13 +++++++++++++
 1 files changed, 13 insertions(+)

Index: ide-2.6/drivers/ide/pdc202xx_old.c
===================================================================
--- ide-2.6.orig/drivers/ide/pdc202xx_old.c
+++ ide-2.6/drivers/ide/pdc202xx_old.c
@@ -104,6 +104,18 @@ static void pdc202xx_set_pio_mode(ide_dr
 	pdc202xx_set_mode(drive, XFER_PIO_0 + pio);
 }
 
+static int pdc202xx_test_irq(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	unsigned long high_16	= pci_resource_start(dev, 4);
+	u8 sc1d			= inb(high_16 + 0x1d);
+
+	if (hwif->channel)
+		return ((sc1d & 0x50) == 0x40) ? 1 : 0;
+	else
+		return ((sc1d & 0x05) == 0x04) ? 1 : 0;
+}
+
 static u8 pdc2026x_cable_detect(ide_hwif_t *hwif)
 {
 	struct pci_dev *dev = to_pci_dev(hwif->dev);
@@ -231,6 +243,7 @@ static void __devinit pdc202ata4_fixup_i
 static const struct ide_port_ops pdc20246_port_ops = {
 	.set_pio_mode		= pdc202xx_set_pio_mode,
 	.set_dma_mode		= pdc202xx_set_mode,
+	.test_irq		= pdc202xx_test_irq,
 };
 
 static const struct ide_port_ops pdc2026x_port_ops = {

Re: [PATCH 7/9] pdc202xx_old: implement test_irq() method

[Bartlomiej Zolnierkiewicz]

On Wednesday 10 June 2009 21:01:09 Sergei Shtylyov wrote:
> Implement test_irq() method based on the driver's former dma_test_irq() method.
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> ---
> The patch is atop of ide-2.6.git 'for-next' branch.
> Bart, it wouldn't hurt to check this against the documentation...

no luck with the docs again, they list all those bits as reserved..

Re: [PATCH 7/9] pdc202xx_old: implement test_irq() method

[Sergei Shtylyov]

Bartlomiej Zolnierkiewicz wrote:

>> Implement test_irq() method based on the driver's former dma_test_irq() method.
>>
>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>
>> ---
>> The patch is atop of ide-2.6.git 'for-next' branch.
>> Bart, it wouldn't hurt to check this against the documentation...
>>     
>
> no luck with the docs again, they list all those bits as reserved..
>   

  Ugh, I wonder where the author got the info then -- the bits were 
quite well described...

MBR, Sergei

[PATCH 8/9] siimage: implement test_irq() method

[Sergei Shtylyov]

Implement test_irq() method.  Remove interrupt bit test (not trusted anyway)
from siimage_io_dma_test_irq() and siimage_mmio_dma_test_irq(); this allows to
replace the former function with now identical ide_dma_test_irq()...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/siimage.c |   26 +++++++-------------------
 1 files changed, 7 insertions(+), 19 deletions(-)

Index: ide-2.6/drivers/ide/siimage.c
===================================================================
--- ide-2.6.orig/drivers/ide/siimage.c
+++ ide-2.6/drivers/ide/siimage.c
@@ -337,24 +337,14 @@ static void sil_set_dma_mode(ide_drive_t
 	sil_iowrite16(dev, ultra, ua);
 }
 
-/* returns 1 if dma irq issued, 0 otherwise */
-static int siimage_io_dma_test_irq(ide_drive_t *drive)
+static int sil_test_irq(ide_hwif_t *hwif)
 {
-	ide_hwif_t *hwif	= drive->hwif;
 	struct pci_dev *dev	= to_pci_dev(hwif->dev);
-	u8 dma_altstat		= 0;
 	unsigned long addr	= siimage_selreg(hwif, 1);
+	u8 val			= sil_ioread8(dev, addr);
 
-	/* return 1 if INTR asserted */
-	if (inb(hwif->dma_base + ATA_DMA_STATUS) & 4)
-		return 1;
-
-	/* return 1 if Device INTR asserted */
-	pci_read_config_byte(dev, addr, &dma_altstat);
-	if (dma_altstat & 8)
-		return 0;	/* return 1; */
-
-	return 0;
+	/* Return 1 if INTRQ asserted */
+	return (val & 8) ? 1 : 0;
 }
 
 /**
@@ -397,10 +387,6 @@ static int siimage_mmio_dma_test_irq(ide
 	if (readb((void __iomem *)(hwif->dma_base + ATA_DMA_STATUS)) & 4)
 		return 1;
 
-	/* return 1 if Device INTR asserted */
-	if (readb((void __iomem *)addr) & 8)
-		return 0;	/* return 1; */
-
 	return 0;
 }
 
@@ -409,7 +395,7 @@ static int siimage_dma_test_irq(ide_driv
 	if (drive->hwif->host_flags & IDE_HFLAG_MMIO)
 		return siimage_mmio_dma_test_irq(drive);
 	else
-		return siimage_io_dma_test_irq(drive);
+		return ide_dma_test_irq(drive);
 }
 
 /**
@@ -694,6 +680,7 @@ static const struct ide_port_ops sil_pat
 	.set_pio_mode		= sil_set_pio_mode,
 	.set_dma_mode		= sil_set_dma_mode,
 	.quirkproc		= sil_quirkproc,
+	.test_irq		= sil_test_irq,
 	.udma_filter		= sil_pata_udma_filter,
 	.cable_detect		= sil_cable_detect,
 };
@@ -704,6 +691,7 @@ static const struct ide_port_ops sil_sat
 	.reset_poll		= sil_sata_reset_poll,
 	.pre_reset		= sil_sata_pre_reset,
 	.quirkproc		= sil_quirkproc,
+	.test_irq		= sil_test_irq,
 	.udma_filter		= sil_sata_udma_filter,
 	.cable_detect		= sil_cable_detect,
 };

Re: [PATCH 8/9] siimage: implement test_irq() method

[Sergei Shtylyov]

Hello, I wrote:

> Implement test_irq() method.  Remove interrupt bit test (not trusted anyway)
> from siimage_io_dma_test_irq() and siimage_mmio_dma_test_irq(); this allows to
> replace the former function with now identical ide_dma_test_irq()...
>
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>
> ---
> The patch is atop of ide-2.6.git 'for-next' branch.
>   

   Hum, maybe this should have been 2 patches, not one...

>  drivers/ide/siimage.c |   26 +++++++-------------------
>  1 files changed, 7 insertions(+), 19 deletions(-)
>
> Index: ide-2.6/drivers/ide/siimage.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/siimage.c
> +++ ide-2.6/drivers/ide/siimage.c
> @@ -337,24 +337,14 @@ static void sil_set_dma_mode(ide_drive_t
>  	sil_iowrite16(dev, ultra, ua);
>  }
>  
> -/* returns 1 if dma irq issued, 0 otherwise */
> -static int siimage_io_dma_test_irq(ide_drive_t *drive)
> +static int sil_test_irq(ide_hwif_t *hwif)
>  {
> -	ide_hwif_t *hwif	= drive->hwif;
>  	struct pci_dev *dev	= to_pci_dev(hwif->dev);
> -	u8 dma_altstat		= 0;
>  	unsigned long addr	= siimage_selreg(hwif, 1);
> +	u8 val			= sil_ioread8(dev, addr);
>  
> -	/* return 1 if INTR asserted */
> -	if (inb(hwif->dma_base + ATA_DMA_STATUS) & 4)
> -		return 1;
> -
> -	/* return 1 if Device INTR asserted */
> -	pci_read_config_byte(dev, addr, &dma_altstat);
> -	if (dma_altstat & 8)
> -		return 0;	/* return 1; */
> -
> -	return 0;
> +	/* Return 1 if INTRQ asserted */
> +	return (val & 8) ? 1 : 0;
>  }
>  
>  /**
> @@ -397,10 +387,6 @@ static int siimage_mmio_dma_test_irq(ide
>  	if (readb((void __iomem *)(hwif->dma_base + ATA_DMA_STATUS)) & 4)
>  		return 1;
>  
> -	/* return 1 if Device INTR asserted */
> -	if (readb((void __iomem *)addr) & 8)
> -		return 0;	/* return 1; */
> -
>  	return 0;
>  }
>  
> @@ -409,7 +395,7 @@ static int siimage_dma_test_irq(ide_driv
>  	if (drive->hwif->host_flags & IDE_HFLAG_MMIO)
>  		return siimage_mmio_dma_test_irq(drive);
>  	else
> -		return siimage_io_dma_test_irq(drive);
> +		return ide_dma_test_irq(drive);
>  }
>  
>  /**
>   

WBR, Sergei

[PATCH 7.5/9] siimage: use ide_dma_test_irq()

[Sergei Shtylyov]

Remove interrupt bit test (not trusted anyway) from siimage_io_dma_test_irq()
and siimage_mmio_dma_test_irq() -- this allows to replace the former function
with now identical ide_dma_test_irq()...

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

---
This has been split from the yesterday's patch implementing test_irq().
The patch is atop of ide-2.6.git 'for-next' branch...

 drivers/ide/siimage.c |   26 +-------------------------
 1 files changed, 1 insertion(+), 25 deletions(-)

Index: ide-2.6/drivers/ide/siimage.c
===================================================================
--- ide-2.6.orig/drivers/ide/siimage.c
+++ ide-2.6/drivers/ide/siimage.c
@@ -337,26 +337,6 @@ static void sil_set_dma_mode(ide_drive_t
 	sil_iowrite16(dev, ultra, ua);
 }
 
-/* returns 1 if dma irq issued, 0 otherwise */
-static int siimage_io_dma_test_irq(ide_drive_t *drive)
-{
-	ide_hwif_t *hwif	= drive->hwif;
-	struct pci_dev *dev	= to_pci_dev(hwif->dev);
-	u8 dma_altstat		= 0;
-	unsigned long addr	= siimage_selreg(hwif, 1);
-
-	/* return 1 if INTR asserted */
-	if (inb(hwif->dma_base + ATA_DMA_STATUS) & 4)
-		return 1;
-
-	/* return 1 if Device INTR asserted */
-	pci_read_config_byte(dev, addr, &dma_altstat);
-	if (dma_altstat & 8)
-		return 0;	/* return 1; */
-
-	return 0;
-}
-
 /**
  *	siimage_mmio_dma_test_irq	-	check we caused an IRQ
  *	@drive: drive we are testing
@@ -397,10 +377,6 @@ static int siimage_mmio_dma_test_irq(ide
 	if (readb((void __iomem *)(hwif->dma_base + ATA_DMA_STATUS)) & 4)
 		return 1;
 
-	/* return 1 if Device INTR asserted */
-	if (readb((void __iomem *)addr) & 8)
-		return 0;	/* return 1; */
-
 	return 0;
 }
 
@@ -409,7 +385,7 @@ static int siimage_dma_test_irq(ide_driv
 	if (drive->hwif->host_flags & IDE_HFLAG_MMIO)
 		return siimage_mmio_dma_test_irq(drive);
 	else
-		return siimage_io_dma_test_irq(drive);
+		return ide_dma_test_irq(drive);
 }
 
 /**

Re: [PATCH 7.5/9] siimage: use ide_dma_test_irq()

[Bartlomiej Zolnierkiewicz]

On Thursday 11 June 2009 20:39:19 Sergei Shtylyov wrote:
> Remove interrupt bit test (not trusted anyway) from siimage_io_dma_test_irq()
> and siimage_mmio_dma_test_irq() -- this allows to replace the former function
> with now identical ide_dma_test_irq()...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

drivers/ide/siimage.c: In function ‘siimage_mmio_dma_test_irq’:
drivers/ide/siimage.c:351: warning: unused variable ‘addr’

Re: [PATCH 7.5/9] siimage: use ide_dma_test_irq()

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>> Remove interrupt bit test (not trusted anyway) from siimage_io_dma_test_irq()
>> and siimage_mmio_dma_test_irq() -- this allows to replace the former function
>> with now identical ide_dma_test_irq()...
>>
>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>     
>
> drivers/ide/siimage.c: In function ‘siimage_mmio_dma_test_irq’:
> drivers/ide/siimage.c:351: warning: unused variable ‘addr’
>   

   Saw that but apparently forgot to fix. Sigh... :-/

MBR, Sergei

[PATCH 8/9] siimage: implement test_irq() method

[Sergei Shtylyov]

Implement test_irq() method based on the driver's former dma_test_irq() methods.

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

---
This is a yesterday's patch after splitting.
The patch is atop of ide-2.6.git 'for-next' branch...

 drivers/ide/siimage.c |   12 ++++++++++++
 1 files changed, 12 insertions(+)

Index: ide-2.6/drivers/ide/siimage.c
===================================================================
--- ide-2.6.orig/drivers/ide/siimage.c
+++ ide-2.6/drivers/ide/siimage.c
@@ -337,6 +337,16 @@ static void sil_set_dma_mode(ide_drive_t
 	sil_iowrite16(dev, ultra, ua);
 }
 
+static int sil_test_irq(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	unsigned long addr	= siimage_selreg(hwif, 1);
+	u8 val			= sil_ioread8(dev, addr);
+
+	/* Return 1 if INTRQ asserted */
+	return (val & 8) ? 1 : 0;
+}
+
 /**
  *	siimage_mmio_dma_test_irq	-	check we caused an IRQ
  *	@drive: drive we are testing
@@ -670,6 +680,7 @@ static const struct ide_port_ops sil_pat
 	.set_pio_mode		= sil_set_pio_mode,
 	.set_dma_mode		= sil_set_dma_mode,
 	.quirkproc		= sil_quirkproc,
+	.test_irq		= sil_test_irq,
 	.udma_filter		= sil_pata_udma_filter,
 	.cable_detect		= sil_cable_detect,
 };
@@ -680,6 +691,7 @@ static const struct ide_port_ops sil_sat
 	.reset_poll		= sil_sata_reset_poll,
 	.pre_reset		= sil_sata_pre_reset,
 	.quirkproc		= sil_quirkproc,
+	.test_irq		= sil_test_irq,
 	.udma_filter		= sil_sata_udma_filter,
 	.cable_detect		= sil_cable_detect,
 };

[PATCH 3/10] siimage: use ide_dma_test_irq() (take 2)

[Sergei Shtylyov]

Remove interrupt bit test (not trusted anyway) from siimage_io_dma_test_irq()
and siimage_mmio_dma_test_irq() -- this allows to replace the former function
with now identical ide_dma_test_irq()...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

Changes since previous take:
- fixed warning about unused variable in siimage_mmio_dma_test_irq().

 drivers/ide/siimage.c |   27 +--------------------------
 1 files changed, 1 insertion(+), 26 deletions(-)

Index: ide-2.6/drivers/ide/siimage.c
===================================================================
--- ide-2.6.orig/drivers/ide/siimage.c
+++ ide-2.6/drivers/ide/siimage.c
@@ -337,26 +337,6 @@ static void sil_set_dma_mode(ide_drive_t
 	sil_iowrite16(dev, ultra, ua);
 }
 
-/* returns 1 if dma irq issued, 0 otherwise */
-static int siimage_io_dma_test_irq(ide_drive_t *drive)
-{
-	ide_hwif_t *hwif	= drive->hwif;
-	struct pci_dev *dev	= to_pci_dev(hwif->dev);
-	u8 dma_altstat		= 0;
-	unsigned long addr	= siimage_selreg(hwif, 1);
-
-	/* return 1 if INTR asserted */
-	if (inb(hwif->dma_base + ATA_DMA_STATUS) & 4)
-		return 1;
-
-	/* return 1 if Device INTR asserted */
-	pci_read_config_byte(dev, addr, &dma_altstat);
-	if (dma_altstat & 8)
-		return 0;	/* return 1; */
-
-	return 0;
-}
-
 /**
  *	siimage_mmio_dma_test_irq	-	check we caused an IRQ
  *	@drive: drive we are testing
@@ -368,7 +348,6 @@ static int siimage_io_dma_test_irq(ide_d
 static int siimage_mmio_dma_test_irq(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= drive->hwif;
-	unsigned long addr	= siimage_selreg(hwif, 0x1);
 	void __iomem *sata_error_addr
 		= (void __iomem *)hwif->sata_scr[SATA_ERROR_OFFSET];
 
@@ -397,10 +376,6 @@ static int siimage_mmio_dma_test_irq(ide
 	if (readb((void __iomem *)(hwif->dma_base + ATA_DMA_STATUS)) & 4)
 		return 1;
 
-	/* return 1 if Device INTR asserted */
-	if (readb((void __iomem *)addr) & 8)
-		return 0;	/* return 1; */
-
 	return 0;
 }
 
@@ -409,7 +384,7 @@ static int siimage_dma_test_irq(ide_driv
 	if (drive->hwif->host_flags & IDE_HFLAG_MMIO)
 		return siimage_mmio_dma_test_irq(drive);
 	else
-		return siimage_io_dma_test_irq(drive);
+		return ide_dma_test_irq(drive);
 }
 
 /**

[PATCH 9/9] sl82c105: implement test_irq() method

[Sergei Shtylyov]

Implement test_irq() method.

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/sl82c105.c |   11 +++++++++++
 1 files changed, 11 insertions(+)

Index: ide-2.6/drivers/ide/sl82c105.c
===================================================================
--- ide-2.6.orig/drivers/ide/sl82c105.c
+++ ide-2.6/drivers/ide/sl82c105.c
@@ -115,6 +115,16 @@ static void sl82c105_set_dma_mode(ide_dr
 	drive->drive_data |= (unsigned long)drv_ctrl << 16;
 }
 
+static int sl82c105_test_irq(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	u32 val, mask		= hwif->channel ? CTRL_IDE_IRQB : CTRL_IDE_IRQA;
+
+	pci_read_config_dword(dev, 0x40, &val);
+
+	return (val & mask) ? 1 : 0;
+}
+
 /*
  * The SL82C105 holds off all IDE interrupts while in DMA mode until
  * all DMA activity is completed.  Sometimes this causes problems (eg,
@@ -289,6 +299,7 @@ static const struct ide_port_ops sl82c10
 	.set_pio_mode		= sl82c105_set_pio_mode,
 	.set_dma_mode		= sl82c105_set_dma_mode,
 	.resetproc		= sl82c105_resetproc,
+	.test_irq		= sl82c105_test_irq,
 };
 
 static const struct ide_dma_ops sl82c105_dma_ops = {

[PATCH] sgiioc4: coding style cleanup

[Sergei Shtylyov]

Fix several errors and warnings given by checkpatch.pl:

- space between the asterisk and parameter name;

- inconsistent spacing between operator and operands;

- space between *sizeof* and open parenthesis;

- #include <asm/io.h> instead of #include <linux/io.h>

- use of *typedef* instead of a structure tag;

- line over 80 characters.

In addition to these changes, also do the following:

- indent with tabs instead of spaces;

- put the function's result type and name/parameters on the same line;

- join back the needlessly broken lines;

- get rid of needless type cast in sgiioc4_checkirq();

- remove space between the type cast and the variable name;

- remove commented out field initializer;

- uppercase the acronyms, lowercase the normal words in the comments;

- fix up the multi-line comment style...

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

---
This patch has been laying around for several months now, time to send it out.
It is atop of ide-2.6.git 'for-next' branch.
                                          
 drivers/ide/sgiioc4.c |  119 ++++++++++++++++++++++----------------------------
 1 files changed, 54 insertions(+), 65 deletions(-)

Index: ide-2.6/drivers/ide/sgiioc4.c
===================================================================
--- ide-2.6.orig/drivers/ide/sgiioc4.c
+++ ide-2.6/drivers/ide/sgiioc4.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2003-2006 Silicon Graphics, Inc.  All Rights Reserved.
- * Copyright (C) 2008 MontaVista Software, Inc.
+ * Copyright (C) 2008-2009 MontaVista Software, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of version 2 of the GNU General Public License
@@ -29,8 +29,7 @@
 #include <linux/blkdev.h>
 #include <linux/scatterlist.h>
 #include <linux/ioc4.h>
-#include <asm/io.h>
-
+#include <linux/io.h>
 #include <linux/ide.h>
 
 #define DRV_NAME "SGIIOC4"
@@ -72,7 +71,7 @@
 #define IOC4_CMD_CTL_BLK_SIZE	0x20
 #define IOC4_SUPPORTED_FIRMWARE_REV 46
 
-typedef struct {
+struct ioc4_dma_regs {
 	u32 timing_reg0;
 	u32 timing_reg1;
 	u32 low_mem_ptr;
@@ -82,17 +81,18 @@ typedef struct {
 	u32 dev_byte_count;
 	u32 mem_byte_count;
 	u32 status;
-} ioc4_dma_regs_t;
+};
 
 /* Each Physical Region Descriptor Entry size is 16 bytes (2 * 64 bits) */
 /* IOC4 has only 1 IDE channel */
-#define IOC4_PRD_BYTES       16
-#define IOC4_PRD_ENTRIES     (PAGE_SIZE /(4*IOC4_PRD_BYTES))
+#define IOC4_PRD_BYTES		16
+#define IOC4_PRD_ENTRIES	(PAGE_SIZE / (4 * IOC4_PRD_BYTES))
 
 
-static void
-sgiioc4_init_hwif_ports(struct ide_hw *hw, unsigned long data_port,
-			unsigned long ctrl_port, unsigned long irq_port)
+static void sgiioc4_init_hwif_ports(struct ide_hw *hw,
+				    unsigned long data_port,
+				    unsigned long ctrl_port,
+				    unsigned long irq_port)
 {
 	unsigned long reg = data_port;
 	int i;
@@ -105,13 +105,11 @@ sgiioc4_init_hwif_ports(struct ide_hw *h
 	hw->io_ports.irq_addr = irq_port;
 }
 
-static int
-sgiioc4_checkirq(ide_hwif_t * hwif)
+static int sgiioc4_checkirq(ide_hwif_t *hwif)
 {
-	unsigned long intr_addr =
-		hwif->io_ports.irq_addr + IOC4_INTR_REG * 4;
+	unsigned long intr_addr = hwif->io_ports.irq_addr + IOC4_INTR_REG * 4;
 
-	if ((u8)readl((void __iomem *)intr_addr) & 0x03)
+	if (readl((void __iomem *)intr_addr) & 0x03)
 		return 1;
 
 	return 0;
@@ -119,8 +117,7 @@ sgiioc4_checkirq(ide_hwif_t * hwif)
 
 static u8 sgiioc4_read_status(ide_hwif_t *);
 
-static int
-sgiioc4_clearirq(ide_drive_t * drive)
+static int sgiioc4_clearirq(ide_drive_t *drive)
 {
 	u32 intr_reg;
 	ide_hwif_t *hwif = drive->hwif;
@@ -158,12 +155,10 @@ sgiioc4_clearirq(ide_drive_t * drive)
 				readl((void __iomem *)(io_ports->irq_addr + 4));
 			pci_read_config_dword(dev, PCI_COMMAND,
 					      &pci_stat_cmd_reg);
-			printk(KERN_ERR
-			       "%s(%s) : PCI Bus Error when doing DMA:"
-				   " status-cmd reg is 0x%x\n",
+			printk(KERN_ERR "%s(%s): PCI Bus Error when doing DMA: "
+			       "status-cmd reg is 0x%x\n",
 			       __func__, drive->name, pci_stat_cmd_reg);
-			printk(KERN_ERR
-			       "%s(%s) : PCI Error Address is 0x%x%x\n",
+			printk(KERN_ERR "%s(%s): PCI Error Address is 0x%x%x\n",
 			       __func__, drive->name,
 			       pci_err_addr_high, pci_err_addr_low);
 			/* Clear the PCI Error indicator */
@@ -189,8 +184,7 @@ static void sgiioc4_dma_start(ide_drive_
 	writel(temp_reg, (void __iomem *)ioc4_dma_addr);
 }
 
-static u32
-sgiioc4_ide_dma_stop(ide_hwif_t *hwif, u64 dma_base)
+static u32 sgiioc4_ide_dma_stop(ide_hwif_t *hwif, u64 dma_base)
 {
 	unsigned long ioc4_dma_addr = dma_base + IOC4_DMA_CTRL * 4;
 	u32	ioc4_dma;
@@ -227,7 +221,7 @@ static int sgiioc4_dma_end(ide_drive_t *
 	}
 
 	/*
-	 * The IOC4 will DMA 1's to the ending dma area to indicate that
+	 * The IOC4 will DMA 1's to the ending DMA area to indicate that
 	 * previous data DMA is complete.  This is necessary because of relaxed
 	 * ordering between register reads and DMA writes on the Altix.
 	 */
@@ -265,7 +259,7 @@ static void sgiioc4_set_dma_mode(ide_dri
 {
 }
 
-/* returns 1 if dma irq issued, 0 otherwise */
+/* Returns 1 if DMA IRQ issued, 0 otherwise */
 static int sgiioc4_dma_test_irq(ide_drive_t *drive)
 {
 	return sgiioc4_checkirq(drive->hwif);
@@ -286,8 +280,7 @@ static void sgiioc4_resetproc(ide_drive_
 	sgiioc4_clearirq(drive);
 }
 
-static void
-sgiioc4_dma_lost_irq(ide_drive_t * drive)
+static void sgiioc4_dma_lost_irq(ide_drive_t *drive)
 {
 	sgiioc4_resetproc(drive);
 
@@ -313,13 +306,13 @@ static u8 sgiioc4_read_status(ide_hwif_t
 	return reg;
 }
 
-/* Creates a dma map for the scatter-gather list entries */
-static int __devinit
-ide_dma_sgiioc4(ide_hwif_t *hwif, const struct ide_port_info *d)
+/* Creates a DMA map for the scatter-gather list entries */
+static int __devinit ide_dma_sgiioc4(ide_hwif_t *hwif,
+				     const struct ide_port_info *d)
 {
 	struct pci_dev *dev = to_pci_dev(hwif->dev);
 	unsigned long dma_base = pci_resource_start(dev, 0) + IOC4_DMA_OFFSET;
-	int num_ports = sizeof (ioc4_dma_regs_t);
+	int num_ports = sizeof(struct ioc4_dma_regs);
 	void *pad;
 
 	printk(KERN_INFO "    %s: MMIO-DMA\n", hwif->name);
@@ -362,8 +355,7 @@ dma_pci_alloc_failure:
 }
 
 /* Initializes the IOC4 DMA Engine */
-static void
-sgiioc4_configure_for_dma(int dma_direction, ide_drive_t * drive)
+static void sgiioc4_configure_for_dma(int dma_direction, ide_drive_t *drive)
 {
 	u32 ioc4_dma;
 	ide_hwif_t *hwif = drive->hwif;
@@ -374,31 +366,27 @@ sgiioc4_configure_for_dma(int dma_direct
 	ioc4_dma = readl((void __iomem *)ioc4_dma_addr);
 
 	if (ioc4_dma & IOC4_S_DMA_ACTIVE) {
-		printk(KERN_WARNING
-			"%s(%s):Warning!! DMA from previous transfer was still active\n",
-		       __func__, drive->name);
+		printk(KERN_WARNING "%s(%s): Warning!! DMA from previous "
+		       "transfer was still active\n", __func__, drive->name);
 		writel(IOC4_S_DMA_STOP, (void __iomem *)ioc4_dma_addr);
 		ioc4_dma = sgiioc4_ide_dma_stop(hwif, dma_base);
 
 		if (ioc4_dma & IOC4_S_DMA_STOP)
-			printk(KERN_ERR
-			       "%s(%s) : IOC4 Dma STOP bit is still 1\n",
-			       __func__, drive->name);
+			printk(KERN_ERR "%s(%s): IOC4 DMA STOP bit is "
+			       "still 1\n", __func__, drive->name);
 	}
 
 	ioc4_dma = readl((void __iomem *)ioc4_dma_addr);
 	if (ioc4_dma & IOC4_S_DMA_ERROR) {
-		printk(KERN_WARNING
-		       "%s(%s) : Warning!! - DMA Error during Previous"
-		       " transfer | status 0x%x\n",
+		printk(KERN_WARNING "%s(%s): Warning!! DMA Error during "
+		       "previous transfer, status 0x%x\n",
 		       __func__, drive->name, ioc4_dma);
 		writel(IOC4_S_DMA_STOP, (void __iomem *)ioc4_dma_addr);
 		ioc4_dma = sgiioc4_ide_dma_stop(hwif, dma_base);
 
 		if (ioc4_dma & IOC4_S_DMA_STOP)
-			printk(KERN_ERR
-			       "%s(%s) : IOC4 DMA STOP bit is still 1\n",
-			       __func__, drive->name);
+			printk(KERN_ERR "%s(%s): IOC4 DMA STOP bit is "
+			       "still 1\n", __func__, drive->name);
 	}
 
 	/* Address of the Scatter Gather List */
@@ -408,20 +396,22 @@ sgiioc4_configure_for_dma(int dma_direct
 	/* Address of the Ending DMA */
 	memset(ide_get_hwifdata(hwif), 0, IOC4_IDE_CACHELINE_SIZE);
 	ending_dma_addr = cpu_to_le32(hwif->extra_base);
-	writel(ending_dma_addr, (void __iomem *)(dma_base + IOC4_DMA_END_ADDR * 4));
+	writel(ending_dma_addr, (void __iomem *)(dma_base +
+						 IOC4_DMA_END_ADDR * 4));
 
 	writel(dma_direction, (void __iomem *)ioc4_dma_addr);
 }
 
-/* IOC4 Scatter Gather list Format 					 */
+/* IOC4 Scatter Gather list Format					 */
 /* 128 Bit entries to support 64 bit addresses in the future		 */
 /* The Scatter Gather list Entry should be in the BIG-ENDIAN Format	 */
 /* --------------------------------------------------------------------- */
-/* | Upper 32 bits - Zero           |	 	Lower 32 bits- address | */
+/* | Upper 32 bits - Zero	    |		Lower 32 bits- address | */
 /* --------------------------------------------------------------------- */
 /* | Upper 32 bits - Zero	    |EOL| 15 unused     | 16 Bit Length| */
 /* --------------------------------------------------------------------- */
-/* Creates the scatter gather list, DMA Table */
+/* Creates the scatter gather list, DMA Table				 */
+
 static int sgiioc4_build_dmatable(ide_drive_t *drive, struct ide_cmd *cmd)
 {
 	ide_hwif_t *hwif = drive->hwif;
@@ -448,8 +438,10 @@ static int sgiioc4_build_dmatable(ide_dr
 				if (bcount > cur_len)
 					bcount = cur_len;
 
-				/* put the addr, length in
-				 * the IOC4 dma-table format */
+				/*
+				 * Put the address, length in
+				 * the IOC4 dma-table format
+				 */
 				*table = 0x0;
 				table++;
 				*table = cpu_to_be32(cur_addr);
@@ -540,8 +532,7 @@ static const struct ide_port_info sgiioc
 	.mwdma_mask		= ATA_MWDMA2_ONLY,
 };
 
-static int __devinit
-sgiioc4_ide_setup_pci_device(struct pci_dev *dev)
+static int __devinit sgiioc4_ide_setup_pci_device(struct pci_dev *dev)
 {
 	unsigned long cmd_base, irqport;
 	unsigned long bar0, cmd_phys_base, ctl;
@@ -549,7 +540,7 @@ sgiioc4_ide_setup_pci_device(struct pci_
 	struct ide_hw hw, *hws[] = { &hw };
 	int rc;
 
-	/*  Get the CmdBlk and CtrlBlk Base Registers */
+	/* Get the CmdBlk and CtrlBlk base registers */
 	bar0 = pci_resource_start(dev, 0);
 	virt_base = pci_ioremap_bar(dev, 0);
 	if (virt_base == NULL) {
@@ -557,9 +548,9 @@ sgiioc4_ide_setup_pci_device(struct pci_
 				DRV_NAME, bar0);
 		return -ENOMEM;
 	}
-	cmd_base = (unsigned long) virt_base + IOC4_CMD_OFFSET;
-	ctl = (unsigned long) virt_base + IOC4_CTRL_OFFSET;
-	irqport = (unsigned long) virt_base + IOC4_INTR_OFFSET;
+	cmd_base = (unsigned long)virt_base + IOC4_CMD_OFFSET;
+	ctl = (unsigned long)virt_base + IOC4_CTRL_OFFSET;
+	irqport = (unsigned long)virt_base + IOC4_INTR_OFFSET;
 
 	cmd_phys_base = bar0 + IOC4_CMD_OFFSET;
 	if (request_mem_region(cmd_phys_base, IOC4_CMD_CTL_BLK_SIZE,
@@ -577,7 +568,7 @@ sgiioc4_ide_setup_pci_device(struct pci_
 	hw.irq = dev->irq;
 	hw.dev = &dev->dev;
 
-	/* Initializing chipset IRQ Registers */
+	/* Initialize chipset IRQ registers */
 	writel(0x03, (void __iomem *)(irqport + IOC4_INTR_SET * 4));
 
 	rc = ide_host_add(&sgiioc4_port_info, hws, 1, NULL);
@@ -590,8 +581,7 @@ req_mem_rgn_err:
 	return rc;
 }
 
-static unsigned int __devinit
-pci_init_sgiioc4(struct pci_dev *dev)
+static unsigned int __devinit pci_init_sgiioc4(struct pci_dev *dev)
 {
 	int ret;
 
@@ -611,10 +601,10 @@ out:
 	return ret;
 }
 
-int __devinit
-ioc4_ide_attach_one(struct ioc4_driver_data *idd)
+int __devinit ioc4_ide_attach_one(struct ioc4_driver_data *idd)
 {
-	/* PCI-RT does not bring out IDE connection.
+	/*
+	 * PCI-RT does not bring out IDE connection.
 	 * Do not attach to this particular IOC4.
 	 */
 	if (idd->idd_variant == IOC4_VARIANT_PCI_RT)
@@ -627,7 +617,6 @@ static struct ioc4_submodule __devinitda
 	.is_name = "IOC4_ide",
 	.is_owner = THIS_MODULE,
 	.is_probe = ioc4_ide_attach_one,
-/*	.is_remove = ioc4_ide_remove_one,	*/
 };
 
 static int __init ioc4_ide_init(void)

Re: [PATCH] sgiioc4: coding style cleanup

[Bartlomiej Zolnierkiewicz]

On Thursday 11 June 2009 19:54:40 Sergei Shtylyov wrote:
> Fix several errors and warnings given by checkpatch.pl:
> 
> - space between the asterisk and parameter name;
> 
> - inconsistent spacing between operator and operands;
> 
> - space between *sizeof* and open parenthesis;
> 
> - #include <asm/io.h> instead of #include <linux/io.h>
> 
> - use of *typedef* instead of a structure tag;
> 
> - line over 80 characters.
> 
> In addition to these changes, also do the following:
> 
> - indent with tabs instead of spaces;
> 
> - put the function's result type and name/parameters on the same line;
> 
> - join back the needlessly broken lines;
> 
> - get rid of needless type cast in sgiioc4_checkirq();
> 
> - remove space between the type cast and the variable name;
> 
> - remove commented out field initializer;
> 
> - uppercase the acronyms, lowercase the normal words in the comments;
> 
> - fix up the multi-line comment style...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

applied

[PATCH 1/10] ide: call clear_irq() method in ide_timer_expiry()

[Sergei Shtylyov]

Now the clear_irq() method is called only from ide_intr() but ide_timer_expiry()
also should call this method in case when drive_is_ready() succeeds...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/ide-io.c |    3 +++
 1 files changed, 3 insertions(+)

Index: ide-2.6/drivers/ide/ide-io.c
===================================================================
--- ide-2.6.orig/drivers/ide/ide-io.c
+++ ide-2.6/drivers/ide/ide-io.c
@@ -669,6 +669,9 @@ void ide_timer_expiry (unsigned long dat
 				hwif->dma_ops->dma_lost_irq(drive);
 			if (hwif->ack_intr)
 				hwif->ack_intr(hwif);
+			if (hwif->port_ops && hwif->port_ops->clear_irq)
+				hwif->port_ops->clear_irq(drive);
+
 			printk(KERN_WARNING "%s: lost interrupt\n",
 				drive->name);
 			startstop = handler(drive);

Re: [PATCH 1/10] ide: call clear_irq() method in ide_timer_expiry()

[Bartlomiej Zolnierkiewicz]

On Saturday 13 June 2009 15:29:44 Sergei Shtylyov wrote:
> Now the clear_irq() method is called only from ide_intr() but ide_timer_expiry()
> also should call this method in case when drive_is_ready() succeeds...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

applied 1-10

[PATCH 2/10] cmd64x: implement clear_irq() method (take 2)

[Sergei Shtylyov]

Convert the driver's two dma_end() methods into clear_irq() methods -- the
driver will now use the standard dma_end() method implementation, ide_dma_end().

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

Changes since previous take:
- fixed cmd648_clear_irq() to use pci_resource_start() ISO hwif->dma_base.

 drivers/ide/cmd64x.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

Index: ide-2.6/drivers/ide/cmd64x.c
===================================================================
--- ide-2.6.orig/drivers/ide/cmd64x.c
+++ ide-2.6/drivers/ide/cmd64x.c
@@ -7,7 +7,7 @@
  * Copyright (C) 1998		David S. Miller (davem@redhat.com)
  *
  * Copyright (C) 1999-2002	Andre Hedrick <andre@linux-ide.org>
- * Copyright (C) 2007		MontaVista Software, Inc. <source@mvista.com>
+ * Copyright (C) 2007,2009	MontaVista Software, Inc. <source@mvista.com>
  */
 
 #include <linux/module.h>
@@ -226,11 +226,11 @@ static void cmd64x_set_dma_mode(ide_driv
 		(void) pci_write_config_byte(dev, pciU, regU);
 }
 
-static int cmd648_dma_end(ide_drive_t *drive)
+static void cmd648_clear_irq(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= drive->hwif;
-	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
-	int err			= ide_dma_end(drive);
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	unsigned long base	= pci_resource_start(dev, 4);
 	u8  irq_mask		= hwif->channel ? MRDMODE_INTR_CH1 :
 						  MRDMODE_INTR_CH0;
 	u8  mrdmode		= inb(base + 1);
@@ -238,11 +238,9 @@ static int cmd648_dma_end(ide_drive_t *d
 	/* clear the interrupt bit */
 	outb((mrdmode & ~(MRDMODE_INTR_CH0 | MRDMODE_INTR_CH1)) | irq_mask,
 	     base + 1);
-
-	return err;
 }
 
-static int cmd64x_dma_end(ide_drive_t *drive)
+static void cmd64x_clear_irq(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= drive->hwif;
 	struct pci_dev *dev	= to_pci_dev(hwif->dev);
@@ -250,13 +248,10 @@ static int cmd64x_dma_end(ide_drive_t *d
 	u8  irq_mask		= hwif->channel ? ARTTIM23_INTR_CH1 :
 						  CFR_INTR_CH0;
 	u8  irq_stat		= 0;
-	int err			= ide_dma_end(drive);
 
 	(void) pci_read_config_byte(dev, irq_reg, &irq_stat);
 	/* clear the interrupt bit */
 	(void) pci_write_config_byte(dev, irq_reg, irq_stat | irq_mask);
-
-	return err;
 }
 
 static int cmd648_dma_test_irq(ide_drive_t *drive)
@@ -370,6 +365,14 @@ static u8 cmd64x_cable_detect(ide_hwif_t
 static const struct ide_port_ops cmd64x_port_ops = {
 	.set_pio_mode		= cmd64x_set_pio_mode,
 	.set_dma_mode		= cmd64x_set_dma_mode,
+	.clear_irq		= cmd64x_clear_irq,
+	.cable_detect		= cmd64x_cable_detect,
+};
+
+static const struct ide_port_ops cmd648_port_ops = {
+	.set_pio_mode		= cmd64x_set_pio_mode,
+	.set_dma_mode		= cmd64x_set_dma_mode,
+	.clear_irq		= cmd648_clear_irq,
 	.cable_detect		= cmd64x_cable_detect,
 };
 
@@ -377,7 +380,7 @@ static const struct ide_dma_ops cmd64x_d
 	.dma_host_set		= ide_dma_host_set,
 	.dma_setup		= ide_dma_setup,
 	.dma_start		= ide_dma_start,
-	.dma_end		= cmd64x_dma_end,
+	.dma_end		= ide_dma_end,
 	.dma_test_irq		= cmd64x_dma_test_irq,
 	.dma_lost_irq		= ide_dma_lost_irq,
 	.dma_timer_expiry	= ide_dma_sff_timer_expiry,
@@ -399,7 +402,7 @@ static const struct ide_dma_ops cmd648_d
 	.dma_host_set		= ide_dma_host_set,
 	.dma_setup		= ide_dma_setup,
 	.dma_start		= ide_dma_start,
-	.dma_end		= cmd648_dma_end,
+	.dma_end		= ide_dma_end,
 	.dma_test_irq		= cmd648_dma_test_irq,
 	.dma_lost_irq		= ide_dma_lost_irq,
 	.dma_timer_expiry	= ide_dma_sff_timer_expiry,
@@ -423,7 +426,7 @@ static const struct ide_port_info cmd64x
 		.name		= DRV_NAME,
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
-		.port_ops	= &cmd64x_port_ops,
+		.port_ops	= &cmd648_port_ops,
 		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_SERIALIZE |
 				  IDE_HFLAG_ABUSE_PREFETCH,
@@ -435,7 +438,7 @@ static const struct ide_port_info cmd64x
 		.name		= DRV_NAME,
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
-		.port_ops	= &cmd64x_port_ops,
+		.port_ops	= &cmd648_port_ops,
 		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
@@ -446,7 +449,7 @@ static const struct ide_port_info cmd64x
 		.name		= DRV_NAME,
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
-		.port_ops	= &cmd64x_port_ops,
+		.port_ops	= &cmd648_port_ops,
 		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
@@ -484,6 +487,7 @@ static int __devinit cmd64x_init_one(str
 			 */
 			if (dev->revision < 3) {
 				d.enablebits[0].reg = 0;
+				d.port_ops = &cmd64x_port_ops;
 				if (dev->revision == 1)
 					d.dma_ops = &cmd646_rev1_dma_ops;
 				else

[PATCH 4/10] ide: move IRQ clearing from ack_intr() method to clear_irq() method (take 2)

[Sergei Shtylyov]

There are now two methods that clear the port interrupt: ack_intr() method,
implemented only on M680x0 machines, that is called at the start of ide_intr(),
and clear_irq() method, that is called somewhat later in this function.  In
order to stop this duplication, delegate the task of clearing the interrupt
to clear_irq() method, only leaving to ack_intr() the task of testing for the
port interrupt.

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

Changes since previous take:
- undid the code reordering in ide_intr();
- converted xsurf_ack_intr() into xsurf_clear_irq();
- fixed error in gayle_init() by adding missed &;
- fixed 2 warnings by making macide_clear_irq() void.

 drivers/ide/buddha.c |   27 +++++++++++++--------------
 drivers/ide/gayle.c  |   23 +++++++++++------------
 drivers/ide/ide-io.c |    2 --
 drivers/ide/macide.c |   18 ++++++++++++++----
 4 files changed, 38 insertions(+), 32 deletions(-)

Index: ide-2.6/drivers/ide/buddha.c
===================================================================
--- ide-2.6.orig/drivers/ide/buddha.c
+++ ide-2.6/drivers/ide/buddha.c
@@ -109,16 +109,12 @@ static int buddha_ack_intr(ide_hwif_t *h
     return 1;
 }
 
-static int xsurf_ack_intr(ide_hwif_t *hwif)
+static void xsurf_clear_irq(ide_drive_t *drive)
 {
-    unsigned char ch;
-
-    ch = z_readb(hwif->io_ports.irq_addr);
-    /* X-Surf needs a 0 written to IRQ register to ensure ISA bit A11 stays at 0 */
-    z_writeb(0, hwif->io_ports.irq_addr);
-    if (!(ch & 0x80))
-	    return 0;
-    return 1;
+    /*
+     * X-Surf needs 0 written to IRQ register to ensure ISA bit A11 stays at 0
+     */
+    z_writeb(0, drive->hwif->io_ports.irq_addr);
 }
 
 static void __init buddha_setup_ports(struct ide_hw *hw, unsigned long base,
@@ -141,6 +137,10 @@ static void __init buddha_setup_ports(st
 	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops xsurf_port_ops = {
+	.clear_irq		= xsurf_clear_irq,
+};
+
 static const struct ide_port_info buddha_port_info = {
 	.host_flags		= IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA,
 	.irq_flags		= IRQF_SHARED,
@@ -161,6 +161,7 @@ static int __init buddha_init(void)
 	while ((z = zorro_find_device(ZORRO_WILDCARD, z))) {
 		unsigned long board;
 		struct ide_hw hw[MAX_NUM_HWIFS], *hws[MAX_NUM_HWIFS];
+		struct ide_port_info d = buddha_port_info;
 
 		if (z->id == ZORRO_PROD_INDIVIDUAL_COMPUTERS_BUDDHA) {
 			buddha_num_hwifs = BUDDHA_NUM_HWIFS;
@@ -171,6 +172,7 @@ static int __init buddha_init(void)
 		} else if (z->id == ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF) {
 			buddha_num_hwifs = XSURF_NUM_HWIFS;
 			type=BOARD_XSURF;
+			d.port_ops = &xsurf_port_ops;
 		} else 
 			continue;
 		
@@ -203,28 +205,25 @@ fail_base2:
 
 		for (i = 0; i < buddha_num_hwifs; i++) {
 			unsigned long base, ctl, irq_port;
-			ide_ack_intr_t *ack_intr;
 
 			if (type != BOARD_XSURF) {
 				base = buddha_board + buddha_bases[i];
 				ctl = base + BUDDHA_CONTROL;
 				irq_port = buddha_board + buddha_irqports[i];
-				ack_intr = buddha_ack_intr;
 			} else {
 				base = buddha_board + xsurf_bases[i];
 				/* X-Surf has no CS1* (Control/AltStat) */
 				ctl = 0;
 				irq_port = buddha_board + xsurf_irqports[i];
-				ack_intr = xsurf_ack_intr;
 			}
 
 			buddha_setup_ports(&hw[i], base, ctl, irq_port,
-					   ack_intr);
+					   buddha_ack_intr);
 
 			hws[i] = &hw[i];
 		}
 
-		ide_host_add(&buddha_port_info, hws, i, NULL);
+		ide_host_add(&d, hws, i, NULL);
 	}
 
 	return 0;
Index: ide-2.6/drivers/ide/gayle.c
===================================================================
--- ide-2.6.orig/drivers/ide/gayle.c
+++ ide-2.6/drivers/ide/gayle.c
@@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
      *  Check and acknowledge the interrupt status
      */
 
-static int gayle_ack_intr_a4000(ide_hwif_t *hwif)
+static int gayle_ack_intr(ide_hwif_t *hwif)
 {
     unsigned char ch;
 
@@ -76,16 +76,12 @@ static int gayle_ack_intr_a4000(ide_hwif
     return 1;
 }
 
-static int gayle_ack_intr_a1200(ide_hwif_t *hwif)
+static void gayle_a1200_clear_irq(ide_drive_t *drive)
 {
-    unsigned char ch;
+    ide_hwif_t *hwif = drive->hwif;
 
-    ch = z_readb(hwif->io_ports.irq_addr);
-    if (!(ch & GAYLE_IRQ_IDE))
-	return 0;
     (void)z_readb(hwif->io_ports.status_addr);
     z_writeb(0x7c, hwif->io_ports.irq_addr);
-    return 1;
 }
 
 static void __init gayle_setup_ports(struct ide_hw *hw, unsigned long base,
@@ -108,6 +104,10 @@ static void __init gayle_setup_ports(str
 	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops gayle_a1200_port_ops = {
+	.clear_irq		= gayle_a1200_clear_irq,
+};
+
 static const struct ide_port_info gayle_port_info = {
 	.host_flags		= IDE_HFLAG_MMIO | IDE_HFLAG_SERIALIZE |
 				  IDE_HFLAG_NO_DMA,
@@ -123,9 +123,9 @@ static int __init gayle_init(void)
 {
     unsigned long phys_base, res_start, res_n;
     unsigned long base, ctrlport, irqport;
-    ide_ack_intr_t *ack_intr;
     int a4000, i, rc;
     struct ide_hw hw[GAYLE_NUM_HWIFS], *hws[GAYLE_NUM_HWIFS];
+    struct ide_port_info d = gayle_port_info;
 
     if (!MACH_IS_AMIGA)
 	return -ENODEV;
@@ -148,11 +148,10 @@ found:
 	if (a4000) {
 	    phys_base = GAYLE_BASE_4000;
 	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_4000);
-	    ack_intr = gayle_ack_intr_a4000;
 	} else {
 	    phys_base = GAYLE_BASE_1200;
 	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_1200);
-	    ack_intr = gayle_ack_intr_a1200;
+	    d.port_ops = &gayle_a1200_port_ops;
 	}
 
 	res_start = ((unsigned long)phys_base) & ~(GAYLE_NEXT_PORT-1);
@@ -165,12 +164,12 @@ found:
 	base = (unsigned long)ZTWO_VADDR(phys_base + i * GAYLE_NEXT_PORT);
 	ctrlport = GAYLE_HAS_CONTROL_REG ? (base + GAYLE_CONTROL) : 0;
 
-	gayle_setup_ports(&hw[i], base, ctrlport, irqport, ack_intr);
+	gayle_setup_ports(&hw[i], base, ctrlport, irqport, gayle_ack_intr);
 
 	hws[i] = &hw[i];
     }
 
-    rc = ide_host_add(&gayle_port_info, hws, i, NULL);
+    rc = ide_host_add(&d, hws, i, NULL);
     if (rc)
 	release_mem_region(res_start, res_n);
 
Index: ide-2.6/drivers/ide/ide-io.c
===================================================================
--- ide-2.6.orig/drivers/ide/ide-io.c
+++ ide-2.6/drivers/ide/ide-io.c
@@ -671,8 +671,6 @@ void ide_timer_expiry (unsigned long dat
 		} else if (drive_is_ready(drive)) {
 			if (drive->waiting_for_dma)
 				hwif->dma_ops->dma_lost_irq(drive);
-			if (hwif->ack_intr)
-				hwif->ack_intr(hwif);
 			if (hwif->port_ops && hwif->port_ops->clear_irq)
 				hwif->port_ops->clear_irq(drive);
 
Index: ide-2.6/drivers/ide/macide.c
===================================================================
--- ide-2.6.orig/drivers/ide/macide.c
+++ ide-2.6/drivers/ide/macide.c
@@ -55,13 +55,16 @@ volatile unsigned char *ide_ifr = (unsig
 
 int macide_ack_intr(ide_hwif_t* hwif)
 {
-	if (*ide_ifr & 0x20) {
-		*ide_ifr &= ~0x20;
+	if (*ide_ifr & 0x20)
 		return 1;
-	}
 	return 0;
 }
 
+static void macide_clear_irq(ide_drive_t *drive)
+{
+	*ide_ifr &= ~0x20;
+}
+
 static void __init macide_setup_ports(struct ide_hw *hw, unsigned long base,
 				      int irq, ide_ack_intr_t *ack_intr)
 {
@@ -78,7 +81,12 @@ static void __init macide_setup_ports(st
 	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops macide_port_ops = {
+	.clear_irq		= macide_clear_irq,
+};
+
 static const struct ide_port_info macide_port_info = {
+	.port_ops		= &macide_port_ops,
 	.host_flags		= IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA,
 	.irq_flags		= IRQF_SHARED,
 	.chipset		= ide_generic,
@@ -97,6 +105,7 @@ static int __init macide_init(void)
 	unsigned long base;
 	int irq;
 	struct ide_hw hw, *hws[] = { &hw };
+	struct ide_port_info d = macide_port_info;
 
 	if (!MACH_IS_MAC)
 		return -ENODEV;
@@ -115,6 +124,7 @@ static int __init macide_init(void)
 	case MAC_IDE_BABOON:
 		base = BABOON_BASE;
 		ack_intr = NULL;
+		d.port_ops = NULL;
 		irq = IRQ_BABOON_1;
 		break;
 	default:
@@ -126,7 +136,7 @@ static int __init macide_init(void)
 
 	macide_setup_ports(&hw, base, irq, ack_intr);
 
-	return ide_host_add(&macide_port_info, hws, 1, NULL);
+	return ide_host_add(&d, hws, 1, NULL);
 }
 
 module_init(macide_init);

[PATCH 5/10] ide: move ack_intr() method into 'struct ide_port_ops' (take 2)

[Sergei Shtylyov]

Move the ack_intr() method into 'struct ide_port_ops', also renaming it to
test_irq() while at it...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

Changes since previous take:
- added the missed changes to ide-probe.c, falconide.c, and q40ide.c;
- fixed up buddha.c atop of the updated preceding patch;
- fixed error in gayle_init() by adding missed &...

 drivers/ide/buddha.c    |   15 +++++++++------
 drivers/ide/falconide.c |    1 -
 drivers/ide/gayle.c     |   14 +++++++++-----
 drivers/ide/ide-io.c    |    3 ++-
 drivers/ide/ide-probe.c |    1 -
 drivers/ide/macide.c    |   12 ++++--------
 drivers/ide/q40ide.c    |    7 ++-----
 include/linux/ide.h     |   10 +---------
 8 files changed, 27 insertions(+), 36 deletions(-)

Index: ide-2.6/drivers/ide/buddha.c
===================================================================
--- ide-2.6.orig/drivers/ide/buddha.c
+++ ide-2.6/drivers/ide/buddha.c
@@ -99,7 +99,7 @@ static const char *buddha_board_name[] =
      *  Check and acknowledge the interrupt status
      */
 
-static int buddha_ack_intr(ide_hwif_t *hwif)
+static int buddha_test_irq(ide_hwif_t *hwif)
 {
     unsigned char ch;
 
@@ -118,8 +118,7 @@ static void xsurf_clear_irq(ide_drive_t 
 }
 
 static void __init buddha_setup_ports(struct ide_hw *hw, unsigned long base,
-				      unsigned long ctl, unsigned long irq_port,
-				      ide_ack_intr_t *ack_intr)
+				      unsigned long ctl, unsigned long irq_port)
 {
 	int i;
 
@@ -134,14 +133,19 @@ static void __init buddha_setup_ports(st
 	hw->io_ports.irq_addr = irq_port;
 
 	hw->irq = IRQ_AMIGA_PORTS;
-	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops buddha_port_ops = {
+	.test_irq		= buddha_test_irq,
+};
+
 static const struct ide_port_ops xsurf_port_ops = {
 	.clear_irq		= xsurf_clear_irq,
+	.test_irq		= buddha_test_irq,
 };
 
 static const struct ide_port_info buddha_port_info = {
+	.port_ops		= &buddha_port_ops,
 	.host_flags		= IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA,
 	.irq_flags		= IRQF_SHARED,
 	.chipset		= ide_generic,
@@ -217,8 +221,7 @@ fail_base2:
 				irq_port = buddha_board + xsurf_irqports[i];
 			}
 
-			buddha_setup_ports(&hw[i], base, ctl, irq_port,
-					   buddha_ack_intr);
+			buddha_setup_ports(&hw[i], base, ctl, irq_port);
 
 			hws[i] = &hw[i];
 		}
Index: ide-2.6/drivers/ide/falconide.c
===================================================================
--- ide-2.6.orig/drivers/ide/falconide.c
+++ ide-2.6/drivers/ide/falconide.c
@@ -128,7 +128,6 @@ static void __init falconide_setup_ports
 	hw->io_ports.ctl_addr = ATA_HD_BASE + ATA_HD_CONTROL;
 
 	hw->irq = IRQ_MFP_IDE;
-	hw->ack_intr = NULL;
 }
 
     /*
Index: ide-2.6/drivers/ide/gayle.c
===================================================================
--- ide-2.6.orig/drivers/ide/gayle.c
+++ ide-2.6/drivers/ide/gayle.c
@@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
      *  Check and acknowledge the interrupt status
      */
 
-static int gayle_ack_intr(ide_hwif_t *hwif)
+static int gayle_test_irq(ide_hwif_t *hwif)
 {
     unsigned char ch;
 
@@ -85,8 +85,7 @@ static void gayle_a1200_clear_irq(ide_dr
 }
 
 static void __init gayle_setup_ports(struct ide_hw *hw, unsigned long base,
-				     unsigned long ctl, unsigned long irq_port,
-				     ide_ack_intr_t *ack_intr)
+				     unsigned long ctl, unsigned long irq_port)
 {
 	int i;
 
@@ -101,11 +100,15 @@ static void __init gayle_setup_ports(str
 	hw->io_ports.irq_addr = irq_port;
 
 	hw->irq = IRQ_AMIGA_PORTS;
-	hw->ack_intr = ack_intr;
 }
 
+static const struct ide_port_ops gayle_a4000_port_ops = {
+	.test_irq		= gayle_test_irq,
+};
+
 static const struct ide_port_ops gayle_a1200_port_ops = {
 	.clear_irq		= gayle_a1200_clear_irq,
+	.test_irq		= gayle_test_irq,
 };
 
 static const struct ide_port_info gayle_port_info = {
@@ -148,6 +151,7 @@ found:
 	if (a4000) {
 	    phys_base = GAYLE_BASE_4000;
 	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_4000);
+	    d.port_ops = &gayle_a4000_port_ops;
 	} else {
 	    phys_base = GAYLE_BASE_1200;
 	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_1200);
@@ -164,7 +168,7 @@ found:
 	base = (unsigned long)ZTWO_VADDR(phys_base + i * GAYLE_NEXT_PORT);
 	ctrlport = GAYLE_HAS_CONTROL_REG ? (base + GAYLE_CONTROL) : 0;
 
-	gayle_setup_ports(&hw[i], base, ctrlport, irqport, gayle_ack_intr);
+	gayle_setup_ports(&hw[i], base, ctrlport, irqport);
 
 	hws[i] = &hw[i];
     }
Index: ide-2.6/drivers/ide/ide-io.c
===================================================================
--- ide-2.6.orig/drivers/ide/ide-io.c
+++ ide-2.6/drivers/ide/ide-io.c
@@ -789,7 +789,8 @@ irqreturn_t ide_intr (int irq, void *dev
 
 	spin_lock_irqsave(&hwif->lock, flags);
 
-	if (hwif->ack_intr && hwif->ack_intr(hwif) == 0)
+	if (hwif->port_ops && hwif->port_ops->test_irq &&
+	    hwif->port_ops->test_irq(hwif) == 0)
 		goto out;
 
 	handler = hwif->handler;
Index: ide-2.6/drivers/ide/ide-probe.c
===================================================================
--- ide-2.6.orig/drivers/ide/ide-probe.c
+++ ide-2.6/drivers/ide/ide-probe.c
@@ -1170,7 +1170,6 @@ static void ide_init_port_hw(ide_hwif_t 
 	hwif->irq = hw->irq;
 	hwif->dev = hw->dev;
 	hwif->gendev.parent = hw->parent ? hw->parent : hw->dev;
-	hwif->ack_intr = hw->ack_intr;
 	hwif->config_data = hw->config;
 }
 
Index: ide-2.6/drivers/ide/macide.c
===================================================================
--- ide-2.6.orig/drivers/ide/macide.c
+++ ide-2.6/drivers/ide/macide.c
@@ -53,7 +53,7 @@
 
 volatile unsigned char *ide_ifr = (unsigned char *) (IDE_BASE + IDE_IFR);
 
-int macide_ack_intr(ide_hwif_t* hwif)
+int macide_test_irq(ide_hwif_t *hwif)
 {
 	if (*ide_ifr & 0x20)
 		return 1;
@@ -66,7 +66,7 @@ static void macide_clear_irq(ide_drive_t
 }
 
 static void __init macide_setup_ports(struct ide_hw *hw, unsigned long base,
-				      int irq, ide_ack_intr_t *ack_intr)
+				      int irq)
 {
 	int i;
 
@@ -78,11 +78,11 @@ static void __init macide_setup_ports(st
 	hw->io_ports.ctl_addr = base + IDE_CONTROL;
 
 	hw->irq = irq;
-	hw->ack_intr = ack_intr;
 }
 
 static const struct ide_port_ops macide_port_ops = {
 	.clear_irq		= macide_clear_irq,
+	.test_irq		= macide_test_irq,
 };
 
 static const struct ide_port_info macide_port_info = {
@@ -101,7 +101,6 @@ static const char *mac_ide_name[] =
 
 static int __init macide_init(void)
 {
-	ide_ack_intr_t *ack_intr;
 	unsigned long base;
 	int irq;
 	struct ide_hw hw, *hws[] = { &hw };
@@ -113,17 +112,14 @@ static int __init macide_init(void)
 	switch (macintosh_config->ide_type) {
 	case MAC_IDE_QUADRA:
 		base = IDE_BASE;
-		ack_intr = macide_ack_intr;
 		irq = IRQ_NUBUS_F;
 		break;
 	case MAC_IDE_PB:
 		base = IDE_BASE;
-		ack_intr = macide_ack_intr;
 		irq = IRQ_NUBUS_C;
 		break;
 	case MAC_IDE_BABOON:
 		base = BABOON_BASE;
-		ack_intr = NULL;
 		d.port_ops = NULL;
 		irq = IRQ_BABOON_1;
 		break;
@@ -134,7 +130,7 @@ static int __init macide_init(void)
 	printk(KERN_INFO "ide: Macintosh %s IDE controller\n",
 			 mac_ide_name[macintosh_config->ide_type - 1]);
 
-	macide_setup_ports(&hw, base, irq, ack_intr);
+	macide_setup_ports(&hw, base, irq);
 
 	return ide_host_add(&d, hws, 1, NULL);
 }
Index: ide-2.6/drivers/ide/q40ide.c
===================================================================
--- ide-2.6.orig/drivers/ide/q40ide.c
+++ ide-2.6/drivers/ide/q40ide.c
@@ -51,9 +51,7 @@ static int q40ide_default_irq(unsigned l
 /*
  * Addresses are pretranslated for Q40 ISA access.
  */
-static void q40_ide_setup_ports(struct ide_hw *hw, unsigned long base,
-			ide_ack_intr_t *ack_intr,
-			int irq)
+static void q40_ide_setup_ports(struct ide_hw *hw, unsigned long base, int irq)
 {
 	memset(hw, 0, sizeof(*hw));
 	/* BIG FAT WARNING: 
@@ -69,7 +67,6 @@ static void q40_ide_setup_ports(struct i
 	hw->io_ports.ctl_addr = Q40_ISA_IO_B(base + 0x206);
 
 	hw->irq = irq;
-	hw->ack_intr = ack_intr;
 }
 
 static void q40ide_input_data(ide_drive_t *drive, struct ide_cmd *cmd,
@@ -156,7 +153,7 @@ static int __init q40ide_init(void)
 		release_region(pcide_bases[i], 8);
 		continue;
 	}
-	q40_ide_setup_ports(&hw[i], pcide_bases[i], NULL,
+	q40_ide_setup_ports(&hw[i], pcide_bases[i],
 			q40ide_default_irq(pcide_bases[i]));
 
 	hws[i] = &hw[i];
Index: ide-2.6/include/linux/ide.h
===================================================================
--- ide-2.6.orig/include/linux/ide.h
+++ ide-2.6/include/linux/ide.h
@@ -157,12 +157,6 @@ enum {
 #define REQ_UNPARK_HEADS	0x23
 
 /*
- * Check for an interrupt and acknowledge the interrupt status
- */
-struct hwif_s;
-typedef int (ide_ack_intr_t)(struct hwif_s *);
-
-/*
  * hwif_chipset_t is used to keep track of the specific hardware
  * chipset used by each IDE interface, if known.
  */
@@ -185,7 +179,6 @@ struct ide_hw {
 	};
 
 	int		irq;			/* our irq number */
-	ide_ack_intr_t	*ack_intr;		/* acknowledge interrupt */
 	struct device	*dev, *parent;
 	unsigned long	config;
 };
@@ -649,6 +642,7 @@ struct ide_port_ops {
 	void	(*maskproc)(ide_drive_t *, int);
 	void	(*quirkproc)(ide_drive_t *);
 	void	(*clear_irq)(ide_drive_t *);
+	int	(*test_irq)(struct hwif_s *);
 
 	u8	(*mdma_filter)(ide_drive_t *);
 	u8	(*udma_filter)(ide_drive_t *);
@@ -708,8 +702,6 @@ typedef struct hwif_s {
 
 	struct device *dev;
 
-	ide_ack_intr_t *ack_intr;
-
 	void (*rw_disk)(ide_drive_t *, struct request *);
 
 	const struct ide_tp_ops		*tp_ops;

Re: [PATCH 5/10] ide: move ack_intr() method into 'struct ide_port_ops' (take 2)

[Finn Thain]

On 13/06/09 17:31, Sergei Shtylyov wrote:
> Move the ack_intr() method into 'struct ide_port_ops', also renaming it to
> test_irq() while at it...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> ---
> The patch is atop of ide-2.6.git 'for-next' branch.
> 
> Changes since previous take:
> - added the missed changes to ide-probe.c, falconide.c, and q40ide.c;
> - fixed up buddha.c atop of the updated preceding patch;
> - fixed error in gayle_init() by adding missed &...
> 
>  drivers/ide/buddha.c    |   15 +++++++++------
>  drivers/ide/falconide.c |    1 -
>  drivers/ide/gayle.c     |   14 +++++++++-----
>  drivers/ide/ide-io.c    |    3 ++-
>  drivers/ide/ide-probe.c |    1 -
>  drivers/ide/macide.c    |   12 ++++--------
>  drivers/ide/q40ide.c    |    7 ++-----
>  include/linux/ide.h     |   10 +---------
>  8 files changed, 27 insertions(+), 36 deletions(-)
> 
> Index: ide-2.6/drivers/ide/buddha.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/buddha.c
> +++ ide-2.6/drivers/ide/buddha.c
> @@ -99,7 +99,7 @@ static const char *buddha_board_name[] =
>       *  Check and acknowledge the interrupt status
>       */
>  
> -static int buddha_ack_intr(ide_hwif_t *hwif)
> +static int buddha_test_irq(ide_hwif_t *hwif)
>  {
>      unsigned char ch;
>  
> @@ -118,8 +118,7 @@ static void xsurf_clear_irq(ide_drive_t 
>  }
>  
>  static void __init buddha_setup_ports(struct ide_hw *hw, unsigned long base,
> -				      unsigned long ctl, unsigned long irq_port,
> -				      ide_ack_intr_t *ack_intr)
> +				      unsigned long ctl, unsigned long irq_port)
>  {
>  	int i;
>  
> @@ -134,14 +133,19 @@ static void __init buddha_setup_ports(st
>  	hw->io_ports.irq_addr = irq_port;
>  
>  	hw->irq = IRQ_AMIGA_PORTS;
> -	hw->ack_intr = ack_intr;
>  }
>  
> +static const struct ide_port_ops buddha_port_ops = {
> +	.test_irq		= buddha_test_irq,
> +};
> +
>  static const struct ide_port_ops xsurf_port_ops = {
>  	.clear_irq		= xsurf_clear_irq,
> +	.test_irq		= buddha_test_irq,
>  };
>  
>  static const struct ide_port_info buddha_port_info = {
> +	.port_ops		= &buddha_port_ops,
>  	.host_flags		= IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA,
>  	.irq_flags		= IRQF_SHARED,
>  	.chipset		= ide_generic,
> @@ -217,8 +221,7 @@ fail_base2:
>  				irq_port = buddha_board + xsurf_irqports[i];
>  			}
>  
> -			buddha_setup_ports(&hw[i], base, ctl, irq_port,
> -					   buddha_ack_intr);
> +			buddha_setup_ports(&hw[i], base, ctl, irq_port);
>  
>  			hws[i] = &hw[i];
>  		}
> Index: ide-2.6/drivers/ide/falconide.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/falconide.c
> +++ ide-2.6/drivers/ide/falconide.c
> @@ -128,7 +128,6 @@ static void __init falconide_setup_ports
>  	hw->io_ports.ctl_addr = ATA_HD_BASE + ATA_HD_CONTROL;
>  
>  	hw->irq = IRQ_MFP_IDE;
> -	hw->ack_intr = NULL;
>  }
>  
>      /*
> Index: ide-2.6/drivers/ide/gayle.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/gayle.c
> +++ ide-2.6/drivers/ide/gayle.c
> @@ -66,7 +66,7 @@ MODULE_PARM_DESC(doubler, "enable suppor
>       *  Check and acknowledge the interrupt status
>       */
>  
> -static int gayle_ack_intr(ide_hwif_t *hwif)
> +static int gayle_test_irq(ide_hwif_t *hwif)
>  {
>      unsigned char ch;
>  
> @@ -85,8 +85,7 @@ static void gayle_a1200_clear_irq(ide_dr
>  }
>  
>  static void __init gayle_setup_ports(struct ide_hw *hw, unsigned long base,
> -				     unsigned long ctl, unsigned long irq_port,
> -				     ide_ack_intr_t *ack_intr)
> +				     unsigned long ctl, unsigned long irq_port)
>  {
>  	int i;
>  
> @@ -101,11 +100,15 @@ static void __init gayle_setup_ports(str
>  	hw->io_ports.irq_addr = irq_port;
>  
>  	hw->irq = IRQ_AMIGA_PORTS;
> -	hw->ack_intr = ack_intr;
>  }
>  
> +static const struct ide_port_ops gayle_a4000_port_ops = {
> +	.test_irq		= gayle_test_irq,
> +};
> +
>  static const struct ide_port_ops gayle_a1200_port_ops = {
>  	.clear_irq		= gayle_a1200_clear_irq,
> +	.test_irq		= gayle_test_irq,
>  };
>  
>  static const struct ide_port_info gayle_port_info = {
> @@ -148,6 +151,7 @@ found:
>  	if (a4000) {
>  	    phys_base = GAYLE_BASE_4000;
>  	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_4000);
> +	    d.port_ops = &gayle_a4000_port_ops;
>  	} else {
>  	    phys_base = GAYLE_BASE_1200;
>  	    irqport = (unsigned long)ZTWO_VADDR(GAYLE_IRQ_1200);
> @@ -164,7 +168,7 @@ found:
>  	base = (unsigned long)ZTWO_VADDR(phys_base + i * GAYLE_NEXT_PORT);
>  	ctrlport = GAYLE_HAS_CONTROL_REG ? (base + GAYLE_CONTROL) : 0;
>  
> -	gayle_setup_ports(&hw[i], base, ctrlport, irqport, gayle_ack_intr);
> +	gayle_setup_ports(&hw[i], base, ctrlport, irqport);
>  
>  	hws[i] = &hw[i];
>      }
> Index: ide-2.6/drivers/ide/ide-io.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/ide-io.c
> +++ ide-2.6/drivers/ide/ide-io.c
> @@ -789,7 +789,8 @@ irqreturn_t ide_intr (int irq, void *dev
>  
>  	spin_lock_irqsave(&hwif->lock, flags);
>  
> -	if (hwif->ack_intr && hwif->ack_intr(hwif) == 0)
> +	if (hwif->port_ops && hwif->port_ops->test_irq &&
> +	    hwif->port_ops->test_irq(hwif) == 0)

Wouldn't that be 

+     if (hwif->port_ops && hwif->port_ops->test_irq(hwif) &&
+         hwif->port_ops->clear_irq(hwif) == 0)

(this was never compiled?)

>  		goto out;
>  
>  	handler = hwif->handler;
> Index: ide-2.6/drivers/ide/ide-probe.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/ide-probe.c
> +++ ide-2.6/drivers/ide/ide-probe.c
> @@ -1170,7 +1170,6 @@ static void ide_init_port_hw(ide_hwif_t 
>  	hwif->irq = hw->irq;
>  	hwif->dev = hw->dev;
>  	hwif->gendev.parent = hw->parent ? hw->parent : hw->dev;
> -	hwif->ack_intr = hw->ack_intr;
>  	hwif->config_data = hw->config;
>  }
>  
> Index: ide-2.6/drivers/ide/macide.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/macide.c
> +++ ide-2.6/drivers/ide/macide.c
> @@ -53,7 +53,7 @@
>  
>  volatile unsigned char *ide_ifr = (unsigned char *) (IDE_BASE + IDE_IFR);
>  
> -int macide_ack_intr(ide_hwif_t* hwif)
> +int macide_test_irq(ide_hwif_t *hwif)
>  {
>  	if (*ide_ifr & 0x20)
>  		return 1;
> @@ -66,7 +66,7 @@ static void macide_clear_irq(ide_drive_t
>  }
>  
>  static void __init macide_setup_ports(struct ide_hw *hw, unsigned long base,
> -				      int irq, ide_ack_intr_t *ack_intr)
> +				      int irq)
>  {
>  	int i;
>  
> @@ -78,11 +78,11 @@ static void __init macide_setup_ports(st
>  	hw->io_ports.ctl_addr = base + IDE_CONTROL;
>  
>  	hw->irq = irq;
> -	hw->ack_intr = ack_intr;
>  }
>  
>  static const struct ide_port_ops macide_port_ops = {
>  	.clear_irq		= macide_clear_irq,
> +	.test_irq		= macide_test_irq,
>  };
>  
>  static const struct ide_port_info macide_port_info = {
> @@ -101,7 +101,6 @@ static const char *mac_ide_name[] =
>  
>  static int __init macide_init(void)
>  {
> -	ide_ack_intr_t *ack_intr;
>  	unsigned long base;
>  	int irq;
>  	struct ide_hw hw, *hws[] = { &hw };
> @@ -113,17 +112,14 @@ static int __init macide_init(void)
>  	switch (macintosh_config->ide_type) {
>  	case MAC_IDE_QUADRA:
>  		base = IDE_BASE;
> -		ack_intr = macide_ack_intr;
>  		irq = IRQ_NUBUS_F;
>  		break;
>  	case MAC_IDE_PB:
>  		base = IDE_BASE;
> -		ack_intr = macide_ack_intr;
>  		irq = IRQ_NUBUS_C;
>  		break;
>  	case MAC_IDE_BABOON:
>  		base = BABOON_BASE;
> -		ack_intr = NULL;

I think you mean:

	case MAC_IDE_BABOON:
		base = BABOON_BASE;
-		ack_intr = NULL;
+		macide_port_ops.clear_irq = NULL;

Why is this broken into two patches? Bisecting in between doesn't work.

Finn


>  		d.port_ops = NULL;
>  		irq = IRQ_BABOON_1;
>  		break;
> @@ -134,7 +130,7 @@ static int __init macide_init(void)
>  	printk(KERN_INFO "ide: Macintosh %s IDE controller\n",
>  			 mac_ide_name[macintosh_config->ide_type - 1]);
>  
> -	macide_setup_ports(&hw, base, irq, ack_intr);
> +	macide_setup_ports(&hw, base, irq);
>  
>  	return ide_host_add(&d, hws, 1, NULL);
>  }
> Index: ide-2.6/drivers/ide/q40ide.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/q40ide.c
> +++ ide-2.6/drivers/ide/q40ide.c
> @@ -51,9 +51,7 @@ static int q40ide_default_irq(unsigned l
>  /*
>   * Addresses are pretranslated for Q40 ISA access.
>   */
> -static void q40_ide_setup_ports(struct ide_hw *hw, unsigned long base,
> -			ide_ack_intr_t *ack_intr,
> -			int irq)
> +static void q40_ide_setup_ports(struct ide_hw *hw, unsigned long base, int irq)
>  {
>  	memset(hw, 0, sizeof(*hw));
>  	/* BIG FAT WARNING: 
> @@ -69,7 +67,6 @@ static void q40_ide_setup_ports(struct i
>  	hw->io_ports.ctl_addr = Q40_ISA_IO_B(base + 0x206);
>  
>  	hw->irq = irq;
> -	hw->ack_intr = ack_intr;
>  }
>  
>  static void q40ide_input_data(ide_drive_t *drive, struct ide_cmd *cmd,
> @@ -156,7 +153,7 @@ static int __init q40ide_init(void)
>  		release_region(pcide_bases[i], 8);
>  		continue;
>  	}
> -	q40_ide_setup_ports(&hw[i], pcide_bases[i], NULL,
> +	q40_ide_setup_ports(&hw[i], pcide_bases[i],
>  			q40ide_default_irq(pcide_bases[i]));
>  
>  	hws[i] = &hw[i];
> Index: ide-2.6/include/linux/ide.h
> ===================================================================
> --- ide-2.6.orig/include/linux/ide.h
> +++ ide-2.6/include/linux/ide.h
> @@ -157,12 +157,6 @@ enum {
>  #define REQ_UNPARK_HEADS	0x23
>  
>  /*
> - * Check for an interrupt and acknowledge the interrupt status
> - */
> -struct hwif_s;
> -typedef int (ide_ack_intr_t)(struct hwif_s *);
> -
> -/*
>   * hwif_chipset_t is used to keep track of the specific hardware
>   * chipset used by each IDE interface, if known.
>   */
> @@ -185,7 +179,6 @@ struct ide_hw {
>  	};
>  
>  	int		irq;			/* our irq number */
> -	ide_ack_intr_t	*ack_intr;		/* acknowledge interrupt */
>  	struct device	*dev, *parent;
>  	unsigned long	config;
>  };
> @@ -649,6 +642,7 @@ struct ide_port_ops {
>  	void	(*maskproc)(ide_drive_t *, int);
>  	void	(*quirkproc)(ide_drive_t *);
>  	void	(*clear_irq)(ide_drive_t *);
> +	int	(*test_irq)(struct hwif_s *);
>  
>  	u8	(*mdma_filter)(ide_drive_t *);
>  	u8	(*udma_filter)(ide_drive_t *);
> @@ -708,8 +702,6 @@ typedef struct hwif_s {
>  
>  	struct device *dev;
>  
> -	ide_ack_intr_t *ack_intr;
> -
>  	void (*rw_disk)(ide_drive_t *, struct request *);
>  
>  	const struct ide_tp_ops		*tp_ops;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/10] ide: move ack_intr() method into 'struct ide_port_ops' (take 2)

[Finn Thain]

On 14/06/09 02:15, Finn Thain wrote:
> 
> > +++ ide-2.6/drivers/ide/ide-io.c
> > @@ -789,7 +789,8 @@ irqreturn_t ide_intr (int irq, void *dev
> >  
> >  	spin_lock_irqsave(&hwif->lock, flags);
> >  
> > -	if (hwif->ack_intr && hwif->ack_intr(hwif) == 0)
> > +	if (hwif->port_ops && hwif->port_ops->test_irq &&
> > +	    hwif->port_ops->test_irq(hwif) == 0)
> 
> Wouldn't that be 
> 
> +     if (hwif->port_ops && hwif->port_ops->test_irq(hwif) &&
> +         hwif->port_ops->clear_irq(hwif) == 0)
> 
> (this was never compiled?)

My bad.
(Note to self: don't try to quickly understand patches at 2am.)

> >  	case MAC_IDE_BABOON:
> >  		base = BABOON_BASE;
> > -		ack_intr = NULL;
> 
> I think you mean:
> 
> 	case MAC_IDE_BABOON:
> 		base = BABOON_BASE;
> -		ack_intr = NULL;
> +		macide_port_ops.clear_irq = NULL;
> 
> Why is this broken into two patches? Bisecting in between doesn't work.

Same here.

Sorry for the noise.

Finn

[PATCH 6/10] cmd640: implement test_irq() method

[Sergei Shtylyov]

Implement test_irq() method, adding the drive 2/3 interrupt bit definition.

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

---
This is based on PCI0640B datasheet.
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/cmd640.c |   13 +++++++++++++
 1 files changed, 13 insertions(+)

Index: ide-2.6/drivers/ide/cmd640.c
===================================================================
--- ide-2.6.orig/drivers/ide/cmd640.c
+++ ide-2.6/drivers/ide/cmd640.c
@@ -153,6 +153,7 @@ static int cmd640_vlb;
 #define ARTTIM23	0x57
 #define   ARTTIM23_DIS_RA2	0x04
 #define   ARTTIM23_DIS_RA3	0x08
+#define   ARTTIM23_IDE23INTR	0x10
 #define DRWTIM23	0x58
 #define BRST		0x59
 
@@ -629,12 +630,24 @@ static void cmd640_init_dev(ide_drive_t 
 #endif /* CONFIG_BLK_DEV_CMD640_ENHANCED */
 }
 
+static int cmd640_test_irq(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	int irq_reg		= hwif->channel ? ARTTIM23 : CFR;
+	u8  irq_stat, irq_mask	= hwif->channel ? ARTTIM23_IDE23INTR :
+						  CFR_IDE01INTR;
+
+	pci_read_config_byte(dev, irq_reg, &irq_stat);
+
+	return (irq_stat & irq_mask) ? 1 : 0;
+}
 
 static const struct ide_port_ops cmd640_port_ops = {
 	.init_dev		= cmd640_init_dev,
 #ifdef CONFIG_BLK_DEV_CMD640_ENHANCED
 	.set_pio_mode		= cmd640_set_pio_mode,
 #endif
+	.test_irq		= cmd640_test_irq,
 };
 
 static int pci_conf1(void)

[PATCH 7/10] cmd64x: implement test_irq() method

[Sergei Shtylyov]

Convert the driver's two dma_test_irq() methods into test_irq() methods. The
driver will now use the standard dma_test_irq() method implementation which
allows to remove 'cmd54x_dma_ops' and 'cmd648_dma_ops' that become identical
to 'sff_dma_ops'...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/cmd64x.c |   69 ++++++++-------------------------------------------
 1 files changed, 12 insertions(+), 57 deletions(-)

Index: ide-2.6/drivers/ide/cmd64x.c
===================================================================
--- ide-2.6.orig/drivers/ide/cmd64x.c
+++ ide-2.6/drivers/ide/cmd64x.c
@@ -253,53 +253,34 @@ static void cmd64x_clear_irq(ide_drive_t
 	(void) pci_write_config_byte(dev, irq_reg, irq_stat | irq_mask);
 }
 
-static int cmd648_dma_test_irq(ide_drive_t *drive)
+static int cmd648_test_irq(ide_hwif_t *hwif)
 {
-	ide_hwif_t *hwif	= drive->hwif;
-	unsigned long base	= hwif->dma_base - (hwif->channel * 8);
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	unsigned long base	= pci_resource_start(dev, 4);
 	u8 irq_mask		= hwif->channel ? MRDMODE_INTR_CH1 :
 						  MRDMODE_INTR_CH0;
-	u8 dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 	u8 mrdmode		= inb(base + 1);
 
-#ifdef DEBUG
-	printk("%s: dma_stat: 0x%02x mrdmode: 0x%02x irq_mask: 0x%02x\n",
-	       drive->name, dma_stat, mrdmode, irq_mask);
-#endif
-	if (!(mrdmode & irq_mask))
-		return 0;
-
-	/* return 1 if INTR asserted */
-	if (dma_stat & 4)
-		return 1;
+	pr_debug("%s: mrdmode: 0x%02x irq_mask: 0x%02x\n",
+		 hwif->name, mrdmode, irq_mask);
 
-	return 0;
+	return (mrdmode & irq_mask) ? 1 : 0;
 }
 
-static int cmd64x_dma_test_irq(ide_drive_t *drive)
+static int cmd64x_test_irq(ide_hwif_t *hwif)
 {
-	ide_hwif_t *hwif	= drive->hwif;
 	struct pci_dev *dev	= to_pci_dev(hwif->dev);
 	int irq_reg		= hwif->channel ? ARTTIM23 : CFR;
 	u8  irq_mask		= hwif->channel ? ARTTIM23_INTR_CH1 :
 						  CFR_INTR_CH0;
-	u8  dma_stat		= inb(hwif->dma_base + ATA_DMA_STATUS);
 	u8  irq_stat		= 0;
 
 	(void) pci_read_config_byte(dev, irq_reg, &irq_stat);
 
-#ifdef DEBUG
-	printk("%s: dma_stat: 0x%02x irq_stat: 0x%02x irq_mask: 0x%02x\n",
-	       drive->name, dma_stat, irq_stat, irq_mask);
-#endif
-	if (!(irq_stat & irq_mask))
-		return 0;
-
-	/* return 1 if INTR asserted */
-	if (dma_stat & 4)
-		return 1;
+	pr_debug("%s: irq_stat: 0x%02x irq_mask: 0x%02x\n",
+		 hwif->name, irq_stat, irq_mask);
 
-	return 0;
+	return (irq_stat & irq_mask) ? 1 : 0;
 }
 
 /*
@@ -365,6 +346,7 @@ static const struct ide_port_ops cmd64x_
 	.set_pio_mode		= cmd64x_set_pio_mode,
 	.set_dma_mode		= cmd64x_set_dma_mode,
 	.clear_irq		= cmd64x_clear_irq,
+	.test_irq		= cmd64x_test_irq,
 	.cable_detect		= cmd64x_cable_detect,
 };
 
@@ -372,20 +354,10 @@ static const struct ide_port_ops cmd648_
 	.set_pio_mode		= cmd64x_set_pio_mode,
 	.set_dma_mode		= cmd64x_set_dma_mode,
 	.clear_irq		= cmd648_clear_irq,
+	.test_irq		= cmd648_test_irq,
 	.cable_detect		= cmd64x_cable_detect,
 };
 
-static const struct ide_dma_ops cmd64x_dma_ops = {
-	.dma_host_set		= ide_dma_host_set,
-	.dma_setup		= ide_dma_setup,
-	.dma_start		= ide_dma_start,
-	.dma_end		= ide_dma_end,
-	.dma_test_irq		= cmd64x_dma_test_irq,
-	.dma_lost_irq		= ide_dma_lost_irq,
-	.dma_timer_expiry	= ide_dma_sff_timer_expiry,
-	.dma_sff_read_status	= ide_dma_sff_read_status,
-};
-
 static const struct ide_dma_ops cmd646_rev1_dma_ops = {
 	.dma_host_set		= ide_dma_host_set,
 	.dma_setup		= ide_dma_setup,
@@ -397,24 +369,12 @@ static const struct ide_dma_ops cmd646_r
 	.dma_sff_read_status	= ide_dma_sff_read_status,
 };
 
-static const struct ide_dma_ops cmd648_dma_ops = {
-	.dma_host_set		= ide_dma_host_set,
-	.dma_setup		= ide_dma_setup,
-	.dma_start		= ide_dma_start,
-	.dma_end		= ide_dma_end,
-	.dma_test_irq		= cmd648_dma_test_irq,
-	.dma_lost_irq		= ide_dma_lost_irq,
-	.dma_timer_expiry	= ide_dma_sff_timer_expiry,
-	.dma_sff_read_status	= ide_dma_sff_read_status,
-};
-
 static const struct ide_port_info cmd64x_chipsets[] __devinitdata = {
 	{	/* 0: CMD643 */
 		.name		= DRV_NAME,
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x00,0x00,0x00}, {0x51,0x08,0x08}},
 		.port_ops	= &cmd64x_port_ops,
-		.dma_ops	= &cmd64x_dma_ops,
 		.host_flags	= IDE_HFLAG_CLEAR_SIMPLEX |
 				  IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
@@ -426,7 +386,6 @@ static const struct ide_port_info cmd64x
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
 		.port_ops	= &cmd648_port_ops,
-		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_SERIALIZE |
 				  IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
@@ -438,7 +397,6 @@ static const struct ide_port_info cmd64x
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
 		.port_ops	= &cmd648_port_ops,
-		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
 		.mwdma_mask	= ATA_MWDMA2,
@@ -449,7 +407,6 @@ static const struct ide_port_info cmd64x
 		.init_chipset	= init_chipset_cmd64x,
 		.enablebits	= {{0x51,0x04,0x04}, {0x51,0x08,0x08}},
 		.port_ops	= &cmd648_port_ops,
-		.dma_ops	= &cmd648_dma_ops,
 		.host_flags	= IDE_HFLAG_ABUSE_PREFETCH,
 		.pio_mask	= ATA_PIO5,
 		.mwdma_mask	= ATA_MWDMA2,
@@ -489,8 +446,6 @@ static int __devinit cmd64x_init_one(str
 				d.port_ops = &cmd64x_port_ops;
 				if (dev->revision == 1)
 					d.dma_ops = &cmd646_rev1_dma_ops;
-				else
-					d.dma_ops = &cmd64x_dma_ops;
 			}
 		}
 	}

[PATCH 8/10] pdc202xx_old: implement test_irq() method (take 2)

[Sergei Shtylyov]

Implement test_irq() method based on the driver's former dma_test_irq() method.

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

---
The patch is atop of ide-2.6.git 'for-next' branch.
Hopefully it won't break the driver -- previosly the bits have been effectively
ignored...

Changes sice previos take:
- decided to document the register bits...

 drivers/ide/pdc202xx_old.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+)

Index: ide-2.6/drivers/ide/pdc202xx_old.c
===================================================================
--- ide-2.6.orig/drivers/ide/pdc202xx_old.c
+++ ide-2.6/drivers/ide/pdc202xx_old.c
@@ -104,6 +104,27 @@ static void pdc202xx_set_pio_mode(ide_dr
 	pdc202xx_set_mode(drive, XFER_PIO_0 + pio);
 }
 
+static int pdc202xx_test_irq(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	unsigned long high_16	= pci_resource_start(dev, 4);
+	u8 sc1d			= inb(high_16 + 0x1d);
+
+	if (hwif->channel) {
+		/*
+		 * bit 7: error, bit 6: interrupting,
+		 * bit 5: FIFO full, bit 4: FIFO empty
+		 */
+		return ((sc1d & 0x50) == 0x40) ? 1 : 0;
+	} else	{
+		/*
+		 * bit 3: error, bit 2: interrupting,
+		 * bit 1: FIFO full, bit 0: FIFO empty
+		 */
+		return ((sc1d & 0x05) == 0x04) ? 1 : 0;
+	}
+}
+
 static u8 pdc2026x_cable_detect(ide_hwif_t *hwif)
 {
 	struct pci_dev *dev = to_pci_dev(hwif->dev);
@@ -231,6 +252,7 @@ static void __devinit pdc202ata4_fixup_i
 static const struct ide_port_ops pdc20246_port_ops = {
 	.set_pio_mode		= pdc202xx_set_pio_mode,
 	.set_dma_mode		= pdc202xx_set_mode,
+	.test_irq		= pdc202xx_test_irq,
 };
 
 static const struct ide_port_ops pdc2026x_port_ops = {

Re: [PATCH 8/10] pdc202xx_old: implement test_irq() method (take 2)

[Sergei Shtylyov]

Hello, I wrote:

> Implement test_irq() method based on the driver's former dma_test_irq() method.
>
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>
> ---
> The patch is atop of ide-2.6.git 'for-next' branch.
> Hopefully it won't break the driver -- previosly the bits have been effectively
> ignored...
>
> Changes sice previos take:
> - decided to document the register bits...
>
>  drivers/ide/pdc202xx_old.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+)
>
> Index: ide-2.6/drivers/ide/pdc202xx_old.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/pdc202xx_old.c
> +++ ide-2.6/drivers/ide/pdc202xx_old.c
>   
[...]
> @@ -231,6 +252,7 @@ static void __devinit pdc202ata4_fixup_i
>  static const struct ide_port_ops pdc20246_port_ops = {
>  	.set_pio_mode		= pdc202xx_set_pio_mode,
>  	.set_dma_mode		= pdc202xx_set_mode,
> +	.test_irq		= pdc202xx_test_irq,
>  };
>  
>  static const struct ide_port_ops pdc2026x_port_ops = {
>   

   Ugh, I forgot to modify this structure, it seems... no wonder Russell 
has emphasized that the bug he's fixed only concerns PDC2024x. :-)

WBR, Sergei

[PATCH 9/10] siimage: implement test_irq() method

[Sergei Shtylyov]

Implement test_irq() method.  Remove interrupt bit test (not trusted anyway)
from siimage_io_dma_test_irq() and siimage_mmio_dma_test_irq(); this allows to
replace the former function with now identical ide_dma_test_irq()...

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/siimage.c |   26 +++++++-------------------
 1 files changed, 7 insertions(+), 19 deletions(-)

Index: ide-2.6/drivers/ide/siimage.c
===================================================================
--- ide-2.6.orig/drivers/ide/siimage.c
+++ ide-2.6/drivers/ide/siimage.c
@@ -337,24 +337,14 @@ static void sil_set_dma_mode(ide_drive_t
 	sil_iowrite16(dev, ultra, ua);
 }
 
-/* returns 1 if dma irq issued, 0 otherwise */
-static int siimage_io_dma_test_irq(ide_drive_t *drive)
+static int sil_test_irq(ide_hwif_t *hwif)
 {
-	ide_hwif_t *hwif	= drive->hwif;
 	struct pci_dev *dev	= to_pci_dev(hwif->dev);
-	u8 dma_altstat		= 0;
 	unsigned long addr	= siimage_selreg(hwif, 1);
+	u8 val			= sil_ioread8(dev, addr);
 
-	/* return 1 if INTR asserted */
-	if (inb(hwif->dma_base + ATA_DMA_STATUS) & 4)
-		return 1;
-
-	/* return 1 if Device INTR asserted */
-	pci_read_config_byte(dev, addr, &dma_altstat);
-	if (dma_altstat & 8)
-		return 0;	/* return 1; */
-
-	return 0;
+	/* Return 1 if INTRQ asserted */
+	return (val & 8) ? 1 : 0;
 }
 
 /**
@@ -397,10 +387,6 @@ static int siimage_mmio_dma_test_irq(ide
 	if (readb((void __iomem *)(hwif->dma_base + ATA_DMA_STATUS)) & 4)
 		return 1;
 
-	/* return 1 if Device INTR asserted */
-	if (readb((void __iomem *)addr) & 8)
-		return 0;	/* return 1; */
-
 	return 0;
 }
 
@@ -409,7 +395,7 @@ static int siimage_dma_test_irq(ide_driv
 	if (drive->hwif->host_flags & IDE_HFLAG_MMIO)
 		return siimage_mmio_dma_test_irq(drive);
 	else
-		return siimage_io_dma_test_irq(drive);
+		return ide_dma_test_irq(drive);
 }
 
 /**
@@ -694,6 +680,7 @@ static const struct ide_port_ops sil_pat
 	.set_pio_mode		= sil_set_pio_mode,
 	.set_dma_mode		= sil_set_dma_mode,
 	.quirkproc		= sil_quirkproc,
+	.test_irq		= sil_test_irq,
 	.udma_filter		= sil_pata_udma_filter,
 	.cable_detect		= sil_cable_detect,
 };
@@ -704,6 +691,7 @@ static const struct ide_port_ops sil_sat
 	.reset_poll		= sil_sata_reset_poll,
 	.pre_reset		= sil_sata_pre_reset,
 	.quirkproc		= sil_quirkproc,
+	.test_irq		= sil_test_irq,
 	.udma_filter		= sil_sata_udma_filter,
 	.cable_detect		= sil_cable_detect,
 };

[PATCH 10/10] sl82c105: implement test_irq() method

[Sergei Shtylyov]

Implement test_irq() method.

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

---
The patch is atop of ide-2.6.git 'for-next' branch.

 drivers/ide/sl82c105.c |   11 +++++++++++
 1 files changed, 11 insertions(+)

Index: ide-2.6/drivers/ide/sl82c105.c
===================================================================
--- ide-2.6.orig/drivers/ide/sl82c105.c
+++ ide-2.6/drivers/ide/sl82c105.c
@@ -115,6 +115,16 @@ static void sl82c105_set_dma_mode(ide_dr
 	drive->drive_data |= (unsigned long)drv_ctrl << 16;
 }
 
+static int sl82c105_test_irq(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	u32 val, mask		= hwif->channel ? CTRL_IDE_IRQB : CTRL_IDE_IRQA;
+
+	pci_read_config_dword(dev, 0x40, &val);
+
+	return (val & mask) ? 1 : 0;
+}
+
 /*
  * The SL82C105 holds off all IDE interrupts while in DMA mode until
  * all DMA activity is completed.  Sometimes this causes problems (eg,
@@ -289,6 +299,7 @@ static const struct ide_port_ops sl82c10
 	.set_pio_mode		= sl82c105_set_pio_mode,
 	.set_dma_mode		= sl82c105_set_dma_mode,
 	.resetproc		= sl82c105_resetproc,
+	.test_irq		= sl82c105_test_irq,
 };
 
 static const struct ide_dma_ops sl82c105_dma_ops = {

[PATCH] hpt366: kill unused #define's

[Sergei Shtylyov]

These two have been long unused but I've just noticed...

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

---
The patch is against ide-2.6.git tree...

 drivers/ide/hpt366.c |    2 --
 1 files changed, 2 deletions(-)

Index: ide-2.6/drivers/ide/hpt366.c
===================================================================
--- ide-2.6.orig/drivers/ide/hpt366.c
+++ ide-2.6/drivers/ide/hpt366.c
@@ -426,14 +426,12 @@ static u32 sixty_six_base_hpt37x[] = {
 };
 #endif
 
-#define HPT366_DEBUG_DRIVE_INFO		0
 #define HPT371_ALLOW_ATA133_6		1
 #define HPT302_ALLOW_ATA133_6		1
 #define HPT372_ALLOW_ATA133_6		1
 #define HPT370_ALLOW_ATA100_5		0
 #define HPT366_ALLOW_ATA66_4		1
 #define HPT366_ALLOW_ATA66_3		1
-#define HPT366_MAX_DEVS			8
 
 /* Supported ATA clock frequencies */
 enum ata_clock {

Re: [PATCH] hpt366: kill unused #define's

[David Miller]

From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Date: Fri, 9 Oct 2009 17:36:39 +0400

> These two have been long unused but I've just noticed...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

Applied to ide-next-2.6

[PATCH] hpt366: add debounce delay to cable_detect() method

[Sergei Shtylyov]

Alan Cox reported that cable detection sometimes works unreliably
for HPT3xxN and that the issue is fixed by adding debounce delay
as used by the vendor drivers.

While at it, get rid of unneeded parens/space in the vicinity...

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: stable@kernel.org

---
The patcn is atop of ide-2.6.git tree...

 drivers/ide/hpt366.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

Index: ide-2.6/drivers/ide/hpt366.c
===================================================================
--- ide-2.6.orig/drivers/ide/hpt366.c
+++ ide-2.6/drivers/ide/hpt366.c
@@ -1236,8 +1236,9 @@ static u8 hpt3xx_cable_detect(ide_hwif_t
 		u16 mcr;
 
 		pci_read_config_word(dev, mcr_addr, &mcr);
-		pci_write_config_word(dev, mcr_addr, (mcr | 0x8000));
-		/* now read cable id register */
+		pci_write_config_word(dev, mcr_addr, mcr | 0x8000);
+		/* Debounce, then read cable ID register */
+		udelay(10);
 		pci_read_config_byte(dev, 0x5a, &scr1);
 		pci_write_config_word(dev, mcr_addr, mcr);
 	} else if (chip_type >= HPT370) {
@@ -1248,10 +1249,11 @@ static u8 hpt3xx_cable_detect(ide_hwif_t
 		u8 scr2 = 0;
 
 		pci_read_config_byte(dev, 0x5b, &scr2);
-		pci_write_config_byte(dev, 0x5b, (scr2 & ~1));
-		/* now read cable id register */
+		pci_write_config_byte(dev, 0x5b, scr2 & ~1);
+		/* Debounce, then read cable ID register */
+		udelay(10);
 		pci_read_config_byte(dev, 0x5a, &scr1);
-		pci_write_config_byte(dev, 0x5b,  scr2);
+		pci_write_config_byte(dev, 0x5b, scr2);
 	} else
 		pci_read_config_byte(dev, 0x5a, &scr1);
 

Re: [PATCH] hpt366: add debounce delay to cable_detect() method

[Alan Cox]

On Fri, 20 Nov 2009 22:52:22 +0400
Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:

> Alan Cox reported that cable detection sometimes works unreliably
> for HPT3xxN and that the issue is fixed by adding debounce delay
> as used by the vendor drivers.
> 
> While at it, get rid of unneeded parens/space in the vicinity...
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> Cc: stable@kernel.org

Acked-by: Alan Cox <alan@linux.intel.com>

Re: [PATCH] hpt366: add debounce delay to cable_detect() method

[Sergei Shtylyov]

Hello, I wrote:

> Alan Cox reported that cable detection sometimes works unreliably
> for HPT3xxN and that the issue is fixed by adding debounce delay
> as used by the vendor drivers.

> While at it, get rid of unneeded parens/space in the vicinity...

> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> Cc: stable@kernel.org

    David, will you (finally) apply this patch?

MBR, Sergei

Re: [PATCH] hpt366: add debounce delay to cable_detect() method

[Sergei Shtylyov]

Hello.

I wrote:

>> Alan Cox reported that cable detection sometimes works unreliably
>> for HPT3xxN and that the issue is fixed by adding debounce delay
>> as used by the vendor drivers.

>> While at it, get rid of unneeded parens/space in the vicinity...

>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>> Cc: stable@kernel.org

>    David, will you (finally) apply this patch?

    Well, nine months has passed, and I have to ask again -- are you going to at 
last apply it? :-/

MBR, Sergei

[PATCH] hpt366: fix clock turnaround

[Sergei Shtylyov]

DPLL clock (0x21) should be used for writes and PCI clock (0x23) for reads,
not vice versa.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: stable@kernel.org

---
The patch is atop of ide-2.6.git tree.

Not sure why I didn't fix this back in 2007... well, nobody has ever
complained. :-)

 drivers/ide/hpt366.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: ide-2.6/drivers/ide/hpt366.c
===================================================================
--- ide-2.6.orig/drivers/ide/hpt366.c
+++ ide-2.6/drivers/ide/hpt366.c
@@ -901,7 +901,7 @@ static void hpt3xxn_set_clock(ide_hwif_t
 
 static void hpt3xxn_rw_disk(ide_drive_t *drive, struct request *rq)
 {
-	hpt3xxn_set_clock(drive->hwif, rq_data_dir(rq) ? 0x23 : 0x21);
+	hpt3xxn_set_clock(drive->hwif, rq_data_dir(rq) ? 0x21 : 0x23);
 }
 
 /**

Re: [PATCH] hpt366: fix clock turnaround

[Sergei Shtylyov]

Hello.

I wrote:

> DPLL clock (0x21) should be used for writes and PCI clock (0x23) for reads,
> not vice versa.

> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> Cc: stable@kernel.org

> ---
> The patch is atop of ide-2.6.git tree.

> Not sure why I didn't fix this back in 2007... well, nobody has ever
> complained. :-)

>  drivers/ide/hpt366.c |    2 +-
>  1 files changed, 1 insertion(+), 1 deletion(-)
> 
> Index: ide-2.6/drivers/ide/hpt366.c
> ===================================================================
> --- ide-2.6.orig/drivers/ide/hpt366.c
> +++ ide-2.6/drivers/ide/hpt366.c
> @@ -901,7 +901,7 @@ static void hpt3xxn_set_clock(ide_hwif_t
>  
>  static void hpt3xxn_rw_disk(ide_drive_t *drive, struct request *rq)
>  {
> -	hpt3xxn_set_clock(drive->hwif, rq_data_dir(rq) ? 0x23 : 0x21);
> +	hpt3xxn_set_clock(drive->hwif, rq_data_dir(rq) ? 0x21 : 0x23);
>  }
>  
>  /**
> 

    David, looks like this patch posted 12/07/2009 got lost somewhere? I still 
don't see it applied...

WBR, Sergei

Re: [PATCH] hpt366: fix clock turnaround

[David Miller]

From: Sergei Shtylyov <sshtylyov@mvista.com>
Date: Sat, 25 Sep 2010 20:40:43 +0400

>    David, looks like this patch posted 12/07/2009 got lost somewhere? I
>    still don't see it applied...

Please submit your hpt366 patches from scratch again and I will apply
them immediately.

If the patches are in patchwork, which they will be if you submit them
fresh right now, I can process them efficiently.

Sorry for the delay.