RFC: interrupt trigger flags from IRQ resource

From: Ben Dooks
Date: Mon May 07 2007 - 10:00:09 EST

Next message: Pavel Machek: "Re: [PATCH 5/8] APM emulation driver for class batteries"
Previous message: Pavel Machek: "Re: Back to the future."
Next in thread: Anton Vorontsov: "Re: RFC: interrupt trigger flags from IRQ resource"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

At least one driver (drivers/net/dm9000.c) could
do with being able to configure the trigger type
of the IRQ depending on the system it is connected
to. There are two methods for this, and requesting
comments on these solutions.

1) The platform data supplied with the device
carries an unsigned long field with the
IRQF_TRIGGER_xxx for the system.

2) The driver convers the flags passed in the
IORESOURCE_IRQ into an IRQF_TRIGGERR_xxx.

Method (1) is fine on a per-driver basis, but
is not generic. I propose to show several methods
of acheiving (2) as follows:

a) Use the method proposed in [1] where there is
a simple conversion of the IORESOURCE flags into
IRQF_TRIGGER flags with a mask. This method is
naive as it requres <linux/ioport.h> and
<linux/interrupt.h> to agree.

The patch supplied in [1] does not make an attempt
to do keep the two in sync.

It could be cleaned up by changing interrupt.h
to define IRQF_TRIGGER as the relevant IORESOURCE
types. This however would still require checking
the IRQF_TRIGGER and IRQF_xxx do not overlap.

b) Create a conversion function, such as irqf_from_resource()
which converts the flags from the resource to the
relevant IRQF flags. This method allows the code
to compensate for drift between ioport.h and the
interrupt.h files.

Several implementations will be show below t
detail how this method can be made reasonably
optimal.

I belive <linux/interrupt.h> is the best place to
put this.

All compilation tests where done with gcc-4.0.2 compiling
2.6.21-git7 for ARM.

Method 1: Simple use of if and the | operator. This
results in output code looking very much like the
input, with 4 tests and 4 ORR instructions.

diff -urpN -X linux-2.6.21-git7/Documentation/dontdiff linux-2.6.21-git7/include/linux/interrupt.h linux-2.6.21-git7-flags1/include/linux/interrupt.h
--- linux-2.6.21-git7/include/linux/interrupt.h 2007-05-07 11:19:50.000000000 +0000
+++ linux-2.6.21-git7-flags1/include/linux/interrupt.h 2007-05-07 11:31:41.000000000 +0000
@@ -13,6 +13,7 @@
#include <linux/irqflags.h>
#include <linux/bottom_half.h>
#include <linux/device.h>
+#include <linux/ioport.h>
#include <asm/atomic.h>
#include <asm/ptrace.h>
#include <asm/system.h>
@@ -92,6 +93,22 @@ extern int devm_request_irq(struct devic
const char *devname, void *dev_id);
extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);

