[PATCH] ARM: gic: use handle_fasteoi_irq for SPIs

[Thomas Gleixner]

On Thu, 17 Feb 2011, Russell King - ARM Linux wrote:
> On Wed, Feb 16, 2011 at 08:20:17PM +0100, Thomas Gleixner wrote:
> > On Wed, 16 Feb 2011, Rabin Vincent wrote:
> > > On Wed, Feb 16, 2011 at 21:47, Will Deacon <will.deacon at arm.com> wrote:
> > > > Ah yes, thanks for the explanation. After looking at the plat-omap code
> > > > I finally understand what's going on and I can't help but feel that the
> > > > chained GPIO handlers are terminally broken! The generic irq chip high-level
> > > > handlers (handle_{edge,level}_irq for example) at least check to see if
> > > > the irq_chip functions are non-NULL before calling them.
> > > >
> > > > Ideally, the chained handler would be able to query the irq_chip to find
> > > > out what types of IRQ flow-control it supports and then assume that behaviour.
> > > 
> > > Thomas, suggestions on how best to handle this?  (Some of these chained
> > > handlers are the ones in plat-omap/gpio.c, plat-nomadik/gpio.c, and
> > > mach-s5pv310/irq-combiner.c.)
> > 
> > I'm not much of a fan of those chained handlers. They work fine, when
> > they are tied into the irq_chip implementation of a SoC where the
> > chained handler code is 1:1 related to that irq_chip.
> > 
> > Once you start assigning those handlers somewhere else or even using
> > the same handler for different underlying primary irq chips, then it's
> > a lost case and wreckage like this is just lurking around the corner.
> 
> That's a result of genirq.  With the ARM non-genirq implementation
> where we had just ack, mask and unmask, it all turned out much easier.
> In fact, we hardly ever called any of these functions in chained
> handlers (except ack) - the only time when mask and unmask were called
> was when we had an IRQ multiplexer which didn't allow us to mask the
> individual interrupts.
> 
> With the advent of genirq and its expansion of the irq chip methods it's
> become much more complicated.  That's one of the faults of trying to
> generalize this across different architectures.
> 
> You can't do without chained handlers on ARM - we used to do it without
> them and it was proven to be a complete disaster.  Dropped interrupts
> and various drivers just didn't work with chained interrupt controllers.

I really have a hard time to see the difference between doing:

  chained_handler()
	fiddle_with_primary_chip()
	demux()
	fiddle_with_primary_chip()
	
and

  handle_xxxx_irq()
	fiddle_with_primary_chip()

	chained_handler()
		demux()

	fiddle_with_primary_chip()

Why should that drop more interrupt? The only difference is that you
make the demux handler oblivious of the underlying primary chip and
let the chip chose the appropriate flow handler which issues the right
chip methods.

> So getting rid of it is also a 'lost cause'.

I didn't say that we need to get rid of chained handlers. You just
have to be very careful about them. Nothing new - with or without
genirq.

Thanks,

	tglx