Re: [Xenomai-core] Missing IRQ end function on PowerPC

[Philippe Gerum <rpm@xenomai.org>]

Jan Kiszka wrote:
> Philippe Gerum wrote:
> 
>>Jan Kiszka wrote:
>>
>>>Wolfgang Grandegger wrote:
>>>
>>>
>>>>>This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
>>>>>
>>>>>Philippe Gerum wrote:
>>>>>
>>>>>
>>>>>>Gilles Chanteperdrix wrote:
>>>>>>
>>>>>>
>>>>>>>Wolfgang Grandegger wrote:
>>>>>>>
>>>>>>>>Therefore we need a dedicated function to re-enable interrupts in
>>>>>>>
>>>>>>>the  > ISR. We could name it *_end_irq, but maybe *_enable_isr_irq is
>>>>>>>more  > obvious. On non-PPC archs it would translate to *_irq_enable.
>>>>>>>I  > realized, that *_irq_enable is used in various place/skins and
>>>>>>>therefore  > I have not yet provided a patch.
>>>>>>>
>>>>>>>The function xnarch_irq_enable seems to be called in only two
>>>>
>>>>functions,
>>>>
>>>>
>>>>>>>xintr_enable and xnintr_irq_handler when the flag XN_ISR_ENABLE is
>>>>>>>set.
>>>>>>>
>>>>>>>In any case, since I am not sure if this has to be done at the Adeos
>>>>>>>level or in Xenomai, we will wait for Philippe to come back and
>>>>>>>decide.
>>>>>>>
>>>>>>
>>>>>>->enable() and ->end() all mixed up illustrates a silly x86 bias I
>>>>>>once
>>>>>>had. We do need to differentiate the mere enabling from the IRQ
>>>>>>epilogue
>>>>>>at PIC level since Linux does it - i.e. we don't want to change the
>>>>>>semantics here.
>>>>>>
>>>>>>I would go for adding xnarch_end_irq -> rthal_irq_end to stick with
>>>>>>the
>>>>>>Linux naming scheme, and have the proper epilogue done from there on a
>>>>>>per-arch basis.
>>>>>>
>>>>>>Current uses of xnarch_enable_irq() should be reserved to the
>>>>>>non-epilogue case, like xnintr_enable() i.e. forcibly unmasking the
>>>>>>IRQ
>>>>>>source at PIC level outside of any ISR context for such interrupt (*).
>>>>>>XN_ISR_ENABLE would trigger a call to xnarch_end_irq, instead of
>>>>>>xnarch_enable_irq. I see no reason for this fix to leak to the Adeos
>>>>>>layer, since the HAL already controls the way interrupts are ended
>>>>>>actually; it just does it improperly on some platforms.
>>>>>>
>>>>>>(*) Jan, does rtdm_irq_enable() have the same meaning, or is it
>>>>>>intended
>>>>>>to be used from the ISR too in order to revalidate the source at PIC
>>>>
>>>>level?
>>>>
>>>>
>>>>>Nope, rtdm_irq_enable() was never intended to re-enable an IRQ line
>>>>>after an interrupt, and the documentation does not suggest this either.
>>>>>I see no problem here.
>>>>
>>>>But RTDM needs a rtdm_irq_end() functions as well in case the
>>>>user wants to reenable the interrupt outside the ISR, I think.
>>>
>>>
>>>If this is a valid use-case, it should be really straightforward to add
>>>this abstraction to RTDM. We should just document that rtdm_irq_end()
>>>shall not be invoked from IRQ context -
>>
>>It's the other way around: ->end() would indeed be called from the ISR
>>epilogue, and ->enable() would not.
> 
> 
> I think we are talking about different things: Wolfgang was asking for
> an equivalent of RTDM_IRQ_ENABLE outside the IRQ handler. That's the
> user API. You are now referring to the back-end which had to provide
> some end-mechanism to be used both by the nucleus' ISR epilogue and that
> rtdm_irq_end(), and that mechanism shall be told apart from the IRQ
> enable/disable API. Well, that's at least how I think to have got it...
> 

My point was only about naming here: *_end() should be reserved for the epilogue 
stuff, hence be callable from ISR context. Aside of that, I'm ok with the 
abstraction you described though.

> 
>> to avoid breaking the chain in
>>
>>>the shared-IRQ scenario. RTDM_IRQ_ENABLE must remain the way to
>>>re-enable the line from the handler.
>>>
>>>Jan
>>>
>>>
>>
>>
> 
> Jan
> 


-- 

Philippe.