On 24/06/2024 12:14, Shresth Prasad wrote: > 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`. 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
