| From 362f4562466c3b9490e733e06999025638310d4a Mon Sep 17 00:00:00 2001 |
| From: Tony Lindgren <tony@atomide.com> |
| Date: Thu, 19 Jan 2017 08:49:08 -0800 |
| Subject: dmaengine: cppi41: Fix oops in cppi41_runtime_resume |
| |
| From: Tony Lindgren <tony@atomide.com> |
| |
| commit 362f4562466c3b9490e733e06999025638310d4a upstream. |
| |
| 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 re-plugging 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 |
| ... |
| [<bf0d92cc>] (cppi41_runtime_resume [cppi41]) from [<c0589838>] |
| (__rpm_callback+0xc0/0x214) |
| [<c0589838>] (__rpm_callback) from [<c05899ac>] (rpm_callback+0x20/0x80) |
| [<c05899ac>] (rpm_callback) from [<c0589460>] (rpm_resume+0x504/0x78c) |
| [<c0589460>] (rpm_resume) from [<c058a1a0>] (pm_runtime_work+0x60/0xa8) |
| [<c058a1a0>] (pm_runtime_work) from [<c0156120>] (process_one_work+0x2b4/0x808) |
| |
| This is because of a race with runtime PM and cppi41_dma_issue_pending() |
| as reported by Alexandre Bailon <abailon@baylibre.com> 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 runtime PM and can't rely on |
| pm_runtime_active() to tell us when we can use the DMA. |
| |
| And we need to make sure the DMA transfers get triggered in the queued |
| order. So let's always queue the transfers, then flush the queue |
| from both cppi41_dma_issue_pending() and cppi41_runtime_resume() |
| as suggested by Grygorii Strashko <grygorii.strashko@ti.com> in an |
| earlier example patch. |
| |
| 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 |
| <grygorii.strashko@ti.com>. |
| |
| Based on earlier patches from Alexandre Bailon <abailon@baylibre.com> |
| and Grygorii Strashko <grygorii.strashko@ti.com> modified based on |
| testing and what was discussed on the mailing lists. |
| |
| Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support") |
| Cc: Andy Shevchenko <andy.shevchenko@gmail.com> |
| Cc: Bin Liu <b-liu@ti.com> |
| Cc: Grygorii Strashko <grygorii.strashko@ti.com> |
| Cc: Kevin Hilman <khilman@baylibre.com> |
| Cc: Patrick Titiano <ptitiano@baylibre.com> |
| Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
| Reported-by: Alexandre Bailon <abailon@baylibre.com> |
| Signed-off-by: Tony Lindgren <tony@atomide.com> |
| Tested-by: Bin Liu <b-liu@ti.com> |
| Signed-off-by: Vinod Koul <vinod.koul@intel.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/dma/cppi41.c | 40 +++++++++++++++++++++++++--------------- |
| 1 file changed, 25 insertions(+), 15 deletions(-) |
| |
| --- a/drivers/dma/cppi41.c |
| +++ b/drivers/dma/cppi41.c |
| @@ -153,6 +153,8 @@ struct cppi41_dd { |
| |
| /* context for suspend/resume */ |
| unsigned int dma_tdfdq; |
| + |
| + bool is_suspended; |
| }; |
| |
| #define FIST_COMPLETION_QUEUE 93 |
| @@ -470,20 +472,26 @@ static void push_desc_queue(struct cppi4 |
| cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num)); |
| } |
| |
| -static void pending_desc(struct cppi41_channel *c) |
| +/* |
| + * Caller must hold cdd->lock to prevent push_desc_queue() |
| + * getting called out of order. We have both cppi41_dma_issue_pending() |
| + * and cppi41_runtime_resume() call this function. |
| + */ |
| +static void cppi41_run_queue(struct cppi41_dd *cdd) |
| { |
| - struct cppi41_dd *cdd = c->cdd; |
| - unsigned long flags; |
| + struct cppi41_channel *c, *_c; |
| |
| - spin_lock_irqsave(&cdd->lock, flags); |
| - list_add_tail(&c->node, &cdd->pending); |
| - spin_unlock_irqrestore(&cdd->lock, flags); |
| + list_for_each_entry_safe(c, _c, &cdd->pending, node) { |
| + push_desc_queue(c); |
| + list_del(&c->node); |
| + } |
| } |
| |
| static void cppi41_dma_issue_pending(struct dma_chan *chan) |
| { |
| struct cppi41_channel *c = to_cpp41_chan(chan); |
| struct cppi41_dd *cdd = c->cdd; |
| + unsigned long flags; |
| int error; |
| |
| error = pm_runtime_get(cdd->ddev.dev); |
| @@ -495,10 +503,11 @@ static void cppi41_dma_issue_pending(str |
| return; |
| } |
| |
| - if (likely(pm_runtime_active(cdd->ddev.dev))) |
| - push_desc_queue(c); |
| - else |
| - pending_desc(c); |
| + spin_lock_irqsave(&cdd->lock, flags); |
| + list_add_tail(&c->node, &cdd->pending); |
| + if (!cdd->is_suspended) |
| + cppi41_run_queue(cdd); |
| + spin_unlock_irqrestore(&cdd->lock, flags); |
| |
| pm_runtime_mark_last_busy(cdd->ddev.dev); |
| pm_runtime_put_autosuspend(cdd->ddev.dev); |
| @@ -1166,8 +1175,12 @@ static int __maybe_unused cppi41_resume( |
| static int __maybe_unused cppi41_runtime_suspend(struct device *dev) |
| { |
| struct cppi41_dd *cdd = dev_get_drvdata(dev); |
| + unsigned long flags; |
| |
| + spin_lock_irqsave(&cdd->lock, flags); |
| + cdd->is_suspended = true; |
| WARN_ON(!list_empty(&cdd->pending)); |
| + spin_unlock_irqrestore(&cdd->lock, flags); |
| |
| return 0; |
| } |
| @@ -1175,14 +1188,11 @@ static int __maybe_unused cppi41_runtime |
| static int __maybe_unused cppi41_runtime_resume(struct device *dev) |
| { |
| struct cppi41_dd *cdd = dev_get_drvdata(dev); |
| - struct cppi41_channel *c, *_c; |
| unsigned long flags; |
| |
| spin_lock_irqsave(&cdd->lock, flags); |
| - list_for_each_entry_safe(c, _c, &cdd->pending, node) { |
| - push_desc_queue(c); |
| - list_del(&c->node); |
| - } |
| + cdd->is_suspended = false; |
| + cppi41_run_queue(cdd); |
| spin_unlock_irqrestore(&cdd->lock, flags); |
| |
| return 0; |