[Xenomai-core] [PATCH] Shared irqs v.6

[Xenomai-core] [PATCH] Shared irqs v.6

[Dmitry Adamushko]

[-- Attachment #1.1: Type: text/plain, Size: 198 bytes --]

Hi,

the previous version contained an ugly bug, namely the misuse of the
test_and_set/clear_bit interface
that led to a broken system from time to time.

--
Best regards,
Dmitry Adamushko

[-- Attachment #1.2: Type: text/html, Size: 228 bytes --]

[-- Attachment #2: shirq-v6.patch --]
[-- Type: application/octet-stream, Size: 12742 bytes --]

diff -urp xenomai-cvs/include/asm-generic/hal.h xenomai/include/asm-generic/hal.h
--- xenomai-cvs/include/asm-generic/hal.h	2006-01-13 15:10:23.000000000 +0100
+++ xenomai/include/asm-generic/hal.h	2006-01-18 12:35:49.000000000 +0100
@@ -62,6 +62,7 @@
 #define RTHAL_NR_CPUS		IPIPE_NR_CPUS
 #define RTHAL_ROOT_PRIO		IPIPE_ROOT_PRIO
 #define RTHAL_NR_FAULTS		IPIPE_NR_FAULTS
+#define RTHAL_NR_IRQS		IPIPE_NR_IRQS
 
 #define RTHAL_SERVICE_IPI0	IPIPE_SERVICE_IPI0
 #define RTHAL_SERVICE_VECTOR0	IPIPE_SERVICE_VECTOR0
diff -urp xenomai-cvs/include/asm-generic/system.h xenomai/include/asm-generic/system.h
--- xenomai-cvs/include/asm-generic/system.h	2006-01-03 19:53:37.000000000 +0100
+++ xenomai/include/asm-generic/system.h	2006-01-31 18:01:49.000000000 +0100
@@ -101,6 +101,8 @@ typedef struct { volatile unsigned long 
 
 #define XNARCH_NR_CPUS               RTHAL_NR_CPUS
 
+#define XNARCH_TIMER_IRQ	     RTHAL_TIMER_IRQ
+
 #define XNARCH_ROOT_STACKSZ   0	/* Only a placeholder -- no stack */
 
 #define XNARCH_PROMPT "Xenomai: "
@@ -470,6 +472,16 @@ static inline unsigned long long xnarch_
 
 #ifdef XENO_INTR_MODULE
 
+#define XNARCH_NR_IRQS		     RTHAL_NR_IRQS
+
+static inline unsigned long xnarch_critical_enter(void (*synch)(void)) {
+    return rthal_critical_enter(synch);
+}
+
+static inline void xnarch_critical_exit(unsigned long flags) {
+    return rthal_critical_exit(flags);
+}
+
 static inline int xnarch_hook_irq (unsigned irq,
 				   rthal_irq_handler_t handler,
 				   rthal_irq_ackfn_t ackfn,
diff -urp xenomai-cvs/include/asm-i386/hal.h xenomai/include/asm-i386/hal.h
--- xenomai-cvs/include/asm-i386/hal.h	2006-01-04 11:32:29.000000000 +0100
+++ xenomai/include/asm-i386/hal.h	2006-01-23 10:36:13.000000000 +0100
@@ -191,6 +191,9 @@ static inline __attribute_const__ unsign
 #define RTHAL_APIC_TIMER_VECTOR    RTHAL_SERVICE_VECTOR3
 #define RTHAL_APIC_TIMER_IPI       RTHAL_SERVICE_IPI3
 #define RTHAL_APIC_ICOUNT          ((RTHAL_TIMER_FREQ + HZ/2)/HZ)
+#define RTHAL_TIMER_IRQ 	   RTHAL_APIC_TIMER_IPI
+#else  /* !CONFIG_X86_LOCAL_APIC */
+#define RTHAL_TIMER_IRQ		   RTHAL_8254_IRQ
 #endif /* CONFIG_X86_LOCAL_APIC */
 
 #define RTHAL_NMICLK_FREQ	RTHAL_CPU_FREQ
diff -urp xenomai-cvs/include/asm-sim/system.h xenomai/include/asm-sim/system.h
--- xenomai-cvs/include/asm-sim/system.h	2006-01-03 19:53:37.000000000 +0100
+++ xenomai/include/asm-sim/system.h	2006-01-31 18:01:29.000000000 +0100
@@ -75,6 +75,9 @@ typedef unsigned long xnlock_t;
 
 #define XNARCH_NR_CPUS              1
 
+/* Should be equal to the value used for creating the mvmtimer object (mvm_start_timer). */
+#define XNARCH_TIMER_IRQ	    1
+
 #define XNARCH_DEFAULT_TICK         10000000 /* ns, i.e. 10ms */
 #define XNARCH_HOST_TICK            0 /* No host ticking service. */
 
@@ -365,6 +368,16 @@ void mvm_tcl_build_pendq(mvm_tcl_listobj
 typedef void (*rthal_irq_handler_t)(unsigned, void *);
 typedef int (*rthal_irq_ackfn_t)(unsigned);
 
+#define XNARCH_NR_IRQS		     MVM_IRQ_LEVELS
+
+static inline unsigned long xnarch_critical_enter(void (*synch)(void)) {
+    return 0;
+}
+
+static inline void xnarch_critical_exit(unsigned long flags) {
+    return;
+}
+
 static inline int xnarch_hook_irq (unsigned irq,
 				   rthal_irq_handler_t handler,
 				   rthal_irq_ackfn_t ackfn, /* Ignored. */
diff -urp xenomai-cvs/include/asm-uvm/system.h xenomai/include/asm-uvm/system.h
--- xenomai-cvs/include/asm-uvm/system.h	2006-01-08 15:35:54.000000000 +0100
+++ xenomai/include/asm-uvm/system.h	2006-01-31 18:01:34.000000000 +0100
@@ -74,6 +74,8 @@ typedef unsigned long xnlock_t;
 
 #define XNARCH_NR_CPUS             1
 
+#define XNARCH_TIMER_IRQ	   0
+
 #define XNARCH_DEFAULT_TICK        1000000 /* ns, i.e. 1ms */
 #define XNARCH_SIG_RESTART         SIGUSR1
 #define XNARCH_HOST_TICK           0	/* No host ticking service */
@@ -270,6 +272,16 @@ void xnarch_sync_irq (void) /* Synchroni
 typedef void (*rthal_irq_handler_t)(unsigned, void *);
 typedef int (*rthal_irq_ackfn_t)(unsigned);
 
+#define XNARCH_NR_IRQS		     0
+
+static inline unsigned long xnarch_critical_enter(void (*synch)(void)) {
+    return 0;
+}
+
+static inline void xnarch_critical_exit(unsigned long flags) {
+    return;
+}
+
 static inline int xnarch_hook_irq (unsigned irq,
 				   rthal_irq_handler_t handler,
 				   rthal_irq_ackfn_t ackfn,
diff -urp xenomai-cvs/include/nucleus/intr.h xenomai/include/nucleus/intr.h
--- xenomai-cvs/include/nucleus/intr.h	2005-11-01 23:37:45.000000000 +0100
+++ xenomai/include/nucleus/intr.h	2006-01-18 15:32:09.000000000 +0100
@@ -26,21 +26,31 @@
 #define XN_ISR_CHAINED   0x1
 #define XN_ISR_ENABLE    0x2
 
+/* Creation flags. */
+#define XN_ISR_SHARED	 0x1
+
+/* Operational flags. */
+#define XN_ISR_ATTACHED	 0x100
+
 #if defined(__KERNEL__) || defined(__XENO_UVM__) || defined(__XENO_SIM__)
 
 struct xnintr;
 
 typedef struct xnintr {
 
-    unsigned irq;	/* !< IRQ number. */
+    struct xnintr *next;
 
     xnisr_t isr;	/* !< Interrupt service routine. */
 
-    xniack_t iack;	/* !< Interrupt acknowledge routine. */
+    void *cookie;	/* !< User-defined cookie value. */
 
     unsigned long hits;	/* !< Number of receipts (since attachment). */
 
-    void *cookie;	/* !< User-defined cookie value. */
+    xnflags_t flags; 	/* !< Creation flags. */
+
+    unsigned irq;	/* !< IRQ number. */
+
+    xniack_t iack;	/* !< Interrupt acknowledge routine. */
 
 } xnintr_t;
 
@@ -50,6 +60,8 @@ extern xnintr_t nkclock;
 extern "C" {
 #endif
 
+int xnintr_mount(void);
+
 void xnintr_clock_handler(void);
 
     /* Public interface. */
diff -urp xenomai-cvs/ksrc/nucleus/intr.c xenomai/ksrc/nucleus/intr.c
--- xenomai-cvs/ksrc/nucleus/intr.c	2005-12-02 19:56:20.000000000 +0100
+++ xenomai/ksrc/nucleus/intr.c	2006-01-31 19:50:51.000000000 +0100
@@ -41,6 +41,57 @@ xnintr_t nkclock;
 static void xnintr_irq_handler(unsigned irq,
 			       void *cookie);
 
+typedef struct xnintr_shirq {
+
+    xnintr_t *handlers;
+    
+#ifdef CONFIG_SMP
+    atomic_counter_t active;
+#endif /* CONFIG_SMP */
+
+} xnintr_shirq_t;
+
+static xnintr_shirq_t xnshirqs[XNARCH_NR_IRQS];
+
+#ifdef CONFIG_SMP
+
+static inline void xnintr_shirq_lock(xnintr_shirq_t *shirq)
+{
+    xnarch_atomic_inc(&shirq->active);
+}
+
+static inline void xnintr_shirq_unlock(xnintr_shirq_t *shirq)
+{
+    xnarch_atomic_dec(&shirq->active);
+}
+
+static inline void xnintr_shirq_spin(xnintr_shirq_t *shirq)
+{
+    while (xnarch_atomic_get(&shirq->active))
+	cpu_relax();
+}
+
+#else /* !CONFIG_SMP */
+
+static inline void xnintr_shirq_lock(xnintr_shirq_t *shirq) {}
+static inline void xnintr_shirq_unlock(xnintr_shirq_t *shirq) {}
+static inline void xnintr_shirq_spin(xnintr_shirq_t *shirq) {}
+
+#endif /* CONFIG_SMP */
+
+int xnintr_mount(void)
+{
+    int i;
+    for (i = 0; i < XNARCH_NR_IRQS; ++i)
+	{
+	xnshirqs[i].handlers = NULL;
+#ifdef CONFIG_SMP
+	atomic_set(&xnshirqs[i].active,0);
+#endif /* CONFIG_SMP */
+	}
+    return 0;
+}
+
 /*! 
  * \fn int xnintr_init (xnintr_t *intr,unsigned irq,xnisr_t isr,xniack_t iack,xnflags_t flags)
  * \brief Initialize an interrupt object.
@@ -130,6 +181,8 @@ int xnintr_init (xnintr_t *intr,
     intr->iack = iack;
     intr->cookie = NULL;
     intr->hits = 0;
+    intr->flags = flags;
+    intr->next = NULL;
 
     return 0;
 }
@@ -164,7 +217,8 @@ int xnintr_init (xnintr_t *intr,
 int xnintr_destroy (xnintr_t *intr)
 
 {
-    return xnintr_detach(intr);
+    xnintr_detach(intr);
+    return 0;
 }
 
 /*! 
@@ -181,8 +235,7 @@ int xnintr_destroy (xnintr_t *intr)
  *
  * @param cookie A user-defined opaque value which is stored into the
  * interrupt object descriptor for further retrieval by the ISR/ISR
- * handlers.
- *
+ * handlers. *
  * @return 0 is returned on success. Otherwise, -EINVAL is returned if
  * a low-level error occurred while attaching the interrupt. -EBUSY is
  * specifically returned if the interrupt object was already attached.
@@ -204,9 +257,62 @@ int xnintr_destroy (xnintr_t *intr)
 int xnintr_attach (xnintr_t *intr,
 		   void *cookie)
 {
+    xnintr_shirq_t *shirq;
+    xnintr_t *prev, **p;
+    unsigned long flags;
+    int err = 0, attached = 0;
+
+    if (intr->irq >= XNARCH_NR_IRQS)
+	return -EINVAL;
+
     intr->hits = 0;
+    intr->next = NULL;
     intr->cookie = cookie;
-    return xnarch_hook_irq(intr->irq,&xnintr_irq_handler,intr->iack,intr);
+    shirq = &xnshirqs[intr->irq];
+
+    flags = xnarch_critical_enter(NULL);
+
+    if (testbits(intr->flags,XN_ISR_ATTACHED))
+	{
+	err = -EPERM;
+	goto unlock_and_exit;
+	}
+
+    p = &shirq->handlers;
+
+    if ((prev = *p) != NULL)
+	{
+	/* Check on whether the shared mode is allowed. */
+	if (!(prev->flags & intr->flags & XN_ISR_SHARED) || (prev->iack != intr->iack))
+	    {
+	    err = -EBUSY;
+	    goto unlock_and_exit;
+	    }
+
+	/* Get a position at the end of the list to insert the new element. */
+	while (prev)
+	    {
+	    p = &prev->next;
+	    prev = *p;
+	    }
+	}
+    else
+	{
+	/* Initialize the corresponding interrupt channel */
+	err = xnarch_hook_irq(intr->irq,&xnintr_irq_handler,intr->iack,NULL);
+	if (err)
+	    goto unlock_and_exit;
+	}
+
+    setbits(intr->flags,XN_ISR_ATTACHED);
+
+    /* Add a given interrupt object. */
+    *p = intr;
+
+unlock_and_exit:
+
+    xnarch_critical_exit(flags);
+    return err;
 }
 
 /*! 
@@ -241,7 +347,52 @@ int xnintr_attach (xnintr_t *intr,
 int xnintr_detach (xnintr_t *intr)
 
 {
-    return xnarch_release_irq(intr->irq);
+    xnintr_shirq_t *shirq;
+    unsigned long flags;
+    xnintr_t *e, **p;
+    int err = 0;
+
+    if (intr->irq >= XNARCH_NR_IRQS)
+	return -EINVAL;
+
+    shirq = &xnshirqs[intr->irq];
+
+    flags = xnarch_critical_enter(NULL);
+
+    if (!testbits(intr->flags,XN_ISR_ATTACHED))
+	{
+	xnarch_critical_exit(flags);
+	return -EPERM;
+	}
+
+    clrbits(intr->flags,XN_ISR_ATTACHED);
+
+    p = &shirq->handlers;
+
+    while ((e = *p) != NULL)
+	{
+	if (e == intr)
+	    {
+	    /* Remove a given interrupt object from the list. */
+	    if ((*p = e->next) == NULL)
+		err = xnarch_release_irq(intr->irq);
+    
+	    xnarch_critical_exit(flags);
+
+	    /* The idea here is to keep a detached interrupt object valid as long
+	       as the corresponding irq handler is running. This is one of the requirements
+	       to iterate over the xnintr_shirq_t::handlers list in xnintr_irq_handler()
+	       in a lockless way. */
+	    xnintr_shirq_spin(shirq);
+	    return err;
+	    }
+	p = &e->next;
+	}
+
+    xnarch_critical_exit(flags);
+
+    xnlogerr("Attempted to detach a non previously attached interrupt object");
+    return err;
 }
 
 /*! 
@@ -350,17 +501,39 @@ static void xnintr_irq_handler (unsigned
 
 {
     xnsched_t *sched = xnpod_current_sched();
-    xnintr_t *intr = (xnintr_t *)cookie;
-    int s;
+    xnintr_shirq_t *shirq;
+    xnintr_t *intr;
+    int s = 0;
 
     xnarch_memory_barrier();
 
     xnltt_log_event(xeno_ev_ienter,irq);
 
     ++sched->inesting;
-    s = intr->isr(intr);
+
+    if (irq == XNARCH_TIMER_IRQ)
+	{
+	intr = (xnintr_t*)cookie;
+	s = intr->isr(intr);
+	++intr->hits;
+	}
+    else
+	{
+	shirq = &xnshirqs[irq];
+	intr = shirq->handlers;
+
+	xnintr_shirq_lock(shirq);
+	while (intr)
+	    {
+	    s |= intr->isr(intr);
+	    ++intr->hits;
+
+	    intr = intr->next;
+	    }
+	xnintr_shirq_unlock(shirq);
+	}
+
     --sched->inesting;
-    ++intr->hits;
 
     if (s & XN_ISR_ENABLE)
 	xnarch_enable_irq(irq);
@@ -384,6 +557,7 @@ static void xnintr_irq_handler (unsigned
     xnltt_log_event(xeno_ev_iexit,irq);
 }
 
+
 /*@}*/
 
 EXPORT_SYMBOL(xnintr_attach);
diff -urp xenomai-cvs/ksrc/nucleus/module.c xenomai/ksrc/nucleus/module.c
--- xenomai-cvs/ksrc/nucleus/module.c	2005-11-30 10:36:33.000000000 +0100
+++ xenomai/ksrc/nucleus/module.c	2006-01-17 20:42:09.000000000 +0100
@@ -713,6 +713,8 @@ int __init __xeno_sys_init (void)
     xnpod_init_proc();
 #endif /* CONFIG_PROC_FS */
 
+    xnintr_mount();
+
 #ifdef CONFIG_LTT
     xnltt_mount();
 #endif /* CONFIG_LTT */
diff -urp xenomai-cvs/ksrc/nucleus/pod.c xenomai/ksrc/nucleus/pod.c
--- xenomai-cvs/ksrc/nucleus/pod.c	2006-01-10 21:47:24.000000000 +0100
+++ xenomai/ksrc/nucleus/pod.c	2006-01-23 10:58:44.000000000 +0100
@@ -3038,11 +3038,10 @@ unlock_and_exit:
 
     /* The clock interrupt does not need to be attached since the
        timer service will handle the arch-dependent setup. The IRQ
-       number (arg #2) is not used since the IRQ source will be
-       attached directly by the arch-dependent layer
+       source will be attached directly by the arch-dependent layer
        (xnarch_start_timer). */
 
-    xnintr_init(&nkclock,0,tickhandler,NULL,0);
+    xnintr_init(&nkclock,XNARCH_TIMER_IRQ,tickhandler,NULL,0);
 
     __setbits(nkpod->status,XNTIMED);
 

Re: [Xenomai-core] [PATCH] Shared irqs v.6

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

Dmitry Adamushko wrote:
> Hi,
> 
> the previous version contained an ugly bug, namely the misuse of the
> test_and_set/clear_bit interface
> that led to a broken system from time to time.
> 

What about the ISA/edge-triggered stuff? :D

My problem is that we cannot test here on real hardware because all our
shared IRQs reside on PC104 cards.

Jan


PS: I guess this one does not add much functionality: ;)

@@ -181,8 +235,7 @@ int xnintr_destroy (xnintr_t *intr)
  *
  * @param cookie A user-defined opaque value which is stored into the
  * interrupt object descriptor for further retrieval by the ISR/ISR
- * handlers.
- *
+ * handlers. *
  * @return 0 is returned on success. Otherwise, -EINVAL is returned if
  * a low-level error occurred while attaching the interrupt. -EBUSY is
  * specifically returned if the interrupt object was already attached.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-core] [PATCH] Shared irqs v.6

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

On 31/01/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>
> Dmitry Adamushko wrote:
> > Hi,
> >
> > the previous version contained an ugly bug, namely the misuse of the
> > test_and_set/clear_bit interface
> > that led to a broken system from time to time.
> >
>
> What about the ISA/edge-triggered stuff? :D
>
> My problem is that we cannot test here on real hardware because all our
> shared IRQs reside on PC104 cards.


Could you provide me with some real piece of such code as an example?

Anyway, design-wise, it's about introducing a separate xnintr_handler() for
such a special case to avoid getting a mixture with the rest of "sane" code.
I mean that the support of shared interrupts for ISA boards (edge-triggered
stuff) is a kind of emulation to overcome the shortcommings of the initial
design on the hardware level. The hardware was just not supposed to support
shared interrupt channels. So, let's keep it a bit aside from another code
:o)

