* [PATCH 2/7] dmaengine: sprd: Add validation of current descriptor in irq handler
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
In-Reply-To: <cover.1555330115.git.baolin.wang@linaro.org>
When user terminates one DMA channel to free all its descriptors, but
at the same time one transaction interrupt was triggered possibly, now
we should not handle this interrupt by validating if the 'schan->cur_desc'
was set as NULL to avoid crashing the kernel.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index e29342a..431e289 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -552,12 +552,17 @@ static irqreturn_t dma_irq_handle(int irq, void *dev_id)
schan = &sdev->channels[i];
spin_lock(&schan->vc.lock);
+
+ sdesc = schan->cur_desc;
+ if (!sdesc) {
+ spin_unlock(&schan->vc.lock);
+ return IRQ_HANDLED;
+ }
+
int_type = sprd_dma_get_int_type(schan);
req_type = sprd_dma_get_req_type(schan);
sprd_dma_clear_int(schan);
- sdesc = schan->cur_desc;
-
/* cyclic mode schedule callback */
cyclic = schan->linklist.phy_addr ? true : false;
if (cyclic == true) {
--
1.7.9.5
^ permalink raw reply related
* [PATCH 6/7] dmaengine: sprd: Fix the right place to configure 2-stage transfer
From: Baolin Wang @ 2019-04-15 12:15 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
In-Reply-To: <cover.1555330115.git.baolin.wang@linaro.org>
From: Eric Long <eric.long@unisoc.com>
Move the 2-stage configuration before configuring the link-list mode,
since we will use some 2-stage configuration to fill the link-list
configuration.
Signed-off-by: Eric Long <eric.long@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index a64271e..cc9c24d 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -911,6 +911,12 @@ static int sprd_dma_fill_linklist_desc(struct dma_chan *chan,
schan->linklist.virt_addr = 0;
}
+ /* Set channel mode and trigger mode for 2-stage transfer */
+ schan->chn_mode =
+ (flags >> SPRD_DMA_CHN_MODE_SHIFT) & SPRD_DMA_CHN_MODE_MASK;
+ schan->trg_mode =
+ (flags >> SPRD_DMA_TRG_MODE_SHIFT) & SPRD_DMA_TRG_MODE_MASK;
+
sdesc = kzalloc(sizeof(*sdesc), GFP_NOWAIT);
if (!sdesc)
return NULL;
@@ -944,12 +950,6 @@ static int sprd_dma_fill_linklist_desc(struct dma_chan *chan,
}
}
- /* Set channel mode and trigger mode for 2-stage transfer */
- schan->chn_mode =
- (flags >> SPRD_DMA_CHN_MODE_SHIFT) & SPRD_DMA_CHN_MODE_MASK;
- schan->trg_mode =
- (flags >> SPRD_DMA_TRG_MODE_SHIFT) & SPRD_DMA_TRG_MODE_MASK;
-
ret = sprd_dma_fill_desc(chan, &sdesc->chn_hw, 0, 0, src, dst, len,
dir, flags, slave_cfg);
if (ret) {
--
1.7.9.5
^ permalink raw reply related
* [PATCH 7/7] dmaengine: sprd: Add interrupt support for 2-stage transfer
From: Baolin Wang @ 2019-04-15 12:15 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
In-Reply-To: <cover.1555330115.git.baolin.wang@linaro.org>
For 2-stage transfer, some users like Audio still need transaction interrupt
to notify when the 2-stage transfer is completed. Thus we should enable
2-stage transfer interrupt to support this feature.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index cc9c24d..4c18f44 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -62,6 +62,8 @@
/* SPRD_DMA_GLB_2STAGE_GRP register definition */
#define SPRD_DMA_GLB_2STAGE_EN BIT(24)
#define SPRD_DMA_GLB_CHN_INT_MASK GENMASK(23, 20)
+#define SPRD_DMA_GLB_DEST_INT BIT(22)
+#define SPRD_DMA_GLB_SRC_INT BIT(20)
#define SPRD_DMA_GLB_LIST_DONE_TRG BIT(19)
#define SPRD_DMA_GLB_TRANS_DONE_TRG BIT(18)
#define SPRD_DMA_GLB_BLOCK_DONE_TRG BIT(17)
@@ -135,6 +137,7 @@
/* define DMA channel mode & trigger mode mask */
#define SPRD_DMA_CHN_MODE_MASK GENMASK(7, 0)
#define SPRD_DMA_TRG_MODE_MASK GENMASK(7, 0)
+#define SPRD_DMA_INT_TYPE_MASK GENMASK(7, 0)
/* define the DMA transfer step type */
#define SPRD_DMA_NONE_STEP 0
@@ -190,6 +193,7 @@ struct sprd_dma_chn {
u32 dev_id;
enum sprd_dma_chn_mode chn_mode;
enum sprd_dma_trg_mode trg_mode;
+ enum sprd_dma_int_type int_type;
struct sprd_dma_desc *cur_desc;
};
@@ -429,6 +433,9 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = chn & SPRD_DMA_GLB_SRC_CHN_MASK;
val |= BIT(schan->trg_mode - 1) << SPRD_DMA_GLB_TRG_OFFSET;
val |= SPRD_DMA_GLB_2STAGE_EN;
+ if (schan->int_type != SPRD_DMA_NO_INT)
+ val |= SPRD_DMA_GLB_SRC_INT;
+
sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
break;
@@ -436,6 +443,9 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = chn & SPRD_DMA_GLB_SRC_CHN_MASK;
val |= BIT(schan->trg_mode - 1) << SPRD_DMA_GLB_TRG_OFFSET;
val |= SPRD_DMA_GLB_2STAGE_EN;
+ if (schan->int_type != SPRD_DMA_NO_INT)
+ val |= SPRD_DMA_GLB_SRC_INT;
+
sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
break;
@@ -443,6 +453,9 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = (chn << SPRD_DMA_GLB_DEST_CHN_OFFSET) &
SPRD_DMA_GLB_DEST_CHN_MASK;
val |= SPRD_DMA_GLB_2STAGE_EN;
+ if (schan->int_type != SPRD_DMA_NO_INT)
+ val |= SPRD_DMA_GLB_DEST_INT;
+
sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
break;
@@ -450,6 +463,9 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = (chn << SPRD_DMA_GLB_DEST_CHN_OFFSET) &
SPRD_DMA_GLB_DEST_CHN_MASK;
val |= SPRD_DMA_GLB_2STAGE_EN;
+ if (schan->int_type != SPRD_DMA_NO_INT)
+ val |= SPRD_DMA_GLB_DEST_INT;
+
sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
break;
@@ -911,11 +927,15 @@ static int sprd_dma_fill_linklist_desc(struct dma_chan *chan,
schan->linklist.virt_addr = 0;
}
- /* Set channel mode and trigger mode for 2-stage transfer */
+ /*
+ * Set channel mode, interrupt mode and trigger mode for 2-stage
+ * transfer.
+ */
schan->chn_mode =
(flags >> SPRD_DMA_CHN_MODE_SHIFT) & SPRD_DMA_CHN_MODE_MASK;
schan->trg_mode =
(flags >> SPRD_DMA_TRG_MODE_SHIFT) & SPRD_DMA_TRG_MODE_MASK;
+ schan->int_type = flags & SPRD_DMA_INT_TYPE_MASK;
sdesc = kzalloc(sizeof(*sdesc), GFP_NOWAIT);
if (!sdesc)
--
1.7.9.5
^ permalink raw reply related
* [PATCH 5/7] dmaengine: sprd: Fix block length overflow
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
In-Reply-To: <cover.1555330115.git.baolin.wang@linaro.org>
From: Eric Long <eric.long@unisoc.com>
The maximum value of block length is 0xffff, so if the configured transfer length
is more than 0xffff, that will cause block length overflow to lead a configuration
error.
Thus we can set block length as the maximum burst length to avoid this issue, since
the maximum burst length will not be a big value which is more than 0xffff.
Signed-off-by: Eric Long <eric.long@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 9f99d4b..a64271e 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -778,7 +778,7 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
temp |= slave_cfg->src_maxburst & SPRD_DMA_FRG_LEN_MASK;
hw->frg_len = temp;
- hw->blk_len = len & SPRD_DMA_BLK_LEN_MASK;
+ hw->blk_len = slave_cfg->src_maxburst & SPRD_DMA_BLK_LEN_MASK;
hw->trsc_len = len & SPRD_DMA_TRSC_LEN_MASK;
temp = (dst_step & SPRD_DMA_TRSF_STEP_MASK) << SPRD_DMA_DEST_TRSF_STEP_OFFSET;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 4/7] dmaengine: sprd: Add device validation to support multiple controllers
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
In-Reply-To: <cover.1555330115.git.baolin.wang@linaro.org>
From: Eric Long <eric.long@unisoc.com>
Since we can support multiple DMA engine controllers, we should add
device validation in filter function to check if the correct controller
to be requested.
Signed-off-by: Eric Long <eric.long@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 0f92e60..9f99d4b 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -1020,8 +1020,13 @@ static void sprd_dma_free_desc(struct virt_dma_desc *vd)
static bool sprd_dma_filter_fn(struct dma_chan *chan, void *param)
{
struct sprd_dma_chn *schan = to_sprd_dma_chan(chan);
+ struct of_phandle_args *dma_spec =
+ container_of(param, struct of_phandle_args, args[0]);
u32 slave_id = *(u32 *)param;
+ if (chan->device->dev->of_node != dma_spec->np)
+ return false;
+
schan->dev_id = slave_id;
return true;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/7] dmaengine: sprd: Fix the incorrect start for 2-stage destination channels
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
In-Reply-To: <cover.1555330115.git.baolin.wang@linaro.org>
From: Eric Long <eric.long@unisoc.com>
The 2-stage destination channel will be triggered by source channel
automatically, which means we should not trigger it by software request.
Signed-off-by: Eric Long <eric.long@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 431e289..0f92e60 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -510,7 +510,9 @@ static void sprd_dma_start(struct sprd_dma_chn *schan)
sprd_dma_set_uid(schan);
sprd_dma_enable_chn(schan);
- if (schan->dev_id == SPRD_DMA_SOFTWARE_UID)
+ if (schan->dev_id == SPRD_DMA_SOFTWARE_UID &&
+ schan->chn_mode != SPRD_DMA_DST_CHN0 &&
+ schan->chn_mode != SPRD_DMA_DST_CHN1)
sprd_dma_soft_request(schan);
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 0/7] Fix some bugs and add new feature for Spreadtrum DMA engine
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
Hi,
This patch set fixes some DMA engine bugs and adds interrupt support
for 2-stage transfer.
Baolin Wang (3):
dmaengine: sprd: Fix the possible crash when getting engine status
dmaengine: sprd: Add validation of current descriptor in irq handler
dmaengine: sprd: Add interrupt support for 2-stage transfer
Eric Long (4):
dmaengine: sprd: Fix the incorrect start for 2-stage destination
channels
dmaengine: sprd: Add device validation to support multiple
controllers
dmaengine: sprd: Fix block length overflow
dmaengine: sprd: Fix the right place to configure 2-stage transfer
drivers/dma/sprd-dma.c | 54 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 43 insertions(+), 11 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [7/7] dmaengine: sprd: Add interrupt support for 2-stage transfer
From: Baolin Wang @ 2019-04-15 12:15 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
For 2-stage transfer, some users like Audio still need transaction interrupt
to notify when the 2-stage transfer is completed. Thus we should enable
2-stage transfer interrupt to support this feature.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index cc9c24d..4c18f44 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -62,6 +62,8 @@
/* SPRD_DMA_GLB_2STAGE_GRP register definition */
#define SPRD_DMA_GLB_2STAGE_EN BIT(24)
#define SPRD_DMA_GLB_CHN_INT_MASK GENMASK(23, 20)
+#define SPRD_DMA_GLB_DEST_INT BIT(22)
+#define SPRD_DMA_GLB_SRC_INT BIT(20)
#define SPRD_DMA_GLB_LIST_DONE_TRG BIT(19)
#define SPRD_DMA_GLB_TRANS_DONE_TRG BIT(18)
#define SPRD_DMA_GLB_BLOCK_DONE_TRG BIT(17)
@@ -135,6 +137,7 @@
/* define DMA channel mode & trigger mode mask */
#define SPRD_DMA_CHN_MODE_MASK GENMASK(7, 0)
#define SPRD_DMA_TRG_MODE_MASK GENMASK(7, 0)
+#define SPRD_DMA_INT_TYPE_MASK GENMASK(7, 0)
/* define the DMA transfer step type */
#define SPRD_DMA_NONE_STEP 0
@@ -190,6 +193,7 @@ struct sprd_dma_chn {
u32 dev_id;
enum sprd_dma_chn_mode chn_mode;
enum sprd_dma_trg_mode trg_mode;
+ enum sprd_dma_int_type int_type;
struct sprd_dma_desc *cur_desc;
};
@@ -429,6 +433,9 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = chn & SPRD_DMA_GLB_SRC_CHN_MASK;
val |= BIT(schan->trg_mode - 1) << SPRD_DMA_GLB_TRG_OFFSET;
val |= SPRD_DMA_GLB_2STAGE_EN;
+ if (schan->int_type != SPRD_DMA_NO_INT)
+ val |= SPRD_DMA_GLB_SRC_INT;
+
sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
break;
@@ -436,6 +443,9 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = chn & SPRD_DMA_GLB_SRC_CHN_MASK;
val |= BIT(schan->trg_mode - 1) << SPRD_DMA_GLB_TRG_OFFSET;
val |= SPRD_DMA_GLB_2STAGE_EN;
+ if (schan->int_type != SPRD_DMA_NO_INT)
+ val |= SPRD_DMA_GLB_SRC_INT;
+
sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
break;
@@ -443,6 +453,9 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = (chn << SPRD_DMA_GLB_DEST_CHN_OFFSET) &
SPRD_DMA_GLB_DEST_CHN_MASK;
val |= SPRD_DMA_GLB_2STAGE_EN;
+ if (schan->int_type != SPRD_DMA_NO_INT)
+ val |= SPRD_DMA_GLB_DEST_INT;
+
sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
break;
@@ -450,6 +463,9 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = (chn << SPRD_DMA_GLB_DEST_CHN_OFFSET) &
SPRD_DMA_GLB_DEST_CHN_MASK;
val |= SPRD_DMA_GLB_2STAGE_EN;
+ if (schan->int_type != SPRD_DMA_NO_INT)
+ val |= SPRD_DMA_GLB_DEST_INT;
+
sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
break;
@@ -911,11 +927,15 @@ static int sprd_dma_fill_linklist_desc(struct dma_chan *chan,
schan->linklist.virt_addr = 0;
}
- /* Set channel mode and trigger mode for 2-stage transfer */
+ /*
+ * Set channel mode, interrupt mode and trigger mode for 2-stage
+ * transfer.
+ */
schan->chn_mode =
(flags >> SPRD_DMA_CHN_MODE_SHIFT) & SPRD_DMA_CHN_MODE_MASK;
schan->trg_mode =
(flags >> SPRD_DMA_TRG_MODE_SHIFT) & SPRD_DMA_TRG_MODE_MASK;
+ schan->int_type = flags & SPRD_DMA_INT_TYPE_MASK;
sdesc = kzalloc(sizeof(*sdesc), GFP_NOWAIT);
if (!sdesc)
^ permalink raw reply related
* [6/7] dmaengine: sprd: Fix the right place to configure 2-stage transfer
From: Baolin Wang @ 2019-04-15 12:15 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
From: Eric Long <eric.long@unisoc.com>
Move the 2-stage configuration before configuring the link-list mode,
since we will use some 2-stage configuration to fill the link-list
configuration.
Signed-off-by: Eric Long <eric.long@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index a64271e..cc9c24d 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -911,6 +911,12 @@ static int sprd_dma_fill_linklist_desc(struct dma_chan *chan,
schan->linklist.virt_addr = 0;
}
+ /* Set channel mode and trigger mode for 2-stage transfer */
+ schan->chn_mode =
+ (flags >> SPRD_DMA_CHN_MODE_SHIFT) & SPRD_DMA_CHN_MODE_MASK;
+ schan->trg_mode =
+ (flags >> SPRD_DMA_TRG_MODE_SHIFT) & SPRD_DMA_TRG_MODE_MASK;
+
sdesc = kzalloc(sizeof(*sdesc), GFP_NOWAIT);
if (!sdesc)
return NULL;
@@ -944,12 +950,6 @@ static int sprd_dma_fill_linklist_desc(struct dma_chan *chan,
}
}
- /* Set channel mode and trigger mode for 2-stage transfer */
- schan->chn_mode =
- (flags >> SPRD_DMA_CHN_MODE_SHIFT) & SPRD_DMA_CHN_MODE_MASK;
- schan->trg_mode =
- (flags >> SPRD_DMA_TRG_MODE_SHIFT) & SPRD_DMA_TRG_MODE_MASK;
-
ret = sprd_dma_fill_desc(chan, &sdesc->chn_hw, 0, 0, src, dst, len,
dir, flags, slave_cfg);
if (ret) {
^ permalink raw reply related
* [5/7] dmaengine: sprd: Fix block length overflow
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
From: Eric Long <eric.long@unisoc.com>
The maximum value of block length is 0xffff, so if the configured transfer length
is more than 0xffff, that will cause block length overflow to lead a configuration
error.
Thus we can set block length as the maximum burst length to avoid this issue, since
the maximum burst length will not be a big value which is more than 0xffff.
Signed-off-by: Eric Long <eric.long@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 9f99d4b..a64271e 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -778,7 +778,7 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
temp |= slave_cfg->src_maxburst & SPRD_DMA_FRG_LEN_MASK;
hw->frg_len = temp;
- hw->blk_len = len & SPRD_DMA_BLK_LEN_MASK;
+ hw->blk_len = slave_cfg->src_maxburst & SPRD_DMA_BLK_LEN_MASK;
hw->trsc_len = len & SPRD_DMA_TRSC_LEN_MASK;
temp = (dst_step & SPRD_DMA_TRSF_STEP_MASK) << SPRD_DMA_DEST_TRSF_STEP_OFFSET;
^ permalink raw reply related
* [4/7] dmaengine: sprd: Add device validation to support multiple controllers
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
From: Eric Long <eric.long@unisoc.com>
Since we can support multiple DMA engine controllers, we should add
device validation in filter function to check if the correct controller
to be requested.
Signed-off-by: Eric Long <eric.long@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 0f92e60..9f99d4b 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -1020,8 +1020,13 @@ static void sprd_dma_free_desc(struct virt_dma_desc *vd)
static bool sprd_dma_filter_fn(struct dma_chan *chan, void *param)
{
struct sprd_dma_chn *schan = to_sprd_dma_chan(chan);
+ struct of_phandle_args *dma_spec =
+ container_of(param, struct of_phandle_args, args[0]);
u32 slave_id = *(u32 *)param;
+ if (chan->device->dev->of_node != dma_spec->np)
+ return false;
+
schan->dev_id = slave_id;
return true;
}
^ permalink raw reply related
* [3/7] dmaengine: sprd: Fix the incorrect start for 2-stage destination channels
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
From: Eric Long <eric.long@unisoc.com>
The 2-stage destination channel will be triggered by source channel
automatically, which means we should not trigger it by software request.
Signed-off-by: Eric Long <eric.long@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 431e289..0f92e60 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -510,7 +510,9 @@ static void sprd_dma_start(struct sprd_dma_chn *schan)
sprd_dma_set_uid(schan);
sprd_dma_enable_chn(schan);
- if (schan->dev_id == SPRD_DMA_SOFTWARE_UID)
+ if (schan->dev_id == SPRD_DMA_SOFTWARE_UID &&
+ schan->chn_mode != SPRD_DMA_DST_CHN0 &&
+ schan->chn_mode != SPRD_DMA_DST_CHN1)
sprd_dma_soft_request(schan);
}
^ permalink raw reply related
* [2/7] dmaengine: sprd: Add validation of current descriptor in irq handler
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
When user terminates one DMA channel to free all its descriptors, but
at the same time one transaction interrupt was triggered possibly, now
we should not handle this interrupt by validating if the 'schan->cur_desc'
was set as NULL to avoid crashing the kernel.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index e29342a..431e289 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -552,12 +552,17 @@ static irqreturn_t dma_irq_handle(int irq, void *dev_id)
schan = &sdev->channels[i];
spin_lock(&schan->vc.lock);
+
+ sdesc = schan->cur_desc;
+ if (!sdesc) {
+ spin_unlock(&schan->vc.lock);
+ return IRQ_HANDLED;
+ }
+
int_type = sprd_dma_get_int_type(schan);
req_type = sprd_dma_get_req_type(schan);
sprd_dma_clear_int(schan);
- sdesc = schan->cur_desc;
-
/* cyclic mode schedule callback */
cyclic = schan->linklist.phy_addr ? true : false;
if (cyclic == true) {
^ permalink raw reply related
* [1/7] dmaengine: sprd: Fix the possible crash when getting engine status
From: Baolin Wang @ 2019-04-15 12:14 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, orsonzhai, zhang.lyra, broonie, baolin.wang, dmaengine,
linux-kernel
We will get a NULL virtual descriptor by vchan_find_desc() when the descriptor
has been submitted, that will crash the kernel when getting the engine status.
In this case, since the descriptor has been submitted, which means the pointer
'schan->cur_desc' will point to the current descriptor, then we can use
'schan->cur_desc' to get the engine status to avoid this issue.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/dma/sprd-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 48431e2..e29342a 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -625,7 +625,7 @@ static enum dma_status sprd_dma_tx_status(struct dma_chan *chan,
else
pos = 0;
} else if (schan->cur_desc && schan->cur_desc->vd.tx.cookie == cookie) {
- struct sprd_dma_desc *sdesc = to_sprd_dma_desc(vd);
+ struct sprd_dma_desc *sdesc = schan->cur_desc;
if (sdesc->dir == DMA_DEV_TO_MEM)
pos = sprd_dma_get_dst_addr(schan);
^ permalink raw reply related
* Re: [PATCH 4/6] dma: tegra: add accurate reporting of dma state
From: Dmitry Osipenko @ 2019-04-14 15:20 UTC (permalink / raw)
To: Ben Dooks; +Cc: dan.j.williams, vkoul, ldewangan, dmaengine, linux-tegra
In-Reply-To: <85a8afa6-8b0d-dcc2-3254-a79cf90e90df@gmail.com>
22.02.2019 21:10, Dmitry Osipenko пишет:
> 22.02.2019 20:23, Ben Dooks пишет:
>> On 21/02/2019 13:02, Dmitry Osipenko wrote:
>>> 21.02.2019 13:06, Ben Dooks пишет:
>>>> On 21/02/2019 00:41, Dmitry Osipenko wrote:
>>>>> 31.10.2018 19:03, Ben Dooks пишет:
>>>>>> The tx_status callback does not report the state of the transfer
>>>>>> beyond complete segments. This causes problems with users such as
>>>>>> ALSA when applications want to know accurately how much data has
>>>>>> been moved.
>>>>>>
>>>>>> This patch addes a function tegra_dma_update_residual() to query
>>>>>> the hardware and modify the residual information accordinly. It
>>>>>> takes into account any hardware issues when trying to read the
>>>>>> state, such as delays between finishing a buffer and signalling
>>>>>> the interrupt.
>>>>>>
>>>>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>>>>
>>>>> Hello Ben,
>>>>>
>>>>> Do you have any plans to submit a new version of this patch? It's really useful and fixes a real problem with the audio playback. I could help with finalizing the patch and could submit it for you if you happened to lost the interest.
>>>>
>>>> Personally I think the original version was fine. It has been tested
>>>> and returns fairly quickly (I am not a fan of just adding more delay in)
>>>>
>>>> My notes say the condition doesn't last for long and the loop tends
>>>> to terminate within 2 runs.
>>>>
>>>
>>> Okay, so are you going to re-send the patch? We can back to the review after, you need at least to re-send because this series has been outdated. Also please take a look and feel free to use as-is the reduced variant of yours patch that I was carrying and testing for months now [0], it works great.
>>>
>>> [0] https://github.com/grate-driver/linux/commit/ab8a67a6f47185f265f16749b55df214aaaefad4
>>>
>>
>> I can try rebasing, but I have not got a lot of time to do any testing
>> at the moment. I agree I should have remembered to chase this stuff up
>> earlier.
>
> No problems, thank you. I'll help with the testing. And I could rebase the patch and send it out for you if will be needed, please just let me know if you're okay with it.
>
Hello Ben,
Do you have any status update on the state of the patch? Again, I could help with sending it out for you if you're too busy or something else. Please let me know how we could proceed with getting the fix upstreamed and sorry for disturbing you again with this.
^ permalink raw reply
* [4/6] dma: tegra: add accurate reporting of dma state
From: Dmitry Osipenko @ 2019-04-14 15:20 UTC (permalink / raw)
To: Ben Dooks; +Cc: dan.j.williams, vkoul, ldewangan, dmaengine, linux-tegra
22.02.2019 21:10, Dmitry Osipenko пишет:
> 22.02.2019 20:23, Ben Dooks пишет:
>> On 21/02/2019 13:02, Dmitry Osipenko wrote:
>>> 21.02.2019 13:06, Ben Dooks пишет:
>>>> On 21/02/2019 00:41, Dmitry Osipenko wrote:
>>>>> 31.10.2018 19:03, Ben Dooks пишет:
>>>>>> The tx_status callback does not report the state of the transfer
>>>>>> beyond complete segments. This causes problems with users such as
>>>>>> ALSA when applications want to know accurately how much data has
>>>>>> been moved.
>>>>>>
>>>>>> This patch addes a function tegra_dma_update_residual() to query
>>>>>> the hardware and modify the residual information accordinly. It
>>>>>> takes into account any hardware issues when trying to read the
>>>>>> state, such as delays between finishing a buffer and signalling
>>>>>> the interrupt.
>>>>>>
>>>>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>>>>
>>>>> Hello Ben,
>>>>>
>>>>> Do you have any plans to submit a new version of this patch? It's really useful and fixes a real problem with the audio playback. I could help with finalizing the patch and could submit it for you if you happened to lost the interest.
>>>>
>>>> Personally I think the original version was fine. It has been tested
>>>> and returns fairly quickly (I am not a fan of just adding more delay in)
>>>>
>>>> My notes say the condition doesn't last for long and the loop tends
>>>> to terminate within 2 runs.
>>>>
>>>
>>> Okay, so are you going to re-send the patch? We can back to the review after, you need at least to re-send because this series has been outdated. Also please take a look and feel free to use as-is the reduced variant of yours patch that I was carrying and testing for months now [0], it works great.
>>>
>>> [0] https://github.com/grate-driver/linux/commit/ab8a67a6f47185f265f16749b55df214aaaefad4
>>>
>>
>> I can try rebasing, but I have not got a lot of time to do any testing
>> at the moment. I agree I should have remembered to chase this stuff up
>> earlier.
>
> No problems, thank you. I'll help with the testing. And I could rebase the patch and send it out for you if will be needed, please just let me know if you're okay with it.
>
Hello Ben,
Do you have any status update on the state of the patch? Again, I could help with sending it out for you if you're too busy or something else. Please let me know how we could proceed with getting the fix upstreamed and sorry for disturbing you again with this.
^ permalink raw reply
* Re: [PATCH v3 1/2] dmaengine: sh: rcar-dmac: With cyclic DMA residue 0 is valid
From: Laurent Pinchart @ 2019-04-12 9:54 UTC (permalink / raw)
To: Dirk Behme
Cc: linux-renesas-soc, dmaengine, vkoul, geert+renesas,
niklas.soderlund+renesas, laurent.pinchart+renesas,
Achim.Dahlhoff, yoshihiro.shimoda.uh, ylhuajnu,
hiroyuki.yokoyama.vx, kuninori.morimoto.gx, stable
In-Reply-To: <20190412052914.16006-1-dirk.behme@de.bosch.com>
Hi Dirk,
Thank you for the patch.
On Fri, Apr 12, 2019 at 07:29:13AM +0200, Dirk Behme wrote:
> Having a cyclic DMA, a residue 0 is not an indication of a completed
> DMA. In case of cyclic DMA make sure that dma_set_residue() is called
> and with this a residue of 0 is forwarded correctly to the caller.
>
> Fixes: 3544d2878817 ("dmaengine: rcar-dmac: use result of updated get_residue in tx_status")
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com>
> Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> Signed-off-by: Yao Lihua <ylhuajnu@outlook.com>
> Cc: <stable@vger.kernel.org> # v4.8+
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Note: Patch done against mainline v5.0
>
> Changes in v2: None
>
> Changes in v3: Move reading rchan into the spin lock protection.
>
> drivers/dma/sh/rcar-dmac.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 2b4f25698169..54810ffd95e2 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1368,6 +1368,7 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,
> enum dma_status status;
> unsigned long flags;
> unsigned int residue;
> + bool cyclic;
>
> status = dma_cookie_status(chan, cookie, txstate);
> if (status == DMA_COMPLETE || !txstate)
> @@ -1375,10 +1376,11 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,
>
> spin_lock_irqsave(&rchan->lock, flags);
> residue = rcar_dmac_chan_get_residue(rchan, cookie);
> + cyclic = rchan->desc.running ? rchan->desc.running->cyclic : false;
> spin_unlock_irqrestore(&rchan->lock, flags);
>
> /* if there's no residue, the cookie is complete */
> - if (!residue)
> + if (!residue && !cyclic)
> return DMA_COMPLETE;
>
> dma_set_residue(txstate, residue);
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [v3,1/2] dmaengine: sh: rcar-dmac: With cyclic DMA residue 0 is valid
From: Laurent Pinchart @ 2019-04-12 9:54 UTC (permalink / raw)
To: Dirk Behme
Cc: linux-renesas-soc, dmaengine, vkoul, geert+renesas,
niklas.soderlund+renesas, laurent.pinchart+renesas,
Achim.Dahlhoff, yoshihiro.shimoda.uh, ylhuajnu,
hiroyuki.yokoyama.vx, kuninori.morimoto.gx, stable
Hi Dirk,
Thank you for the patch.
On Fri, Apr 12, 2019 at 07:29:13AM +0200, Dirk Behme wrote:
> Having a cyclic DMA, a residue 0 is not an indication of a completed
> DMA. In case of cyclic DMA make sure that dma_set_residue() is called
> and with this a residue of 0 is forwarded correctly to the caller.
>
> Fixes: 3544d2878817 ("dmaengine: rcar-dmac: use result of updated get_residue in tx_status")
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com>
> Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> Signed-off-by: Yao Lihua <ylhuajnu@outlook.com>
> Cc: <stable@vger.kernel.org> # v4.8+
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Note: Patch done against mainline v5.0
>
> Changes in v2: None
>
> Changes in v3: Move reading rchan into the spin lock protection.
>
> drivers/dma/sh/rcar-dmac.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 2b4f25698169..54810ffd95e2 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1368,6 +1368,7 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,
> enum dma_status status;
> unsigned long flags;
> unsigned int residue;
> + bool cyclic;
>
> status = dma_cookie_status(chan, cookie, txstate);
> if (status == DMA_COMPLETE || !txstate)
> @@ -1375,10 +1376,11 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,
>
> spin_lock_irqsave(&rchan->lock, flags);
> residue = rcar_dmac_chan_get_residue(rchan, cookie);
> + cyclic = rchan->desc.running ? rchan->desc.running->cyclic : false;
> spin_unlock_irqrestore(&rchan->lock, flags);
>
> /* if there's no residue, the cookie is complete */
> - if (!residue)
> + if (!residue && !cyclic)
> return DMA_COMPLETE;
>
> dma_set_residue(txstate, residue);
^ permalink raw reply
* RE: [PATCH v3 2/2] dmaengine: sh: rcar-dmac: Fix glitch in dmaengine_tx_status
From: Yoshihiro Shimoda @ 2019-04-12 8:52 UTC (permalink / raw)
To: REE dirk.behme@de.bosch.com
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, REE dirk.behme@de.bosch.com,
ylhuajnu@outlook.com, HIROYUKI YOKOYAMA, Kuninori Morimoto,
stable@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <20190412052914.16006-2-dirk.behme@de.bosch.com>
Hi Dirk-san,
> From: Dirk Behme, Sent: Friday, April 12, 2019 2:29 PM
>
> 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")
I guess this driver has this issue potencially from previous of the commit
(maybe commit ccadee9b1e9 ("dmaengine: rcar-dmac: Implement support for hardware descriptor lists"?).
But, this patch seems good to fix the issue.
So,
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Best regards,
Yoshihiro Shimoda
^ permalink raw reply
* [v3,2/2] dmaengine: sh: rcar-dmac: Fix glitch in dmaengine_tx_status
From: Yoshihiro Shimoda @ 2019-04-12 8:52 UTC (permalink / raw)
To: REE dirk.behme@de.bosch.com
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, ylhuajnu@outlook.com,
HIROYUKI YOKOYAMA, Kuninori Morimoto, stable@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
Hi Dirk-san,
> From: Dirk Behme, Sent: Friday, April 12, 2019 2:29 PM
>
> 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")
I guess this driver has this issue potencially from previous of the commit
(maybe commit ccadee9b1e9 ("dmaengine: rcar-dmac: Implement support for hardware descriptor lists"?).
But, this patch seems good to fix the issue.
So,
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Best regards,
Yoshihiro Shimoda
^ permalink raw reply
* RE: [PATCH v3 1/2] dmaengine: sh: rcar-dmac: With cyclic DMA residue 0 is valid
From: Yoshihiro Shimoda @ 2019-04-12 8:33 UTC (permalink / raw)
To: REE dirk.behme@de.bosch.com
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, REE dirk.behme@de.bosch.com,
ylhuajnu@outlook.com, HIROYUKI YOKOYAMA, Kuninori Morimoto,
stable@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <20190412052914.16006-1-dirk.behme@de.bosch.com>
Hi Dirk-san,
> From: Dirk Behme, Sent: Friday, April 12, 2019 2:29 PM
>
> Having a cyclic DMA, a residue 0 is not an indication of a completed
> DMA. In case of cyclic DMA make sure that dma_set_residue() is called
> and with this a residue of 0 is forwarded correctly to the caller.
>
> Fixes: 3544d2878817 ("dmaengine: rcar-dmac: use result of updated get_residue in tx_status")
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com>
> Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> Signed-off-by: Yao Lihua <ylhuajnu@outlook.com>
> Cc: <stable@vger.kernel.org> # v4.8+
> ---
Thank you for the patch! Since cyclic transfers will not stop even if
the residue is 0, I agree this driver should not return DMA_COMPLETE
(transaction completed). So,
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Best regards,
Yoshihiro Shimoda
^ permalink raw reply
* [v3,1/2] dmaengine: sh: rcar-dmac: With cyclic DMA residue 0 is valid
From: Yoshihiro Shimoda @ 2019-04-12 8:33 UTC (permalink / raw)
To: REE dirk.behme@de.bosch.com
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, ylhuajnu@outlook.com,
HIROYUKI YOKOYAMA, Kuninori Morimoto, stable@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
Hi Dirk-san,
> From: Dirk Behme, Sent: Friday, April 12, 2019 2:29 PM
>
> Having a cyclic DMA, a residue 0 is not an indication of a completed
> DMA. In case of cyclic DMA make sure that dma_set_residue() is called
> and with this a residue of 0 is forwarded correctly to the caller.
>
> Fixes: 3544d2878817 ("dmaengine: rcar-dmac: use result of updated get_residue in tx_status")
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com>
> Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> Signed-off-by: Yao Lihua <ylhuajnu@outlook.com>
> Cc: <stable@vger.kernel.org> # v4.8+
> ---
Thank you for the patch! Since cyclic transfers will not stop even if
the residue is 0, I agree this driver should not return DMA_COMPLETE
(transaction completed). So,
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Best regards,
Yoshihiro Shimoda
^ permalink raw reply
* [PATCH v3 1/2] dmaengine: sh: rcar-dmac: With cyclic DMA residue 0 is valid
From: Dirk Behme @ 2019-04-12 5:29 UTC (permalink / raw)
To: linux-renesas-soc
Cc: dmaengine, vkoul, geert+renesas, niklas.soderlund+renesas,
laurent.pinchart+renesas, Achim.Dahlhoff, dirk.behme,
yoshihiro.shimoda.uh, ylhuajnu, hiroyuki.yokoyama.vx,
kuninori.morimoto.gx, stable
Having a cyclic DMA, a residue 0 is not an indication of a completed
DMA. In case of cyclic DMA make sure that dma_set_residue() is called
and with this a residue of 0 is forwarded correctly to the caller.
Fixes: 3544d2878817 ("dmaengine: rcar-dmac: use result of updated get_residue in tx_status")
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com>
Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
Signed-off-by: Yao Lihua <ylhuajnu@outlook.com>
Cc: <stable@vger.kernel.org> # v4.8+
---
Note: Patch done against mainline v5.0
Changes in v2: None
Changes in v3: Move reading rchan into the spin lock protection.
drivers/dma/sh/rcar-dmac.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 2b4f25698169..54810ffd95e2 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1368,6 +1368,7 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,
enum dma_status status;
unsigned long flags;
unsigned int residue;
+ bool cyclic;
status = dma_cookie_status(chan, cookie, txstate);
if (status == DMA_COMPLETE || !txstate)
@@ -1375,10 +1376,11 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,
spin_lock_irqsave(&rchan->lock, flags);
residue = rcar_dmac_chan_get_residue(rchan, cookie);
+ cyclic = rchan->desc.running ? rchan->desc.running->cyclic : false;
spin_unlock_irqrestore(&rchan->lock, flags);
/* if there's no residue, the cookie is complete */
- if (!residue)
+ if (!residue && !cyclic)
return DMA_COMPLETE;
dma_set_residue(txstate, residue);
--
2.20.0
^ permalink raw reply related
* [PATCH v3 2/2] dmaengine: sh: rcar-dmac: Fix glitch in dmaengine_tx_status
From: Dirk Behme @ 2019-04-12 5:29 UTC (permalink / raw)
To: linux-renesas-soc
Cc: dmaengine, vkoul, geert+renesas, niklas.soderlund+renesas,
laurent.pinchart+renesas, Achim.Dahlhoff, dirk.behme,
yoshihiro.shimoda.uh, ylhuajnu, hiroyuki.yokoyama.vx,
kuninori.morimoto.gx, stable
In-Reply-To: <20190412052914.16006-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
Changes in v3: None
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 54810ffd95e2..e2a5398f89b5 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
^ permalink raw reply related
* [v3,2/2] dmaengine: sh: rcar-dmac: Fix glitch in dmaengine_tx_status
From: Dirk Behme @ 2019-04-12 5:29 UTC (permalink / raw)
To: linux-renesas-soc
Cc: dmaengine, vkoul, geert+renesas, niklas.soderlund+renesas,
laurent.pinchart+renesas, Achim.Dahlhoff, dirk.behme,
yoshihiro.shimoda.uh, ylhuajnu, hiroyuki.yokoyama.vx,
kuninori.morimoto.gx, stable
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
Changes in v3: None
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 54810ffd95e2..e2a5398f89b5 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;
}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox