[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

The eCos, QNX, ChorusOs irq handling API

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

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

>>>>>>     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???
>>>>     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 chnaging openpic masks than yet it should be 

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