2020-11-20 14:35:35

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 0/4] Bunch of improvements for STM32 DMA controllers

This series brings 3 patches for STM32 DMA and 1 for STM32 MDMA.
They increase the reliability and the efficiency of the transfers.

Amelie Delaunay (4):
dmaengine: stm32-dma: rework irq handler to manage error before xfer
events
dmaengine: stm32-dma: clean channel configuration when channel is
freed
dmaengine: stm32-dma: take address into account when computing max
width
dmaengine: stm32-mdma: rework interrupt handler

drivers/dma/stm32-dma.c | 47 +++++++++++++++++++----------
drivers/dma/stm32-mdma.c | 64 +++++++++++++++++++++-------------------
2 files changed, 65 insertions(+), 46 deletions(-)

--
2.17.1


2020-11-20 14:36:25

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 4/4] dmaengine: stm32-mdma: rework interrupt handler

To avoid multiple entries in MDMA interrupt handler for each flag&interrupt
enable, manage all flags set at once.

Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/dma/stm32-mdma.c | 64 +++++++++++++++++++++-------------------
1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c
index 29e5e30524bb..e4637ec786d3 100644
--- a/drivers/dma/stm32-mdma.c
+++ b/drivers/dma/stm32-mdma.c
@@ -1346,7 +1346,7 @@ static irqreturn_t stm32_mdma_irq_handler(int irq, void *devid)
{
struct stm32_mdma_device *dmadev = devid;
struct stm32_mdma_chan *chan = devid;
- u32 reg, id, ien, status, flag;
+ u32 reg, id, ccr, ien, status;

/* Find out which channel generates the interrupt */
status = readl_relaxed(dmadev->base + STM32_MDMA_GISR0);
@@ -1368,67 +1368,71 @@ static irqreturn_t stm32_mdma_irq_handler(int irq, void *devid)

chan = &dmadev->chan[id];
if (!chan) {
- dev_dbg(mdma2dev(dmadev), "MDMA channel not initialized\n");
- goto exit;
+ dev_warn(mdma2dev(dmadev), "MDMA channel not initialized\n");
+ return IRQ_NONE;
}

/* Handle interrupt for the channel */
spin_lock(&chan->vchan.lock);
- status = stm32_mdma_read(dmadev, STM32_MDMA_CISR(chan->id));
- ien = stm32_mdma_read(dmadev, STM32_MDMA_CCR(chan->id));
- ien &= STM32_MDMA_CCR_IRQ_MASK;
- ien >>= 1;
+ status = stm32_mdma_read(dmadev, STM32_MDMA_CISR(id));
+ /* Mask Channel ReQuest Active bit which can be set in case of MEM2MEM */
+ status &= ~STM32_MDMA_CISR_CRQA;
+ ccr = stm32_mdma_read(dmadev, STM32_MDMA_CCR(id));
+ ien = (ccr & STM32_MDMA_CCR_IRQ_MASK) >> 1;

if (!(status & ien)) {
spin_unlock(&chan->vchan.lock);
- dev_dbg(chan2dev(chan),
- "spurious it (status=0x%04x, ien=0x%04x)\n",
- status, ien);
+ dev_warn(chan2dev(chan),
+ "spurious it (status=0x%04x, ien=0x%04x)\n",
+ status, ien);
return IRQ_NONE;
}

- flag = __ffs(status & ien);
- reg = STM32_MDMA_CIFCR(chan->id);
+ reg = STM32_MDMA_CIFCR(id);

- switch (1 << flag) {
- case STM32_MDMA_CISR_TEIF:
- id = chan->id;
- status = readl_relaxed(dmadev->base + STM32_MDMA_CESR(id));
- dev_err(chan2dev(chan), "Transfer Err: stat=0x%08x\n", status);
+ if (status & STM32_MDMA_CISR_TEIF) {
+ dev_err(chan2dev(chan), "Transfer Err: stat=0x%08x\n",
+ readl_relaxed(dmadev->base + STM32_MDMA_CESR(id)));
stm32_mdma_set_bits(dmadev, reg, STM32_MDMA_CIFCR_CTEIF);
- break;
+ status &= ~STM32_MDMA_CISR_TEIF;
+ }

- case STM32_MDMA_CISR_CTCIF:
+ if (status & STM32_MDMA_CISR_CTCIF) {
stm32_mdma_set_bits(dmadev, reg, STM32_MDMA_CIFCR_CCTCIF);
+ status &= ~STM32_MDMA_CISR_CTCIF;
stm32_mdma_xfer_end(chan);
- break;
+ }

- case STM32_MDMA_CISR_BRTIF:
+ if (status & STM32_MDMA_CISR_BRTIF) {
stm32_mdma_set_bits(dmadev, reg, STM32_MDMA_CIFCR_CBRTIF);
- break;
+ status &= ~STM32_MDMA_CISR_BRTIF;
+ }

- case STM32_MDMA_CISR_BTIF:
+ if (status & STM32_MDMA_CISR_BTIF) {
stm32_mdma_set_bits(dmadev, reg, STM32_MDMA_CIFCR_CBTIF);
+ status &= ~STM32_MDMA_CISR_BTIF;
chan->curr_hwdesc++;
if (chan->desc && chan->desc->cyclic) {
if (chan->curr_hwdesc == chan->desc->count)
chan->curr_hwdesc = 0;
vchan_cyclic_callback(&chan->desc->vdesc);
}
- break;
+ }

- case STM32_MDMA_CISR_TCIF:
+ if (status & STM32_MDMA_CISR_TCIF) {
stm32_mdma_set_bits(dmadev, reg, STM32_MDMA_CIFCR_CLTCIF);
- break;
+ status &= ~STM32_MDMA_CISR_TCIF;
+ }

- default:
- dev_err(chan2dev(chan), "it %d unhandled (status=0x%04x)\n",
- 1 << flag, status);
+ if (status) {
+ stm32_mdma_set_bits(dmadev, reg, status);
+ dev_err(chan2dev(chan), "DMA error: status=0x%08x\n", status);
+ if (!(ccr & STM32_MDMA_CCR_EN))
+ dev_err(chan2dev(chan), "chan disabled by HW\n");
}

spin_unlock(&chan->vchan.lock);

-exit:
return IRQ_HANDLED;
}

--
2.17.1

2020-11-20 14:36:40

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 1/4] dmaengine: stm32-dma: rework irq handler to manage error before xfer events

