Re: [PATCH 2/4] libata-core.c: add another IRQ calls
flat view
From: Jeff Garzik <hidden>
Date: 2007-01-25 01:18:01
Also in:
linux-ide
Akira Iguchi wrote:
quoted
The real benefits from identifying a common case is to inline the code for it. So the fact that the common case is still a full-blown function call here and not inline code means that there's much less benefit from rewriting an indirect call as an if/indirect/direct sequence.According to your comment, this patch adds IRQ calls (irq_on, irq_ack) to each driver and always uses these indirect calls. For irq_on, most drivers use ata_irq_on(). Some drivers (ahci.c, sata_sil24.c) use ata_dummy_irq_on() because they don't have either explicit or implicit assignment (ex: ata_pci_init_one) of ctl_addr. For irq_ack, ata_irq_ack() is used. Signed-off-by: Kou Ishizaki <redacted> Signed-off-by: Akira Iguchi <redacted>
very close to perfect :) ahci and sata_sil24 need dummy functions for ->irq_ack(). As you can see, ata_irq_ack() is only used in debug situations, and it [the current code] is very wrong for ahci and sata_sil24. Also, please split up the patch into two pieces: (1) update core and headers, and (2) update every driver.
quoted hunk
diff -uprN -X linux-2.6.20-rc4/Documentation/dontdiff linux-2.6.20-rc4/include/linux/libata.h linux-2.6.20-rc4.mod/include/linux/libata.h--- linux-2.6.20-rc4/include/linux/libata.h 2007-01-17 23:23:27.000000000 +0900 +++ linux-2.6.20-rc4.mod/include/linux/libata.h 2007-01-17 23:24:49.000000000 +0900@@ -638,6 +638,8 @@ struct ata_port_operations { irq_handler_t irq_handler; void (*irq_clear) (struct ata_port *); + u8 (*irq_on) (struct ata_port *); + u8 (*irq_ack) (struct ata_port *ap, unsigned int chk_drq); u32 (*scr_read) (struct ata_port *ap, unsigned int sc_reg); void (*scr_write) (struct ata_port *ap, unsigned int sc_reg,@@ -761,6 +763,7 @@ extern void ata_port_queue_task(struct a extern u32 ata_wait_register(void __iomem *reg, u32 mask, u32 val, unsigned long interval_msec, unsigned long timeout_msec); +u8 ata_irq_on(struct ata_port *ap); /* * Default driver ops implementations@@ -1202,6 +1205,8 @@ static inline u8 ata_irq_ack(struct ata_ return status; } +static inline u8 ata_dummy_irq_on (struct ata_port *ap) { return 0; } +
This won't work, you need to create a non-inline function and EXPORT_SYMBOL_GPL it. Jeff