Add Intel LGM soc DMA support

Add DT bindings YAML schema for DMA controller driver of Lightning Mountain(LGM) SoC.

Signed-off-by Amireddy Mallikarjuna reddy <[email protected]>

---
v1:
- Initial version.

v2:
- Fix bot errors.

v3:
- No change.

v4:
- Address Thomas langer comments
- use node name pattern as dma-controller as in common binding.
- Remove "_" (underscore) in instance name.
- Remove "port-" and "chan-" in attribute name for both 'dma-ports' & 'dma-channels' child nodes.

v5:
- Moved some of the attributes in 'dma-ports' & 'dma-channels' child nodes to dma client/consumer side as cells in 'dmas' properties.

---
 .../devicetree/bindings/dma/intel,ldma.yaml        | 319 +++++++++++++++++++++
 1 file changed, 319 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/intel,ldma.yaml

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +11 -0

new file mode 100644
index 000000000000..9beaf191a6de
@@ -0,0 +1,319 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/intel,ldma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lightning Mountain centralized low speed DMA and high speed DMA controllers.
+
+maintainers:
+  - [email protected]
+  - [email protected]

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +76 -0

+
+ reg:
+  maxItems: 1
+
+ clocks:
+  maxItems: 1
+
+ resets:
+  maxItems: 1
+
+ interrupts:
+  maxItems: 1
+
+ intel,dma-poll-cnt:
+   $ref: /schemas/types.yaml#definitions/uint32
+   description:
+     DMA descriptor polling counter. It may need fine tune according
+     to the system application scenario.
+
+ intel,dma-byte-en:
+   type: boolean
+   description:
+     DMA byte enable is only valid for DMA write(RX).
+     Byte enable(1) means DMA write will be based on the number of dwords
+     instead of the whole burst.
+
+ intel,dma-drb:
+    type: boolean
+    description:
+      DMA descriptor read back to make sure data and desc synchronization.
+
+ intel,dma-burst:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+       Specifiy the DMA burst size(in dwords), the valid value will be 8, 16, 32.
+       Default is 16 for data path dma, 32 is for memcopy DMA.
+
+ intel,dma-polling-cnt:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+       DMA descriptor polling counter. It may need fine tune according to
+       the system application scenario.
+
+ intel,dma-desc-in-sram:
+    type: boolean
+    description:
+       DMA descritpors in SRAM or not. Some old controllers descriptors
+       can be in DRAM or SRAM. The new ones are all in SRAM.
+
+ intel,dma-orrc:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+       DMA outstanding read counter. The maximum value is 16, and it may
+       need fine tune according to the system application scenarios.
+
+ intel,dma-dburst-wr:
+    type: boolean
+    description:
+       Enable RX dynamic burst write. It only applies to RX DMA and memcopy DMA.
+
+
+ dma-ports:
+    type: object
+    description:
+       This sub-node must contain a sub-node for each DMA port.
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^dma-ports@[0-9]+$":
+          type: object
+
+          properties:

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +37 -0

+
+          required:
+            - reg
+            - intel,name
+            - intel,chans
+
+
+ ldma-channels:
+    type: object
+    description:
+       This sub-node must contain a sub-node for each DMA channel.
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^ldma-channels@[0-15]+$":
+          type: object
+
+          properties:
+            reg:
+              items:
+                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
+              description:
+                 Which channel this node refers to.
+
+            intel,desc_num:
+              $ref: /schemas/types.yaml#/definitions/uint32
+              description:
+                 Per channel maximum descriptor number. The max value is 255.
+
+            intel,hdr-mode:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The first parameter is header mode size, the second

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +129 -0

