2018-07-21 11:11:46

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 00/18] JZ4780 DMA patchset v3

Hi,

This is the version 3 of my jz4780-dma driver update patchset.

Apologies to the DMA people, the v2 of this patchset did not make it to
their mailing-list; see the bottom of this email for a description of
what happened in v2.

Changelog from v2 to v3:

- Modified the devicetree bindings to comply with the specification

- New patch [06/18] allows the JZ4780 DMA driver to be compiled within a
generic MIPS kernel.

Changelog from v1 to v2:

- All documentation changes have been moved to one single patch [01/17].

- The new patch [02/17] enforces that we're probed from devicetree.

- The driver will not fail if only one memory resource has been supplied
in the devicetree, to keep compatibility with old devicetree files.

- A new patch [17/17] adds a devicetree node for the DMA driver in the
JZ4740 DTS file.

- Some other small changes; see per-file changelog.

Regards,
-Paul




2018-07-21 11:09:31

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 18/18] MIPS: JZ4740: DTS: Add DMA nodes

Add the devicetree nodes for the DMA core of the JZ4740 SoC, disabled
by default, as currently there are no clients for the DMA driver
(until the MMC driver and/or others get a devicetree node).

Signed-off-by: Paul Cercueil <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
arch/mips/boot/dts/ingenic/jz4740.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)

v2: New patch in this series

v3: Modify node to comply with devicetree specification

diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
index 26c6b561d6f7..6fb16fd24035 100644
--- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
@@ -154,6 +154,21 @@
clock-names = "baud", "module";
};

+ dmac: dma-controller@13020000 {
+ compatible = "ingenic,jz4740-dma";
+ reg = <0x13020000 0xbc
+ 0x13020300 0x14>;
+ #dma-cells = <2>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <29>;
+
+ clocks = <&cgu JZ4740_CLK_DMA>;
+
+ /* Disable dmac until we have something that uses it */
+ status = "disabled";
+ };
+
uhc: uhc@13030000 {
compatible = "ingenic,jz4740-ohci", "generic-ohci";
reg = <0x13030000 0x1000>;
--
2.11.0


2018-07-21 11:09:31

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 07/18] dmaengine: dma-jz4780: Add support for the JZ4770 SoC

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 <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 46 +++++++++++++++++++++++++++++++++++++++-------
1 file changed, 39 insertions(+), 7 deletions(-)

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

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 23e92d153919..a5f4a8d54516 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
@@ -132,11 +135,13 @@ struct jz4780_dma_chan {
};

enum jz_version {
+ ID_JZ4770,
ID_JZ4780,
};

struct jz4780_dma_soc_data {
unsigned int nb_channels;
+ unsigned int transfer_ord_max;
};

struct jz4780_dma_dev {
@@ -200,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->version == ID_JZ4770)
+ 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->version == ID_JZ4770)
+ 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)
@@ -234,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;

