Re: [PATCH v5 1/2] dt-bindings: dma: Add bindings for intel LGM SOC
From: Reddy, MallikarjunaX
Date: Tue Aug 18 2020 - 03:01:42 EST
Hi Rob,
Thanks for your valuable comments. Please see my comments inline..
On 8/15/2020 4:32 AM, Rob Herring wrote:
On Fri, Aug 14, 2020 at 01:26:09PM +0800, Amireddy Mallikarjuna reddy wrote:
Add DT bindings YAML schema for DMA controller driver
of Lightning Mountain(LGM) SoC.
Signed-off-by: Amireddy Mallikarjuna reddy <mallikarjunax.reddy@xxxxxxxxxxxxxxx>
---
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
diff --git a/Documentation/devicetree/bindings/dma/intel,ldma.yaml b/Documentation/devicetree/bindings/dma/intel,ldma.yaml
new file mode 100644
index 000000000000..9beaf191a6de
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/intel,ldma.yaml
@@ -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:
+ - chuanhua.lei@xxxxxxxxx
+ - mallikarjunax.reddy@xxxxxxxxx
+
+allOf:
+ - $ref: "dma-controller.yaml#"
+
+properties:
+ $nodename:
+ pattern: "^dma-controller(@.*)?$"
+
+ "#dma-cells":
+ const: 1
Example says 3.
OK, i will fix it.
+
+ 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'.
ok. I will update it.
+
+ 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]
+ 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?
Answered below. (*ABC)
+
+ 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.
+
+ 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.
+
+ 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?
+
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - '#dma-cells'
Add:
additionalProperties: false
Sure i will update it in the next patch..
+
+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.
ok. will Document in the next patch.
+ 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>;
+ };
+ };
+ };
--
2.11.0