The eCos, QNX, ChorusOs irq handling API => resent message has been trcunacted due to a linux crash

[Valette Eric]

till wrote:

> On the point of driver unloding: I absolutely agree with Sergei on that 
> this is NOT
> the job of irqOff. I thought (based on Erics comments in the header 
> files) that this
> routine should disable the IRQ at the device level - Erics API says 
> NOTHING about
> a driver giving up all its resources and stop as a side effect of this 
> call.
> This is simply not acceptable.

This is the initial goal indeed as comment says. At least it enables the
driver to do some cleanup and disable for sure the interrupts at
hardware level...

> Again: on/off, if it's there (and I believe they are not really 
> necessary) should
> do what it says: enable/disable the IRQ at the device and nothing else

Agreed. I just say that having a call to current driver code provides
way to do much more than just masking the interrupts if needed.

>>>>
>>>> 1) If tasks start manipulating the PIC because you expose this API, 
>>>> and are
>>>> prempted by other tasks performing the same thing vevrything will be 
>>>> rapidly
>>>> broken (To not even mention SMP problems with this API). You better 
>>>> disable
>>>> the real cause of the interrupt (device) because the PIC are shared 
>>>> among all
>>>> interrupts while the device is not.
>>>
>>>
>>>
>>>
>>> The PIC is shared resource, so *implementation* of API routines that 
>>> needs to
>>> access the PIC should take care to use appropriate mutual exclusion
>>> primitives. Anyway, I don't see how the on/off interface helps here, 
>>> sorry.
>>
>>
> I agree with S. again.

What about SMP? Is it simply feasible?

> Sure, but if you have to manipulate the masks you have to assert mutual 
> exclusion
> which means that you cannot handle interrupts (that critical section 
> better be short).

But the section is usually  :
	1) Mask other lower priority interrupts by mapipulating PIC 	masks
	2) execute the handler,
	3) Restore original pic masks

This cannot be guaranteed to be *ALWAYS* short...

>>
>>>> 2) What if I want to mask a set of interrupt via a masks (as most 
>>>> moderm PIC
>>>> implement it)?
>>>
>>>
>>>
>>>
>>> With this particular API it obviously requires multiple calls to the 
>>> routine.
>>> While I think it's rarely needed and thus multiple calls could be 
>>> acceptable,
>>> I see two solution for this problem:
>>>
>>> 1. Design API that accepts bit masks.
>>>
>>> 2. Let BSPs provide specific optimized routines for more advanced 
>>> hardware
>>>    management.
>>>
>>> I personally prefer (2).
>>
>>
>>
>> First I think applying bit masks is the only way to implement the 
>> notion of IRQ priority on many PICs.  Therefore applying masks is 
>> mandatory contrary to what you suggest... Now, the masks are totally 
>> PIC dependent and so even if you provide an API, any usage inside a 
>> drivers will be surrounded by a zillion of ifdefs one for each PIC... 
>> And also when there are cascaded hardware 5openPic + 8259, SIU +CPM), 
>> there are several masks that should be applyied at diffrent time 
>> isnide the handler routine...
> 
> 
> I don't understand. IMO implementing priorities by masks is the job of 
> the BSP/PIC driver
> and is hidden from the 'normal' driver writer. For most other cases, 
> just disabling one vector
> is probably fine.

This breaks the possibility to mask a set of interrupts while handling
the current one which is the closest definition I can get to interrupt
priorities when there is a single interrupt level... If you think users 
will in general not require interrupts priorities then we have seen 
different sets of users :-)


>>>>>>     3) not acknowledging the irq by generic layer means that a 
>>>>>> driver can
>>>>>>        break any form of irq priorities if the PIC must be 
>>>>>> acknowledged
>>>>>>        before issuing the next interrupt.
>>>>>
>>>>>
>>>>>
>>>>> A driver could break anything no matter if acknowledge is performed 
>>>>> by the
>>>>> generic layer or not. Anyway, if BSP writer decides to ack irqs in the
>>>>> generic layer, he can make interface 
>>>>> 'cyg_interrupt_acknowledge(vector)'
>>>>> routine to be a no-op.
>>>>>
>>>>>
>>>>>>        And while it seems necessary to acknowledge explicitely, 
>>>>>> there is
>>>>>>        no API to mask the current IRQ at PIC level
>>>>>
>>>>>
>>>>>
>>>>> I believe cyg_interrupt_mask()/cyg_interrupt_unmask() are exactly 
>>>>> for this
>>>>> purpose, so the rest of your sentence below makes no sense.
>>>>
>>>>
>>>>
>>>> So each driver will then have to include the sequence :
>>>>     1) Get the current mask
>>>>     2) Apply a new mask than at least mask the current IRQ. BTW 
>>>> sometimes
>>>>         you want to mask more than one IRQ source so the proposed 
>>>> eCos API
>>>>         is not sufficient 
>>>
>>>
> Why would you want to do such a thing in a driver ISR???

I'm not sure I understand the question. If you want to implement 
interrupt priority then this sequence should be done iether by the 
generic PIC management code or by the ISR code itself to prevent lower 
inettrupt priority to be nested while executing high prioirity ISR code.
That said, I agree with you that, it shoul not be let to the ISR writer.

