[PATCH 0/5] dmaengine: dma-jz4780: Driver updates

[PATCH 0/5] dmaengine: dma-jz4780: Driver updates

[Paul Cercueil]

Hi Vinod,

A small set of updates to the dma-jz4780 driver.

It adds support for the MDMA/BDMA engines in the JZ4760(B) and JZ4770
SoCs, which are just regular cores with less channels.

It also adds support for bidirectional channels, so that devices that
only do half-duplex transfers can request a single DMA channel for both
directions.

Cheers,
-Paul

Paul Cercueil (5):
  dt-bindings: dma: ingenic: Add compatible strings for MDMA and BDMA
  dt-bindings: dma: ingenic: Support #dma-cells = <3>
  dmaengine: jz4780: Work around hardware bug on JZ4760 SoCs
  dmaengine: jz4780: Add support for the MDMA and BDMA in the JZ4760(B)
  dmaengine: jz4780: Support bidirectional I/O on one channel

 .../devicetree/bindings/dma/ingenic,dma.yaml  | 34 +++++---
 drivers/dma/dma-jz4780.c                      | 84 +++++++++++++++----
 2 files changed, 91 insertions(+), 27 deletions(-)

-- 
2.33.0

[PATCH 1/5] dt-bindings: dma: ingenic: Add compatible strings for MDMA and BDMA

[Paul Cercueil]

The JZ4760 and JZ4760B SoCs have two additional DMA controllers: the
MDMA, which only supports memcpy operations, and the BDMA which is
mostly used for transfer between memories and the BCH controller.
The JZ4770 also features the same BDMA as in the JZ4760B, but does not
seem to have a MDMA.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 .../devicetree/bindings/dma/ingenic,dma.yaml  | 26 ++++++++++++-------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/ingenic,dma.yaml b/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
index ac4d59494fc8..f45fd5235879 100644
--- a/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
+++ b/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
@@ -14,15 +14,23 @@ allOf:
 
 properties:
   compatible:
-    enum:
-      - ingenic,jz4740-dma
-      - ingenic,jz4725b-dma
-      - ingenic,jz4760-dma
-      - ingenic,jz4760b-dma
-      - ingenic,jz4770-dma
-      - ingenic,jz4780-dma
-      - ingenic,x1000-dma
-      - ingenic,x1830-dma
+    oneOf:
+      - enum:
+        - ingenic,jz4740-dma
+        - ingenic,jz4725b-dma
+        - ingenic,jz4760-dma
+        - ingenic,jz4760-bdma
+        - ingenic,jz4760-mdma
+        - ingenic,jz4760b-dma
+        - ingenic,jz4760b-bdma
+        - ingenic,jz4760b-mdma
+        - ingenic,jz4770-dma
+        - ingenic,jz4780-dma
+        - ingenic,x1000-dma
+        - ingenic,x1830-dma
+      - items:
+        - const: ingenic,jz4770-bdma
+        - const: ingenic,jz4760b-bdma
 
   reg:
     items:
-- 
2.33.0

Re: [PATCH 1/5] dt-bindings: dma: ingenic: Add compatible strings for MDMA and BDMA

[Rob Herring]

On Mon, 11 Oct 2021 16:36:48 +0200, Paul Cercueil wrote:
> The JZ4760 and JZ4760B SoCs have two additional DMA controllers: the
> MDMA, which only supports memcpy operations, and the BDMA which is
> mostly used for transfer between memories and the BCH controller.
> The JZ4770 also features the same BDMA as in the JZ4760B, but does not
> seem to have a MDMA.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  .../devicetree/bindings/dma/ingenic,dma.yaml  | 26 ++++++++++++-------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/dma/ingenic,dma.yaml:19:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/dma/ingenic,dma.yaml:32:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1539355

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 1/5] dt-bindings: dma: ingenic: Add compatible strings for MDMA and BDMA

[Rob Herring]

