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

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

[Shresth Prasad]

Convert txt bindings of Marvell XOR v2 engines to dtschema to allow
for validation.

Signed-off-by: Shresth Prasad <shresthprasad7@gmail.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@kernel.org>
+
+properties:
+  compatible:
+    contains:
+      enum:
+        - marvell,armada-7k-xor
+        - marvell,xor-v2
+
+  reg:
+    items:
+      - description: DMA registers location and length
+      - description: global registers location and length
+
+  clocks:
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: core
+      - const: reg
+
+  msi-parent:
+    description:
+      Phandle to the MSI-capable interrupt controller used for
+      interrupts.
+    maxItems: 1
+
+  dma-coherent: true
+
+required:
+  - compatible
+  - reg
+  - msi-parent
+  - dma-coherent
+
+if:
+  required:
+    - clocks
+  properties:
+    clocks:
+      minItems: 2
+      maxItems: 2
+then:
+  required:
+    - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    xor0@6a0000 {
+        compatible = "marvell,armada-7k-xor", "marvell,xor-v2";
+        reg = <0x6a0000 0x1000>, <0x6b0000 0x1000>;
+        clocks = <&ap_clk 0>, <&ap_clk 1>;
+        clock-names = "core", "reg";
+        msi-parent = <&gic_v2m0>;
+        dma-coherent;
+    };
diff --git a/Documentation/devicetree/bindings/dma/mv-xor-v2.txt b/Documentation/devicetree/bindings/dma/mv-xor-v2.txt
deleted file mode 100644
index 9c38bbe7e6d7..000000000000
--- a/Documentation/devicetree/bindings/dma/mv-xor-v2.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-* Marvell XOR v2 engines
-
-Required properties:
-- compatible: one of the following values:
-    "marvell,armada-7k-xor"
-    "marvell,xor-v2"
-- reg: Should contain registers location and length (two sets)
-    the first set is the DMA registers
-    the second set is the global registers
-- msi-parent: Phandle to the MSI-capable interrupt controller used for
-  interrupts.
-
-Optional properties:
-- clocks: Optional reference to the clocks used by the XOR engine.
-- clock-names: mandatory if there is a second clock, in this case the
-   name must be "core" for the first clock and "reg" for the second
-   one
-
-
-Example:
-
-	xor0@400000 {
-		compatible = "marvell,xor-v2";
-		reg = <0x400000 0x1000>,
-		      <0x410000 0x1000>;
-		msi-parent = <&gic_v2m0>;
-		dma-coherent;
-	};
-- 
2.45.2

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

[Krzysztof Kozlowski]

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@gmail.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@kernel.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@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

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

[Shresth Prasad]

On Mon, Jun 24, 2024 at 10:47 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> 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@gmail.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@kernel.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:

I'm not sure what you mean, the original txt stated that `clock-names`
is only required if there are two `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?

Sorry about that, I added this because all the nodes I looked at
contained `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.
>

I did it this way to allow for `clock-names` to only be required if there
are two `clocks` present. Is there another way I should be doing this?

> > +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...

Right, thank you for your feedback.
I'll most certainly read up more on this more.

Regards,
Shresth

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

[Krzysztof Kozlowski]

On 24/06/2024 12:14, Shresth Prasad wrote:
> On Mon, Jun 24, 2024 at 10:47 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> 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@gmail.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@kernel.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:
> 
> I'm not sure what you mean, the original txt stated that `clock-names`
> is only required if there are two `clocks`.

Exactly. It said "required", not "disallowed for 1 clock case". You
basically made it impossible to use for one case, so standard reply:
these should be always in sync.

> 
>>
>>> +
>>> +  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?
> 
> Sorry about that, I added this because all the nodes I looked at
> contained `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.
>>
> 
> I did it this way to allow for `clock-names` to only be required if there
> are two `clocks` present. Is there another way I should be doing this?

Why number of clocks would mean you need clock-names? Why does it
matter? If the driver is taking second clock by name, it does not mean
second clock name can be anything for other cases.


Best regards,
Krzysztof