To better understand error that can be detected by the DMA controller,
manage the error flags before the transfer flags.
This way, it is possible to know if the FIFO error flag is set for an
over/underrun condition or a FIFO level error.
When a FIFO over/underrun condition occurs, the data is not lost because
peripheral request is not acknowledged by the stream until the over/
underrun condition is cleared. If this acknowledge takes too much time,
the peripheral itself may detect an over/underrun condition of its internal
buffer and data might be lost.
That's why in case the FIFO error flag is set, we check if the channel is
disabled or not, and if a Transfer Complete flag is set, which means that
the channel is disabled because of the end of transfer.
Because channel is disabled by hardware either by a FIFO level error, or by
an end of transfer.

Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/dma/stm32-dma.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index d0055d2f0b9a..55a6bd381219 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -648,21 +648,12 @@ static irqreturn_t stm32_dma_chan_irq(int irq, void *devid)
scr = stm32_dma_read(dmadev, STM32_DMA_SCR(chan->id));
sfcr = stm32_dma_read(dmadev, STM32_DMA_SFCR(chan->id));

- if (status & STM32_DMA_TCI) {
- stm32_dma_irq_clear(chan, STM32_DMA_TCI);
- if (scr & STM32_DMA_SCR_TCIE)
- stm32_dma_handle_chan_done(chan);
- status &= ~STM32_DMA_TCI;
- }
- if (status & STM32_DMA_HTI) {
- stm32_dma_irq_clear(chan, STM32_DMA_HTI);
- status &= ~STM32_DMA_HTI;
- }
if (status & STM32_DMA_FEI) {
stm32_dma_irq_clear(chan, STM32_DMA_FEI);
status &= ~STM32_DMA_FEI;
if (sfcr & STM32_DMA_SFCR_FEIE) {
- if (!(scr & STM32_DMA_SCR_EN))
+ if (!(scr & STM32_DMA_SCR_EN) &&
+ !(status & STM32_DMA_TCI))
dev_err(chan2dev(chan), "FIFO Error\n");
else
dev_dbg(chan2dev(chan), "FIFO over/underrun\n");
@@ -674,6 +665,19 @@ static irqreturn_t stm32_dma_chan_irq(int irq, void *devid)
if (sfcr & STM32_DMA_SCR_DMEIE)
dev_dbg(chan2dev(chan), "Direct mode overrun\n");
}
+
+ if (status & STM32_DMA_TCI) {
+ stm32_dma_irq_clear(chan, STM32_DMA_TCI);
+ if (scr & STM32_DMA_SCR_TCIE)
+ stm32_dma_handle_chan_done(chan);
+ status &= ~STM32_DMA_TCI;
+ }
+
+ if (status & STM32_DMA_HTI) {
+ stm32_dma_irq_clear(chan, STM32_DMA_HTI);
+ status &= ~STM32_DMA_HTI;
+ }
+
if (status) {
stm32_dma_irq_clear(chan, status);
dev_err(chan2dev(chan), "DMA error: status=0x%08x\n", status);
--
2.17.1

2020-11-20 14:37:50

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 2/4] dmaengine: stm32-dma: clean channel configuration when channel is freed

When dma_channel_release is called, it means that the channel won't be used
anymore with the configuration it had. To ensure a future client can safely
use the channel after it has been released, clean the configuration done
when channel was requested.

Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/dma/stm32-dma.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index 55a6bd381219..62501e5d9e9d 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -1220,6 +1220,8 @@ static void stm32_dma_free_chan_resources(struct dma_chan *c)
pm_runtime_put(dmadev->ddev.dev);

vchan_free_chan_resources(to_virt_chan(c));
+ stm32_dma_clear_reg(&chan->chan_reg);
+ chan->threshold = 0;
}

