ide: ->ide_dma_clear_irq() -> ->clear_irq()

PATCH In review Bartlomiej Zolnierkiewicz 3 participants 12 messages Aug 19, 2008 – May 22, 2009 libata

lore

Share & tools

lore

Bartlomiej Zolnierkiewicz

author

2008-08-19 18:37 UTC #0 lore raw

PATCH 2/5 ide: ->ide_dma_clear_irq() -> ->clear_irq()

* 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(-)

drivers/ide/ide-io.c +4 ~~-11~~

Index: 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

drivers/ide/pci/piix.c +23 ~~-16~~

Index: 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, \

include/linux/ide.h +2 -2

Index: 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) */

Sergei Shtylyov

Acked-by on v1 2/5

2008-08-19 22:22 UTC #1 lore raw

Hello.

Quoted 12 lines · Bartlomiej Zolnierkiewicz

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

Quoted 1 line

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

Acked-by Sergei Shtylyov <[email protected]>

MBR, Sergei

Sergei Shtylyov

2008-09-15 22:11 UTC #2 lore raw

Quoted 7 lines · Hello, I wrote:

Quoted 7 lines
* 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.

Quoted 8 lines

Quoted 5 lines
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...

I forgot that it does clear them in its dma_end() methods (which I
myself have reworked :-).
It seems however that at least for SFF-8038 compatibles, it makes
sense to leave it that way since INTRQ might be asserted while BMIDE interrupt bit is not, so the interrupt latch would need clearing even on DMA timeout...

Quoted 2 lines

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

Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...

Quoted 3 lines

Quoted 1 line
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...

MBR, Sergei

Bartlomiej Zolnierkiewicz

author

2008-09-15 22:30 UTC #3 lore raw

Quoted 28 lines · On Monday 15 September 2008 15:11:21 Sergei Shtylyov wrote:

Quoted 10 lines · Hello, I wrote:
Quoted 10 lines
* 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.
Quoted 8 lines
Quoted 2 lines
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...
I forgot that it does clear them in its dma_end() methods (which I
myself have reworked :-).
It seems however that at least for SFF-8038 compatibles, it makes
sense to leave it that way since INTRQ might be asserted while BMIDE interrupt bit is not, so the interrupt latch would need clearing even on DMA timeout...

Quoted 6 lines

Quoted 3 lines
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...
Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...

Or we can test for ->waiting_for_dma inside ->test_irq.

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
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.

Thanks,
Bart

Sergei Shtylyov

2008-09-16 09:17 UTC #4 lore raw

Hello.

Quoted 21 lines · Bartlomiej Zolnierkiewicz

Quoted 21 lines
Quoted 7 lines
Quoted 7 lines
* 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.
Quoted 9 lines
Quoted 6 lines
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...
I forgot that it does clear them in its dma_end() methods (which I
myself have reworked :-).
It seems however that at least for SFF-8038 compatibles, it makes
sense to leave it that way since INTRQ might be asserted while BMIDE interrupt bit is not, so the interrupt latch would need clearing even on DMA timeout...

Quoted 9 lines

Quoted 6 lines
Quoted 3 lines
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...
Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...
Or we can test for ->waiting_for_dma inside ->test_irq.

That also will do...

Quoted 11 lines

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
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.

Quoted 3 lines

Thanks,
Bart

MBR, Sergei

Jeremy Higdon

2008-11-12 02:01 UTC #5 lore raw

Quoted 12 lines · Sergei Shtylyov

Quoted 11 lines
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
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.

Quoted 10 lines

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

Sergei Shtylyov

2009-05-21 14:06 UTC #6 lore raw

In reply to Sergei Shtylyov

Quoted 11 lines · Hello, I wrote:

Quoted 11 lines
Quoted 11 lines
* 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.

[...]

Quoted 5 lines

Quoted 2 lines
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...
Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...

