Dmitry Adamushko wrote:
> Good point, leaves us with 2 possible return values for shared handlers:
>
> HANDLED
> NOT_HANDLED
>
> i.e. shared handlers should never defer the end'ing of the interrupt
(which
> makes sense, since this would affect the other [shared] handlers).
HANDLED_NOEBNABLE could be supported too.
Yes, but it breaks decoupling between shared handlers; interrupts will be
deferred for all [shared] handlers until it is properly ended.
There would be no need in
reenventing
a wheel, just do it the way Linux does it.
But it's about some additional re-designing of the current codebase
(e.g. nested calling for irq_enable/disable())
I'm not sure we do need it for something else rather than irq sharing
code but it affects the rest of the code.
And we have a kind of wrong concept :
XN_ISR_ENABLE (or NOENABLE) corresponds to xnarch_end_irq().
Agree
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).
> Yes, "should". And this "should" is best be handled by
>
> a) Documenting the potential conflict in the same place when describing
> the return values
>
> b) Placing some debug warning in the nucleus' IRQ trampoline function to
> bail out (once per line) when running into such situation
>
> But I'm against any further runtime restrictions, especially as most
> drivers will never return anything else than NOT_HANDLED or HANDLED.
> Actually, this was the reason why I tried to separate the NO_ENABLE and
> PROPAGATE features as *additional* bits from HANDLED and
> NOT_HANDLED/UNHANDLED/NOINT. But I acknowledge that having all valid bit
> combination present as constants can be more helpful for the user. We
> just have to draw some line between the standard values and the
> additional gurus return codes
>
(documentation: "don't use NO_ENABLE or
> PROPAGATE unless you understand their side-effects and pitfalls
precisely").
I agree with you on PROPAGATE case, but NO_ENABLE that, as pointed out
above,
should (IMHO and at least, in theory) only mean "keep the IRQ line disabled"
(and have nothing to do with .end-ing the IRQ line) would be better
supported.
But this is, again as was pointed out above, about some redesigning of the
current code => some overhead that likely affects non-shared aware code too.
So on one hand,
I'm ready to re-work code with :
HANDLED and UNHANDLED (or NOINT)
+ 2 additional bits : NOENABLE and PROPAGATE.
and document it like you suggested "don't use NO_ENABLE or
PROPAGATE with shared interrupts
unless you understand their side-effects and pitfalls precisely";
on the other hand,
I'd say that I'm almost ready to vote against merging the irq sharing
code at all as it looks to be a rather partial solution.
I vote for (even though I'm the one who complains the most), BUT I think it is
important to keep the rules for using it simple (that's why I worry about the
plethora of return-flags).
--
Regards
Anders