> 
>>>>
>>>>     3) send an EOI
>>>>     4) reenable IRQ a processor level
>>>>     5) handle the hardware
>>>>     6) restore the original mask
>>>>
>>>> Great. Do you think It cannot be factorized as already done on Ix86 
>>>> and PPC?
>>>> And do you really think you can avoid this sequence?
>>>
>>>
>>>
>>>
>>> I really think that it is *a* sequence, not *the* sequence, and I 
>>> don't see
>>> why this one should be preferred over other possible sequences. 
>>
>>
> Well, but here I tend to agree with Eric. The 1-6 sequence is certainly 
> one that
> makes sense in most cases while ensuring low latency. Strangely, Eric,
> you didn't implement OpenPIC interrupts that way, however. There, you
> send EOI only after running the user ISR - shouldn't we fix this??
> 
>>> The simplest
>>> possible sequence is:
>>>
>>>         1) handle the hardware
>>>         2) do something useful
>>>         3) acknowledge the IRQ
>>
>>
>>
>> If you do that, then no other interrupt will occur before 3 and thus 
>> you increase the global interrupt latency of the system by a single 
>> driver... 
> 
> 
> BTW: Eric, that is how you handle (non ISA) IRQs on PPC/shared, yep.

Readding the openpic specs, it is not clear you can acknoledge before
actually cleaning the source of the interrupt because I have found no
mask to set that prevent the interrupt to be reasserted immediately. If
this can be achieved via changing openpic masks than yet it should be
done... For sure for VME part this should be done as you have another 
PIC level like on ISA


>> You simply cannot implememt SMP safe drivers API if you impose the 
>> usage of PIC bitmask manipulation in handlers as several handler may 
>> be executed concurently meaning that you have no mean the correct mask 
>> that need to be restored before returning from hdl... 
> 
> 
> Of course, bitmask manipulation (i.e. 'disable(vector)' / 
> 'enable(vector)') has to be SMP safe.
> BTW: one example of a driver who needs to do that is the tundra universe 
> PCI-VME bridge.
> It encorporates a "PIC" itself and hence wants to re-enable it's PCI IRQ 
> at the OpenPIC ASAP
> (to support VME interrupt priorities in software)

Yes same for ISA inetrrupt. Browsing the code to answer greg question I
was wondering why you did not acknloledge the VME request immediately as
I have done on ISA :-)

> Sorry, I still don't see the great benefit of the on/off routines. 
> Especially since it's explicitely
> left to the driver implementor to make them no-ops. Hence, you can not 
> really rely on them
> really disabling/enabling interrupts at the device...

Providing a nop function is a bad thing I just wanted to point out that 
is is simply to convert existing driver to the existing API.

  Please don't let us get personal here. Not only does this not matter, I
 > believe, on the contrary,
 > that the "USER's" opinion about the API should be of utmost importance
 > to us. Anyone
 > (Sergei not being precisely "anyone") who takes his/her time to think
 > about this and share
 > their thoughs is welcome.
 >
 > I'd love to see more contributions by other people rather than talking a
 > lot myself or
 > to continue argueing with Eric about the benefits of his current
 > implementation.


The way the IRQ API specification are handled really piss me off. You 
can expects users to express requirements not to define the right API or 
understand why their requirements are either not realistic in general or 
not even possible to implement on some arch. How can you compare the 
merits of one API compared to another one when you have not expressed 
and validated the requirements and sorted them by priorities. This is 
the basics of making specifications (or am I alraedy so old that I can't 
get it?)

So I propose to do the following :

	1) Express a first set of requirements with the help of the maximum users,
	2) Clean them up via pondering priorities,
	3) Propose only then an API,
	4) See how the API does fullfill the requirements,. If we find 
impossibilities, then either remove the requirement, let is open to BSP 
specific implementations, and at least clerly document the limitation of 
the API,
	5) Check that the proposed API is at least implementable on mpc8XX, 
Intel, motorola shared and 68K and MIPS as we have people ready to test it,

Till, I have requested this several times and most of this rather 
lenghty technical, flame prone discussions would not have existed if the 
requirement has been expressed from the beginning. Just to clean up in 
front of my door :  I expressed the requirement when designing the new 
API in 98. The irq.h does even still contain many rationale for the API. 
Now, as ever, it can be enhanced but at least take time to see the 
merits of the current implementation before reinventing the wheel.

If we do not take a methodological approach for making the job, I will 
simply shut up and let you guys do the work and latter support it. I 
have indeed provided support for my API for 5 years :-) And anyway, 
whatever you define, I'm able to recode my own implementation without 
needing help.

PS : my own set of requirements by importance order :

     0) Interrupt latency should be keep to minimal and should not rely 
on *each* handler to perform specific actions (e.g EOI),
     1) There should be a way to mask a set of interrupt when a 
particular interrupt occurs) via a BSP dependent and *reconfigurable* 
means. Individual IRQ priorities having no meaning on most processor (e. 
Intel, PPC, ARM), a table defining priorities one way or other should be 
used. This table can be static (as on PPC) or better dynamic as on Intel,
     2) It should be as simple as possible to write drivers and in 
particular, it should not require knowledge of anything except the 
target hardware itself and its memory/IO mapping (understand I do not 
want to have to know about the PIC's specificities)
     3) API documentation should clearly define in what state the 
handler is called (what should have been done by the BSP generic irq 
code before the handler is called) and what primitive a ISR handler is 
allowed to call. In particular we must define what has been done on 
warious PICs and what has to be performed by the handler itself,
     4) We have a strategy concerning interrupt sharing. The hability to 
get the previous handler connected is implemented as the minimalistic 
way of sharing...
     5) The API, should provide a mean for generic code to mask 
selective IRQ without knowing the detail of the hardware,
     6) disconnecting a handler should call a callback routine inside 
initial driver code,

-- eric