Re: [PATCH v2] dt-bindings: dmaengine: xlnx,axi-dma: Convert bindings into yaml

[Joseph, Abin]

Hi Krzysztof,


On 2/25/2026 3:52 PM, Krzysztof Kozlowski wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Wed, Feb 25, 2026 at 10:35:21AM +0530, Abin Joseph wrote:
> > Convert the bindings document for Xilinx DMA from txt to yaml.
> > No changes to existing binding description.
> >
> > Signed-off-by: Abin Joseph <abin.joseph@xxxxxxx>
> > ---
> >
> > v2:
> > -> Add examples for each compatible
>
> No, why? We did not ask for that and (see writing schema) we ask for one
> example, more only if they are different.

Sorry about that, my mistake.

Based on Frank Li’s comment,
[the YAML schema example should use only one of the compatible strings.]

I misunderstood this and thought we needed
to add examples for each compatible.

I’ll fix this and update it correctly in the next revision.


> ...
>
>
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - xlnx,axi-cdma-1.00.a
> > +      - xlnx,axi-dma-1.00.a
> > +      - xlnx,axi-mcdma-1.00.a
> > +      - xlnx,axi-vdma-1.00.a
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#dma-cells":
> > +    const: 1
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 1
> > +
> > +  interrupts:
> > +    items:
> > +      - description: Interrupt for single channel (MM2S or S2MM)
> > +      - description: Interrupt for dual channel configuration
> > +    minItems: 1
> > +    description:
> > +      Interrupt lines for the DMA controller. Only used when
> > +      xlnx,axistream-connected is present (DMA connected to AXI Stream
> > +      IP). When child dma-channel nodes are present, interrupts are
> > +      specified in the child nodes instead.
> > +
> > +  clocks:
> > +    minItems: 1
> > +    maxItems: 5
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 5
> > +
> > +  dma-ranges: true
> > +
> > +  xlnx,addrwidth:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [32, 64]
> > +    description: The DMA addressing size in bits.
> > +
> > +  xlnx,num-fstores:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 1
> > +    maximum: 32
> > +    description: Should be the number of framebuffers as configured in h/w.
> > +
> > +  xlnx,flush-fsync:
> > +    type: boolean
> > +    description: Tells which channel to Flush on Frame sync.
> > +
> > +  xlnx,sg-length-width:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 8
> > +    maximum: 26
> > +    default: 23
> > +    description:
> > +      Width in bits of the length register as configured in hardware.
> > +
> > +  xlnx,irq-delay:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 0
> > +    maximum: 255
> > +    description:
> > +      Tells the interrupt delay timeout value. Valid range is from 0-255.
> > +      Setting this value to zero disables the delay timer interrupt.
> > +      1 timeout interval = 125 * clock period of SG clock.
> > +
> > +  xlnx,axistream-connected:
> > +    type: boolean
> > +    description: Tells whether DMA is connected to AXI stream IP.
> > +
> > +patternProperties:
> > +  "^dma-channel(-mm2s|-s2mm)?$":
> > +    type: object
> > +    description:
> > +      Should have at least one channel and can have up to two channels per
> > +      device. This node specifies the properties of each DMA channel.
> > +
> > +    properties:
> > +      compatible:
> > +        enum:
> > +          - xlnx,axi-vdma-mm2s-channel
> > +          - xlnx,axi-vdma-s2mm-channel
> > +          - xlnx,axi-cdma-channel
> > +          - xlnx,axi-dma-mm2s-channel
> > +          - xlnx,axi-dma-s2mm-channel
> > +
> > +      interrupts:
> > +        maxItems: 1
> > +
> > +      xlnx,datawidth:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        enum: [32, 64, 128, 256, 512, 1024]
> > +        description: Should contain the stream data width, take values {32,64...1024}.
> > +
> > +      xlnx,include-dre:
> > +        type: boolean
> > +        description: Tells hardware is configured for Data Realignment Engine.
> > +
> > +      xlnx,genlock-mode:
> > +        type: boolean
> > +        description: Tells Genlock synchronization is enabled/disabled in hardware.
> > +
> > +      xlnx,enable-vert-flip:
> > +        type: boolean
> > +        description:
> > +          Tells vertical flip is enabled/disabled in hardware(S2MM path).
> > +
> > +      dma-channels:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: Number of dma channels in child node.
> > +
> > +    required:
> > +      - compatible
> > +      - interrupts
> > +      - xlnx,datawidth
> > +
> > +    additionalProperties: false
> > +
> > +allOf:
> > +  - $ref: ../dma-controller.yaml#
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: xlnx,axi-vdma-1.00.a
> > +    then:
> > +      properties:
> > +        clock-names:
> > +          contains:
> > +            const: s_axi_lite_aclk
>
> This is REALLY unexpected syntax. You are supposed to have strictly
> ordered list - why do you need need it to be so flexible? And if element
> is required it should be the first item. There is no single DTS even
> mentioning it!


I will fix this syntax

> > +          items:
> > +            enum:
> > +              - s_axi_lite_aclk
> > +              - m_axi_mm2s_aclk
> > +              - m_axi_s2mm_aclk
> > +              - m_axis_mm2s_aclk
> > +              - s_axis_s2mm_aclk
> > +          minItems: 1
> > +          maxItems: 5
> > +      patternProperties:
> > +        "^dma-channel(-mm2s|-s2mm)?$":
> > +          properties:
> > +            compatible:
> > +              enum:
> > +                - xlnx,axi-vdma-mm2s-channel
> > +                - xlnx,axi-vdma-s2mm-channel
> > +      required:
> > +        - xlnx,num-fstores
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: xlnx,axi-cdma-1.00.a
> > +    then:
> > +      properties:
> > +        clock-names:
> > +          items:
> > +            - const: s_axi_lite_aclk
> > +            - const: m_axi_aclk
> > +      patternProperties:
> > +        "^dma-channel(-mm2s|-s2mm)?$":
> > +          properties:
> > +            compatible:
> > +              enum:
> > +                - xlnx,axi-cdma-channel
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - xlnx,axi-dma-1.00.a
> > +              - xlnx,axi-mcdma-1.00.a
> > +    then:
> > +      properties:
> > +        clock-names:
> > +          contains:
> > +            const: s_axi_lite_aclk
>
> Why do you need this?
>
> > +          items:
> > +            enum:
> > +              - s_axi_lite_aclk
> > +              - m_axi_mm2s_aclk
> > +              - m_axi_s2mm_aclk
> > +              - m_axi_sg_aclk
>
> Why this cannot be ordered list like we expect (see writing bindings)?

sure will do it.

> > +          minItems: 1
> > +          maxItems: 4
> > +      patternProperties:
> > +        "^dma-channel(-mm2s|-s2mm)?(@[0-9a-f]+)?$":
> > +          properties:
> > +            compatible:
> > +              enum:
> > +                - xlnx,axi-dma-mm2s-channel
> > +                - xlnx,axi-dma-s2mm-channel
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - xlnx,axi-cdma-1.00.a
> > +              - xlnx,axi-mcdma-1.00.a
> > +              - xlnx,axi-dma-1.00.a
> > +    then:
> > +      properties:
> > +        interrupts: false
>
> Why interrupts are flexible in other case?
>
> This should be probably squashed in each of previous if:then:.

I will squash the interrupt constraints into each if:then: block as 
suggested.

For VDMA and CDMA, interrupts are always in child nodes regardless of
axistream-connected, so I'll add "interrupts: false" to their respective 
blocks.

For AXI DMA and MCDMA:
- When xlnx,axistream-connected is present: interrupts are at parent level
- When not connected to AXI stream: interrupts are in child nodes

Since the current schema already allows both parent and child node 
interrupts
for DMA/MCDMA (flexible behavior), I won't add "interrupts: false" to their
block, which correctly permits both configurations.

I will fix this in the next version.


> > +
> > +required:
> > +  - "#dma-cells"
> > +  - reg
> > +  - xlnx,addrwidth
> > +  - dma-ranges
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    dma-controller@40030000 {
> > +        compatible = "xlnx,axi-vdma-1.00.a";
> > +        #dma-cells = <1>;
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        reg = <0x40030000 0x10000>;
> > +        dma-ranges = <0x0 0x0 0x40000000>;
> > +        xlnx,num-fstores = <8>;
> > +        xlnx,flush-fsync;
> > +        xlnx,addrwidth = <32>;
> > +        clocks = <&clk 0>, <&clk 1>, <&clk 2>, <&clk 3>, <&clk 4>;
> > +        clock-names = "s_axi_lite_aclk", "m_axi_mm2s_aclk",
> > +                      "m_axi_s2mm_aclk", "m_axis_mm2s_aclk",
> > +                      "s_axis_s2mm_aclk";
> > +
> > +        dma-channel-mm2s {
> > +            compatible = "xlnx,axi-vdma-mm2s-channel";
> > +            interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
> > +            xlnx,datawidth = <64>;
> > +        };
> > +
> > +        dma-channel-s2mm {
> > +            compatible = "xlnx,axi-vdma-s2mm-channel";
> > +            interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> > +            xlnx,datawidth = <64>;
> > +        };
> > +    };
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    dma-controller@a4030000 {
> > +        compatible = "xlnx,axi-dma-1.00.a";
> > +        #dma-cells = <1>;
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        reg = <0xa4030000 0x10000>;
> > +        dma-ranges = <0x0 0x0 0x40000000>;
> > +        xlnx,addrwidth = <32>;
> > +        xlnx,sg-length-width = <14>;
> > +        clocks = <&clk 0>, <&clk 1>, <&clk 2>, <&clk 3>;
> > +        clock-names = "s_axi_lite_aclk", "m_axi_mm2s_aclk",
> > +                      "m_axi_s2mm_aclk", "m_axi_sg_aclk";
> > +
> > +        dma-channel-mm2s {
> > +            compatible = "xlnx,axi-dma-mm2s-channel";
> > +            interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> > +            xlnx,datawidth = <64>;
> > +            xlnx,include-dre;
> > +        };
> > +
> > +        dma-channel-s2mm {
> > +            compatible = "xlnx,axi-dma-s2mm-channel";
> > +            interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> > +            xlnx,datawidth = <64>;
> > +            xlnx,include-dre;
> > +        };
> > +    };
> > +
>
> Drop all examples past this point.

sure, thanks

Abin Joseph

> Best regards,
> Krzysztof