static void stm32_dma_desc_free(struct virt_dma_desc *vdesc)
--
2.17.1

2020-11-20 14:38:00

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 3/4] dmaengine: stm32-dma: take address into account when computing max width

DMA_SxPAR or DMA_SxM0AR/M1AR registers have to be aligned on PSIZE or MSIZE
respectively. This means that bus width needs to be forced to 1 byte when
computed width is not aligned with address.

Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/dma/stm32-dma.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index 62501e5d9e9d..f54ecb123a52 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -264,9 +264,11 @@ static int stm32_dma_get_width(struct stm32_dma_chan *chan,
}

static enum dma_slave_buswidth stm32_dma_get_max_width(u32 buf_len,
+ dma_addr_t buf_addr,
u32 threshold)
{
enum dma_slave_buswidth max_width;
+ u64 addr = buf_addr;

if (threshold == STM32_DMA_FIFO_THRESHOLD_FULL)
max_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
@@ -277,6 +279,9 @@ static enum dma_slave_buswidth stm32_dma_get_max_width(u32 buf_len,
max_width > DMA_SLAVE_BUSWIDTH_1_BYTE)
max_width = max_width >> 1;

+ if (do_div(addr, max_width))
+ max_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+
return max_width;
}

@@ -707,7 +712,7 @@ static void stm32_dma_issue_pending(struct dma_chan *c)
static int stm32_dma_set_xfer_param(struct stm32_dma_chan *chan,
enum dma_transfer_direction direction,
enum dma_slave_buswidth *buswidth,
- u32 buf_len)
+ u32 buf_len, dma_addr_t buf_addr)
{
enum dma_slave_buswidth src_addr_width, dst_addr_width;
int src_bus_width, dst_bus_width;
@@ -739,7 +744,8 @@ static int stm32_dma_set_xfer_param(struct stm32_dma_chan *chan,
return dst_burst_size;

/* Set memory data size */
- src_addr_width = stm32_dma_get_max_width(buf_len, fifoth);
+ src_addr_width = stm32_dma_get_max_width(buf_len, buf_addr,
+ fifoth);
chan->mem_width = src_addr_width;
src_bus_width = stm32_dma_get_width(chan, src_addr_width);
if (src_bus_width < 0)
@@ -788,7 +794,8 @@ static int stm32_dma_set_xfer_param(struct stm32_dma_chan *chan,
return src_burst_size;

/* Set memory data size */
- dst_addr_width = stm32_dma_get_max_width(buf_len, fifoth);
+ dst_addr_width = stm32_dma_get_max_width(buf_len, buf_addr,
+ fifoth);
chan->mem_width = dst_addr_width;
dst_bus_width = stm32_dma_get_width(chan, dst_addr_width);
if (dst_bus_width < 0)
@@ -876,7 +883,8 @@ static struct dma_async_tx_descriptor *stm32_dma_prep_slave_sg(

for_each_sg(sgl, sg, sg_len, i) {
ret = stm32_dma_set_xfer_param(chan, direction, &buswidth,
- sg_dma_len(sg));
+ sg_dma_len(sg),
+ sg_dma_address(sg));
if (ret < 0)
goto err;

@@ -944,7 +952,8 @@ static struct dma_async_tx_descriptor *stm32_dma_prep_dma_cyclic(
return NULL;
}

- ret = stm32_dma_set_xfer_param(chan, direction, &buswidth, period_len);
+ ret = stm32_dma_set_xfer_param(chan, direction, &buswidth, period_len,
+ buf_addr);
if (ret < 0)
return NULL;

--
2.17.1

2020-12-11 18:10:29

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 0/4] Bunch of improvements for STM32 DMA controllers

On 20-11-20, 15:33, Amelie Delaunay wrote:
> This series brings 3 patches for STM32 DMA and 1 for STM32 MDMA.
> They increase the reliability and the efficiency of the transfers.

Applied, thanks

--
~Vinod