Hello Andy. 2014-02-24 17:08 GMT+04:00 Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>: > On Mon, 2014-02-24 at 15:09 +0400, Alexander Popov wrote: >> @@ -1018,11 +1019,23 @@ static int mpc_dma_probe(struct platform_device *op) >> /* Register DMA engine */ >> dev_set_drvdata(dev, mdma); >> retval = dma_async_device_register(dma); >> - if (retval) { >> - devm_free_irq(dev, mdma->irq, mdma); >> - irq_dispose_mapping(mdma->irq); >> + if (retval) >> + goto out_irq; >> + >> + /* register with OF helpers for DMA lookups (nonfatal) */ >> + if (dev->of_node) { >> + retval = of_dma_controller_register(dev->of_node, >> + of_dma_xlate_by_chan_id, >> + mdma); >> + if (retval) >> + dev_warn(dev, "could not register for OF lookup\n"); >> } >> >> + return 0; >> + >> +out_irq: >> + devm_free_irq(dev, mdma->irq, mdma); > > Something wrong either with devm_request_irq() or you don't need to call > devm_free_irq() explicitly. Once we already try to discuss this earlier > in this mailing list with Lars-Peter(?), though there were no solution > how to keep devm_*_irq usability. Thanks, I've read this discussion. It seems that the current code doesn't do anything bad, though devm_request_irq() and devm_free_irq() can be changed to request_irq() and free_irq() accordingly. Do you think it's worth being done in a separate patch in this series? >> + irq_dispose_mapping(mdma->irq); >> return retval; >> } >> >> @@ -1031,6 +1044,8 @@ static int mpc_dma_remove(struct platform_device *op) >> struct device *dev = &op->dev; >> struct mpc_dma *mdma = dev_get_drvdata(dev); >> >> + if (dev->of_node) >> + of_dma_controller_free(dev->of_node); >> dma_async_device_unregister(&mdma->dma); >> devm_free_irq(dev, mdma->irq, mdma); >> irq_dispose_mapping(mdma->irq); Best regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html