On 07/12/2022 07:09, Vignesh Raghavendra wrote: > 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) I thought that first three entries are the same, but they are not, so indeed keep it like you have it now. > >> >>> + >>> + 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. I was wrong, keep it. > >> >>> + >>> + 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 So here minItems:3, maxItems:5 Best regards, Krzysztof