From: Dirk Behme <dirk.behme@de.bosch.com> To: linux-renesas-soc@vger.kernel.org Cc: dmaengine@vger.kernel.org, vkoul@kernel.org, geert+renesas@glider.be, niklas.soderlund+renesas@ragnatech.se, laurent.pinchart+renesas@ideasonboard.com, Achim.Dahlhoff@de.bosch.com, dirk.behme@de.bosch.com, stable@vger.kernel.org Subject: [2/2] dmaengine: sh: rcar-dmac: Fix glitch in dmaengine_tx_status Date: Tue, 5 Mar 2019 06:56:28 +0100 [thread overview] Message-ID: <20190305055628.11826-2-dirk.behme@de.bosch.com> (raw) From: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com> The tx_status poll in the rcar_dmac driver reads the status register which indicates which chunk is busy (DMACHCRB). Afterwards the point inside the chunk is read from DMATCRB. It is possible that the chunk has changed between the two reads. The result is a non-monotonous increase of the residue. Fix this by introducing a 'safe read' logic. Fixes: 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB instead of TCR for residue") Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> Cc: <stable@vger.kernel.org> # v4.16+ --- Note: Patch done against mainline v5.0 Changes in v2: Switch goto/retry to for loop drivers/dma/sh/rcar-dmac.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index 2ea59229d7f5..cceddc7099b0 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -1282,6 +1282,9 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, enum dma_status status; unsigned int residue = 0; unsigned int dptr = 0; + unsigned int chcrb; + unsigned int tcrb; + unsigned int i; if (!desc) return 0; @@ -1329,6 +1332,24 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, return 0; } + /* + * We need to read two registers. + * Make sure the control register does not skip to next chunk + * while reading the counter. + * Trying it 3 times should be enough: Initial read, retry, retry + * for the paranoid. + */ + for (i = 0; i < 3; i++) { + chcrb = rcar_dmac_chan_read(chan, RCAR_DMACHCRB) & + RCAR_DMACHCRB_DPTR_MASK; + tcrb = rcar_dmac_chan_read(chan, RCAR_DMATCRB); + /* Still the same? */ + if (chcrb == (rcar_dmac_chan_read(chan, RCAR_DMACHCRB) & + RCAR_DMACHCRB_DPTR_MASK)) + break; + } + WARN_ONCE(i >= 3, "residue might be not continuous!"); + /* * In descriptor mode the descriptor running pointer is not maintained * by the interrupt handler, find the running descriptor from the @@ -1336,8 +1357,7 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, * mode just use the running descriptor pointer. */ if (desc->hwdescs.use) { - dptr = (rcar_dmac_chan_read(chan, RCAR_DMACHCRB) & - RCAR_DMACHCRB_DPTR_MASK) >> RCAR_DMACHCRB_DPTR_SHIFT; + dptr = chcrb >> RCAR_DMACHCRB_DPTR_SHIFT; if (dptr == 0) dptr = desc->nchunks; dptr--; @@ -1355,7 +1375,7 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, } /* Add the residue for the current chunk. */ - residue += rcar_dmac_chan_read(chan, RCAR_DMATCRB) << desc->xfer_shift; + residue += tcrb << desc->xfer_shift; return residue; }
WARNING: multiple messages have this Message-ID (diff)
From: Dirk Behme <dirk.behme@de.bosch.com> To: <linux-renesas-soc@vger.kernel.org> Cc: <dmaengine@vger.kernel.org>, <vkoul@kernel.org>, <geert+renesas@glider.be>, <niklas.soderlund+renesas@ragnatech.se>, <laurent.pinchart+renesas@ideasonboard.com>, <Achim.Dahlhoff@de.bosch.com>, <dirk.behme@de.bosch.com>, <stable@vger.kernel.org> Subject: [PATCH 2/2] dmaengine: sh: rcar-dmac: Fix glitch in dmaengine_tx_status Date: Tue, 5 Mar 2019 06:56:28 +0100 [thread overview] Message-ID: <20190305055628.11826-2-dirk.behme@de.bosch.com> (raw) In-Reply-To: <20190305055628.11826-1-dirk.behme@de.bosch.com> From: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com> The tx_status poll in the rcar_dmac driver reads the status register which indicates which chunk is busy (DMACHCRB). Afterwards the point inside the chunk is read from DMATCRB. It is possible that the chunk has changed between the two reads. The result is a non-monotonous increase of the residue. Fix this by introducing a 'safe read' logic. Fixes: 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB instead of TCR for residue") Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> Cc: <stable@vger.kernel.org> # v4.16+ --- Note: Patch done against mainline v5.0 Changes in v2: Switch goto/retry to for loop drivers/dma/sh/rcar-dmac.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index 2ea59229d7f5..cceddc7099b0 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -1282,6 +1282,9 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, enum dma_status status; unsigned int residue = 0; unsigned int dptr = 0; + unsigned int chcrb; + unsigned int tcrb; + unsigned int i; if (!desc) return 0; @@ -1329,6 +1332,24 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, return 0; } + /* + * We need to read two registers. + * Make sure the control register does not skip to next chunk + * while reading the counter. + * Trying it 3 times should be enough: Initial read, retry, retry + * for the paranoid. + */ + for (i = 0; i < 3; i++) { + chcrb = rcar_dmac_chan_read(chan, RCAR_DMACHCRB) & + RCAR_DMACHCRB_DPTR_MASK; + tcrb = rcar_dmac_chan_read(chan, RCAR_DMATCRB); + /* Still the same? */ + if (chcrb == (rcar_dmac_chan_read(chan, RCAR_DMACHCRB) & + RCAR_DMACHCRB_DPTR_MASK)) + break; + } + WARN_ONCE(i >= 3, "residue might be not continuous!"); + /* * In descriptor mode the descriptor running pointer is not maintained * by the interrupt handler, find the running descriptor from the @@ -1336,8 +1357,7 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, * mode just use the running descriptor pointer. */ if (desc->hwdescs.use) { - dptr = (rcar_dmac_chan_read(chan, RCAR_DMACHCRB) & - RCAR_DMACHCRB_DPTR_MASK) >> RCAR_DMACHCRB_DPTR_SHIFT; + dptr = chcrb >> RCAR_DMACHCRB_DPTR_SHIFT; if (dptr == 0) dptr = desc->nchunks; dptr--; @@ -1355,7 +1375,7 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, } /* Add the residue for the current chunk. */ - residue += rcar_dmac_chan_read(chan, RCAR_DMATCRB) << desc->xfer_shift; + residue += tcrb << desc->xfer_shift; return residue; } -- 2.20.0
next reply other threads:[~2019-03-05 5:56 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-05 5:56 Dirk Behme [this message] 2019-03-05 5:56 ` [PATCH 2/2] dmaengine: sh: rcar-dmac: Fix glitch in dmaengine_tx_status Dirk Behme -- strict thread matches above, loose matches on Subject: below -- 2019-04-01 12:47 [2/2] " Dirk Behme 2019-04-01 12:47 ` [PATCH v2 2/2] " Dirk Behme 2019-03-05 5:56 [v2,1/2] dmaengine: sh: rcar-dmac: With cyclic DMA residue 0 is valid Dirk Behme 2019-03-05 5:56 ` [PATCH v2 1/2] " Dirk Behme 2019-03-04 10:37 [2/2] dmaengine: sh: rcar-dmac: Fix glitch in dmaengine_tx_status Laurent Pinchart 2019-03-04 7:56 Dirk Behme
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190305055628.11826-2-dirk.behme@de.bosch.com \ --to=dirk.behme@de.bosch.com \ --cc=Achim.Dahlhoff@de.bosch.com \ --cc=dmaengine@vger.kernel.org \ --cc=geert+renesas@glider.be \ --cc=laurent.pinchart+renesas@ideasonboard.com \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=niklas.soderlund+renesas@ragnatech.se \ --cc=stable@vger.kernel.org \ --cc=vkoul@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.