lkml.org 
[lkml]   [2013]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] chipidea: Use devm_request_irq()
    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-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2013-07-31 12:21    [W:4.357 / U:0.436 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site