linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 2/4] libata-core.c: add another IRQ calls
@ 2007-01-17  9:24 Mikael Pettersson
  2007-01-17 16:49 ` Jeff Garzik
  0 siblings, 1 reply; 11+ messages in thread
From: Mikael Pettersson @ 2007-01-17  9:24 UTC (permalink / raw)
  To: alan, benh, jeff; +Cc: arnd, linuxppc-dev, linux-ide, paulus

On Tue, 16 Jan 2007 17:04:27 -0500, Jeff Garzik wrote:
>Alan wrote:
>> Jeff - at some point we could eliminate a lot of the NULL checks like
>> these by having the registration code fill in the defaults where
>> appropriate ?
>
>libata policy up until this point has been to require all drivers fill 
>in the hook, either with the commonly-used default, or with their own 
>variant.  Thus, no NULL checks for required hooks, and you get an oops 
>if you fail this requirement.  I like that better than defaults buried 
>inside libata-core, where it is easier for programmers to forget them.
>
>But actually, it's an open question for the compiler guys whether this 
>case is preferred:
>
>	if (ap->ops->hook)
>		ap->ops->hook(foo, bar);
>	else
>		commonly_used_default(foo, bar);
>
>or this:
>
>	ap->ops->hook(foo, bar);
>
>With advanced branch prediction in modern CPUs, ISTR the first case may 
>be worth considering, even with the branch.

Indirect function calls incur at least two main costs:
- the 'call *' instruction itself suffers from poor branch
  prediction unless the HW is clever and the call patterns
  are highly biased -- most machines have worse branch
  predictors for indirect jumps than for direct jumps
- the compiler must generate code for setting up parameters
  and spilling the caller's registers, and for reloading the
  caller's registers after the call

If there is a highly common case, the code for that case
is available, and the code is cheap to execute (doesn't
require too many additional variables), then having an
explicit test + inline code is preferred. If any of these
conditions isn't met, then I wouldn't bother converting
the call to the if/inline/call combo.

There are compilers for OOP languages that routinely use
rewrites like the one above in order to optimise method calls.

/Mikael

^ permalink raw reply	[flat|nested] 11+ messages in thread
[parent not found: <200701161046.l0GAk5Go019691@toshiba.co.jp>]
* [PATCH 2/4] libata-core.c: add another IRQ calls
@ 2007-01-16 10:46 Akira Iguchi
  0 siblings, 0 replies; 11+ messages in thread
From: Akira Iguchi @ 2007-01-16 10:46 UTC (permalink / raw)
  To: linux-ide; +Cc: jeff, arnd, linuxppc-dev, paulus, alan

When enabling IRQ, ap->ops->irq_on is checked.
Because most drivers can use ata_irq_on() as is, this
patch allows ap->ops->irq_on to be NULL.
If it is NULL, ata_irq_on() are used.

Similarly, ap->ops->irq_ack is checked when acknowledging a IRQ.
If it is NULL, ata_irq_ack() are used.

And this patch exports ata_dev_try_classify(). It is used in
pata_scc.c to reduce the code duplication.
This driver has the copy of ata_std_softreset(), which has
low-level accessors and cannot be used as is.

Signed-off-by: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
Signed-off-by: Akira Iguchi <akira2.iguchi@toshiba.co.jp>
---

--- linux-2.6.20-rc4/drivers/ata/libata-core.c.orig	2007-01-17 01:45:44.000000000 +0900
+++ linux-2.6.20-rc4/drivers/ata/libata-core.c	2007-01-17 02:16:48.000000000 +0900
@@ -767,7 +767,7 @@ unsigned int ata_dev_classify(const stru
  *	Device type - %ATA_DEV_ATA, %ATA_DEV_ATAPI or %ATA_DEV_NONE.
  */
 
-static unsigned int
+unsigned int
 ata_dev_try_classify(struct ata_port *ap, unsigned int device, u8 *r_err)
 {
 	struct ata_taskfile tf;
@@ -2754,8 +2754,12 @@ void ata_bus_reset(struct ata_port *ap)
 		ap->device[1].class = ata_dev_try_classify(ap, 1, &err);
 
 	/* re-enable interrupts */
-	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
-		ata_irq_on(ap);
+	if (ap->ioaddr.ctl_addr) {	/* FIXME: hack. create a hook instead */
+		if (ap->ops->irq_on)
+			ap->ops->irq_on(ap);
+		else
+			ata_irq_on(ap);
+	}
 
 	/* is double-select really necessary? */
 	if (ap->device[1].class != ATA_DEV_NONE)
@@ -3149,8 +3153,12 @@ void ata_std_postreset(struct ata_port *
 	/* re-enable interrupts */
 	if (!ap->ops->error_handler) {
 		/* FIXME: hack. create a hook instead */
-		if (ap->ioaddr.ctl_addr)
-			ata_irq_on(ap);
+		if (ap->ioaddr.ctl_addr) {
+			if (ap->ops->irq_on)
+				ap->ops->irq_on(ap);
+			else
+				ata_irq_on(ap);
+		}
 	}
 
 	/* is double-select really necessary? */
@@ -4329,7 +4337,10 @@ static void ata_hsm_qc_complete(struct a
 			qc = ata_qc_from_tag(ap, qc->tag);
 			if (qc) {
 				if (likely(!(qc->err_mask & AC_ERR_HSM))) {
-					ata_irq_on(ap);
+					if (ap->ops->irq_on)
+						ap->ops->irq_on(ap);
+					else
+						ata_irq_on(ap);
 					ata_qc_complete(qc);
 				} else
 					ata_port_freeze(ap);
@@ -4345,7 +4356,10 @@ static void ata_hsm_qc_complete(struct a
 	} else {
 		if (in_wq) {
 			spin_lock_irqsave(ap->lock, flags);
-			ata_irq_on(ap);
+			if (ap->ops->irq_on)
+				ap->ops->irq_on(ap);
+			else
+				ata_irq_on(ap);
 			ata_qc_complete(qc);
 			spin_unlock_irqrestore(ap->lock, flags);
 		} else
@@ -5170,7 +5184,10 @@ idle_irq:
 
 #ifdef ATA_IRQ_TRAP
 	if ((ap->stats.idle_irq % 1000) == 0) {
-		ata_irq_ack(ap, 0); /* debug trap */
+		if (ap->ops->irq_ack)
+			ap->ops->irq_ack(ap, 0);
+		else
+			ata_irq_ack(ap, 0); /* debug trap */
 		ata_port_printk(ap, KERN_WARNING, "irq trap\n");
 		return 1;
 	}
@@ -6500,3 +6517,5 @@ EXPORT_SYMBOL_GPL(ata_eh_thaw_port);
 EXPORT_SYMBOL_GPL(ata_eh_qc_complete);
 EXPORT_SYMBOL_GPL(ata_eh_qc_retry);
 EXPORT_SYMBOL_GPL(ata_do_eh);
+EXPORT_SYMBOL_GPL(ata_dev_try_classify);
+EXPORT_SYMBOL_GPL(ata_probe_ent_alloc);

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2007-01-25  3:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-17  9:24 [PATCH 2/4] libata-core.c: add another IRQ calls Mikael Pettersson
2007-01-17 16:49 ` Jeff Garzik
2007-01-17 20:31   ` Mikael Pettersson
2007-01-18  0:55     ` Akira Iguchi
     [not found]     ` <200701180055.l0I0tl6M021051@toshiba.co.jp>
2007-01-18  1:29       ` [PATCH] driver/ata: PATA driver for Celleb Akira Iguchi
2007-01-25  1:17       ` [PATCH 2/4] libata-core.c: add another IRQ calls Jeff Garzik
     [not found]       ` <200701180129.l0I1TL48013407@toshiba.co.jp>
2007-01-25  1:20         ` [PATCH] driver/ata: PATA driver for Celleb Jeff Garzik
2007-01-25  3:45           ` Benjamin Herrenschmidt
     [not found] <200701161046.l0GAk5Go019691@toshiba.co.jp>
2007-01-16 12:03 ` [PATCH 2/4] libata-core.c: add another IRQ calls Alan
2007-01-16 22:04   ` Jeff Garzik
  -- strict thread matches above, loose matches on Subject: below --
2007-01-16 10:46 Akira Iguchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).