xnintr_attach() will check for a special flag like XN_ISR_ISASHARED and
attach e.g. xnintr_isairq_handler() instead of xnintr_irq_handler().

I'll post an extension to the rt_intr_create(..., const char *name, ...)
tomorrow - made but not tested yet -

as well as the /proc support.


Jan
>


--
Best regards,
Dmitry Adamushko

[-- Attachment #2: Type: text/html, Size: 1858 bytes --]

Re: [Xenomai-core] [PATCH] Shared irqs v.6

[Jan Kiszka]

Dmitry Adamushko wrote:
> On 31/01/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>> Dmitry Adamushko wrote:
>>> Hi,
>>>
>>> the previous version contained an ugly bug, namely the misuse of the
>>> test_and_set/clear_bit interface
>>> that led to a broken system from time to time.
>>>
>> What about the ISA/edge-triggered stuff? :D
>>
>> My problem is that we cannot test here on real hardware because all our
>> shared IRQs reside on PC104 cards.
> 
> 
> Could you provide me with some real piece of such code as an example?

May I remind you of the link to the linux kernel's 8250 driver I gave?
Or what do you mean with real code? xeno_16550A would become such a user
ideally only by adding one or two IRQ-registration flags.

> 
> Anyway, design-wise, it's about introducing a separate xnintr_handler() for
> such a special case to avoid getting a mixture with the rest of "sane" code.

Yep, I would vote for this approach as well.

> I mean that the support of shared interrupts for ISA boards (edge-triggered
> stuff) is a kind of emulation to overcome the shortcommings of the initial
> design on the hardware level. The hardware was just not supposed to support
> shared interrupt channels. So, let's keep it a bit aside from another code
> :o)