+     clocks = <&cgu0 80>;
+     intel,dma-poll-cnt = <4>;
+     intel,dma-byte-en;
+     intel,dma-drb;
+     dma-ports {
+       #address-cells = <1>;
+       #size-cells = <0>;
+
+       dma-ports@0 {
+           reg = <0>;
+           intel,name = "SPI0";
+           intel,chans = <0 2>;
+       };
+       dma-ports@1 {
+           reg = <1>;
+           intel,name = "SPI1";
+           intel,chans = <2 2>;
+       };
+       dma-ports@2 {
+           reg = <2>;
+           intel,name = "SPI2";
+           intel,chans = <4 2>;
+       };
+       dma-ports@3 {
+           reg = <3>;
+           intel,name = "SPI3";
+           intel,chans = <6 2>;
+       };
+       dma-ports@4 {
+           reg = <4>;
+           intel,name = "HSNAND";
+           intel,chans = <8 2>;
+       };
+       dma-ports@5 {
+           reg = <5>;
+           intel,name = "PCM";
+           intel,chans = <10 6>;
+       };
+     };
+     ldma-channels {
+       #address-cells = <1>;
+       #size-cells = <0>;
+
+       ldma-channels@0 {
+           reg = <0>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@1 {
+           reg = <1>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@2 {
+           reg = <2>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@3 {
+           reg = <3>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@4 {
+           reg = <4>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@5 {
+           reg = <5>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@6 {
+           reg = <6>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@7 {
+           reg = <7>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@8 {
+           reg = <8>;
+       };
+       ldma-channels@9 {
+           reg = <9>;
+       };
+       ldma-channels@10 {
+           reg = <10>;
+       };
+       ldma-channels@11 {
+           reg = <11>;
+       };
+       ldma-channels@12 {
+           reg = <12>;
+       };
+       ldma-channels@13 {
+           reg = <13>;
+       };
+       ldma-channels@14 {
+           reg = <14>;
+       };
+       ldma-channels@15 {
+           reg = <15>;
+       };
+     };
+   };
+ - |
+   dma3: dma-controller@ec800000 {
+     compatible = "intel,lgm-dma3";
+     reg = <0xec800000 0x1000>;
+     clocks = <&cgu0 71>;
+     resets = <&rcu0 0x10 9>;
+     #dma-cells = <7>;
+     intel,dma-burst = <32>;
+     intel,dma-polling-cnt = <16>;
+     intel,dma-desc-in-sram;
+     intel,dma-orrc = <16>;
+     intel,dma-byte-en;
+     intel,dma-dburst-wr;
+     ldma-channels {
+         #address-cells = <1>;
+         #size-cells = <0>;
+
+         ldma-channels@12 {
+             reg = <12>;
+             intel,hdr-mode = <128 0>;
+             intel,hw-desc = <0x20000000 8>;
+         };
+         ldma-channels@13 {
+             reg = <13>;
+             intel,hdr-mode = <128 0>;
+         };
+     };
+   };

Signature · 1 line

2.11.0

Quoted 47 lines · Amireddy Mallikarjuna reddy

Add DT bindings YAML schema for DMA controller driver of Lightning Mountain(LGM) SoC.

Signed-off-by Amireddy Mallikarjuna reddy <[email protected]>

---
v1:
- Initial version.

v2:
- Fix bot errors.

v3:
- No change.

v4:
- Address Thomas langer comments
- use node name pattern as dma-controller as in common binding.
- Remove "_" (underscore) in instance name.
- Remove "port-" and "chan-" in attribute name for both 'dma-ports' & 'dma-channels' child nodes.

v5:
- Moved some of the attributes in 'dma-ports' & 'dma-channels' child nodes to dma client/consumer side as cells in 'dmas' properties.

---
  .../devicetree/bindings/dma/intel,ldma.yaml        | 319 +++++++++++++++++++++
  1 file changed, 319 insertions(+)

create mode 100644 Documentation/devicetree/bindings/dma/intel,ldma.yaml

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +14 -0

new file mode 100644
index 000000000000..9beaf191a6de
@@ -0,0 +1,319 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/intel,ldma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lightning Mountain centralized low speed DMA and high speed DMA controllers.
+
+maintainers:
+  - [email protected]
+  - [email protected]
+
+allOf:
+  - $ref: "dma-controller.yaml#"

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 21

Quoted 7 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +7 -0

+
+properties:
+ $nodename:
+   pattern: "^dma-controller(@.*)?$"
+
+ "#dma-cells":
+   const: 1

Example says 3.

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 32

Quoted 12 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +11 -0

+
+ compatible:
+  anyOf:
+   - const: intel,lgm-cdma
+   - const: intel,lgm-dma2tx
+   - const: intel,lgm-dma1rx
+   - const: intel,lgm-dma1tx
+   - const: intel,lgm-dma0tx
+   - const: intel,lgm-dma3
+   - const: intel,lgm-toe-dma30
+   - const: intel,lgm-toe-dma31

'anyOf' doesn't make sense here. This should be a single 'enum'.

Quoted 80 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +79 -0

+
+ reg:
+  maxItems: 1
+
+ clocks:
+  maxItems: 1
+
+ resets:
+  maxItems: 1
+
+ interrupts:
+  maxItems: 1
+
+ intel,dma-poll-cnt:
+   $ref: /schemas/types.yaml#definitions/uint32
+   description:
+     DMA descriptor polling counter. It may need fine tune according
+     to the system application scenario.
+
+ intel,dma-byte-en:
+   type: boolean
+   description:
+     DMA byte enable is only valid for DMA write(RX).
+     Byte enable(1) means DMA write will be based on the number of dwords
+     instead of the whole burst.
+
+ intel,dma-drb:
+    type: boolean
+    description:
+      DMA descriptor read back to make sure data and desc synchronization.
+
+ intel,dma-burst:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+       Specifiy the DMA burst size(in dwords), the valid value will be 8, 16, 32.
+       Default is 16 for data path dma, 32 is for memcopy DMA.
+
+ intel,dma-polling-cnt:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+       DMA descriptor polling counter. It may need fine tune according to
+       the system application scenario.
+
+ intel,dma-desc-in-sram:
+    type: boolean
+    description:
+       DMA descritpors in SRAM or not. Some old controllers descriptors
+       can be in DRAM or SRAM. The new ones are all in SRAM.
+
+ intel,dma-orrc:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+       DMA outstanding read counter. The maximum value is 16, and it may
+       need fine tune according to the system application scenarios.
+
+ intel,dma-dburst-wr:
+    type: boolean
+    description:
+       Enable RX dynamic burst write. It only applies to RX DMA and memcopy DMA.
+
+
+ dma-ports:
+    type: object
+    description:
+       This sub-node must contain a sub-node for each DMA port.
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^dma-ports@[0-9]+$":
+          type: object
+
+          properties:
+            reg:
+              items:
+                - enum: [0, 1, 2, 3, 4, 5]

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 118

Quoted 7 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +7 -0

+              description:
+                 Which port this node refers to.
+
+            intel,name:
+              $ref: /schemas/types.yaml#definitions/string-array
+              description:
+                 Port name of each DMA port.

No other DMA controller needs this, why do you?

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 124

Quoted 7 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +6 -0

+
+            intel,chans:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The channels included on this port. Format is channel start
+                 number and how many channels on this port.

Why does this need to be in DT? This all seems like it can be in the dma cells for each client.

Quoted 41 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +40 -0

+
+          required:
+            - reg
+            - intel,name
+            - intel,chans
+
+
+ ldma-channels:
+    type: object
+    description:
+       This sub-node must contain a sub-node for each DMA channel.
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^ldma-channels@[0-15]+$":
+          type: object
+
+          properties:
+            reg:
+              items:
+                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
+              description:
+                 Which channel this node refers to.
+
+            intel,desc_num:
+              $ref: /schemas/types.yaml#/definitions/uint32
+              description:
+                 Per channel maximum descriptor number. The max value is 255.
+
+            intel,hdr-mode:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The first parameter is header mode size, the second
+                 parameter is checksum enable or disable. If enabled,
+                 header mode size is ignored. If disabled, header mode
+                 size must be provided.

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 171

Quoted 7 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +7 -0

+
+            intel,hw-desc:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 Per channel dma hardware descriptor configuration.
+                 The first parameter is descriptor physical address and the
+                 second parameter hardware descriptor number.

Again, this all seems like per client information for dma cells.

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 179

Quoted 9 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +8 -0

+
+          required:
+            - reg
+
+required:
+ - compatible
+ - reg
+ - '#dma-cells'

Add:

additionalProperties: false

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 190

Quoted 12 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +11 -0

+
+examples:
+ - |
+   dma0: dma-controller@e0e00000 {
+     compatible = "intel,lgm-cdma";
+     reg = <0xe0e00000 0x1000>;
+     #dma-cells = <3>;
+     interrupt-parent = <&ioapic1>;
+     interrupts = <82 1>;
+     resets = <&rcu0 0x30 0>;
+     reset-names = "ctrl";

Not documented.

Quoted 133 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +129 -0

+     clocks = <&cgu0 80>;
+     intel,dma-poll-cnt = <4>;
+     intel,dma-byte-en;
+     intel,dma-drb;
+     dma-ports {
+       #address-cells = <1>;
+       #size-cells = <0>;
+
+       dma-ports@0 {
+           reg = <0>;
+           intel,name = "SPI0";
+           intel,chans = <0 2>;
+       };
+       dma-ports@1 {
+           reg = <1>;
+           intel,name = "SPI1";
+           intel,chans = <2 2>;
+       };
+       dma-ports@2 {
+           reg = <2>;
+           intel,name = "SPI2";
+           intel,chans = <4 2>;
+       };
+       dma-ports@3 {
+           reg = <3>;
+           intel,name = "SPI3";
+           intel,chans = <6 2>;
+       };
+       dma-ports@4 {
+           reg = <4>;
+           intel,name = "HSNAND";
+           intel,chans = <8 2>;
+       };
+       dma-ports@5 {
+           reg = <5>;
+           intel,name = "PCM";
+           intel,chans = <10 6>;
+       };
+     };
+     ldma-channels {
+       #address-cells = <1>;
+       #size-cells = <0>;
+
+       ldma-channels@0 {
+           reg = <0>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@1 {
+           reg = <1>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@2 {
+           reg = <2>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@3 {
+           reg = <3>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@4 {
+           reg = <4>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@5 {
+           reg = <5>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@6 {
+           reg = <6>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@7 {
+           reg = <7>;
+           intel,desc_num = <1>;
+       };
+       ldma-channels@8 {
+           reg = <8>;
+       };
+       ldma-channels@9 {
+           reg = <9>;
+       };
+       ldma-channels@10 {
+           reg = <10>;
+       };
+       ldma-channels@11 {
+           reg = <11>;
+       };
+       ldma-channels@12 {
+           reg = <12>;
+       };
+       ldma-channels@13 {
+           reg = <13>;
+       };
+       ldma-channels@14 {
+           reg = <14>;
+       };
+       ldma-channels@15 {
+           reg = <15>;
+       };
+     };
+   };
+ - |
+   dma3: dma-controller@ec800000 {
+     compatible = "intel,lgm-dma3";
+     reg = <0xec800000 0x1000>;
+     clocks = <&cgu0 71>;
+     resets = <&rcu0 0x10 9>;
+     #dma-cells = <7>;
+     intel,dma-burst = <32>;
+     intel,dma-polling-cnt = <16>;
+     intel,dma-desc-in-sram;
+     intel,dma-orrc = <16>;
+     intel,dma-byte-en;
+     intel,dma-dburst-wr;
+     ldma-channels {
+         #address-cells = <1>;
+         #size-cells = <0>;
+
+         ldma-channels@12 {
+             reg = <12>;
+             intel,hdr-mode = <128 0>;
+             intel,hw-desc = <0x20000000 8>;
+         };
+         ldma-channels@13 {
+             reg = <13>;
+             intel,hdr-mode = <128 0>;
+         };
+     };
+   };

Signature · 1 line

2.11.0

Quoted 8 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 124

Quoted 6 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +6 -0

+
+            intel,chans:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The channels included on this port. Format is channel start
+                 number and how many channels on this port.

Why does this need to be in DT? This all seems like it can be in the dma cells for each client.

(*ABC)
Yes. We need this.
for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22 channels. and the logical channel mapping for the peripherals also differ b/w old and new SOCs.

Because of this hardware limitation we are trying to configure the total channels and port-channel mapping dynamically from device tree.

based on port name we are trying to configure the default values for different peripherals(ports).
Example: burst length is not same for all ports, so using port name to do default configurations.

Quoted 41 lines

Quoted 41 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +40 -0

+
+          required:
+            - reg
+            - intel,name
+            - intel,chans
+
+
+ ldma-channels:
+    type: object
+    description:
+       This sub-node must contain a sub-node for each DMA channel.
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^ldma-channels@[0-15]+$":
+          type: object
+
+          properties:
+            reg:
+              items:
+                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
+              description:
+                 Which channel this node refers to.
+
+            intel,desc_num:
+              $ref: /schemas/types.yaml#/definitions/uint32
+              description:
+                 Per channel maximum descriptor number. The max value is 255.
+
+            intel,hdr-mode:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The first parameter is header mode size, the second
+                 parameter is checksum enable or disable. If enabled,
+                 header mode size is ignored. If disabled, header mode
+                 size must be provided.

Quoted 8 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 171

Quoted 7 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +7 -0

+
+            intel,hw-desc:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 Per channel dma hardware descriptor configuration.
+                 The first parameter is descriptor physical address and the
+                 second parameter hardware descriptor number.

Again, this all seems like per client information for dma cells.

 Ok, if we move all these attributes to 'dmas' then 'dma-channels' child node is not needed in dtsi.
#dma-cells number i am already using 7. If we move all these attributes to 'dmas' then integer cells will increase.

Is there any limitation in using a number of integer cells & as determined by the #dma-cells property?

Quoted 17 lines · MallikarjunaX

Quoted 8 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 124

Quoted 6 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +6 -0

+
+            intel,chans:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The channels included on this port. Format is channel start
+                 number and how many channels on this port.

Why does this need to be in DT? This all seems like it can be in the dma cells for each client.

(*ABC)
Yes. We need this.
for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22 channels. and the logical channel mapping for the peripherals also differ b/w old and new SOCs.

Because of this hardware limitation we are trying to configure the total channels and port-channel mapping dynamically from device tree.

Quoted 4 lines

based on port name we are trying to configure the default values for different peripherals(ports).
Example: burst length is not same for all ports, so using port name to do default configurations.

Sorry that does not make sense to me, why not specify the values to be used here instead of defining your own name scheme!

Only older soc it should create 16 channels and new 22 (hint this is hw description so perfectly okay to specify in DT or in using driver_data and compatible for each version

Quoted 48 lines

Quoted 48 lines

Quoted 37 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +37 -0

+
+          required:
+            - reg
+            - intel,name
+            - intel,chans
+
+
+ ldma-channels:
+    type: object
+    description:
+       This sub-node must contain a sub-node for each DMA channel.
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^ldma-channels@[0-15]+$":
+          type: object
+
+          properties:
+            reg:
+              items:
+                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
+              description:
+                 Which channel this node refers to.
+
+            intel,desc_num:
+              $ref: /schemas/types.yaml#/definitions/uint32
+              description:
+                 Per channel maximum descriptor number. The max value is 255.
+
+            intel,hdr-mode:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The first parameter is header mode size, the second

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 171

Quoted 10 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +10 -0

+                 parameter is checksum enable or disable. If enabled,
+                 header mode size is ignored. If disabled, header mode
+                 size must be provided.
+
+            intel,hw-desc:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 Per channel dma hardware descriptor configuration.
+                 The first parameter is descriptor physical address and the
+                 second parameter hardware descriptor number.

Again, this all seems like per client information for dma cells.

Quoted 7 lines

 Ok, if we move all these attributes to 'dmas' then 'dma-channels' child
node is not needed in dtsi.
#dma-cells number i am already using 7. If we move all these attributes to 'dmas' then integer cells will increase.

Is there any limitation in using a number of integer cells & as determined by the #dma-cells property?

No I dont think there is but it needs to make sense :-)

Hi Vinod,
Thanks for the review comments.

Quoted 19 lines · Vinod Koul

Quoted 17 lines · MallikarjunaX

Quoted 8 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 124

Quoted 6 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +6 -0

+
+            intel,chans:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The channels included on this port. Format is channel start
+                 number and how many channels on this port.

Why does this need to be in DT? This all seems like it can be in the dma cells for each client.

(*ABC)
Yes. We need this.
for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22 channels. and the logical channel mapping for the peripherals also differ b/w old and new SOCs.

Because of this hardware limitation we are trying to configure the total channels and port-channel mapping dynamically from device tree.

Quoted 6 lines

Quoted 4 lines

based on port name we are trying to configure the default values for different peripherals(ports).
Example: burst length is not same for all ports, so using port name to do default configurations.

Sorry that does not make sense to me, why not specify the values to be used here instead of defining your own name scheme!

OK. Agreed. I will remove port name from DT and only use intel,chans

Quoted 23 lines · MallikarjunaX

Hi Vinod,
Thanks for the review comments.

Quoted 19 lines · Vinod Koul

Quoted 17 lines · MallikarjunaX

Quoted 8 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 124

Quoted 6 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +6 -0

+
+            intel,chans:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The channels included on this port. Format is channel start
+                 number and how many channels on this port.

Why does this need to be in DT? This all seems like it can be in the dma cells for each client.

(*ABC)
Yes. We need this.
for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22 channels. and the logical channel mapping for the peripherals also differ b/w old and new SOCs.

Because of this hardware limitation we are trying to configure the total channels and port-channel mapping dynamically from device tree.

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

based on port name we are trying to configure the default values for different peripherals(ports).
Example: burst length is not same for all ports, so using port name to do default configurations.

Sorry that does not make sense to me, why not specify the values to be used here instead of defining your own name scheme!

OK. Agreed. I will remove port name from DT and only use intel,chans

what is intel,chans, why not use dma-channels?

Hi Vinod,

Thanks for the review. Please see my comment inline.

Quoted 28 lines · Vinod Koul

Quoted 27 lines · MallikarjunaX

Hi Vinod,
Thanks for the review comments.

Quoted 23 lines · Vinod Koul

Quoted 21 lines · MallikarjunaX

Quoted 8 lines

Review

Quoted 6 lines

(quoted diff) +6 -0

+
+            intel,chans:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The channels included on this port. Format is channel start
+                 number and how many channels on this port.

Why does this need to be in DT? This all seems like it can be in the dma cells for each client.

(*ABC)
Yes. We need this.
for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22 channels. and the logical channel mapping for the peripherals also differ b/w old and new SOCs.

Because of this hardware limitation we are trying to configure the total channels and port-channel mapping dynamically from device tree.

based on port name we are trying to configure the default values for different peripherals(ports).
Example: burst length is not same for all ports, so using port name to do default configurations.

Quoted 4 lines

Quoted 3 lines

Quoted 2 lines

Sorry that does not make sense to me, why not specify the values to be used here instead of defining your own name scheme!

OK. Agreed. I will remove port name from DT and only use intel,chans

what is intel,chans, why not use dma-channels?

 The intel,chans says about the channels included on the correspondng port.

Format is channel start number and how many channels on this port.

 The reasong behind using this attribute instead of standrad dma-channels is...

DMA_VER22 HW supports 22 channels. But there is a hole in HW, total it can use only 16.

Old soc supports 4ports and 16 channels.
New soc supports 6ports and 22 channels.
(old and new soc carry the same version VER22)

port channel mapping for the old and new soc also not the same. old soc: logical channels:(Rx, Tx)
0, 1 - SPI0
2, 3 - SPI1
4, 5 - HSNAND
12, 14, 13, 15 - Memcopy

New soc: Logical channels(Rx, Tx)
0, 1 - SPI0
2, 3 - SPI1
4, 5 - SPI2
6, 7 - SPI3
8, 9 - HSNAND
10 to 21 - Mcopy

Because of these reasons we are trying to use "intel,chans" attribute, and reading then number of channels from the dt.
Advantaage:
1. we can map the channels correspondign to port
2. Dynamically configure the channels (due to hw limitation)

If this is not ok, please suggest us the better way to handle this.

Quoted 1 line

Quoted 33 lines · MallikarjunaX

Hi Vinod,

Thanks for the review. Please see my comment inline.

Quoted 28 lines · Vinod Koul

Quoted 27 lines · MallikarjunaX

Hi Vinod,
Thanks for the review comments.

Quoted 23 lines · Vinod Koul

Quoted 21 lines · MallikarjunaX

Quoted 8 lines

Review

Quoted 6 lines

(quoted diff) +6 -0

+
+            intel,chans:
+              $ref: /schemas/types.yaml#/definitions/uint32-array
+              description:
+                 The channels included on this port. Format is channel start
+                 number and how many channels on this port.

Why does this need to be in DT? This all seems like it can be in the dma cells for each client.

(*ABC)
Yes. We need this.
for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22 channels. and the logical channel mapping for the peripherals also differ b/w old and new SOCs.

Because of this hardware limitation we are trying to configure the total channels and port-channel mapping dynamically from device tree.

based on port name we are trying to configure the default values for different peripherals(ports).
Example: burst length is not same for all ports, so using port name to do default configurations.

Quoted 5 lines

Quoted 4 lines

Quoted 3 lines

Quoted 2 lines

Sorry that does not make sense to me, why not specify the values to be used here instead of defining your own name scheme!

OK. Agreed. I will remove port name from DT and only use intel,chans

what is intel,chans, why not use dma-channels?

 The intel,chans says about the channels included on the correspondng port.

What do you mean by a port here?

Quoted 2 lines

Format is channel start number and how many channels on this port.

It is perfectly reasonable to have 16 channels but linux not use use all, lets say from 5th channel channel onwards

So you need to use standard dma-channels also with dma-channel-mask to specify which channels linux can use

Quoted 19 lines

 The reasong behind using this attribute instead of standrad dma-channels
is...

DMA_VER22 HW supports 22 channels. But there is a hole in HW, total it can use only 16.

Old soc supports 4ports and 16 channels.
New soc supports 6ports and 22 channels.
(old and new soc carry the same version VER22)

port channel mapping for the old and new soc also not the same. old soc: logical channels:(Rx, Tx)
0, 1 - SPI0
2, 3 - SPI1
4, 5 - HSNAND
12, 14, 13, 15 - Memcopy

Quoted 7 lines

New soc: Logical channels(Rx, Tx)
0, 1 - SPI0
2, 3 - SPI1
4, 5 - SPI2
6, 7 - SPI3
8, 9 - HSNAND
10 to 21 - Mcopy

Mapping is different, client can set that channel required in dmas property and use a specific required channel.

Quoted 8 lines

Because of these reasons we are trying to use "intel,chans" attribute, and reading then number of channels from the dt.
Advantaage:
1. we can map the channels correspondign to port
2. Dynamically configure the channels (due to hw limitation)

If this is not ok, please suggest us the better way to handle this.

Hi Rob,
Thanks for your valuable comments. Please see my comments inline..

Quoted 56 lines · Rob Herring

Quoted 54 lines · Amireddy Mallikarjuna reddy

Add DT bindings YAML schema for DMA controller driver of Lightning Mountain(LGM) SoC.

Signed-off-by: Amireddy Mallikarjuna reddy
<[email protected]>
---
v1:
- Initial version.

v2:
- Fix bot errors.

v3:
- No change.

v4:
- Address Thomas langer comments
   - use node name pattern as dma-controller as in common binding.
   - Remove "_" (underscore) in instance name.
   - Remove "port-" and "chan-" in attribute name for both 'dma-ports' & 'dma-channels' child nodes.

v5:
- Moved some of the attributes in 'dma-ports' & 'dma-channels' child nodes to dma client/consumer side as cells in 'dmas' properties.
---
  .../devicetree/bindings/dma/intel,ldma.yaml        | 319
+++++++++++++++++++++

  1 file changed, 319 insertions(+)

  create mode 100644 Documentation/devicetree/bindings/dma/intel,ldma.yaml

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +7 -0

new file mode 100644
index 000000000000..9beaf191a6de
@@ -0,0 +1,319 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/intel,ldma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lightning Mountain centralized low speed DMA and high speed

DMA controllers.

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +7 -0

+
+maintainers:
+  - [email protected]
+  - [email protected]
+
+allOf:
+  - $ref: "dma-controller.yaml#"

Quoted 8 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 21

Quoted 7 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +7 -0

+
+properties:
+ $nodename:
+   pattern: "^dma-controller(@.*)?$"
+
+ "#dma-cells":
+   const: 1

Example says 3.

OK, i will fix it.

Quoted 68 lines · MallikarjunaX

Hi Rob,
Thanks for your valuable comments. Please see my comments inline..

Quoted 64 lines · Rob Herring

Quoted 51 lines · Amireddy Mallikarjuna reddy

Add DT bindings YAML schema for DMA controller driver of Lightning Mountain(LGM) SoC.

Signed-off-by: Amireddy Mallikarjuna reddy
<[email protected]>
---
v1:
- Initial version.

v2:
- Fix bot errors.

v3:
- No change.

v4:
- Address Thomas langer comments
   - use node name pattern as dma-controller as in common binding.
   - Remove "_" (underscore) in instance name.
   - Remove "port-" and "chan-" in attribute name for both
'dma-ports' & 'dma-channels' child nodes.

v5:
- Moved some of the attributes in 'dma-ports' & 'dma-channels' child nodes to dma client/consumer side as cells in 'dmas' properties.

---
   .../devicetree/bindings/dma/intel,ldma.yaml        | 319
+++++++++++++++++++++
   1 file changed, 319 insertions(+)

  create mode 100644
Documentation/devicetree/bindings/dma/intel,ldma.yaml

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +7 -0

new file mode 100644
index 000000000000..9beaf191a6de
@@ -0,0 +1,319 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/intel,ldma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lightning Mountain centralized low speed DMA and high speed

DMA controllers.

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +4 -0

+
+maintainers:
+  - [email protected]
+  - [email protected]

Documentation/devicetree/bindings/dma/intel,ldma.yaml line 21

Quoted 10 lines

Documentation/devicetree/bindings/dma/intel,ldma.yaml new file +10 -0

+
+allOf:
+  - $ref: "dma-controller.yaml#"
+
+properties:
+ $nodename:
+   pattern: "^dma-controller(@.*)?$"
+
+ "#dma-cells":
+   const: 1

Example says 3.

Quoted 1 line

OK, i will fix it.

It would help if you would add description of what is the meaning of the individual cell.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Quoted 1 line

drivers/dma/lgm/lgm-dma.c:729:5: warning: no previous prototype for function 'intel_dma_chan_desc_cfg' [-Wmissing-prototypes]

729 int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base,

Add DMA controller driver for Lightning Mountain(LGM) family of SoCs.

The main function of the DMA controller is the transfer of data from/to any DPlus compliant peripheral to/from the memory. A memory to memory copy capability can also be configured.

This ldma driver is used for configure the device and channnels for data and control paths.

Signed-off-by Amireddy Mallikarjuna reddy <[email protected]>

drivers/dma/lgm/lgm-dma.c new file +10 -0

+static int ldma_chan_cfg(struct ldma_chan *c)
+{
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	u32 reg;
+
+	reg = c->pden ? DMA_CCTRL_PDEN : 0;
+	reg |= c->onoff ? DMA_CCTRL_ON : 0;
+	reg |= c->rst ? DMA_CCTRL_RST : 0;
+
+	ldma_chan_cctrl_cfg(c, reg);

drivers/dma/lgm/lgm-dma.c new file +7 -0

+
+		if (ldma_chan_is_hw_desc(c))
+			ldma_chan_desc_hw_cfg(c, c->desc_phys, c->desc_cnt);
+	}
+
+	return 0;
+}

drivers/dma/lgm/lgm-dma.c new file +52 -0

+}
+
+static struct dw2_desc_sw *
+dma_alloc_desc_resource(int num, struct ldma_chan *c)
+{
+	struct device *dev = c->vchan.chan.device->dev;
+	struct dw2_desc_sw *ds;
+
+	if (num > c->desc_num) {
+		dev_err(dev, "sg num %d exceed max %d\n", num, c->desc_num);
+		return NULL;
+	}
+
+	ds = kzalloc(sizeof(*ds), GFP_NOWAIT);
+	if (!ds)
+		return NULL;
+
+	ds->chan = c;
+
+	ds->desc_hw = dma_pool_zalloc(c->desc_pool, GFP_ATOMIC,
+				      &ds->desc_phys);
+	if (!ds->desc_hw) {
+		dev_dbg(dev, "out of memory for link descriptor\n");
+		kfree(ds);
+		return NULL;
+	}
+	ds->desc_cnt = num;
+
+	return ds;
+}
+
+static void ldma_chan_irq_en(struct ldma_chan *c)
+{
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	unsigned long flags;
+
+	spin_lock_irqsave(&d->dev_lock, flags);
+	writel(c->nr, d->base + DMA_CS);
+	writel(DMA_CI_EOP, d->base + DMA_CIE);
+	writel(BIT(c->nr), d->base + DMA_IRNEN);
+	spin_unlock_irqrestore(&d->dev_lock, flags);
+}
+
+static void dma_issue_pending(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	unsigned long flags;
+
+	if (d->ver == DMA_VER22) {
+		spin_lock_irqsave(&c->vchan.lock, flags);
+		if (vchan_issue_pending(&c->vchan)) {

drivers/dma/lgm/lgm-dma.c new file +122 -0

+	}
+	ldma_chan_on(c);
+}
+
+static void dma_synchronize(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+
+	/*
+	 * clear any pending work if any. In that
+	 * case the resource needs to be free here.
+	 */
+	cancel_work_sync(&c->work);
+	vchan_synchronize(&c->vchan);
+	if (c->ds)
+		dma_free_desc_resource(&c->ds->vdesc);
+}
+
+static int dma_terminate_all(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	unsigned long flags;
+	LIST_HEAD(head);
+
+	spin_lock_irqsave(&c->vchan.lock, flags);
+	vchan_get_all_descriptors(&c->vchan, &head);
+	spin_unlock_irqrestore(&c->vchan.lock, flags);
+	vchan_dma_desc_free_list(&c->vchan, &head);
+
+	return ldma_chan_reset(c);
+}
+
+static int dma_resume_chan(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+
+	ldma_chan_on(c);
+
+	return 0;
+}
+
+static int dma_pause_chan(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+
+	return ldma_chan_off(c);
+}
+
+static enum dma_status
+dma_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
+	      struct dma_tx_state *txstate)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	enum dma_status status = DMA_COMPLETE;
+
+	if (d->ver == DMA_VER22)
+		status = dma_cookie_status(chan, cookie, txstate);
+
+	return status;
+}
+
+static void dma_chan_irq(int irq, void *data)
+{
+	struct ldma_chan *c = data;
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	u32 stat;
+
+	/* Disable channel interrupts  */
+	writel(c->nr, d->base + DMA_CS);
+	stat = readl(d->base + DMA_CIS);
+	if (!stat)
+		return;
+
+	writel(readl(d->base + DMA_CIE) & ~DMA_CI_ALL, d->base + DMA_CIE);
+	writel(stat, d->base + DMA_CIS);
+	queue_work(d->wq, &c->work);
+}
+
+static irqreturn_t dma_interrupt(int irq, void *dev_id)
+{
+	struct ldma_dev *d = dev_id;
+	struct ldma_chan *c;
+	unsigned long irncr;
+	u32 cid;
+
+	irncr = readl(d->base + DMA_IRNCR);
+	if (!irncr) {
+		dev_err(d->dev, "dummy interrupt\n");
+		return IRQ_NONE;
+	}
+
+	for_each_set_bit(cid, &irncr, d->chan_nrs) {
+		/* Mask */
+		writel(readl(d->base + DMA_IRNEN) & ~BIT(cid), d->base + DMA_IRNEN);
+		/* Ack */
+		writel(readl(d->base + DMA_IRNCR) | BIT(cid), d->base + DMA_IRNCR);
+
+		c = &d->chans[cid];
+		dma_chan_irq(irq, c);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct dma_async_tx_descriptor *
+dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
+		  unsigned int sglen, enum dma_transfer_direction dir,
+		  unsigned long flags, void *context)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	size_t len, avail, total = 0;
+	struct dw2_desc *hw_ds;
+	struct dw2_desc_sw *ds;
+	struct scatterlist *sg;
+	int num = sglen, i;
+	dma_addr_t addr;
+
+	if (!sgl)
+		return NULL;
+
+	for_each_sg(sgl, sg, sglen, i) {

drivers/dma/lgm/lgm-dma.c new file +55 -0

+
+	num = 0;
+	/* sop and eop has to be handled nicely */
+	for_each_sg(sgl, sg, sglen, i) {
+		addr = sg_dma_address(sg);
+		avail = sg_dma_len(sg);
+		total += avail;
+
+		do {
+			len = min_t(size_t, avail, DMA_MAX_SIZE);
+
+			hw_ds = &ds->desc_hw[num];
+			switch (sglen) {
+			case 1:
+				hw_ds->status.field.sop = 1;
+				hw_ds->status.field.eop = 1;
+				break;
+			default:
+				if (num == 0) {
+					hw_ds->status.field.sop = 1;
+					hw_ds->status.field.eop = 0;
+				} else if (num == (sglen - 1)) {
+					hw_ds->status.field.sop = 0;
+					hw_ds->status.field.eop = 1;
+				} else {
+					hw_ds->status.field.sop = 0;
+					hw_ds->status.field.eop = 0;
+				}
+				break;
+			}
+
+			/* Only 32 bit address supported */
+			hw_ds->addr = (u32)addr;
+			hw_ds->status.field.len = len;
+			hw_ds->status.field.c = 0;
+			hw_ds->status.field.bofs = addr & 0x3;
+			/* Ensure data ready before ownership change */
+			wmb();
+			hw_ds->status.field.own = DMA_OWN;
+			/* Ensure ownership changed before moving forward */
+			wmb();
+			num++;
+			addr += len;
+			avail -= len;
+		} while (avail);
+	}
+
+	ds->size = total;
+
+	return vchan_tx_prep(&c->vchan, &ds->vdesc, DMA_CTRL_ACK);
+}
+
+static int
+dma_slave_config(struct dma_chan *chan, struct dma_slave_config *cfg)
+{

drivers/dma/lgm/lgm-dma.c new file +7 -0

+
+	/* Must be the same */
+	if (cfg->src_maxburst && cfg->dst_maxburst &&
+	    cfg->src_maxburst != cfg->dst_maxburst)
+		return -EINVAL;
+
+	if (cfg->dst_maxburst)

drivers/dma/lgm/lgm-dma.c new file +45 -0

+
+	return 0;
+}
+
+static int dma_alloc_chan_resources(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	struct device *dev = c->vchan.chan.device->dev;
+	size_t	desc_sz;
+
+	if (d->ver > DMA_VER22) {
+		c->flags |= CHAN_IN_USE;
+		return 0;
+	}
+
+	if (c->desc_pool)
+		return c->desc_num;
+
+	desc_sz = c->desc_num * sizeof(struct dw2_desc);
+	c->desc_pool = dma_pool_create(c->name, dev, desc_sz,
+				       __alignof__(struct dw2_desc), 0);
+
+	if (!c->desc_pool) {
+		dev_err(dev, "unable to allocate descriptor pool\n");
+		return -ENOMEM;
+	}
+
+	return c->desc_num;
+}
+
+static void dma_free_chan_resources(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+
+	if (d->ver == DMA_VER22) {
+		dma_pool_destroy(c->desc_pool);
+		c->desc_pool = NULL;
+		vchan_free_chan_resources(to_virt_chan(chan));
+		ldma_chan_reset(c);
+	} else {
+		c->flags &= ~CHAN_IN_USE;
+	}
+}

drivers/dma/lgm/lgm-dma.c new file +307 -0

+}
+
+int update_client_configs(struct of_dma *ofdma, struct of_phandle_args *spec)
+{
+	struct ldma_dev *d = ofdma->of_dma_data;
+	struct ldma_port *p;
+	struct ldma_chan *c;
+	u32 chan_id =  spec->args[0];
+	u32 port_id =  spec->args[1];
+
+	if (chan_id >= d->chan_nrs || port_id >= d->port_nrs)
+		return 0;
+
+	p = &d->ports[port_id];
+	c = &d->chans[chan_id];
+
+	if (d->ver == DMA_VER22) {
+		u32 burst = spec->args[2];
+
+		if (burst != 2 && burst != 4 && burst != 8)
+			return 0;
+
+		/* TX and RX has the same burst length */
+		p->txbl = ilog2(burst);
+		p->rxbl = p->txbl;
+
+		ldma_port_cfg(p);
+	} else {
+		if (spec->args[2] > 0 && spec->args[2] <= DMA_ENDIAN_TYPE3) {
+			c->data_endian = spec->args[2];
+			c->data_endian_en = true;
+		}
+
+		if (spec->args[3] > 0 && spec->args[3] <= DMA_ENDIAN_TYPE3) {
+			c->desc_endian = spec->args[3];
+			c->desc_endian_en = true;
+		}
+
+		if (spec->args[4] > 0 && spec->args[4] < 128)
+			c->boff_len = spec->args[4];
+
+		if (spec->args[5])
+			c->desc_rx_np = true;
+
+		/*
+		 * If channel packet drop enabled, port packet drop should
+		 * be enabled
+		 */
+		if (spec->args[6]) {
+			c->pden = true;
+			p->pkt_drop = DMA_PKT_DROP_EN;
+		}
+		ldma_port_cfg(p);
+		ldma_chan_cfg(c);
+	}
+
+	return 1;
+}
+
+static struct dma_chan *ldma_xlate(struct of_phandle_args *spec,
+				   struct of_dma *ofdma)
+{
+	struct ldma_dev *d = ofdma->of_dma_data;
+	u32 chan_id =  spec->args[0];
+	int ret;
+
+	if (!spec->args_count)
+		return NULL;
+
+	/* if args_count is 1 use driver default config settings */
+	if (spec->args_count > 1) {
+		ret = update_client_configs(ofdma, spec);
+		if (!ret)
+			return NULL;
+	}
+
+	return dma_get_slave_channel(&d->chans[chan_id].vchan.chan);
+}
+
+static void ldma_clk_disable(void *data)
+{
+	struct ldma_dev *d = data;
+
+	clk_disable_unprepare(d->core_clk);
+}
+
+static struct dma_dev_ops dma0_ops = {
+	.device_alloc_chan_resources = dma_alloc_chan_resources,
+	.device_free_chan_resources = dma_free_chan_resources,
+	.device_config = dma_slave_config,
+	.device_prep_slave_sg = dma_prep_slave_sg,
+	.device_tx_status = dma_tx_status,
+	.device_pause = dma_pause_chan,
+	.device_resume = dma_resume_chan,
+	.device_terminate_all = dma_terminate_all,
+	.device_synchronize = dma_synchronize,
+	.device_issue_pending = dma_issue_pending,
+};
+
+static struct dma_dev_ops hdma_ops = {
+	.device_alloc_chan_resources = dma_alloc_chan_resources,
+	.device_free_chan_resources = dma_free_chan_resources,
+	.device_terminate_all = dma_terminate_all,
+	.device_issue_pending = dma_issue_pending,
+	.device_tx_status = dma_tx_status,
+	.device_resume = dma_resume_chan,
+	.device_pause = dma_pause_chan,
+};
+
+static const struct ldma_inst_data dma0 = {
+	.name = "dma0",
+	.ops = &dma0_ops,
+};
+
+static const struct ldma_inst_data dma2tx = {
+	.name = "dma2tx",
+	.type = DMA_TYPE_TX,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data dma1rx = {
+	.name = "dma1rx",
+	.type = DMA_TYPE_RX,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data dma1tx = {
+	.name = "dma1tx",
+	.type = DMA_TYPE_TX,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data dma0tx = {
+	.name = "dma0tx",
+	.type = DMA_TYPE_TX,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data dma3 = {
+	.name = "dma3",
+	.type = DMA_TYPE_MCPY,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data toe_dma30 = {
+	.name = "toe_dma30",
+	.type = DMA_TYPE_MCPY,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data toe_dma31 = {
+	.name = "toe_dma31",
+	.type = DMA_TYPE_MCPY,
+	.ops = &hdma_ops,
+};
+
+static const struct of_device_id intel_ldma_match[] = {
+	{ .compatible = "intel,lgm-cdma", .data = &dma0},
+	{ .compatible = "intel,lgm-dma2tx", .data = &dma2tx},
+	{ .compatible = "intel,lgm-dma1rx", .data = &dma1rx},
+	{ .compatible = "intel,lgm-dma1tx", .data = &dma1tx},
+	{ .compatible = "intel,lgm-dma0tx", .data = &dma0tx},
+	{ .compatible = "intel,lgm-dma3", .data = &dma3},
+	{ .compatible = "intel,lgm-toe-dma30", .data = &toe_dma30},
+	{ .compatible = "intel,lgm-toe-dma31", .data = &toe_dma31},
+	{}
+};
+
+static int intel_ldma_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct dma_device *dma_dev;
+	struct ldma_chan *c;
+	struct ldma_port *p;
+	struct ldma_dev *d;
+	u32 id, bitn = 32;
+	int i, j, k, ret;
+
+	d = devm_kzalloc(dev, sizeof(*d), GFP_KERNEL);
+	if (!d)
+		return -ENOMEM;
+
+	/* Link controller to platform device */
+	d->dev = &pdev->dev;
+
+	d->inst = device_get_match_data(dev);
+	if (!d->inst) {
+		dev_err(dev, "No device match found\n");
+		return -ENODEV;
+	}
+
+	d->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(d->base))
+		return PTR_ERR(d->base);
+
+	/* Power up and reset the dma engine, some DMAs always on?? */
+	d->core_clk = devm_clk_get_optional(dev, NULL);
+	if (IS_ERR(d->core_clk))
+		return PTR_ERR(d->core_clk);
+	clk_prepare_enable(d->core_clk);
+
+	ret = devm_add_action_or_reset(dev, ldma_clk_disable, d);
+	if (ret) {
+		dev_err(dev, "Failed to devm_add_action_or_reset, %d\n", ret);
+		return ret;
+	}
+
+	d->rst = devm_reset_control_get_optional(dev, NULL);
+	if (IS_ERR(d->rst))
+		return PTR_ERR(d->rst);
+	reset_control_deassert(d->rst);
+
+	id = readl(d->base + DMA_ID);
+	d->chan_nrs = FIELD_GET(DMA_ID_CHNR, id);
+	d->port_nrs = FIELD_GET(DMA_ID_PNR, id);
+	d->ver = FIELD_GET(DMA_ID_REV, id);
+
+	if (id & DMA_ID_AW_36B)
+		d->flags |= DMA_ADDR_36BIT;
+
+	if (IS_ENABLED(CONFIG_64BIT)) {
+		if (id & DMA_ID_AW_36B)
+			bitn = 36;
+	}
+
+	if (id & DMA_ID_DW_128B)
+		d->flags |= DMA_DATA_128BIT;
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(bitn));
+	if (ret) {
+		dev_err(dev, "No usable DMA configuration\n");
+		return ret;
+	}
+
+	if (d->ver == DMA_VER22) {
+		d->irq = platform_get_irq(pdev, 0);
+		if (d->irq < 0)
+			return d->irq;
+
+		ret = devm_request_irq(&pdev->dev, d->irq, dma_interrupt,
+				       0, DRIVER_NAME, d);
+		if (ret)
+			return ret;
+
+		d->wq = alloc_ordered_workqueue("dma_wq", WQ_MEM_RECLAIM |
+						WQ_HIGHPRI);
+		if (!d->wq)
+			return -ENOMEM;
+	}
+
+	dma_dev = &d->dma_dev;
+	dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);
+
+	/* Channel initializations */
+	INIT_LIST_HEAD(&dma_dev->channels);
+
+	/* Port Initializations */
+	d->ports = devm_kcalloc(dev, d->port_nrs, sizeof(*p), GFP_KERNEL);
+	if (!d->ports)
+		return -ENOMEM;
+
+	for (i = 0; i < d->port_nrs; i++) {
+		p = &d->ports[i];
+		p->portid = i;
+		p->ldev = d;
+	}
+
+	ret = ldma_cfg_init(d);
+	if (ret)
+		return ret;
+
+	dma_dev->dev = &pdev->dev;
+	/*
+	 * Link channel id to channel index and link to dma channel list
+	 * It also back points to controller and its port
+	 */
+	for (i = 0, k = 0; i < d->port_nrs; i++) {
+		if (d->ver == DMA_VER22) {
+			u32 chan_end;
+
+			p = &d->ports[i];
+			chan_end = p->chan_start + p->chan_sz;
+			for (j = p->chan_start; j < chan_end; j++) {
+				c = &d->chans[k];
+				c->port = p;
+				c->nr = j; /* Real channel number */
+				c->rst = DMA_CHAN_RST;
+				snprintf(c->name, sizeof(c->name), "chan%d",
+					 c->nr);
+				INIT_WORK(&c->work, dma_work);
+				c->vchan.desc_free = dma_free_desc_resource;
+				vchan_init(&c->vchan, dma_dev);
+				k++;
+			}
+		} else {
+			p = &d->ports[i];
+			for (i = 0; i < d->chan_nrs; i++) {
+				c = &d->chans[i];
+				c->port = p;
+				c->data_endian = DMA_DFT_ENDIAN;
+				c->desc_endian = DMA_DFT_ENDIAN;
+				c->data_endian_en = false;
+				c->desc_endian_en = false;
+				c->desc_rx_np = false;
+				c->flags |= DEVICE_ALLOC_DESC;
+				c->onoff = DMA_CH_OFF;
+				c->rst = DMA_CHAN_RST;

drivers/dma/lgm/lgm-dma.c new file +12 -0

+
+	dma_dev->device_alloc_chan_resources =
+		d->inst->ops->device_alloc_chan_resources;
+	dma_dev->device_free_chan_resources =
+		d->inst->ops->device_free_chan_resources;
+	dma_dev->device_terminate_all = d->inst->ops->device_terminate_all;
+	dma_dev->device_issue_pending = d->inst->ops->device_issue_pending;
+	dma_dev->device_tx_status = d->inst->ops->device_tx_status;
+	dma_dev->device_resume = d->inst->ops->device_resume;
+	dma_dev->device_pause = d->inst->ops->device_pause;
+	dma_dev->device_config = d->inst->ops->device_config;
+	dma_dev->device_prep_slave_sg = d->inst->ops->device_prep_slave_sg;

drivers/dma/lgm/lgm-dma.c new file +38 -0

+
+	platform_set_drvdata(pdev, d);
+
+	ldma_dev_init(d);
+
+	ret = dma_async_device_register(dma_dev);
+	if (ret) {
+		dev_err(dev, "Failed to register slave DMA engine device\n");
+		return ret;
+	}
+
+	ret = of_dma_controller_register(pdev->dev.of_node, ldma_xlate, d);
+	if (ret) {
+		dev_err(dev, "Failed to register of DMA controller\n");
+		dma_async_device_unregister(dma_dev);
+		return ret;
+	}
+
+	dev_info(dev, "Init done - rev: %x, ports: %d channels: %d\n", d->ver,
+		 d->port_nrs, d->chan_nrs);
+
+	return 0;
+}
+
+static struct platform_driver intel_ldma_driver = {
+	.probe = intel_ldma_probe,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = intel_ldma_match,
+	},
+};
+
+static int __init intel_ldma_init(void)
+{
+	return platform_driver_register(&intel_ldma_driver);
+}
+
+device_initcall(intel_ldma_init);

include/linux/dma/lgm_dma.h new file +17 -0

new file mode 100644
index 000000000000..3a2ee6ad0710
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016 ~ 2019 Intel Corporation.
+ */
+#ifndef LGM_DMA_H
+#define LGM_DMA_H
+
+#include <linux/types.h>
+#include <linux/dmaengine.h>
+
+/*!
+ * \fn int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base,
+ *                                 int desc_num)
+ * \brief Configure low level channel descriptors
+ * \param[in] chan   pointer to DMA channel that the client is using
+ * \param[in] desc_base   descriptor base physical address
+ * \param[in] desc_num   number of descriptors

Quoted 9 lines

drivers/dma/lgm/lgm-dma.c new file +9 -0

+static void dma_issue_pending(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	unsigned long flags;
+
+	if (d->ver == DMA_VER22) {
+		spin_lock_irqsave(&c->vchan.lock, flags);
+		if (vchan_issue_pending(&c->vchan)) {

drivers/dma/lgm/lgm-dma.c line 1288

Quoted 10 lines

drivers/dma/lgm/lgm-dma.c new file +10 -0

+			struct virt_dma_desc *vdesc;
+
+			/* Get the next descriptor */
+			vdesc = vchan_next_desc(&c->vchan);
+			if (!vdesc) {
+				c->ds = NULL;
+				return;
+			}
+			list_del(&vdesc->node);
+			c->ds = to_lgm_dma_desc(vdesc);

you have set c->ds in dma_prep_slave_sg and the only way I can see that you will not leak memory is that the client must terminate_sync() after each transfer so that the synchronize callback is invoked between each prep_sg/issue_pending/competion.

drivers/dma/lgm/lgm-dma.c line 1292

Quoted 4 lines

drivers/dma/lgm/lgm-dma.c new file +4 -0

+			spin_unlock_irqrestore(&c->vchan.lock, flags);
+			ldma_chan_desc_hw_cfg(c, c->ds->desc_phys, c->ds->desc_cnt);
+			ldma_chan_irq_en(c);
+		}

If there is nothing pending, you will leave the spinlock wide open...

drivers/dma/lgm/lgm-dma.c line 1295

Quoted 4 lines

drivers/dma/lgm/lgm-dma.c new file +3 -0

+	}
+	ldma_chan_on(c);
+}

Hi,

Quoted 10 lines · Amireddy Mallikarjuna reddy

Add DMA controller driver for Lightning Mountain(LGM) family of SoCs.

The main function of the DMA controller is the transfer of data from/to any DPlus compliant peripheral to/from the memory. A memory to memory copy capability can also be configured.

This ldma driver is used for configure the device and channnels for data and control paths.

Signed-off-by Amireddy Mallikarjuna reddy <[email protected]>

...

Quoted 14 lines

drivers/dma/lgm/lgm-dma.c new file +14 -0

+static int ldma_chan_cfg(struct ldma_chan *c)
+{
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	u32 reg;
+
+	reg = c->pden ? DMA_CCTRL_PDEN : 0;
+	reg |= c->onoff ? DMA_CCTRL_ON : 0;
+	reg |= c->rst ? DMA_CCTRL_RST : 0;
+
+	ldma_chan_cctrl_cfg(c, reg);
+	ldma_chan_irq_init(c);
+
+	if (d->ver > DMA_VER22) {
+		ldma_chan_set_class(c, c->nr);

drivers/dma/lgm/lgm-dma.c line 942

Quoted 6 lines

drivers/dma/lgm/lgm-dma.c new file +6 -0

+		ldma_chan_byte_offset_cfg(c, c->boff_len);
+		ldma_chan_data_endian_cfg(c, c->data_endian_en, c->data_endian);
+		ldma_chan_desc_endian_cfg(c, c->desc_endian_en, c->desc_endian);
+		ldma_chan_hdr_mode_cfg(c, c->hdrm_len, c->hdrm_csum);
+		ldma_chan_rxwr_np_cfg(c, c->desc_rx_np);
+		ldma_chan_abc_cfg(c, c->abc_en);

Each of these functions will lock and unlock the same lock, would it make sense to restructur things to have less activity with the spinlock?

Quoted 8 lines

drivers/dma/lgm/lgm-dma.c new file +7 -0

+
+		if (ldma_chan_is_hw_desc(c))
+			ldma_chan_desc_hw_cfg(c, c->desc_phys, c->desc_cnt);
+	}
+
+	return 0;
+}

...

drivers/dma/lgm/lgm-dma.c line 1226

Quoted 8 lines

drivers/dma/lgm/lgm-dma.c new file +8 -0

+static void dma_free_desc_resource(struct virt_dma_desc *vdesc)
+{
+	struct dw2_desc_sw *ds = to_lgm_dma_desc(vdesc);
+	struct ldma_chan *c = ds->chan;
+
+	dma_pool_free(c->desc_pool, ds->desc_hw, ds->desc_phys);
+	kfree(ds);
+	c->ds = NULL;

Is there a chance that c->ds != ds?

Quoted 59 lines

drivers/dma/lgm/lgm-dma.c new file +58 -0

+}
+
+static struct dw2_desc_sw *
+dma_alloc_desc_resource(int num, struct ldma_chan *c)
+{
+	struct device *dev = c->vchan.chan.device->dev;
+	struct dw2_desc_sw *ds;
+
+	if (num > c->desc_num) {
+		dev_err(dev, "sg num %d exceed max %d\n", num, c->desc_num);
+		return NULL;
+	}
+
+	ds = kzalloc(sizeof(*ds), GFP_NOWAIT);
+	if (!ds)
+		return NULL;
+
+	ds->chan = c;
+
+	ds->desc_hw = dma_pool_zalloc(c->desc_pool, GFP_ATOMIC,
+				      &ds->desc_phys);
+	if (!ds->desc_hw) {
+		dev_dbg(dev, "out of memory for link descriptor\n");
+		kfree(ds);
+		return NULL;
+	}
+	ds->desc_cnt = num;
+
+	return ds;
+}
+
+static void ldma_chan_irq_en(struct ldma_chan *c)
+{
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	unsigned long flags;
+
+	spin_lock_irqsave(&d->dev_lock, flags);
+	writel(c->nr, d->base + DMA_CS);
+	writel(DMA_CI_EOP, d->base + DMA_CIE);
+	writel(BIT(c->nr), d->base + DMA_IRNEN);
+	spin_unlock_irqrestore(&d->dev_lock, flags);
+}
+
+static void dma_issue_pending(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	unsigned long flags;
+
+	if (d->ver == DMA_VER22) {
+		spin_lock_irqsave(&c->vchan.lock, flags);
+		if (vchan_issue_pending(&c->vchan)) {
+			struct virt_dma_desc *vdesc;
+
+			/* Get the next descriptor */
+			vdesc = vchan_next_desc(&c->vchan);
+			if (!vdesc) {
+				c->ds = NULL;

drivers/dma/lgm/lgm-dma.c line 1288

Quoted 4 lines

drivers/dma/lgm/lgm-dma.c new file +4 -0

+				return;
+			}
+			list_del(&vdesc->node);
+			c->ds = to_lgm_dma_desc(vdesc);

you have set c->ds in dma_prep_slave_sg and the only way I can see that you will not leak memory is that the client must terminate_sync() after each transfer so that the synchronize callback is invoked between each prep_sg/issue_pending/competion.

drivers/dma/lgm/lgm-dma.c line 1292

Quoted 5 lines

drivers/dma/lgm/lgm-dma.c new file +4 -0

+			spin_unlock_irqrestore(&c->vchan.lock, flags);
+			ldma_chan_desc_hw_cfg(c, c->ds->desc_phys, c->ds->desc_cnt);
+			ldma_chan_irq_en(c);
+		}

If there is nothing peding, you will leave the spinlock wide open...

Quoted 127 lines

drivers/dma/lgm/lgm-dma.c new file +126 -0

+	}
+	ldma_chan_on(c);
+}
+
+static void dma_synchronize(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+
+	/*
+	 * clear any pending work if any. In that
+	 * case the resource needs to be free here.
+	 */
+	cancel_work_sync(&c->work);
+	vchan_synchronize(&c->vchan);
+	if (c->ds)
+		dma_free_desc_resource(&c->ds->vdesc);
+}
+
+static int dma_terminate_all(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	unsigned long flags;
+	LIST_HEAD(head);
+
+	spin_lock_irqsave(&c->vchan.lock, flags);
+	vchan_get_all_descriptors(&c->vchan, &head);
+	spin_unlock_irqrestore(&c->vchan.lock, flags);
+	vchan_dma_desc_free_list(&c->vchan, &head);
+
+	return ldma_chan_reset(c);
+}
+
+static int dma_resume_chan(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+
+	ldma_chan_on(c);
+
+	return 0;
+}
+
+static int dma_pause_chan(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+
+	return ldma_chan_off(c);
+}
+
+static enum dma_status
+dma_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
+	      struct dma_tx_state *txstate)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	enum dma_status status = DMA_COMPLETE;
+
+	if (d->ver == DMA_VER22)
+		status = dma_cookie_status(chan, cookie, txstate);
+
+	return status;
+}
+
+static void dma_chan_irq(int irq, void *data)
+{
+	struct ldma_chan *c = data;
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	u32 stat;
+
+	/* Disable channel interrupts  */
+	writel(c->nr, d->base + DMA_CS);
+	stat = readl(d->base + DMA_CIS);
+	if (!stat)
+		return;
+
+	writel(readl(d->base + DMA_CIE) & ~DMA_CI_ALL, d->base + DMA_CIE);
+	writel(stat, d->base + DMA_CIS);
+	queue_work(d->wq, &c->work);
+}
+
+static irqreturn_t dma_interrupt(int irq, void *dev_id)
+{
+	struct ldma_dev *d = dev_id;
+	struct ldma_chan *c;
+	unsigned long irncr;
+	u32 cid;
+
+	irncr = readl(d->base + DMA_IRNCR);
+	if (!irncr) {
+		dev_err(d->dev, "dummy interrupt\n");
+		return IRQ_NONE;
+	}
+
+	for_each_set_bit(cid, &irncr, d->chan_nrs) {
+		/* Mask */
+		writel(readl(d->base + DMA_IRNEN) & ~BIT(cid), d->base + DMA_IRNEN);
+		/* Ack */
+		writel(readl(d->base + DMA_IRNCR) | BIT(cid), d->base + DMA_IRNCR);
+
+		c = &d->chans[cid];
+		dma_chan_irq(irq, c);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct dma_async_tx_descriptor *
+dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
+		  unsigned int sglen, enum dma_transfer_direction dir,
+		  unsigned long flags, void *context)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	size_t len, avail, total = 0;
+	struct dw2_desc *hw_ds;
+	struct dw2_desc_sw *ds;
+	struct scatterlist *sg;
+	int num = sglen, i;
+	dma_addr_t addr;
+
+	if (!sgl)
+		return NULL;
+
+	for_each_sg(sgl, sg, sglen, i) {
+		avail = sg_dma_len(sg);
+		if (avail > DMA_MAX_SIZE)
+			num += DIV_ROUND_UP(avail, DMA_MAX_SIZE) - 1;
+	}

drivers/dma/lgm/lgm-dma.c line 1424

Quoted 6 lines

drivers/dma/lgm/lgm-dma.c new file +6 -0

+
+	ds = dma_alloc_desc_resource(num, c);
+	if (!ds)
+		return NULL;
+
+	c->ds = ds;

If you still have a transfer running then you are going to get lost that dscriptor?

Quoted 60 lines

drivers/dma/lgm/lgm-dma.c new file +59 -0

+
+	num = 0;
+	/* sop and eop has to be handled nicely */
+	for_each_sg(sgl, sg, sglen, i) {
+		addr = sg_dma_address(sg);
+		avail = sg_dma_len(sg);
+		total += avail;
+
+		do {
+			len = min_t(size_t, avail, DMA_MAX_SIZE);
+
+			hw_ds = &ds->desc_hw[num];
+			switch (sglen) {
+			case 1:
+				hw_ds->status.field.sop = 1;
+				hw_ds->status.field.eop = 1;
+				break;
+			default:
+				if (num == 0) {
+					hw_ds->status.field.sop = 1;
+					hw_ds->status.field.eop = 0;
+				} else if (num == (sglen - 1)) {
+					hw_ds->status.field.sop = 0;
+					hw_ds->status.field.eop = 1;
+				} else {
+					hw_ds->status.field.sop = 0;
+					hw_ds->status.field.eop = 0;
+				}
+				break;
+			}
+
+			/* Only 32 bit address supported */
+			hw_ds->addr = (u32)addr;
+			hw_ds->status.field.len = len;
+			hw_ds->status.field.c = 0;
+			hw_ds->status.field.bofs = addr & 0x3;
+			/* Ensure data ready before ownership change */
+			wmb();
+			hw_ds->status.field.own = DMA_OWN;
+			/* Ensure ownership changed before moving forward */
+			wmb();
+			num++;
+			addr += len;
+			avail -= len;
+		} while (avail);
+	}
+
+	ds->size = total;
+
+	return vchan_tx_prep(&c->vchan, &ds->vdesc, DMA_CTRL_ACK);
+}
+
+static int
+dma_slave_config(struct dma_chan *chan, struct dma_slave_config *cfg)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	struct ldma_port *p = c->port;
+	unsigned long flags;

drivers/dma/lgm/lgm-dma.c line 1489

Quoted 6 lines

drivers/dma/lgm/lgm-dma.c new file +6 -0

+	u32 bl;
+
+	if ((cfg->direction == DMA_DEV_TO_MEM &&
+	     cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) ||
+	    (cfg->direction == DMA_MEM_TO_DEV &&
+	     cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) ||

According to the probe function these width restrictions are only valid for DMA_VER22?

drivers/dma/lgm/lgm-dma.c line 1495

Quoted 7 lines

drivers/dma/lgm/lgm-dma.c new file +6 -0

+	    !is_slave_direction(cfg->direction))
+		return -EINVAL;
+
+	/* Default setting will be used */
+	if (!cfg->src_maxburst && !cfg->dst_maxburst)
+		return 0;

maxburst == 0 is identical to maxburst == 1, it is just not set explicitly. Iow 1 word per DMA request.

Quoted 8 lines

drivers/dma/lgm/lgm-dma.c new file +7 -0

+
+	/* Must be the same */
+	if (cfg->src_maxburst && cfg->dst_maxburst &&
+	    cfg->src_maxburst != cfg->dst_maxburst)
+		return -EINVAL;
+
+	if (cfg->dst_maxburst)

drivers/dma/lgm/lgm-dma.c line 1512

Quoted 10 lines

drivers/dma/lgm/lgm-dma.c new file +10 -0

+		cfg->src_maxburst = cfg->dst_maxburst;
+
+	bl = ilog2(cfg->src_maxburst);
+
+	spin_lock_irqsave(&d->dev_lock, flags);
+	writel(p->portid, d->base + DMA_PS);
+	ldma_update_bits(d, DMA_PCTRL_RXBL | DMA_PCTRL_TXBL,
+			 FIELD_PREP(DMA_PCTRL_RXBL, bl) |
+			 FIELD_PREP(DMA_PCTRL_TXBL, bl), DMA_PCTRL);
+	spin_unlock_irqrestore(&d->dev_lock, flags);

What drivers usually do is to save the cfg and inprepare time take it into account when setting up the transfer.
Write the change to the HW before the trasnfer is started (if it has changed from previous settings)

Client drivers usually set the slave config ones, in most cases during probe, so the slave config rarely changes runtime, but there are cases for that.

Quoted 49 lines

drivers/dma/lgm/lgm-dma.c new file +48 -0

+
+	return 0;
+}
+
+static int dma_alloc_chan_resources(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+	struct device *dev = c->vchan.chan.device->dev;
+	size_t	desc_sz;
+
+	if (d->ver > DMA_VER22) {
+		c->flags |= CHAN_IN_USE;
+		return 0;
+	}
+
+	if (c->desc_pool)
+		return c->desc_num;
+
+	desc_sz = c->desc_num * sizeof(struct dw2_desc);
+	c->desc_pool = dma_pool_create(c->name, dev, desc_sz,
+				       __alignof__(struct dw2_desc), 0);
+
+	if (!c->desc_pool) {
+		dev_err(dev, "unable to allocate descriptor pool\n");
+		return -ENOMEM;
+	}
+
+	return c->desc_num;
+}
+
+static void dma_free_chan_resources(struct dma_chan *chan)
+{
+	struct ldma_chan *c = to_ldma_chan(chan);
+	struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+
+	if (d->ver == DMA_VER22) {
+		dma_pool_destroy(c->desc_pool);
+		c->desc_pool = NULL;
+		vchan_free_chan_resources(to_virt_chan(chan));
+		ldma_chan_reset(c);
+	} else {
+		c->flags &= ~CHAN_IN_USE;
+	}
+}
+
+static void dma_work(struct work_struct *work)
+{

drivers/dma/lgm/lgm-dma.c line 1567

Quoted 7 lines

drivers/dma/lgm/lgm-dma.c new file +7 -0

+	struct ldma_chan *c = container_of(work, struct ldma_chan, work);
+	struct dma_async_tx_descriptor *tx = &c->ds->vdesc.tx;
+	struct dmaengine_desc_callback cb;
+
+	dmaengine_desc_get_callback(tx, &cb);
+	dma_cookie_complete(tx);
+	dmaengine_desc_callback_invoke(&cb, NULL);

When you are going to free up the descriptor?

Quoted 311 lines

drivers/dma/lgm/lgm-dma.c new file +310 -0

+}
+
+int update_client_configs(struct of_dma *ofdma, struct of_phandle_args *spec)
+{
+	struct ldma_dev *d = ofdma->of_dma_data;
+	struct ldma_port *p;
+	struct ldma_chan *c;
+	u32 chan_id =  spec->args[0];
+	u32 port_id =  spec->args[1];
+
+	if (chan_id >= d->chan_nrs || port_id >= d->port_nrs)
+		return 0;
+
+	p = &d->ports[port_id];
+	c = &d->chans[chan_id];
+
+	if (d->ver == DMA_VER22) {
+		u32 burst = spec->args[2];
+
+		if (burst != 2 && burst != 4 && burst != 8)
+			return 0;
+
+		/* TX and RX has the same burst length */
+		p->txbl = ilog2(burst);
+		p->rxbl = p->txbl;
+
+		ldma_port_cfg(p);
+	} else {
+		if (spec->args[2] > 0 && spec->args[2] <= DMA_ENDIAN_TYPE3) {
+			c->data_endian = spec->args[2];
+			c->data_endian_en = true;
+		}
+
+		if (spec->args[3] > 0 && spec->args[3] <= DMA_ENDIAN_TYPE3) {
+			c->desc_endian = spec->args[3];
+			c->desc_endian_en = true;
+		}
+
+		if (spec->args[4] > 0 && spec->args[4] < 128)
+			c->boff_len = spec->args[4];
+
+		if (spec->args[5])
+			c->desc_rx_np = true;
+
+		/*
+		 * If channel packet drop enabled, port packet drop should
+		 * be enabled
+		 */
+		if (spec->args[6]) {
+			c->pden = true;
+			p->pkt_drop = DMA_PKT_DROP_EN;
+		}
+		ldma_port_cfg(p);
+		ldma_chan_cfg(c);
+	}
+
+	return 1;
+}
+
+static struct dma_chan *ldma_xlate(struct of_phandle_args *spec,
+				   struct of_dma *ofdma)
+{
+	struct ldma_dev *d = ofdma->of_dma_data;
+	u32 chan_id =  spec->args[0];
+	int ret;
+
+	if (!spec->args_count)
+		return NULL;
+
+	/* if args_count is 1 use driver default config settings */
+	if (spec->args_count > 1) {
+		ret = update_client_configs(ofdma, spec);
+		if (!ret)
+			return NULL;
+	}
+
+	return dma_get_slave_channel(&d->chans[chan_id].vchan.chan);
+}
+
+static void ldma_clk_disable(void *data)
+{
+	struct ldma_dev *d = data;
+
+	clk_disable_unprepare(d->core_clk);
+}
+
+static struct dma_dev_ops dma0_ops = {
+	.device_alloc_chan_resources = dma_alloc_chan_resources,
+	.device_free_chan_resources = dma_free_chan_resources,
+	.device_config = dma_slave_config,
+	.device_prep_slave_sg = dma_prep_slave_sg,
+	.device_tx_status = dma_tx_status,
+	.device_pause = dma_pause_chan,
+	.device_resume = dma_resume_chan,
+	.device_terminate_all = dma_terminate_all,
+	.device_synchronize = dma_synchronize,
+	.device_issue_pending = dma_issue_pending,
+};
+
+static struct dma_dev_ops hdma_ops = {
+	.device_alloc_chan_resources = dma_alloc_chan_resources,
+	.device_free_chan_resources = dma_free_chan_resources,
+	.device_terminate_all = dma_terminate_all,
+	.device_issue_pending = dma_issue_pending,
+	.device_tx_status = dma_tx_status,
+	.device_resume = dma_resume_chan,
+	.device_pause = dma_pause_chan,
+};
+
+static const struct ldma_inst_data dma0 = {
+	.name = "dma0",
+	.ops = &dma0_ops,
+};
+
+static const struct ldma_inst_data dma2tx = {
+	.name = "dma2tx",
+	.type = DMA_TYPE_TX,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data dma1rx = {
+	.name = "dma1rx",
+	.type = DMA_TYPE_RX,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data dma1tx = {
+	.name = "dma1tx",
+	.type = DMA_TYPE_TX,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data dma0tx = {
+	.name = "dma0tx",
+	.type = DMA_TYPE_TX,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data dma3 = {
+	.name = "dma3",
+	.type = DMA_TYPE_MCPY,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data toe_dma30 = {
+	.name = "toe_dma30",
+	.type = DMA_TYPE_MCPY,
+	.ops = &hdma_ops,
+};
+
+static const struct ldma_inst_data toe_dma31 = {
+	.name = "toe_dma31",
+	.type = DMA_TYPE_MCPY,
+	.ops = &hdma_ops,
+};
+
+static const struct of_device_id intel_ldma_match[] = {
+	{ .compatible = "intel,lgm-cdma", .data = &dma0},
+	{ .compatible = "intel,lgm-dma2tx", .data = &dma2tx},
+	{ .compatible = "intel,lgm-dma1rx", .data = &dma1rx},
+	{ .compatible = "intel,lgm-dma1tx", .data = &dma1tx},
+	{ .compatible = "intel,lgm-dma0tx", .data = &dma0tx},
+	{ .compatible = "intel,lgm-dma3", .data = &dma3},
+	{ .compatible = "intel,lgm-toe-dma30", .data = &toe_dma30},
+	{ .compatible = "intel,lgm-toe-dma31", .data = &toe_dma31},
+	{}
+};
+
+static int intel_ldma_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct dma_device *dma_dev;
+	struct ldma_chan *c;
+	struct ldma_port *p;
+	struct ldma_dev *d;
+	u32 id, bitn = 32;
+	int i, j, k, ret;
+
+	d = devm_kzalloc(dev, sizeof(*d), GFP_KERNEL);
+	if (!d)
+		return -ENOMEM;
+
+	/* Link controller to platform device */
+	d->dev = &pdev->dev;
+
+	d->inst = device_get_match_data(dev);
+	if (!d->inst) {
+		dev_err(dev, "No device match found\n");
+		return -ENODEV;
+	}
+
+	d->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(d->base))
+		return PTR_ERR(d->base);
+
+	/* Power up and reset the dma engine, some DMAs always on?? */
+	d->core_clk = devm_clk_get_optional(dev, NULL);
+	if (IS_ERR(d->core_clk))
+		return PTR_ERR(d->core_clk);
+	clk_prepare_enable(d->core_clk);
+
+	ret = devm_add_action_or_reset(dev, ldma_clk_disable, d);
+	if (ret) {
+		dev_err(dev, "Failed to devm_add_action_or_reset, %d\n", ret);
+		return ret;
+	}
+
+	d->rst = devm_reset_control_get_optional(dev, NULL);
+	if (IS_ERR(d->rst))
+		return PTR_ERR(d->rst);
+	reset_control_deassert(d->rst);
+
+	id = readl(d->base + DMA_ID);
+	d->chan_nrs = FIELD_GET(DMA_ID_CHNR, id);
+	d->port_nrs = FIELD_GET(DMA_ID_PNR, id);
+	d->ver = FIELD_GET(DMA_ID_REV, id);
+
+	if (id & DMA_ID_AW_36B)
+		d->flags |= DMA_ADDR_36BIT;
+
+	if (IS_ENABLED(CONFIG_64BIT)) {
+		if (id & DMA_ID_AW_36B)
+			bitn = 36;
+	}
+
+	if (id & DMA_ID_DW_128B)
+		d->flags |= DMA_DATA_128BIT;
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(bitn));
+	if (ret) {
+		dev_err(dev, "No usable DMA configuration\n");
+		return ret;
+	}
+
+	if (d->ver == DMA_VER22) {
+		d->irq = platform_get_irq(pdev, 0);
+		if (d->irq < 0)
+			return d->irq;
+
+		ret = devm_request_irq(&pdev->dev, d->irq, dma_interrupt,
+				       0, DRIVER_NAME, d);
+		if (ret)
+			return ret;
+
+		d->wq = alloc_ordered_workqueue("dma_wq", WQ_MEM_RECLAIM |
+						WQ_HIGHPRI);
+		if (!d->wq)
+			return -ENOMEM;
+	}
+
+	dma_dev = &d->dma_dev;
+	dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);
+
+	/* Channel initializations */
+	INIT_LIST_HEAD(&dma_dev->channels);
+
+	/* Port Initializations */
+	d->ports = devm_kcalloc(dev, d->port_nrs, sizeof(*p), GFP_KERNEL);
+	if (!d->ports)
+		return -ENOMEM;
+
+	for (i = 0; i < d->port_nrs; i++) {
+		p = &d->ports[i];
+		p->portid = i;
+		p->ldev = d;
+	}
+
+	ret = ldma_cfg_init(d);
+	if (ret)
+		return ret;
+
+	dma_dev->dev = &pdev->dev;
+	/*
+	 * Link channel id to channel index and link to dma channel list
+	 * It also back points to controller and its port
+	 */
+	for (i = 0, k = 0; i < d->port_nrs; i++) {
+		if (d->ver == DMA_VER22) {
+			u32 chan_end;
+
+			p = &d->ports[i];
+			chan_end = p->chan_start + p->chan_sz;
+			for (j = p->chan_start; j < chan_end; j++) {
+				c = &d->chans[k];
+				c->port = p;
+				c->nr = j; /* Real channel number */
+				c->rst = DMA_CHAN_RST;
+				snprintf(c->name, sizeof(c->name), "chan%d",
+					 c->nr);
+				INIT_WORK(&c->work, dma_work);
+				c->vchan.desc_free = dma_free_desc_resource;
+				vchan_init(&c->vchan, dma_dev);
+				k++;
+			}
+		} else {
+			p = &d->ports[i];
+			for (i = 0; i < d->chan_nrs; i++) {
+				c = &d->chans[i];
+				c->port = p;
+				c->data_endian = DMA_DFT_ENDIAN;
+				c->desc_endian = DMA_DFT_ENDIAN;
+				c->data_endian_en = false;
+				c->desc_endian_en = false;
+				c->desc_rx_np = false;
+				c->flags |= DEVICE_ALLOC_DESC;
+				c->onoff = DMA_CH_OFF;
+				c->rst = DMA_CHAN_RST;
+				c->abc_en = true;
+				c->nr = i;
+				c->vchan.desc_free = dma_free_desc_resource;

drivers/dma/lgm/lgm-dma.c line 1884

Quoted 7 lines

drivers/dma/lgm/lgm-dma.c new file +7 -0

+				vchan_init(&c->vchan, dma_dev);
+			}
+		}
+	}
+
+	/* Set DMA capabilities */
+	dma_cap_zero(dma_dev->cap_mask);

You just cleared the DMA_SLAVE capability you set earlier...

Quoted 17 lines

drivers/dma/lgm/lgm-dma.c new file +16 -0

+
+	dma_dev->device_alloc_chan_resources =
+		d->inst->ops->device_alloc_chan_resources;
+	dma_dev->device_free_chan_resources =
+		d->inst->ops->device_free_chan_resources;
+	dma_dev->device_terminate_all = d->inst->ops->device_terminate_all;
+	dma_dev->device_issue_pending = d->inst->ops->device_issue_pending;
+	dma_dev->device_tx_status = d->inst->ops->device_tx_status;
+	dma_dev->device_resume = d->inst->ops->device_resume;
+	dma_dev->device_pause = d->inst->ops->device_pause;
+	dma_dev->device_config = d->inst->ops->device_config;
+	dma_dev->device_prep_slave_sg = d->inst->ops->device_prep_slave_sg;
+	dma_dev->device_synchronize = d->inst->ops->device_synchronize;
+
+	if (d->ver == DMA_VER22) {
+		dma_dev->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);

drivers/dma/lgm/lgm-dma.c line 1906

Quoted 6 lines

drivers/dma/lgm/lgm-dma.c new file +6 -0

+		dma_dev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
+		dma_dev->directions = BIT(DMA_MEM_TO_DEV) |
+				      BIT(DMA_DEV_TO_MEM);
+		dma_dev->residue_granularity =
+					DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
+	}

So, if version is != DMA_VER22, then you don't support any direction? Why register the DMA device if it can not do any transfer?

Quoted 63 lines

drivers/dma/lgm/lgm-dma.c new file +38 -0

+
+	platform_set_drvdata(pdev, d);
+
+	ldma_dev_init(d);
+
+	ret = dma_async_device_register(dma_dev);
+	if (ret) {
+		dev_err(dev, "Failed to register slave DMA engine device\n");
+		return ret;
+	}
+
+	ret = of_dma_controller_register(pdev->dev.of_node, ldma_xlate, d);
+	if (ret) {
+		dev_err(dev, "Failed to register of DMA controller\n");
+		dma_async_device_unregister(dma_dev);
+		return ret;
+	}
+
+	dev_info(dev, "Init done - rev: %x, ports: %d channels: %d\n", d->ver,
+		 d->port_nrs, d->chan_nrs);
+
+	return 0;
+}
+
+static struct platform_driver intel_ldma_driver = {
+	.probe = intel_ldma_probe,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = intel_ldma_match,
+	},
+};
+
+static int __init intel_ldma_init(void)
+{
+	return platform_driver_register(&intel_ldma_driver);
+}
+
+device_initcall(intel_ldma_init);

include/linux/dma/lgm_dma.h new file +17 -0

new file mode 100644
index 000000000000..3a2ee6ad0710
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016 ~ 2019 Intel Corporation.
+ */
+#ifndef LGM_DMA_H
+#define LGM_DMA_H
+
+#include <linux/types.h>
+#include <linux/dmaengine.h>
+
+/*!
+ * \fn int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base,
+ *                                 int desc_num)
+ * \brief Configure low level channel descriptors
+ * \param[in] chan   pointer to DMA channel that the client is using
+ * \param[in] desc_base   descriptor base physical address
+ * \param[in] desc_num   number of descriptors

include/linux/dma/lgm_dma.h line 27

Quoted 10 lines

include/linux/dma/lgm_dma.h new file +10 -0

+ * \return   0 on success
+ * \return   kernel bug reported on failure
+ *
+ * This function configure the low level channel descriptors. It will be
+ * used by CBM whose descriptor is not DDR, actually some registers.
+ */
+int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base,
+			    int desc_num);
+
+#endif /* LGM_DMA_H */

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Quoted 9 lines

drivers/dma/lgm/lgm-dma.c line 1226

Quoted 8 lines

drivers/dma/lgm/lgm-dma.c new file +8 -0

+static void dma_free_desc_resource(struct virt_dma_desc *vdesc)
+{
+    struct dw2_desc_sw *ds = to_lgm_dma_desc(vdesc);
+    struct ldma_chan *c = ds->chan;
+
+    dma_pool_free(c->desc_pool, ds->desc_hw, ds->desc_phys);
+    kfree(ds);
+    c->ds = NULL;

Is there a chance that c->ds != ds?

No, from the code i don't see any such scenario, let me know if you find any corner case.

Quoted 58 lines

Quoted 58 lines

drivers/dma/lgm/lgm-dma.c new file +58 -0

+}
+
+static struct dw2_desc_sw *
+dma_alloc_desc_resource(int num, struct ldma_chan *c)
+{
+    struct device *dev = c->vchan.chan.device->dev;
+    struct dw2_desc_sw *ds;
+
+    if (num > c->desc_num) {
+        dev_err(dev, "sg num %d exceed max %d\n", num, c->desc_num);
+        return NULL;
+    }
+
+    ds = kzalloc(sizeof(*ds), GFP_NOWAIT);
+    if (!ds)
+        return NULL;
+
+    ds->chan = c;
+
+    ds->desc_hw = dma_pool_zalloc(c->desc_pool, GFP_ATOMIC,
+                      &ds->desc_phys);
+    if (!ds->desc_hw) {
+        dev_dbg(dev, "out of memory for link descriptor\n");
+        kfree(ds);
+        return NULL;
+    }
+    ds->desc_cnt = num;
+
+    return ds;
+}
+
+static void ldma_chan_irq_en(struct ldma_chan *c)
+{
+    struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+    unsigned long flags;
+
+    spin_lock_irqsave(&d->dev_lock, flags);
+    writel(c->nr, d->base + DMA_CS);
+    writel(DMA_CI_EOP, d->base + DMA_CIE);
+    writel(BIT(c->nr), d->base + DMA_IRNEN);
+    spin_unlock_irqrestore(&d->dev_lock, flags);
+}
+
+static void dma_issue_pending(struct dma_chan *chan)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+    struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+    unsigned long flags;
+
+    if (d->ver == DMA_VER22) {
+        spin_lock_irqsave(&c->vchan.lock, flags);
+        if (vchan_issue_pending(&c->vchan)) {
+            struct virt_dma_desc *vdesc;
+
+            /* Get the next descriptor */
+            vdesc = vchan_next_desc(&c->vchan);
+            if (!vdesc) {
+                c->ds = NULL;

Quoted 8 lines

Quoted 4 lines

+                return;
+            }
+            list_del(&vdesc->node);
+            c->ds = to_lgm_dma_desc(vdesc);

you have set c->ds in dma_prep_slave_sg and the only way I can see that you will not leak memory is that the client must terminate_sync() after each transfer so that the synchronize callback is invoked between each prep_sg/issue_pending/competion.

Yes, client must call dmaengine_synchronize after each transfer to make sure free the memory assoicated with previously issued descriptors if any.

and also from the driver we are freeing up the descriptor from work queue atfer each transfer.(addressed below comments **)

Quoted 7 lines

Quoted 6 lines

+            spin_unlock_irqrestore(&c->vchan.lock, flags); +            ldma_chan_desc_hw_cfg(c, c->ds->desc_phys, c->ds->desc_cnt);
+            ldma_chan_irq_en(c);
+        }

If there is nothing peding, you will leave the spinlock wide open...

Seems i misplaced the lock. i will fix it in next version.

Quoted 129 lines

Quoted 129 lines

+    }
+    ldma_chan_on(c);

drivers/dma/lgm/lgm-dma.c new file +93 -0

+}
+
+static void dma_synchronize(struct dma_chan *chan)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+
+    /*
+     * clear any pending work if any. In that
+     * case the resource needs to be free here.
+     */
+    cancel_work_sync(&c->work);
+    vchan_synchronize(&c->vchan);
+    if (c->ds)
+        dma_free_desc_resource(&c->ds->vdesc);
+}
+
+static int dma_terminate_all(struct dma_chan *chan)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+    unsigned long flags;
+    LIST_HEAD(head);
+
+    spin_lock_irqsave(&c->vchan.lock, flags);
+    vchan_get_all_descriptors(&c->vchan, &head);
+    spin_unlock_irqrestore(&c->vchan.lock, flags);
+    vchan_dma_desc_free_list(&c->vchan, &head);
+
+    return ldma_chan_reset(c);
+}
+
+static int dma_resume_chan(struct dma_chan *chan)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+
+    ldma_chan_on(c);
+
+    return 0;
+}
+
+static int dma_pause_chan(struct dma_chan *chan)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+
+    return ldma_chan_off(c);
+}
+
+static enum dma_status
+dma_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
+          struct dma_tx_state *txstate)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+    struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+    enum dma_status status = DMA_COMPLETE;
+
+    if (d->ver == DMA_VER22)
+        status = dma_cookie_status(chan, cookie, txstate);
+
+    return status;
+}
+
+static void dma_chan_irq(int irq, void *data)
+{
+    struct ldma_chan *c = data;
+    struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+    u32 stat;
+
+    /* Disable channel interrupts  */
+    writel(c->nr, d->base + DMA_CS);
+    stat = readl(d->base + DMA_CIS);
+    if (!stat)
+        return;
+
+    writel(readl(d->base + DMA_CIE) & ~DMA_CI_ALL, d->base + DMA_CIE);
+    writel(stat, d->base + DMA_CIS);
+    queue_work(d->wq, &c->work);
+}
+
+static irqreturn_t dma_interrupt(int irq, void *dev_id)
+{
+    struct ldma_dev *d = dev_id;
+    struct ldma_chan *c;
+    unsigned long irncr;
+    u32 cid;
+
+    irncr = readl(d->base + DMA_IRNCR);
+    if (!irncr) {
+        dev_err(d->dev, "dummy interrupt\n");
+        return IRQ_NONE;
+    }
+
+    for_each_set_bit(cid, &irncr, d->chan_nrs) {
+        /* Mask */
+        writel(readl(d->base + DMA_IRNEN) & ~BIT(cid), d->base +

DMA_IRNEN);
+        /* Ack */
+        writel(readl(d->base + DMA_IRNCR) | BIT(cid), d->base + DMA_IRNCR);

drivers/dma/lgm/lgm-dma.c new file +29 -0

+
+        c = &d->chans[cid];
+        dma_chan_irq(irq, c);
+    }
+
+    return IRQ_HANDLED;
+}
+
+static struct dma_async_tx_descriptor *
+dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
+          unsigned int sglen, enum dma_transfer_direction dir,
+          unsigned long flags, void *context)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+    size_t len, avail, total = 0;
+    struct dw2_desc *hw_ds;
+    struct dw2_desc_sw *ds;
+    struct scatterlist *sg;
+    int num = sglen, i;
+    dma_addr_t addr;
+
+    if (!sgl)
+        return NULL;
+
+    for_each_sg(sgl, sg, sglen, i) {
+        avail = sg_dma_len(sg);
+        if (avail > DMA_MAX_SIZE)
+            num += DIV_ROUND_UP(avail, DMA_MAX_SIZE) - 1;
+    }

