On Wed, Jul 22, 2009 at 06:40:21PM +0200, Thomas Gleixner wrote:

On Wed, 22 Jul 2009, Dmitry Torokhov wrote:

quoted

quoted

I don't think it belongs into the driver code. It belongs into the
platform code which sets up the system and knows what's on which
interrupt line.
 

I do think this should be set up by the driver - the platform/arch code
can't be 100% certain what model of servicing interrupts driver will
chose, nor the driver can know whether arch code set things up for
threaded or classic interrupt handling.

Since handle_level_oneshot_irq requires drivers to use threaded IRQ
model (in absence of thread interrupt will never be unmasked) it would
be better if we did set it up automatically, right there in
request_threaded_irq(). This would reduce maintenance issues between
platform and driver code.

No, it's the wrong way round. 

The platform code sets up the platform devices. So there is no real
good reason that the platform code does not know about the details.

If it conveys the wrong irq number then it wont work, if it sets the
wrong handler it wont work either. So what ?

If the driver is implemented to use a threaded handler, which it
better is no matter what as it uses a bus, and the interrupt
controller logic is proper implemented as well then the driver does
not care about those details at all. It will magically work with any
interrupt controller you put in front of it.

If the platform maintainer sets the wrong handler or the wrong
platform data then it's not the driver writers problem.

It is not matter of setting handler or platform data incorrctly. There
can be a perfectly working driver using I2C and non-threaded IRQ and
there can be a similar driver using threaded IRQ. The problem with your
approach that only _one_ can work with given platform setup. If platform
sets the oneshot interrup handler then threaded will work and standard
will break, otherwise standard will work and threaded will break. This
is not the best outcome, don't you agree?

-- 
Dmitry