Unfortunately, this crappy hardware is still quite common in the
embedded domain.

> 
> xnintr_attach() will check for a special flag like XN_ISR_ISASHARED and
> attach e.g. xnintr_isairq_handler() instead of xnintr_irq_handler().
> 
> I'll post an extension to the rt_intr_create(..., const char *name, ...)
> tomorrow - made but not tested yet -
> 
> as well as the /proc support.
> 

This should be easy... Great!

Jan

Re: [Xenomai-core] [PATCH] Shared irqs v.6

[Jeroen Van den Keybus]

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

>
> > I mean that the support of shared interrupts for ISA boards
> (edge-triggered
> > stuff) is a kind of emulation to overcome the shortcommings of the
> initial
> > design on the hardware level. The hardware was just not supposed to
> support
> > shared interrupt channels. So, let's keep it a bit aside from another
> code
> > :o)
>
> Unfortunately, this crappy hardware is still quite common in the
> embedded domain.


If I've understood correctly, the only reason for having this support is to
avoid - after discovering an interrupting UART - that the IRQ line remains
high upon exit, which would cause the 8259 not to issue the CPU IRQ for that
line anymore ?

The proposed solution is therefore to traverse the entire list of
UARTs connected to that IRQ line and make sure none of them was interrupting
in two consecutive passes (by checking their status registers). That would
mean the IRQ line must be deasserted and the 8259 will properly detect any
newly arriving interrupts, since the 8259 has been acknowledged before
handling the interrupt.