/*
@@ -247,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;

@@ -300,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) {
@@ -429,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;
@@ -490,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);

@@ -537,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);
@@ -548,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,
@@ -775,10 +803,12 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
}

static const struct jz4780_dma_soc_data jz4780_dma_soc_data[] = {
- [ID_JZ4780] = { .nb_channels = 32, },
+ [ID_JZ4770] = { .nb_channels = 6, .transfer_ord_max = 6, },
+ [ID_JZ4780] = { .nb_channels = 32, .transfer_ord_max = 7, },
};

static const struct of_device_id jz4780_dma_dt_match[] = {
+ { .compatible = "ingenic,jz4770-dma", .data = (void *)ID_JZ4770 },
{ .compatible = "ingenic,jz4780-dma", .data = (void *)ID_JZ4780 },
{},
};
@@ -901,7 +931,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 (jzdma->version == ID_JZ4780)
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);

INIT_LIST_HEAD(&dd->channels);

--
2.11.0


2018-07-21 11:09:31

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 02/18] dmaengine: dma-jz4780: Return error if not probed from DT

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 <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 5 +++++
1 file changed, 5 insertions(+)

v2: New patch
v3: No change

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;
--
2.11.0


2018-07-21 11:09:31

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 03/18] dmaengine: dma-jz4780: Avoid hardcoding number of channels

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 <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 53 +++++++++++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 14 deletions(-)

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

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 987899610b46..a26107c85ee7 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,24 @@ struct jz4780_dma_chan {
unsigned int curr_hwdesc;
};

+enum jz_version {
+ ID_JZ4780,
+};
+
+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;
+ enum jz_version version;
+ 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 +657,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 +737,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);
@@ -752,13 +761,27 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
}
}

+static const struct jz4780_dma_soc_data jz4780_dma_soc_data[] = {
+ [ID_JZ4780] = { .nb_channels = 32, },
+};
+
+static const struct of_device_id jz4780_dma_dt_match[] = {
+ { .compatible = "ingenic,jz4780-dma", .data = (void *)ID_JZ4780 },
+ {},
+};
+MODULE_DEVICE_TABLE(of, jz4780_dma_dt_match);
+
static int jz4780_dma_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ const struct of_device_id *of_id = of_match_device(
+ jz4780_dma_dt_match, dev);
+ const struct jz4780_dma_soc_data *soc_data;
struct jz4780_dma_dev *jzdma;
struct jz4780_dma_chan *jzchan;
struct dma_device *dd;
struct resource *res;
+ enum jz_version version;
int i, ret;

if (!dev->of_node) {
@@ -766,10 +789,18 @@ static int jz4780_dma_probe(struct platform_device *pdev)
return -EINVAL;
}

- jzdma = devm_kzalloc(dev, sizeof(*jzdma), GFP_KERNEL);
+ version = (enum jz_version)of_id->data;
+ soc_data = &jz4780_dma_soc_data[version];
+
+ jzdma = devm_kzalloc(dev, sizeof(*jzdma)
+ + sizeof(*jzdma->chan) * soc_data->nb_channels,
+ GFP_KERNEL);
if (!jzdma)
return -ENOMEM;

+ jzdma->soc_data = soc_data;
+ jzdma->version = version;
+
platform_set_drvdata(pdev, jzdma);

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -844,7 +875,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,19 +920,13 @@ 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 of_device_id jz4780_dma_dt_match[] = {
- { .compatible = "ingenic,jz4780-dma", .data = NULL },
- {},
-};
-MODULE_DEVICE_TABLE(of, jz4780_dma_dt_match);
-
static struct platform_driver jz4780_dma_driver = {
.probe = jz4780_dma_probe,
.remove = jz4780_dma_remove,
--
2.11.0


2018-07-21 11:09:31

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 01/18] doc: dt-bindings: jz4780-dma: Update bindings to reflect driver changes

The driver is now compatible with four SoCs: JZ4780, JZ4770, JZ4725B and
JZ4740.

Besides, it 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 <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
Documentation/devicetree/bindings/dma/jz4780-dma.txt | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

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

diff --git a/Documentation/devicetree/bindings/dma/jz4780-dma.txt b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
index f25feee62b15..14f33305e194 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.
- interrupt-parent: Should be the phandle of the interrupt controller that
- clocks: Should contain a clock specifier for the JZ4780 PDMA clock.
@@ -20,9 +25,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>;
--
2.11.0


2018-07-21 11:09:31

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 16/18] MIPS: JZ4780: DTS: Update DMA node to match driver changes

The driver now accepts two memory resources, the first one for the
channel-specific registers, the second one for the controller-specific
registers.

Note that older devicetrees, without this commit, will still work with
the jz4780-dma driver.

Signed-off-by: Paul Cercueil <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
arch/mips/boot/dts/ingenic/jz4780.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

v2: Update info about devicetree ABI compatibility in commit message

v3: No change

diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index aa4e8f75ff5d..ad3b1f827cf5 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -247,7 +247,8 @@

dma: dma@13420000 {
compatible = "ingenic,jz4780-dma";
- reg = <0x13420000 0x10000>;
+ reg = <0x13420000 0x400
+ 0x13421000 0x40>;
#dma-cells = <2>;

interrupt-parent = <&intc>;
--
2.11.0


2018-07-21 11:09:31

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 17/18] MIPS: JZ4770: DTS: Add DMA nodes

Add the two devicetree nodes for the two DMA cores of the JZ4770 SoC,
disabled by default, as currently there are no clients for the DMA
driver (until the MMC driver and/or others get a devicetree node).

Signed-off-by: Paul Cercueil <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
arch/mips/boot/dts/ingenic/jz4770.dtsi | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

v2: No change

v3: Modify nodes to comply with devicetree specification

diff --git a/arch/mips/boot/dts/ingenic/jz4770.dtsi b/arch/mips/boot/dts/ingenic/jz4770.dtsi
index 7c2804f3f5f1..49ede6c14ff3 100644
--- a/arch/mips/boot/dts/ingenic/jz4770.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4770.dtsi
@@ -196,6 +196,36 @@
status = "disabled";
};

+ dmac0: dma-controller@13420000 {
+ compatible = "ingenic,jz4770-dma";
+ reg = <0x13420000 0xC0
+ 0x13420300 0x20>;
+
+ #dma-cells = <1>;
+
+ clocks = <&cgu JZ4770_CLK_DMA>;
+ interrupt-parent = <&intc>;
+ interrupts = <24>;
+
+ /* Disable dmac0 until we have something that uses it */
+ status = "disabled";
+ };
+
+ dmac1: dma-controller@13420100 {
+ compatible = "ingenic,jz4770-dma";
+ reg = <0x13420100 0xC0
+ 0x13420400 0x20>;
+
+ #dma-cells = <1>;
+
+ clocks = <&cgu JZ4770_CLK_DMA>;
+ interrupt-parent = <&intc>;
+ interrupts = <23>;
+
+ /* Disable dmac1 until we have something that uses it */
+ status = "disabled";
+ };
+
uhc: uhc@13430000 {
compatible = "generic-ohci";
reg = <0x13430000 0x1000>;
--
2.11.0


2018-07-21 11:09:31

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 04/18] dmaengine: dma-jz4780: Separate chan/ctrl registers

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 <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 115 ++++++++++++++++++++++++++++++-----------------
1 file changed, 74 insertions(+), 41 deletions(-)

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

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index a26107c85ee7..2f17a0fb1e5c 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
@@ -144,7 +146,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;
enum jz_version version;
@@ -178,16 +181,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(
@@ -482,17 +497,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)
@@ -518,7 +534,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;
@@ -567,8 +583,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;
}

