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

[Krzysztof Kozlowski]

On 07/07/2024 11:13, Shresth Prasad wrote:
> Convert txt bindings of Marvell XOR v2 engines to dtschema to allow
> for validation.
> 
> Also add missing property `dma-coherent` as `drivers/dma/mv_xor_v2.c`
> calls various dma-coherent memory functions.
> 
> Signed-off-by: Shresth Prasad <shresthprasad7@xxxxxxxxx>
> ---
> Changes in v2:
>     - Update commit message to indicate addition of `dma-coherent`
>     - Change maintainer
>     - Change compatible section
>     - Add `minItems` to `clock-names`
>     - Remove "location and length" from reg description
>     - List out `clock-names` items in `if:`
>     - Create two variants of `if:`
> 
> Tested against `marvell/armada-7040-db.dtb`, `marvell/armada-7040-mochabin.dtb`
> and `marvell/armada-8080-db.dtb`
> 
>  .../bindings/dma/marvell,xor-v2.yaml          | 86 +++++++++++++++++++
>  .../devicetree/bindings/dma/mv-xor-v2.txt     | 28 ------
>  2 files changed, 86 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..da58f6e0feab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml
> @@ -0,0 +1,86 @@
> +# 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:
> +  - Hans de Goede <hdegoede@xxxxxxxxxx>

I don't think Hans maintains this platform - Marvell SoCs. Didn't we
talk already what is "platform"?


> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: marvell,xor-v2
> +      - items:
> +          - enum:
> +              - marvell,armada-7k-xor
> +          - const: marvell,xor-v2
> +

> +
> +allOf:
> +  - if:
> +      properties:
> +        clocks:
> +          maxItems: 1

Still not much improved. There are plenty of examples how this is done,
so please do not invent one, entirely new style. Think for a sec, why
doing things differently? Why this code is the first example of such syntax?

I asked to limit it pear each variant.

https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132



> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: core
> +  - if:
> +      properties:
> +        clocks:
> +          minItems: 2
> +      required:
> +        - clocks
> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: reg
> +      required:
> +        - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    xor0@6a0000 {

xor@6a0000

Best regards,
Krzysztof