1. Wouldn't it be more efficient to make this a compile-time option, instead
of burdening the nucleus with it ?
2. Would it be an option, in the embedded boards Jan is speaking of, to put
the 8259 in level sensitive mode ? Some of the boards I know don't actually
have this selection logic for the built-in interrupt sources such as the
timer and the IDE I/F and it therefore only applies to the ISA bus.
3. Beware of UARTs that cause interrupts and have a problem that causes them
to spuriously return 'all green' upon reading of the IIR register. I have
dealt with an integrated ('Super I/O') card with that problem before. The
only solution was to look at LSR and check THRE as well, even when no TX
interrupt was seemingly present. Must be a logic race.

Jeroen.

[-- Attachment #2: Type: text/html, Size: 2172 bytes --]

Re: [Xenomai-core] [PATCH] Shared irqs v.6

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]

Jeroen Van den Keybus wrote:
>>> I mean that the support of shared interrupts for ISA boards
>> (edge-triggered
>>> stuff) is a kind of emulation to overcome the shortcommings of the
>> initial
>>> design on the hardware level. The hardware was just not supposed to
>> support
>>> shared interrupt channels. So, let's keep it a bit aside from another
>> code
>>> :o)
>> Unfortunately, this crappy hardware is still quite common in the
>> embedded domain.
> 
> 
> If I've understood correctly, the only reason for having this support is to
> avoid - after discovering an interrupting UART - that the IRQ line remains
> high upon exit, which would cause the 8259 not to issue the CPU IRQ for that
> line anymore ?

Yep.

