Linux-Devicetree Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Add RZ/G2L DMAC support
@ 2021-06-11 11:36 Biju Das
  2021-06-11 11:36 ` [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings Biju Das
  2021-06-11 11:36 ` [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support Biju Das
  0 siblings, 2 replies; 21+ messages in thread
From: Biju Das @ 2021-06-11 11:36 UTC (permalink / raw
  To: Vinod Koul, Rob Herring
  Cc: Biju Das, Chris Brandt, dmaengine, devicetree, Geert Uytterhoeven,
	Chris Paterson, Prabhakar Mahadev Lad, linux-renesas-soc

This patch series add basic support for RZ/G2L DMAC driver.

It is based on the work done by Chris Brandt for RZ/A DMA driver.

This patch set is based on master branch [1]

[1] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/

Biju Das (5):
  dt-bindings: dma: Document RZ/G2L bindings
  drivers: clk: renesas: r9a07g044-cpg: Add DMAC clocks
  drivers: dma: sh: Add DMAC driver for RZ/G2L SoC
  arm64: dts: renesas: r9a07g044: Add DMAC support
  arm64: defconfig: Enable DMA controller for RZ/G2L SoC's

 .../bindings/dma/renesas,rz-dmac.yaml         |  132 +++
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi    |   38 +
 arch/arm64/configs/defconfig                  |    1 +
 drivers/clk/renesas/r9a07g044-cpg.c           |    3 +
 drivers/dma/sh/Kconfig                        |    8 +
 drivers/dma/sh/Makefile                       |    1 +
 drivers/dma/sh/rz-dmac.c                      | 1050 +++++++++++++++++
 7 files changed, 1233 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
 create mode 100644 drivers/dma/sh/rz-dmac.c

-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-11 11:36 [PATCH 0/5] Add RZ/G2L DMAC support Biju Das
@ 2021-06-11 11:36 ` Biju Das
  2021-06-11 17:55   ` Rob Herring
                     ` (2 more replies)
  2021-06-11 11:36 ` [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support Biju Das
  1 sibling, 3 replies; 21+ messages in thread
From: Biju Das @ 2021-06-11 11:36 UTC (permalink / raw
  To: Vinod Koul, Rob Herring
  Cc: Biju Das, Chris Brandt, dmaengine, devicetree, Geert Uytterhoeven,
	Chris Paterson, Prabhakar Mahadev Lad, linux-renesas-soc

Document RZ/G2L DMAC bindings.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../bindings/dma/renesas,rz-dmac.yaml         | 132 ++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml

diff --git a/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
new file mode 100644
index 000000000000..df54bd6ddfd4
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
@@ -0,0 +1,132 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/renesas,rz-dmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L DMA Controller
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+allOf:
+  - $ref: "dma-controller.yaml#"
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
+      - const: renesas,rz-dmac
+
+  reg:
+    items:
+      - description: Control and channel register block
+      - description: DMA extension resource selector block
+
+  interrupts:
+    maxItems: 17
+
+  interrupt-names:
+    maxItems: 17
+    items:
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - pattern: "^ch([0-9]|1[0-5])$"
+      - const: error
+
+  clocks:
+    maxItems: 1
+
+  '#dma-cells':
+    const: 1
+    description:
+      The cell specifies the MID/RID of the DMAC port connected to
+      the DMA client.
+
+  dma-channels:
+    const: 16
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  renesas,rz-dmac-slavecfg:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: |
+      DMA configuration for a slave channel. Each channel must have an array of
+      3 items as below.
+      first item in the array is MID+RID
+      second item in the array is slave src or dst address
+      third item in the array is channel configuration value.
+    items:
+      minItems: 3
+      maxItems: 48
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - '#dma-cells'
+  - dma-channels
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+
+    dmac: dma-controller@11820000 {
+        compatible = "renesas,dmac-r9a07g044",
+                     "renesas,rz-dmac";
+        reg = <0x11820000 0x10000>,
+              <0x11830000 0x10000>;
+        interrupts = <GIC_SPI 125 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 127 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 128 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 130 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 131 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 133 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 135 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 136 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 137 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 138 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 139 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 140 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 141 IRQ_TYPE_EDGE_RISING>;
+        interrupt-names = "ch0", "ch1", "ch2", "ch3",
+                          "ch4", "ch5", "ch6", "ch7",
+                          "ch8", "ch9", "ch10", "ch11",
+                          "ch12", "ch13", "ch14", "ch15",
+                          "error";
+        clocks = <&cpg CPG_MOD R9A07G044_CLK_DMAC>;
+        power-domains = <&cpg>;
+        resets = <&cpg R9A07G044_CLK_DMAC>;
+        #dma-cells = <1>;
+        dma-channels = <16>;
+        renesas,rz-dmac-slavecfg = <0x255 0x10049C18 0x0011228>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
  2021-06-11 11:36 [PATCH 0/5] Add RZ/G2L DMAC support Biju Das
  2021-06-11 11:36 ` [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings Biju Das
@ 2021-06-11 11:36 ` Biju Das
  2021-06-14 12:15   ` Geert Uytterhoeven
  1 sibling, 1 reply; 21+ messages in thread
From: Biju Das @ 2021-06-11 11:36 UTC (permalink / raw
  To: Rob Herring
  Cc: Biju Das, Geert Uytterhoeven, Magnus Damm, linux-renesas-soc,
	devicetree, Chris Paterson, Chris Brandt, Prabhakar Mahadev Lad

Add DMAC support to RZ/G2L SoC DT.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
This patch depend on [1]
[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210609153230.6967-11-prabhakar.mahadev-lad.rj@bp.renesas.com/
---
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 476ee9a69065..47f9fafd6c06 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -8,6 +8,10 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/r9a07g044-cpg.h>
 
+#define CH_CFG(reqd, loen, hien, lvl, am, sds, dds, tm) \
+	((((tm) << 22) | ((dds) << 16) | ((sds) << 12) | ((am) << 8) | \
+	((lvl) << 6) | ((hien) << 5) | ((loen) << 4) | ((reqd) << 3)) & 0x004FF778)
+
 / {
 	compatible = "renesas,r9a07g044";
 	#address-cells = <2>;
@@ -111,6 +115,40 @@
 			status = "disabled";
 		};
 
+		dmac: dma-controller@11820000 {
+			compatible = "renesas,dmac-r9a07g044",
+				     "renesas,rz-dmac";
+			reg = <0 0x11820000 0 0x10000>,
+			      <0 0x11830000 0 0x10000>;
+			interrupts = <GIC_SPI 125 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 127 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 128 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 130 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 131 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 133 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 135 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 136 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 137 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 138 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 139 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 140 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 141 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "ch0", "ch1", "ch2", "ch3",
+					  "ch4", "ch5", "ch6", "ch7",
+					  "ch8", "ch9", "ch10", "ch11",
+					  "ch12", "ch13", "ch14", "ch15",
+					  "error";
+			clocks = <&cpg CPG_MOD R9A07G044_CLK_DMAC>;
+			power-domains = <&cpg>;
+			resets = <&cpg R9A07G044_CLK_DMAC>;
+			#dma-cells = <1>;
+			dma-channels = <16>;
+		};
+
 		gic: interrupt-controller@11900000 {
 			compatible = "arm,gic-v3";
 			#interrupt-cells = <3>;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-11 11:36 ` [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings Biju Das
@ 2021-06-11 17:55   ` Rob Herring
  2021-06-12 12:17     ` Biju Das
  2021-06-11 19:39   ` Rob Herring
  2021-06-14 12:11   ` Geert Uytterhoeven
  2 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2021-06-11 17:55 UTC (permalink / raw
  To: Biju Das
  Cc: Prabhakar Mahadev Lad, devicetree, Chris Paterson, Vinod Koul,
	linux-renesas-soc, Chris Brandt, Geert Uytterhoeven, Rob Herring,
	dmaengine

On Fri, 11 Jun 2021 12:36:38 +0100, Biju Das wrote:
> Document RZ/G2L DMAC bindings.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  .../bindings/dma/renesas,rz-dmac.yaml         | 132 ++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> 

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:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/dma/renesas,rz-dmac.example.dts:20:18: fatal error: dt-bindings/clock/r9a07g044-cpg.h: No such file or directory
   20 |         #include <dt-bindings/clock/r9a07g044-cpg.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/dma/renesas,rz-dmac.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1416: dt_binding_check] Error 2
\ndoc reference errors (make refcheckdocs):

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

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.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-11 11:36 ` [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings Biju Das
  2021-06-11 17:55   ` Rob Herring
@ 2021-06-11 19:39   ` Rob Herring
  2021-06-12 12:26     ` Biju Das
  2021-06-14 12:11   ` Geert Uytterhoeven
  2 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2021-06-11 19:39 UTC (permalink / raw
  To: Biju Das
  Cc: Vinod Koul, Chris Brandt, dmaengine, devicetree,
	Geert Uytterhoeven, Chris Paterson, Prabhakar Mahadev Lad,
	linux-renesas-soc

On Fri, Jun 11, 2021 at 12:36:38PM +0100, Biju Das wrote:
> Document RZ/G2L DMAC bindings.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  .../bindings/dma/renesas,rz-dmac.yaml         | 132 ++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> 
> diff --git a/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> new file mode 100644
> index 000000000000..df54bd6ddfd4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> @@ -0,0 +1,132 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/renesas,rz-dmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L DMA Controller
> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +allOf:
> +  - $ref: "dma-controller.yaml#"
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> +      - const: renesas,rz-dmac
> +
> +  reg:
> +    items:
> +      - description: Control and channel register block
> +      - description: DMA extension resource selector block
> +
> +  interrupts:
> +    maxItems: 17
> +
> +  interrupt-names:
> +    maxItems: 17
> +    items:
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"
> +      - pattern: "^ch([0-9]|1[0-5])$"

Is there some reason these need be in undefined order?

> +      - const: error
> +
> +  clocks:
> +    maxItems: 1
> +
> +  '#dma-cells':
> +    const: 1
> +    description:
> +      The cell specifies the MID/RID of the DMAC port connected to
> +      the DMA client.
> +
> +  dma-channels:
> +    const: 16
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  renesas,rz-dmac-slavecfg:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: |
> +      DMA configuration for a slave channel. Each channel must have an array of
> +      3 items as below.
> +      first item in the array is MID+RID
> +      second item in the array is slave src or dst address
> +      third item in the array is channel configuration value.

Why not put all these in the dma-cells? You already have 1 of them.

Though doesn't the client device know what address to use?

> +    items:
> +      minItems: 3
> +      maxItems: 48
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - '#dma-cells'
> +  - dma-channels
> +  - power-domains
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> +
> +    dmac: dma-controller@11820000 {
> +        compatible = "renesas,dmac-r9a07g044",
> +                     "renesas,rz-dmac";
> +        reg = <0x11820000 0x10000>,
> +              <0x11830000 0x10000>;
> +        interrupts = <GIC_SPI 125 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 127 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 128 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 130 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 131 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 133 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 135 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 136 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 137 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 138 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 139 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 140 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 141 IRQ_TYPE_EDGE_RISING>;
> +        interrupt-names = "ch0", "ch1", "ch2", "ch3",
> +                          "ch4", "ch5", "ch6", "ch7",
> +                          "ch8", "ch9", "ch10", "ch11",
> +                          "ch12", "ch13", "ch14", "ch15",
> +                          "error";
> +        clocks = <&cpg CPG_MOD R9A07G044_CLK_DMAC>;
> +        power-domains = <&cpg>;
> +        resets = <&cpg R9A07G044_CLK_DMAC>;
> +        #dma-cells = <1>;
> +        dma-channels = <16>;
> +        renesas,rz-dmac-slavecfg = <0x255 0x10049C18 0x0011228>;
> +    };
> -- 
> 2.17.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-11 17:55   ` Rob Herring
@ 2021-06-12 12:17     ` Biju Das
  0 siblings, 0 replies; 21+ messages in thread
From: Biju Das @ 2021-06-12 12:17 UTC (permalink / raw
  To: Rob Herring
  Cc: Prabhakar Mahadev Lad, devicetree@vger.kernel.org, Chris Paterson,
	Vinod Koul, linux-renesas-soc@vger.kernel.org, Chris Brandt,
	Geert Uytterhoeven, Rob Herring, dmaengine@vger.kernel.org


Hi Rob,

Thanks for the review.

> Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> 
> On Fri, 11 Jun 2021 12:36:38 +0100, Biju Das wrote:
> > Document RZ/G2L DMAC bindings.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  .../bindings/dma/renesas,rz-dmac.yaml         | 132 ++++++++++++++++++
> >  1 file changed, 132 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> >
> 
> 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:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/dma/renesas,rz-dmac.example.dts:20:18:
> fatal error: dt-bindings/clock/r9a07g044-cpg.h: No such file or directory
>    20 |         #include <dt-bindings/clock/r9a07g044-cpg.h>
>       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:380:
> Documentation/devicetree/bindings/dma/renesas,rz-dmac.example.dt.yaml]
> Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1416: dt_binding_check] Error 2 \ndoc reference errors
> (make refcheckdocs):
> 
> See
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.ozlabs.org%2Fpatch%2F1490917&amp;data=04%7C01%7Cbiju.das.jz%40bp.renesas
> .com%7C0b0cf26acc004dba607508d92d0222fe%7C53d82571da1947e49cb4625a166a4a2a
> %7C0%7C0%7C637590309476730777%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi
> LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=lZ%2F0v9D4
> 9djAW8E4sA3zcHOFs4x%2F073f40FkAGZJ0ZI%3D&amp;reserved=0
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.

Sorry, The dependency patch is just queued. Next time, I will make sure
dependency patch is in the most recent rc1 before posting.

> 
> 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.

Sure will check and re-submit.

Regards,
Biju


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-11 19:39   ` Rob Herring
@ 2021-06-12 12:26     ` Biju Das
  0 siblings, 0 replies; 21+ messages in thread
From: Biju Das @ 2021-06-12 12:26 UTC (permalink / raw
  To: Rob Herring
  Cc: Vinod Koul, Chris Brandt, dmaengine@vger.kernel.org,
	devicetree@vger.kernel.org, Geert Uytterhoeven, Chris Paterson,
	Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org

Hi Rob,

Thanks for the feedback.

> Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> 
> On Fri, Jun 11, 2021 at 12:36:38PM +0100, Biju Das wrote:
> > Document RZ/G2L DMAC bindings.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  .../bindings/dma/renesas,rz-dmac.yaml         | 132 ++++++++++++++++++
> >  1 file changed, 132 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > new file mode 100644
> > index 000000000000..df54bd6ddfd4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > @@ -0,0 +1,132 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&amp;data=04%7C
> > +01%7Cbiju.das.jz%40bp.renesas.com%7Ce46660b298b942937fe408d92d109c19%
> > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637590371623792000%7CUnk
> > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=kirztzPuCmsjeKEivOgQZqP5obsByrSaTnQ
> > +bzQbU%2BRM%3D&amp;reserved=0
> > +$schema:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > +jz%40bp.renesas.com%7Ce46660b298b942937fe408d92d109c19%7C53d82571da19
> > +47e49cb4625a166a4a2a%7C0%7C0%7C637590371623792000%7CUnknown%7CTWFpbGZ
> > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> > +3D%7C1000&amp;sdata=U2lrBvVVhySXVYHK6Qk41VTGijep8yPaTCMJpSjRsXs%3D&am
> > +p;reserved=0
> > +
> > +title: Renesas RZ/G2L DMA Controller
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +allOf:
> > +  - $ref: "dma-controller.yaml#"
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > +      - const: renesas,rz-dmac
> > +
> > +  reg:
> > +    items:
> > +      - description: Control and channel register block
> > +      - description: DMA extension resource selector block
> > +
> > +  interrupts:
> > +    maxItems: 17
> > +
> > +  interrupt-names:
> > +    maxItems: 17
> > +    items:
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> > +      - pattern: "^ch([0-9]|1[0-5])$"
> 
> Is there some reason these need be in undefined order?
No. I will make it as defined order in next version.

> 
> > +      - const: error
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  '#dma-cells':
> > +    const: 1
> > +    description:
> > +      The cell specifies the MID/RID of the DMAC port connected to
> > +      the DMA client.
> > +
> > +  dma-channels:
> > +    const: 16
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  renesas,rz-dmac-slavecfg:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    description: |
> > +      DMA configuration for a slave channel. Each channel must have an
> array of
> > +      3 items as below.
> > +      first item in the array is MID+RID
> > +      second item in the array is slave src or dst address
> > +      third item in the array is channel configuration value.
> 
> Why not put all these in the dma-cells? You already have 1 of them.

Thanks for the suggestion. I will make use of dma-cells and will remove the above property
in next revision. Basically It simplifies the implementation as well.

> Though doesn't the client device know what address to use?
Indeed. it knows.

Cheers,
Biju
> 
> > +    items:
> > +      minItems: 3
> > +      maxItems: 48
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-names
> > +  - clocks
> > +  - '#dma-cells'
> > +  - dma-channels
> > +  - power-domains
> > +  - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> > +
> > +    dmac: dma-controller@11820000 {
> > +        compatible = "renesas,dmac-r9a07g044",
> > +                     "renesas,rz-dmac";
> > +        reg = <0x11820000 0x10000>,
> > +              <0x11830000 0x10000>;
> > +        interrupts = <GIC_SPI 125 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 127 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 128 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 130 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 131 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 133 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 135 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 136 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 137 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 138 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 139 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 140 IRQ_TYPE_EDGE_RISING>,
> > +                     <GIC_SPI 141 IRQ_TYPE_EDGE_RISING>;
> > +        interrupt-names = "ch0", "ch1", "ch2", "ch3",
> > +                          "ch4", "ch5", "ch6", "ch7",
> > +                          "ch8", "ch9", "ch10", "ch11",
> > +                          "ch12", "ch13", "ch14", "ch15",
> > +                          "error";
> > +        clocks = <&cpg CPG_MOD R9A07G044_CLK_DMAC>;
> > +        power-domains = <&cpg>;
> > +        resets = <&cpg R9A07G044_CLK_DMAC>;
> > +        #dma-cells = <1>;
> > +        dma-channels = <16>;
> > +        renesas,rz-dmac-slavecfg = <0x255 0x10049C18 0x0011228>;
> > +    };
> > --
> > 2.17.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-11 11:36 ` [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings Biju Das
  2021-06-11 17:55   ` Rob Herring
  2021-06-11 19:39   ` Rob Herring
@ 2021-06-14 12:11   ` Geert Uytterhoeven
  2021-06-14 12:54     ` Biju Das
  2 siblings, 1 reply; 21+ messages in thread
From: Geert Uytterhoeven @ 2021-06-14 12:11 UTC (permalink / raw
  To: Biju Das
  Cc: Vinod Koul, Rob Herring, Chris Brandt, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas,
	Laurent Pinchart

Hi Biju,

On Fri, Jun 11, 2021 at 1:36 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Document RZ/G2L DMAC bindings.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> @@ -0,0 +1,132 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/renesas,rz-dmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L DMA Controller
> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +allOf:
> +  - $ref: "dma-controller.yaml#"
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}

Please use "renesas,r9a07g044-dmac".

> +      - const: renesas,rz-dmac

Does this need many changes for RZ/A1H and RZ/A2M?

> +  renesas,rz-dmac-slavecfg:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: |
> +      DMA configuration for a slave channel. Each channel must have an array of
> +      3 items as below.
> +      first item in the array is MID+RID

Already in dmas.

> +      second item in the array is slave src or dst address

As pointed out by Rob, already known by the slave driver.

> +      third item in the array is channel configuration value.

What exactly is this?
Does the R-Car DMAC have this too? If yes, how does its driver handle it?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
  2021-06-11 11:36 ` [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support Biju Das
@ 2021-06-14 12:15   ` Geert Uytterhoeven
  2021-06-14 13:02     ` Biju Das
  0 siblings, 1 reply; 21+ messages in thread
From: Geert Uytterhoeven @ 2021-06-14 12:15 UTC (permalink / raw
  To: Biju Das
  Cc: Rob Herring, Magnus Damm, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Chris Brandt, Prabhakar Mahadev Lad

Hi Biju,

On Fri, Jun 11, 2021 at 1:36 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add DMAC support to RZ/G2L SoC DT.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> @@ -8,6 +8,10 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/clock/r9a07g044-cpg.h>
>
> +#define CH_CFG(reqd, loen, hien, lvl, am, sds, dds, tm) \
> +       ((((tm) << 22) | ((dds) << 16) | ((sds) << 12) | ((am) << 8) | \
> +       ((lvl) << 6) | ((hien) << 5) | ((loen) << 4) | ((reqd) << 3)) & 0x004FF778)
> +

I assume the above will be removed?

>  / {
>         compatible = "renesas,r9a07g044";
>         #address-cells = <2>;
> @@ -111,6 +115,40 @@
>                         status = "disabled";
>                 };
>
> +               dmac: dma-controller@11820000 {
> +                       compatible = "renesas,dmac-r9a07g044",
> +                                    "renesas,rz-dmac";
> +                       reg = <0 0x11820000 0 0x10000>,
> +                             <0 0x11830000 0 0x10000>;
> +                       interrupts = <GIC_SPI 125 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 127 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 128 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 130 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 131 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 133 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 135 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 136 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 137 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 138 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 139 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 140 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 141 IRQ_TYPE_EDGE_RISING>;
> +                       interrupt-names = "ch0", "ch1", "ch2", "ch3",
> +                                         "ch4", "ch5", "ch6", "ch7",
> +                                         "ch8", "ch9", "ch10", "ch11",
> +                                         "ch12", "ch13", "ch14", "ch15",
> +                                         "error";
> +                       clocks = <&cpg CPG_MOD R9A07G044_CLK_DMAC>;
> +                       power-domains = <&cpg>;
> +                       resets = <&cpg R9A07G044_CLK_DMAC>;
> +                       #dma-cells = <1>;
> +                       dma-channels = <16>;
> +               };
> +
>                 gic: interrupt-controller@11900000 {
>                         compatible = "arm,gic-v3";
>                         #interrupt-cells = <3>;

The rest looks good to me, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 12:11   ` Geert Uytterhoeven
@ 2021-06-14 12:54     ` Biju Das
  2021-06-14 14:29       ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2021-06-14 12:54 UTC (permalink / raw
  To: Geert Uytterhoeven
  Cc: Vinod Koul, Rob Herring, Chris Brandt, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas,
	Laurent Pinchart

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> 
> Hi Biju,
> 
> On Fri, Jun 11, 2021 at 1:36 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Document RZ/G2L DMAC bindings.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > @@ -0,0 +1,132 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&amp;data=04%7C
> > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%
> > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnk
> > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrViQCcrNtzuDejp8wo
> > +Nrx9iO0ht8%3D&amp;reserved=0
> > +$schema:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%7C53d82571da19
> > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnknown%7CTWFpbGZ
> > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJgRIAdLnhJraw%3D&
> > +amp;reserved=0
> > +
> > +title: Renesas RZ/G2L DMA Controller
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +allOf:
> > +  - $ref: "dma-controller.yaml#"
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> 
> Please use "renesas,r9a07g044-dmac".

OK. Will change.

> > +      - const: renesas,rz-dmac
> 
> Does this need many changes for RZ/A1H and RZ/A2M?

It will work on both RZ/A1H and RZ/A2M. I have n't tested since I don't have the board.
There is some difference in MID bit size. Other wise both identical.

 
> > +  renesas,rz-dmac-slavecfg:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    description: |
> > +      DMA configuration for a slave channel. Each channel must have an
> array of
> > +      3 items as below.
> > +      first item in the array is MID+RID
> 
> Already in dmas.
> 
> > +      second item in the array is slave src or dst address
> 
> As pointed out by Rob, already known by the slave driver.
> 
> > +      third item in the array is channel configuration value.
> 
> What exactly is this?
> Does the R-Car DMAC have this too? If yes, how does its driver handle it?

On R-CAR DMAC, we have only MID + RID values. Where as here we have channel configuration value With different set of parameter as mentioned in Table 16.4.

Please see Page 569, Table 16.4 On-Chip Module requests section. 

For eg:- as per Rob's suggestion, I have modelled the driver with the below entries in ALSA driver for playback/record use case.

dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
       <&dmac 0x256 0x10049c1c CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
dma-names = "tx", "rx";

Using first parameter, it gets dmac channel. using second and third parameter it configures 
the channel.

Regards,
Biju

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
  2021-06-14 12:15   ` Geert Uytterhoeven
@ 2021-06-14 13:02     ` Biju Das
  2021-06-14 13:48       ` Geert Uytterhoeven
  0 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2021-06-14 13:02 UTC (permalink / raw
  To: Geert Uytterhoeven
  Cc: Rob Herring, Magnus Damm, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Chris Brandt, Prabhakar Mahadev Lad

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
> 
> Hi Biju,
> 
> On Fri, Jun 11, 2021 at 1:36 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add DMAC support to RZ/G2L SoC DT.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > @@ -8,6 +8,10 @@
> >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> >  #include <dt-bindings/clock/r9a07g044-cpg.h>
> >
> > +#define CH_CFG(reqd, loen, hien, lvl, am, sds, dds, tm) \
> > +       ((((tm) << 22) | ((dds) << 16) | ((sds) << 12) | ((am) << 8) | \
> > +       ((lvl) << 6) | ((hien) << 5) | ((loen) << 4) | ((reqd) << 3))
> > +& 0x004FF778)
> > +
> 
> I assume the above will be removed?

Basically the macro simplifies the channel configuration values in Table 16.4 page 569 of the hardware manual.

Client driver will use MID+RID, and pass (Src address or dest address along with the channel configuration values
For configuring DMA channel.

For eg:-

                ssi0: ssi@10049c00 {
                        compatible = "renesas,r9a07g044-ssi",
                                     "renesas,rz-ssi";
                        reg = <0 0x10049c00 0 0x400>;
                        interrupts = <GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH>,
                                     <GIC_SPI 327 IRQ_TYPE_EDGE_RISING>,
                                     <GIC_SPI 328 IRQ_TYPE_EDGE_RISING>;
                        interrupt-names = "int", "rx", "tx";
                        clocks = <&cpg CPG_MOD R9A07G044_CLK_SSI0>,
                                 <&audio_clk1>,
                                 <&audio_clk2>;
                        clock-names = "ssi", "audio_clk1", "audio_clk2";
                        resets = <&cpg R9A07G044_CLK_SSI0>;
                        dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
                               <&dmac 0x256 0x10049c1c CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
                        dma-names = "tx", "rx";
                        power-domains = <&cpg>;
                        #sound-dai-cells = <0>;
                        status = "disabled";
                };

Please let me know your thoughts on this.

Regards,
Biju
 
> >  / {
> >         compatible = "renesas,r9a07g044";
> >         #address-cells = <2>;
> > @@ -111,6 +115,40 @@
> >                         status = "disabled";
> >                 };
> >
> > +               dmac: dma-controller@11820000 {
> > +                       compatible = "renesas,dmac-r9a07g044",
> > +                                    "renesas,rz-dmac";
> > +                       reg = <0 0x11820000 0 0x10000>,
> > +                             <0 0x11830000 0 0x10000>;
> > +                       interrupts = <GIC_SPI 125 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 127 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 128 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 130 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 131 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 133 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 135 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 136 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 137 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 138 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 139 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 140 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 141 IRQ_TYPE_EDGE_RISING>;
> > +                       interrupt-names = "ch0", "ch1", "ch2", "ch3",
> > +                                         "ch4", "ch5", "ch6", "ch7",
> > +                                         "ch8", "ch9", "ch10", "ch11",
> > +                                         "ch12", "ch13", "ch14",
> "ch15",
> > +                                         "error";
> > +                       clocks = <&cpg CPG_MOD R9A07G044_CLK_DMAC>;
> > +                       power-domains = <&cpg>;
> > +                       resets = <&cpg R9A07G044_CLK_DMAC>;
> > +                       #dma-cells = <1>;
> > +                       dma-channels = <16>;
> > +               };
> > +
> >                 gic: interrupt-controller@11900000 {
> >                         compatible = "arm,gic-v3";
> >                         #interrupt-cells = <3>;
> 
> The rest looks good to me, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
  2021-06-14 13:02     ` Biju Das
@ 2021-06-14 13:48       ` Geert Uytterhoeven
  2021-06-15  8:12         ` Biju Das
  0 siblings, 1 reply; 21+ messages in thread
From: Geert Uytterhoeven @ 2021-06-14 13:48 UTC (permalink / raw
  To: Biju Das
  Cc: Rob Herring, Magnus Damm, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Chris Brandt, Prabhakar Mahadev Lad,
	Laurent Pinchart

Hi Biju,

On Mon, Jun 14, 2021 at 3:02 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
> > On Fri, Jun 11, 2021 at 1:36 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Add DMAC support to RZ/G2L SoC DT.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > @@ -8,6 +8,10 @@
> > >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> > >  #include <dt-bindings/clock/r9a07g044-cpg.h>
> > >
> > > +#define CH_CFG(reqd, loen, hien, lvl, am, sds, dds, tm) \
> > > +       ((((tm) << 22) | ((dds) << 16) | ((sds) << 12) | ((am) << 8) | \
> > > +       ((lvl) << 6) | ((hien) << 5) | ((loen) << 4) | ((reqd) << 3))
> > > +& 0x004FF778)
> > > +
> >
> > I assume the above will be removed?
>
> Basically the macro simplifies the channel configuration values in Table 16.4 page 569 of the hardware manual.
>
> Client driver will use MID+RID, and pass (Src address or dest address along with the channel configuration values
> For configuring DMA channel.
>
> For eg:-
>
>                 ssi0: ssi@10049c00 {
>                         compatible = "renesas,r9a07g044-ssi",
>                                      "renesas,rz-ssi";
>                         reg = <0 0x10049c00 0 0x400>;
>                         interrupts = <GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH>,
>                                      <GIC_SPI 327 IRQ_TYPE_EDGE_RISING>,
>                                      <GIC_SPI 328 IRQ_TYPE_EDGE_RISING>;
>                         interrupt-names = "int", "rx", "tx";
>                         clocks = <&cpg CPG_MOD R9A07G044_CLK_SSI0>,
>                                  <&audio_clk1>,
>                                  <&audio_clk2>;
>                         clock-names = "ssi", "audio_clk1", "audio_clk2";
>                         resets = <&cpg R9A07G044_CLK_SSI0>;
>                         dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
>                                <&dmac 0x256 0x10049c1c CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
>                         dma-names = "tx", "rx";
>                         power-domains = <&cpg>;
>                         #sound-dai-cells = <0>;
>                         status = "disabled";
>                 };
>
> Please let me know your thoughts on this.

How will this work with (existing) drivers?
E.g. drivers/tty/serial/sh-sci.c:sci_request_dma_chan() already knows the
source and destination addresses.
The other CHCFG bits may be new, though.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 12:54     ` Biju Das
@ 2021-06-14 14:29       ` Laurent Pinchart
  2021-06-14 16:09         ` Biju Das
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2021-06-14 14:29 UTC (permalink / raw
  To: Biju Das
  Cc: Geert Uytterhoeven, Vinod Koul, Rob Herring, Chris Brandt,
	dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas

On Mon, Jun 14, 2021 at 12:54:02PM +0000, Biju Das wrote:
> > -----Original Message-----
> > Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> > 
> > Hi Biju,
> > 
> > On Fri, Jun 11, 2021 at 1:36 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Document RZ/G2L DMAC bindings.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > 
> > Thanks for your patch!
> > 
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > > @@ -0,0 +1,132 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id:
> > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&amp;data=04%7C
> > > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%
> > > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnk
> > > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> > > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrViQCcrNtzuDejp8wo
> > > +Nrx9iO0ht8%3D&amp;reserved=0
> > > +$schema:
> > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%7C53d82571da19
> > > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnknown%7CTWFpbGZ
> > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> > > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJgRIAdLnhJraw%3D&
> > > +amp;reserved=0

*sigh*

> > > +
> > > +title: Renesas RZ/G2L DMA Controller
> > > +
> > > +maintainers:
> > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > +
> > > +allOf:
> > > +  - $ref: "dma-controller.yaml#"
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > 
> > Please use "renesas,r9a07g044-dmac".
> 
> OK. Will change.
> 
> > > +      - const: renesas,rz-dmac
> > 
> > Does this need many changes for RZ/A1H and RZ/A2M?
> 
> It will work on both RZ/A1H and RZ/A2M. I have n't tested since I don't have the board.
> There is some difference in MID bit size. Other wise both identical.
> 
>  
> > > +  renesas,rz-dmac-slavecfg:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +    description: |
> > > +      DMA configuration for a slave channel. Each channel must have an
> > array of
> > > +      3 items as below.
> > > +      first item in the array is MID+RID
> > 
> > Already in dmas.
> > 
> > > +      second item in the array is slave src or dst address
> > 
> > As pointed out by Rob, already known by the slave driver.
> > 
> > > +      third item in the array is channel configuration value.
> > 
> > What exactly is this?

What would prevent the DMA client from passing the configuration to the
DMA channel through the DMA engine API, just like it passes the slave
source or destination address ?

> > Does the R-Car DMAC have this too? If yes, how does its driver handle it?
> 
> On R-CAR DMAC, we have only MID + RID values. Where as here we have channel configuration value With different set of parameter as mentioned in Table 16.4.
> 
> Please see Page 569, Table 16.4 On-Chip Module requests section. 
> 
> For eg:- as per Rob's suggestion, I have modelled the driver with the below entries in ALSA driver for playback/record use case.
> 
> dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
>        <&dmac 0x256 0x10049c1c CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> dma-names = "tx", "rx";
> 
> Using first parameter, it gets dmac channel. using second and third parameter it configures 
> the channel.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 14:29       ` Laurent Pinchart
@ 2021-06-14 16:09         ` Biju Das
  2021-06-14 16:17           ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2021-06-14 16:09 UTC (permalink / raw
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Vinod Koul, Rob Herring, Chris Brandt,
	dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas

Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> 
> On Mon, Jun 14, 2021 at 12:54:02PM +0000, Biju Das wrote:
> > > -----Original Message-----
> > > Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> > >
> > > Hi Biju,
> > >
> > > On Fri, Jun 11, 2021 at 1:36 PM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > Document RZ/G2L DMAC bindings.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Reviewed-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > > > @@ -0,0 +1,132 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> > > > +1.2
> > > > +---
> > > > +$id:
> > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> > > > +devi
> > > > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&amp;data=0
> > > > +4%7C
> > > > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da
> > > > +0c0%
> > > > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7
> > > > +CUnk
> > > > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
> > > > +1haW
> > > > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrViQCcrNtzuDej
> > > > +p8wo
> > > > +Nrx9iO0ht8%3D&amp;reserved=0
> > > > +$schema:
> > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> > > > +devi
> > > > +cetree.org%2Fmeta-
> schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > > > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%7C53d82571
> > > > +da19
> > > > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnknown%7CTWF
> > > > +pbGZ
> > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> > > > +Mn0%
> > > > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJgRIAdLnhJraw
> > > > +%3D&
> > > > +amp;reserved=0
> 
> *sigh*
> 
> > > > +
> > > > +title: Renesas RZ/G2L DMA Controller
> > > > +
> > > > +maintainers:
> > > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > > +
> > > > +allOf:
> > > > +  - $ref: "dma-controller.yaml#"
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - enum:
> > > > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > >
> > > Please use "renesas,r9a07g044-dmac".
> >
> > OK. Will change.
> >
> > > > +      - const: renesas,rz-dmac
> > >
> > > Does this need many changes for RZ/A1H and RZ/A2M?
> >
> > It will work on both RZ/A1H and RZ/A2M. I have n't tested since I don't
> have the board.
> > There is some difference in MID bit size. Other wise both identical.
> >
> >
> > > > +  renesas,rz-dmac-slavecfg:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +    description: |
> > > > +      DMA configuration for a slave channel. Each channel must
> > > > + have an
> > > array of
> > > > +      3 items as below.
> > > > +      first item in the array is MID+RID
> > >
> > > Already in dmas.
> > >
> > > > +      second item in the array is slave src or dst address
> > >
> > > As pointed out by Rob, already known by the slave driver.
> > >
> > > > +      third item in the array is channel configuration value.
> > >
> > > What exactly is this?
> 
> What would prevent the DMA client from passing the configuration to the
> DMA channel through the DMA engine API, just like it passes the slave
> source or destination address ?

On RZ/G2L, there is 1 case(SSIF ch2) where MID+RID is same for both tx and rx. 
The only way we can distinguish it is from channel configuration value.

Cheers,
Biju


> 
> > > Does the R-Car DMAC have this too? If yes, how does its driver handle
> it?
> >
> > On R-CAR DMAC, we have only MID + RID values. Where as here we have
> channel configuration value With different set of parameter as mentioned
> in Table 16.4.
> >
> > Please see Page 569, Table 16.4 On-Chip Module requests section.
> >
> > For eg:- as per Rob's suggestion, I have modelled the driver with the
> below entries in ALSA driver for playback/record use case.
> >
> > dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
> >        <&dmac 0x256 0x10049c1c
> > CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> > dma-names = "tx", "rx";
> >
> > Using first parameter, it gets dmac channel. using second and third
> > parameter it configures the channel.
> 
> --
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 16:09         ` Biju Das
@ 2021-06-14 16:17           ` Laurent Pinchart
  2021-06-14 16:24             ` Biju Das
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2021-06-14 16:17 UTC (permalink / raw
  To: Biju Das
  Cc: Geert Uytterhoeven, Vinod Koul, Rob Herring, Chris Brandt,
	dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas

Hi Biju,

On Mon, Jun 14, 2021 at 04:09:04PM +0000, Biju Das wrote:
> > On Mon, Jun 14, 2021 at 12:54:02PM +0000, Biju Das wrote:
> > > > On Fri, Jun 11, 2021 at 1:36 PM Biju Das wrote:
> > > > > Document RZ/G2L DMAC bindings.
> > > > >
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > > > > @@ -0,0 +1,132 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> > > > > +1.2
> > > > > +---
> > > > > +$id:
> > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> > > > > +devi
> > > > > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&amp;data=0
> > > > > +4%7C
> > > > > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da
> > > > > +0c0%
> > > > > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7
> > > > > +CUnk
> > > > > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
> > > > > +1haW
> > > > > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrViQCcrNtzuDej
> > > > > +p8wo
> > > > > +Nrx9iO0ht8%3D&amp;reserved=0
> > > > > +$schema:
> > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> > > > > +devi
> > > > > +cetree.org%2Fmeta-
> > schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > > > > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%7C53d82571
> > > > > +da19
> > > > > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnknown%7CTWF
> > > > > +pbGZ
> > > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> > > > > +Mn0%
> > > > > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJgRIAdLnhJraw
> > > > > +%3D&
> > > > > +amp;reserved=0
> > 
> > *sigh*
> > 
> > > > > +
> > > > > +title: Renesas RZ/G2L DMA Controller
> > > > > +
> > > > > +maintainers:
> > > > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > > > +
> > > > > +allOf:
> > > > > +  - $ref: "dma-controller.yaml#"
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    items:
> > > > > +      - enum:
> > > > > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > > >
> > > > Please use "renesas,r9a07g044-dmac".
> > >
> > > OK. Will change.
> > >
> > > > > +      - const: renesas,rz-dmac
> > > >
> > > > Does this need many changes for RZ/A1H and RZ/A2M?
> > >
> > > It will work on both RZ/A1H and RZ/A2M. I have n't tested since I don't have the board.
> > > There is some difference in MID bit size. Other wise both identical.
> > >
> > > > > +  renesas,rz-dmac-slavecfg:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > +    description: |
> > > > > +      DMA configuration for a slave channel. Each channel must
> > > > > + have an array of
> > > > > +      3 items as below.
> > > > > +      first item in the array is MID+RID
> > > >
> > > > Already in dmas.
> > > >
> > > > > +      second item in the array is slave src or dst address
> > > >
> > > > As pointed out by Rob, already known by the slave driver.
> > > >
> > > > > +      third item in the array is channel configuration value.
> > > >
> > > > What exactly is this?
> > 
> > What would prevent the DMA client from passing the configuration to the
> > DMA channel through the DMA engine API, just like it passes the slave
> > source or destination address ?
> 
> On RZ/G2L, there is 1 case(SSIF ch2) where MID+RID is same for both tx and rx. 
> The only way we can distinguish it is from channel configuration value.

Are those two different hardware DMA channels ? And configuration values
change between the two ?

> > > > Does the R-Car DMAC have this too? If yes, how does its driver
> > > > handle it?
> > >
> > > On R-CAR DMAC, we have only MID + RID values. Where as here we
> > > have channel configuration value With different set of parameter
> > > as mentioned in Table 16.4.
> > >
> > > Please see Page 569, Table 16.4 On-Chip Module requests section.
> > >
> > > For eg:- as per Rob's suggestion, I have modelled the driver with
> > > the below entries in ALSA driver for playback/record use case.
> > >
> > > dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
> > >        <&dmac 0x256 0x10049c1c
> > > CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> > > dma-names = "tx", "rx";
> > >
> > > Using first parameter, it gets dmac channel. using second and third
> > > parameter it configures the channel.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 16:17           ` Laurent Pinchart
@ 2021-06-14 16:24             ` Biju Das
  2021-06-14 16:28               ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2021-06-14 16:24 UTC (permalink / raw
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Vinod Koul, Rob Herring, Chris Brandt,
	dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas

Hi Laurent,

> Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> 
> Hi Biju,
> 
> On Mon, Jun 14, 2021 at 04:09:04PM +0000, Biju Das wrote:
> > > On Mon, Jun 14, 2021 at 12:54:02PM +0000, Biju Das wrote:
> > > > > On Fri, Jun 11, 2021 at 1:36 PM Biju Das wrote:
> > > > > > Document RZ/G2L DMAC bindings.
> > > > > >
> > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > Reviewed-by: Lad Prabhakar
> > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > >
> > > > > Thanks for your patch!
> > > > >
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.ya
> > > > > > +++ ml
> > > > > > @@ -0,0 +1,132 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML
> > > > > > +1.2
> > > > > > +---
> > > > > > +$id:
> > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2
> > > > > > +F%2F
> > > > > > +devi
> > > > > > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&amp;da
> > > > > > +ta=0
> > > > > > +4%7C
> > > > > > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92
> > > > > > +f2da
> > > > > > +0c0%
> > > > > > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C6375926952868468
> > > > > > +09%7
> > > > > > +CUnk
> > > > > > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> > > > > > +I6Ik
> > > > > > +1haW
> > > > > > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrViQCcrNtz
> > > > > > +uDej
> > > > > > +p8wo
> > > > > > +Nrx9iO0ht8%3D&amp;reserved=0
> > > > > > +$schema:
> > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2
> > > > > > +F%2F
> > > > > > +devi
> > > > > > +cetree.org%2Fmeta-
> > > schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > > > > > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%7C53d8
> > > > > > +2571
> > > > > > +da19
> > > > > > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnknown%7
> > > > > > +CTWF
> > > > > > +pbGZ
> > > > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> > > > > > +VCI6
> > > > > > +Mn0%
> > > > > > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJgRIAdLnh
> > > > > > +Jraw
> > > > > > +%3D&
> > > > > > +amp;reserved=0
> > >
> > > *sigh*
> > >
> > > > > > +
> > > > > > +title: Renesas RZ/G2L DMA Controller
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > +
> > > > > > +allOf:
> > > > > > +  - $ref: "dma-controller.yaml#"
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    items:
> > > > > > +      - enum:
> > > > > > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > > > >
> > > > > Please use "renesas,r9a07g044-dmac".
> > > >
> > > > OK. Will change.
> > > >
> > > > > > +      - const: renesas,rz-dmac
> > > > >
> > > > > Does this need many changes for RZ/A1H and RZ/A2M?
> > > >
> > > > It will work on both RZ/A1H and RZ/A2M. I have n't tested since I
> don't have the board.
> > > > There is some difference in MID bit size. Other wise both identical.
> > > >
> > > > > > +  renesas,rz-dmac-slavecfg:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > > +    description: |
> > > > > > +      DMA configuration for a slave channel. Each channel
> > > > > > + must have an array of
> > > > > > +      3 items as below.
> > > > > > +      first item in the array is MID+RID
> > > > >
> > > > > Already in dmas.
> > > > >
> > > > > > +      second item in the array is slave src or dst address
> > > > >
> > > > > As pointed out by Rob, already known by the slave driver.
> > > > >
> > > > > > +      third item in the array is channel configuration value.
> > > > >
> > > > > What exactly is this?
> > >
> > > What would prevent the DMA client from passing the configuration to
> > > the DMA channel through the DMA engine API, just like it passes the
> > > slave source or destination address ?
> >
> > On RZ/G2L, there is 1 case(SSIF ch2) where MID+RID is same for both tx
> and rx.
> > The only way we can distinguish it is from channel configuration value.
> 
> Are those two different hardware DMA channels ? And configuration values
> change between the two ?

Yes, REQD is different, apart from this Rx have transfer source and Tx have Transfer destination.
This particular SSIF ch2 is used only for half duplex compared to other SSIF channels.

Regards,
Biju

> 
> > > > > Does the R-Car DMAC have this too? If yes, how does its driver
> > > > > handle it?
> > > >
> > > > On R-CAR DMAC, we have only MID + RID values. Where as here we
> > > > have channel configuration value With different set of parameter
> > > > as mentioned in Table 16.4.
> > > >
> > > > Please see Page 569, Table 16.4 On-Chip Module requests section.
> > > >
> > > > For eg:- as per Rob's suggestion, I have modelled the driver with
> > > > the below entries in ALSA driver for playback/record use case.
> > > >
> > > > dmas = <&dmac 0x255 0x10049c18
> CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
> > > >        <&dmac 0x256 0x10049c1c
> > > > CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> > > > dma-names = "tx", "rx";
> > > >
> > > > Using first parameter, it gets dmac channel. using second and
> > > > third parameter it configures the channel.
> 
> --
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 16:24             ` Biju Das
@ 2021-06-14 16:28               ` Laurent Pinchart
  2021-06-14 16:33                 ` Biju Das
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2021-06-14 16:28 UTC (permalink / raw
  To: Biju Das
  Cc: Geert Uytterhoeven, Vinod Koul, Rob Herring, Chris Brandt,
	dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas

Hi Biju,

On Mon, Jun 14, 2021 at 04:24:38PM +0000, Biju Das wrote:
> > On Mon, Jun 14, 2021 at 04:09:04PM +0000, Biju Das wrote:
> > > > On Mon, Jun 14, 2021 at 12:54:02PM +0000, Biju Das wrote:
> > > > > > On Fri, Jun 11, 2021 at 1:36 PM Biju Das wrote:
> > > > > > > Document RZ/G2L DMAC bindings.
> > > > > > >
> > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > Reviewed-by: Lad Prabhakar
> > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > >
> > > > > > Thanks for your patch!
> > > > > >
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.ya
> > > > > > > +++ ml
> > > > > > > @@ -0,0 +1,132 @@
> > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > +%YAML
> > > > > > > +1.2
> > > > > > > +---
> > > > > > > +$id:
> > > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2
> > > > > > > +F%2F
> > > > > > > +devi
> > > > > > > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&amp;da
> > > > > > > +ta=0
> > > > > > > +4%7C
> > > > > > > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92
> > > > > > > +f2da
> > > > > > > +0c0%
> > > > > > > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C6375926952868468
> > > > > > > +09%7
> > > > > > > +CUnk
> > > > > > > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> > > > > > > +I6Ik
> > > > > > > +1haW
> > > > > > > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrViQCcrNtz
> > > > > > > +uDej
> > > > > > > +p8wo
> > > > > > > +Nrx9iO0ht8%3D&amp;reserved=0
> > > > > > > +$schema:
> > > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2
> > > > > > > +F%2F
> > > > > > > +devi
> > > > > > > +cetree.org%2Fmeta-
> > > > schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > > > > > > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%7C53d8
> > > > > > > +2571
> > > > > > > +da19
> > > > > > > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnknown%7
> > > > > > > +CTWF
> > > > > > > +pbGZ
> > > > > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> > > > > > > +VCI6
> > > > > > > +Mn0%
> > > > > > > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJgRIAdLnh
> > > > > > > +Jraw
> > > > > > > +%3D&
> > > > > > > +amp;reserved=0
> > > >
> > > > *sigh*
> > > >
> > > > > > > +
> > > > > > > +title: Renesas RZ/G2L DMA Controller
> > > > > > > +
> > > > > > > +maintainers:
> > > > > > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > +
> > > > > > > +allOf:
> > > > > > > +  - $ref: "dma-controller.yaml#"
> > > > > > > +
> > > > > > > +properties:
> > > > > > > +  compatible:
> > > > > > > +    items:
> > > > > > > +      - enum:
> > > > > > > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > > > > >
> > > > > > Please use "renesas,r9a07g044-dmac".
> > > > >
> > > > > OK. Will change.
> > > > >
> > > > > > > +      - const: renesas,rz-dmac
> > > > > >
> > > > > > Does this need many changes for RZ/A1H and RZ/A2M?
> > > > >
> > > > > It will work on both RZ/A1H and RZ/A2M. I have n't tested since I don't have the board.
> > > > > There is some difference in MID bit size. Other wise both identical.
> > > > >
> > > > > > > +  renesas,rz-dmac-slavecfg:
> > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > > > +    description: |
> > > > > > > +      DMA configuration for a slave channel. Each channel
> > > > > > > + must have an array of
> > > > > > > +      3 items as below.
> > > > > > > +      first item in the array is MID+RID
> > > > > >
> > > > > > Already in dmas.
> > > > > >
> > > > > > > +      second item in the array is slave src or dst address
> > > > > >
> > > > > > As pointed out by Rob, already known by the slave driver.
> > > > > >
> > > > > > > +      third item in the array is channel configuration value.
> > > > > >
> > > > > > What exactly is this?
> > > >
> > > > What would prevent the DMA client from passing the configuration to
> > > > the DMA channel through the DMA engine API, just like it passes the
> > > > slave source or destination address ?
> > >
> > > On RZ/G2L, there is 1 case(SSIF ch2) where MID+RID is same for both tx and rx.
> > > The only way we can distinguish it is from channel configuration value.
> > 
> > Are those two different hardware DMA channels ? And configuration values
> > change between the two ?
> 
> Yes, REQD is different, apart from this Rx have transfer source and Tx have Transfer destination.
> This particular SSIF ch2 is used only for half duplex compared to other SSIF channels.

Does this mean there's a single DMA channel, used by two clients, but
not at the same time as it only supports half-duplex ?

> > > > > > Does the R-Car DMAC have this too? If yes, how does its driver
> > > > > > handle it?
> > > > >
> > > > > On R-CAR DMAC, we have only MID + RID values. Where as here we
> > > > > have channel configuration value With different set of parameter
> > > > > as mentioned in Table 16.4.
> > > > >
> > > > > Please see Page 569, Table 16.4 On-Chip Module requests section.
> > > > >
> > > > > For eg:- as per Rob's suggestion, I have modelled the driver with
> > > > > the below entries in ALSA driver for playback/record use case.
> > > > >
> > > > > dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
> > > > >        <&dmac 0x256 0x10049c1c
> > > > > CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> > > > > dma-names = "tx", "rx";
> > > > >
> > > > > Using first parameter, it gets dmac channel. using second and
> > > > > third parameter it configures the channel.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 16:28               ` Laurent Pinchart
@ 2021-06-14 16:33                 ` Biju Das
  2021-06-14 17:30                   ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2021-06-14 16:33 UTC (permalink / raw
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Vinod Koul, Rob Herring, Chris Brandt,
	dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas

Hi Laurent,

> Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> 
> Hi Biju,
> 
> On Mon, Jun 14, 2021 at 04:24:38PM +0000, Biju Das wrote:
> > > On Mon, Jun 14, 2021 at 04:09:04PM +0000, Biju Das wrote:
> > > > > On Mon, Jun 14, 2021 at 12:54:02PM +0000, Biju Das wrote:
> > > > > > > On Fri, Jun 11, 2021 at 1:36 PM Biju Das wrote:
> > > > > > > > Document RZ/G2L DMAC bindings.
> > > > > > > >
> > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > Reviewed-by: Lad Prabhakar
> > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > >
> > > > > > > Thanks for your patch!
> > > > > > >
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dma
> > > > > > > > +++ c.ya
> > > > > > > > +++ ml
> > > > > > > > @@ -0,0 +1,132 @@
> > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > > +%YAML
> > > > > > > > +1.2
> > > > > > > > +---
> > > > > > > > +$id:
> > > > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%
> > > > > > > > +3A%2
> > > > > > > > +F%2F
> > > > > > > > +devi
> > > > > > > > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&am
> > > > > > > > +p;da
> > > > > > > > +ta=0
> > > > > > > > +4%7C
> > > > > > > > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4d850
> > > > > > > > +8d92
> > > > > > > > +f2da
> > > > > > > > +0c0%
> > > > > > > > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637592695286
> > > > > > > > +8468
> > > > > > > > +09%7
> > > > > > > > +CUnk
> > > > > > > > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> > > > > > > > +JBTi
> > > > > > > > +I6Ik
> > > > > > > > +1haW
> > > > > > > > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrViQCc
> > > > > > > > +rNtz
> > > > > > > > +uDej
> > > > > > > > +p8wo
> > > > > > > > +Nrx9iO0ht8%3D&amp;reserved=0
> > > > > > > > +$schema:
> > > > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%
> > > > > > > > +3A%2
> > > > > > > > +F%2F
> > > > > > > > +devi
> > > > > > > > +cetree.org%2Fmeta-
> > > > > schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > > > > > > > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%7C
> > > > > > > > +53d8
> > > > > > > > +2571
> > > > > > > > +da19
> > > > > > > > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnkno
> > > > > > > > +wn%7
> > > > > > > > +CTWF
> > > > > > > > +pbGZ
> > > > > > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> > > > > > > > +LCJX
> > > > > > > > +VCI6
> > > > > > > > +Mn0%
> > > > > > > > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJgRIA
> > > > > > > > +dLnh
> > > > > > > > +Jraw
> > > > > > > > +%3D&
> > > > > > > > +amp;reserved=0
> > > > >
> > > > > *sigh*
> > > > >
> > > > > > > > +
> > > > > > > > +title: Renesas RZ/G2L DMA Controller
> > > > > > > > +
> > > > > > > > +maintainers:
> > > > > > > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > +
> > > > > > > > +allOf:
> > > > > > > > +  - $ref: "dma-controller.yaml#"
> > > > > > > > +
> > > > > > > > +properties:
> > > > > > > > +  compatible:
> > > > > > > > +    items:
> > > > > > > > +      - enum:
> > > > > > > > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > > > > > >
> > > > > > > Please use "renesas,r9a07g044-dmac".
> > > > > >
> > > > > > OK. Will change.
> > > > > >
> > > > > > > > +      - const: renesas,rz-dmac
> > > > > > >
> > > > > > > Does this need many changes for RZ/A1H and RZ/A2M?
> > > > > >
> > > > > > It will work on both RZ/A1H and RZ/A2M. I have n't tested since
> I don't have the board.
> > > > > > There is some difference in MID bit size. Other wise both
> identical.
> > > > > >
> > > > > > > > +  renesas,rz-dmac-slavecfg:
> > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > > > > +    description: |
> > > > > > > > +      DMA configuration for a slave channel. Each channel
> > > > > > > > + must have an array of
> > > > > > > > +      3 items as below.
> > > > > > > > +      first item in the array is MID+RID
> > > > > > >
> > > > > > > Already in dmas.
> > > > > > >
> > > > > > > > +      second item in the array is slave src or dst
> > > > > > > > + address
> > > > > > >
> > > > > > > As pointed out by Rob, already known by the slave driver.
> > > > > > >
> > > > > > > > +      third item in the array is channel configuration
> value.
> > > > > > >
> > > > > > > What exactly is this?
> > > > >
> > > > > What would prevent the DMA client from passing the configuration
> > > > > to the DMA channel through the DMA engine API, just like it
> > > > > passes the slave source or destination address ?
> > > >
> > > > On RZ/G2L, there is 1 case(SSIF ch2) where MID+RID is same for both
> tx and rx.
> > > > The only way we can distinguish it is from channel configuration
> value.
> > >
> > > Are those two different hardware DMA channels ? And configuration
> > > values change between the two ?
> >
> > Yes, REQD is different, apart from this Rx have transfer source and Tx
> have Transfer destination.
> > This particular SSIF ch2 is used only for half duplex compared to other
> SSIF channels.
> 
> Does this mean there's a single DMA channel, used by two clients, but not
> at the same time as it only supports half-duplex ?


From hardware perspective, it is 2 channel. For eg:- playback/recording use case.
You cannot do simultaneous playback, but you can do playback or record separately.

Cheers,
Biju

> > > > > > > Does the R-Car DMAC have this too? If yes, how does its
> > > > > > > driver handle it?
> > > > > >
> > > > > > On R-CAR DMAC, we have only MID + RID values. Where as here we
> > > > > > have channel configuration value With different set of
> > > > > > parameter as mentioned in Table 16.4.
> > > > > >
> > > > > > Please see Page 569, Table 16.4 On-Chip Module requests section.
> > > > > >
> > > > > > For eg:- as per Rob's suggestion, I have modelled the driver
> > > > > > with the below entries in ALSA driver for playback/record use
> case.
> > > > > >
> > > > > > dmas = <&dmac 0x255 0x10049c18
> CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
> > > > > >        <&dmac 0x256 0x10049c1c
> > > > > > CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> > > > > > dma-names = "tx", "rx";
> > > > > >
> > > > > > Using first parameter, it gets dmac channel. using second and
> > > > > > third parameter it configures the channel.
> 
> --
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 16:33                 ` Biju Das
@ 2021-06-14 17:30                   ` Laurent Pinchart
  2021-06-15  8:06                     ` Biju Das
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2021-06-14 17:30 UTC (permalink / raw
  To: Biju Das
  Cc: Geert Uytterhoeven, Vinod Koul, Rob Herring, Chris Brandt,
	dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas

Hi Biju,

On Mon, Jun 14, 2021 at 04:33:03PM +0000, Biju Das wrote:
> > On Mon, Jun 14, 2021 at 04:24:38PM +0000, Biju Das wrote:
> > > > On Mon, Jun 14, 2021 at 04:09:04PM +0000, Biju Das wrote:
> > > > > > On Mon, Jun 14, 2021 at 12:54:02PM +0000, Biju Das wrote:
> > > > > > > > On Fri, Jun 11, 2021 at 1:36 PM Biju Das wrote:
> > > > > > > > > Document RZ/G2L DMAC bindings.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > >
> > > > > > > > Thanks for your patch!
> > > > > > > >
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dma
> > > > > > > > > +++ c.ya
> > > > > > > > > +++ ml
> > > > > > > > > @@ -0,0 +1,132 @@
> > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > > > +%YAML
> > > > > > > > > +1.2
> > > > > > > > > +---
> > > > > > > > > +$id:
> > > > > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%
> > > > > > > > > +3A%2
> > > > > > > > > +F%2F
> > > > > > > > > +devi
> > > > > > > > > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%23&am
> > > > > > > > > +p;da
> > > > > > > > > +ta=0
> > > > > > > > > +4%7C
> > > > > > > > > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4d850
> > > > > > > > > +8d92
> > > > > > > > > +f2da
> > > > > > > > > +0c0%
> > > > > > > > > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637592695286
> > > > > > > > > +8468
> > > > > > > > > +09%7
> > > > > > > > > +CUnk
> > > > > > > > > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> > > > > > > > > +JBTi
> > > > > > > > > +I6Ik
> > > > > > > > > +1haW
> > > > > > > > > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrViQCc
> > > > > > > > > +rNtz
> > > > > > > > > +uDej
> > > > > > > > > +p8wo
> > > > > > > > > +Nrx9iO0ht8%3D&amp;reserved=0
> > > > > > > > > +$schema:
> > > > > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%
> > > > > > > > > +3A%2
> > > > > > > > > +F%2F
> > > > > > > > > +devi
> > > > > > > > > +cetree.org%2Fmeta-
> > > > > > schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > > > > > > > > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c0%7C
> > > > > > > > > +53d8
> > > > > > > > > +2571
> > > > > > > > > +da19
> > > > > > > > > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CUnkno
> > > > > > > > > +wn%7
> > > > > > > > > +CTWF
> > > > > > > > > +pbGZ
> > > > > > > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> > > > > > > > > +LCJX
> > > > > > > > > +VCI6
> > > > > > > > > +Mn0%
> > > > > > > > > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJgRIA
> > > > > > > > > +dLnh
> > > > > > > > > +Jraw
> > > > > > > > > +%3D&
> > > > > > > > > +amp;reserved=0
> > > > > >
> > > > > > *sigh*
> > > > > >
> > > > > > > > > +
> > > > > > > > > +title: Renesas RZ/G2L DMA Controller
> > > > > > > > > +
> > > > > > > > > +maintainers:
> > > > > > > > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > > +
> > > > > > > > > +allOf:
> > > > > > > > > +  - $ref: "dma-controller.yaml#"
> > > > > > > > > +
> > > > > > > > > +properties:
> > > > > > > > > +  compatible:
> > > > > > > > > +    items:
> > > > > > > > > +      - enum:
> > > > > > > > > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > > > > > > >
> > > > > > > > Please use "renesas,r9a07g044-dmac".
> > > > > > >
> > > > > > > OK. Will change.
> > > > > > >
> > > > > > > > > +      - const: renesas,rz-dmac
> > > > > > > >
> > > > > > > > Does this need many changes for RZ/A1H and RZ/A2M?
> > > > > > >
> > > > > > > It will work on both RZ/A1H and RZ/A2M. I have n't tested since I don't have the board.
> > > > > > > There is some difference in MID bit size. Other wise both identical.
> > > > > > >
> > > > > > > > > +  renesas,rz-dmac-slavecfg:
> > > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > > > > > +    description: |
> > > > > > > > > +      DMA configuration for a slave channel. Each channel
> > > > > > > > > + must have an array of
> > > > > > > > > +      3 items as below.
> > > > > > > > > +      first item in the array is MID+RID
> > > > > > > >
> > > > > > > > Already in dmas.
> > > > > > > >
> > > > > > > > > +      second item in the array is slave src or dst
> > > > > > > > > + address
> > > > > > > >
> > > > > > > > As pointed out by Rob, already known by the slave driver.
> > > > > > > >
> > > > > > > > > +      third item in the array is channel configuration value.
> > > > > > > >
> > > > > > > > What exactly is this?
> > > > > >
> > > > > > What would prevent the DMA client from passing the configuration
> > > > > > to the DMA channel through the DMA engine API, just like it
> > > > > > passes the slave source or destination address ?
> > > > >
> > > > > On RZ/G2L, there is 1 case(SSIF ch2) where MID+RID is same for both tx and rx.
> > > > > The only way we can distinguish it is from channel configuration value.
> > > >
> > > > Are those two different hardware DMA channels ? And configuration
> > > > values change between the two ?
> > >
> > > Yes, REQD is different, apart from this Rx have transfer source and Tx
> > > have Transfer destination.
> > > This particular SSIF ch2 is used only for half duplex compared to other
> > > SSIF channels.
> > 
> > Does this mean there's a single DMA channel, used by two clients, but not
> > at the same time as it only supports half-duplex ?
> 
> From hardware perspective, it is 2 channel. For eg:- playback/recording use case.
> You cannot do simultaneous playback, but you can do playback or record separately.

If the two channels have the same MID+RID and only differ by the
direction, I'd add a cell in the dmas property with the direction only.
The source/destination address should be dropped, as it's already known
by the driver.

This being said, in your example below, you have

dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
       <&dmac 0x256 0x10049c1c CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
dma-names = "tx", "rx";

This looks like different MID+RID values for the two channels.

> > > > > > > > Does the R-Car DMAC have this too? If yes, how does its
> > > > > > > > driver handle it?
> > > > > > >
> > > > > > > On R-CAR DMAC, we have only MID + RID values. Where as here we
> > > > > > > have channel configuration value With different set of
> > > > > > > parameter as mentioned in Table 16.4.
> > > > > > >
> > > > > > > Please see Page 569, Table 16.4 On-Chip Module requests section.
> > > > > > >
> > > > > > > For eg:- as per Rob's suggestion, I have modelled the driver
> > > > > > > with the below entries in ALSA driver for playback/record use case.
> > > > > > >
> > > > > > > dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
> > > > > > >        <&dmac 0x256 0x10049c1c CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> > > > > > > dma-names = "tx", "rx";
> > > > > > >
> > > > > > > Using first parameter, it gets dmac channel. using second and
> > > > > > > third parameter it configures the channel.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
  2021-06-14 17:30                   ` Laurent Pinchart
@ 2021-06-15  8:06                     ` Biju Das
  0 siblings, 0 replies; 21+ messages in thread
From: Biju Das @ 2021-06-15  8:06 UTC (permalink / raw
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Vinod Koul, Rob Herring, Chris Brandt,
	dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Prabhakar Mahadev Lad, Linux-Renesas

Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings
> 
> Hi Biju,
> 
> On Mon, Jun 14, 2021 at 04:33:03PM +0000, Biju Das wrote:
> > > On Mon, Jun 14, 2021 at 04:24:38PM +0000, Biju Das wrote:
> > > > > On Mon, Jun 14, 2021 at 04:09:04PM +0000, Biju Das wrote:
> > > > > > > On Mon, Jun 14, 2021 at 12:54:02PM +0000, Biju Das wrote:
> > > > > > > > > On Fri, Jun 11, 2021 at 1:36 PM Biju Das wrote:
> > > > > > > > > > Document RZ/G2L DMAC bindings.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > > > Reviewed-by: Lad Prabhakar
> > > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > > >
> > > > > > > > > Thanks for your patch!
> > > > > > > > >
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz
> > > > > > > > > > +++ -dma
> > > > > > > > > > +++ c.ya
> > > > > > > > > > +++ ml
> > > > > > > > > > @@ -0,0 +1,132 @@
> > > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR
> > > > > > > > > > +BSD-2-Clause) %YAML
> > > > > > > > > > +1.2
> > > > > > > > > > +---
> > > > > > > > > > +$id:
> > > > > > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=h
> > > > > > > > > > +ttp%
> > > > > > > > > > +3A%2
> > > > > > > > > > +F%2F
> > > > > > > > > > +devi
> > > > > > > > > > +cetree.org%2Fschemas%2Fdma%2Frenesas%2Crz-dmac.yaml%2
> > > > > > > > > > +3&am
> > > > > > > > > > +p;da
> > > > > > > > > > +ta=0
> > > > > > > > > > +4%7C
> > > > > > > > > > +01%7Cbiju.das.jz%40bp.renesas.com%7C4b547e10cbe64b6f4
> > > > > > > > > > +d850
> > > > > > > > > > +8d92
> > > > > > > > > > +f2da
> > > > > > > > > > +0c0%
> > > > > > > > > > +7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63759269
> > > > > > > > > > +5286
> > > > > > > > > > +8468
> > > > > > > > > > +09%7
> > > > > > > > > > +CUnk
> > > > > > > > > > +nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz
> > > > > > > > > > +IiLC
> > > > > > > > > > +JBTi
> > > > > > > > > > +I6Ik
> > > > > > > > > > +1haW
> > > > > > > > > > +wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5Jh%2FxPaia5ZOY0CrV
> > > > > > > > > > +iQCc
> > > > > > > > > > +rNtz
> > > > > > > > > > +uDej
> > > > > > > > > > +p8wo
> > > > > > > > > > +Nrx9iO0ht8%3D&amp;reserved=0
> > > > > > > > > > +$schema:
> > > > > > > > > > +https://jpn01.safelinks.protection.outlook.com/?url=h
> > > > > > > > > > +ttp%
> > > > > > > > > > +3A%2
> > > > > > > > > > +F%2F
> > > > > > > > > > +devi
> > > > > > > > > > +cetree.org%2Fmeta-
> > > > > > > schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
> > > > > > > > > > +jz%40bp.renesas.com%7C4b547e10cbe64b6f4d8508d92f2da0c
> > > > > > > > > > +0%7C
> > > > > > > > > > +53d8
> > > > > > > > > > +2571
> > > > > > > > > > +da19
> > > > > > > > > > +47e49cb4625a166a4a2a%7C0%7C0%7C637592695286846809%7CU
> > > > > > > > > > +nkno
> > > > > > > > > > +wn%7
> > > > > > > > > > +CTWF
> > > > > > > > > > +pbGZ
> > > > > > > > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> > > > > > > > > > +aWwi
> > > > > > > > > > +LCJX
> > > > > > > > > > +VCI6
> > > > > > > > > > +Mn0%
> > > > > > > > > > +3D%7C1000&amp;sdata=5qQ1PljM3e4Bn4%2FjdldYUHRBQL3jArJ
> > > > > > > > > > +gRIA
> > > > > > > > > > +dLnh
> > > > > > > > > > +Jraw
> > > > > > > > > > +%3D&
> > > > > > > > > > +amp;reserved=0
> > > > > > >
> > > > > > > *sigh*
> > > > > > >
> > > > > > > > > > +
> > > > > > > > > > +title: Renesas RZ/G2L DMA Controller
> > > > > > > > > > +
> > > > > > > > > > +maintainers:
> > > > > > > > > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > > > +
> > > > > > > > > > +allOf:
> > > > > > > > > > +  - $ref: "dma-controller.yaml#"
> > > > > > > > > > +
> > > > > > > > > > +properties:
> > > > > > > > > > +  compatible:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - enum:
> > > > > > > > > > +          - renesas,dmac-r9a07g044  # RZ/G2{L,LC}
> > > > > > > > >
> > > > > > > > > Please use "renesas,r9a07g044-dmac".
> > > > > > > >
> > > > > > > > OK. Will change.
> > > > > > > >
> > > > > > > > > > +      - const: renesas,rz-dmac
> > > > > > > > >
> > > > > > > > > Does this need many changes for RZ/A1H and RZ/A2M?
> > > > > > > >
> > > > > > > > It will work on both RZ/A1H and RZ/A2M. I have n't tested
> since I don't have the board.
> > > > > > > > There is some difference in MID bit size. Other wise both
> identical.
> > > > > > > >
> > > > > > > > > > +  renesas,rz-dmac-slavecfg:
> > > > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > > > > > > +    description: |
> > > > > > > > > > +      DMA configuration for a slave channel. Each
> > > > > > > > > > + channel must have an array of
> > > > > > > > > > +      3 items as below.
> > > > > > > > > > +      first item in the array is MID+RID
> > > > > > > > >
> > > > > > > > > Already in dmas.
> > > > > > > > >
> > > > > > > > > > +      second item in the array is slave src or dst
> > > > > > > > > > + address
> > > > > > > > >
> > > > > > > > > As pointed out by Rob, already known by the slave driver.
> > > > > > > > >
> > > > > > > > > > +      third item in the array is channel configuration
> value.
> > > > > > > > >
> > > > > > > > > What exactly is this?
> > > > > > >
> > > > > > > What would prevent the DMA client from passing the
> > > > > > > configuration to the DMA channel through the DMA engine API,
> > > > > > > just like it passes the slave source or destination address ?
> > > > > >
> > > > > > On RZ/G2L, there is 1 case(SSIF ch2) where MID+RID is same for
> both tx and rx.
> > > > > > The only way we can distinguish it is from channel configuration
> value.
> > > > >
> > > > > Are those two different hardware DMA channels ? And
> > > > > configuration values change between the two ?
> > > >
> > > > Yes, REQD is different, apart from this Rx have transfer source
> > > > and Tx have Transfer destination.
> > > > This particular SSIF ch2 is used only for half duplex compared to
> > > > other SSIF channels.
> > >
> > > Does this mean there's a single DMA channel, used by two clients,
> > > but not at the same time as it only supports half-duplex ?
> >
> > From hardware perspective, it is 2 channel. For eg:- playback/recording
> use case.
> > You cannot do simultaneous playback, but you can do playback or record
> separately.
> 
> If the two channels have the same MID+RID and only differ by the
> direction, I'd add a cell in the dmas property with the direction only.
> The source/destination address should be dropped, as it's already known by
> the driver.

I have cross checked the manual again and it seems it is same DMA Tranfer request signal(ssif_dma_rt) for that
particular Dma client (SSIF ch2). So it is just one DMA. SO I will drop cell2 and cell3 and just use cell1 with 
MID+RID values in next version.


> This being said, in your example below, you have
> 
> dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
>        <&dmac 0x256 0x10049c1c CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> dma-names = "tx", "rx";
> 
> This looks like different MID+RID values for the two channels.

Yes, it is for SSIF ch0. Where it supports full duplex and it has DMA Tranfer request signal
ssif_dma_rx0 for receive and ssif_dma_tx0 for transmit.

Thanks,
Biju

> 
> > > > > > > > > Does the R-Car DMAC have this too? If yes, how does its
> > > > > > > > > driver handle it?
> > > > > > > >
> > > > > > > > On R-CAR DMAC, we have only MID + RID values. Where as
> > > > > > > > here we have channel configuration value With different
> > > > > > > > set of parameter as mentioned in Table 16.4.
> > > > > > > >
> > > > > > > > Please see Page 569, Table 16.4 On-Chip Module requests
> section.
> > > > > > > >
> > > > > > > > For eg:- as per Rob's suggestion, I have modelled the
> > > > > > > > driver with the below entries in ALSA driver for
> playback/record use case.
> > > > > > > >
> > > > > > > > dmas = <&dmac 0x255 0x10049c18
> CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
> > > > > > > >        <&dmac 0x256 0x10049c1c
> > > > > > > > CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> > > > > > > > dma-names = "tx", "rx";
> > > > > > > >
> > > > > > > > Using first parameter, it gets dmac channel. using second
> > > > > > > > and third parameter it configures the channel.
> 
> --
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
  2021-06-14 13:48       ` Geert Uytterhoeven
@ 2021-06-15  8:12         ` Biju Das
  0 siblings, 0 replies; 21+ messages in thread
From: Biju Das @ 2021-06-15  8:12 UTC (permalink / raw
  To: Geert Uytterhoeven
  Cc: Rob Herring, Magnus Damm, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Paterson, Chris Brandt, Prabhakar Mahadev Lad,
	Laurent Pinchart

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
> 
> Hi Biju,
> 
> On Mon, Jun 14, 2021 at 3:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC
> > > support On Fri, Jun 11, 2021 at 1:36 PM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > Add DMAC support to RZ/G2L SoC DT.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Reviewed-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > @@ -8,6 +8,10 @@
> > > >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > >  #include <dt-bindings/clock/r9a07g044-cpg.h>
> > > >
> > > > +#define CH_CFG(reqd, loen, hien, lvl, am, sds, dds, tm) \
> > > > +       ((((tm) << 22) | ((dds) << 16) | ((sds) << 12) | ((am) << 8)
> | \
> > > > +       ((lvl) << 6) | ((hien) << 5) | ((loen) << 4) | ((reqd) <<
> > > > +3)) & 0x004FF778)
> > > > +
> > >
> > > I assume the above will be removed?
> >
> > Basically the macro simplifies the channel configuration values in Table
> 16.4 page 569 of the hardware manual.
> >
> > Client driver will use MID+RID, and pass (Src address or dest address
> > along with the channel configuration values For configuring DMA channel.
> >
> > For eg:-
> >
> >                 ssi0: ssi@10049c00 {
> >                         compatible = "renesas,r9a07g044-ssi",
> >                                      "renesas,rz-ssi";
> >                         reg = <0 0x10049c00 0 0x400>;
> >                         interrupts = <GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH>,
> >                                      <GIC_SPI 327 IRQ_TYPE_EDGE_RISING>,
> >                                      <GIC_SPI 328 IRQ_TYPE_EDGE_RISING>;
> >                         interrupt-names = "int", "rx", "tx";
> >                         clocks = <&cpg CPG_MOD R9A07G044_CLK_SSI0>,
> >                                  <&audio_clk1>,
> >                                  <&audio_clk2>;
> >                         clock-names = "ssi", "audio_clk1", "audio_clk2";
> >                         resets = <&cpg R9A07G044_CLK_SSI0>;
> >                         dmas = <&dmac 0x255 0x10049c18
> CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
> >                                <&dmac 0x256 0x10049c1c
> CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
> >                         dma-names = "tx", "rx";
> >                         power-domains = <&cpg>;
> >                         #sound-dai-cells = <0>;
> >                         status = "disabled";
> >                 };
> >
> > Please let me know your thoughts on this.
> 
> How will this work with (existing) drivers?
> E.g. drivers/tty/serial/sh-sci.c:sci_request_dma_chan() already knows the
> source and destination addresses.
> The other CHCFG bits may be new, though.

OK will use only MID+RID for the next version and will drop CH_CFG Macro.
CH_CFG values can be supplied through DMA api's.

Regards,
Biju

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-06-15  8:12 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-11 11:36 [PATCH 0/5] Add RZ/G2L DMAC support Biju Das
2021-06-11 11:36 ` [PATCH 1/5] dt-bindings: dma: Document RZ/G2L bindings Biju Das
2021-06-11 17:55   ` Rob Herring
2021-06-12 12:17     ` Biju Das
2021-06-11 19:39   ` Rob Herring
2021-06-12 12:26     ` Biju Das
2021-06-14 12:11   ` Geert Uytterhoeven
2021-06-14 12:54     ` Biju Das
2021-06-14 14:29       ` Laurent Pinchart
2021-06-14 16:09         ` Biju Das
2021-06-14 16:17           ` Laurent Pinchart
2021-06-14 16:24             ` Biju Das
2021-06-14 16:28               ` Laurent Pinchart
2021-06-14 16:33                 ` Biju Das
2021-06-14 17:30                   ` Laurent Pinchart
2021-06-15  8:06                     ` Biju Das
2021-06-11 11:36 ` [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support Biju Das
2021-06-14 12:15   ` Geert Uytterhoeven
2021-06-14 13:02     ` Biju Das
2021-06-14 13:48       ` Geert Uytterhoeven
2021-06-15  8:12         ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).