* [v5,10/18] dmaengine: dma-jz4780: Enable Fast DMA to the AIC
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
With the fast DMA bit set, the DMA will transfer twice as much data
per clock period to the AIC, so there is little point not to set it.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
Notes:
v2: No change
v3: No change
v4: No change
v5: No change
drivers/dma/dma-jz4780.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 565971c2a33c..3a4d0a4b550d 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -52,6 +52,7 @@
#define JZ_DMA_DMAC_DMAE BIT(0)
#define JZ_DMA_DMAC_AR BIT(2)
#define JZ_DMA_DMAC_HLT BIT(3)
+#define JZ_DMA_DMAC_FAIC BIT(27)
#define JZ_DMA_DMAC_FMSC BIT(31)
#define JZ_DMA_DRT_AUTO 0x8
@@ -923,8 +924,8 @@ static int jz4780_dma_probe(struct platform_device *pdev)
* Also set the FMSC bit - it increases MSC performance, so it makes
* little sense not to enable it.
*/
- jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC,
- JZ_DMA_DMAC_DMAE | JZ_DMA_DMAC_FMSC);
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC, JZ_DMA_DMAC_DMAE |
+ JZ_DMA_DMAC_FAIC | JZ_DMA_DMAC_FMSC);
if (soc_data->flags & JZ_SOC_DATA_PROGRAMMABLE_DMA)
jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);
^ permalink raw reply related
* [v5,09/18] dmaengine: dma-jz4780: Add support for the JZ4725B SoC
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
The JZ4725B has one DMA core starring six DMA channels.
As for the JZ4770, each DMA channel's clock can be enabled with
a register write, the difference here being that once started, it
is not possible to turn it off.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
Notes:
v2: - Add comments about channel enabling/disabling
- The documentation update is now in patch 01/17
v3: No change
v4: Drop the SoC version ID and use the 'flags' field of the
jz4780_dma_soc_data structure
v5: No change
drivers/dma/dma-jz4780.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 2d194dfa697e..565971c2a33c 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -94,6 +94,7 @@
#define JZ_SOC_DATA_ALLOW_LEGACY_DT BIT(0)
#define JZ_SOC_DATA_PROGRAMMABLE_DMA BIT(1)
#define JZ_SOC_DATA_PER_CHAN_PM BIT(2)
+#define JZ_SOC_DATA_NO_DCKES_DCKEC BIT(3)
/**
* struct jz4780_dma_hwdesc - descriptor structure read by the DMA controller.
@@ -208,14 +209,23 @@ static inline void jz4780_dma_ctrl_writel(struct jz4780_dma_dev *jzdma,
static inline void jz4780_dma_chan_enable(struct jz4780_dma_dev *jzdma,
unsigned int chn)
{
- if (jzdma->soc_data->flags & JZ_SOC_DATA_PER_CHAN_PM)
- jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKES, BIT(chn));
+ if (jzdma->soc_data->flags & JZ_SOC_DATA_PER_CHAN_PM) {
+ unsigned int reg;
+
+ if (jzdma->soc_data->flags & JZ_SOC_DATA_NO_DCKES_DCKEC)
+ reg = JZ_DMA_REG_DCKE;
+ else
+ reg = JZ_DMA_REG_DCKES;
+
+ jz4780_dma_ctrl_writel(jzdma, reg, BIT(chn));
+ }
}
static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma,
unsigned int chn)
{
- if (jzdma->soc_data->flags & JZ_SOC_DATA_PER_CHAN_PM)
+ if ((jzdma->soc_data->flags & JZ_SOC_DATA_PER_CHAN_PM) &&
+ !(jzdma->soc_data->flags & JZ_SOC_DATA_NO_DCKES_DCKEC))
jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn));
}
@@ -978,6 +988,12 @@ static const struct jz4780_dma_soc_data jz4740_dma_soc_data = {
.transfer_ord_max = 5,
};
+static const struct jz4780_dma_soc_data jz4725b_dma_soc_data = {
+ .nb_channels = 6,
+ .transfer_ord_max = 5,
+ .flags = JZ_SOC_DATA_PER_CHAN_PM | JZ_SOC_DATA_NO_DCKES_DCKEC,
+};
+
static const struct jz4780_dma_soc_data jz4770_dma_soc_data = {
.nb_channels = 6,
.transfer_ord_max = 6,
@@ -992,6 +1008,7 @@ static const struct jz4780_dma_soc_data jz4780_dma_soc_data = {
static const struct of_device_id jz4780_dma_dt_match[] = {
{ .compatible = "ingenic,jz4740-dma", .data = &jz4740_dma_soc_data },
+ { .compatible = "ingenic,jz4725b-dma", .data = &jz4725b_dma_soc_data },
{ .compatible = "ingenic,jz4770-dma", .data = &jz4770_dma_soc_data },
{ .compatible = "ingenic,jz4780-dma", .data = &jz4780_dma_soc_data },
{},
^ permalink raw reply related
* [v5,08/18] dmaengine: dma-jz4780: Add support for the JZ4740 SoC
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
The JZ4740 SoC has a single DMA core starring six DMA channels.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
Notes:
v2: The documentation update is now in patch 01/17
v3: The Kconfig update was dropped thanks to patch 06/18
v4: No change
v5: No change
drivers/dma/dma-jz4780.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 184d1a2bf9ba..2d194dfa697e 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -973,6 +973,11 @@ static int jz4780_dma_remove(struct platform_device *pdev)
return 0;
}
+static const struct jz4780_dma_soc_data jz4740_dma_soc_data = {
+ .nb_channels = 6,
+ .transfer_ord_max = 5,
+};
+
static const struct jz4780_dma_soc_data jz4770_dma_soc_data = {
.nb_channels = 6,
.transfer_ord_max = 6,
@@ -986,6 +991,7 @@ static const struct jz4780_dma_soc_data jz4780_dma_soc_data = {
};
static const struct of_device_id jz4780_dma_dt_match[] = {
+ { .compatible = "ingenic,jz4740-dma", .data = &jz4740_dma_soc_data },
{ .compatible = "ingenic,jz4770-dma", .data = &jz4770_dma_soc_data },
{ .compatible = "ingenic,jz4780-dma", .data = &jz4780_dma_soc_data },
{},
^ permalink raw reply related
* [v5,07/18] dmaengine: dma-jz4780: Add support for the JZ4770 SoC
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
The JZ4770 SoC has two DMA cores, each one featuring six DMA channels.
The major change is that each channel's clock can be enabled or disabled
through register writes.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: - Move transfer_ord_max variable to the new jz4780_dma_soc_data
structure
- The documentation update is now in patch 01/17
v3: The Kconfig update was dropped thanks to patch 06/18
v4: Pass jz4780_dma_soc_data structure pointer directly as devicetree
match data; Add a 'flags' field in that structure and macros for
it, to replace checking vs. the SoC version.
v5: No change
drivers/dma/dma-jz4780.c | 61 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 7683de9fb9ee..184d1a2bf9ba 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -29,6 +29,9 @@
#define JZ_DMA_REG_DIRQP 0x04
#define JZ_DMA_REG_DDR 0x08
#define JZ_DMA_REG_DDRS 0x0c
+#define JZ_DMA_REG_DCKE 0x10
+#define JZ_DMA_REG_DCKES 0x14
+#define JZ_DMA_REG_DCKEC 0x18
#define JZ_DMA_REG_DMACP 0x1c
#define JZ_DMA_REG_DSIRQP 0x20
#define JZ_DMA_REG_DSIRQM 0x24
@@ -87,6 +90,11 @@
#define JZ4780_DMA_CTRL_OFFSET 0x1000
+/* macros for use with jz4780_dma_soc_data.flags */
+#define JZ_SOC_DATA_ALLOW_LEGACY_DT BIT(0)
+#define JZ_SOC_DATA_PROGRAMMABLE_DMA BIT(1)
+#define JZ_SOC_DATA_PER_CHAN_PM BIT(2)
+
/**
* struct jz4780_dma_hwdesc - descriptor structure read by the DMA controller.
* @dcm: value for the DCM (channel command) register
@@ -133,6 +141,8 @@ struct jz4780_dma_chan {
struct jz4780_dma_soc_data {
unsigned int nb_channels;
+ unsigned int transfer_ord_max;
+ unsigned long flags;
};
struct jz4780_dma_dev {
@@ -195,6 +205,20 @@ static inline void jz4780_dma_ctrl_writel(struct jz4780_dma_dev *jzdma,
writel(val, jzdma->ctrl_base + reg);
}
+static inline void jz4780_dma_chan_enable(struct jz4780_dma_dev *jzdma,
+ unsigned int chn)
+{
+ if (jzdma->soc_data->flags & JZ_SOC_DATA_PER_CHAN_PM)
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKES, BIT(chn));
+}
+
+static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma,
+ unsigned int chn)
+{
+ if (jzdma->soc_data->flags & JZ_SOC_DATA_PER_CHAN_PM)
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn));
+}
+
static struct jz4780_dma_desc *jz4780_dma_desc_alloc(
struct jz4780_dma_chan *jzchan, unsigned int count,
enum dma_transaction_type type)
@@ -229,8 +253,10 @@ static void jz4780_dma_desc_free(struct virt_dma_desc *vdesc)
kfree(desc);
}
-static uint32_t jz4780_dma_transfer_size(unsigned long val, uint32_t *shift)
+static uint32_t jz4780_dma_transfer_size(struct jz4780_dma_chan *jzchan,
+ unsigned long val, uint32_t *shift)
{
+ struct jz4780_dma_dev *jzdma = jz4780_dma_chan_parent(jzchan);
int ord = ffs(val) - 1;
/*
@@ -242,8 +268,8 @@ static uint32_t jz4780_dma_transfer_size(unsigned long val, uint32_t *shift)
*/
if (ord == 3)
ord = 2;
- else if (ord > 7)
- ord = 7;
+ else if (ord > jzdma->soc_data->transfer_ord_max)
+ ord = jzdma->soc_data->transfer_ord_max;
*shift = ord;
@@ -295,7 +321,7 @@ static int jz4780_dma_setup_hwdesc(struct jz4780_dma_chan *jzchan,
* divisible by the transfer size, and we must not use more than the
* maximum burst specified by the user.
*/
- tsz = jz4780_dma_transfer_size(addr | len | (width * maxburst),
+ tsz = jz4780_dma_transfer_size(jzchan, addr | len | (width * maxburst),
&jzchan->transfer_shift);
switch (width) {
@@ -424,7 +450,7 @@ static struct dma_async_tx_descriptor *jz4780_dma_prep_dma_memcpy(
if (!desc)
return NULL;
- tsz = jz4780_dma_transfer_size(dest | src | len,
+ tsz = jz4780_dma_transfer_size(jzchan, dest | src | len,
&jzchan->transfer_shift);
jzchan->transfer_type = JZ_DMA_DRT_AUTO;
@@ -485,6 +511,9 @@ static void jz4780_dma_begin(struct jz4780_dma_chan *jzchan)
(jzchan->curr_hwdesc + 1) % jzchan->desc->count;
}
+ /* Enable the channel's clock. */
+ jz4780_dma_chan_enable(jzdma, jzchan->id);
+
/* Use 4-word descriptors. */
jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS, 0);
@@ -532,6 +561,8 @@ static int jz4780_dma_terminate_all(struct dma_chan *chan)
jzchan->desc = NULL;
}
+ jz4780_dma_chan_disable(jzdma, jzchan->id);
+
vchan_get_all_descriptors(&jzchan->vchan, &head);
spin_unlock_irqrestore(&jzchan->vchan.lock, flags);
@@ -543,8 +574,10 @@ static int jz4780_dma_terminate_all(struct dma_chan *chan)
static void jz4780_dma_synchronize(struct dma_chan *chan)
{
struct jz4780_dma_chan *jzchan = to_jz4780_dma_chan(chan);
+ struct jz4780_dma_dev *jzdma = jz4780_dma_chan_parent(jzchan);
vchan_synchronize(&jzchan->vchan);
+ jz4780_dma_chan_disable(jzdma, jzchan->id);
}
static int jz4780_dma_config(struct dma_chan *chan,
@@ -812,13 +845,16 @@ static int jz4780_dma_probe(struct platform_device *pdev)
jzdma->ctrl_base = devm_ioremap_resource(dev, res);
if (IS_ERR(jzdma->ctrl_base))
return PTR_ERR(jzdma->ctrl_base);
- } else {
+ } else if (soc_data->flags & JZ_SOC_DATA_ALLOW_LEGACY_DT) {
/*
* On JZ4780, if the second memory resource was not supplied,
* assume we're using an old devicetree, and calculate the
* offset to the control registers.
*/
jzdma->ctrl_base = jzdma->chn_base + JZ4780_DMA_CTRL_OFFSET;
+ } else {
+ dev_err(dev, "failed to get I/O memory\n");
+ return -EINVAL;
}
ret = platform_get_irq(pdev, 0);
@@ -879,7 +915,9 @@ static int jz4780_dma_probe(struct platform_device *pdev)
*/
jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC,
JZ_DMA_DMAC_DMAE | JZ_DMA_DMAC_FMSC);
- jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);
+
+ if (soc_data->flags & JZ_SOC_DATA_PROGRAMMABLE_DMA)
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);
INIT_LIST_HEAD(&dd->channels);
@@ -935,11 +973,20 @@ static int jz4780_dma_remove(struct platform_device *pdev)
return 0;
}
+static const struct jz4780_dma_soc_data jz4770_dma_soc_data = {
+ .nb_channels = 6,
+ .transfer_ord_max = 6,
+ .flags = JZ_SOC_DATA_PER_CHAN_PM,
+};
+
static const struct jz4780_dma_soc_data jz4780_dma_soc_data = {
.nb_channels = 32,
+ .transfer_ord_max = 7,
+ .flags = JZ_SOC_DATA_ALLOW_LEGACY_DT | JZ_SOC_DATA_PROGRAMMABLE_DMA,
};
static const struct of_device_id jz4780_dma_dt_match[] = {
+ { .compatible = "ingenic,jz4770-dma", .data = &jz4770_dma_soc_data },
{ .compatible = "ingenic,jz4780-dma", .data = &jz4780_dma_soc_data },
{},
};
^ permalink raw reply related
* [v5,06/18] dmaengine: dma-jz4780: Don't depend on MACH_JZ4780
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
If we make this driver depend on MACH_JZ4780, that means it can be
enabled only if we're building a kernel specially crafted for a
JZ4780-based board, while most GNU/Linux distributions will want one
generic MIPS kernel that works on multiple boards.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
Notes:
v3: New patch
v4: No change
v5: No change
drivers/dma/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index dacf3f42426d..a4f95574eb9a 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -143,7 +143,7 @@ config DMA_JZ4740
config DMA_JZ4780
tristate "JZ4780 DMA support"
- depends on MACH_JZ4780 || COMPILE_TEST
+ depends on MIPS || COMPILE_TEST
select DMA_ENGINE
select DMA_VIRTUAL_CHANNELS
help
^ permalink raw reply related
* [v5,05/18] dmaengine: dma-jz4780: Use 4-word descriptors
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
The only information we use in the 8-word version of the hardware DMA
descriptor that is not present in the 4-word version is the transfer
type, aka. the ID of the source or recipient device.
Since the transfer type will never change for a DMA channel in use,
we can just set it once for all in the corresponding DMA register
before starting any transfer.
This has several benefits:
* the driver will handle twice as many hardware DMA descriptors;
* the driver is closer to support the JZ4740, which only supports 4-word
hardware DMA descriptors;
* the JZ4770 SoC needs the transfer type to be set in the corresponding
DMA register anyway, even if 8-word descriptors are in use.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
Notes:
v2: No change
v3: No change
v4: No change
v5: No change
drivers/dma/dma-jz4780.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index adf4d5efced6..7683de9fb9ee 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -95,17 +95,12 @@
* @dtc: transfer count (number of blocks of the transfer size specified in DCM
* to transfer) in the low 24 bits, offset of the next descriptor from the
* descriptor base address in the upper 8 bits.
- * @sd: target/source stride difference (in stride transfer mode).
- * @drt: request type
*/
struct jz4780_dma_hwdesc {
uint32_t dcm;
uint32_t dsa;
uint32_t dta;
uint32_t dtc;
- uint32_t sd;
- uint32_t drt;
- uint32_t reserved[2];
};
/* Size of allocations for hardware descriptor blocks. */
@@ -281,7 +276,6 @@ static int jz4780_dma_setup_hwdesc(struct jz4780_dma_chan *jzchan,
desc->dcm = JZ_DMA_DCM_SAI;
desc->dsa = addr;
desc->dta = config->dst_addr;
- desc->drt = jzchan->transfer_type;
width = config->dst_addr_width;
maxburst = config->dst_maxburst;
@@ -289,7 +283,6 @@ static int jz4780_dma_setup_hwdesc(struct jz4780_dma_chan *jzchan,
desc->dcm = JZ_DMA_DCM_DAI;
desc->dsa = config->src_addr;
desc->dta = addr;
- desc->drt = jzchan->transfer_type;
width = config->src_addr_width;
maxburst = config->src_maxburst;
@@ -434,9 +427,10 @@ static struct dma_async_tx_descriptor *jz4780_dma_prep_dma_memcpy(
tsz = jz4780_dma_transfer_size(dest | src | len,
&jzchan->transfer_shift);
+ jzchan->transfer_type = JZ_DMA_DRT_AUTO;
+
desc->desc[0].dsa = src;
desc->desc[0].dta = dest;
- desc->desc[0].drt = JZ_DMA_DRT_AUTO;
desc->desc[0].dcm = JZ_DMA_DCM_TIE | JZ_DMA_DCM_SAI | JZ_DMA_DCM_DAI |
tsz << JZ_DMA_DCM_TSZ_SHIFT |
JZ_DMA_WIDTH_32_BIT << JZ_DMA_DCM_SP_SHIFT |
@@ -491,9 +485,12 @@ static void jz4780_dma_begin(struct jz4780_dma_chan *jzchan)
(jzchan->curr_hwdesc + 1) % jzchan->desc->count;
}
- /* Use 8-word descriptors. */
- jz4780_dma_chn_writel(jzdma, jzchan->id,
- JZ_DMA_REG_DCS, JZ_DMA_DCS_DES8);
+ /* Use 4-word descriptors. */
+ jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS, 0);
+
+ /* Set transfer type. */
+ jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DRT,
+ jzchan->transfer_type);
/* Write descriptor address and initiate descriptor fetch. */
desc_phys = jzchan->desc->desc_phys +
@@ -503,7 +500,7 @@ static void jz4780_dma_begin(struct jz4780_dma_chan *jzchan)
/* Enable the channel. */
jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS,
- JZ_DMA_DCS_DES8 | JZ_DMA_DCS_CTE);
+ JZ_DMA_DCS_CTE);
}
static void jz4780_dma_issue_pending(struct dma_chan *chan)
^ permalink raw reply related
* [v5,04/18] dmaengine: dma-jz4780: Separate chan/ctrl registers
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
The register area of the JZ4780 DMA core can be split into different
sections for different purposes:
* one set of registers is used to perform actions at the DMA core level,
that will generally affect all channels;
* one set of registers per DMA channel, to perform actions at the DMA
channel level, that will only affect the channel in question.
The problem rises when trying to support new versions of the JZ47xx
Ingenic SoC. For instance, the JZ4770 has two DMA cores, each one
with six DMA channels, and the register sets are interleaved:
<DMA0 chan regs> <DMA1 chan regs> <DMA0 ctrl regs> <DMA1 ctrl regs>
By using one memory resource for the channel-specific registers and
one memory resource for the core-specific registers, we can support
the JZ4770, by initializing the driver once per DMA core with different
addresses.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: - Add a fallback mechanism for JZ4780 if the second memory resource
was not supplied in the devicetree.
- The documentation update was moved to patch 01/17
v3: No change
v4: No change
v5: No change
drivers/dma/dma-jz4780.c | 112 ++++++++++++++++++++++++++++++-----------------
1 file changed, 71 insertions(+), 41 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 9ec22cf15a33..adf4d5efced6 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -25,26 +25,26 @@
#include "virt-dma.h"
/* Global registers. */
-#define JZ_DMA_REG_DMAC 0x1000
-#define JZ_DMA_REG_DIRQP 0x1004
-#define JZ_DMA_REG_DDR 0x1008
-#define JZ_DMA_REG_DDRS 0x100c
-#define JZ_DMA_REG_DMACP 0x101c
-#define JZ_DMA_REG_DSIRQP 0x1020
-#define JZ_DMA_REG_DSIRQM 0x1024
-#define JZ_DMA_REG_DCIRQP 0x1028
-#define JZ_DMA_REG_DCIRQM 0x102c
+#define JZ_DMA_REG_DMAC 0x00
+#define JZ_DMA_REG_DIRQP 0x04
+#define JZ_DMA_REG_DDR 0x08
+#define JZ_DMA_REG_DDRS 0x0c
+#define JZ_DMA_REG_DMACP 0x1c
+#define JZ_DMA_REG_DSIRQP 0x20
+#define JZ_DMA_REG_DSIRQM 0x24
+#define JZ_DMA_REG_DCIRQP 0x28
+#define JZ_DMA_REG_DCIRQM 0x2c
/* Per-channel registers. */
#define JZ_DMA_REG_CHAN(n) (n * 0x20)
-#define JZ_DMA_REG_DSA(n) (0x00 + JZ_DMA_REG_CHAN(n))
-#define JZ_DMA_REG_DTA(n) (0x04 + JZ_DMA_REG_CHAN(n))
-#define JZ_DMA_REG_DTC(n) (0x08 + JZ_DMA_REG_CHAN(n))
-#define JZ_DMA_REG_DRT(n) (0x0c + JZ_DMA_REG_CHAN(n))
-#define JZ_DMA_REG_DCS(n) (0x10 + JZ_DMA_REG_CHAN(n))
-#define JZ_DMA_REG_DCM(n) (0x14 + JZ_DMA_REG_CHAN(n))
-#define JZ_DMA_REG_DDA(n) (0x18 + JZ_DMA_REG_CHAN(n))
-#define JZ_DMA_REG_DSD(n) (0x1c + JZ_DMA_REG_CHAN(n))
+#define JZ_DMA_REG_DSA 0x00
+#define JZ_DMA_REG_DTA 0x04
+#define JZ_DMA_REG_DTC 0x08
+#define JZ_DMA_REG_DRT 0x0c
+#define JZ_DMA_REG_DCS 0x10
+#define JZ_DMA_REG_DCM 0x14
+#define JZ_DMA_REG_DDA 0x18
+#define JZ_DMA_REG_DSD 0x1c
#define JZ_DMA_DMAC_DMAE BIT(0)
#define JZ_DMA_DMAC_AR BIT(2)
@@ -85,6 +85,8 @@
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES))
+#define JZ4780_DMA_CTRL_OFFSET 0x1000
+
/**
* struct jz4780_dma_hwdesc - descriptor structure read by the DMA controller.
* @dcm: value for the DCM (channel command) register
@@ -140,7 +142,8 @@ struct jz4780_dma_soc_data {
struct jz4780_dma_dev {
struct dma_device dma_device;
- void __iomem *base;
+ void __iomem *chn_base;
+ void __iomem *ctrl_base;
struct clk *clk;
unsigned int irq;
const struct jz4780_dma_soc_data *soc_data;
@@ -173,16 +176,28 @@ static inline struct jz4780_dma_dev *jz4780_dma_chan_parent(
dma_device);
}
-static inline uint32_t jz4780_dma_readl(struct jz4780_dma_dev *jzdma,
+static inline uint32_t jz4780_dma_chn_readl(struct jz4780_dma_dev *jzdma,
+ unsigned int chn, unsigned int reg)
+{
+ return readl(jzdma->chn_base + reg + JZ_DMA_REG_CHAN(chn));
+}
+
+static inline void jz4780_dma_chn_writel(struct jz4780_dma_dev *jzdma,
+ unsigned int chn, unsigned int reg, uint32_t val)
+{
+ writel(val, jzdma->chn_base + reg + JZ_DMA_REG_CHAN(chn));
+}
+
+static inline uint32_t jz4780_dma_ctrl_readl(struct jz4780_dma_dev *jzdma,
unsigned int reg)
{
- return readl(jzdma->base + reg);
+ return readl(jzdma->ctrl_base + reg);
}
-static inline void jz4780_dma_writel(struct jz4780_dma_dev *jzdma,
+static inline void jz4780_dma_ctrl_writel(struct jz4780_dma_dev *jzdma,
unsigned int reg, uint32_t val)
{
- writel(val, jzdma->base + reg);
+ writel(val, jzdma->ctrl_base + reg);
}
static struct jz4780_dma_desc *jz4780_dma_desc_alloc(
@@ -477,17 +492,18 @@ static void jz4780_dma_begin(struct jz4780_dma_chan *jzchan)
}
/* Use 8-word descriptors. */
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id), JZ_DMA_DCS_DES8);
+ jz4780_dma_chn_writel(jzdma, jzchan->id,
+ JZ_DMA_REG_DCS, JZ_DMA_DCS_DES8);
/* Write descriptor address and initiate descriptor fetch. */
desc_phys = jzchan->desc->desc_phys +
(jzchan->curr_hwdesc * sizeof(*jzchan->desc->desc));
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DDA(jzchan->id), desc_phys);
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DDRS, BIT(jzchan->id));
+ jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DDA, desc_phys);
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DDRS, BIT(jzchan->id));
/* Enable the channel. */
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id),
- JZ_DMA_DCS_DES8 | JZ_DMA_DCS_CTE);
+ jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS,
+ JZ_DMA_DCS_DES8 | JZ_DMA_DCS_CTE);
}
static void jz4780_dma_issue_pending(struct dma_chan *chan)
@@ -513,7 +529,7 @@ static int jz4780_dma_terminate_all(struct dma_chan *chan)
spin_lock_irqsave(&jzchan->vchan.lock, flags);
/* Clear the DMA status and stop the transfer. */
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id), 0);
+ jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS, 0);
if (jzchan->desc) {
vchan_terminate_vdesc(&jzchan->desc->vdesc);
jzchan->desc = NULL;
@@ -562,8 +578,8 @@ static size_t jz4780_dma_desc_residue(struct jz4780_dma_chan *jzchan,
residue += desc->desc[i].dtc << jzchan->transfer_shift;
if (next_sg != 0) {
- count = jz4780_dma_readl(jzdma,
- JZ_DMA_REG_DTC(jzchan->id));
+ count = jz4780_dma_chn_readl(jzdma, jzchan->id,
+ JZ_DMA_REG_DTC);
residue += count << jzchan->transfer_shift;
}
@@ -610,8 +626,8 @@ static void jz4780_dma_chan_irq(struct jz4780_dma_dev *jzdma,
spin_lock(&jzchan->vchan.lock);
- dcs = jz4780_dma_readl(jzdma, JZ_DMA_REG_DCS(jzchan->id));
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id), 0);
+ dcs = jz4780_dma_chn_readl(jzdma, jzchan->id, JZ_DMA_REG_DCS);
+ jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS, 0);
if (dcs & JZ_DMA_DCS_AR) {
dev_warn(&jzchan->vchan.chan.dev->device,
@@ -650,7 +666,7 @@ static irqreturn_t jz4780_dma_irq_handler(int irq, void *data)
uint32_t pending, dmac;
int i;
- pending = jz4780_dma_readl(jzdma, JZ_DMA_REG_DIRQP);
+ pending = jz4780_dma_ctrl_readl(jzdma, JZ_DMA_REG_DIRQP);
for (i = 0; i < jzdma->soc_data->nb_channels; i++) {
if (!(pending & (1<<i)))
@@ -660,12 +676,12 @@ static irqreturn_t jz4780_dma_irq_handler(int irq, void *data)
}
/* Clear halt and address error status of all channels. */
- dmac = jz4780_dma_readl(jzdma, JZ_DMA_REG_DMAC);
+ dmac = jz4780_dma_ctrl_readl(jzdma, JZ_DMA_REG_DMAC);
dmac &= ~(JZ_DMA_DMAC_HLT | JZ_DMA_DMAC_AR);
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DMAC, dmac);
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC, dmac);
/* Clear interrupt pending status. */
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DIRQP, 0);
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DIRQP, 0);
return IRQ_HANDLED;
}
@@ -790,9 +806,23 @@ static int jz4780_dma_probe(struct platform_device *pdev)
return -EINVAL;
}
- jzdma->base = devm_ioremap_resource(dev, res);
- if (IS_ERR(jzdma->base))
- return PTR_ERR(jzdma->base);
+ jzdma->chn_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(jzdma->chn_base))
+ return PTR_ERR(jzdma->chn_base);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (res) {
+ jzdma->ctrl_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(jzdma->ctrl_base))
+ return PTR_ERR(jzdma->ctrl_base);
+ } else {
+ /*
+ * On JZ4780, if the second memory resource was not supplied,
+ * assume we're using an old devicetree, and calculate the
+ * offset to the control registers.
+ */
+ jzdma->ctrl_base = jzdma->chn_base + JZ4780_DMA_CTRL_OFFSET;
+ }
ret = platform_get_irq(pdev, 0);
if (ret < 0) {
@@ -850,9 +880,9 @@ static int jz4780_dma_probe(struct platform_device *pdev)
* Also set the FMSC bit - it increases MSC performance, so it makes
* little sense not to enable it.
*/
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DMAC,
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC,
JZ_DMA_DMAC_DMAE | JZ_DMA_DMAC_FMSC);
- jz4780_dma_writel(jzdma, JZ_DMA_REG_DMACP, 0);
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);
INIT_LIST_HEAD(&dd->channels);
^ permalink raw reply related
* [v5,03/18] dmaengine: dma-jz4780: Avoid hardcoding number of channels
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
As part of the work to support various other Ingenic JZ47xx SoC versions,
which don't feature the same number of DMA channels per core, we now
deduce the number of DMA channels available from the devicetree
compatible string.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: - don't hardcode jz_version to ID_JZ4780 when not probed from DT,
because it cannot happen
- Put SoC-specific data into a jz4780_dma_soc_data structure
v3: No change
v4: Remove jz_version; pass the jz4780_dma_soc_data structure pointer
as devicetree match data, and use device_get_match_data().
v5: No change
drivers/dma/dma-jz4780.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 987899610b46..9ec22cf15a33 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -16,6 +16,7 @@
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/of_dma.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -23,8 +24,6 @@
#include "dmaengine.h"
#include "virt-dma.h"
-#define JZ_DMA_NR_CHANNELS 32
-
/* Global registers. */
#define JZ_DMA_REG_DMAC 0x1000
#define JZ_DMA_REG_DIRQP 0x1004
@@ -135,14 +134,19 @@ struct jz4780_dma_chan {
unsigned int curr_hwdesc;
};
+struct jz4780_dma_soc_data {
+ unsigned int nb_channels;
+};
+
struct jz4780_dma_dev {
struct dma_device dma_device;
void __iomem *base;
struct clk *clk;
unsigned int irq;
+ const struct jz4780_dma_soc_data *soc_data;
uint32_t chan_reserved;
- struct jz4780_dma_chan chan[JZ_DMA_NR_CHANNELS];
+ struct jz4780_dma_chan chan[];
};
struct jz4780_dma_filter_data {
@@ -648,7 +652,7 @@ static irqreturn_t jz4780_dma_irq_handler(int irq, void *data)
pending = jz4780_dma_readl(jzdma, JZ_DMA_REG_DIRQP);
- for (i = 0; i < JZ_DMA_NR_CHANNELS; i++) {
+ for (i = 0; i < jzdma->soc_data->nb_channels; i++) {
if (!(pending & (1<<i)))
continue;
@@ -728,7 +732,7 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
data.channel = dma_spec->args[1];
if (data.channel > -1) {
- if (data.channel >= JZ_DMA_NR_CHANNELS) {
+ if (data.channel >= jzdma->soc_data->nb_channels) {
dev_err(jzdma->dma_device.dev,
"device requested non-existent channel %u\n",
data.channel);
@@ -755,6 +759,7 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
static int jz4780_dma_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ const struct jz4780_dma_soc_data *soc_data;
struct jz4780_dma_dev *jzdma;
struct jz4780_dma_chan *jzchan;
struct dma_device *dd;
@@ -766,10 +771,17 @@ static int jz4780_dma_probe(struct platform_device *pdev)
return -EINVAL;
}
- jzdma = devm_kzalloc(dev, sizeof(*jzdma), GFP_KERNEL);
+ soc_data = device_get_match_data(dev);
+ if (!soc_data)
+ return -EINVAL;
+
+ jzdma = devm_kzalloc(dev, sizeof(*jzdma)
+ + sizeof(*jzdma->chan) * soc_data->nb_channels,
+ GFP_KERNEL);
if (!jzdma)
return -ENOMEM;
+ jzdma->soc_data = soc_data;
platform_set_drvdata(pdev, jzdma);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -844,7 +856,7 @@ static int jz4780_dma_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&dd->channels);
- for (i = 0; i < JZ_DMA_NR_CHANNELS; i++) {
+ for (i = 0; i < soc_data->nb_channels; i++) {
jzchan = &jzdma->chan[i];
jzchan->id = i;
@@ -889,15 +901,19 @@ static int jz4780_dma_remove(struct platform_device *pdev)
free_irq(jzdma->irq, jzdma);
- for (i = 0; i < JZ_DMA_NR_CHANNELS; i++)
+ for (i = 0; i < jzdma->soc_data->nb_channels; i++)
tasklet_kill(&jzdma->chan[i].vchan.task);
dma_async_device_unregister(&jzdma->dma_device);
return 0;
}
+static const struct jz4780_dma_soc_data jz4780_dma_soc_data = {
+ .nb_channels = 32,
+};
+
static const struct of_device_id jz4780_dma_dt_match[] = {
- { .compatible = "ingenic,jz4780-dma", .data = NULL },
+ { .compatible = "ingenic,jz4780-dma", .data = &jz4780_dma_soc_data },
{},
};
MODULE_DEVICE_TABLE(of, jz4780_dma_dt_match);
^ permalink raw reply related
* [v5,02/18] dmaengine: dma-jz4780: Return error if not probed from DT
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
The driver calls clk_get() with the clock name set to NULL, which means
that the driver could only work when probed from devicetree. From now
on, we explicitly require the driver to be probed from devicetree.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: New patch
v3: No change
v4: No change
v5: No change
drivers/dma/dma-jz4780.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 85820a2d69d4..987899610b46 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -761,6 +761,11 @@ static int jz4780_dma_probe(struct platform_device *pdev)
struct resource *res;
int i, ret;
+ if (!dev->of_node) {
+ dev_err(dev, "This driver must be probed from devicetree\n");
+ return -EINVAL;
+ }
+
jzdma = devm_kzalloc(dev, sizeof(*jzdma), GFP_KERNEL);
if (!jzdma)
return -ENOMEM;
^ permalink raw reply related
* [v5,01/18] dt-bindings: jz4780-dma: Update bindings to reflect driver changes
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
The driver now expects the devicetree to supply a second memory
resource. This resource is mandatory on the newly supported SoCs.
For the JZ4780, new devicetree code must also provide it, although the
driver is still compatible with older devicetree binaries.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Notes:
v2: New patch in this series; regroups the changes made to the
jz4780-dma.txt doc file in the previous version of the patchset.
v3: Updated example to comply with devicetree specification
v4: Removed driver-specific comment in commit message
v5: Remove 'doc' in patch title
Documentation/devicetree/bindings/dma/jz4780-dma.txt | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/jz4780-dma.txt b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
index 03e9cf7b42e0..636fcb26b164 100644
--- a/Documentation/devicetree/bindings/dma/jz4780-dma.txt
+++ b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
@@ -2,8 +2,13 @@
Required properties:
-- compatible: Should be "ingenic,jz4780-dma"
-- reg: Should contain the DMA controller registers location and length.
+- compatible: Should be one of:
+ * ingenic,jz4740-dma
+ * ingenic,jz4725b-dma
+ * ingenic,jz4770-dma
+ * ingenic,jz4780-dma
+- reg: Should contain the DMA channel registers location and length, followed
+ by the DMA controller registers location and length.
- interrupts: Should contain the interrupt specifier of the DMA controller.
- clocks: Should contain a clock specifier for the JZ4780 PDMA clock.
- #dma-cells: Must be <2>. Number of integer cells in the dmas property of
@@ -19,9 +24,10 @@ Optional properties:
Example:
-dma: dma@13420000 {
+dma: dma-controller@13420000 {
compatible = "ingenic,jz4780-dma";
- reg = <0x13420000 0x10000>;
+ reg = <0x13420000 0x400
+ 0x13421000 0x40>;
interrupt-parent = <&intc>;
interrupts = <10>;
^ permalink raw reply related
* [3/3] dmaengine: xilinx_dma: Fix 64-bit simple CDMA transfer
From: Radhey Shyam Pandey @ 2018-08-29 17:05 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams@intel.com, Michal Simek,
Appana Durga Kedareswara Rao, lars@metafoo.de,
dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
> -----Original Message-----
> From: Vinod <vkoul@kernel.org>
> Sent: Wednesday, August 29, 2018 9:31 AM
> To: Radhey Shyam Pandey <radheys@xilinx.com>
> Cc: dan.j.williams@intel.com; Michal Simek <michals@xilinx.com>; Appana
> Durga Kedareswara Rao <appanad@xilinx.com>; lars@metafoo.de;
> dmaengine@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 3/3] dmaengine: xilinx_dma: Fix 64-bit simple CDMA
> transfer
>
> On 28-08-18, 14:03, Radhey Shyam Pandey wrote:
> > > On 27-07-18, 16:20, Radhey Shyam Pandey wrote:
> > > > In AXI CDMA simple mode also pass MSB bits of source and destination
> > > > address to xilinx_write function. This fixes simple CDMA operation
> > > > mode using 64-bit addressing.
> > > >
> > > > Signed-off-by: Radhey Shyam Pandey
> <radhey.shyam.pandey@xilinx.com>
> > > > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > > > ---
> > > > drivers/dma/xilinx/xilinx_dma.c | 6 ++++--
> > > > 1 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/dma/xilinx/xilinx_dma.c
> b/drivers/dma/xilinx/xilinx_dma.c
> > > > index a37871e..2e15d86 100644
> > > > --- a/drivers/dma/xilinx/xilinx_dma.c
> > > > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > > > @@ -1245,8 +1245,10 @@ static void xilinx_cdma_start_transfer(struct
> > > xilinx_dma_chan *chan)
> > > >
> > > > hw = &segment->hw;
> > > >
> > > > - xilinx_write(chan, XILINX_CDMA_REG_SRCADDR, hw-
> > > >src_addr);
> > > > - xilinx_write(chan, XILINX_CDMA_REG_DSTADDR, hw-
> > > >dest_addr);
> > > > + xilinx_write(chan, XILINX_CDMA_REG_SRCADDR,
> > > (dma_addr_t)
> > > > + ((u64)hw->src_addr_msb << 32 | hw->src_addr));
> > >
> > > so this is:
> > > (dma_addr_t)((u64)hw->src_addr_msb << 32 | hw->src_addr)
> > >
> > > what is src_addr data type? I think its u32. It would be better to
> > > update xilinx_write() to take u64 and not dma_addr_t.
> >
> > Yes, src_addr_msb and src_addr BD fields are u32. To explain: There is no
> > prob in xilinx_write it takes dma_addr_t as an arg which is 32/64 bit
> > depending on _DMA_ADDR_T_64BIT. In 64bit CDMA transfer, there was a
> bug
> > i.e in the call to xilinx_write src_addr_msb 32 bits were not passed. To fix
> > that combine MSB and LSB 32 bits before passing it to xilinx_write.
>
> Yeah that part was clear but the implementation can be better..
Thanks! Separate fields for source address are needed due to CDMA BD structure.
Please suggest if it doesn't look ok.
>
> --
> ~Vinod
^ permalink raw reply
* [1/2] ACPI / LPSS: Add alternative ACPI HIDs for Cherry Trail DMA controllers
From: Andy Shevchenko @ 2018-08-29 16:50 UTC (permalink / raw)
To: Hans de Goede
Cc: Rafael J . Wysocki, Len Brown, Viresh Kumar, Vinod Koul,
linux-acpi, Dan Williams, dmaengine
On Mon, Aug 27, 2018 at 09:45:44AM +0200, Hans de Goede wrote:
> Bay and Cherry Trail DSTDs represent a different set of devices depending
> on which OS the device think it is booting. One set of decices for Windows
> and another set of devices for Android which targets the Android-x86 Linux
> kernel fork (which e.g. used to have its own display driver instead of
> using the i915 driver).
>
> Which set of devices we are actually going to get is out of our control,
> this is controlled by the ACPI OSID variable, which gets either set through
> an EFI setup option, or sometimes is autodetected. So we need to support
> both.
>
> This commit adds support for the 80862286 and 808622C0 ACPI HIDs which we
> get for the first resp. second DMA controller on Cherry Trail devices when
> OSID is set to Android.
>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/acpi/acpi_lpss.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index 0569972b02e1..95d9c45e13cc 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -327,9 +327,11 @@ static const struct acpi_device_id acpi_lpss_device_ids[] = {
> { "INT33FC", },
>
> /* Braswell LPSS devices */
> + { "80862286", LPSS_ADDR(lpss_dma_desc) },
> { "80862288", LPSS_ADDR(bsw_pwm_dev_desc) },
> { "8086228A", LPSS_ADDR(bsw_uart_dev_desc) },
> { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) },
> + { "808622C0", LPSS_ADDR(lpss_dma_desc) },
> { "808622C1", LPSS_ADDR(bsw_i2c_dev_desc) },
>
> /* Broadwell LPSS devices */
> --
> 2.18.0
>
^ permalink raw reply
* [2/2] dmaengine: dw: Add alternative ACPI HIDs for Cherry Trail DMA controllers
From: Andy Shevchenko @ 2018-08-29 16:50 UTC (permalink / raw)
To: Hans de Goede
Cc: Rafael J . Wysocki, Len Brown, Viresh Kumar, Vinod Koul,
linux-acpi, Dan Williams, dmaengine
On Mon, Aug 27, 2018 at 09:45:45AM +0200, Hans de Goede wrote:
> Bay and Cherry Trail DSTDs represent a different set of devices depending
> on which OS the device think it is booting. One set of decices for Windows
> and another set of devices for Android which targets the Android-x86 Linux
> kernel fork (which e.g. used to have its own display driver instead of
> using the i915 driver).
>
> Which set of devices we are actually going to get is out of our control,
> this is controlled by the ACPI OSID variable, which gets either set through
> an EFI setup option, or sometimes is autodetected. So we need to support
> both.
>
> This commit adds support for the 80862286 and 808622C0 ACPI HIDs which we
> get for the first resp. second DMA controller on Cherry Trail devices when
> OSID is set to Android.
>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/dma/dw/platform.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> index f62dd0944908..f01b2c173fa6 100644
> --- a/drivers/dma/dw/platform.c
> +++ b/drivers/dma/dw/platform.c
> @@ -284,6 +284,8 @@ MODULE_DEVICE_TABLE(of, dw_dma_of_id_table);
> #ifdef CONFIG_ACPI
> static const struct acpi_device_id dw_dma_acpi_id_table[] = {
> { "INTL9C60", 0 },
> + { "80862286", 0 },
> + { "808622C0", 0 },
> { }
> };
> MODULE_DEVICE_TABLE(acpi, dw_dma_acpi_id_table);
> --
> 2.18.0
>
^ permalink raw reply
* [v2,1/2] dmaengine: use SPDX identifier for Renesas drivers
From: Vinod Koul @ 2018-08-29 16:30 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-renesas-soc, Kuninori Morimoto, Dan Williams, dmaengine,
linux-kernel
On 23-08-18, 15:34, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Applied, thanks
^ permalink raw reply
* [2/2] dmaengine: dw: Add alternative ACPI HIDs for Cherry Trail DMA controllers
From: Vinod Koul @ 2018-08-29 16:25 UTC (permalink / raw)
To: Hans de Goede
Cc: Rafael J . Wysocki, Len Brown, Viresh Kumar, Andy Shevchenko,
linux-acpi, Dan Williams, dmaengine
On 27-08-18, 09:45, Hans de Goede wrote:
> Bay and Cherry Trail DSTDs represent a different set of devices depending
> on which OS the device think it is booting. One set of decices for Windows
> and another set of devices for Android which targets the Android-x86 Linux
> kernel fork (which e.g. used to have its own display driver instead of
> using the i915 driver).
>
> Which set of devices we are actually going to get is out of our control,
> this is controlled by the ACPI OSID variable, which gets either set through
> an EFI setup option, or sometimes is autodetected. So we need to support
> both.
>
> This commit adds support for the 80862286 and 808622C0 ACPI HIDs which we
> get for the first resp. second DMA controller on Cherry Trail devices when
> OSID is set to Android.
Applied, thanks
^ permalink raw reply
* dmaengine: Add metadata_ops for dma_async_tx_descriptor
From: Vinod Koul @ 2018-08-29 16:22 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: dan.j.williams, dmaengine, linux-kernel, lars, radheys
On 29-08-18, 19:14, Peter Ujfalusi wrote:
> Vinod,
>
> On 08/29/2018 06:52 PM, Vinod wrote:
> > On 23-08-18, 16:07, Peter Ujfalusi wrote:
> >> The metadata is best described as side band data or parameters traveling
> >> alongside the data DMAd by the DMA engine. It is data
> >> which is understood by the peripheral and the peripheral driver only, the
> >> DMA engine see it only as data block and it is not interpreting it in any
> >> way.
> >>
> >> The metadata can be different per descriptor as it is a parameter for the
> >> data being transferred.
> >>
> >> If the DMA supports per descriptor metadata it can implement the attach,
> >> get_ptr/set_len callbacks.
> >>
> >> Client drivers must only use either attach or get_ptr/set_len to avoid
> >> miss configuration.
> >
> > misconfiguration?
>
> Sorry for the typos, I'll got through them again.
>
> >> Client driver can check if a given metadata mode is supported by the
> >> channel during probe time with
> >> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_CLIENT);
> >> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_ENGINE);
> >>
> >> and based on this information can use either mode.
> >>
> >> Wrappers are also added for the metadata_ops.
> >>
> >> To be used in DESC_METADATA_CLIENT mode:
> >> dmaengine_desc_attach_metadata()
> >>
> >> To be used in DESC_METADATA_ENGINE mode:
> >> dmaengine_desc_get_metadata_ptr()
> >> dmaengine_desc_set_metadata_len()
> >>
> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> >> ---
> >> Hi,
> >>
> >> Changes since rfc:
> >> - DESC_METADATA_EMBEDDED renamed to DESC_METADATA_ENGINE
> >> - Use flow is added for both CLIENT and ENGINE metadata modes
> >>
> >> Regards,
> >> Peter
> >>
> >> include/linux/dmaengine.h | 144 ++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 144 insertions(+)
> >>
> >> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> >> index 3db833a8c542..f809635cfeaa 100644
> >> --- a/include/linux/dmaengine.h
> >> +++ b/include/linux/dmaengine.h
> >> @@ -231,6 +231,57 @@ typedef struct { DECLARE_BITMAP(bits, DMA_TX_TYPE_END); } dma_cap_mask_t;
> >> * @bytes_transferred: byte counter
> >> */
> >>
> >> +/**
> >> + * enum dma_desc_metadata_mode - per descriptor metadata mode types supported
> >> + * @DESC_METADATA_CLIENT - the metadata buffer is allocated/provided by the
> >> + * client driver and it is attached (via the dmaengine_desc_attach_metadata()
> >> + * helper) to the descriptor.
> >> + *
> >> + * Client drivers interested to use this mode can follow:
> >> + * - DMA_MEM_TO_DEV:
> >> + * 1. prepare the descriptor (dmaengine_prep_*)
> >> + * construct the metadata in the clinet's buffer
> >
> > typo clinet
> >
> >> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
> >> + * descriptor
> >> + * 3. submit the transfer
> >> + * - DMA_DEV_TO_MEM:
> >> + * 1. prepare the descriptor (dmaengine_prep_*)
> >> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
> >> + * descriptor
> >> + * 3. submit the transfer
> >> + * 4. when the transfer is completed, the metadata should be available in the
> >> + * attached buffer
> >
> > I guess this is good to be moved into Documentation
>
> Should I create a new file for metadata? I guess it would make sense as the
> information is for both clients and engines.
Hmm not sure, lets see how it looks as entries in these files, detailing
roles of clients and providers
>
> >
> > also we dont allow this for memcpy txn?
>
> I have not thought about that, but if I think about it it should be along the
> same lines as MEM_TO_DEV.
> I'll add the MEM_TO_MEM as well to the documentation.
Okay and lets not implement it then..
>
> >> + *
> >> + * @DESC_METADATA_ENGINE - the metadata buffer is allocated/managed by the DMA
> >> + * driver. The client driver can ask for the pointer, maximum size and the
> >> + * currently used size of the metadata and can directly update or read it.
> >> + * dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is
> >> + * provided as helper functions.
> >> + *
> >> + * Client drivers interested to use this mode can follow:
> >> + * - DMA_MEM_TO_DEV:
> >> + * 1. prepare the descriptor (dmaengine_prep_*)
> >> + * 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the engine's
> >> + * metadata area
> >> + * 3. update the metadata at the pointer
> >> + * 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine the amount
> >> + * of data the client has placed into the metadata buffer
> >> + * 5. submit the transfer
> >> + * - DMA_DEV_TO_MEM:
> >> + * 1. prepare the descriptor (dmaengine_prep_*)
> >> + * 2. submit the transfer
> >> + * 3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
> >> + * pointer to the engine's metadata are
> >> + * 4. Read out the metadate from the pointer
> >> + *
> >> + * Note: the two mode is not compatible and clients must use one mode for a
> >> + * descriptor.
> >> + */
> >> +enum dma_desc_metadata_mode {
> >> + DESC_METADATA_CLIENT = (1 << 0),
> >> + DESC_METADATA_ENGINE = (1 << 1),
> >
> > BIT(x)
>
> OK, I followed what we have in the header to not mix (1 << x) and BIT(x)
yeah lets update :)
>
> >
> >> +};
> >> +
> >> struct dma_chan_percpu {
> >> /* stats */
> >> unsigned long memcpy_count;
> >> @@ -494,6 +545,18 @@ struct dmaengine_unmap_data {
> >> dma_addr_t addr[0];
> >> };
> >>
> >> +struct dma_async_tx_descriptor;
> >> +
> >> +struct dma_descriptor_metadata_ops {
> >> + int (*attach)(struct dma_async_tx_descriptor *desc, void *data,
> >> + size_t len);
> >> +
> >> + void *(*get_ptr)(struct dma_async_tx_descriptor *desc,
> >> + size_t *payload_len, size_t *max_len);
> >> + int (*set_len)(struct dma_async_tx_descriptor *desc,
> >> + size_t payload_len);
> >> +};
> >> +
> >> /**
> >> * struct dma_async_tx_descriptor - async transaction descriptor
> >> * ---dma generic offload fields---
> >> @@ -523,6 +586,8 @@ struct dma_async_tx_descriptor {
> >> dma_async_tx_callback_result callback_result;
> >> void *callback_param;
> >> struct dmaengine_unmap_data *unmap;
> >> + enum dma_desc_metadata_mode desc_metadata_mode;
> >> + struct dma_descriptor_metadata_ops *metadata_ops;
>
> I forgot to update the comment section for the dma_async_tx_descriptor, I'll
> address it in v2.
>
> >> #ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
> >> struct dma_async_tx_descriptor *next;
> >> struct dma_async_tx_descriptor *parent;
> >> @@ -685,6 +750,7 @@ struct dma_filter {
> >> * @global_node: list_head for global dma_device_list
> >> * @filter: information for device/slave to filter function/param mapping
> >> * @cap_mask: one or more dma_capability flags
> >> + * @desc_metadata_modes: supported metadata modes by the DMA device
> >> * @max_xor: maximum number of xor sources, 0 if no capability
> >> * @max_pq: maximum number of PQ sources and PQ-continue capability
> >> * @copy_align: alignment shift for memcpy operations
> >> @@ -749,6 +815,7 @@ struct dma_device {
> >> struct list_head global_node;
> >> struct dma_filter filter;
> >> dma_cap_mask_t cap_mask;
> >> + enum dma_desc_metadata_mode desc_metadata_modes;
> >> unsigned short max_xor;
> >> unsigned short max_pq;
> >> enum dmaengine_alignment copy_align;
> >> @@ -935,6 +1002,83 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memcpy(
> >> len, flags);
> >> }
> >>
> >> +static inline bool dmaengine_is_metadata_mode_supported(struct dma_chan *chan,
> >> + enum dma_desc_metadata_mode mode)
> >> +{
> >> + return !!(chan->device->desc_metadata_modes & mode);
> >> +}
> >> +
> >> +static inline int _desc_check_and_set_metadata_mode(
> >
> > why does this need to start with _ ?
>
> To scare people to use in client code ;)
Lets not expose to them :D
>
> >
> >> + struct dma_async_tx_descriptor *desc, enum dma_desc_metadata_mode mode)
> >> +{
> >> + /* Make sure that the metadata mode is not mixed */
> >> + if (!desc->desc_metadata_mode) {
> >> + if (dmaengine_is_metadata_mode_supported(desc->chan, mode))
> >> + desc->desc_metadata_mode = mode;
> >> + else
> >> + return -ENOTSUPP;
> >> + } else if (desc->desc_metadata_mode != mode) {
> >> + return -EINVAL;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static inline int dmaengine_desc_attach_metadata(
> >> + struct dma_async_tx_descriptor *desc, void *data, size_t len)
> >> +{
> >> + int ret;
> >> +
> >> + if (!desc)
> >> + return -EINVAL;
> >> +
> >> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_CLIENT);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (!desc->metadata_ops || !desc->metadata_ops->attach)
> >> + return -ENOTSUPP;
> >> +
> >> + return desc->metadata_ops->attach(desc, data, len);
> >> +}
> >> +
> >> +static inline void *dmaengine_desc_get_metadata_ptr(
> >> + struct dma_async_tx_descriptor *desc, size_t *payload_len,
> >> + size_t *max_len)
> >> +{
> >> + int ret;
> >> +
> >> + if (!desc)
> >> + return ERR_PTR(-EINVAL);
> >> +
> >> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
> >> + if (ret)
> >> + return ERR_PTR(ret);
> >> +
> >> + if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
> >> + return ERR_PTR(-ENOTSUPP);
> >> +
> >> + return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
> >> +}
> >> +
> >> +static inline int dmaengine_desc_set_metadata_len(
> >> + struct dma_async_tx_descriptor *desc, size_t payload_len)
> >> +{
> >> + int ret;
> >> +
> >> + if (!desc)
> >> + return -EINVAL;
> >> +
> >> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (!desc->metadata_ops || !desc->metadata_ops->set_len)
> >> + return -ENOTSUPP;
> >> +
> >> + return desc->metadata_ops->set_len(desc, payload_len);
> >> +}
> >
> > thats bit too much code for a header file :( Lets move it to C file
> > please. We can utilize local dmaengine.h and not expose all these and
> > possible misuse by clients
>
> OK, I have thought about that as well.
>
> > Also I would like to see a use :-) before further comments.
>
> You mean the DMA driver and at least one client?
DMA driver to _at_least_ start with. Client even better
> I have the DMA driver in my public facing branch [1], but it is not an easy
> read with it's close to 4k loc.
It doesnt exist :P
> The client is not in my branch and it is actually using an older version of
> the metadata support.
>
> The problem is that I don't know when I will be able to send the driver for
> review as all of this is targeting a brand new SoC (AM654) with completely new
> data movement architecture. There are lots of dependencies still need to be
> upstreamed before I can send something which at least compiles.
>
> I can offer snippets from the client driver, if that is good enough or a link
> to the public tree where it can be accessed, but it is not going to go
> upstream before the DMA driver.
TBH that's not going to help much, lets come back to it when you need
this upstream.
^ permalink raw reply
* dmaengine: Add metadata_ops for dma_async_tx_descriptor
From: Peter Ujfalusi @ 2018-08-29 16:14 UTC (permalink / raw)
To: Vinod; +Cc: dan.j.williams, dmaengine, linux-kernel, lars, radheys
Vinod,
On 08/29/2018 06:52 PM, Vinod wrote:
> On 23-08-18, 16:07, Peter Ujfalusi wrote:
>> The metadata is best described as side band data or parameters traveling
>> alongside the data DMAd by the DMA engine. It is data
>> which is understood by the peripheral and the peripheral driver only, the
>> DMA engine see it only as data block and it is not interpreting it in any
>> way.
>>
>> The metadata can be different per descriptor as it is a parameter for the
>> data being transferred.
>>
>> If the DMA supports per descriptor metadata it can implement the attach,
>> get_ptr/set_len callbacks.
>>
>> Client drivers must only use either attach or get_ptr/set_len to avoid
>> miss configuration.
>
> misconfiguration?
Sorry for the typos, I'll got through them again.
>> Client driver can check if a given metadata mode is supported by the
>> channel during probe time with
>> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_CLIENT);
>> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_ENGINE);
>>
>> and based on this information can use either mode.
>>
>> Wrappers are also added for the metadata_ops.
>>
>> To be used in DESC_METADATA_CLIENT mode:
>> dmaengine_desc_attach_metadata()
>>
>> To be used in DESC_METADATA_ENGINE mode:
>> dmaengine_desc_get_metadata_ptr()
>> dmaengine_desc_set_metadata_len()
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>> Hi,
>>
>> Changes since rfc:
>> - DESC_METADATA_EMBEDDED renamed to DESC_METADATA_ENGINE
>> - Use flow is added for both CLIENT and ENGINE metadata modes
>>
>> Regards,
>> Peter
>>
>> include/linux/dmaengine.h | 144 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 144 insertions(+)
>>
>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>> index 3db833a8c542..f809635cfeaa 100644
>> --- a/include/linux/dmaengine.h
>> +++ b/include/linux/dmaengine.h
>> @@ -231,6 +231,57 @@ typedef struct { DECLARE_BITMAP(bits, DMA_TX_TYPE_END); } dma_cap_mask_t;
>> * @bytes_transferred: byte counter
>> */
>>
>> +/**
>> + * enum dma_desc_metadata_mode - per descriptor metadata mode types supported
>> + * @DESC_METADATA_CLIENT - the metadata buffer is allocated/provided by the
>> + * client driver and it is attached (via the dmaengine_desc_attach_metadata()
>> + * helper) to the descriptor.
>> + *
>> + * Client drivers interested to use this mode can follow:
>> + * - DMA_MEM_TO_DEV:
>> + * 1. prepare the descriptor (dmaengine_prep_*)
>> + * construct the metadata in the clinet's buffer
>
> typo clinet
>
>> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
>> + * descriptor
>> + * 3. submit the transfer
>> + * - DMA_DEV_TO_MEM:
>> + * 1. prepare the descriptor (dmaengine_prep_*)
>> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
>> + * descriptor
>> + * 3. submit the transfer
>> + * 4. when the transfer is completed, the metadata should be available in the
>> + * attached buffer
>
> I guess this is good to be moved into Documentation
Should I create a new file for metadata? I guess it would make sense as the
information is for both clients and engines.
>
> also we dont allow this for memcpy txn?
I have not thought about that, but if I think about it it should be along the
same lines as MEM_TO_DEV.
I'll add the MEM_TO_MEM as well to the documentation.
>> + *
>> + * @DESC_METADATA_ENGINE - the metadata buffer is allocated/managed by the DMA
>> + * driver. The client driver can ask for the pointer, maximum size and the
>> + * currently used size of the metadata and can directly update or read it.
>> + * dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is
>> + * provided as helper functions.
>> + *
>> + * Client drivers interested to use this mode can follow:
>> + * - DMA_MEM_TO_DEV:
>> + * 1. prepare the descriptor (dmaengine_prep_*)
>> + * 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the engine's
>> + * metadata area
>> + * 3. update the metadata at the pointer
>> + * 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine the amount
>> + * of data the client has placed into the metadata buffer
>> + * 5. submit the transfer
>> + * - DMA_DEV_TO_MEM:
>> + * 1. prepare the descriptor (dmaengine_prep_*)
>> + * 2. submit the transfer
>> + * 3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
>> + * pointer to the engine's metadata are
>> + * 4. Read out the metadate from the pointer
>> + *
>> + * Note: the two mode is not compatible and clients must use one mode for a
>> + * descriptor.
>> + */
>> +enum dma_desc_metadata_mode {
>> + DESC_METADATA_CLIENT = (1 << 0),
>> + DESC_METADATA_ENGINE = (1 << 1),
>
> BIT(x)
OK, I followed what we have in the header to not mix (1 << x) and BIT(x)
>
>> +};
>> +
>> struct dma_chan_percpu {
>> /* stats */
>> unsigned long memcpy_count;
>> @@ -494,6 +545,18 @@ struct dmaengine_unmap_data {
>> dma_addr_t addr[0];
>> };
>>
>> +struct dma_async_tx_descriptor;
>> +
>> +struct dma_descriptor_metadata_ops {
>> + int (*attach)(struct dma_async_tx_descriptor *desc, void *data,
>> + size_t len);
>> +
>> + void *(*get_ptr)(struct dma_async_tx_descriptor *desc,
>> + size_t *payload_len, size_t *max_len);
>> + int (*set_len)(struct dma_async_tx_descriptor *desc,
>> + size_t payload_len);
>> +};
>> +
>> /**
>> * struct dma_async_tx_descriptor - async transaction descriptor
>> * ---dma generic offload fields---
>> @@ -523,6 +586,8 @@ struct dma_async_tx_descriptor {
>> dma_async_tx_callback_result callback_result;
>> void *callback_param;
>> struct dmaengine_unmap_data *unmap;
>> + enum dma_desc_metadata_mode desc_metadata_mode;
>> + struct dma_descriptor_metadata_ops *metadata_ops;
I forgot to update the comment section for the dma_async_tx_descriptor, I'll
address it in v2.
>> #ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
>> struct dma_async_tx_descriptor *next;
>> struct dma_async_tx_descriptor *parent;
>> @@ -685,6 +750,7 @@ struct dma_filter {
>> * @global_node: list_head for global dma_device_list
>> * @filter: information for device/slave to filter function/param mapping
>> * @cap_mask: one or more dma_capability flags
>> + * @desc_metadata_modes: supported metadata modes by the DMA device
>> * @max_xor: maximum number of xor sources, 0 if no capability
>> * @max_pq: maximum number of PQ sources and PQ-continue capability
>> * @copy_align: alignment shift for memcpy operations
>> @@ -749,6 +815,7 @@ struct dma_device {
>> struct list_head global_node;
>> struct dma_filter filter;
>> dma_cap_mask_t cap_mask;
>> + enum dma_desc_metadata_mode desc_metadata_modes;
>> unsigned short max_xor;
>> unsigned short max_pq;
>> enum dmaengine_alignment copy_align;
>> @@ -935,6 +1002,83 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memcpy(
>> len, flags);
>> }
>>
>> +static inline bool dmaengine_is_metadata_mode_supported(struct dma_chan *chan,
>> + enum dma_desc_metadata_mode mode)
>> +{
>> + return !!(chan->device->desc_metadata_modes & mode);
>> +}
>> +
>> +static inline int _desc_check_and_set_metadata_mode(
>
> why does this need to start with _ ?
To scare people to use in client code ;)
>
>> + struct dma_async_tx_descriptor *desc, enum dma_desc_metadata_mode mode)
>> +{
>> + /* Make sure that the metadata mode is not mixed */
>> + if (!desc->desc_metadata_mode) {
>> + if (dmaengine_is_metadata_mode_supported(desc->chan, mode))
>> + desc->desc_metadata_mode = mode;
>> + else
>> + return -ENOTSUPP;
>> + } else if (desc->desc_metadata_mode != mode) {
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static inline int dmaengine_desc_attach_metadata(
>> + struct dma_async_tx_descriptor *desc, void *data, size_t len)
>> +{
>> + int ret;
>> +
>> + if (!desc)
>> + return -EINVAL;
>> +
>> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_CLIENT);
>> + if (ret)
>> + return ret;
>> +
>> + if (!desc->metadata_ops || !desc->metadata_ops->attach)
>> + return -ENOTSUPP;
>> +
>> + return desc->metadata_ops->attach(desc, data, len);
>> +}
>> +
>> +static inline void *dmaengine_desc_get_metadata_ptr(
>> + struct dma_async_tx_descriptor *desc, size_t *payload_len,
>> + size_t *max_len)
>> +{
>> + int ret;
>> +
>> + if (!desc)
>> + return ERR_PTR(-EINVAL);
>> +
>> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
>> + if (ret)
>> + return ERR_PTR(ret);
>> +
>> + if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
>> + return ERR_PTR(-ENOTSUPP);
>> +
>> + return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
>> +}
>> +
>> +static inline int dmaengine_desc_set_metadata_len(
>> + struct dma_async_tx_descriptor *desc, size_t payload_len)
>> +{
>> + int ret;
>> +
>> + if (!desc)
>> + return -EINVAL;
>> +
>> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
>> + if (ret)
>> + return ret;
>> +
>> + if (!desc->metadata_ops || !desc->metadata_ops->set_len)
>> + return -ENOTSUPP;
>> +
>> + return desc->metadata_ops->set_len(desc, payload_len);
>> +}
>
> thats bit too much code for a header file :( Lets move it to C file
> please. We can utilize local dmaengine.h and not expose all these and
> possible misuse by clients
OK, I have thought about that as well.
> Also I would like to see a use :-) before further comments.
You mean the DMA driver and at least one client?
I have the DMA driver in my public facing branch [1], but it is not an easy
read with it's close to 4k loc.
The client is not in my branch and it is actually using an older version of
the metadata support.
The problem is that I don't know when I will be able to send the driver for
review as all of this is targeting a brand new SoC (AM654) with completely new
data movement architecture. There are lots of dependencies still need to be
upstreamed before I can send something which at least compiles.
I can offer snippets from the client driver, if that is good enough or a link
to the public tree where it can be accessed, but it is not going to go
upstream before the DMA driver.
[1]
https://git.ti.com/ti-linux-kernel/ti-linux-kernel/trees/ti-linux-4.14.y/drivers/dma/ti/
https://github.com/omap-audio/linux-audio/tree/peter/ti-linux-4.14.y/wip/drivers/dma/ti
k3-udma.c/h is the DMAengine driver, k3-navss-pricate.c k3-navss-udma.c is
something I try to get rid of (it is wrapper for the networking drivers to use
DMA)
^ permalink raw reply
* dmaengine: Add metadata_ops for dma_async_tx_descriptor
From: Vinod Koul @ 2018-08-29 15:52 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: dan.j.williams, dmaengine, linux-kernel, lars, radheys
On 23-08-18, 16:07, Peter Ujfalusi wrote:
> The metadata is best described as side band data or parameters traveling
> alongside the data DMAd by the DMA engine. It is data
> which is understood by the peripheral and the peripheral driver only, the
> DMA engine see it only as data block and it is not interpreting it in any
> way.
>
> The metadata can be different per descriptor as it is a parameter for the
> data being transferred.
>
> If the DMA supports per descriptor metadata it can implement the attach,
> get_ptr/set_len callbacks.
>
> Client drivers must only use either attach or get_ptr/set_len to avoid
> miss configuration.
misconfiguration?
> Client driver can check if a given metadata mode is supported by the
> channel during probe time with
> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_CLIENT);
> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_ENGINE);
>
> and based on this information can use either mode.
>
> Wrappers are also added for the metadata_ops.
>
> To be used in DESC_METADATA_CLIENT mode:
> dmaengine_desc_attach_metadata()
>
> To be used in DESC_METADATA_ENGINE mode:
> dmaengine_desc_get_metadata_ptr()
> dmaengine_desc_set_metadata_len()
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> Hi,
>
> Changes since rfc:
> - DESC_METADATA_EMBEDDED renamed to DESC_METADATA_ENGINE
> - Use flow is added for both CLIENT and ENGINE metadata modes
>
> Regards,
> Peter
>
> include/linux/dmaengine.h | 144 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 144 insertions(+)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 3db833a8c542..f809635cfeaa 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -231,6 +231,57 @@ typedef struct { DECLARE_BITMAP(bits, DMA_TX_TYPE_END); } dma_cap_mask_t;
> * @bytes_transferred: byte counter
> */
>
> +/**
> + * enum dma_desc_metadata_mode - per descriptor metadata mode types supported
> + * @DESC_METADATA_CLIENT - the metadata buffer is allocated/provided by the
> + * client driver and it is attached (via the dmaengine_desc_attach_metadata()
> + * helper) to the descriptor.
> + *
> + * Client drivers interested to use this mode can follow:
> + * - DMA_MEM_TO_DEV:
> + * 1. prepare the descriptor (dmaengine_prep_*)
> + * construct the metadata in the clinet's buffer
typo clinet
> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
> + * descriptor
> + * 3. submit the transfer
> + * - DMA_DEV_TO_MEM:
> + * 1. prepare the descriptor (dmaengine_prep_*)
> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
> + * descriptor
> + * 3. submit the transfer
> + * 4. when the transfer is completed, the metadata should be available in the
> + * attached buffer
I guess this is good to be moved into Documentation
also we dont allow this for memcpy txn?
> + *
> + * @DESC_METADATA_ENGINE - the metadata buffer is allocated/managed by the DMA
> + * driver. The client driver can ask for the pointer, maximum size and the
> + * currently used size of the metadata and can directly update or read it.
> + * dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is
> + * provided as helper functions.
> + *
> + * Client drivers interested to use this mode can follow:
> + * - DMA_MEM_TO_DEV:
> + * 1. prepare the descriptor (dmaengine_prep_*)
> + * 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the engine's
> + * metadata area
> + * 3. update the metadata at the pointer
> + * 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine the amount
> + * of data the client has placed into the metadata buffer
> + * 5. submit the transfer
> + * - DMA_DEV_TO_MEM:
> + * 1. prepare the descriptor (dmaengine_prep_*)
> + * 2. submit the transfer
> + * 3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
> + * pointer to the engine's metadata are
> + * 4. Read out the metadate from the pointer
> + *
> + * Note: the two mode is not compatible and clients must use one mode for a
> + * descriptor.
> + */
> +enum dma_desc_metadata_mode {
> + DESC_METADATA_CLIENT = (1 << 0),
> + DESC_METADATA_ENGINE = (1 << 1),
BIT(x)
> +};
> +
> struct dma_chan_percpu {
> /* stats */
> unsigned long memcpy_count;
> @@ -494,6 +545,18 @@ struct dmaengine_unmap_data {
> dma_addr_t addr[0];
> };
>
> +struct dma_async_tx_descriptor;
> +
> +struct dma_descriptor_metadata_ops {
> + int (*attach)(struct dma_async_tx_descriptor *desc, void *data,
> + size_t len);
> +
> + void *(*get_ptr)(struct dma_async_tx_descriptor *desc,
> + size_t *payload_len, size_t *max_len);
> + int (*set_len)(struct dma_async_tx_descriptor *desc,
> + size_t payload_len);
> +};
> +
> /**
> * struct dma_async_tx_descriptor - async transaction descriptor
> * ---dma generic offload fields---
> @@ -523,6 +586,8 @@ struct dma_async_tx_descriptor {
> dma_async_tx_callback_result callback_result;
> void *callback_param;
> struct dmaengine_unmap_data *unmap;
> + enum dma_desc_metadata_mode desc_metadata_mode;
> + struct dma_descriptor_metadata_ops *metadata_ops;
> #ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
> struct dma_async_tx_descriptor *next;
> struct dma_async_tx_descriptor *parent;
> @@ -685,6 +750,7 @@ struct dma_filter {
> * @global_node: list_head for global dma_device_list
> * @filter: information for device/slave to filter function/param mapping
> * @cap_mask: one or more dma_capability flags
> + * @desc_metadata_modes: supported metadata modes by the DMA device
> * @max_xor: maximum number of xor sources, 0 if no capability
> * @max_pq: maximum number of PQ sources and PQ-continue capability
> * @copy_align: alignment shift for memcpy operations
> @@ -749,6 +815,7 @@ struct dma_device {
> struct list_head global_node;
> struct dma_filter filter;
> dma_cap_mask_t cap_mask;
> + enum dma_desc_metadata_mode desc_metadata_modes;
> unsigned short max_xor;
> unsigned short max_pq;
> enum dmaengine_alignment copy_align;
> @@ -935,6 +1002,83 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memcpy(
> len, flags);
> }
>
> +static inline bool dmaengine_is_metadata_mode_supported(struct dma_chan *chan,
> + enum dma_desc_metadata_mode mode)
> +{
> + return !!(chan->device->desc_metadata_modes & mode);
> +}
> +
> +static inline int _desc_check_and_set_metadata_mode(
why does this need to start with _ ?
> + struct dma_async_tx_descriptor *desc, enum dma_desc_metadata_mode mode)
> +{
> + /* Make sure that the metadata mode is not mixed */
> + if (!desc->desc_metadata_mode) {
> + if (dmaengine_is_metadata_mode_supported(desc->chan, mode))
> + desc->desc_metadata_mode = mode;
> + else
> + return -ENOTSUPP;
> + } else if (desc->desc_metadata_mode != mode) {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static inline int dmaengine_desc_attach_metadata(
> + struct dma_async_tx_descriptor *desc, void *data, size_t len)
> +{
> + int ret;
> +
> + if (!desc)
> + return -EINVAL;
> +
> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_CLIENT);
> + if (ret)
> + return ret;
> +
> + if (!desc->metadata_ops || !desc->metadata_ops->attach)
> + return -ENOTSUPP;
> +
> + return desc->metadata_ops->attach(desc, data, len);
> +}
> +
> +static inline void *dmaengine_desc_get_metadata_ptr(
> + struct dma_async_tx_descriptor *desc, size_t *payload_len,
> + size_t *max_len)
> +{
> + int ret;
> +
> + if (!desc)
> + return ERR_PTR(-EINVAL);
> +
> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
> + return ERR_PTR(-ENOTSUPP);
> +
> + return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
> +}
> +
> +static inline int dmaengine_desc_set_metadata_len(
> + struct dma_async_tx_descriptor *desc, size_t payload_len)
> +{
> + int ret;
> +
> + if (!desc)
> + return -EINVAL;
> +
> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
> + if (ret)
> + return ret;
> +
> + if (!desc->metadata_ops || !desc->metadata_ops->set_len)
> + return -ENOTSUPP;
> +
> + return desc->metadata_ops->set_len(desc, payload_len);
> +}
thats bit too much code for a header file :( Lets move it to C file
please. We can utilize local dmaengine.h and not expose all these and
possible misuse by clients
Also I would like to see a use :-) before further comments.
^ permalink raw reply
* [v4,3/7] dt-bindings: dmaengine: xilinx_dma: add optional xlnx,sg-length-width property
From: Andrea Merello @ 2018-08-29 8:14 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-kernel, Rob Herring, Mark Rutland, devicetree,
Radhey Shyam Pandey
On Mon, Aug 27, 2018 at 7:31 AM Vinod <vkoul@kernel.org> wrote:
>
> On 02-08-18, 16:10, Andrea Merello wrote:
> > The width of the "length register" cannot be autodetected, and it is now
> > specified with a DT property. Add DOC for it.
>
> Add Documentation for it...
OK
> >
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: devicetree@vger.kernel.org
> > Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> > Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > ---
> > Changes in v2:
> > - change property name
> > - property is now optional
> > - cc DT maintainer
> > Changes in v3:
> > - reword
> > - cc DT maintainerS and ML
> > Changes in v4:
> > - specify the unit, the valid range and the default value
> > ---
> > Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > index a2b8bfaec43c..aec4a41a03ae 100644
> > --- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > @@ -41,6 +41,10 @@ Optional properties:
> > - xlnx,include-sg: Tells configured for Scatter-mode in
> > the hardware.
> > Optional properties for AXI DMA:
> > +- xlnx,sg-length-width: Should be set to the width in bits of the length
> > + register as configured in h/w. Takes values {8...26}. If the property
> > + is missing or invalid then the default value 23 is used. This is the
> > + maximum value that is supported by all IP versions.
> > - xlnx,mcdma: Tells whether configured for multi-channel mode in the hardware.
> > Optional properties for VDMA:
> > - xlnx,flush-fsync: Tells which channel to Flush on Frame sync.
> > --
> > 2.17.1
>
> --
> ~Vinod
^ permalink raw reply
* [v4,2/7] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors
From: Andrea Merello @ 2018-08-29 8:12 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
v4-000linux-arm-kernel, linux-kernel, Rob Herring, Mark Rutland,
devicetree, Radhey Shyam Pandey
On Mon, Aug 27, 2018 at 7:30 AM Vinod <vkoul@kernel.org> wrote:
>
> On 02-08-18, 16:10, Andrea Merello wrote:
>
> s/cylic/cyclic in patch title
OK
> > Whenever a single or cyclic transaction is prepared, the driver
> > could eventually split it over several SG descriptors in order
> > to deal with the HW maximum transfer length.
> >
> > This could end up in DMA operations starting from a misaligned
> > address. This seems fatal for the HW if DRE is not enabled.
>
> DRE?
Stands for "Data Realignment Engine". I will add this string nearby
the acronym..
> >
> > This patch eventually adjusts the transfer size in order to make sure
> > all operations start from an aligned address.
> >
> > Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> > Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > ---
> > Changes in v2:
> > - don't introduce copy_mask field, rather rely on already-esistent
> > copy_align field. Suggested by Radhey Shyam Pandey
> > - reword title
> > Changes in v3:
> > - fix bug introduced in v2: wrong copy size when DRE is enabled
> > - use implementation suggested by Radhey Shyam Pandey
> > Changes in v4:
> > - rework on the top of 1/6
> > ---
> > drivers/dma/xilinx/xilinx_dma.c | 22 ++++++++++++++++++----
> > 1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> > index a3aaa0e34cc7..aaa6de8a70e4 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -954,15 +954,28 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
> >
> > /**
> > * xilinx_dma_calc_copysize - Calculate the amount of data to copy
> > + * @chan: Driver specific DMA channel
> > * @size: Total data that needs to be copied
> > * @done: Amount of data that has been already copied
> > *
> > * Return: Amount of data that has to be copied
> > */
> > -static int xilinx_dma_calc_copysize(int size, int done)
> > +static int xilinx_dma_calc_copysize(struct xilinx_dma_chan *chan,
> > + int size, int done)
>
> please align with opening brace
OK
> > {
> > - return min_t(size_t, size - done,
> > + size_t copy = min_t(size_t, size - done,
> > XILINX_DMA_MAX_TRANS_LEN);
> > +
> > + if ((copy + done < size) &&
> > + chan->xdev->common.copy_align) {
> > + /*
> > + * If this is not the last descriptor, make sure
> > + * the next one will be properly aligned
> > + */
> > + copy = rounddown(copy,
> > + (1 << chan->xdev->common.copy_align));
> > + }
> > + return copy;
> > }
> >
> > /**
> > @@ -1804,7 +1817,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
> > * Calculate the maximum number of bytes to transfer,
> > * making sure it is less than the hw limit
> > */
> > - copy = xilinx_dma_calc_copysize(sg_dma_len(sg),
> > + copy = xilinx_dma_calc_copysize(chan, sg_dma_len(sg),
> > sg_used);
> > hw = &segment->hw;
> >
> > @@ -1909,7 +1922,8 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
> > * Calculate the maximum number of bytes to transfer,
> > * making sure it is less than the hw limit
> > */
> > - copy = xilinx_dma_calc_copysize(period_len, sg_used);
> > + copy = xilinx_dma_calc_copysize(chan,
> > + period_len, sg_used);
> > hw = &segment->hw;
> > xilinx_axidma_buf(chan, hw, buf_addr, sg_used,
> > period_len * i);
> > --
> > 2.17.1
>
> --
> ~Vinod
^ permalink raw reply
* [3/3] dmaengine: xilinx_dma: Fix 64-bit simple CDMA transfer
From: Vinod Koul @ 2018-08-29 4:01 UTC (permalink / raw)
To: Radhey Shyam Pandey
Cc: dan.j.williams@intel.com, Michal Simek,
Appana Durga Kedareswara Rao, lars@metafoo.de,
dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
On 28-08-18, 14:03, Radhey Shyam Pandey wrote:
> > On 27-07-18, 16:20, Radhey Shyam Pandey wrote:
> > > In AXI CDMA simple mode also pass MSB bits of source and destination
> > > address to xilinx_write function. This fixes simple CDMA operation
> > > mode using 64-bit addressing.
> > >
> > > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > > ---
> > > drivers/dma/xilinx/xilinx_dma.c | 6 ++++--
> > > 1 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> > > index a37871e..2e15d86 100644
> > > --- a/drivers/dma/xilinx/xilinx_dma.c
> > > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > > @@ -1245,8 +1245,10 @@ static void xilinx_cdma_start_transfer(struct
> > xilinx_dma_chan *chan)
> > >
> > > hw = &segment->hw;
> > >
> > > - xilinx_write(chan, XILINX_CDMA_REG_SRCADDR, hw-
> > >src_addr);
> > > - xilinx_write(chan, XILINX_CDMA_REG_DSTADDR, hw-
> > >dest_addr);
> > > + xilinx_write(chan, XILINX_CDMA_REG_SRCADDR,
> > (dma_addr_t)
> > > + ((u64)hw->src_addr_msb << 32 | hw->src_addr));
> >
> > so this is:
> > (dma_addr_t)((u64)hw->src_addr_msb << 32 | hw->src_addr)
> >
> > what is src_addr data type? I think its u32. It would be better to
> > update xilinx_write() to take u64 and not dma_addr_t.
>
> Yes, src_addr_msb and src_addr BD fields are u32. To explain: There is no
> prob in xilinx_write it takes dma_addr_t as an arg which is 32/64 bit
> depending on _DMA_ADDR_T_64BIT. In 64bit CDMA transfer, there was a bug
> i.e in the call to xilinx_write src_addr_msb 32 bits were not passed. To fix
> that combine MSB and LSB 32 bits before passing it to xilinx_write.
Yeah that part was clear but the implementation can be better..
^ permalink raw reply
* [v2,1/2] dt-bindings: dmaengine: add DT binding for UniPhier MIO DMAC
From: Rob Herring @ 2018-08-29 0:36 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Vinod Koul, dmaengine, devicetree, Jassi Brar, Mark Rutland,
linux-arm-kernel
On Fri, 24 Aug 2018 10:41:12 +0900, Masahiro Yamada wrote:
> The MIO DMAC (Media IO DMA Controller) is used in UniPhier LD4,
> Pro4, and sLD8 SoCs.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Changes in v2:
> - Rename the node "dmac" to "dma-controller"
> - Remove dma-channels property
>
> .../devicetree/bindings/dma/uniphier-mio-dmac.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/uniphier-mio-dmac.txt
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [3/3] dmaengine: xilinx_dma: Fix 64-bit simple CDMA transfer
From: Radhey Shyam Pandey @ 2018-08-28 14:03 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams@intel.com, Michal Simek,
Appana Durga Kedareswara Rao, lars@metafoo.de,
dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
> -----Original Message-----
> From: Vinod <vkoul@kernel.org>
> Sent: Tuesday, August 21, 2018 9:26 PM
> To: Radhey Shyam Pandey <radheys@xilinx.com>
> Cc: dan.j.williams@intel.com; Michal Simek <michals@xilinx.com>; Appana
> Durga Kedareswara Rao <appanad@xilinx.com>; lars@metafoo.de;
> dmaengine@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 3/3] dmaengine: xilinx_dma: Fix 64-bit simple CDMA
> transfer
>
> On 27-07-18, 16:20, Radhey Shyam Pandey wrote:
> > In AXI CDMA simple mode also pass MSB bits of source and destination
> > address to xilinx_write function. This fixes simple CDMA operation
> > mode using 64-bit addressing.
> >
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > ---
> > drivers/dma/xilinx/xilinx_dma.c | 6 ++++--
> > 1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> > index a37871e..2e15d86 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -1245,8 +1245,10 @@ static void xilinx_cdma_start_transfer(struct
> xilinx_dma_chan *chan)
> >
> > hw = &segment->hw;
> >
> > - xilinx_write(chan, XILINX_CDMA_REG_SRCADDR, hw-
> >src_addr);
> > - xilinx_write(chan, XILINX_CDMA_REG_DSTADDR, hw-
> >dest_addr);
> > + xilinx_write(chan, XILINX_CDMA_REG_SRCADDR,
> (dma_addr_t)
> > + ((u64)hw->src_addr_msb << 32 | hw->src_addr));
>
> so this is:
> (dma_addr_t)((u64)hw->src_addr_msb << 32 | hw->src_addr)
>
> what is src_addr data type? I think its u32. It would be better to
> update xilinx_write() to take u64 and not dma_addr_t.
Yes, src_addr_msb and src_addr BD fields are u32. To explain: There is no
prob in xilinx_write it takes dma_addr_t as an arg which is 32/64 bit
depending on _DMA_ADDR_T_64BIT. In 64bit CDMA transfer, there was a bug
i.e in the call to xilinx_write src_addr_msb 32 bits were not passed. To fix
that combine MSB and LSB 32 bits before passing it to xilinx_write.
Thanks,
Radhey
>
>
> > + xilinx_write(chan, XILINX_CDMA_REG_DSTADDR,
> (dma_addr_t)
> > + ((u64)hw->dest_addr_msb << 32 | hw-
> >dest_addr));
> >
> > /* Start the transfer */
> > dma_ctrl_write(chan, XILINX_DMA_REG_BTT,
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> ~Vinod
^ permalink raw reply
* [2/3] dmaengine: xilinx_dma: Refactor axidma channel validation
From: Radhey Shyam Pandey @ 2018-08-28 13:03 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams@intel.com, Michal Simek,
Appana Durga Kedareswara Rao, lars@metafoo.de,
dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
> -----Original Message-----
> From: Vinod <vkoul@kernel.org>
> Sent: Tuesday, August 21, 2018 9:20 PM
> To: Radhey Shyam Pandey <radheys@xilinx.com>
> Cc: dan.j.williams@intel.com; Michal Simek <michals@xilinx.com>; Appana
> Durga Kedareswara Rao <appanad@xilinx.com>; lars@metafoo.de;
> dmaengine@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] dmaengine: xilinx_dma: Refactor axidma channel
> validation
>
> On 27-07-18, 16:20, Radhey Shyam Pandey wrote:
> > In axidma start_transfer, prefer checking channel states before other
> > params i.e pending_list.
>
> and what that preference be?
There is no strict preference. I thought to group and first check channel
states(idle/error) and then look for pending list.
Thanks,
Radhey
>
> >
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > ---
> > drivers/dma/xilinx/xilinx_dma.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c
> > b/drivers/dma/xilinx/xilinx_dma.c index 06d1632..a37871e 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -1271,10 +1271,10 @@ static void xilinx_dma_start_transfer(struct
> xilinx_dma_chan *chan)
> > if (chan->err)
> > return;
> >
> > - if (list_empty(&chan->pending_list))
> > + if (!chan->idle)
> > return;
> >
> > - if (!chan->idle)
> > + if (list_empty(&chan->pending_list))
> > return;
> >
> > head_desc = list_first_entry(&chan->pending_list,
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe dmaengine"
> > in the body of a message to majordomo@vger.kernel.org More majordomo
> > info at http://vger.kernel.org/majordomo-info.html
>
> --
> ~Vinod
^ permalink raw reply
* [RESEND,v2] dmaengine: sprd: Support DMA link-list mode
From: Baolin Wang @ 2018-08-28 11:09 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: eric.long, broonie, baolin.wang, dmaengine, linux-kernel
From: Eric Long <eric.long@spreadtrum.com>
The Spreadtrum DMA can support the link-list transaction mode, which means
DMA controller can do transaction one by one automatically once we linked
these transaction by link-list register.
Signed-off-by: Eric Long <eric.long@spreadtrum.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
Changes since v1:
- Remove sprd_dma_fill_chn_desc() function.
- Remove the redundant validation of 'sglen' in sprd_dma_fill_linklist_desc().
- Add some comments make code more clear.
- Fix the typos.
---
drivers/dma/sprd-dma.c | 81 ++++++++++++++++++++++++++++++++++++++----
include/linux/dma/sprd-dma.h | 69 +++++++++++++++++++++++++++++++++++
2 files changed, 143 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 55df0d4..38d4e4f 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -68,6 +68,7 @@
/* SPRD_DMA_CHN_CFG register definition */
#define SPRD_DMA_CHN_EN BIT(0)
+#define SPRD_DMA_LINKLIST_EN BIT(4)
#define SPRD_DMA_WAIT_BDONE_OFFSET 24
#define SPRD_DMA_DONOT_WAIT_BDONE 1
@@ -103,7 +104,7 @@
#define SPRD_DMA_REQ_MODE_MASK GENMASK(1, 0)
#define SPRD_DMA_FIX_SEL_OFFSET 21
#define SPRD_DMA_FIX_EN_OFFSET 20
-#define SPRD_DMA_LLIST_END_OFFSET 19
+#define SPRD_DMA_LLIST_END BIT(19)
#define SPRD_DMA_FRG_LEN_MASK GENMASK(16, 0)
/* SPRD_DMA_CHN_BLK_LEN register definition */
@@ -164,6 +165,7 @@ struct sprd_dma_desc {
struct sprd_dma_chn {
struct virt_dma_chan vc;
void __iomem *chn_base;
+ struct sprd_dma_linklist linklist;
struct dma_slave_config slave_cfg;
u32 chn_num;
u32 dev_id;
@@ -582,7 +584,8 @@ static int sprd_dma_get_step(enum dma_slave_buswidth buswidth)
}
static int sprd_dma_fill_desc(struct dma_chan *chan,
- struct sprd_dma_desc *sdesc,
+ struct sprd_dma_chn_hw *hw,
+ unsigned int sglen, int sg_index,
dma_addr_t src, dma_addr_t dst, u32 len,
enum dma_transfer_direction dir,
unsigned long flags,
@@ -590,7 +593,6 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
{
struct sprd_dma_dev *sdev = to_sprd_dma_dev(chan);
struct sprd_dma_chn *schan = to_sprd_dma_chan(chan);
- struct sprd_dma_chn_hw *hw = &sdesc->chn_hw;
u32 req_mode = (flags >> SPRD_DMA_REQ_SHIFT) & SPRD_DMA_REQ_MODE_MASK;
u32 int_mode = flags & SPRD_DMA_INT_MASK;
int src_datawidth, dst_datawidth, src_step, dst_step;
@@ -670,12 +672,52 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
temp |= (src_step & SPRD_DMA_TRSF_STEP_MASK) << SPRD_DMA_SRC_TRSF_STEP_OFFSET;
hw->trsf_step = temp;
+ /* link-list configuration */
+ if (schan->linklist.phy_addr) {
+ if (sg_index == sglen - 1)
+ hw->frg_len |= SPRD_DMA_LLIST_END;
+
+ hw->cfg |= SPRD_DMA_LINKLIST_EN;
+
+ /* link-list index */
+ temp = (sg_index + 1) % sglen;
+ /* Next link-list configuration's physical address offset */
+ temp = temp * sizeof(*hw) + SPRD_DMA_CHN_SRC_ADDR;
+ /*
+ * Set the link-list pointer point to next link-list
+ * configuration's physical address.
+ */
+ hw->llist_ptr = schan->linklist.phy_addr + temp;
+ } else {
+ hw->llist_ptr = 0;
+ }
+
hw->frg_step = 0;
hw->src_blk_step = 0;
hw->des_blk_step = 0;
return 0;
}
+static int sprd_dma_fill_linklist_desc(struct dma_chan *chan,
+ unsigned int sglen, int sg_index,
+ dma_addr_t src, dma_addr_t dst, u32 len,
+ enum dma_transfer_direction dir,
+ unsigned long flags,
+ struct dma_slave_config *slave_cfg)
+{
+ struct sprd_dma_chn *schan = to_sprd_dma_chan(chan);
+ struct sprd_dma_chn_hw *hw;
+
+ if (!schan->linklist.virt_addr)
+ return -EINVAL;
+
+ hw = (struct sprd_dma_chn_hw *)(schan->linklist.virt_addr +
+ sg_index * sizeof(*hw));
+
+ return sprd_dma_fill_desc(chan, hw, sglen, sg_index, src, dst, len,
+ dir, flags, slave_cfg);
+}
+
static struct dma_async_tx_descriptor *
sprd_dma_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
size_t len, unsigned long flags)
@@ -744,10 +786,20 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
u32 len = 0;
int ret, i;
- /* TODO: now we only support one sg for each DMA configuration. */
- if (!is_slave_direction(dir) || sglen > 1)
+ if (!is_slave_direction(dir))
return NULL;
+ if (context) {
+ struct sprd_dma_linklist *ll_cfg =
+ (struct sprd_dma_linklist *)context;
+
+ schan->linklist.phy_addr = ll_cfg->phy_addr;
+ schan->linklist.virt_addr = ll_cfg->virt_addr;
+ } else {
+ schan->linklist.phy_addr = 0;
+ schan->linklist.virt_addr = 0;
+ }
+
sdesc = kzalloc(sizeof(*sdesc), GFP_NOWAIT);
if (!sdesc)
return NULL;
@@ -762,10 +814,25 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
src = slave_cfg->src_addr;
dst = sg_dma_address(sg);
}
+
+ /*
+ * The link-list mode needs at least 2 link-list
+ * configurations. If there is only one sg, it doesn't
+ * need to fill the link-list configuration.
+ */
+ if (sglen < 2)
+ break;
+
+ ret = sprd_dma_fill_linklist_desc(chan, sglen, i, src, dst, len,
+ dir, flags, slave_cfg);
+ if (ret) {
+ kfree(sdesc);
+ return NULL;
+ }
}
- ret = sprd_dma_fill_desc(chan, sdesc, src, dst, len, dir, flags,
- slave_cfg);
+ ret = sprd_dma_fill_desc(chan, &sdesc->chn_hw, 0, 0, src, dst, len,
+ dir, flags, slave_cfg);
if (ret) {
kfree(sdesc);
return NULL;
diff --git a/include/linux/dma/sprd-dma.h b/include/linux/dma/sprd-dma.h
index b0115e3..b42b80e5 100644
--- a/include/linux/dma/sprd-dma.h
+++ b/include/linux/dma/sprd-dma.h
@@ -58,4 +58,73 @@ enum sprd_dma_int_type {
SPRD_DMA_CFGERR_INT,
};
+/*
+ * struct sprd_dma_linklist - DMA link-list address structure
+ * @virt_addr: link-list virtual address to configure link-list node
+ * @phy_addr: link-list physical address to link DMA transfer
+ *
+ * The Spreadtrum DMA controller supports the link-list mode, that means slaves
+ * can supply several groups configurations (each configuration represents one
+ * DMA transfer) saved in memory, and DMA controller will link these groups
+ * configurations by writing the physical address of each configuration into the
+ * link-list register.
+ *
+ * Just as shown below, the link-list pointer register will be pointed to the
+ * physical address of 'configuration 1', and the 'configuration 1' link-list
+ * pointer will be pointed to 'configuration 2', and so on.
+ * Once trigger the DMA transfer, the DMA controller will load 'configuration
+ * 1' to its registers automatically, after 'configuration 1' transaction is
+ * done, DMA controller will load 'configuration 2' automatically, until all
+ * DMA transactions are done.
+ *
+ * Note: The last link-list pointer should point to the physical address
+ * of 'configuration 1', which can avoid DMA controller loads incorrect
+ * configuration when the last configuration transaction is done.
+ *
+ * DMA controller linklist memory
+ * ====================== -----------------------
+ *| | | configuration 1 |<---
+ *| DMA controller | ------->| | |
+ *| | | | | |
+ *| | | | | |
+ *| | | | | |
+ *| linklist pointer reg |---- ----| linklist pointer | |
+ * ====================== | ----------------------- |
+ * | |
+ * | ----------------------- |
+ * | | configuration 2 | |
+ * --->| | |
+ * | | |
+ * | | |
+ * | | |
+ * ----| linklist pointer | |
+ * | ----------------------- |
+ * | |
+ * | ----------------------- |
+ * | | configuration 3 | |
+ * --->| | |
+ * | | |
+ * | . | |
+ * . |
+ * . |
+ * . |
+ * | . |
+ * | ----------------------- |
+ * | | configuration n | |
+ * --->| | |
+ * | | |
+ * | | |
+ * | | |
+ * | linklist pointer |----
+ * -----------------------
+ *
+ * To support the link-list mode, DMA slaves should allocate one segment memory
+ * from always-on IRAM or dma coherent memory to store these groups of DMA
+ * configuration, and pass the virtual and physical address to DMA controller.
+ */
+struct sprd_dma_linklist {
+ unsigned long virt_addr;
+ phys_addr_t phy_addr;
+};
+
#endif
^ 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