[RFC] Refactored interrupt handling on ARM

[Ian Lepore]

On Tue, 2014-04-15 at 17:29 +0200, Jakub Klama wrote:
> On Tue, 15 Apr 2014 08:11:57 -0600, Warner Losh wrote:
> > This is looking nice on the surface. I’ve done a small portion of
> > this sort of work for the Atmel port, so I’ll see how well this can 
> > be
> > extended to work there. Unlike the gic device, there’s 3 or 4 pio
> > controllers that act as interrupt handlers, plus the need to wire up
> > pins relatively early...
> >
> > I’d clean up the name ‘intrng.c’ though, since ng things tend to
> > become dated… ARM_INTRNG also suffers from this naming flaw. Is there
> > any reason not to have it on all the time? Also, the ARM_INTRNG ifdef
> > is inconsistently applied.
> 
>  Well, the naming thing is the least significant one I guess... and
>  one which can be fixed in a moment.
> 
>  ARM_INTRNG should be enabled only on platforms which have pic drivers
>  using pic_if.m interface instead of the old one. The purpose for that
>  option is to not break existing code.
> 

IMO we do too much of this.  Unless there's a really good reason not to
update the older platforms to use this new scheme, I think we should
just convert everything to the new way.

> > Looking at the FDT, it appears that the interrupt numbers are hard
> > coded for things that appear to be connected to GPIOs. This hard
> > coding (and changes to reflect differences in mapping) is really bad.
> 
>  Can you give an example?
> 
> > Linux specifies the GPIO more directly (though each SoC is different
> > in the details). I haven’t looked at LPC (where I noticed the change)
> > on Linux to see if these changes bring us closer to being able to use
> > the stock FDT for that platform.  Do these changes help with that?
> 
>  LPC changes purpose was rather to give an example, surely other
>  platforms can benefit more from these.
> 
> > While I appreciate the aesthetic appeal of having purely an interrupt
> > number and PIC (interrupt parent), I’m not sure that will work
> > everywhere.
> >
> > dosoftints() does nothing, is that on purpose?
> 
>  It's present to not break compatibility and I'm not even sure what
>  should it do. However:
> 
>  % ack dosoftints
>  arm/arm/intrng.c
>  417:void dosoftints(void);
>  419:dosoftints(void)
> 
>  arm/arm/intr.c
>  138:void dosoftints(void);
>  140:dosoftints(void)
> 
>  is it needed anyways?
> 
> > 255 is too few interrupts to have. We need easily twice that for
> > Atmel. And chance we can remove NIRQ as well?
> 
>  Currently we can have 255 pics and 256 irqs on each, but because irq
>  number is stored in int, we easily (down to changing two lines of code)
>  divide it by 16:16 or 8:24, thus having 2^16 pics and 2^16 irqs on each
>  or 256 pics with 2^24 irqs on each maximum.
> 
>  Regarding NIRQ, there's surely possible to get rid of it, but it will
>  need also removing intrcnt/sintrcnt/intrnames and creating more
>  flexible interface instead (for other archs too). Anyways, it's some
>  step in removing it.
> 
> > Finally, I’m not sure what the change to kern_intr.c is supposed to
> > accomplish. It seems weird to me. Can you explain it? I’d think that
> > would have a bad effect on other platforms.
> 
>  trapframe is passed to the first intr_event_handle() call and saved
>  in td->td_intr_frame (just like it was before). If the consecutive
>  calls to intr_event_handle() supply 0/NULL as trapframe, filter
>  function which wants trapframe instead of argument will get the
>  saved one. So the only situation where the behavior is different
>  is case where you call intr_event_handle(ie, NULL). BTW This change
>  was discussed some time ago with cognet at .

Since this is only needed by the new arm code, and the new arm code only
calls intr_event_handle() from arm_dispatch_irq(), can't this logic just
move into there?

Also, I have a feeling EOI isn't being done in all the right places, but
it could be a matter of misreading the diff.  I need to actually apply
the diff and read the code, and I'll get to that soon, really I will (I
seem to be writing a new definition for "soon" every time I say that).

-- Ian