Hi Krzysztof, On 06/12/22 14:02, Krzysztof Kozlowski wrote: > On 06/12/2022 05:35, Vignesh Raghavendra wrote: >> AM62A SoC has a dedicated BCDMA that serves Camera Serial Interface >> (CSI) IP. Add new compatible for the same. Unlike system >> BCDMA, this instance only has RX DMA channels and lack TX or block copy >> channel. Thus make those properties optional. Additionally CSI RX has >> independent power domain, add the binding for the same. >> >> Signed-off-by: Vignesh Raghavendra <vigneshr@xxxxxx> >> --- >> .../devicetree/bindings/dma/ti/k3-bcdma.yaml | 87 ++++++++++++++----- >> 1 file changed, 63 insertions(+), 24 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml >> index 08627d91e607..d7b5adbb9b2e 100644 >> --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml >> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml >> @@ -32,9 +32,66 @@ allOf: >> - $ref: /schemas/dma/dma-controller.yaml# >> - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml# >> > > When adding if:then:, please move entire allOf after "required:" part. Sure, will do > >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: ti,am62a-dmss-bcdma-csirx >> + then: >> + properties: >> + ti,sci-rm-range-bchan: false >> + ti,sci-rm-range-tchan: false >> + >> + reg: >> + maxItems: 3 >> + >> + reg-names: >> + items: >> + - const: gcfg >> + - const: rchanrt >> + - const: ringrt > > With my changes further this can be only "maxItems: 3" Yes, but wont that mean any of the 3 reg-names out of the 5? Would it not be better to further restrict specifically to above 3 reg-names (as thats how the IP is) > >> + >> + required: >> + - compatible >> + - "#dma-cells" >> + - reg >> + - reg-names >> + - msi-parent >> + - ti,sci >> + - ti,sci-dev-id >> + - ti,sci-rm-range-rchan >> + - power-domains >> + >> + else: >> + properties: >> + reg: >> + maxItems: 5 >> + >> + reg-names: >> + items: >> + - const: gcfg >> + - const: bchanrt >> + - const: rchanrt >> + - const: tchanrt >> + - const: ringrt > > With my changes further this can be only "minItems: 5" Ok. > >> + >> + required: >> + - compatible >> + - "#dma-cells" >> + - reg >> + - reg-names >> + - msi-parent >> + - ti,sci >> + - ti,sci-dev-id >> + - ti,sci-rm-range-bchan >> + - ti,sci-rm-range-tchan >> + - ti,sci-rm-range-rchan >> + >> properties: >> compatible: >> - const: ti,am64-dmss-bcdma >> + enum: >> + - ti,am64-dmss-bcdma >> + - ti,am62a-dmss-bcdma-csirx > > Keep some order, e.g. alphabetical. This reduces later conflicts on > simultaneous edits. Will fix! > >> >> "#dma-cells": >> const: 3 >> @@ -65,19 +122,13 @@ properties: >> >> cell 3: ASEL value for the channel >> >> - reg: >> - maxItems: 5 > > Keep it here with widest constrains - minItems: 3, maxItems: 5 > >> - >> - reg-names: >> - items: >> - - const: gcfg >> - - const: bchanrt >> - - const: rchanrt >> - - const: tchanrt >> - - const: ringrt > > Keep the list here with minItems: 3 > >> - >> msi-parent: true >> >> + power-domains: >> + description: >> + Power domain if available >> + maxItems: 1 >> + >> ti,asel: >> $ref: /schemas/types.yaml#/definitions/uint32 >> description: ASEL value for non slave channels >> @@ -115,18 +166,6 @@ properties: >> items: >> maximum: 0x3f >> >> -required: >> - - compatible >> - - "#dma-cells" >> - - reg >> - - reg-names >> - - msi-parent >> - - ti,sci >> - - ti,sci-dev-id >> - - ti,sci-rm-range-bchan >> - - ti,sci-rm-range-tchan >> - - ti,sci-rm-range-rchan > > Keep required here. Customize it if needed in if:then:else. Got it, will fix accordingly... > >> - >> unevaluatedProperties: false >> >> examples: > > Best regards, > Krzysztof > Thanks for the review! -- Regards Vignesh