The eCos, QNX, ChorusOs irq handling API

[Sergei Organov]

Valette Eric <eric.valette at free.fr> writes:
> Sergei Organov wrote:
> > Valette Eric <eric.valette at free.fr> writes:
> 
> > And I think it shouldn't. I think it should call driver code that in turn
> > should disconnect handler if it is asked to. How to find a driver that owns
> > the IRQ is another issue that IMHO requires more general handling at upper
> > level (apparently through some kind of driver registration).
> 
> Nice try : to replace a simple function pointer you propose to add driver
> registration routines...

But simple function pointer attached to an interrupt doesn't solve the
problem, it only solves part of it :-( Next time one will need to figure out
which driver owns another resource... If a solution is simpler than the
problem it promises to solve, the solution is broken ;-)

Anyway, the registration could be as simple as direct map between interrupt
number and required function pointer(s), I think.

> It surely simplify the API that as you seem to recognise IRQ management API
> will not be complete without that...

No, I believe IRQ management API is complete without it. The usage of the Off
routine you suggest (as I understand it) is to provide a way for a driver to
manage other resources it uses when its routine is disconnected from the IRQ.
My opinion is that it should be kept separate from the IRQ API itself and
should be a part of a separate [driver] API.

> 
> > Yes, it could be used for that purpose. But for me it looks like a limited
> > solution you've invented to overcome absence of generic drivers API.
> 
> Do you think we shall provide it to avoid the On/Off :-)

I'm afraid if we actually want to provide a complete framework for dynamically
attached/detached drivers, we need to. At least this won't impose any overhead
on systems that don't need such feature.

BTW, couldn't all this [optionally] be handled using already proposed void*
argument?

> 
> >>>>	2) There is no way for code outside a driver to stop interrupt for a
> >>>>	   while and then reenable them without having knowledge of all the
> >>>>	   devices. This type of functionnality is necessary for profilling,
> >>>>	   system debuggers, irq monitoring,
> >>>
> >>>Why? cyg_interrupt_mask(vector) will do.
> >>
> >>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.
> 
> They cannot has there will be separate call to set the mask and restore it
> later on. If you prevent any PIC access during this sequence, then you
> cannot handle any other interrupts???

So in your generic isr management code there is no such code? If so, why a
driver will need such a code?

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

If they are mandatory, they are mandatory only for those PICs. I thought we
are talking about generic APIs here.

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

If there is no way to make a set of useful routines to simplify handling of
such complicated architectures (or BSP writer is too lazy to carefully design
the BSP), then BSP writer is still allowed to implement ISR handling the way
you suggest -- I don't see any problem here.

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

In fact no other interrupt will occur before RTEMS returns to an interrupted
thread as I didn't even re-enable IRQs at processor level. I can't believe you
don't know there are designs that don't use nested ISRs at all.

> 
> > where exactly one of (1), (2) or (3) could in fact be a no-op. I can build
> > quite a few other likely sequences as well, and in fact I believe the
> > sequence you've provided is one of the least likely sequences in real-time
> > applications where it's a common practice to do absolute minimum of work
> > in ISRs deferring as much as possible to threads level.
> 
> Well it is the sequence I have seen in Linux, ChorusOs, BSD, ...

Don't know about ChorusOs, but the remaining two are solid-kernel UNIX-like
OSes. I believe an [embedded] thread-based RTOS could have rather different
requirements.

> But anyway 1,2,3 represent actually 5 instructions on Ix86, not much more on
> PCC *BUT* That way I can say the interrupt latency is low...

And what I would do if I wish my handler to be invoked at global interrupts
disabled? You will again tell me that I would never need such handler?

> 
> > While I myself didn't carefully examine which implications SMP may have on
> > the discussed API, I think that it is *the implementation* of the API
> > should take care of SMP.
> 
> 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...

But you do PIC bitmask manipulations in your generic handler, don't you? So
there is a way to do it correctly. Sorry, but I can't believe there is no
useful set of routines that will allow drivers to implement their own custom
sequences.

> 
> > Average driver writer will need to understand only API (the specific
> > hardware the driver should manage aside), and suggested API is not
> > platform/PIC dependent. The implementation of the API will manage PIC (if
> > any), not driver writer.
> 
> Not if you impose to call routine that manipulate the PIC to correctly hanlde
> an interrupt...

If there is a code that at all is capable to correctly handle PIC
manipulations, it most probably could be split into a set of useful routines.
And the more complex the code is, the more important such a careful splitting
becomes as it will simplify any ISR management code, including the generic
one.

> 
> > Flexibility does matter here. Having slightly more complex handler worth
> > the flexibility the approach provides, IMHO.
> 
> I disagree. Flexibility for you would probably be seen by many other on this
> list as unecessary complexity...

I've already said that BSP writer is free to provide whatever simplified
approach he wishes. The only requirement is to support standard API routines
as careful as possible to provide flexibility for those who really needs it.

> 
> > 1. Because it seems you believe there is an advantage. Don't you?
> > 2. Because on(irq_number), unlike your p->on(irq_connect_data), could be
> >    even an inline routine thus reducing to a few asm instructions, and I
> >    *do care* about performance of such operation.
> 
> There is no performance impact has this code *again* is not called by
> handler... You already forgot it?

So at least here we agree. For a code wishing to just temporary disable
an interrupt there should be a better API.

> 
> > The more we discuss the issue the more feeling I have that it's not only
> > useless, but also misleading. Maybe I'm the only so stupid here who doesn't
> > understand what it is for?
> 
> For sure you are probably the only one who proposed many things regarding the
> IRQ API without even suggesting that you would care to really implement
> anything and not even taking care to see if the API implementation is
> realistic or not on 8XX, motorola_shared or Ix86 :-)

There already is at least one person (you) who cares about implementability of
the IRQ API on these platforms. I'm sure you know them better than I and won't
allow unimplementable/useless API to appear in RTEMS. Thus I don't care about
these platforms but instead I'm trying to present another point of view based
on my experience with different kinds of systems.

Unlike you, I don't promote my own inventions and I didn't propose anything
that is not found anywhere else but in my own code :-)

Anyway, for me it seems that further discussion of the issue is useless, so
I'm shutting up and returning to coding. Ah, it's already time to go home...

-- 
Sergei.