| Message ID | 20230810174356.3322583-2-vigneshr@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dt-bindings: dma: ti: k3* : Update optional reg regions | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Thu, Aug 10, 2023 at 11:13:53PM +0530, Vignesh Raghavendra wrote: > Block copy DMA(BCDMA)module on K3 SoCs have ring cfg, TX and RX > channel cfg register regions which are usually configured by a Device > Management firmware. But certain entities such as bootloader (like > U-Boot) may have to access them directly. Describe this region in the > binding documentation for completeness of module description. > > Keep the binding compatible with existing DTS files by requiring first > five regions to be present at least. > > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > --- > .../devicetree/bindings/dma/ti/k3-bcdma.yaml | 25 +++++++++++++------ > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml > index 4ca300a42a99..d166e284532b 100644 > --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml > +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml > @@ -37,11 +37,11 @@ properties: > > reg: > minItems: 3 > - maxItems: 5 > + maxItems: 8 How come none of these reg entries have a description? What differentiates a "gcfg" from a "cfg" for example? > > reg-names: > minItems: 3 > - maxItems: 5 > + maxItems: 8 > > "#dma-cells": > const: 3 > @@ -161,14 +161,19 @@ allOf: > properties: > reg: > minItems: 5 > + maxItems: 8 > > reg-names: > + minItems: 5 > items: > - const: gcfg > - const: bchanrt > - const: rchanrt > - const: tchanrt > - const: ringrt > + - const: cfg > + - const: tchan > + - const: rchan > > required: > - ti,sci-rm-range-bchan > @@ -216,12 +221,16 @@ examples: > main_bcdma: dma-controller@485c0100 { > compatible = "ti,am64-dmss-bcdma"; > > - reg = <0x0 0x485c0100 0x0 0x100>, > - <0x0 0x4c000000 0x0 0x20000>, > - <0x0 0x4a820000 0x0 0x20000>, > - <0x0 0x4aa40000 0x0 0x20000>, > - <0x0 0x4bc00000 0x0 0x100000>; > - reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt"; > + reg = <0x00 0x485c0100 0x00 0x100>, Why have you added extra zeros? (0x00) Thanks, Conor. > + <0x00 0x4c000000 0x00 0x20000>, > + <0x00 0x4a820000 0x00 0x20000>, > + <0x00 0x4aa40000 0x00 0x20000>, > + <0x00 0x4bc00000 0x00 0x100000>, > + <0x00 0x48600000 0x00 0x8000>, > + <0x00 0x484a4000 0x00 0x2000>, > + <0x00 0x484c2000 0x00 0x2000>; > + reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt", > + "cfg", "tchan", "rchan"; > msi-parent = <&inta_main_dmss>; > #dma-cells = <3>; > > -- > 2.41.0 >
On 11/08/23 00:05, Conor Dooley wrote: > On Thu, Aug 10, 2023 at 11:13:53PM +0530, Vignesh Raghavendra wrote: >> Block copy DMA(BCDMA)module on K3 SoCs have ring cfg, TX and RX >> channel cfg register regions which are usually configured by a Device >> Management firmware. But certain entities such as bootloader (like >> U-Boot) may have to access them directly. Describe this region in the >> binding documentation for completeness of module description. >> >> Keep the binding compatible with existing DTS files by requiring first >> five regions to be present at least. >> >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >> --- >> .../devicetree/bindings/dma/ti/k3-bcdma.yaml | 25 +++++++++++++------ >> 1 file changed, 17 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml >> index 4ca300a42a99..d166e284532b 100644 >> --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml >> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml >> @@ -37,11 +37,11 @@ properties: >> >> reg: >> minItems: 3 >> - maxItems: 5 >> + maxItems: 8 > > How come none of these reg entries have a description? What > differentiates a "gcfg" from a "cfg" for example? > Ok, I will a patch to describe the regions first before adding new ones. >> >> reg-names: >> minItems: 3 >> - maxItems: 5 >> + maxItems: 8 >> >> "#dma-cells": >> const: 3 >> @@ -161,14 +161,19 @@ allOf: >> properties: >> reg: >> minItems: 5 >> + maxItems: 8 >> >> reg-names: >> + minItems: 5 >> items: >> - const: gcfg >> - const: bchanrt >> - const: rchanrt >> - const: tchanrt >> - const: ringrt >> + - const: cfg >> + - const: tchan >> + - const: rchan >> >> required: >> - ti,sci-rm-range-bchan >> @@ -216,12 +221,16 @@ examples: >> main_bcdma: dma-controller@485c0100 { >> compatible = "ti,am64-dmss-bcdma"; >> >> - reg = <0x0 0x485c0100 0x0 0x100>, >> - <0x0 0x4c000000 0x0 0x20000>, >> - <0x0 0x4a820000 0x0 0x20000>, >> - <0x0 0x4aa40000 0x0 0x20000>, >> - <0x0 0x4bc00000 0x0 0x100000>; >> - reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt"; >> + reg = <0x00 0x485c0100 0x00 0x100>, > > Why have you added extra zeros? (0x00) Sorry, copy paste error, was trying to copy example from real DT that use 0x00. Will fix. Thanks! > > Thanks, > Conor. > >> + <0x00 0x4c000000 0x00 0x20000>, >> + <0x00 0x4a820000 0x00 0x20000>, >> + <0x00 0x4aa40000 0x00 0x20000>, >> + <0x00 0x4bc00000 0x00 0x100000>, >> + <0x00 0x48600000 0x00 0x8000>, >> + <0x00 0x484a4000 0x00 0x2000>, >> + <0x00 0x484c2000 0x00 0x2000>; >> + reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt", >> + "cfg", "tchan", "rchan"; >> msi-parent = <&inta_main_dmss>; >> #dma-cells = <3>; >> >> -- >> 2.41.0 >>
On 10/08/2023 20:43, Vignesh Raghavendra wrote: > Block copy DMA(BCDMA)module on K3 SoCs have ring cfg, TX and RX > channel cfg register regions which are usually configured by a Device > Management firmware. But certain entities such as bootloader (like > U-Boot) may have to access them directly. Describe this region in the > binding documentation for completeness of module description. > > Keep the binding compatible with existing DTS files by requiring first > five regions to be present at least. > > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > --- > .../devicetree/bindings/dma/ti/k3-bcdma.yaml | 25 +++++++++++++------ > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml > index 4ca300a42a99..d166e284532b 100644 > --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml > +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml > @@ -37,11 +37,11 @@ properties: > > reg: > minItems: 3 > - maxItems: 5 > + maxItems: 8 > > reg-names: > minItems: 3 > - maxItems: 5 > + maxItems: 8 > > "#dma-cells": > const: 3 > @@ -161,14 +161,19 @@ allOf: > properties: > reg: > minItems: 5 > + maxItems: 8 > > reg-names: > + minItems: 5 > items: > - const: gcfg > - const: bchanrt > - const: rchanrt > - const: tchanrt > - const: ringrt > + - const: cfg > + - const: tchan > + - const: rchan > > required: > - ti,sci-rm-range-bchan > @@ -216,12 +221,16 @@ examples: > main_bcdma: dma-controller@485c0100 { > compatible = "ti,am64-dmss-bcdma"; > > - reg = <0x0 0x485c0100 0x0 0x100>, > - <0x0 0x4c000000 0x0 0x20000>, > - <0x0 0x4a820000 0x0 0x20000>, > - <0x0 0x4aa40000 0x0 0x20000>, > - <0x0 0x4bc00000 0x0 0x100000>; > - reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt"; > + reg = <0x00 0x485c0100 0x00 0x100>, > + <0x00 0x4c000000 0x00 0x20000>, > + <0x00 0x4a820000 0x00 0x20000>, > + <0x00 0x4aa40000 0x00 0x20000>, > + <0x00 0x4bc00000 0x00 0x100000>, > + <0x00 0x48600000 0x00 0x8000>, This is BCDMA_RING region and named as 'cfg'? > + <0x00 0x484a4000 0x00 0x2000>, > + <0x00 0x484c2000 0x00 0x2000>; > + reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt", > + "cfg", "tchan", "rchan"; If you do this then add the bchan region also? > msi-parent = <&inta_main_dmss>; > #dma-cells = <3>; >
diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml index 4ca300a42a99..d166e284532b 100644 --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml @@ -37,11 +37,11 @@ properties: reg: minItems: 3 - maxItems: 5 + maxItems: 8 reg-names: minItems: 3 - maxItems: 5 + maxItems: 8 "#dma-cells": const: 3 @@ -161,14 +161,19 @@ allOf: properties: reg: minItems: 5 + maxItems: 8 reg-names: + minItems: 5 items: - const: gcfg - const: bchanrt - const: rchanrt - const: tchanrt - const: ringrt + - const: cfg + - const: tchan + - const: rchan required: - ti,sci-rm-range-bchan @@ -216,12 +221,16 @@ examples: main_bcdma: dma-controller@485c0100 { compatible = "ti,am64-dmss-bcdma"; - reg = <0x0 0x485c0100 0x0 0x100>, - <0x0 0x4c000000 0x0 0x20000>, - <0x0 0x4a820000 0x0 0x20000>, - <0x0 0x4aa40000 0x0 0x20000>, - <0x0 0x4bc00000 0x0 0x100000>; - reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt"; + reg = <0x00 0x485c0100 0x00 0x100>, + <0x00 0x4c000000 0x00 0x20000>, + <0x00 0x4a820000 0x00 0x20000>, + <0x00 0x4aa40000 0x00 0x20000>, + <0x00 0x4bc00000 0x00 0x100000>, + <0x00 0x48600000 0x00 0x8000>, + <0x00 0x484a4000 0x00 0x2000>, + <0x00 0x484c2000 0x00 0x2000>; + reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt", + "cfg", "tchan", "rchan"; msi-parent = <&inta_main_dmss>; #dma-cells = <3>;
Block copy DMA(BCDMA)module on K3 SoCs have ring cfg, TX and RX channel cfg register regions which are usually configured by a Device Management firmware. But certain entities such as bootloader (like U-Boot) may have to access them directly. Describe this region in the binding documentation for completeness of module description. Keep the binding compatible with existing DTS files by requiring first five regions to be present at least. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> --- .../devicetree/bindings/dma/ti/k3-bcdma.yaml | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-)