From bbcb87555869cb6c249bf00d13d3bc400c476c84 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Mon, 9 Jul 2018 17:09:58 +0530 Subject: [PATCH 1/3] dmaengine: pl330: Mark expected switch fall-through In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Vinod Koul --- drivers/dma/pl330.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index defcdde4d358..04fc4d8da0e9 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -1046,13 +1046,16 @@ static bool _start(struct pl330_thread *thrd) if (_state(thrd) == PL330_STATE_KILLING) UNTIL(thrd, PL330_STATE_STOPPED) + /* fall through */ case PL330_STATE_FAULTING: _stop(thrd); + /* fall through */ case PL330_STATE_KILLING: case PL330_STATE_COMPLETING: UNTIL(thrd, PL330_STATE_STOPPED) + /* fall through */ case PL330_STATE_STOPPED: return _trigger(thrd); From 2f903bab92dea8dec8c93e4fa3c7c5295ef0a0fe Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Mon, 9 Jul 2018 20:08:48 +0530 Subject: [PATCH 2/3] dmaengine: pl330: remove set but unused variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiler complains (with W=1): drivers/dma/pl330.c: In function ‘pl330_release_channel’: drivers/dma/pl330.c:1782:21: warning: variable ‘pl330’ set but not used [-Wunused-but-set-variable] struct pl330_dmac *pl330; ^~~~~ Remove the pl330 variable in pl330_release_channel as it is set but never used. Signed-off-by: Vinod Koul --- drivers/dma/pl330.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index 04fc4d8da0e9..451370da909d 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -1782,8 +1782,6 @@ static inline void _free_event(struct pl330_thread *thrd, int ev) static void pl330_release_channel(struct pl330_thread *thrd) { - struct pl330_dmac *pl330; - if (!thrd || thrd->free) return; @@ -1792,8 +1790,6 @@ static void pl330_release_channel(struct pl330_thread *thrd) dma_pl330_rqcb(thrd->req[1 - thrd->lstenq].desc, PL330_ERR_ABORT); dma_pl330_rqcb(thrd->req[thrd->lstenq].desc, PL330_ERR_ABORT); - pl330 = thrd->dmac; - _free_event(thrd, thrd->ev); thrd->free = true; } From e49756544a21f5625b379b3871d27d8500764670 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Tue, 17 Jul 2018 11:48:16 +0100 Subject: [PATCH 3/3] dmaengine: pl330: fix irq race with terminate_all In pl330_update() when checking if a channel has been aborted, the channel's lock is not taken, only the overall pl330_dmac lock. But in pl330_terminate_all() the aborted flag (req_running==-1) is set under the channel lock and not the pl330_dmac lock. With threaded interrupts, this leads to a potential race: pl330_terminate_all pl330_update ------------------- ------------ lock channel entry lock pl330 _stop channel unlock pl330 lock pl330 check req_running != -1 req_running = -1 _start channel Signed-off-by: John Keeping Signed-off-by: Vinod Koul --- drivers/dma/pl330.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index 451370da909d..9e38947f6ffe 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -2256,13 +2256,14 @@ static int pl330_terminate_all(struct dma_chan *chan) pm_runtime_get_sync(pl330->ddma.dev); spin_lock_irqsave(&pch->lock, flags); + spin_lock(&pl330->lock); _stop(pch->thread); - spin_unlock(&pl330->lock); - pch->thread->req[0].desc = NULL; pch->thread->req[1].desc = NULL; pch->thread->req_running = -1; + spin_unlock(&pl330->lock); + power_down = pch->active; pch->active = false;