[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <62c05c34-0b69-4091-8c3a-d0b8befa9150@kernel.org>
Date: Mon, 24 Jun 2024 07:17:16 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Shresth Prasad <shresthprasad7@...il.com>, vkoul@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org
Cc: dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, skhan@...uxfoundation.org,
javier.carrasco.cruz@...il.com
Subject: Re: [PATCH] dt-bindings: dma: mv-xor-v2: Convert to dtschema
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@...il.com>
> ---
> 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@...nel.org>
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@...000 {
> + 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
Powered by blists - more mailing lists