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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 24, 2024 at 10:47 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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@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:

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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux