Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Ingo Molnar <mingo@elte.hu>]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Fri, 14 Aug 2009, Peter Zijlstra wrote:
> > On Thu, 2009-08-13 at 19:40 +0000, Thomas Gleixner wrote:
> > >  
> > > +/**
> > > + *	disable_slowbus_irq - disable an slowbus irq and wait for completion
> > > + *	@irq: Interrupt to disable
> > > + *
> > > + *	Disable the selected interrupt line.  Enables and Disables are
> > > + *	nested.
> > > + *	This function waits for any pending IRQ handlers for this interrupt
> > > + *	to complete before returning. If you use this function while
> > > + *	holding a resource the IRQ handler may need you will deadlock.
> > > + *
> > > + *	This function must not be called from IRQ context.
> > > + */
> > > +void disable_slowbus_irq(unsigned int irq)
> > > +{
> > > +	struct irq_desc *desc = irq_to_desc(irq);
> > > +
> > > +	if (!desc || !desc->chip || !desc->chip->bus_lock)
> > > +		return;
> > > +
> > > +	desc->chip->bus_lock(irq);
> > > +	disable_irq_nosync(irq);
> > > +	if (desc->action)
> > > +		synchronize_irq(irq);
> > > +	desc->chip->bus_sync_unlock(irq);
> > > +}
> > > +EXPORT_SYMBOL(disable_slowbus_irq);
> > 
> > Should we also not check that desc->chip->bus_lock is not set for the
> > regular function disable_irq()?
> > 
> > It seems to me mixing disable_irq() and disable_slowbus_irq() 
> > is a recipe for disaster.
> > 
> > Same for the other slowbus functions of course.
> 
> Yeah, that's what I wanted to avoid with the first version, which 
> did the conditional locking and did not require a separate API, 
> but Ingo frowned upon the conditional lock.

Mind posting that version too?

Conditional locking is really nasty but if the only other option is 
nastier there's not much we can do, is there?

	Ingo