Dmitry Adamushko wrote:
> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>> Dmitry Adamushko wrote:
>>> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>>>> Dmitry Adamushko wrote:
>>>>> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>>>>> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this
>>>>>> before merging (i.e. make XN_ISR_HANDLED non-zero)?
>>>>> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now
>>>>> non-zero.
>>>>> Actually, at first I wanted to make it just the other way about.
>>>> Hmm, thinking about this again, I would like to revert my suggestion in
>>>> favour of a third approach (trying to keep you busy ;) ).
>>> Ok, you are wellcome :)
>>> I didn't get it, at least while looking briefly. To make it a bit
>> easier, at
>>> least for me, let's go another way.
>>> At the left is the list of allowed values as Philippe pointed out.
>>> At the right is another list which corresponds to the left one but when
>>> NOINT is used instead of HANDLED. Moreover, I have added another case -
>> pure
>>> NOINT. The ISR says it's not mine and, well, it doesn't know whether it
>>> should be propagated or no
>>> (ok, so far CHAINED standed for NOINT).
>>> 1.) HANDLED                     ->  0
>>> 2.) HANDLED | ENABLE     ->  ENABLE
>>> 4.) CHAINED                      ->  CHAINED | NOINT
>>> 5.)                                    ->   NOINT
>>> Could you provide the 3-d corresponding table using your new flags?
>> Still not 3D, but hopefully clarifying: :)
>>     (nucleus ignores implicit IRQ enable)
> Then why do we need XN_ISR_NOINT? is it not just the same as ~HANDLED?

The idea is to have at least a define so that

a) users explicitly have to state what they mean
b) we can map the define to some other value whenever we may need this
in the future

The first property could even be used to apply some debug checks in the
user's return value.

> Moreover, I tend to think now that PROPAGATE (CHAINED) should not be
> normally used by ISRs at all. The only exception would be irq sharing
> between RT and non-RT domains, but that's of no concern for most part of rt
> drivers and, as last events revealed, it's very partially supported at the
> moment and possible implementations are quite arguable.

Ack. But you should still export this feature for those who think they
know what they are doing. ;)

> So HANDLED means the irq line is .end-ed by xnintr_irq_handler()
> (xnarch_end_irq(irq)  is called).
> If one wants to do it on his own later, NOENABLE can be returned
> additionally.

Yep. So let's call it XN_ISR_NOENABLE.

> But then, one must use xnintr_end_irq() and not just xnintr_irq_enable().

Yep, but a different (add-on) topic.

> If xnintr_end_irq() would be always the same as xnintr_irq_enable() or we
> would decouple ending and enabling operations on the line (don't have
> sources at hand); we could go Linux way and make use of a "nested" counter
> in xnintr_t, so that xnintr_irq_disable() could be called in a nested way.
> This said, calling xnintr_irq_disable() in ISR leads to the interrupt line
> being still disabled upon return from xnintr_irq_handler() and re-enabled
> again by the user later.
> ~HANDLED means the ISR was not concerned about this interrupt. i.e. :
> - there is ISR in another domain (down the pipeline) which is interested in
> this irq ;
>      well, the designer of the system should be aware of such cases and
> probably avoid that on hw layer or, at least, to know what she is doing.
> - this is a spurious irq.
> So upon getting ~HANDLED, xnintr_irq_handler() does :
> success = xnintr_irq_chained(irq);

Only if requested via XN_ISR_PROPAGATE (or _CHAINED)! There is still a
real-time ISR registered on that IRQ you are about to forward now, and
you don't know if someone is handling it down the pipeline. Actually,
this is a fatal situation. The system is broken, bail out, and switch
this line off.

> if (success)
> {
> /* i.e. another domain is really interested and got this irq as pending*/
> return without .end-ing
> }
> else
> {
> /* this is a spurious irq */
> make use of some accounting of spurious interrupts (if we need it)), i.e.
> keep the irq line disabled if the % of unhandled requests > some limit.
> otherwise
> xnintr_end_irq() and return
> }
> too long :) ok, it's quite similar indeed to what you have suggested,
> with the exception that
> - no need for NOINT (if it's really == ~HANDLED);
> - nucleus provides some additional logic while handling ~HANDLED case.
> - user don't need to return ENABLE explicitly - in most cases, that's what
> he actually needs but sometimes just forgets (I recall a few questions on
> the mailing list about "only one interrupt".
> Jan
> --
> Best regards,
> Dmitry Adamushko


Attachment: signature.asc
Description: OpenPGP digital signature

Xenomai-core mailing list

Reply via email to