Quoted 8 lines

drivers/dma/lgm/lgm-dma.c line 1424

Quoted 6 lines

drivers/dma/lgm/lgm-dma.c new file +6 -0

+
+    ds = dma_alloc_desc_resource(num, c);
+    if (!ds)
+        return NULL;
+
+    c->ds = ds;

If you still have a transfer running then you are going to get lost that dscriptor?

No, please let me know if you find any such corner case.

Quoted 60 lines

Quoted 60 lines

drivers/dma/lgm/lgm-dma.c new file +59 -0

+
+    num = 0;
+    /* sop and eop has to be handled nicely */
+    for_each_sg(sgl, sg, sglen, i) {
+        addr = sg_dma_address(sg);
+        avail = sg_dma_len(sg);
+        total += avail;
+
+        do {
+            len = min_t(size_t, avail, DMA_MAX_SIZE);
+
+            hw_ds = &ds->desc_hw[num];
+            switch (sglen) {
+            case 1:
+                hw_ds->status.field.sop = 1;
+                hw_ds->status.field.eop = 1;
+                break;
+            default:
+                if (num == 0) {
+                    hw_ds->status.field.sop = 1;
+                    hw_ds->status.field.eop = 0;
+                } else if (num == (sglen - 1)) {
+                    hw_ds->status.field.sop = 0;
+                    hw_ds->status.field.eop = 1;
+                } else {
+                    hw_ds->status.field.sop = 0;
+                    hw_ds->status.field.eop = 0;
+                }
+                break;
+            }
+
+            /* Only 32 bit address supported */
+            hw_ds->addr = (u32)addr;
+            hw_ds->status.field.len = len;
+            hw_ds->status.field.c = 0;
+            hw_ds->status.field.bofs = addr & 0x3;
+            /* Ensure data ready before ownership change */
+            wmb();
+            hw_ds->status.field.own = DMA_OWN;
+            /* Ensure ownership changed before moving forward */
+            wmb();
+            num++;
+            addr += len;
+            avail -= len;
+        } while (avail);
+    }
+
+    ds->size = total;
+
+    return vchan_tx_prep(&c->vchan, &ds->vdesc, DMA_CTRL_ACK);
+}
+
+static int
+dma_slave_config(struct dma_chan *chan, struct dma_slave_config *cfg)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+    struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+    struct ldma_port *p = c->port;
+    unsigned long flags;

