| Message ID | 20221206043554.1521522-2-vigneshr@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dmaengine: Add support for AM62A SoC DMAs | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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@ti.com> > --- > .../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. > + - 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" > + > + 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" > + > + 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. > > "#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. > - > unevaluatedProperties: false > > examples: Best regards, Krzysztof
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@ti.com> >> --- >> .../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!
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@ti.com> >>> --- >>> .../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
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# + - 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 + + 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 + + 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 "#dma-cells": const: 3 @@ -65,19 +122,13 @@ properties: cell 3: ASEL value for the channel - reg: - maxItems: 5 - - reg-names: - items: - - const: gcfg - - const: bchanrt - - const: rchanrt - - const: tchanrt - - const: ringrt - 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 - unevaluatedProperties: false examples:
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@ti.com> --- .../devicetree/bindings/dma/ti/k3-bcdma.yaml | 87 ++++++++++++++----- 1 file changed, 63 insertions(+), 24 deletions(-)