@@ -615,8 +631,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,
@@ -655,7 +671,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)))
@@ -665,12 +681,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;
}
@@ -809,9 +825,26 @@ 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 if (version == ID_JZ4780) {
+ /*
+ * 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);
if (ret < 0) {
@@ -869,9 +902,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);

--
2.11.0


2018-07-21 11:09:36

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 15/18] dmaengine: dma-jz4780: Use dma_set_residue()

From: Daniel Silsby <[email protected]>

This is the standard method provided by dmaengine header.

Signed-off-by: Daniel Silsby <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

v2: No change

v3: No change

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index b9db539a5b34..ea17886031fa 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -637,6 +637,7 @@ static enum dma_status jz4780_dma_tx_status(struct dma_chan *chan,
struct virt_dma_desc *vdesc;
enum dma_status status;
unsigned long flags;
+ unsigned long residue = 0;

status = dma_cookie_status(chan, cookie, txstate);
if ((status == DMA_COMPLETE) || (txstate == NULL))
@@ -647,13 +648,13 @@ static enum dma_status jz4780_dma_tx_status(struct dma_chan *chan,
vdesc = vchan_find_desc(&jzchan->vchan, cookie);
if (vdesc) {
/* On the issued list, so hasn't been processed yet */
- txstate->residue = jz4780_dma_desc_residue(jzchan,
+ residue = jz4780_dma_desc_residue(jzchan,
to_jz4780_dma_desc(vdesc), 0);
} else if (cookie == jzchan->desc->vdesc.tx.cookie) {
- txstate->residue = jz4780_dma_desc_residue(jzchan, jzchan->desc,
+ residue = jz4780_dma_desc_residue(jzchan, jzchan->desc,
jzchan->curr_hwdesc + 1);
- } else
- txstate->residue = 0;
+ }
+ dma_set_residue(txstate, residue);

if (vdesc && jzchan->desc && vdesc == &jzchan->desc->vdesc
&& jzchan->desc->status & (JZ_DMA_DCS_AR | JZ_DMA_DCS_HLT))
--
2.11.0


2018-07-21 11:09:42

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 14/18] dmaengine: dma-jz4780: Further residue status fix

From: Daniel Silsby <[email protected]>

Func jz4780_dma_desc_residue() expects the index to the next hw
descriptor as its last parameter. Caller func jz4780_dma_tx_status(),
however, applied modulus before passing it. When the current hw
descriptor was last in the list, the index passed became zero.

The resulting excess of reported residue especially caused problems
with cyclic DMA transfer clients, i.e. ALSA AIC audio output, which
rely on this for determining current DMA location within buffer.

Combined with the recent and related residue-reporting fixes, spurious
ALSA audio underruns on jz4770 hardware are now fixed.

Signed-off-by: Daniel Silsby <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

v2: No change