Quoted 8 lines

Quoted 6 lines

+    u32 bl;
+
+    if ((cfg->direction == DMA_DEV_TO_MEM &&
+         cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) || +        (cfg->direction == DMA_MEM_TO_DEV &&
+         cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) ||

According to the probe function these width restrictions are only valid for DMA_VER22?

YES

Quoted 9 lines

Quoted 7 lines

+        !is_slave_direction(cfg->direction)) +        return -EINVAL;
+
+    /* Default setting will be used */
+    if (!cfg->src_maxburst && !cfg->dst_maxburst) +        return 0;

maxburst == 0 is identical to maxburst == 1, it is just not set explicitly. Iow 1 word per DMA request.

This is not clear to me. Can you elaborate?

Quoted 23 lines

Quoted 8 lines

drivers/dma/lgm/lgm-dma.c new file +7 -0

+
+    /* Must be the same */
+    if (cfg->src_maxburst && cfg->dst_maxburst &&
+        cfg->src_maxburst != cfg->dst_maxburst)
+        return -EINVAL;
+
+    if (cfg->dst_maxburst)

drivers/dma/lgm/lgm-dma.c line 1512

Quoted 10 lines

drivers/dma/lgm/lgm-dma.c new file +10 -0

+        cfg->src_maxburst = cfg->dst_maxburst;
+
+    bl = ilog2(cfg->src_maxburst);
+
+    spin_lock_irqsave(&d->dev_lock, flags);
+    writel(p->portid, d->base + DMA_PS);
+    ldma_update_bits(d, DMA_PCTRL_RXBL | DMA_PCTRL_TXBL,
+             FIELD_PREP(DMA_PCTRL_RXBL, bl) |
+             FIELD_PREP(DMA_PCTRL_TXBL, bl), DMA_PCTRL);
+    spin_unlock_irqrestore(&d->dev_lock, flags);

