N.B. Amongst other things, some thoughts about CHAINED with shared interrupts.
On 20/02/06, Anders Blomdell <[EMAIL PROTECTED]> wrote:
Yep, each ISR may return a different value and all of them are
accumulated in the "s" variable ( s |= intr->isr(intr); ).
So the loop may end up with "s" which contains all of the possible bits:
(e.g.
isr1 - HANDLED | ENABLE
isr2 - HANDLED (don't want the irq to be enabled)
isr3 - CHAINED
)
s = HANDLED | ENABLE | CHAINED;
Then CHAINED will be ignored because of the following code :
+ if (s & XN_ISR_ENABLE)
+ xnarch_end_irq(irq);
+ else if (s & XN_ISR_CHAINED) (*)
+ xnarch_chain_irq(irq);
the current code in the CVS doen not contain "else" in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination.
This said, we suppose that one knows what he is doing.
In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE).
So my feeling is that CHAINED should not be used by drivers which registered their ISRs as SHARED.
Moreover, I actually see the only scenario of CHAINED (I provided it before) :
all ISRs in the primary domain have reported UNHANDLED => nucleus propagates the interrupt down the pipeline with xnacrh_chain_irq().
This call actually returns 1 upon successful propagation (some domain down the pipeline was interested in this irq) and 0 otherwise.
Upon 0, this is a spurious irq (none of domains was interested in its handling).
ok, let's suppose now :
we have 2 ISRs on the same shared line :
isr1 : HANDLED (will be enabled by rt task. Note, rt task must call xnarch_end_irq() and not just xnarch_enable_irq()! )
isr2 : CHAINED
So HANDLED | CHAINED is ok for the single ISR on the line, but it may lead to HANDLED | CHAINED | ENABLE in a case of the shared line.
rt task that works jointly with isr1 just calls xnarch_end_irq() at some moment of time and some ISR in the linux domain does the same later => the line is .end-ed 2 times.
ISR should never return CHAINED as to indicate _only_ that it is not interested in this irq, but ~HANDLED or NOINT (if we'll support it) instead.
If the ISR nevertheless wants to propagate the IRQ to the Linux domain _explicitly_, it _must not_ register itself as SHARED, i.e. it _must_ be the only ISR on this line, otherwise that may lead to the IRQ line being .end-ed twice (lost interrupts in some cases).
Yep, I'd agree with you. Moreover, PROPAGATE should not be used for shared interrupts.
A number of questions arise:
1. What happens if one of the shared handlers leaves the interrupt asserted,
returns NOENABLE|HANDLED and another return only HANDLED?
2. What happens if one returns PROPAGATE and another returns HANDLED?
Yep, each ISR may return a different value and all of them are
accumulated in the "s" variable ( s |= intr->isr(intr); ).
So the loop may end up with "s" which contains all of the possible bits:
(e.g.
isr1 - HANDLED | ENABLE
isr2 - HANDLED (don't want the irq to be enabled)
isr3 - CHAINED
)
s = HANDLED | ENABLE | CHAINED;
Then CHAINED will be ignored because of the following code :
+ if (s & XN_ISR_ENABLE)
+ xnarch_end_irq(irq);
+ else if (s & XN_ISR_CHAINED) (*)
+ xnarch_chain_irq(irq);
the current code in the CVS doen not contain "else" in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination.
This said, we suppose that one knows what he is doing.
In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE).
So my feeling is that CHAINED should not be used by drivers which registered their ISRs as SHARED.
Moreover, I actually see the only scenario of CHAINED (I provided it before) :
all ISRs in the primary domain have reported UNHANDLED => nucleus propagates the interrupt down the pipeline with xnacrh_chain_irq().
This call actually returns 1 upon successful propagation (some domain down the pipeline was interested in this irq) and 0 otherwise.
Upon 0, this is a spurious irq (none of domains was interested in its handling).
ok, let's suppose now :
we have 2 ISRs on the same shared line :
isr1 : HANDLED (will be enabled by rt task. Note, rt task must call xnarch_end_irq() and not just xnarch_enable_irq()! )
isr2 : CHAINED
So HANDLED | CHAINED is ok for the single ISR on the line, but it may lead to HANDLED | CHAINED | ENABLE in a case of the shared line.
rt task that works jointly with isr1 just calls xnarch_end_irq() at some moment of time and some ISR in the linux domain does the same later => the line is .end-ed 2 times.
ISR should never return CHAINED as to indicate _only_ that it is not interested in this irq, but ~HANDLED or NOINT (if we'll support it) instead.
If the ISR nevertheless wants to propagate the IRQ to the Linux domain _explicitly_, it _must not_ register itself as SHARED, i.e. it _must_ be the only ISR on this line, otherwise that may lead to the IRQ line being .end-ed twice (lost interrupts in some cases).
#define UNHANDLED 0
#define HANDLED_ENABLE 1
#define HANDLED_NOENABLE 2
#define PROPAGATE 3
Yep, I'd agree with you. Moreover, PROPAGATE should not be used for shared interrupts.
--
Best regards,
Dmitry Adamushko