Yet this seems what ack_intr() method is doing already...
What it does is testing IRQ status and "acknowledging" it (the semantics
of "acknowledge" is not clear to me, yet it seems that it's clearing the interrupt latch in the drivers where it's implemented). And the call site of ack_intr() method corresponds to where test_irq() should have been called, so it seems we don't need yet another method and probably didn't even need clear_irq() method in the first place?..
Bart, could you clarify about how ack_intr() is supposed to work?

MBR, Sergei

Bartlomiej Zolnierkiewicz

author

2009-05-22 18:44 UTC #7 lore raw

->ack_intr in m68k IDE drivers [was: Re: [PATCH 2/5] ide: ->ide_dma_clear_irq() -> ->clear_irq()]

Quoted 23 lines · On Thursday 21 May 2009 16:07:06 Sergei Shtylyov wrote:

Quoted 11 lines · Hello, I wrote:
Quoted 11 lines
Quoted 11 lines
* 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.
[...]
Quoted 6 lines
Quoted 3 lines
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...
Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...

Quoted 7 lines

Yet this seems what ack_intr() method is doing already... What it does is testing IRQ status and "acknowledging" it (the semantics
of "acknowledge" is not clear to me, yet it seems that it's clearing the interrupt latch in the drivers where it's implemented). And the call site of ack_intr() method corresponds to where test_irq() should have been called, so it seems we don't need yet another method and probably didn't even need clear_irq() method in the first place?..

They have different goals -- the main purpose of ack_intr() (despite its name) seems to be testing whether the IRQ is ours, OTOH in clear_irq() we know that already and we just want to clear the pending IRQ status.

So I'm not sure if unification is desirable... though some improvements are definitely possibly there (less confusing naming at least)...

Quoted 1 line

Bart, could you clarify about how ack_intr() is supposed to work?

Good question, m68k list would be the best place to look for an answer..

Bartlomiej Zolnierkiewicz

author

2009-05-22 18:55 UTC #8 lore raw

->ack_intr in m68k IDE drivers [was: Re: [PATCH 2/5] ide: ->ide_dma_clear_irq() -> ->clear_irq()]

In reply to Sergei Shtylyov

Quoted 23 lines · On Thursday 21 May 2009 16:07:06 Sergei Shtylyov wrote:

Quoted 11 lines · Hello, I wrote:
Quoted 11 lines
Quoted 11 lines
* 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.
[...]
Quoted 6 lines
Quoted 3 lines
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...
Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...

Quoted 7 lines

Yet this seems what ack_intr() method is doing already... What it does is testing IRQ status and "acknowledging" it (the semantics
of "acknowledge" is not clear to me, yet it seems that it's clearing the interrupt latch in the drivers where it's implemented). And the call site of ack_intr() method corresponds to where test_irq() should have been called, so it seems we don't need yet another method and probably didn't even need clear_irq() method in the first place?..

So I'm not sure if unification is desirable... though some improvements are definitely possibly there (less confusing naming at least)...

Quoted 1 line

Bart, could you clarify about how ack_intr() is supposed to work?

Good question, m68k list would be the best place to look for an answer..

Sergei Shtylyov

2009-05-22 19:18 UTC #9 lore raw

Re: ->ack_intr in m68k IDE drivers [was: Re: [PATCH 2/5] ide: ->ide_dma_clear_irq() -> ->clear_irq()]

Quoted 6 lines · Bartlomiej Zolnierkiewicz

Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
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...
Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...

Quoted 6 lines

Quoted 3 lines
Yet this seems what ack_intr() method is doing already...
What it does is testing IRQ status and "acknowledging" it (the semantics
of "acknowledge" is not clear to me, yet it seems that it's clearing the interrupt latch in the drivers where it's implemented). And the call site of ack_intr() method corresponds to where test_irq() should have been called, so it seems we don't need yet another method and probably didn't even need clear_irq() method in the first place?..
They have different goals -- the main purpose of ack_intr() (despite its name) seems to be testing whether the IRQ is ours,

