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

[Thomas Gleixner <tglx@linutronix.de>]

On Sat, 15 Aug 2009, Ingo Molnar wrote:
> * Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> 
> > On Fri, Aug 14, 2009 at 12:20:49PM +0200, Thomas Gleixner wrote:
> > > On Fri, 14 Aug 2009, Pavel Machek wrote:
> > 
> > > > AFAICT this means that driver would need to know what kind of IRQ it
> > > > is hooked to, right? That will lead to some ugly code in drivers that
> > > > can handle both normal and slowbus irqs, right?
> > 
> > > Are there such drivers in reality ?
> > 
> > Yes.  The GPIO based stuff is the prime example but there's other 
> > examples - one is the WM831x touchscreen (no driver in mainline 
> > yet) which can use interrupts via the main interrupt controller on 
> > the CPU but also has the option of bringing the interrupt signals 
> > out to dedicated pins on the chip for direct connection to the CPU 
> > precisely to avoid the overheads of these slow interrupt 
> > controllers.
> 
> This would call for Thomas's first version of the patch, that is 
> transparent to drivers - the IRQ subsystem will know how to lock 
> access to the line.
> 
> How about implementing that first patch in a cleaner way - can we 
> somehow express the slow-bus property purely via the irqchip? Or is 
> that too lowlevel?

The problem here is that the management functions serialize via
irqdesc->lock and call the chip level function with the lock held
(preemption and interrupts disabled). So we can not access the slow
bus chips from these low level functions.

If the low level functions just store the information and schedule it
for bus access then there is no serialization anymore. Lets look at
disable_irq():

	spin_lock_irqsave(&desc->lock);
	....
	desc->chip->mask();
		schedule bus access;
	...
	spin_lock_irqsave(&desc->lock);

So now we return, but the mask has not reached the chip.

The idea of bus_lock/bus_sync_unlock() was to provide well defined
synchronization points for the chip level implementation to do the bus
update and have this serialized against other management functions.

	desc->chip->bus_lock();
		Take the chip->bus mutex

	spin_lock_irqsave(&desc->lock);
	....
	desc->chip->mask();
		store mask information
	...
	spin_lock_irqsave(&desc->lock);

	desc->chip->bus_sync_unlock();
		Update the mask via the slow bus
		Release chip->bus mutex

That way we have made sure that the change to the chip actually hits
the hardware before we allow further management operations and it
simplifies the code for the chip implementation as the bus access can
be done in the context of the caller w/o the need of an extra
thread/workqueue ...

Thanks,

	tglx