v3: No change

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 78849131c81d..b9db539a5b34 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -651,7 +651,7 @@ static enum dma_status jz4780_dma_tx_status(struct dma_chan *chan,
to_jz4780_dma_desc(vdesc), 0);
} else if (cookie == jzchan->desc->vdesc.tx.cookie) {
txstate->residue = jz4780_dma_desc_residue(jzchan, jzchan->desc,
- (jzchan->curr_hwdesc + 1) % jzchan->desc->count);
+ jzchan->curr_hwdesc + 1);
} else
txstate->residue = 0;

--
2.11.0


2018-07-21 11:09:48

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 13/18] dmaengine: dma-jz4780: Set DTCn register explicitly

From: Daniel Silsby <[email protected]>

Normally, we wouldn't set the channel transfer count register directly
when using descriptor-driven transfers. However, there is no harm in
doing so, and it allows jz4780_dma_desc_residue() to report the correct
residue of an ongoing transfer, no matter when it is called.

Signed-off-by: Daniel Silsby <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 9 +++++++++
1 file changed, 9 insertions(+)

v2: No change

v3: No change

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index cc2a86844db4..78849131c81d 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -530,6 +530,15 @@ static void jz4780_dma_begin(struct jz4780_dma_chan *jzchan)
jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DRT,
jzchan->transfer_type);

+ /*
+ * Set the transfer count. This is redundant for a descriptor-driven
+ * transfer. However, there can be a delay between the transfer start
+ * time and when DTCn reg contains the new transfer count. Setting
+ * it explicitly ensures residue is computed correctly at all times.
+ */
+ jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DTC,
+ jzchan->desc->desc[jzchan->curr_hwdesc].dtc);
+
/* Write descriptor address and initiate descriptor fetch. */
desc_phys = jzchan->desc->desc_phys +
(jzchan->curr_hwdesc * sizeof(*jzchan->desc->desc));
--
2.11.0


2018-07-21 11:09:58

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 10/18] dmaengine: dma-jz4780: Enable Fast DMA to the AIC

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 <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
Reviewed-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/dma/dma-jz4780.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

v2: No change

v3: No change

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 88ce3f0157f6..3c9d3952e23a 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
@@ -941,8 +942,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 (jzdma->version == ID_JZ4780)
jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);
--
2.11.0


2018-07-21 11:10:08

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 11/18] dmaengine: dma-jz4780: Add missing residue DTC mask

From: Daniel Silsby <[email protected]>

The 'dtc' word in jz DMA descriptors contains two fields: The
lowest 24 bits are the transfer count, and upper 8 bits are the DOA
offset to next descriptor. The upper 8 bits are now correctly masked
off when computing residue in jz4780_dma_desc_residue(). Note that
reads of the DTCn hardware reg are automatically masked this way.

Signed-off-by: Daniel Silsby <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

v2: No change

v3: No change

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 3c9d3952e23a..fa926de082ba 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -614,7 +614,8 @@ static size_t jz4780_dma_desc_residue(struct jz4780_dma_chan *jzchan,
residue = 0;

for (i = next_sg; i < desc->count; i++)
- residue += desc->desc[i].dtc << jzchan->transfer_shift;
+ residue += (desc->desc[i].dtc & 0xffffff) <<
+ jzchan->transfer_shift;

if (next_sg != 0) {
count = jz4780_dma_chn_readl(jzdma, jzchan->id,
--
2.11.0


2018-07-21 11:10:15

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 09/18] dmaengine: dma-jz4780: Add support for the JZ4725B SoC

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 <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
Reviewed-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/dma/dma-jz4780.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

v2: - Add comments about channel enabling/disabling
- The documentation update is now in patch 01/17

v3: No change

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 084d4023637e..88ce3f0157f6 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -136,6 +136,7 @@ struct jz4780_dma_chan {

enum jz_version {
ID_JZ4740,
+ ID_JZ4725B,
ID_JZ4770,
ID_JZ4780,
};
@@ -209,8 +210,12 @@ 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->version == ID_JZ4770)
+ if (jzdma->version == ID_JZ4770) {
jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKES, BIT(chn));
+ } else if (jzdma->version == ID_JZ4725B) {
+ /* JZ4725B has no DCKES, it uses DCKE to enable channels */
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKE, BIT(chn));
+ }
}

static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma,
@@ -218,6 +223,8 @@ static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma,
{
if (jzdma->version == ID_JZ4770)
jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn));
+
+ /* On JZ4725B it is not possible to stop a DMA channel once enabled */
}

