Hi,
Changes since v2:
- Dropped patch 10 from v2 (simplify direction configuration...)
- Dropped the channel priority related patches since we are going to go via
different route for configuring the priority.
- Added ACK from Joel for the patches since they are not changed since v2
Changes since v1:
- ASoC patches removed
- Comments from Andriy Shevchenko addressed
- patch added to fix cases when src/dst_maxburst is set to 0
The series contains now only:
Support for DMA pause/resume in cyclic mode
device_slave_caps callback and DMA_CYCLIC flag correction.
While debugging the edma to get things sorted out I noticed that the debug was
too verbose and the important information was hidden even when the we did not
asked for verbose dmaengine debug.
I have included some debug cleanups for the edma dmaengine driver also.
Regards,
Peter
---
Peter Ujfalusi (10):
platform_data: edma: Be precise with the paRAM struct
arm: common: edma: Save the number of event queues/TCs
dmaengine: edma: Correct the handling of src/dst_maxburst == 0
dmaengine: edma: Add support for DMA_PAUSE/RESUME operation
dmaengine: edma: Set DMA_CYCLIC capability flag
dmaengine: edma: Implement device_slave_caps callback
dmaengine: edma: Reduce debug print verbosity for non verbose
debugging
dmaengine: edma: Prefix debug prints where the text were identical in
prep callbacks
dmaengine: edma: Add channel number to debug prints
dmaengine: edma: Print the direction value as well when it is not
supported
arch/arm/common/edma.c | 4 ++
drivers/dma/edma.c | 87 ++++++++++++++++++++++++++++++--------
include/linux/platform_data/edma.h | 18 ++++----
3 files changed, 83 insertions(+), 26 deletions(-)
--
1.9.2
The edmacc_param struct should follow the layout of the paRAM area in the
HW. Be explicit on the size of the fields (u32) and also mark the struct
as packed to avoid any padding on non 32bit architectures.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
include/linux/platform_data/edma.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index f50821cb64be..923f8a3e4ce0 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -43,15 +43,15 @@
/* PaRAM slots are laid out like this */
struct edmacc_param {
- unsigned int opt;
- unsigned int src;
- unsigned int a_b_cnt;
- unsigned int dst;
- unsigned int src_dst_bidx;
- unsigned int link_bcntrld;
- unsigned int src_dst_cidx;
- unsigned int ccnt;
-};
+ u32 opt;
+ u32 src;
+ u32 a_b_cnt;
+ u32 dst;
+ u32 src_dst_bidx;
+ u32 link_bcntrld;
+ u32 src_dst_cidx;
+ u32 ccnt;
+} __packed;
/* fields in edmacc_param.opt */
#define SAM BIT(0)
--
1.9.2
In case of not supported direction it is better to print the direction also.
It is unlikely, but in such an event it helps with the debugging.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
drivers/dma/edma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 4aa5eb005b5c..91849aa50de1 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -432,7 +432,7 @@ static struct dma_async_tx_descriptor *edma_prep_slave_sg(
dev_width = echan->cfg.dst_addr_width;
burst = echan->cfg.dst_maxburst;
} else {
- dev_err(dev, "%s: bad direction?\n", __func__);
+ dev_err(dev, "%s: bad direction: %d\n", __func__, direction);
return NULL;
}
@@ -525,7 +525,7 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
dev_width = echan->cfg.dst_addr_width;
burst = echan->cfg.dst_maxburst;
} else {
- dev_err(dev, "%s: bad direction?\n", __func__);
+ dev_err(dev, "%s: bad direction: %d\n", __func__, direction);
return NULL;
}
--
1.9.2
It helps to identify issues if we have some information regarding to the
channel which the event is associated.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
drivers/dma/edma.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index bc8175c92e0c..4aa5eb005b5c 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -185,7 +185,8 @@ static void edma_execute(struct edma_chan *echan)
edma_resume(echan->ch_num);
if (edesc->processed <= MAX_NR_SG) {
- dev_dbg(dev, "first transfer starting %d\n", echan->ch_num);
+ dev_dbg(dev, "first transfer starting on channel %d\n",
+ echan->ch_num);
edma_start(echan->ch_num);
}
@@ -195,7 +196,7 @@ static void edma_execute(struct edma_chan *echan)
* MAX_NR_SG
*/
if (echan->missed) {
- dev_dbg(dev, "missed event in execute detected\n");
+ dev_dbg(dev, "missed event on channel %d\n", echan->ch_num);
edma_clean_channel(echan->ch_num);
edma_stop(echan->ch_num);
edma_start(echan->ch_num);
@@ -735,7 +736,7 @@ static int edma_alloc_chan_resources(struct dma_chan *chan)
echan->alloced = true;
echan->slot[0] = echan->ch_num;
- dev_dbg(dev, "allocated channel for %u:%u\n",
+ dev_dbg(dev, "allocated channel %d for %u:%u\n", echan->ch_num,
EDMA_CTLR(echan->ch_num), EDMA_CHAN_SLOT(echan->ch_num));
return 0;
--
1.9.2
Indicate that the edma dmaengine driver has support for cyclic mode.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
arch/arm/common/edma.c | 1 +
drivers/dma/edma.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 999266bf69b9..0b37f7734d0f 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1574,6 +1574,7 @@ static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
return ERR_PTR(ret);
dma_cap_set(DMA_SLAVE, edma_filter_info.dma_cap);
+ dma_cap_set(DMA_CYCLIC, edma_filter_info.dma_cap);
of_dma_controller_register(dev->of_node, of_dma_simple_xlate,
&edma_filter_info);
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 7891378a03f0..1dd9e8806975 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -891,6 +891,7 @@ static int edma_probe(struct platform_device *pdev)
dma_cap_zero(ecc->dma_slave.cap_mask);
dma_cap_set(DMA_SLAVE, ecc->dma_slave.cap_mask);
+ dma_cap_set(DMA_CYCLIC, ecc->dma_slave.cap_mask);
edma_dma_init(ecc, &ecc->dma_slave, &pdev->dev);
--
1.9.2
With the callback implemented omap-dma can provide information to client
drivers regarding to supported address widths, directions, residue
granularity, etc.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
drivers/dma/edma.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 1dd9e8806975..2f58c04cbcb1 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -852,6 +852,23 @@ static void __init edma_chan_init(struct edma_cc *ecc,
}
}
+#define EDMA_DMA_BUSWIDTHS (BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
+ BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
+ BIT(DMA_SLAVE_BUSWIDTH_4_BYTES))
+
+static int edma_dma_device_slave_caps(struct dma_chan *dchan,
+ struct dma_slave_caps *caps)
+{
+ caps->src_addr_widths = EDMA_DMA_BUSWIDTHS;
+ caps->dstn_addr_widths = EDMA_DMA_BUSWIDTHS;
+ caps->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
+ caps->cmd_pause = true;
+ caps->cmd_terminate = true;
+ caps->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
+
+ return 0;
+}
+
static void edma_dma_init(struct edma_cc *ecc, struct dma_device *dma,
struct device *dev)
{
@@ -862,6 +879,7 @@ static void edma_dma_init(struct edma_cc *ecc, struct dma_device *dma,
dma->device_issue_pending = edma_issue_pending;
dma->device_tx_status = edma_tx_status;
dma->device_control = edma_control;
+ dma->device_slave_caps = edma_dma_device_slave_caps;
dma->dev = dev;
INIT_LIST_HEAD(&dma->channels);
--
1.9.2
Do not print the paRAM information when verbose debugging is not asked and
also reduce the number of lines printed in edma_prep_dma_cyclic()
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
drivers/dma/edma.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 2f58c04cbcb1..6d9edc47150d 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -141,7 +141,7 @@ static void edma_execute(struct edma_chan *echan)
for (i = 0; i < nslots; i++) {
j = i + edesc->processed;
edma_write_slot(echan->slot[i], &edesc->pset[j]);
- dev_dbg(echan->vchan.chan.device->dev,
+ dev_vdbg(echan->vchan.chan.device->dev,
"\n pset[%d]:\n"
" chnum\t%d\n"
" slot\t%d\n"
@@ -560,9 +560,8 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
edesc->cyclic = 1;
edesc->pset_nr = nslots;
- dev_dbg(dev, "%s: nslots=%d\n", __func__, nslots);
- dev_dbg(dev, "%s: period_len=%d\n", __func__, period_len);
- dev_dbg(dev, "%s: buf_len=%d\n", __func__, buf_len);
+ dev_dbg(dev, "%s: channel=%d nslots=%d period_len=%zu buf_len=%zu\n",
+ __func__, echan->ch_num, nslots, period_len, buf_len);
for (i = 0; i < nslots; i++) {
/* Allocate a PaRAM slot, if needed */
@@ -596,8 +595,8 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
else
src_addr += period_len;
- dev_dbg(dev, "%s: Configure period %d of buf:\n", __func__, i);
- dev_dbg(dev,
+ dev_vdbg(dev, "%s: Configure period %d of buf:\n", __func__, i);
+ dev_vdbg(dev,
"\n pset[%d]:\n"
" chnum\t%d\n"
" slot\t%d\n"
--
1.9.2
prep_slave_sg and prep_dma_cyclic callbacks have mostly same failure cases
with the same texts printed in case we hit them. It helps when debugging if
we know exactly which callback generated the errors.
At the same time change the debug level for descriptor allocation failure
from dbg to err since all other error cases are dev_err and this failure is
similarly fatal as the other ones.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
drivers/dma/edma.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 6d9edc47150d..bc8175c92e0c 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -436,14 +436,14 @@ static struct dma_async_tx_descriptor *edma_prep_slave_sg(
}
if (dev_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) {
- dev_err(dev, "Undefined slave buswidth\n");
+ dev_err(dev, "%s: Undefined slave buswidth\n", __func__);
return NULL;
}
edesc = kzalloc(sizeof(*edesc) + sg_len *
sizeof(edesc->pset[0]), GFP_ATOMIC);
if (!edesc) {
- dev_dbg(dev, "Failed to allocate a descriptor\n");
+ dev_err(dev, "%s: Failed to allocate a descriptor\n", __func__);
return NULL;
}
@@ -459,7 +459,8 @@ static struct dma_async_tx_descriptor *edma_prep_slave_sg(
EDMA_SLOT_ANY);
if (echan->slot[i] < 0) {
kfree(edesc);
- dev_err(dev, "Failed to allocate slot\n");
+ dev_err(dev, "%s: Failed to allocate slot\n",
+ __func__);
return NULL;
}
}
@@ -528,7 +529,7 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
}
if (dev_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) {
- dev_err(dev, "Undefined slave buswidth\n");
+ dev_err(dev, "%s: Undefined slave buswidth\n", __func__);
return NULL;
}
@@ -553,7 +554,7 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
edesc = kzalloc(sizeof(*edesc) + nslots *
sizeof(edesc->pset[0]), GFP_ATOMIC);
if (!edesc) {
- dev_dbg(dev, "Failed to allocate a descriptor\n");
+ dev_err(dev, "%s: Failed to allocate a descriptor\n", __func__);
return NULL;
}
@@ -571,7 +572,8 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
EDMA_SLOT_ANY);
if (echan->slot[i] < 0) {
kfree(edesc);
- dev_err(dev, "Failed to allocate slot\n");
+ dev_err(dev, "%s: Failed to allocate slot\n",
+ __func__);
return NULL;
}
}
--
1.9.2
Pause/Resume can be used by the audio stack when the stream is paused/resumed
The edma platform code has support for this and the legacy audio stack used
this.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
drivers/dma/edma.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 2742867fd1e6..7891378a03f0 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -240,6 +240,26 @@ static int edma_slave_config(struct edma_chan *echan,
return 0;
}
+static int edma_dma_pause(struct edma_chan *echan)
+{
+ /* Pause/Resume only allowed with cyclic mode */
+ if (!echan->edesc->cyclic)
+ return -EINVAL;
+
+ edma_pause(echan->ch_num);
+ return 0;
+}
+
+static int edma_dma_resume(struct edma_chan *echan)
+{
+ /* Pause/Resume only allowed with cyclic mode */
+ if (!echan->edesc->cyclic)
+ return -EINVAL;
+
+ edma_resume(echan->ch_num);
+ return 0;
+}
+
static int edma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
unsigned long arg)
{
@@ -255,6 +275,14 @@ static int edma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
config = (struct dma_slave_config *)arg;
ret = edma_slave_config(echan, config);
break;
+ case DMA_PAUSE:
+ ret = edma_dma_pause(echan);
+ break;
+
+ case DMA_RESUME:
+ ret = edma_dma_resume(echan);
+ break;
+
default:
ret = -ENOSYS;
}
--
1.9.2
For later use save the number of queues available for the CC.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
arch/arm/common/edma.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 41bca32409fc..999266bf69b9 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1768,6 +1768,9 @@ static int edma_probe(struct platform_device *pdev)
map_queue_tc(j, queue_tc_mapping[i][0],
queue_tc_mapping[i][1]);
+ /* Save the number of TCs */
+ edma_cc[j]->num_tc = i;
+
/* Event queue priority mapping */
for (i = 0; queue_priority_mapping[i][0] != -1; i++)
assign_priority_to_queue(j,
--
1.9.2
When clients asks for maxburst = 0 it is basically the same case as if they
were asking for maxburst = 1 since in both case ASYNC need to be used and
the eDMA is expected to write/read one word per DMA request.
Signed-off-by: Peter Ujfalusi <[email protected]>
Acked-by: Joel Fernandes <[email protected]>
---
drivers/dma/edma.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index cd04eb7b182e..2742867fd1e6 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -285,6 +285,10 @@ static int edma_config_pset(struct dma_chan *chan, struct edmacc_param *pset,
int absync;
acnt = dev_width;
+
+ /* src/dst_maxburst == 0 is the same case as src/dst_maxburst == 1 */
+ if (!burst)
+ burst = 1;
/*
* If the maxburst is equal to the fifo width, use
* A-synced transfers. This allows for large contiguous
--
1.9.2
On 04/14/2014 06:41 AM, Peter Ujfalusi wrote:
> Hi,
>
> Changes since v2:
> - Dropped patch 10 from v2 (simplify direction configuration...)
> - Dropped the channel priority related patches since we are going to go via
> different route for configuring the priority.
> - Added ACK from Joel for the patches since they are not changed since v2
>
> Changes since v1:
> - ASoC patches removed
> - Comments from Andriy Shevchenko addressed
> - patch added to fix cases when src/dst_maxburst is set to 0
>
> The series contains now only:
> Support for DMA pause/resume in cyclic mode
> device_slave_caps callback and DMA_CYCLIC flag correction.
> While debugging the edma to get things sorted out I noticed that the debug was
> too verbose and the important information was hidden even when the we did not
> asked for verbose dmaengine debug.
> I have included some debug cleanups for the edma dmaengine driver also.
>
> Regards,
> Peter
> ---
> Peter Ujfalusi (10):
> platform_data: edma: Be precise with the paRAM struct
> arm: common: edma: Save the number of event queues/TCs
> dmaengine: edma: Correct the handling of src/dst_maxburst == 0
> dmaengine: edma: Add support for DMA_PAUSE/RESUME operation
> dmaengine: edma: Set DMA_CYCLIC capability flag
> dmaengine: edma: Implement device_slave_caps callback
> dmaengine: edma: Reduce debug print verbosity for non verbose
> debugging
> dmaengine: edma: Prefix debug prints where the text were identical in
> prep callbacks
> dmaengine: edma: Add channel number to debug prints
> dmaengine: edma: Print the direction value as well when it is not
> supported
>
> arch/arm/common/edma.c | 4 ++
> drivers/dma/edma.c | 87 ++++++++++++++++++++++++++++++--------
> include/linux/platform_data/edma.h | 18 ++++----
> 3 files changed, 83 insertions(+), 26 deletions(-)
>
I reviewed and tested all the patches in the new series to make sure it
doesn't break anything with non-cyclic users (MMC and Crypto).
Reviewed-and-Tested-by: Joel Fernandes <[email protected]>
regards,
-Joel
Hi Vinod, Dan,
On 04/14/2014 06:41 AM, Peter Ujfalusi wrote:
> Hi,
>
> Changes since v2:
> - Dropped patch 10 from v2 (simplify direction configuration...)
> - Dropped the channel priority related patches since we are going to go via
> different route for configuring the priority.
> - Added ACK from Joel for the patches since they are not changed since v2
>
> Changes since v1:
> - ASoC patches removed
> - Comments from Andriy Shevchenko addressed
> - patch added to fix cases when src/dst_maxburst is set to 0
>
> The series contains now only:
> Support for DMA pause/resume in cyclic mode
> device_slave_caps callback and DMA_CYCLIC flag correction.
> While debugging the edma to get things sorted out I noticed that the debug was
> too verbose and the important information was hidden even when the we did not
> asked for verbose dmaengine debug.
> I have included some debug cleanups for the edma dmaengine driver also.
I reviewed/tested these patches and they look OK to me. Also the point
of contention was priority which is now dropped from the series. If the
patches look OK and there are no further review comments can they be
queued for -next?
I also have a memcpy and another fix patch for edma so I could queue all
together in my tree and send a consolidated pull request to make it easier.
thanks,
-Joel
On Mon, Apr 14, 2014 at 02:41:55PM +0300, Peter Ujfalusi wrote:
> Hi,
>
> Changes since v2:
> - Dropped patch 10 from v2 (simplify direction configuration...)
> - Dropped the channel priority related patches since we are going to go via
> different route for configuring the priority.
> - Added ACK from Joel for the patches since they are not changed since v2
>
> Changes since v1:
> - ASoC patches removed
> - Comments from Andriy Shevchenko addressed
> - patch added to fix cases when src/dst_maxburst is set to 0
>
> The series contains now only:
> Support for DMA pause/resume in cyclic mode
> device_slave_caps callback and DMA_CYCLIC flag correction.
> While debugging the edma to get things sorted out I noticed that the debug was
> too verbose and the important information was hidden even when the we did not
> asked for verbose dmaengine debug.
> I have included some debug cleanups for the edma dmaengine driver also.
>
Applied all, expect 9th one!
--
~Vinod
> Regards,
> Peter
> ---
> Peter Ujfalusi (10):
> platform_data: edma: Be precise with the paRAM struct
> arm: common: edma: Save the number of event queues/TCs
> dmaengine: edma: Correct the handling of src/dst_maxburst == 0
> dmaengine: edma: Add support for DMA_PAUSE/RESUME operation
> dmaengine: edma: Set DMA_CYCLIC capability flag
> dmaengine: edma: Implement device_slave_caps callback
> dmaengine: edma: Reduce debug print verbosity for non verbose
> debugging
> dmaengine: edma: Prefix debug prints where the text were identical in
> prep callbacks
> dmaengine: edma: Add channel number to debug prints
> dmaengine: edma: Print the direction value as well when it is not
> supported
>
> arch/arm/common/edma.c | 4 ++
> drivers/dma/edma.c | 87 ++++++++++++++++++++++++++++++--------
> include/linux/platform_data/edma.h | 18 ++++----
> 3 files changed, 83 insertions(+), 26 deletions(-)
>
> --
> 1.9.2
>
--
On Mon, Apr 14, 2014 at 02:42:04PM +0300, Peter Ujfalusi wrote:
> It helps to identify issues if we have some information regarding to the
> channel which the event is associated.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> Acked-by: Joel Fernandes <[email protected]>
This failed to apply, cna you pls rebase this and resend...
--
~Vinod