On Mon, 11 Oct 2021 16:36:48 +0200, Paul Cercueil wrote:
> The JZ4760 and JZ4760B SoCs have two additional DMA controllers: the
> MDMA, which only supports memcpy operations, and the BDMA which is
> mostly used for transfer between memories and the BCH controller.
> The JZ4770 also features the same BDMA as in the JZ4760B, but does not
> seem to have a MDMA.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  .../devicetree/bindings/dma/ingenic,dma.yaml  | 26 ++++++++++++-------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 

With the indentation fixed:

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/5] dt-bindings: dma: ingenic: Add compatible strings for MDMA and BDMA

[Paul Cercueil]

Hi,

Le lun., oct. 11 2021 at 19:48:44 -0500, Rob Herring <robh@kernel.org> 
a écrit :
> On Mon, 11 Oct 2021 16:36:48 +0200, Paul Cercueil wrote:
>>  The JZ4760 and JZ4760B SoCs have two additional DMA controllers: the
>>  MDMA, which only supports memcpy operations, and the BDMA which is
>>  mostly used for transfer between memories and the BCH controller.
>>  The JZ4770 also features the same BDMA as in the JZ4760B, but does 
>> not
>>  seem to have a MDMA.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   .../devicetree/bindings/dma/ingenic,dma.yaml  | 26 
>> ++++++++++++-------
>>   1 file changed, 17 insertions(+), 9 deletions(-)
>> 
> 
> With the indentation fixed:
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Thanks Rob, I'll V2 then.

Could you have a look at patch 2/5 too? It touches the dt-bindings as 
well.

Cheers,
-Paul

[PATCH 2/5] dt-bindings: dma: ingenic: Support #dma-cells = <3>

[Paul Cercueil]

Extend the binding to support specifying a different request type for
each direction.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 Documentation/devicetree/bindings/dma/ingenic,dma.yaml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/ingenic,dma.yaml b/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
index f45fd5235879..51b41e4795a2 100644
--- a/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
+++ b/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
@@ -44,13 +44,17 @@ properties:
     maxItems: 1
 
   "#dma-cells":
-    const: 2
+    enum: [2, 3]
     description: >
       DMA clients must use the format described in dma.txt, giving a phandle
-      to the DMA controller plus the following 2 integer cells:
+      to the DMA controller plus the following integer cells:
 
       - Request type: The DMA request type for transfers to/from the
         device on the allocated channel, as defined in the SoC documentation.
+        If "#dma-cells" is 2, the request type is a single cell. If
+        "#dma-cells" is 3, the request type has two cells; the first one
+        corresponds to the host to device direction, the second one corresponds
+        to the device to host direction.
 
       - Channel: If set to 0xffffffff, any available channel will be allocated
         for the client. Otherwise, the exact channel specified will be used.
-- 
2.33.0

Re: [PATCH 2/5] dt-bindings: dma: ingenic: Support #dma-cells = <3>

[Vinod Koul]

On 11-10-21, 16:36, Paul Cercueil wrote:
> Extend the binding to support specifying a different request type for
> each direction.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  Documentation/devicetree/bindings/dma/ingenic,dma.yaml | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/ingenic,dma.yaml b/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
> index f45fd5235879..51b41e4795a2 100644
> --- a/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
> +++ b/Documentation/devicetree/bindings/dma/ingenic,dma.yaml
> @@ -44,13 +44,17 @@ properties:
>      maxItems: 1
>  
>    "#dma-cells":
> -    const: 2
> +    enum: [2, 3]
>      description: >
>        DMA clients must use the format described in dma.txt, giving a phandle
> -      to the DMA controller plus the following 2 integer cells:
> +      to the DMA controller plus the following integer cells:
>  
>        - Request type: The DMA request type for transfers to/from the
>          device on the allocated channel, as defined in the SoC documentation.
> +        If "#dma-cells" is 2, the request type is a single cell. If
> +        "#dma-cells" is 3, the request type has two cells; the first one
> +        corresponds to the host to device direction, the second one corresponds
> +        to the device to host direction.

Why would you need the direction here, that should be a runtime
parameter and not a DT one?

>  
>        - Channel: If set to 0xffffffff, any available channel will be allocated
>          for the client. Otherwise, the exact channel specified will be used.
> -- 
> 2.33.0

-- 
~Vinod