static struct jz4780_dma_desc *jz4780_dma_desc_alloc(
@@ -805,12 +812,14 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,

static const struct jz4780_dma_soc_data jz4780_dma_soc_data[] = {
[ID_JZ4740] = { .nb_channels = 6, .transfer_ord_max = 5, },
+ [ID_JZ4725B] = { .nb_channels = 6, .transfer_ord_max = 5, },
[ID_JZ4770] = { .nb_channels = 6, .transfer_ord_max = 6, },
[ID_JZ4780] = { .nb_channels = 32, .transfer_ord_max = 7, },
};

static const struct of_device_id jz4780_dma_dt_match[] = {
{ .compatible = "ingenic,jz4740-dma", .data = (void *)ID_JZ4740 },
+ { .compatible = "ingenic,jz4725b-dma", .data = (void *)ID_JZ4725B },
{ .compatible = "ingenic,jz4770-dma", .data = (void *)ID_JZ4770 },
{ .compatible = "ingenic,jz4780-dma", .data = (void *)ID_JZ4780 },
{},
--
2.11.0


2018-07-21 11:10:22

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 12/18] dmaengine: dma-jz4780: Simplify jz4780_dma_desc_residue()

From: Daniel Silsby <[email protected]>

Simple cleanup, no changes to actual logic here.

Signed-off-by: Daniel Silsby <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
---
drivers/dma/dma-jz4780.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)

v2: No change

v3: No change

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index fa926de082ba..cc2a86844db4 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -608,22 +608,17 @@ static size_t jz4780_dma_desc_residue(struct jz4780_dma_chan *jzchan,
struct jz4780_dma_desc *desc, unsigned int next_sg)
{
struct jz4780_dma_dev *jzdma = jz4780_dma_chan_parent(jzchan);
- unsigned int residue, count;
+ unsigned int count = 0;
unsigned int i;

- residue = 0;
-
for (i = next_sg; i < desc->count; i++)
- residue += (desc->desc[i].dtc & 0xffffff) <<
- jzchan->transfer_shift;
+ count += desc->desc[i].dtc & 0xffffff;

- if (next_sg != 0) {
- count = jz4780_dma_chn_readl(jzdma, jzchan->id,
+ if (next_sg != 0)
+ count += jz4780_dma_chn_readl(jzdma, jzchan->id,
JZ_DMA_REG_DTC);
- residue += count << jzchan->transfer_shift;
- }

- return residue;
+ return count << jzchan->transfer_shift;
}

static enum dma_status jz4780_dma_tx_status(struct dma_chan *chan,
--
2.11.0


2018-07-21 11:10:26

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 08/18] dmaengine: dma-jz4780: Add support for the JZ4740 SoC

The JZ4740 SoC has a single DMA core starring six DMA channels.

Signed-off-by: Paul Cercueil <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
Reviewed-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/dma/dma-jz4780.c | 3 +++
1 file changed, 3 insertions(+)

v2: The documentation update is now in patch 01/17

v3: The Kconfig update was dropped thanks to patch 06/18

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index a5f4a8d54516..084d4023637e 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -135,6 +135,7 @@ struct jz4780_dma_chan {
};

enum jz_version {
+ ID_JZ4740,
ID_JZ4770,
ID_JZ4780,
};
@@ -803,11 +804,13 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
}

static const struct jz4780_dma_soc_data jz4780_dma_soc_data[] = {
+ [ID_JZ4740] = { .nb_channels = 6, .transfer_ord_max = 5, },
[ID_JZ4770] = { .nb_channels = 6, .transfer_ord_max = 6, },
[ID_JZ4780] = { .nb_channels = 32, .transfer_ord_max = 7, },
};

static const struct of_device_id jz4780_dma_dt_match[] = {
+ { .compatible = "ingenic,jz4740-dma", .data = (void *)ID_JZ4740 },
{ .compatible = "ingenic,jz4770-dma", .data = (void *)ID_JZ4770 },
{ .compatible = "ingenic,jz4780-dma", .data = (void *)ID_JZ4780 },
{},
--
2.11.0


2018-07-21 11:11:32

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 06/18] dmaengine: dma-jz4780: Don't depend on MACH_JZ4780

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 <[email protected]>
---
drivers/dma/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

v3: New patch

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index ca1680afa20a..0680e1eb0d73 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
--
2.11.0


2018-07-21 11:11:38

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 05/18] dmaengine: dma-jz4780: Use 4-word descriptors

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 <[email protected]>
Tested-by: Mathieu Malaterre <[email protected]>
Reviewed-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/dma/dma-jz4780.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)

v2: No change