> 
> The proposed solution is therefore to traverse the entire list of
> UARTs connected to that IRQ line and make sure none of them was interrupting
> in two consecutive passes (by checking their status registers). That would
> mean the IRQ line must be deasserted and the 8259 will properly detect any
> newly arriving interrupts, since the 8259 has been acknowledged before
> handling the interrupt.

Yep.

> 
> 1. Wouldn't it be more efficient to make this a compile-time option, instead
> of burdening the nucleus with it ?

You need this feature per-IRQ as there will always be also usual
level-triggered IRQs on your system. What Dmitry is now heading for is a
runtime selection of the fitting IRQ trampoline at nucleus level.

> 2. Would it be an option, in the embedded boards Jan is speaking of, to put
> the 8259 in level sensitive mode ? Some of the boards I know don't actually
> have this selection logic for the built-in interrupt sources such as the
> timer and the IDE I/F and it therefore only applies to the ISA bus.

Might be an option, but I'm not THAT deep into it to asses all possible
pitfalls of such an approach. Has anyone ever tried this in reality?

> 3. Beware of UARTs that cause interrupts and have a problem that causes them
> to spuriously return 'all green' upon reading of the IIR register. I have
> dealt with an integrated ('Super I/O') card with that problem before. The
> only solution was to look at LSR and check THRE as well, even when no TX
> interrupt was seemingly present. Must be a logic race.

Yes, there is always the risk of running onto such hardware. Luckily, we
haven't "found" any of it on our systems so far (once you picked
reasonable hardware, you should stay with it - just like we do :) ).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-core] [PATCH] Shared irqs v.6

[Jan Kiszka]

