* Rename ->ide_dma_clear_irq method to ->clear_irq
and move it from ide_hwif_t to struct ide_port_ops.
* Move ->waiting_for_dma check inside ->clear_irq method.
* Move ->dma_base check inside ->clear_irq method.
piix.c:
* Add ich_port_ops and remove init_hwif_ich() wrapper.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ide/ide-io.c | 15 ++++-----------
drivers/ide/pci/piix.c | 39 +++++++++++++++++++++++----------------
include/linux/ide.h | 4 ++--
3 files changed, 29 insertions(+), 29 deletions(-)
Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -1418,23 +1418,16 @@ irqreturn_t ide_intr (int irq, void *dev
del_timer(&hwgroup->timer);
spin_unlock(&ide_lock);
- /* Some controllers might set DMA INTR no matter DMA or PIO;
- * bmdma status might need to be cleared even for
- * PIO interrupts to prevent spurious/lost irq.
- */
- if (hwif->ide_dma_clear_irq && !(drive->waiting_for_dma))
- /* ide_dma_end() needs bmdma status for error checking.
- * So, skip clearing bmdma status here and leave it
- * to ide_dma_end() if this is dma interrupt.
- */
- hwif->ide_dma_clear_irq(drive);
+ 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();
+
/* service this interrupt, may set handler for next interrupt */
startstop = handler(drive);
- spin_lock_irq(&ide_lock);
+ spin_lock_irq(&ide_lock);
/*
* Note that handler() may have set things up for another
* interrupt to occur soon, but it cannot happen until
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -215,17 +215,26 @@ static unsigned int init_chipset_ich(str
}
/**
- * piix_dma_clear_irq - clear BMDMA status
- * @drive: IDE drive to clear
+ * ich_clear_irq - clear BMDMA status
+ * @drive: IDE drive
*
- * Called from ide_intr() for PIO interrupts
- * to clear BMDMA status as needed by ICHx
+ * ICHx contollers set DMA INTR no matter DMA or PIO.
+ * BMDMA status might need to be cleared even for
+ * PIO interrupts to prevent spurious/lost IRQ.
*/
-static void piix_dma_clear_irq(ide_drive_t *drive)
+static void ich_clear_irq(ide_drive_t *drive)
{
ide_hwif_t *hwif = HWIF(drive);
u8 dma_stat;
+ /*
+ * ide_dma_end() needs BMDMA status for error checking.
+ * So, skip clearing BMDMA status here and leave it
+ * to ide_dma_end() if this is DMA interrupt.
+ */
+ if (drive->waiting_for_dma || hwif->dma_base == 0)
+ return;
+
/* clear the INTR & ERROR bits */
dma_stat = inb(hwif->dma_base + ATA_DMA_STATUS);
/* Should we force the bit as well ? */
@@ -293,21 +302,19 @@ static void __devinit init_hwif_piix(ide
hwif->ultra_mask = hwif->mwdma_mask = hwif->swdma_mask = 0;
}
-static void __devinit init_hwif_ich(ide_hwif_t *hwif)
-{
- init_hwif_piix(hwif);
-
- /* ICHx need to clear the BMDMA status for all interrupts */
- if (hwif->dma_base)
- hwif->ide_dma_clear_irq = &piix_dma_clear_irq;
-}
-
static const struct ide_port_ops piix_port_ops = {
.set_pio_mode = piix_set_pio_mode,
.set_dma_mode = piix_set_dma_mode,
.cable_detect = piix_cable_detect,
};
+static const struct ide_port_ops ich_port_ops = {
+ .set_pio_mode = piix_set_pio_mode,
+ .set_dma_mode = piix_set_dma_mode,
+ .clear_irq = ich_clear_irq,
+ .cable_detect = piix_cable_detect,
+};
+
#ifndef CONFIG_IA64
#define IDE_HFLAGS_PIIX IDE_HFLAG_LEGACY_IRQS
#else
@@ -331,9 +338,9 @@ static const struct ide_port_ops piix_po
{ \
.name = DRV_NAME, \
.init_chipset = init_chipset_ich, \
- .init_hwif = init_hwif_ich, \
+ .init_hwif = init_hwif_piix, \
.enablebits = {{0x41,0x80,0x80}, {0x43,0x80,0x80}}, \
- .port_ops = &piix_port_ops, \
+ .port_ops = &ich_port_ops, \
.host_flags = IDE_HFLAGS_PIIX, \
.pio_mask = ATA_PIO4, \
.swdma_mask = ATA_SWDMA2_ONLY, \
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -702,6 +702,7 @@ extern const struct ide_tp_ops default_t
* @resetproc: routine to reset controller after a disk reset
* @maskproc: special host masking for drive selection
* @quirkproc: check host's drive quirk list
+ * @clear_irq: clear IRQ
*
* @mdma_filter: filter MDMA modes
* @udma_filter: filter UDMA modes
@@ -718,6 +719,7 @@ struct ide_port_ops {
void (*resetproc)(ide_drive_t *);
void (*maskproc)(ide_drive_t *, int);
void (*quirkproc)(ide_drive_t *);
+ void (*clear_irq)(ide_drive_t *);
u8 (*mdma_filter)(ide_drive_t *);
u8 (*udma_filter)(ide_drive_t *);
@@ -780,8 +782,6 @@ typedef struct hwif_s {
const struct ide_port_ops *port_ops;
const struct ide_dma_ops *dma_ops;
- void (*ide_dma_clear_irq)(ide_drive_t *drive);
-
/* dma physical region descriptor table (cpu view) */
unsigned int *dmatable_cpu;
/* dma physical region descriptor table (dma view) */
Hello.
Bartlomiej Zolnierkiewicz wrote:
> * Rename ->ide_dma_clear_irq method to ->clear_irq
> and move it from ide_hwif_t to struct ide_port_ops.
>
> * Move ->waiting_for_dma check inside ->clear_irq method.
>
> * Move ->dma_base check inside ->clear_irq method.
>
> piix.c:
> * Add ich_port_ops and remove init_hwif_ich() wrapper.
>
> There should be no functional changes caused by this patch.
>
Good. I think it's worth implementing this method in at least
cmd64x.c which actually reads the IDE interrupt latch bits (independent
from the DMA interrupt status) in the dma_test_irq() methods but never
clears them, so the latches may reflect a non-current state of the IDE
interrupt...
It may also be worth considering turning this method into
test-and-clear, so that we can get the actual IDE interrupt state on the
chips that implement this...
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by: Sergei Shtylyov <[email protected]>
MBR, Sergei
On Tue, Sep 16, 2008 at 01:16:54PM +0400, Sergei Shtylyov wrote:
> >>>>Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> >>>>
> >>>Acked-by: Sergei Shtylyov <[email protected]>
> >>>
> >> Not feeling sure about this patch -- ->waiting_for_dma probably
> >>should've been left where it was...
> >>
> >
> >Well, it doesn't change behavior and I think having ->clear_irq method
> >independent from the transfer mode is a preffered approach.
> >
>
> But its implementations will have to depend on it anyway. And
> clearing the IDE interrupt in general already depends on the transfer
> mode -- the BMIDE interrupt which is a (delayed) reflection of INTRQ is
> cleared implicitly by the dma_end() method -- except in at least sgiioc4
> driver.
> BTW, sgiioc4 seems another candidate for clear_irq() implementation
> -- currently clearing is done implicitly by the read_status() method (I
> don't quite understand why it clears DMA error interrupt there).
> However, since there's no documentation, I'm not sure how the IDE
> interrupt is latched by IOC4.
The person who originally wrote sgiioc4 has been gone for a long time.
I'm guessing that the DMA error is cleared there, because any DMA error
status has already been processed above, and it's easier to clear
unconditionally than to test and clear, and there's no ill effect to
clearing something that's already clear.
jeremy