What drivers usually do is to save the cfg and inprepare time take it into account when setting up the transfer.
Write the change to the HW before the trasnfer is started (if it has changed from previous settings)

Quoted 3 lines

Client drivers usually set the slave config ones, in most cases during probe, so the slave config rarely changes runtime, but there are cases for that.

Ok, got it. i will update in the next version.

Quoted 49 lines

Quoted 49 lines

drivers/dma/lgm/lgm-dma.c new file +48 -0

+
+    return 0;
+}
+
+static int dma_alloc_chan_resources(struct dma_chan *chan)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+    struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+    struct device *dev = c->vchan.chan.device->dev;
+    size_t    desc_sz;
+
+    if (d->ver > DMA_VER22) {
+        c->flags |= CHAN_IN_USE;
+        return 0;
+    }
+
+    if (c->desc_pool)
+        return c->desc_num;
+
+    desc_sz = c->desc_num * sizeof(struct dw2_desc);
+    c->desc_pool = dma_pool_create(c->name, dev, desc_sz,
+                       __alignof__(struct dw2_desc), 0);
+
+    if (!c->desc_pool) {
+        dev_err(dev, "unable to allocate descriptor pool\n");
+        return -ENOMEM;
+    }
+
+    return c->desc_num;
+}
+
+static void dma_free_chan_resources(struct dma_chan *chan)
+{
+    struct ldma_chan *c = to_ldma_chan(chan);
+    struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
+
+    if (d->ver == DMA_VER22) {
+        dma_pool_destroy(c->desc_pool);
+        c->desc_pool = NULL;
+        vchan_free_chan_resources(to_virt_chan(chan));
+        ldma_chan_reset(c);
+    } else {
+        c->flags &= ~CHAN_IN_USE;
+    }
+}
+
+static void dma_work(struct work_struct *work)
+{

Quoted 8 lines

Quoted 7 lines

+    struct ldma_chan *c = container_of(work, struct ldma_chan, work); +    struct dma_async_tx_descriptor *tx = &c->ds->vdesc.tx; +    struct dmaengine_desc_callback cb;
+
+    dmaengine_desc_get_callback(tx, &cb);
+    dma_cookie_complete(tx);
+    dmaengine_desc_callback_invoke(&cb, NULL);

When you are going to free up the descriptor?

**
Seems i missed free up the descriptor here. i will fix and update in the next version.

Quoted 16 lines

Quoted 16 lines

drivers/dma/lgm/lgm-dma.c new file +16 -0

+
+    dma_dev->device_alloc_chan_resources =
+        d->inst->ops->device_alloc_chan_resources;
+    dma_dev->device_free_chan_resources =
+        d->inst->ops->device_free_chan_resources;
+    dma_dev->device_terminate_all = d->inst->ops->device_terminate_all;
+    dma_dev->device_issue_pending = d->inst->ops->device_issue_pending;
+    dma_dev->device_tx_status = d->inst->ops->device_tx_status;
+    dma_dev->device_resume = d->inst->ops->device_resume;
+    dma_dev->device_pause = d->inst->ops->device_pause;
+    dma_dev->device_config = d->inst->ops->device_config;
+    dma_dev->device_prep_slave_sg = d->inst->ops->device_prep_slave_sg;
+    dma_dev->device_synchronize = d->inst->ops->device_synchronize;
+
+    if (d->ver == DMA_VER22) {
+        dma_dev->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);

Quoted 8 lines

Quoted 6 lines

+        dma_dev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); +        dma_dev->directions = BIT(DMA_MEM_TO_DEV) |
+                      BIT(DMA_DEV_TO_MEM);
+        dma_dev->residue_granularity =
+                    DMA_RESIDUE_GRANULARITY_DESCRIPTOR; +    }

