Date:	Wed, 22 Jul 2009 13:18:00 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Trilok Soni <soni.trilok@...il.com>,
	Pavel Machek <pavel@....cz>, Arve Hj?nnev?g <arve@...roid.com>,
	kernel list <linux-kernel@...r.kernel.org>,
	Brian Swetland <swetland@...gle.com>,
	linux-input@...r.kernel.org, Andrew Morton <akpm@...l.org>,
	linux-i2c@...r.kernel.org,
	Joonyoung Shim <jy0922.shim@...sung.com>,
	m.szyprowski@...sung.com, t.fujak@...sung.com,
	kyungmin.park@...sung.com, David Brownell <david-b@...bell.net>,
	Peter Zijlstra <peterz@...radead.org>,
	Daniel Ribeiro <drwyrm@...il.com>
Subject: Re: Threaded interrupts for synaptic touchscreen in HTC dream

On Wed, Jul 22, 2009 at 12:44:01PM +0200, Thomas Gleixner wrote:
> On Tue, 21 Jul 2009, Mark Brown wrote:

> > I'll need to have a more detailed look at that but it's not immediately
> > clear to me how a driver (or even machine) should use that code - it
> > looks more like it's intended to be called from within the IRQ
> > infrastructure than from random driver code.

> All it needs is to set handle_level_oneshot_irq for the interrupt line
> of your I2C or whatever devices. 

>    set_irq_handler(irq, handle_level_oneshot_irq);

Yeah, I know - the issue I was having was that the use of set_irq_handler()
seemed rather rude for driver code.  Grepping around I see that there
are actually a small number of drivers using it already but at first
glance most of them appear to be implementing interrupt controllers.  It
was setting off alarm bells about abstraction layer violation like
set_irq_type() does.

> > Nothing if the above works, though I guess more documentation wouldn't
> > hurt (and possibly a more friendly wrapper).  From the name and

> Wrapper for what ? 

Something to package up the set_irq_handler() and request_threaded_irq()
(possibly a flag for request_threaded_irq()).  This is such a common
thing and request_threaded_irq() looks so much like it should Just Work
that I'd expect it'll help usability a lot to have a single function
which says "this is the idiomatic way to implement this".

> The irq thread finds out which interrupt(s) are active in the
> device. So it raises the interrupt handlers for those from the thread
> which will wake up the relevant interrupt threads for those
> devices. Once all the thread handlers have finished you return from
> the main thread and the interrupt line gets unmasked again.

Yes, this bit of it isn't too much of a problem, it's what's going on in
all the non-genirq infrastructures, but...

> The driver which controls the interrupt device has to expose the
> demultiplexed interrupts via its own irq_chip implementation. Of
> course the chip functions like mask/ack/unmask cannot run in atomic
> context as they require bus access again.

...as you say this is the tricky bit.

> Here in deed we need to put some thought into common infrastructure
> as it seems that such excellent hardware designs are becoming more
> popular :(

This isn't a new issue - it's more that you're seeing a greater degree
of mainline activity from embedded systems.  

FWIW the hardware tradeoff here is in a large part down to the fact that
many of these devices are heavily size (and therefore pin count)
constrained, often on both the device and CPU end of things.  Adding
more pins would either mean a bigger device or a device that's more
expensive to manufacture and use.  This pushes to minimal wire, low
speed control interfaces and for the sort of low volume control these
things need there's not much operational problem there.  These things
are *normally* either pushing events that either won't happen very often
(eg, RTC alarm expiry) or shouldn't happen at all (eg, power failures)
and so aren't performance critical.

> After that bus_sync_unlock() is called outside the atomic context. Now
> the chip implementation issues the bus commands, waits for completion
> and unlocks the interrupt controller bus.

I'll try to find time to implement some use of it and give it a spin -
it looks good at first glance but I'll need to convert one of my drivers
to genirq in order to test.  Someone working on a chip that already uses
genirq might get there first.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists