Re: [PATCH v3 1/4] dt-bindings: dmaengine: Add doc for tegra gpcdma

[Jon Hunter <jonathanh@nvidia.com>]

On 27/08/2021 07:04, Akhil R wrote:
> Add DT binding document for Nvidia Tegra GPCDMA controller.
> 
> Signed-off-by: Rajesh Gumasta <rgumasta@nvidia.com>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>   .../bindings/dma/nvidia,tegra-gpc-dma.yaml         | 99 ++++++++++++++++++++++
>   1 file changed, 99 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/dma/nvidia,tegra-gpc-dma.yaml
> 
> diff --git a/Documentation/devicetree/bindings/dma/nvidia,tegra-gpc-dma.yaml b/Documentation/devicetree/bindings/dma/nvidia,tegra-gpc-dma.yaml
> new file mode 100644
> index 0000000..39827ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/nvidia,tegra-gpc-dma.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/nvidia,tegra-gpc-dma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nvidia Tegra GPC DMA Controller Device Tree Bindings

I think we typically say NVIDIA in all caps.

> +
> +description: |
> +  Tegra GPC DMA controller is a general purpose dma used for faster data

Maybe worth saying that the GPC DMA is the Genernal Purpose Central 
(GPC) DMA controller. Also 'DMA' should be in all caps and not 'dma'.


> +  transfers between memory to memory, memory to device and device to memory.
> +  Terms 'dma' and 'gpcdma' can be used interchangeably.

Note sure this last sentence really adds any value.

> +
> +maintainers:
> +  - Jon Hunter <jonathanh@nvidia.com>
> +  - Rajesh Gumasta <rgumasta@nvidia.com>
> +
> +allOf:
> +  - $ref: "dma-controller.yaml#"
> +
> +properties:
> +  "#dma-cells":
> +    const: 1

Good to add a description here. Look at the 
Documentation/devicetree/bindings/dma/nvidia,tegra210-adma.yaml for 
reference.

> +
> +  compatible:
> +    - enum:
> +      - nvidia,tegra186-gpcdma
> +      - nvidia,tegra194-gpcdma
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1

I believe that this should be 32. Look at the 
Documentation/devicetree/bindings/dma/nvidia,tegra210-adma.yaml for 
reference.

> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: gpcdma
> +
> +  iommus:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - resets
> +  - reset-names
> +  - "#dma-cells"
> +  - iommus
> +
> +examples:
> +  - |
> +    gpcdma: dma@2600000 {
> +	  compatible = "nvidia,tegra186-gpcdma";
> +	  reg = <0x0 0x2600000 0x0 0x210000>;
> +	  resets = <&bpmp TEGRA186_RESET_GPCDMA>;
> +	  reset-names = "gpcdma";
> +	  interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH
> +	                GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH

Please fix indentation.

> +					GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH
> +					GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
> +       #dma-cells = <1>;

Please fix indentation.

> +	   iommus = <&smmu TEGRA_SID_GPCDMA_0>;
> +	   dma-coherent;
> +	};
> +
> +...
> 

Jon

--
nvpublic