Re: [PATCH] dt-bindings: dma: convert text based binding to json schema

[Krzysztof Kozlowski]

On Thu, May 01, 2025 at 01:12:36PM GMT, Charan Pedumuru wrote:
> Update text binding to YAML.
> Changes during conversion:
> - Add a fallback for "nvidia,tegra30-apbdma" as it is
>   compatible with the IP core on "nvidia,tegra20-apbdma".
> - Update examples and include appropriate file directives to resolve
>   errors identified by `dt_binding_check` and `dtbs_check`.
> 

Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

Missing final nvidia,tegra20-apbdma prefix.


> Signed-off-by: Charan Pedumuru <charan.pedumuru@xxxxxxxxx>
> ---
>  .../bindings/dma/nvidia,tegra20-apbdma.txt         | 44 -----------
>  .../bindings/dma/nvidia,tegra20-apbdma.yaml        | 90 ++++++++++++++++++++++
>  2 files changed, 90 insertions(+), 44 deletions(-)
> 

...

> +$id: http://devicetree.org/schemas/dma/nvidia,tegra20-apbdma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA Tegra APB DMA Controller
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The NVIDIA Tegra APB DMA controller is a hardware component that
> +  enables direct memory access (DMA) on Tegra systems. It facilitates
> +  data transfer between I/O devices and main memory without constant
> +  CPU intervention.
> +
> +maintainers:
> +  - Jonathan Hunter <jonathanh@xxxxxxxxxx>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: nvidia,tegra20-apbdma
> +      - items:
> +          - const: nvidia,tegra30-apbdma
> +          - const: nvidia,tegra20-apbdma
> +
> +  "#dma-cells":
> +    description:
> +      Must be <1>. This dictates the length of DMA specifiers
> +      in client node's dmas properties.

Drop description, you are not telling here anything new except
explaining basically DT syntax.

> +    const: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1

reg is the second property. Old binding even had it correct.

> +
> +  interrupts:
> +    description:
> +      Should contain all of the per-channel DMA interrupts in
> +      ascending order with respect to the DMA channel index.
> +    minItems: 1
> +    maxItems: 32
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: dma
> +
> +required:
> +  - compatible
> +  - reg

And here the order is correct...

> +  - interrupts
> +  - clocks

But here different. Keep the same order as in properties.

> +  - resets
> +  - reset-names
> +  - "#dma-cells"
> +

missing allOf: to dma-controller

> +additionalProperties: false

unevaluatedProperties instead. Just open any other DMA binding.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

You included this...

> +    #include <dt-bindings/reset/tegra186-reset.h>
> +    dma@6000a000 {

Doesn't look like correct name. Schema requires specific name.

> +        compatible = "nvidia,tegra30-apbdma", "nvidia,tegra20-apbdma";
> +        reg = <0x6000a000 0x1200>;
> +        interrupts = <0 136 0x04>,

... so use it.

Best regards,
Krzysztof