Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)

[Dmitry Adamushko]

> 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.


--
Best regards,
Dmitry Adamushko