[PATCH 3/5] dmaengine: jz4780: Work around hardware bug on JZ4760 SoCs

[Paul Cercueil]

The JZ4760 SoC has a hardware problem with chan0 not enabling properly
if it's enabled before chan1, after a reset (works fine afterwards).
This is worked around in the probe function by just enabling then
disabling chan1.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/dma/dma-jz4780.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index ebee94dbd630..d71bc7235959 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -937,6 +937,14 @@ static int jz4780_dma_probe(struct platform_device *pdev)
 		jzchan->vchan.desc_free = jz4780_dma_desc_free;
 	}
 
+	/*
+	 * On JZ4760, chan0 won't enable properly the first time.
+	 * Enabling then disabling chan1 will magically make chan0 work
+	 * correctly.
+	 */
+	jz4780_dma_chan_enable(jzdma, 1);
+	jz4780_dma_chan_disable(jzdma, 1);
+
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0)
 		goto err_disable_clk;
-- 
2.33.0

[PATCH 4/5] dmaengine: jz4780: Add support for the MDMA and BDMA in the JZ4760(B)

[Paul Cercueil]

The JZ4760 and JZ4760B SoCs have two regular DMA controllers with 6
channels each. They also have an extra DMA controller named MDMA
with only 2 channels, that only supports memcpy operations, and one
named BDMA with only 3 channels, that is mostly used for transfers
between memories and the BCH controller.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/dma/dma-jz4780.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index d71bc7235959..4d62e24ebff9 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -1018,12 +1018,36 @@ static const struct jz4780_dma_soc_data jz4760_dma_soc_data = {
 	.flags = JZ_SOC_DATA_PER_CHAN_PM | JZ_SOC_DATA_NO_DCKES_DCKEC,
 };
 
+static const struct jz4780_dma_soc_data jz4760_mdma_soc_data = {
+	.nb_channels = 2,
+	.transfer_ord_max = 6,
+	.flags = JZ_SOC_DATA_PER_CHAN_PM | JZ_SOC_DATA_NO_DCKES_DCKEC,
+};
+
+static const struct jz4780_dma_soc_data jz4760_bdma_soc_data = {
+	.nb_channels = 3,
+	.transfer_ord_max = 6,
+	.flags = JZ_SOC_DATA_PER_CHAN_PM | JZ_SOC_DATA_NO_DCKES_DCKEC,
+};
+
 static const struct jz4780_dma_soc_data jz4760b_dma_soc_data = {
 	.nb_channels = 5,
 	.transfer_ord_max = 6,
 	.flags = JZ_SOC_DATA_PER_CHAN_PM,
 };
 
