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

[Charan Pedumuru <charan.pedumuru@gmail.com>]

On 02-05-2025 13:28, Krzysztof Kozlowski wrote:
> 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.


Sure, will update that accordingly.


>
>
>> Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
>> ---
>>   .../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@nvidia.com>
>> +
>> +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


The reason, I didn't include dma-controller is the pattern under 
$nodename in dma-controller is not matching with the pattern present in 
dts files.

So, I excluded it.


>
>> +additionalProperties: false
> unevaluatedProperties instead. Just open any other DMA binding.


sure.


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


I checked the other DMA binding from nvidia, they included the same 
header file. I can change it to irq.h, if required


>
>> +    #include <dt-bindings/reset/tegra186-reset.h>
>> +    dma@6000a000 {
> Doesn't look like correct name. Schema requires specific name.


The dt_binding_check is successful when I included allOf to 
dma-controller and used dma-controller@600 as node name, but when I was 
checking for dtb, the pattern is not matching with the one

inside dma-controller, so I removed allOf and changed the node name in 
examples according to the node present inside the dts file.


>
>> +        compatible = "nvidia,tegra30-apbdma", "nvidia,tegra20-apbdma";
>> +        reg = <0x6000a000 0x1200>;
>> +        interrupts = <0 136 0x04>,
> ... so use it.
>
> Best regards,
> Krzysztof
>
-- 
Best Regards,
Charan.