So, if version is != DMA_VER22, then you don't support any direction? Why register the DMA device if it can not do any transfer?

Only dma0 instance (intel,lgm-cdma) is used as a general purpose slave DMA. we set both control and datapath here.
Other instances we set only control path. data path is taken care by dma client(GSWIP).

Only thing needs to do is get the channel, set the descriptor and just on the channel.

Quoted 79 lines

Quoted 75 lines

drivers/dma/lgm/lgm-dma.c new file +19 -0

+
+    platform_set_drvdata(pdev, d);
+
+    ldma_dev_init(d);
+
+    ret = dma_async_device_register(dma_dev);
+    if (ret) {
+        dev_err(dev, "Failed to register slave DMA engine device\n");
+        return ret;
+    }
+
+    ret = of_dma_controller_register(pdev->dev.of_node, ldma_xlate, d);
+    if (ret) {
+        dev_err(dev, "Failed to register of DMA controller\n");
+        dma_async_device_unregister(dma_dev);
+        return ret;
+    }
+
+    dev_info(dev, "Init done - rev: %x, ports: %d channels: %d\n",

d->ver,
+         d->port_nrs, d->chan_nrs);

drivers/dma/lgm/lgm-dma.c new file +18 -0

+
+    return 0;
+}
+
+static struct platform_driver intel_ldma_driver = {
+    .probe = intel_ldma_probe,
+    .driver = {
+        .name = DRIVER_NAME,
+        .of_match_table = intel_ldma_match,
+    },
+};
+
+static int __init intel_ldma_init(void)
+{
+    return platform_driver_register(&intel_ldma_driver);
+}
+
+device_initcall(intel_ldma_init);

include/linux/dma/lgm_dma.h new file +12 -0

new file mode 100644
index 000000000000..3a2ee6ad0710
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016 ~ 2019 Intel Corporation.
+ */
+#ifndef LGM_DMA_H
+#define LGM_DMA_H
+
+#include <linux/types.h>
+#include <linux/dmaengine.h>
+
+/*!
+ * \fn int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t

desc_base,
+ *                                 int desc_num)
+ * \brief Configure low level channel descriptors
+ * \param[in] chan   pointer to DMA channel that the client is using
+ * \param[in] desc_base   descriptor base physical address
+ * \param[in] desc_num   number of descriptors
+ * \return   0 on success
+ * \return   kernel bug reported on failure
+ *
+ * This function configure the low level channel descriptors. It will be
+ * used by CBM whose descriptor is not DDR, actually some registers.
+ */
+int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base,
+                int desc_num);

include/linux/dma/lgm_dma.h line 27

Quoted 2 lines

include/linux/dma/lgm_dma.h new file +2 -0

+
+#endif /* LGM_DMA_H */

- Péter