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