v3: No change

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 2f17a0fb1e5c..23e92d153919 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. */
@@ -286,7 +281,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;
@@ -294,7 +288,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;
@@ -439,9 +432,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 |
@@ -496,9 +490,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 +
@@ -508,7 +505,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)
--
2.11.0


2018-07-21 15:11:25

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v3 13/18] dmaengine: dma-jz4780: Set DTCn register explicitly

On 07/21/2018 04:06 AM, Paul Cercueil wrote:
> From: Daniel Silsby <[email protected]>
>
> Normally, we wouldn't set the channel transfer count register directly
> when using descriptor-driven transfers. However, there is no harm in
> doing so, and it allows jz4780_dma_desc_residue() to report the correct
> residue of an ongoing transfer, no matter when it is called.
>
> Signed-off-by: Daniel Silsby <[email protected]>
> Tested-by: Mathieu Malaterre <[email protected]>
> ---
> drivers/dma/dma-jz4780.c | 9 +++++++++
> 1 file changed, 9 insertions(+)

Hi,

Documentation/process/submitting-patches.rst says:

The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path.

That means that patches that are from Daniel but you send (delivery path)
should also be Signed-off-by: you.


--
~Randy

2018-07-23 18:00:56

by Paul Burton

[permalink] [raw]
Subject: Re: [PATCH v3 00/18] JZ4780 DMA patchset v3

Hi Paul & Vinod,

On Sat, Jul 21, 2018 at 01:06:25PM +0200, Paul Cercueil wrote:
> This is the version 3 of my jz4780-dma driver update patchset.
>
> Apologies to the DMA people, the v2 of this patchset did not make it to
> their mailing-list; see the bottom of this email for a description of
> what happened in v2.
>
> Changelog from v2 to v3:
>
> - Modified the devicetree bindings to comply with the specification
>
> - New patch [06/18] allows the JZ4780 DMA driver to be compiled within a
> generic MIPS kernel.

Would you prefer to take the MIPS .dts changes in patches 16-18 through
the DMA tree with the rest of the series?

If so then for patches 16-18:

Acked-by: Paul Burton <[email protected]>

Thanks,
Paul

2018-07-24 11:11:27

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v3 00/18] JZ4780 DMA patchset v3

Hi,

Le lun. 23 juil. 2018 ? 19:58, Paul Burton <[email protected]> a
?crit :
> Hi Paul & Vinod,
>
> On Sat, Jul 21, 2018 at 01:06:25PM +0200, Paul Cercueil wrote:
>> This is the version 3 of my jz4780-dma driver update patchset.
>>
>> Apologies to the DMA people, the v2 of this patchset did not make
>> it to
>> their mailing-list; see the bottom of this email for a description
>> of
>> what happened in v2.
>>
>> Changelog from v2 to v3:
>>
>> - Modified the devicetree bindings to comply with the specification
>>
>> - New patch [06/18] allows the JZ4780 DMA driver to be compiled
>> within a
>> generic MIPS kernel.
>
> Would you prefer to take the MIPS .dts changes in patches 16-18
> through
> the DMA tree with the rest of the series?

I think it would make sense yes.

> If so then for patches 16-18:
>
> Acked-by: Paul Burton <[email protected]>
>
> Thanks,
> Paul


2018-07-24 13:12:45

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v3 00/18] JZ4780 DMA patchset v3

On 24-07-18, 13:09, Paul Cercueil wrote:
> Hi,
>
> Le lun. 23 juil. 2018 ? 19:58, Paul Burton <[email protected]> a ?crit :
> > Hi Paul & Vinod,
> >
> > On Sat, Jul 21, 2018 at 01:06:25PM +0200, Paul Cercueil wrote:
> > > This is the version 3 of my jz4780-dma driver update patchset.
> > >
> > > Apologies to the DMA people, the v2 of this patchset did not make
> > > it to
> > > their mailing-list; see the bottom of this email for a description
> > > of
> > > what happened in v2.
> > >
> > > Changelog from v2 to v3:
> > >
> > > - Modified the devicetree bindings to comply with the specification
> > >
> > > - New patch [06/18] allows the JZ4780 DMA driver to be compiled
> > > within a
> > > generic MIPS kernel.
> >
> > Would you prefer to take the MIPS .dts changes in patches 16-18 through
> > the DMA tree with the rest of the series?
>
> I think it would make sense yes.

okay will do so when the series is merged

--
~Vinod

2018-07-24 13:24:13

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v3 03/18] dmaengine: dma-jz4780: Avoid hardcoding number of channels

On 21-07-18, 13:06, Paul Cercueil wrote:

