On Wed, Apr 04, 2012 at 12:18:12PM +0600, Mike Sinkovsky wrote:03.04.2012 19:36, Mark Brown ÐÐÐÐÑÐÐ:
...you use devm_request_threaded_irq() here and rely on it for cleanup.
Are you sure there's no possibility of the interrupt firing after you
start to tear down the device?
By using a specifically threaded IRQ you're also adding a performance
overhead for no good reason if you can call netdev_carrier_*() from IRQ
context and the GPIO is capable of generating a hard IRQ. If you use
request_any_context_irq() instead then the driver will get a hard IRQ if
that's supported.
There isn't devm* variant of request_any_context_irq(), and using
plain version looks inconsistent with other resources handling.
Anyway, this is not performance critical procedure, and latency
around 100 millisecond is acceptable. Some our boards even don't
have this gpio at all, and nothing bad happens, just userspace
doesn't know is carrier on or off.
None of this addresses the primary concern which is that because you're
not (as far as I can tell) ensuring that the interrupt won't fire the
driver might crash if the interrupt fires in between the resources it
needs to handle the interrupt being deallocated and the interrupt being
unregistered. Managed interrupts are relatively tricky to use because
of this issue, with most things like memory it doesn't matter exactly
when it's deallocated but interrupts can potentially trigger actions
themselves.
If you can use it and devm_request_any_context_irq() doesn't exist it'd
be better to add it.