Hello, 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(). > > > handler is using (e.g. ioremap, clk_prepare_enable) are only freed > > > *after* the irq is done. For ioremap that means it must be done using > > > devm_ioremap_resource. For a clock it's not that easy because the irq > > > handler has to assert that a used clk is kept prepared which can only be > > > done using clk_prepare which in turn is not allowed in an irq handler. > > > > > Hmm. So the only possible fixes are > > > - devm* can be told to also care about clk_disable_unprepare > > > - after disabling irqs in the remove callback wait for all > > > active irqs to be done. (i.e. call synchronize_irq(irq)) > > > - don't use devm_request_irq Again, the right thing to do is having a proper deactivation step. This is nothing devm can do automatically. There's no way for it to find out that the device is actually quiesced. Let's say it waits for the current instance of irq handler to finish. How would it know that it won't start again between the flushing of the current instance and irq deregistration. Add an explicit deactivation step using devres_alloc(). > > I'm not sure that devm_ guarantees any ordering in the cleanups it does > > so I'd not like to rely on the first option either, if there were some > > guarantee of that it'd help. The nice thing about explicitly freeing > > the IRQ is that you can tell that all this stuff is safe by inspection. > devm_* at least uses list_for_each_entry_reverse > (drivers/base/devres.c:release_nodes()). Without this guarantee devm_ > would not make much sense IMHO. devm guarantees that the destruction callbacks are called in the reverse order of registration. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