It does clear some interrupt bit if it sees that IRQ is ours too, hence
the same.

Quoted 2 lines

OTOH in clear_irq() we know that
already and we just want to clear the pending IRQ status.

There seems to be duplication of functionality b/w ack_intr() and
clear_irq() now...

Quoted 6 lines

So I'm not sure if unification is desirable... though some improvements are definitely possibly there (less confusing naming at least)...
Quoted 1 line
Bart, could you clarify about how ack_intr() is supposed to work?
Good question, m68k list would be the best place to look for an answer..

Well, I seem to have been able to infer it from the code...

MBR, Sergei

Sergei Shtylyov

2009-05-22 19:18 UTC #10 lore raw

Re: ->ack_intr in m68k IDE drivers [was: Re: [PATCH 2/5] ide: ->ide_dma_clear_irq() -> ->clear_irq()]

In reply to Bartlomiej Zolnierkiewicz

Quoted 6 lines · Bartlomiej Zolnierkiewicz

Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
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...
Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...

Quoted 6 lines

Quoted 3 lines
Yet this seems what ack_intr() method is doing already...
What it does is testing IRQ status and "acknowledging" it (the semantics
of "acknowledge" is not clear to me, yet it seems that it's clearing the interrupt latch in the drivers where it's implemented). And the call site of ack_intr() method corresponds to where test_irq() should have been called, so it seems we don't need yet another method and probably didn't even need clear_irq() method in the first place?..
They have different goals -- the main purpose of ack_intr() (despite its name) seems to be testing whether the IRQ is ours,

It does clear some interrupt bit if it sees that IRQ is ours too, hence
the name.

Quoted 2 lines

OTOH in clear_irq() we know that
already and we just want to clear the pending IRQ status.

There seems to be duplication of functionality b/w ack_intr() and
clear_irq() now...

Quoted 6 lines

So I'm not sure if unification is desirable... though some improvements are definitely possibly there (less confusing naming at least)...
Quoted 1 line
Bart, could you clarify about how ack_intr() is supposed to work?
Good question, m68k list would be the best place to look for an answer..

Well, I seem to have been able to infer it from the code...

MBR, Sergei

Sergei Shtylyov

2009-05-22 19:19 UTC #11 lore raw

Re: ->ack_intr in m68k IDE drivers [was: Re: [PATCH 2/5] ide: ->ide_dma_clear_irq() -> ->clear_irq()]

In reply to Bartlomiej Zolnierkiewicz

Quoted 6 lines · Bartlomiej Zolnierkiewicz

Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
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...
Probably might add the test_irq() method to be called on
!hwif->waiting_for_dma. Cleraing the status at once seems impractical...

Quoted 6 lines

Quoted 3 lines
Yet this seems what ack_intr() method is doing already...
What it does is testing IRQ status and "acknowledging" it (the semantics
of "acknowledge" is not clear to me, yet it seems that it's clearing the interrupt latch in the drivers where it's implemented). And the call site of ack_intr() method corresponds to where test_irq() should have been called, so it seems we don't need yet another method and probably didn't even need clear_irq() method in the first place?..
They have different goals -- the main purpose of ack_intr() (despite its name) seems to be testing whether the IRQ is ours,

It does clear some interrupt bit if it sees that IRQ is ours too, hence
the same.

Quoted 2 lines

OTOH in clear_irq() we know that
already and we just want to clear the pending IRQ status.

There seems to be duplication of functionality b/w ack_intr() and
clear_irq() now...

Quoted 6 lines

So I'm not sure if unification is desirable... though some improvements are definitely possibly there (less confusing naming at least)...
Quoted 1 line
Bart, could you clarify about how ack_intr() is supposed to work?
Good question, m68k list would be the best place to look for an answer..

Well, I seem to have been able to infer it from the code...

MBR, Sergei