[-- Attachment #1.1: Type: text/plain, Size: 520 bytes --]

Dmitry Adamushko wrote:
> Hi,
> 
> the previous version contained an ugly bug, namely the misuse of the
> test_and_set/clear_bit interface
> that led to a broken system from time to time.
> 

And here is a tiny patch to make RTDM shared-IRQ-aware. If you release
another version before a merge, you may include it into your patch.

Note that I struggled a bit with finding the right prefix. I decided to
separate the namespaces of return flags and creation flags - hopefully
in a reasonable manner.

Jan

[-- Attachment #1.2: rtdm_shared_irq.patch --]
[-- Type: text/plain, Size: 1403 bytes --]

Index: ksrc/skins/rtdm/drvlib.c
===================================================================
--- ksrc/skins/rtdm/drvlib.c	(revision 503)
+++ ksrc/skins/rtdm/drvlib.c	(working copy)
@@ -1200,7 +1200,7 @@
  * @param[in,out] irq_handle IRQ handle
  * @param[in] irq_no Line number of the addressed IRQ
  * @param[in] handler Interrupt handler
- * @param[in] flags Currently unused, pass 0
+ * @param[in] flags Registration flags, see @ref RTDM_IRQTYPE_xxx for details
  * @param[in] device_name Optional device name to show up in real-time IRQ
  * lists (not yet implemented)
  * @param[in] arg Pointer to be passed to the interrupt handler on invocation
Index: include/rtdm/rtdm_driver.h
===================================================================
--- include/rtdm/rtdm_driver.h	(revision 503)
+++ include/rtdm/rtdm_driver.h	(working copy)
@@ -693,6 +693,15 @@
 
 typedef xnintr_t                    rtdm_irq_t;
 
+/*!
+ * @anchor RTDM_IRQTYPE_xxx   @name RTDM_IRQTYPE_xxx
+ * Interrupt registrations flags
+ * @{
+ */
+/** Enable IRQ-sharing with other real-time drivers */
+#define RTDM_IRQTYPE_SHARED         XN_ISR_SHARED
+/** @} */
+
 /**
  * Interrupt handler
  *
@@ -702,7 +711,6 @@
  */
 typedef int (*rtdm_irq_handler_t)(rtdm_irq_t *irq_handle);
 
-
 /*!
  * @anchor RTDM_IRQ_xxx   @name RTDM_IRQ_xxx
  * Return flags of interrupt handlers

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

[Xenomai-core] Are XN_ISR_CHAINED and XN_ISR_ENABLE mutually exclusive?

[Anders Blomdell]

While looking into how to implement sharing of interrupts between realtime and 
non-realtime domains (and applying Wolfgang Grandegger's patch 
[https://mail.gna.org/public/xenomai-core/2006-01/msg00233.html], which is 
necessary to make XN_ISR_ENABLE work at all on the PowerPC platform), I'm 
beginning to think that XN_ISR_CHAINED and XN_ISR_ENABLE are mutually exclusive, 
since if both are set, desc->handler->end will be called twice:

   1. When the realtime isr handler returns
   2. When the Linux domain calls it in __do_IRQ

In the solution I have in mind at the moment, I will:

   1. Add an extra iend handler argument to xnintr_init
   2. If XN_ISR_ENABLE is returned from the isr handler,
      replace desc->handler->end with the user supplied
      iend handler.

Hereby I hope to be able to handle interrupts shared between realtime and 
non-realtime domain, without having the realtime domain wait for all 
non-realtime interrupts to finish. This is the scenario I'm thinking of:

   1. A non-RT interrupt occurs
   2. The (RT) isr handler detects the non-RT interrupt,
      disables further non-RT interrupts on that irq-vector, replaces
      desc->handler->end with the user supplied iend handler,
      returns XN_ISR_CHAINED | XN_ISR_ENABLE.
   3. RT interrupts are serviced by the (RT) isr handler,
      returns XN_ISR_ENABLE
   4. The Linux domain get a chance to run the chained interrupt,
      and eventually calls desc->handler->end (supplied iend handler)
   5. The iend handler reenables non-RT interrupts.

Comments on the above are most welcome!

-- 

Anders Blomdell

Re: [Xenomai-core] Are XN_ISR_CHAINED and XN_ISR_ENABLE mutually exclusive?

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]

Anders Blomdell wrote:
> While looking into how to implement sharing of interrupts between
> realtime and non-realtime domains (and applying Wolfgang Grandegger's
> patch [https://mail.gna.org/public/xenomai-core/2006-01/msg00233.html],
> which is necessary to make XN_ISR_ENABLE work at all on the PowerPC
> platform), I'm beginning to think that XN_ISR_CHAINED and XN_ISR_ENABLE
> are mutually exclusive, since if both are set, desc->handler->end will
> be called twice:
> 
>   1. When the realtime isr handler returns
>   2. When the Linux domain calls it in __do_IRQ

Yes, those bits are semantically exclusive. Actually, I think passing
both bits could even cause deadlocks if the RT-IRQ is raised again
before the non-RT handler got a chance to clear the IRQ source in hardware.

> 
> In the solution I have in mind at the moment, I will:
> 
>   1. Add an extra iend handler argument to xnintr_init
>   2. If XN_ISR_ENABLE is returned from the isr handler,
>      replace desc->handler->end with the user supplied
>      iend handler.
> 
> Hereby I hope to be able to handle interrupts shared between realtime
> and non-realtime domain, without having the realtime domain wait for all
> non-realtime interrupts to finish. This is the scenario I'm thinking of:
> 
>   1. A non-RT interrupt occurs
>   2. The (RT) isr handler detects the non-RT interrupt,
>      disables further non-RT interrupts on that irq-vector, replaces

This remains vague to me. How precisely will you disable? I guess at
hardware level, i.e. in a (non-RT) device-specific way: switch off the
bit in some hardware register that says "this device can produce IRQs",
right?

>      desc->handler->end with the user supplied iend handler,
>      returns XN_ISR_CHAINED | XN_ISR_ENABLE.
>   3. RT interrupts are serviced by the (RT) isr handler,
>      returns XN_ISR_ENABLE
>   4. The Linux domain get a chance to run the chained interrupt,
>      and eventually calls desc->handler->end (supplied iend handler)
>   5. The iend handler reenables non-RT interrupts.

Then this would switch on that bit again? Note that this may require to
synchronise the hardware access with parts of the non-RT driver.

Meanwhile I recalled that my hack to realise IRQ sharing between a RT
device and a non-RT eepro100 is filed on a public archive:

https://mail.gna.org/public/xenomai-core/2005-11/msg00012.html

> 
> Comments on the above are most welcome!
> 

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-core] Are XN_ISR_CHAINED and XN_ISR_ENABLE mutually exclusive?

[Anders Blomdell]

Jan Kiszka wrote:
> Anders Blomdell wrote:
> 
>>While looking into how to implement sharing of interrupts between
>>realtime and non-realtime domains (and applying Wolfgang Grandegger's
>>patch [https://mail.gna.org/public/xenomai-core/2006-01/msg00233.html],
>>which is necessary to make XN_ISR_ENABLE work at all on the PowerPC
>>platform), I'm beginning to think that XN_ISR_CHAINED and XN_ISR_ENABLE
>>are mutually exclusive, since if both are set, desc->handler->end will
>>be called twice:
>>
>>  1. When the realtime isr handler returns
>>  2. When the Linux domain calls it in __do_IRQ
> 
> 
> Yes, those bits are semantically exclusive. Actually, I think passing
> both bits could even cause deadlocks if the RT-IRQ is raised again
> before the non-RT handler got a chance to clear the IRQ source in hardware.
My impression as well, but it's nowhere documented, nor enforced in the code.

> 
> 
>>In the solution I have in mind at the moment, I will:
>>
>>  1. Add an extra iend handler argument to xnintr_init
>>  2. If XN_ISR_ENABLE is returned from the isr handler,
>>     replace desc->handler->end with the user supplied
>>     iend handler.
>>
>>Hereby I hope to be able to handle interrupts shared between realtime
>>and non-realtime domain, without having the realtime domain wait for all
>>non-realtime interrupts to finish. This is the scenario I'm thinking of:
>>
>>  1. A non-RT interrupt occurs
>>  2. The (RT) isr handler detects the non-RT interrupt,
>>     disables further non-RT interrupts on that irq-vector, replaces
> 
> 
> This remains vague to me. How precisely will you disable? I guess at
> hardware level, i.e. in a (non-RT) device-specific way: switch off the
> bit in some hardware register that says "this device can produce IRQs",
> right?
Yes.

> 
> 
>>     desc->handler->end with the user supplied iend handler,
>>     returns XN_ISR_CHAINED | XN_ISR_ENABLE.
>>  3. RT interrupts are serviced by the (RT) isr handler,
>>     returns XN_ISR_ENABLE
>>  4. The Linux domain get a chance to run the chained interrupt,
>>     and eventually calls desc->handler->end (supplied iend handler)
>>  5. The iend handler reenables non-RT interrupts.
> 
> 
> Then this would switch on that bit again? Note that this may require to
> synchronise the hardware access with parts of the non-RT driver.
If the non-RT driver sets that bit in its ISR routine, yes. I have the (overly 
optimistic?) view that the non-RT ISR only does whatever is necessary to clear 
the interrupt and leaves the enable/disable bits untouched.

>  Meanwhile I recalled that my hack to realise IRQ sharing between a RT
> device and a non-RT eepro100 is filed on a public archive:
> 
> https://mail.gna.org/public/xenomai-core/2005-11/msg00012.html
Thanks, I'll take a look at it.

-- 

Anders Blomdell

Re: [Xenomai-core] Are XN_ISR_CHAINED and XN_ISR_ENABLE mutually exclusive?

[Anders Blomdell]

Anders Blomdell wrote:
> Jan Kiszka wrote:
> 
>> Anders Blomdell wrote:
>>
>>> While looking into how to implement sharing of interrupts between
>>> realtime and non-realtime domains (and applying Wolfgang Grandegger's
>>> patch [https://mail.gna.org/public/xenomai-core/2006-01/msg00233.html],
>>> which is necessary to make XN_ISR_ENABLE work at all on the PowerPC
>>> platform), I'm beginning to think that XN_ISR_CHAINED and XN_ISR_ENABLE
>>> are mutually exclusive, since if both are set, desc->handler->end will
>>> be called twice:
>>>
>>>  1. When the realtime isr handler returns
>>>  2. When the Linux domain calls it in __do_IRQ
>>
>>
>>
>> Yes, those bits are semantically exclusive. Actually, I think passing
>> both bits could even cause deadlocks if the RT-IRQ is raised again
>> before the non-RT handler got a chance to clear the IRQ source in 
>> hardware.
> 
> My impression as well, but it's nowhere documented, nor enforced in the 
> code.
> 
>>
>>
>>> In the solution I have in mind at the moment, I will:
>>>
>>>  1. Add an extra iend handler argument to xnintr_init
>>>  2. If XN_ISR_ENABLE is returned from the isr handler,
>>>     replace desc->handler->end with the user supplied
>>>     iend handler.
>>>
>>> Hereby I hope to be able to handle interrupts shared between realtime
>>> and non-realtime domain, without having the realtime domain wait for all
>>> non-realtime interrupts to finish. This is the scenario I'm thinking of:
>>>
>>>  1. A non-RT interrupt occurs
>>>  2. The (RT) isr handler detects the non-RT interrupt,
>>>     disables further non-RT interrupts on that irq-vector, replaces
>>
>>
>>
>> This remains vague to me. How precisely will you disable? I guess at
>> hardware level, i.e. in a (non-RT) device-specific way: switch off the
>> bit in some hardware register that says "this device can produce IRQs",
>> right?
> 
> Yes.
> 
>>
>>
>>>     desc->handler->end with the user supplied iend handler,
>>>     returns XN_ISR_CHAINED | XN_ISR_ENABLE.
>>>  3. RT interrupts are serviced by the (RT) isr handler,
>>>     returns XN_ISR_ENABLE
>>>  4. The Linux domain get a chance to run the chained interrupt,
>>>     and eventually calls desc->handler->end (supplied iend handler)
>>>  5. The iend handler reenables non-RT interrupts.
>>
>>
>>
>> Then this would switch on that bit again? Note that this may require to
>> synchronise the hardware access with parts of the non-RT driver.
> 
> If the non-RT driver sets that bit in its ISR routine, yes. I have the 
> (overly optimistic?) view that the non-RT ISR only does whatever is 
> necessary to clear the interrupt and leaves the enable/disable bits 
> untouched.
Or perhaps the whole conceptis of no interest to others, and I should put this 
arbitration in the platform specific part (arch/ppc/platform/prpmc800.c) and 
consider the harrier chip as a cascaded interrupt controller, and handle it as such?

-- 

Anders Blomdell

[Xenomai-core] [BUG] problems with adeos-ipipe-2.6.14-ppc-1.2-00.patch

[Anders Blomdell]

When trying to patch with latest version of this patch, I get:

patching file include/asm-ppc/ipipe.h
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 149.
Hunk #3 FAILED at 160.
Hunk #4 FAILED at 195.

Problem seems to be at line 4168 in the patch, where it says

@@ -0,1 +1,179 @@

but the old [working] patch said

@@ -0,0 +1,178 @@

Seems like the patch is created againt a not totally clean distribution.

-- 

Regards

Anders Blomdell

Re: [Xenomai-core] [BUG] problems with adeos-ipipe-2.6.14-ppc-1.2-00.patch

[Philippe Gerum]

Anders Blomdell wrote:
> When trying to patch with latest version of this patch, I get:
> 
> patching file include/asm-ppc/ipipe.h
> Hunk #1 FAILED at 1.
> Hunk #2 FAILED at 149.
> Hunk #3 FAILED at 160.
> Hunk #4 FAILED at 195.
> 
> Problem seems to be at line 4168 in the patch, where it says
> 
> @@ -0,1 +1,179 @@
> 
> but the old [working] patch said
> 
> @@ -0,0 +1,178 @@
> 
> Seems like the patch is created againt a not totally clean distribution.
> 

Hopefully fixed for good this time. Thanks.

-- 

Philippe.

[Xenomai-core] [PATCH] Slow is faster arch/ppc/syslib/open_pic.c

[Anders Blomdell]

When trying to run Xenomai on PowerPC with OpenPIC, I have (finally) found that 
interrupt latency is much improved with the following patch:



--- arch/ppc/syslib/open_pic.c~ 2006-01-08 03:15:24.000000000 +0100
+++ arch/ppc/syslib/open_pic.c  2006-02-07 16:56:14.000000000 +0100
@@ -820,7 +820,7 @@
   */
  static void openpic_ack_irq(unsigned int irq_nr)
  {
-#ifdef __SLOW_VERSION__
+#if defined(__SLOW_VERSION__) || defined(CONFIG_IPIPE)
         openpic_disable_irq(irq_nr);
         openpic_eoi();
  #else
@@ -831,7 +831,7 @@

  static void openpic_end_irq(unsigned int irq_nr)
  {
-#ifdef __SLOW_VERSION__
+#if defined(__SLOW_VERSION__) || defined(CONFIG_IPIPE)
         if (!(irq_desc[irq_nr].status & (IRQ_DISABLED|IRQ_INPROGRESS))
             && irq_desc[irq_nr].action)
                 openpic_enable_irq(irq_nr);



The reason for this, is that the fast version doesn't call openpic_eoi until the 
interrupt is ended, which means that all RT-interrupts are delayed by a pending 
Linux interrupt.

-- 

Regards

Anders Blomdell

Re: [Xenomai-core] [PATCH] Slow is faster arch/ppc/syslib/open_pic.c

[Philippe Gerum]

Anders Blomdell wrote:
> When trying to run Xenomai on PowerPC with OpenPIC, I have (finally) 
> found that interrupt latency is much improved with the following patch:
> 
> 
> 
> --- arch/ppc/syslib/open_pic.c~ 2006-01-08 03:15:24.000000000 +0100
> +++ arch/ppc/syslib/open_pic.c  2006-02-07 16:56:14.000000000 +0100
> @@ -820,7 +820,7 @@
>   */
>  static void openpic_ack_irq(unsigned int irq_nr)
>  {
> -#ifdef __SLOW_VERSION__
> +#if defined(__SLOW_VERSION__) || defined(CONFIG_IPIPE)
>         openpic_disable_irq(irq_nr);
>         openpic_eoi();
>  #else
> @@ -831,7 +831,7 @@
> 
>  static void openpic_end_irq(unsigned int irq_nr)
>  {
> -#ifdef __SLOW_VERSION__
> +#if defined(__SLOW_VERSION__) || defined(CONFIG_IPIPE)
>         if (!(irq_desc[irq_nr].status & (IRQ_DISABLED|IRQ_INPROGRESS))
>             && irq_desc[irq_nr].action)
>                 openpic_enable_irq(irq_nr);
> 
> 
> 
> The reason for this, is that the fast version doesn't call openpic_eoi 
> until the interrupt is ended, which means that all RT-interrupts are 
> delayed by a pending Linux interrupt.
> 

Gasp. Will check on my Icecube asap, thanks (a lot).

-- 

Philippe.

Re: [Xenomai-core] [PATCH] Slow is faster arch/ppc/syslib/open_pic.c

[Philippe Gerum]

Philippe Gerum wrote:
> Anders Blomdell wrote:
> 
>> When trying to run Xenomai on PowerPC with OpenPIC, I have (finally) 
>> found that interrupt latency is much improved with the following patch:
>>
>>
>>
>> --- arch/ppc/syslib/open_pic.c~ 2006-01-08 03:15:24.000000000 +0100
>> +++ arch/ppc/syslib/open_pic.c  2006-02-07 16:56:14.000000000 +0100
>> @@ -820,7 +820,7 @@
>>   */
>>  static void openpic_ack_irq(unsigned int irq_nr)
>>  {
>> -#ifdef __SLOW_VERSION__
>> +#if defined(__SLOW_VERSION__) || defined(CONFIG_IPIPE)
>>         openpic_disable_irq(irq_nr);
>>         openpic_eoi();
>>  #else
>> @@ -831,7 +831,7 @@
>>
>>  static void openpic_end_irq(unsigned int irq_nr)
>>  {
>> -#ifdef __SLOW_VERSION__
>> +#if defined(__SLOW_VERSION__) || defined(CONFIG_IPIPE)
>>         if (!(irq_desc[irq_nr].status & (IRQ_DISABLED|IRQ_INPROGRESS))
>>             && irq_desc[irq_nr].action)
>>                 openpic_enable_irq(irq_nr);
>>
>>
>>
>> The reason for this, is that the fast version doesn't call openpic_eoi 
>> until the interrupt is ended, which means that all RT-interrupts are 
>> delayed by a pending Linux interrupt.
>>
> 
> Gasp. Will check on my Icecube asap, thanks (a lot).
> 

The mpc52xx is using its own version of PIC management - which should not induce 
such delay on eoi, so I cannot experiment this change yet. However, I've revisited 
your patch so that the OpenPIC code always sends eoi in fast mode, regardless of 
the interrupt polarity. Could you try the patch below and let me know of the 
outcome? TIA,

--- arch/ppc/syslib/open_pic.c~	2005-10-28 02:02:08.000000000 +0200
+++ arch/ppc/syslib/open_pic.c	2006-02-08 10:30:22.000000000 +0100
@@ -824,7 +824,9 @@
  	openpic_disable_irq(irq_nr);
  	openpic_eoi();
  #else
+#ifndef CONFIG_IPIPE
  	if ((irq_desc[irq_nr].status & IRQ_LEVEL) == 0)
+#endif /* CONFIG_IPIPE */
  		openpic_eoi();
  #endif
  }
@@ -836,8 +838,10 @@
  	    && irq_desc[irq_nr].action)
  		openpic_enable_irq(irq_nr);
  #else
+#ifndef CONFIG_IPIPE
  	if ((irq_desc[irq_nr].status & IRQ_LEVEL) != 0)
  		openpic_eoi();
+#endif /* CONFIG_IPIPE */
  #endif
  }


-- 

Philippe.

Re: [Xenomai-core] [PATCH] Slow is faster arch/ppc/syslib/open_pic.c

[Philippe Gerum]

Philippe Gerum wrote:
> Philippe Gerum wrote:
> 
>> Anders Blomdell wrote:
>>
>>> When trying to run Xenomai on PowerPC with OpenPIC, I have (finally) 
>>> found that interrupt latency is much improved with the following patch:
>>>
>>>
>>>
>>> --- arch/ppc/syslib/open_pic.c~ 2006-01-08 03:15:24.000000000 +0100
>>> +++ arch/ppc/syslib/open_pic.c  2006-02-07 16:56:14.000000000 +0100
>>> @@ -820,7 +820,7 @@
>>>   */
>>>  static void openpic_ack_irq(unsigned int irq_nr)
>>>  {
>>> -#ifdef __SLOW_VERSION__
>>> +#if defined(__SLOW_VERSION__) || defined(CONFIG_IPIPE)
>>>         openpic_disable_irq(irq_nr);
>>>         openpic_eoi();
>>>  #else
>>> @@ -831,7 +831,7 @@
>>>
>>>  static void openpic_end_irq(unsigned int irq_nr)
>>>  {
>>> -#ifdef __SLOW_VERSION__
>>> +#if defined(__SLOW_VERSION__) || defined(CONFIG_IPIPE)
>>>         if (!(irq_desc[irq_nr].status & (IRQ_DISABLED|IRQ_INPROGRESS))
>>>             && irq_desc[irq_nr].action)
>>>                 openpic_enable_irq(irq_nr);
>>>
>>>
>>>
>>> The reason for this, is that the fast version doesn't call 
>>> openpic_eoi until the interrupt is ended, which means that all 
>>> RT-interrupts are delayed by a pending Linux interrupt.
>>>
>>
>> Gasp. Will check on my Icecube asap, thanks (a lot).
>>
> 
> The mpc52xx is using its own version of PIC management - which should 
> not induce such delay on eoi, so I cannot experiment this change yet. 
> However, I've revisited your patch so that the OpenPIC code always sends 
> eoi in fast mode, regardless of the interrupt polarity. Could you try 
> the patch below and let me know of the outcome? TIA,
> 

Ok. Drop this to /dev/null. I've misread the original ->ack() code, and as you 
pointed out, this patch would not work. -ENOBRAIN again.

> --- arch/ppc/syslib/open_pic.c~    2005-10-28 02:02:08.000000000 +0200
> +++ arch/ppc/syslib/open_pic.c    2006-02-08 10:30:22.000000000 +0100
> @@ -824,7 +824,9 @@
>      openpic_disable_irq(irq_nr);
>      openpic_eoi();
>  #else
> +#ifndef CONFIG_IPIPE
>      if ((irq_desc[irq_nr].status & IRQ_LEVEL) == 0)
> +#endif /* CONFIG_IPIPE */
>          openpic_eoi();
>  #endif
>  }
> @@ -836,8 +838,10 @@
>          && irq_desc[irq_nr].action)
>          openpic_enable_irq(irq_nr);
>  #else
> +#ifndef CONFIG_IPIPE
>      if ((irq_desc[irq_nr].status & IRQ_LEVEL) != 0)
>          openpic_eoi();
> +#endif /* CONFIG_IPIPE */
>  #endif
>  }
> 
> 


-- 

Philippe.