Dmitry Adamushko wrote:
> For RTDM I'm now almost determined to rework the API in way that only
> HANDLED/UNHANDLED (or what ever their names will be) get exported, any
> additional guru features will remain excluded as long as we have no
> clean usage policy for them.
Good. Then let's go for
HANDLED, UNHANDLED - we may consider them even as 2 scalar values
+
NOENABLE, CHAINED - additional bits.
They are not encouraged to be used with shared interrupts
(explained in docs + debug messages when XENO_OPT_DEBUG is on).
Any ISR on the shared irq line should "understand" that it's
just one among the equals. That said, it should not do anything
that may affect other ISRs and not require any special treatment
(like CHAINED or NOENABLE).
If it wants it indeed, then don't declare itself as SHARED.
We may come back to the topic about possible return values of ISRs
a bit later maybe having got more feedback (hm.. hopefully)
on shared irq support.
>>>
>>> But the later one is not only about enabling the line, but
>>> on some archs - about .end-ing it too (sending EOI).
>>>
>>> And to support HANDLED_NOENABLE properly, those 2 have to be
>>> decoupled, i.e.
>>> EOI should always be sent from xnintr_shirq_handler().
>> But the one returning HANDLED_NOENABLE is likely to leave the interrupt
>> asserted, hence we can't EOI at this point (unless NO_ENABLE means
>> DISABLE).
>
>I guess this is what Dmitry meant: explicitly call disable() if one or
>more ISRs returned NOENABLE - at least on archs where end != enable.
>Will this work? We could then keep on using the existing IRQ-enable API
>from bottom-half IRQ tasks.
Almost.
Let's consider the following only as anorther way of doing some things;
I don't propose to implement it, it's just to illustrate my thoughts.
So one may simply ski-skip-skip it :o)
Let's keep in mind that what is behind .end-ing the IRQ line depends on
archs.
Sometimes end == enable (EOI was sent on .ack step), while in other cases
end == send_EOI [+ re-enabling].
But anyway, all ISRs are running with a given IRQ line disabled.
Supported values : HANDLED, UNHANDLED, PROPAGATE.
nucleus :: xnintr_irq_handler()
{
ret = 0;
...
for each isr in isr_list[ IRQ ]
{
temp = isr->handler();
if (temp > ret)
ret = temp;
}
if (ret == PROPAGATE)
{
// quite dengerous with shared interrupts, be sure you understand
// what you are doing!
xnarch_chain_irq(irq); // will be .end-ed in Linux domain
}
else
{
// HANDLED or UNHANDLED
xnarch_end_irq();
}
...
}
ENABLE or NOENABLE is missing? Nop.
let's say we have 2 rt-ISRs :
isr1 : HANDLED
isr2 : HANDLED + WISH
WISH == I want the IRQ line remain disabled until later
(e.g. bottom half in rt_task will enable it)
How may isr2 express this WISH? Simply, xnarch_irq_disable/enable() should
support an internal counter that allows them to be called in a nested way.
So e.g. if there are 2 consecutive calls to disable_irq(), then
2 calls to enable_irq() are needed to really enable the IRQ line.
This way, the WISH is only about directly calling xnarch_irq_disable()
in isr2
and there is no need in ENABLE or NOENABLE flags.
This way, PROPAGATE really means NOEND - the IRQ will be .end-ed in
Linux domain;
while WISH==NOENABLE - has nothing to do with sending EOI, but only with
enabling/disabling the given IRQ line.
Yes, if one isr (or a few) defers the IRQ line enabling until later, it
will affect
all others ISR => all interrupts are temporary not accepted on this line.
This scenario is possible under Linux, but should be used with even more
care in real-time system. At least, this way HANDLED_NOENABLE case works
and doesn't lead to lost interrupts on some archs.
Moreover, it avoids the need for ENABLE flag even in non-shared
interrupt case.
Ok, general bottom line regarding IRQ support (and the rest):
1- we want the rules applicable to the common case to be simple, well-defined and
straightforward. This applies to the ISR return value as exposed.
2- some requirements might fall outside of the common case; to support them, we
need to refrain from imposing a strong policy, but only provide sound mechanisms
to implement that. Specifically, let's keep the basic ability to control the Adeos
pipelining from Xenomai intact.
3- however, let's be true to the good old UNIX way-of-life: there is no point in
penalizing 99% of the common user base to please only 1% of them who happen to
have very specific needs. E.g., don't slow down the fast path everyone takes,
don't impose hard rules most of the users won't benefit from. [I'm not referring
to the enable/disable nesting count here: this _is_ correct, and is currently
missing -- this should be done at Xeno's arch-level, not from the HAL which should
be kept the way it is to have a direct, unmoderated access to the PIC handling].
IOW, the patch, the way you discussed it recently, is ok for me too.
--
Philippe.
