Subject: Re: [PATCH] powerpc/mpic: supply a .disable callback

From: Benjamin Herrenschmidt <[email-masked]>
Date: Wed, 08 Jan 2014 10:11:38 +1100

On Tue, 2014-01-07 at 10:18 +0000, [email-masked] wrote:
> 
> > -----Original Message-----
> > From: Benjamin Herrenschmidt [mailto:[email-masked]]
> > Sent: Tuesday, January 07, 2014 1:50 PM
> > To: Wang Dongsheng-B40534
> > Cc: Wood Scott-B07421; [email-masked]
> > Subject: Re: [PATCH] powerpc/mpic: supply a .disable callback
> > 
> > On Tue, 2014-01-07 at 13:38 +0800, Dongsheng Wang wrote:
> > > From: Wang Dongsheng <[email-masked]>
> > >
> > > Currently MPIC provides .mask, but not .disable.  This means that
> > > effectively disable_irq() soft-disables the interrupt, and you get
> > > a .mask call if an interrupt actually occurs.
> > >
> > > I'm not sure if this was intended as a performance benefit (it seems common
> > > to omit .disable on powerpc interrupt controllers, but nowhere else), but it
> > > interacts badly with threaded/workqueue interrupts (including KVM
> > > reflection).  In such cases, where the real interrupt handler does a
> > > disable_irq_nosync(), schedules defered handling, and returns, we get two
> > > interrupts for every real interrupt.  The second interrupt does nothing
> > > but see that IRQ_DISABLED is set, and decide that it would be a good
> > > idea to actually call .mask.
> > 
> > We probably don't want to do that for edge, only level interrupts.
> > 
> Sorry Ben, I am not understand your comments.
> 
> This issue is the kernel api irq_disable() only use chip->irq_disable(), but mpic
> not have this interface so we don't real disable the interrupt.

Yes, because we want to keep the existing behaviour of "lazy disable"
for edge interrupts. It's faster.

Cheers,
Ben.

> -Dongsheng
> > Cheers,
> > Ben.