+static inline unsigned long irqf_from_resource(unsigned long flags)
+{
+ unsigned long result = 0;
+
+ if (flags & IORESOURCE_IRQ_HIGHEDGE)
+ result |= IRQF_TRIGGER_RISING;
+ if (flags & IORESOURCE_IRQ_LOWEDGE)
+ result |= IRQF_TRIGGER_FALLING;
+ if (flags & IORESOURCE_IRQ_HIGHLEVEL)
+ result |= IRQF_TRIGGER_HIGH;
+ if (flags & IORESOURCE_IRQ_LOWLEVEL)
+ result |= IRQF_TRIGGER_LOW;
+
+ return result;
+}
+
/*
* On lockdep we dont want to enable hardirqs in hardirq
* context. Use local_irq_enable_in_hardirq() to annotate

Method 2: Use tests with flag equality to try and persuade
the compiler to optimised out any equal flags to a simple
mask and logical-or.

This example results in a single ORR instruction after
loading the resource flags.

diff -urpN -X linux-2.6.21-git7/Documentation/dontdiff linux-2.6.21-git7/include/linux/interrupt.h linux-2.6.21-git7-flags2/include/linux/interrupt.h
--- linux-2.6.21-git7/include/linux/interrupt.h 2007-05-07 11:19:50.000000000 +0000
+++ linux-2.6.21-git7-flags2/include/linux/interrupt.h 2007-05-07 13:11:48.000000000 +0000
@@ -13,6 +13,7 @@
#include <linux/irqflags.h>
#include <linux/bottom_half.h>
#include <linux/device.h>
+#include <linux/ioport.h>
#include <asm/atomic.h>
#include <asm/ptrace.h>
#include <asm/system.h>
@@ -92,6 +93,39 @@ extern int devm_request_irq(struct devic
const char *devname, void *dev_id);
extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);

+/* convert resource flags to irq trigger type */
+
+static inline unsigned long irqf_from_resource(unsigned long flags)
+{
+ unsigned long result = 0;
+
+ if (IORESOURCE_IRQ_HIGHEDGE != IRQF_TRIGGER_RISING &&
+ flags & IORESOURCE_IRQ_HIGHEDGE)
+ result |= IRQF_TRIGGER_RISING;
+ else
+ result |= flags & IRQF_TRIGGER_RISING;
+
+ if (IORESOURCE_IRQ_LOWEDGE != IRQF_TRIGGER_FALLING &&
+ flags & IORESOURCE_IRQ_LOWEDGE)
+ result |= IRQF_TRIGGER_FALLING;
+ else
+ result |= flags & IRQF_TRIGGER_FALLING;
+
+ if (IORESOURCE_IRQ_HIGHLEVEL != IRQF_TRIGGER_HIGH &&
+ flags & IORESOURCE_IRQ_HIGHLEVEL)
+ result |= IRQF_TRIGGER_HIGH;
+ else
+ result |= flags & IRQF_TRIGGER_HIGH;
+
+ if (IORESOURCE_IRQ_LOWLEVEL != IRQF_TRIGGER_LOW &&
+ flags & IORESOURCE_IRQ_LOWLEVEL)
+ result |= IRQF_TRIGGER_LOW;
+ else
+ result |= flags & IRQF_TRIGGER_LOW;
+
+ return result;
+}
+
/*
* On lockdep we dont want to enable hardirqs in hardirq
* context. Use local_irq_enable_in_hardirq() to annotate

Method 3: This is method 2, using a macro to make the
conversion. This produces the same code as #2, but with
less room for mistakes in the conversion.

diff -urpN -X linux-2.6.21-git7/Documentation/dontdiff linux-2.6.21-git7/include/linux/interrupt.h linux-2.6.21-git7-flags3/include/linux/interrupt.h
--- linux-2.6.21-git7/include/linux/interrupt.h 2007-05-07 11:19:50.000000000 +0000
+++ linux-2.6.21-git7-flags3/include/linux/interrupt.h 2007-05-07 13:25:47.000000000 +0000
@@ -13,6 +13,7 @@
#include <linux/irqflags.h>
#include <linux/bottom_half.h>
#include <linux/device.h>
+#include <linux/ioport.h>
#include <asm/atomic.h>
#include <asm/ptrace.h>
#include <asm/system.h>
@@ -92,6 +93,24 @@ extern int devm_request_irq(struct devic
const char *devname, void *dev_id);
extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);

+/* convert resource flags to irq trigger type */
+
+#define irqf_convert(_irqf, _resf) \
+ ((_irqf) != (_resf) ? (flags & (_resf) ? (_irqf) : 0) : \
+ ((flags & (_resf))))
+
+static inline unsigned long irqf_from_resource(unsigned long flags)
+{
+ unsigned long result;
+
+ result = irqf_convert(IRQF_TRIGGER_RISING, IORESOURCE_IRQ_HIGHEDGE);
+ result |= irqf_convert(IRQF_TRIGGER_FALLING, IORESOURCE_IRQ_LOWEDGE);
+ result |= irqf_convert(IRQF_TRIGGER_HIGH, IORESOURCE_IRQ_HIGHLEVEL);
+ result |= irqf_convert(IRQF_TRIGGER_LOW, IORESOURCE_IRQ_LOWLEVEL);
+
+ return result;
+}
+
/*
* On lockdep we dont want to enable hardirqs in hardirq
* context. Use local_irq_enable_in_hardirq() to annotate

My choice would be either 2 or 3. I would like input on what to
name the function, and any comments on where this could be used
other than in drivers/net/dm9000.c

References:

[1] http://www.mail-archive.com/netdev@xxxxxxxxxxxxxxx/msg21428.html

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

Next message: Pavel Machek: "Re: [PATCH 5/8] APM emulation driver for class batteries"
Previous message: Pavel Machek: "Re: Back to the future."
Next in thread: Anton Vorontsov: "Re: RFC: interrupt trigger flags from IRQ resource"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]