| Message ID | 20240623124507.27297-2-shresthprasad7@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dt-bindings: dma: mv-xor-v2: Convert to dtschema | expand |
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
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
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
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; - };
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