> +static const struct jz4780_dma_soc_data jz4780_dma_soc_data[] = {
> + [ID_JZ4780] = { .nb_channels = 32, },

why the array of structs?

> +};
> +
> +static const struct of_device_id jz4780_dma_dt_match[] = {
> + { .compatible = "ingenic,jz4780-dma", .data = (void *)ID_JZ4780 },

the data should be jz4780_dma_soc_data? as you would add more data
later.. and not the enum..

> - jzdma = devm_kzalloc(dev, sizeof(*jzdma), GFP_KERNEL);
> + version = (enum jz_version)of_id->data;
> + soc_data = &jz4780_dma_soc_data[version];

this can be simplified if we do:

soc_data = device_get_match_data(pdev);

with:

static const struct jz4780_dma_soc_data jz4780_dma_soc_data = {
.nb_channels = 32,
};

and
{ .compatible = "ingenic,jz4780-dma", .data = (void *)jz4780_dma_soc_data },

You add more parameters in future patches and store soc_data in driver
object and use as is..

> + jzdma = devm_kzalloc(dev, sizeof(*jzdma)
> + + sizeof(*jzdma->chan) * soc_data->nb_channels,
> + GFP_KERNEL);
> if (!jzdma)
> return -ENOMEM;
>
> + jzdma->soc_data = soc_data;
> + jzdma->version = version;

why do you need to store version, driver should handle values and not
versions..

--
~Vinod

2018-07-24 13:34:09

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v3 07/18] dmaengine: dma-jz4780: Add support for the JZ4770 SoC

On 21-07-18, 13:06, Paul Cercueil wrote:
> +static inline void jz4780_dma_chan_enable(struct jz4780_dma_dev *jzdma,
> + unsigned int chn)

right justified and aligned with preceding please. While adding new
code to a existing driver it is a good idea to conform to existing style

> +{
> + if (jzdma->version == ID_JZ4770)
> + 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->version == ID_JZ4770)
> + jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn));

so if another version has this feature we would do:
if (jzdma->version == ID_JZ4770) ||
if (jzdma->version == ID_JZXXXX))

and so on.. why not add a value, clk_enable in the description and use
that. For each controller it is set to true or false

--
~Vinod

2018-07-24 13:40:06

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v3 11/18] dmaengine: dma-jz4780: Add missing residue DTC mask

On 21-07-18, 13:06, Paul Cercueil wrote:
> From: Daniel Silsby <[email protected]>
>
> The 'dtc' word in jz DMA descriptors contains two fields: The
> lowest 24 bits are the transfer count, and upper 8 bits are the DOA
> offset to next descriptor. The upper 8 bits are now correctly masked
> off when computing residue in jz4780_dma_desc_residue(). Note that
> reads of the DTCn hardware reg are automatically masked this way.
>
> Signed-off-by: Daniel Silsby <[email protected]>
> Tested-by: Mathieu Malaterre <[email protected]>

This needs your s-o-b. Please see Documentation/process/submitting-patches.rst

I think Randy did flag this one some other patch as well. All the
patches need to be signed off by sender as well

--
~Vinod

2018-07-24 15:06:50

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v3 07/18] dmaengine: dma-jz4780: Add support for the JZ4770 SoC

Hi Vinod,

Le mar. 24 juil. 2018 ? 15:32, Vinod <[email protected]> a ?crit :
> On 21-07-18, 13:06, Paul Cercueil wrote:
>> +static inline void jz4780_dma_chan_enable(struct jz4780_dma_dev
>> *jzdma,
>> + unsigned int chn)
>
> right justified and aligned with preceding please. While adding new
> code to a existing driver it is a good idea to conform to existing
> style

OK.

>> +{
>> + if (jzdma->version == ID_JZ4770)
>> + 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->version == ID_JZ4770)
>> + jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn));
>
> so if another version has this feature we would do:
> if (jzdma->version == ID_JZ4770) ||
> if (jzdma->version == ID_JZXXXX))
>
> and so on.. why not add a value, clk_enable in the description and use
> that. For each controller it is set to true or false

I agree with what you said in your other answers.
However here I still need to check the "version", because on JZ4725B
and JZ4770+
the way to start/stop each DMA channel's clock is different, so I can't
use a boolean.

> --
> ~Vinod

Thanks,
-Paul


2018-07-24 16:03:47

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v3 07/18] dmaengine: dma-jz4780: Add support for the JZ4770 SoC

