On Fri, Oct 16, 2015 at 09:25:52AM +0100, Jon Hunter wrote: > @@ -1182,14 +1182,11 @@ static int tegra_dma_alloc_chan_resources(struct dma_chan *dc) > { > struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc); > struct tegra_dma *tdma = tdc->tdma; > - int ret; > > dma_cookie_init(&tdc->dma_chan); > tdc->config_init = false; > - ret = clk_prepare_enable(tdma->dma_clk); > - if (ret < 0) > - dev_err(tdc2dev(tdc), "clk_prepare_enable failed: %d\n", ret); > - return ret; > + > + return pm_runtime_get_sync(tdma->dev); Alloc channel is supposed to return number of descriptors allocated and if pm_runtime_get_sync() returns postive values we get wrong return! > pm_runtime_enable(&pdev->dev); > - if (!pm_runtime_enabled(&pdev->dev)) { > + if (!pm_runtime_enabled(&pdev->dev)) > ret = tegra_dma_runtime_resume(&pdev->dev); > - if (ret) { > - dev_err(&pdev->dev, "dma_runtime_resume failed %d\n", > - ret); > - goto err_pm_disable; > - } > - } > + else > + ret = pm_runtime_get_sync(&pdev->dev); do we need pm_runtime_get() here, should we use pm_request_resume() ? > static int tegra_dma_pm_suspend(struct device *dev) > { > struct tegra_dma *tdma = dev_get_drvdata(dev); > - int i; > - int ret; > + int i, ret; > > /* Enable clock before accessing register */ > - ret = tegra_dma_runtime_resume(dev); > + ret = pm_runtime_get_sync(dev); If you are runtime suspended then core will runtime resume you before invoking suspend, so why do we need this -- ~Vinod -- 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