From patchwork Thu Jan 12 21:30:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Lindgren X-Patchwork-Id: 9514217 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 880A2601E7 for ; Thu, 12 Jan 2017 21:30:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8AD67286DC for ; Thu, 12 Jan 2017 21:30:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7F2D728713; Thu, 12 Jan 2017 21:30:34 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9ED25286DC for ; Thu, 12 Jan 2017 21:30:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750929AbdALVac (ORCPT ); Thu, 12 Jan 2017 16:30:32 -0500 Received: from muru.com ([72.249.23.125]:56822 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750896AbdALVab (ORCPT ); Thu, 12 Jan 2017 16:30:31 -0500 Received: from sampyla.muru.com. (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTP id 3A178827F; Thu, 12 Jan 2017 21:31:12 +0000 (UTC) From: Tony Lindgren To: Dan Williams , Vinod Koul Cc: Bin Liu , Daniel Mack , Felipe Balbi , George Cherian , Johan Hovold , Peter Ujfalusi , Sekhar Nori , Sebastian Andrzej Siewior , dmaengine@vger.kernel.org, linux-usb@vger.kernel.org, linux-omap@vger.kernel.org, Andy Shevchenko , Grygorii Strashko , Kevin Hilman , Patrick Titiano , Sergei Shtylyov Subject: [PATCH] dmaengine: cppi41: Fix oops in cppi41_runtime_resume Date: Thu, 12 Jan 2017 13:30:16 -0800 Message-Id: <20170112213016.19367-1-tony@atomide.com> X-Mailer: git-send-email 2.11.0 Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support") together with recent MUSB changes allowed USB and DMA on BeagleBone to idle when no cable is connected. But looks like few corner case issues still remain. Looks like just by repluging USB cable about ten or so times on BeagleBone when configured in USB peripheral mode we can get warnings and eventually trigger an oops in cppi41 DMA: WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+ x28/0x38 [cppi41] ... WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452 push_desc_queue+0x94/0x9c [cppi41] ... Unable to handle kernel NULL pointer dereference at virtual address 00000104 pgd = c0004000 [00000104] *pgd=00000000 Internal error: Oops: 805 [#1] SMP ARM ... [] (cppi41_runtime_resume [cppi41]) from [] (__rpm_callback+0xc0/0x214) [] (__rpm_callback) from [] (rpm_callback+0x20/0x80) [] (rpm_callback) from [] (rpm_resume+0x504/0x78c) [] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8) [] (pm_runtime_work) from [] (process_one_work+0x2b4/0x808) This is because of a race with PM runtime and cppi41_dma_issue_pending() as reported by Alexandre Bailon in earlier set of patches. Based on mailing list discussions we however came to the conclusion that a different fix from Alexandre's fix is needed in order to guarantee that DMA is really active when we try to use it. To fix the issue, we need to add a driver specific flag as we otherwise can have -EINPROGRESS state set by PM runtime and can't rely on pm_runtime_active() to tell us when we can use the DMA. For reference, this is also documented in Documentation/power/runtime_pm.txt in the example at the end of the file as pointed out by Grygorii Strashko . So let's fix the issue by introducing atomic_t active that gets toggled in runtime_suspend() and runtime_resume(). And then let's replace the pm_runtime_active() checks with atomic_read(). Note that we may also get transactions queued while in -EINPROGRESS state. So we need to check for new elements in cppi41_runtime_resume() by replacing list_for_each_entry_safe() with the commonly used test for while (!list_empty(&cdd->pending)). Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support") Cc: Andy Shevchenko Cc: Bin Liu Cc: Grygorii Strashko Cc: Kevin Hilman Cc: Patrick Titiano Cc: Sergei Shtylyov Reported-by: Alexandre Bailon Signed-off-by: Tony Lindgren --- drivers/dma/cppi41.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c --- a/drivers/dma/cppi41.c +++ b/drivers/dma/cppi41.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -153,6 +154,8 @@ struct cppi41_dd { /* context for suspend/resume */ unsigned int dma_tdfdq; + + atomic_t active; }; #define FIST_COMPLETION_QUEUE 93 @@ -320,7 +323,8 @@ static irqreturn_t cppi41_irq(int irq, void *data) int error; error = pm_runtime_get(cdd->ddev.dev); - if (error < 0) + if (((error != -EINPROGRESS) && error < 0) || + !atomic_read(&cdd->active)) dev_err(cdd->ddev.dev, "%s pm runtime get: %i\n", __func__, error); @@ -482,7 +486,7 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan) return; } - if (likely(pm_runtime_active(cdd->ddev.dev))) + if (likely(atomic_read(&cdd->active))) push_desc_queue(c); else pending_desc(c); @@ -1003,6 +1007,7 @@ static int cppi41_dma_probe(struct platform_device *pdev) cdd->ddev.dst_addr_widths = CPPI41_DMA_BUSWIDTHS; cdd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST; cdd->ddev.dev = dev; + atomic_set(&cdd->active, 0); INIT_LIST_HEAD(&cdd->ddev.channels); cpp41_dma_info.dma_cap = cdd->ddev.cap_mask; @@ -1151,6 +1156,7 @@ static int __maybe_unused cppi41_runtime_suspend(struct device *dev) { struct cppi41_dd *cdd = dev_get_drvdata(dev); + atomic_set(&cdd->active, 0); WARN_ON(!list_empty(&cdd->pending)); return 0; @@ -1159,13 +1165,20 @@ static int __maybe_unused cppi41_runtime_suspend(struct device *dev) static int __maybe_unused cppi41_runtime_resume(struct device *dev) { struct cppi41_dd *cdd = dev_get_drvdata(dev); - struct cppi41_channel *c, *_c; + struct cppi41_channel *c; unsigned long flags; + atomic_set(&cdd->active, 1); + spin_lock_irqsave(&cdd->lock, flags); - list_for_each_entry_safe(c, _c, &cdd->pending, node) { - push_desc_queue(c); + while (!list_empty(&cdd->pending)) { + c = list_first_entry(&cdd->pending, + struct cppi41_channel, + node); list_del(&c->node); + spin_unlock_irqrestore(&cdd->lock, flags); + push_desc_queue(c); + spin_lock_irqsave(&cdd->lock, flags); } spin_unlock_irqrestore(&cdd->lock, flags);