+static const struct jz4780_dma_soc_data jz4760b_mdma_soc_data = {
+	.nb_channels = 2,
+	.transfer_ord_max = 6,
+	.flags = JZ_SOC_DATA_PER_CHAN_PM,
+};
+
+static const struct jz4780_dma_soc_data jz4760b_bdma_soc_data = {
+	.nb_channels = 3,
+	.transfer_ord_max = 6,
+	.flags = JZ_SOC_DATA_PER_CHAN_PM,
+};
+
 static const struct jz4780_dma_soc_data jz4770_dma_soc_data = {
 	.nb_channels = 6,
 	.transfer_ord_max = 6,
@@ -1052,7 +1076,11 @@ 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,jz4760-dma", .data = &jz4760_dma_soc_data },
+	{ .compatible = "ingenic,jz4760-mdma", .data = &jz4760_mdma_soc_data },
+	{ .compatible = "ingenic,jz4760-bdma", .data = &jz4760_bdma_soc_data },
 	{ .compatible = "ingenic,jz4760b-dma", .data = &jz4760b_dma_soc_data },
+	{ .compatible = "ingenic,jz4760b-mdma", .data = &jz4760b_mdma_soc_data },
+	{ .compatible = "ingenic,jz4760b-bdma", .data = &jz4760b_bdma_soc_data },
 	{ .compatible = "ingenic,jz4770-dma", .data = &jz4770_dma_soc_data },
 	{ .compatible = "ingenic,jz4780-dma", .data = &jz4780_dma_soc_data },
 	{ .compatible = "ingenic,x1000-dma", .data = &x1000_dma_soc_data },
-- 
2.33.0

[PATCH 5/5] dmaengine: jz4780: Support bidirectional I/O on one channel

[Paul Cercueil]

For some devices with only half-duplex capabilities, it doesn't make
much sense to use one DMA channel per direction, as both channels will
never be active at the same time.

Add support for bidirectional I/O on DMA channels. The client drivers
can then request a "tx-rx" DMA channel which will be used for both
directions.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/dma/dma-jz4780.c | 48 ++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 4d62e24ebff9..ee1d50792c32 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -122,6 +122,7 @@ struct jz4780_dma_desc {
 	dma_addr_t desc_phys;
 	unsigned int count;
 	enum dma_transaction_type type;
+	uint32_t transfer_type;
 	uint32_t status;
 };
 
@@ -130,7 +131,7 @@ struct jz4780_dma_chan {
 	unsigned int id;
 	struct dma_pool *desc_pool;
 
-	uint32_t transfer_type;
+	uint32_t transfer_type_tx, transfer_type_rx;
 	uint32_t transfer_shift;
 	struct dma_slave_config	config;
 
@@ -157,7 +158,7 @@ struct jz4780_dma_dev {
 };
 
 struct jz4780_dma_filter_data {
-	uint32_t transfer_type;
+	uint32_t transfer_type_tx, transfer_type_rx;
 	int channel;
 };
 
@@ -226,9 +227,10 @@ static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma,
 		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)
+static struct jz4780_dma_desc *
+jz4780_dma_desc_alloc(struct jz4780_dma_chan *jzchan, unsigned int count,
+		      enum dma_transaction_type type,
+		      enum dma_transfer_direction direction)
 {
 	struct jz4780_dma_desc *desc;
 
@@ -248,6 +250,12 @@ static struct jz4780_dma_desc *jz4780_dma_desc_alloc(
 
 	desc->count = count;
 	desc->type = type;
+
+	if (direction == DMA_DEV_TO_MEM)
+		desc->transfer_type = jzchan->transfer_type_rx;
+	else
+		desc->transfer_type = jzchan->transfer_type_tx;
+
 	return desc;
 }
 
@@ -361,7 +369,7 @@ static struct dma_async_tx_descriptor *jz4780_dma_prep_slave_sg(
 	unsigned int i;
 	int err;
 
-	desc = jz4780_dma_desc_alloc(jzchan, sg_len, DMA_SLAVE);
+	desc = jz4780_dma_desc_alloc(jzchan, sg_len, DMA_SLAVE, direction);
 	if (!desc)
 		return NULL;
 
@@ -410,7 +418,7 @@ static struct dma_async_tx_descriptor *jz4780_dma_prep_dma_cyclic(
 
 	periods = buf_len / period_len;
 
-	desc = jz4780_dma_desc_alloc(jzchan, periods, DMA_CYCLIC);
+	desc = jz4780_dma_desc_alloc(jzchan, periods, DMA_CYCLIC, direction);
 	if (!desc)
 		return NULL;
 
@@ -455,14 +463,14 @@ static struct dma_async_tx_descriptor *jz4780_dma_prep_dma_memcpy(
 	struct jz4780_dma_desc *desc;
 	uint32_t tsz;
 
-	desc = jz4780_dma_desc_alloc(jzchan, 1, DMA_MEMCPY);
+	desc = jz4780_dma_desc_alloc(jzchan, 1, DMA_MEMCPY, 0);
 	if (!desc)
 		return NULL;
 
 	tsz = jz4780_dma_transfer_size(jzchan, dest | src | len,
 				       &jzchan->transfer_shift);
 
-	jzchan->transfer_type = JZ_DMA_DRT_AUTO;
+	desc->transfer_type = JZ_DMA_DRT_AUTO;
 
 	desc->desc[0].dsa = src;
 	desc->desc[0].dta = dest;
@@ -528,7 +536,7 @@ static void jz4780_dma_begin(struct jz4780_dma_chan *jzchan)
 
 	/* Set transfer type. */
 	jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DRT,
-			      jzchan->transfer_type);
+			      jzchan->desc->transfer_type);
 
 	/*
 	 * Set the transfer count. This is redundant for a descriptor-driven
@@ -788,7 +796,8 @@ static bool jz4780_dma_filter_fn(struct dma_chan *chan, void *param)
 		return false;
 	}
 
-	jzchan->transfer_type = data->transfer_type;
+	jzchan->transfer_type_tx = data->transfer_type_tx;
+	jzchan->transfer_type_rx = data->transfer_type_rx;
 
 	return true;
 }
@@ -800,11 +809,17 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
 	dma_cap_mask_t mask = jzdma->dma_device.cap_mask;
 	struct jz4780_dma_filter_data data;
 
-	if (dma_spec->args_count != 2)
+	if (dma_spec->args_count == 2) {
+		data.transfer_type_tx = dma_spec->args[0];
+		data.transfer_type_rx = dma_spec->args[0];
+		data.channel = dma_spec->args[1];
+	} else if (dma_spec->args_count == 3) {
+		data.transfer_type_tx = dma_spec->args[0];
+		data.transfer_type_rx = dma_spec->args[1];
+		data.channel = dma_spec->args[2];
+	} else {
 		return NULL;
-
-	data.transfer_type = dma_spec->args[0];
-	data.channel = dma_spec->args[1];
+	}
 
 	if (data.channel > -1) {
 		if (data.channel >= jzdma->soc_data->nb_channels) {
@@ -822,7 +837,8 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
 			return NULL;
 		}
 
-		jzdma->chan[data.channel].transfer_type = data.transfer_type;
+		jzdma->chan[data.channel].transfer_type_tx = data.transfer_type_tx;
+		jzdma->chan[data.channel].transfer_type_rx = data.transfer_type_rx;
 
 		return dma_get_slave_channel(
 			&jzdma->chan[data.channel].vchan.chan);
-- 
2.33.0

Re: [PATCH 5/5] dmaengine: jz4780: Support bidirectional I/O on one channel

[Vinod Koul]

On 11-10-21, 16:36, Paul Cercueil wrote:
> For some devices with only half-duplex capabilities, it doesn't make
> much sense to use one DMA channel per direction, as both channels will
> never be active at the same time.
> 
> Add support for bidirectional I/O on DMA channels. The client drivers
> can then request a "tx-rx" DMA channel which will be used for both
> directions.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/dma/dma-jz4780.c | 48 ++++++++++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
> index 4d62e24ebff9..ee1d50792c32 100644
> --- a/drivers/dma/dma-jz4780.c
> +++ b/drivers/dma/dma-jz4780.c
> @@ -122,6 +122,7 @@ struct jz4780_dma_desc {
>  	dma_addr_t desc_phys;
>  	unsigned int count;
>  	enum dma_transaction_type type;
> +	uint32_t transfer_type;

why not u32?

>  	uint32_t status;
>  };
>  
> @@ -130,7 +131,7 @@ struct jz4780_dma_chan {
>  	unsigned int id;
>  	struct dma_pool *desc_pool;
>  
> -	uint32_t transfer_type;
> +	uint32_t transfer_type_tx, transfer_type_rx;
>  	uint32_t transfer_shift;
>  	struct dma_slave_config	config;
>  
> @@ -157,7 +158,7 @@ struct jz4780_dma_dev {
>  };
>  
>  struct jz4780_dma_filter_data {
> -	uint32_t transfer_type;
> +	uint32_t transfer_type_tx, transfer_type_rx;
>  	int channel;
>  };
>  
> @@ -226,9 +227,10 @@ static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma,
>  		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)
> +static struct jz4780_dma_desc *
> +jz4780_dma_desc_alloc(struct jz4780_dma_chan *jzchan, unsigned int count,
> +		      enum dma_transaction_type type,
> +		      enum dma_transfer_direction direction)
>  {
>  	struct jz4780_dma_desc *desc;
>  
> @@ -248,6 +250,12 @@ static struct jz4780_dma_desc *jz4780_dma_desc_alloc(
>  
>  	desc->count = count;
>  	desc->type = type;
> +
> +	if (direction == DMA_DEV_TO_MEM)
> +		desc->transfer_type = jzchan->transfer_type_rx;
> +	else
> +		desc->transfer_type = jzchan->transfer_type_tx;
> +
>  	return desc;
>  }
>  
> @@ -361,7 +369,7 @@ static struct dma_async_tx_descriptor *jz4780_dma_prep_slave_sg(
>  	unsigned int i;
>  	int err;
>  
> -	desc = jz4780_dma_desc_alloc(jzchan, sg_len, DMA_SLAVE);
> +	desc = jz4780_dma_desc_alloc(jzchan, sg_len, DMA_SLAVE, direction);
>  	if (!desc)
>  		return NULL;
>  
> @@ -410,7 +418,7 @@ static struct dma_async_tx_descriptor *jz4780_dma_prep_dma_cyclic(
>  
>  	periods = buf_len / period_len;
>  
> -	desc = jz4780_dma_desc_alloc(jzchan, periods, DMA_CYCLIC);
> +	desc = jz4780_dma_desc_alloc(jzchan, periods, DMA_CYCLIC, direction);
>  	if (!desc)
>  		return NULL;
>  
> @@ -455,14 +463,14 @@ static struct dma_async_tx_descriptor *jz4780_dma_prep_dma_memcpy(
>  	struct jz4780_dma_desc *desc;
>  	uint32_t tsz;
>  
> -	desc = jz4780_dma_desc_alloc(jzchan, 1, DMA_MEMCPY);
> +	desc = jz4780_dma_desc_alloc(jzchan, 1, DMA_MEMCPY, 0);
>  	if (!desc)
>  		return NULL;
>  
>  	tsz = jz4780_dma_transfer_size(jzchan, dest | src | len,
>  				       &jzchan->transfer_shift);
>  
> -	jzchan->transfer_type = JZ_DMA_DRT_AUTO;
> +	desc->transfer_type = JZ_DMA_DRT_AUTO;
>  
>  	desc->desc[0].dsa = src;
>  	desc->desc[0].dta = dest;
> @@ -528,7 +536,7 @@ static void jz4780_dma_begin(struct jz4780_dma_chan *jzchan)
>  
>  	/* Set transfer type. */
>  	jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DRT,
> -			      jzchan->transfer_type);
> +			      jzchan->desc->transfer_type);
>  
>  	/*
>  	 * Set the transfer count. This is redundant for a descriptor-driven
> @@ -788,7 +796,8 @@ static bool jz4780_dma_filter_fn(struct dma_chan *chan, void *param)
>  		return false;
>  	}
>  
> -	jzchan->transfer_type = data->transfer_type;
> +	jzchan->transfer_type_tx = data->transfer_type_tx;
> +	jzchan->transfer_type_rx = data->transfer_type_rx;
>  
>  	return true;
>  }
> @@ -800,11 +809,17 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
>  	dma_cap_mask_t mask = jzdma->dma_device.cap_mask;
>  	struct jz4780_dma_filter_data data;
>  
> -	if (dma_spec->args_count != 2)
> +	if (dma_spec->args_count == 2) {
> +		data.transfer_type_tx = dma_spec->args[0];
> +		data.transfer_type_rx = dma_spec->args[0];
> +		data.channel = dma_spec->args[1];
> +	} else if (dma_spec->args_count == 3) {
> +		data.transfer_type_tx = dma_spec->args[0];
> +		data.transfer_type_rx = dma_spec->args[1];

aha so you have a different values for tx and rx, that seems okay. Maybe
word a better in binding and also add examples in binding for this

> +		data.channel = dma_spec->args[2];
> +	} else {
>  		return NULL;
> -
> -	data.transfer_type = dma_spec->args[0];
> -	data.channel = dma_spec->args[1];
> +	}
>  
>  	if (data.channel > -1) {
>  		if (data.channel >= jzdma->soc_data->nb_channels) {
> @@ -822,7 +837,8 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
>  			return NULL;
>  		}
>  
> -		jzdma->chan[data.channel].transfer_type = data.transfer_type;
> +		jzdma->chan[data.channel].transfer_type_tx = data.transfer_type_tx;
> +		jzdma->chan[data.channel].transfer_type_rx = data.transfer_type_rx;
>  
>  		return dma_get_slave_channel(
>  			&jzdma->chan[data.channel].vchan.chan);
> -- 
> 2.33.0

-- 
~Vinod

Re: [PATCH 5/5] dmaengine: jz4780: Support bidirectional I/O on one channel

[Paul Cercueil]

Hi Vinod,

Le lun., oct. 18 2021 at 12:03:42 +0530, Vinod Koul <vkoul@kernel.org> 
a écrit :
> On 11-10-21, 16:36, Paul Cercueil wrote:
>>  For some devices with only half-duplex capabilities, it doesn't make
>>  much sense to use one DMA channel per direction, as both channels 
>> will
>>  never be active at the same time.
>> 
>>  Add support for bidirectional I/O on DMA channels. The client 
>> drivers
>>  can then request a "tx-rx" DMA channel which will be used for both
>>  directions.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   drivers/dma/dma-jz4780.c | 48 
>> ++++++++++++++++++++++++++--------------
>>   1 file changed, 32 insertions(+), 16 deletions(-)
>> 
>>  diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
>>  index 4d62e24ebff9..ee1d50792c32 100644
>>  --- a/drivers/dma/dma-jz4780.c
>>  +++ b/drivers/dma/dma-jz4780.c
>>  @@ -122,6 +122,7 @@ struct jz4780_dma_desc {
>>   	dma_addr_t desc_phys;
>>   	unsigned int count;
>>   	enum dma_transaction_type type;
>>  +	uint32_t transfer_type;
> 
> why not u32?

It should be u32 yes. The driver uses uint32_t everywhere so I didn't 
think about it.

> 
>>   	uint32_t status;
>>   };
>> 
>>  @@ -130,7 +131,7 @@ struct jz4780_dma_chan {
>>   	unsigned int id;
>>   	struct dma_pool *desc_pool;
>> 
>>  -	uint32_t transfer_type;
>>  +	uint32_t transfer_type_tx, transfer_type_rx;
>>   	uint32_t transfer_shift;
>>   	struct dma_slave_config	config;
>> 
>>  @@ -157,7 +158,7 @@ struct jz4780_dma_dev {
>>   };
>> 
>>   struct jz4780_dma_filter_data {
>>  -	uint32_t transfer_type;
>>  +	uint32_t transfer_type_tx, transfer_type_rx;
>>   	int channel;
>>   };
>> 
>>  @@ -226,9 +227,10 @@ static inline void 
>> jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma,
>>   		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)
>>  +static struct jz4780_dma_desc *
>>  +jz4780_dma_desc_alloc(struct jz4780_dma_chan *jzchan, unsigned int 
>> count,
>>  +		      enum dma_transaction_type type,
>>  +		      enum dma_transfer_direction direction)
>>   {
>>   	struct jz4780_dma_desc *desc;
>> 
>>  @@ -248,6 +250,12 @@ static struct jz4780_dma_desc 
>> *jz4780_dma_desc_alloc(
>> 
>>   	desc->count = count;
>>   	desc->type = type;
>>  +
>>  +	if (direction == DMA_DEV_TO_MEM)
>>  +		desc->transfer_type = jzchan->transfer_type_rx;
>>  +	else
>>  +		desc->transfer_type = jzchan->transfer_type_tx;
>>  +
>>   	return desc;
>>   }
>> 
>>  @@ -361,7 +369,7 @@ static struct dma_async_tx_descriptor 
>> *jz4780_dma_prep_slave_sg(
>>   	unsigned int i;
>>   	int err;
>> 
>>  -	desc = jz4780_dma_desc_alloc(jzchan, sg_len, DMA_SLAVE);
>>  +	desc = jz4780_dma_desc_alloc(jzchan, sg_len, DMA_SLAVE, 
>> direction);
>>   	if (!desc)
>>   		return NULL;
>> 
>>  @@ -410,7 +418,7 @@ static struct dma_async_tx_descriptor 
>> *jz4780_dma_prep_dma_cyclic(
>> 
>>   	periods = buf_len / period_len;
>> 
>>  -	desc = jz4780_dma_desc_alloc(jzchan, periods, DMA_CYCLIC);
>>  +	desc = jz4780_dma_desc_alloc(jzchan, periods, DMA_CYCLIC, 
>> direction);
>>   	if (!desc)
>>   		return NULL;
>> 
>>  @@ -455,14 +463,14 @@ static struct dma_async_tx_descriptor 
>> *jz4780_dma_prep_dma_memcpy(
>>   	struct jz4780_dma_desc *desc;
>>   	uint32_t tsz;
>> 
>>  -	desc = jz4780_dma_desc_alloc(jzchan, 1, DMA_MEMCPY);
>>  +	desc = jz4780_dma_desc_alloc(jzchan, 1, DMA_MEMCPY, 0);
>>   	if (!desc)
>>   		return NULL;
>> 
>>   	tsz = jz4780_dma_transfer_size(jzchan, dest | src | len,
>>   				       &jzchan->transfer_shift);
>> 
>>  -	jzchan->transfer_type = JZ_DMA_DRT_AUTO;
>>  +	desc->transfer_type = JZ_DMA_DRT_AUTO;
>> 
>>   	desc->desc[0].dsa = src;
>>   	desc->desc[0].dta = dest;
>>  @@ -528,7 +536,7 @@ static void jz4780_dma_begin(struct 
>> jz4780_dma_chan *jzchan)
>> 
>>   	/* Set transfer type. */
>>   	jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DRT,
>>  -			      jzchan->transfer_type);
>>  +			      jzchan->desc->transfer_type);
>> 
>>   	/*
>>   	 * Set the transfer count. This is redundant for a 
>> descriptor-driven
>>  @@ -788,7 +796,8 @@ static bool jz4780_dma_filter_fn(struct 
>> dma_chan *chan, void *param)
>>   		return false;
>>   	}
>> 
>>  -	jzchan->transfer_type = data->transfer_type;
>>  +	jzchan->transfer_type_tx = data->transfer_type_tx;
>>  +	jzchan->transfer_type_rx = data->transfer_type_rx;
>> 
>>   	return true;
>>   }
>>  @@ -800,11 +809,17 @@ static struct dma_chan 
>> *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
>>   	dma_cap_mask_t mask = jzdma->dma_device.cap_mask;
>>   	struct jz4780_dma_filter_data data;
>> 
>>  -	if (dma_spec->args_count != 2)
>>  +	if (dma_spec->args_count == 2) {
>>  +		data.transfer_type_tx = dma_spec->args[0];
>>  +		data.transfer_type_rx = dma_spec->args[0];
>>  +		data.channel = dma_spec->args[1];
>>  +	} else if (dma_spec->args_count == 3) {
>>  +		data.transfer_type_tx = dma_spec->args[0];
>>  +		data.transfer_type_rx = dma_spec->args[1];
> 
> aha so you have a different values for tx and rx, that seems okay. 
> Maybe
> word a better in binding and also add examples in binding for this

Alright.

Cheers,
-Paul

> 
>>  +		data.channel = dma_spec->args[2];
>>  +	} else {
>>   		return NULL;
>>  -
>>  -	data.transfer_type = dma_spec->args[0];
>>  -	data.channel = dma_spec->args[1];
>>  +	}
>> 
>>   	if (data.channel > -1) {
>>   		if (data.channel >= jzdma->soc_data->nb_channels) {
>>  @@ -822,7 +837,8 @@ static struct dma_chan 
>> *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
>>   			return NULL;
>>   		}
>> 
>>  -		jzdma->chan[data.channel].transfer_type = data.transfer_type;
>>  +		jzdma->chan[data.channel].transfer_type_tx = 
>> data.transfer_type_tx;
>>  +		jzdma->chan[data.channel].transfer_type_rx = 
>> data.transfer_type_rx;
>> 
>>   		return dma_get_slave_channel(
>>   			&jzdma->chan[data.channel].vchan.chan);
>>  --
>>  2.33.0
> 
> --
> ~Vinod