Re: [PATCH 2/2] chipidea: Use devm_request_irq()

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]

On Wed, Jul 31, 2013 at 05:54:11AM -0400, Tejun Heo wrote:
> On Wed, Jul 31, 2013 at 11:44:34AM +0200, Uwe Kleine-König wrote:

> > > > OK, so the possible problem is that remove is called while the irq is
> > > > still active. That means you have to assert that all resources the irq

> If your driver destruction path is running while your irq handler is
> still running, it's a crappy / broken driver.  You need a deactivation
> step whether you're using devm or not.  IRQs can be shared and the
> device should be in a quiesced state before the driver detaches
> itself.  Note that you can queue deactivation routine using devm.  For
> an example, please take a look at
> drivers/ata/libata-core.c::ata_host_start().

I'm not sure I understand how this relates the problem.  The main issue
here is that for the shared IRQ case quiescing the device doesn't make
any difference since one of the other users of the interrupt could cause
the interrupt handler to be called regardless of what the hardware is
doing.  This means that we need to guarantee that anything the interrupt
handler relies on has not been deallocated before the interrupt handler
is unregistered.

> irq deregistration.  Add an explicit deactivation step using
> devres_alloc().

> devm guarantees that the destruction callbacks are called in the
> reverse order of registration.

OK, that's helpful.  It'd be good to document this if it's something
the API is intending to guarantee, though - devres.txt doesn't mention
this and it's not something I'd intuitively expect to be the case.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]