Re: [PATCH 1/4] ide: remove IDE_ARCH_INTR

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thursday 29 January 2009, Michael Schmitz wrote:
Hi,


-            (void)ide_ack_intr(hwif);
+            if (hwif->ack_intr(hwif))
+                hwif->ack_intr(hwif);

Shouldn't that be:

-            (void)ide_ack_intr(hwif);
+            if (hwif->ack_intr)
+                hwif->ack_intr(hwif);

Seconded. No point in ack'ing the int twice (not to mention the other side 
effects).

Brown paper bag time and another proof that having helpful people looking
over your patches really matters...

Thanks guys, I fixed it in v2.

From: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
Subject: [PATCH] ide: remove IDE_ARCH_INTR (v2)

This micro-optimization is not worth it.  Just always check for
existence of ->ack_intr method in ide_intr() and ide_timer_expiry().

v2:
Fix brown-paper-bag bug spotted by David D. Kilzer.

Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Cc: Michael Schmitz <schmitz@xxxxxxxxxx>
Cc: "David D. Kilzer" <ddkilzer@xxxxxxxxxx>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
---
 drivers/ide/ide-io.c   |    5 +++--
 include/asm-m68k/ide.h |    3 ---
 include/linux/ide.h    |    5 -----
 3 files changed, 3 insertions(+), 10 deletions(-)

Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -736,7 +736,8 @@ 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);
-			(void)ide_ack_intr(hwif);
+			if (hwif->ack_intr)
+				hwif->ack_intr(hwif);
 			printk(KERN_WARNING "%s: lost interrupt\n",
 				drive->name);
 			startstop = handler(drive);
@@ -851,7 +852,7 @@ irqreturn_t ide_intr (int irq, void *dev
 
 	spin_lock_irqsave(&hwif->lock, flags);
 
-	if (!ide_ack_intr(hwif))
+	if (hwif->ack_intr && hwif->ack_intr(hwif) == 0)
 		goto out;
 
 	handler = hwif->handler;
Index: b/include/asm-m68k/ide.h
===================================================================
--- a/include/asm-m68k/ide.h
+++ b/include/asm-m68k/ide.h
@@ -123,8 +123,5 @@ ide_get_lock(irq_handler_t handler, void
 }
 #endif /* CONFIG_BLK_DEV_FALCON_IDE */
 
-#define IDE_ARCH_ACK_INTR
-#define ide_ack_intr(hwif)	((hwif)->ack_intr ? (hwif)->ack_intr(hwif) : 1)
-
 #endif /* __KERNEL__ */
 #endif /* _M68K_IDE_H */
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -202,11 +202,6 @@ static inline void ide_std_init_ports(hw
 
 #define MAX_HWIFS	10
 
-/* Currently only m68k, apus and m8xx need it */
-#ifndef IDE_ARCH_ACK_INTR
-# define ide_ack_intr(hwif) (1)
-#endif
-
 /* Currently only Atari needs it */
 #ifndef IDE_ARCH_LOCK
 # define ide_release_lock()			do {} while (0)

--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux