Re: [PATCH] dt-bindings: dma: mv-xor-v2: Convert to dtschema

From: Krzysztof Kozlowski
Date: Mon Jun 24 2024 - 01:17:30 EST


On 23/06/2024 14:45, Shresth Prasad wrote:
> Convert txt bindings of Marvell XOR v2 engines to dtschema to allow
> for validation.
>
> Signed-off-by: Shresth Prasad <shresthprasad7@xxxxxxxxx>
> ---
> Tested against `marvell/armada-7040-db.dtb`, `marvell/armada-7040-mochabin.dtb`
> and `marvell/armada-8080-db.dtb`
>
> .../bindings/dma/marvell,xor-v2.yaml | 69 +++++++++++++++++++
> .../devicetree/bindings/dma/mv-xor-v2.txt | 28 --------
> 2 files changed, 69 insertions(+), 28 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml
> delete mode 100644 Documentation/devicetree/bindings/dma/mv-xor-v2.txt
>
> diff --git a/Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml b/Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml
> new file mode 100644
> index 000000000000..3d7481c1917e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/marvell,xor-v2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell XOR v2 engines
> +
> +maintainers:
> + - Vinod Koul <vkoul@xxxxxxxxxx>

Should be rather platform maintainer.

> +
> +properties:
> + compatible:
> + contains:

This cannot be unspecific. Drop contains.

> + enum:
> + - marvell,armada-7k-xor
> + - marvell,xor-v2
> +
> + reg:
> + items:
> + - description: DMA registers location and length
> + - description: global registers location and length

Drop "location and length", redundant.

> +
> + clocks:
> + minItems: 1
> + maxItems: 2
> +
> + clock-names:
> + items:
> + - const: core
> + - const: reg

This does not match number of items in clocks:

> +
> + msi-parent:
> + description:
> + Phandle to the MSI-capable interrupt controller used for
> + interrupts.
> + maxItems: 1
> +
> + dma-coherent: true

This was not present in the binding and commit msg did not explain why
this is needed. Are devices really DMA coherent?

> +
> +required:
> + - compatible
> + - reg
> + - msi-parent
> + - dma-coherent
> +
> +if:

Put it under allOf: in this place.

> + required:
> + - clocks

This does not work and does not make much sense. Probably you want to
list the items per variant?


> + properties:
> + clocks:
> + minItems: 2
> + maxItems: 2

Instead list and describe the items.

> +then:
> + required:
> + - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + xor0@6a0000 {
> + compatible = "marvell,armada-7k-xor", "marvell,xor-v2";

This totally does not match your binding.

Please, read example-schema, other bindings, my old talks and other
resources before doing conversions, so we can avoid such trivial
mistakes. You enumerated compatibles (enum), but here have a list. A
list is not an enumeration, obviously...

Best regards,
Krzysztof