On 24-07-18, 17:04, Paul Cercueil wrote:
> Hi Vinod,
>
> Le mar. 24 juil. 2018 ? 15:32, Vinod <[email protected]> a ?crit :
> > On 21-07-18, 13:06, Paul Cercueil wrote:
> > > +static inline void jz4780_dma_chan_enable(struct jz4780_dma_dev
> > > *jzdma,
> > > + unsigned int chn)
> >
> > right justified and aligned with preceding please. While adding new
> > code to a existing driver it is a good idea to conform to existing style
>
> OK.
>
> > > +{
> > > + if (jzdma->version == ID_JZ4770)
> > > + 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->version == ID_JZ4770)
> > > + jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn));
> >
> > so if another version has this feature we would do:
> > if (jzdma->version == ID_JZ4770) ||
> > if (jzdma->version == ID_JZXXXX))
> >
> > and so on.. why not add a value, clk_enable in the description and use
> > that. For each controller it is set to true or false
>
> I agree with what you said in your other answers.
> However here I still need to check the "version", because on JZ4725B and
> JZ4770+
> the way to start/stop each DMA channel's clock is different, so I can't use
> a boolean

sure describe the behavior and use that. Versions is not a very scalable
way..

--
~Vinod

2018-07-24 23:37:13

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 01/18] doc: dt-bindings: jz4780-dma: Update bindings to reflect driver changes

On Sat, Jul 21, 2018 at 01:06:26PM +0200, Paul Cercueil wrote:
> The driver is now compatible with four SoCs: JZ4780, JZ4770, JZ4725B and
> JZ4740.

What the driver supports is irrelevant to the binding.
>
> Besides, it 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 <[email protected]>
> Tested-by: Mathieu Malaterre <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/jz4780-dma.txt | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> 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
>
> diff --git a/Documentation/devicetree/bindings/dma/jz4780-dma.txt b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
> index f25feee62b15..14f33305e194 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

So none of these are compatible with each other? It should be one valid
combination per line.

> +- 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.
> - interrupt-parent: Should be the phandle of the interrupt controller that
> - clocks: Should contain a clock specifier for the JZ4780 PDMA clock.
> @@ -20,9 +25,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>;
> --
> 2.11.0
>

2018-07-30 21:15:15

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v3 01/18] doc: dt-bindings: jz4780-dma: Update bindings to reflect driver changes

Hi,

Le mer. 25 juil. 2018 ? 1:35, Rob Herring <[email protected]> a ?crit :
> On Sat, Jul 21, 2018 at 01:06:26PM +0200, Paul Cercueil wrote:
>> The driver is now compatible with four SoCs: JZ4780, JZ4770,
>> JZ4725B and
>> JZ4740.
>
> What the driver supports is irrelevant to the binding.

That's just informative. But I can remove it, no problem.

>>
>> Besides, it 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 <[email protected]>
>> Tested-by: Mathieu Malaterre <[email protected]>
>> ---
>> Documentation/devicetree/bindings/dma/jz4780-dma.txt | 14
>> ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> 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
>>
>> diff --git a/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>> b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>> index f25feee62b15..14f33305e194 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
>
> So none of these are compatible with each other? It should be one
> valid
> combination per line.

That's correct - they are all slightly different.

>> +- 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.
>> - interrupt-parent: Should be the phandle of the interrupt
>> controller that
>> - clocks: Should contain a clock specifier for the JZ4780 PDMA
>> clock.
>> @@ -20,9 +25,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>;
>> --
>> 2.11.0
>>


2018-08-04 09:23:43

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v3 07/18] dmaengine: dma-jz4780: Add support for the JZ4770 SoC

Hi Vinod,

Le mar. 24 juil. 2018 ? 15:32, Vinod <[email protected]> a ?crit :
> On 21-07-18, 13:06, Paul Cercueil wrote:
>> +static inline void jz4780_dma_chan_enable(struct jz4780_dma_dev
>> *jzdma,
>> + unsigned int chn)
>
> right justified and aligned with preceding please. While adding new
> code to a existing driver it is a good idea to conform to existing
> style

Well that's exactly what I did, this is the style used in the DMA
driver,
so I tried to conform to it.

>> +{
>> + if (jzdma->version == ID_JZ4770)
>> + 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->version == ID_JZ4770)
>> + jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn));
>
> so if another version has this feature we would do:
> if (jzdma->version == ID_JZ4770) ||
> if (jzdma->version == ID_JZXXXX))
>
> and so on.. why not add a value, clk_enable in the description and use
> that. For each controller it is set to true or false
>
> --
> ~Vinod