| Message ID | 20191101084135.14811-9-peter.ujfalusi@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dmaengine/soc: Add Texas Instruments UDMA support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 4 errors, 1 warnings, 190 lines checked" |
| robh/dt-meta-schema | fail | build log |
On Fri, Nov 01, 2019 at 10:41:28AM +0200, Peter Ujfalusi wrote: > New binding document for > Texas Instruments K3 NAVSS Unified DMA – Peripheral Root Complex (UDMA-P). > > UDMA-P is introduced as part of the K3 architecture and can be found in > AM654 and j721e. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > Rob, > > can you give me some hint on how to fix these two warnings from dt_binding_check: > > DTC Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml > Documentation/devicetree/bindings/dma/ti/k3-udma.example.dts:23.13-72: Warning (ranges_format): /example-0/interconnect@30800000:ranges: "ranges" property has invalid length (24 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 2) > CHECK Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml The default #address-cells is 1 for examples. So you need to either override it or change ranges parent address size. > > Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml: interconnect@30800000: $nodename:0: 'interconnect@30800000' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' Use 'bus' for the node name of 'simple-bus'. > > Thanks, > Peter > > .../devicetree/bindings/dma/ti/k3-udma.yaml | 190 ++++++++++++++++++ > 1 file changed, 190 insertions(+) > create mode 100644 Documentation/devicetree/bindings/dma/ti/k3-udma.yaml > > diff --git a/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml > new file mode 100644 > index 000000000000..e00fe3b2364e > --- /dev/null > +++ b/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml > @@ -0,0 +1,190 @@ > +# SPDX-License-Identifier: GPL-2.0 Dual license new bindings: # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/dma/ti/k3-udma.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments K3 NAVSS Unified DMA Device Tree Bindings > + > +maintainers: > + - Peter Ujfalusi <peter.ujfalusi@ti.com> > + > +description: | > + The UDMA-P is intended to perform similar (but significantly upgraded) > + functions as the packet-oriented DMA used on previous SoC devices. The UDMA-P > + module supports the transmission and reception of various packet types. > + The UDMA-P is architected to facilitate the segmentation and reassembly of > + SoC DMA data structure compliant packets to/from smaller data blocks that are > + natively compatible with the specific requirements of each connected > + peripheral. > + Multiple Tx and Rx channels are provided within the DMA which allow multiple > + segmentation or reassembly operations to be ongoing. The DMA controller > + maintains state information for each of the channels which allows packet > + segmentation and reassembly operations to be time division multiplexed between > + channels in order to share the underlying DMA hardware. An external DMA > + scheduler is used to control the ordering and rate at which this multiplexing > + occurs for Transmit operations. The ordering and rate of Receive operations > + is indirectly controlled by the order in which blocks are pushed into the DMA > + on the Rx PSI-L interface. > + > + The UDMA-P also supports acting as both a UTC and UDMA-C for its internal > + channels. Channels in the UDMA-P can be configured to be either Packet-Based > + or Third-Party channels on a channel by channel basis. > + > + All transfers within NAVSS is done between PSI-L source and destination > + threads. > + The peripherals serviced by UDMA can be PSI-L native (sa2ul, cpsw, etc) or > + legacy, non PSI-L native peripherals. In the later case a special, small PDMA > + is tasked to act as a bridge between the PSI-L fabric and the legacy > + peripheral. > + > + PDMAs can be configured via UDMAP peer registers to match with the > + configuration of the legacy peripheral. > + > +allOf: > + - $ref: "../dma-controller.yaml#" > + > +properties: > + "#dma-cells": > + const: 1 > + description: | > + The cell is the PSI-L thread ID of the remote (to UDMAP) end. > + Valid ranges for thread ID depends on the data movement direction: > + for source thread IDs (rx): 0 - 0x7fff > + for destination thread IDs (tx): 0x8000 - 0xffff > + > + PLease refer to the device documentation for the PSI-L thread map and also > + the PSI-L peripheral chapter for the correct thread ID. > + > + compatible: > + oneOf: > + - const: ti,am654-navss-main-udmap > + - const: ti,am654-navss-mcu-udmap > + - const: ti,j721e-navss-main-udmap > + - const: ti,j721e-navss-mcu-udmap enum works better than oneOf+const. Better error messages. > + > + reg: > + maxItems: 3 > + > + reg-names: > + items: > + - const: gcfg > + - const: rchanrt > + - const: tchanrt > + > + msi-parent: true > + > + ti,sci: > + description: | Doesn't need to be a literal block (can drop the '|'). > + phandle to TI-SCI compatible System controller node > + maxItems: 1 Drop this, not an array. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/phandle > + > + ti,sci-dev-id: > + description: | > + TI-SCI device id of UDMAP > + maxItems: 1 Drop this. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + > + ti,ringacc: > + description: | > + phandle to the ring accelerator node > + maxItems: 1 Drop this. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/phandle > + > + ti,sci-rm-range-tchan: > + description: | > + Array of UDMA tchan resource subtypes for resource allocation for this > + host > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32-array > + items: > + minItems: 1 > + # Should be enough > + maxItems: 255 These should not be under 'items'. Drop 'items'. Any constraints on the values of the array elements? > + > + ti,sci-rm-range-rchan: > + description: | > + Array of UDMA rchan resource subtypes for resource allocation for this > + host > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32-array > + items: > + minItems: 1 > + # Should be enough > + maxItems: 255 Same here. > + > + ti,sci-rm-range-rflow: > + description: | > + Array of UDMA rflow resource subtypes for resource allocation for this > + host > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32-array > + items: > + minItems: 1 > + # Should be enough > + maxItems: 255 And here. > + > +required: > + - compatible > + - "#dma-cells" > + - reg > + - reg-names > + - msi-parent > + - ti,sci > + - ti,sci-dev-id > + - ti,ringacc > + - ti,sci-rm-range-tchan > + - ti,sci-rm-range-rchan > + - ti,sci-rm-range-rflow > + > +examples: > + - |+ > + cbass_main_navss: interconnect@30800000 { > + compatible = "simple-bus"; > + #address-cells = <2>; > + #size-cells = <2>; > + dma-coherent; > + dma-ranges; > + ranges = <0x00 0x30800000 0x00 0x30800000 0x00 0x0bc00000>; > + > + ti,sci-dev-id = <118>; > + > + main_udmap: dma-controller@31150000 { > + compatible = "ti,am654-navss-main-udmap"; > + reg = <0x0 0x31150000 0x0 0x100>, > + <0x0 0x34000000 0x0 0x100000>, > + <0x0 0x35000000 0x0 0x100000>; > + reg-names = "gcfg", "rchanrt", "tchanrt"; > + #dma-cells = <1>; > + > + ti,ringacc = <&ringacc>; > + > + msi-parent = <&inta_main_udmass>; > + > + ti,sci = <&dmsc>; > + ti,sci-dev-id = <188>; > + > + ti,sci-rm-range-tchan = <0x1>, /* TX_HCHAN */ > + <0x2>; /* TX_CHAN */ > + ti,sci-rm-range-rchan = <0x4>, /* RX_HCHAN */ > + <0x5>; /* RX_CHAN */ > + ti,sci-rm-range-rflow = <0x6>; /* GP RFLOW */ > + }; > + }; > + > + mcasp0: mcasp@02B00000 { > + dmas = <&main_udmap 0xc400>, <&main_udmap 0x4400>; > + dma-names = "tx", "rx"; > + }; > + > + crypto: crypto@4E00000 { > + compatible = "ti,sa2ul-crypto"; > + > + dmas = <&main_udmap 0xc000>, <&main_udmap 0x4000>, <&main_udmap 0x4001>; > + dma-names = "tx", "rx1", "rx2"; > + }; > + > -- > Peter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
On 05/11/2019 4.19, Rob Herring wrote: > On Fri, Nov 01, 2019 at 10:41:28AM +0200, Peter Ujfalusi wrote: >> New binding document for >> Texas Instruments K3 NAVSS Unified DMA – Peripheral Root Complex (UDMA-P). >> >> UDMA-P is introduced as part of the K3 architecture and can be found in >> AM654 and j721e. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> --- >> Rob, >> >> can you give me some hint on how to fix these two warnings from dt_binding_check: >> >> DTC Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml >> Documentation/devicetree/bindings/dma/ti/k3-udma.example.dts:23.13-72: Warning (ranges_format): /example-0/interconnect@30800000:ranges: "ranges" property has invalid length (24 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 2) >> CHECK Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml > > The default #address-cells is 1 for examples. So you need to > either override it or change ranges parent address size. wrapping the cbass_main_navss inside: cbass_main { #address-cells = <2>; #size-cells = <2>; ... }; fixes it. >> >> Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml: interconnect@30800000: $nodename:0: 'interconnect@30800000' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > > Use 'bus' for the node name of 'simple-bus'. I took the navss node from the upstream dts (I'm going to fix it there as well). It has simple-bus for the navss, which is not quite right as NAVSS is not a bus, but a big subsystem with multiple components (UDMAP, ringacc, INTA, INTR, timers, etc). What about to change the binding doc to simple-mfd like this cbass_main_navss: navss@30800000 { compatible = "simple-mfd"; #address-cells = <2>; #size-cells = <2>; ... }; and fix up the DT when I got to the point when I can send the patches to enable DMA for am654 and j721e? >> >> .../devicetree/bindings/dma/ti/k3-udma.yaml | 190 ++++++++++++++++++ >> 1 file changed, 190 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/dma/ti/k3-udma.yaml >> >> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml >> new file mode 100644 >> index 000000000000..e00fe3b2364e >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml >> @@ -0,0 +1,190 @@ >> +# SPDX-License-Identifier: GPL-2.0 > > Dual license new bindings: > > # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) OK. >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/dma/ti/k3-udma.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Texas Instruments K3 NAVSS Unified DMA Device Tree Bindings >> + >> +maintainers: >> + - Peter Ujfalusi <peter.ujfalusi@ti.com> >> + >> +description: | >> + The UDMA-P is intended to perform similar (but significantly upgraded) >> + functions as the packet-oriented DMA used on previous SoC devices. The UDMA-P >> + module supports the transmission and reception of various packet types. >> + The UDMA-P is architected to facilitate the segmentation and reassembly of >> + SoC DMA data structure compliant packets to/from smaller data blocks that are >> + natively compatible with the specific requirements of each connected >> + peripheral. >> + Multiple Tx and Rx channels are provided within the DMA which allow multiple >> + segmentation or reassembly operations to be ongoing. The DMA controller >> + maintains state information for each of the channels which allows packet >> + segmentation and reassembly operations to be time division multiplexed between >> + channels in order to share the underlying DMA hardware. An external DMA >> + scheduler is used to control the ordering and rate at which this multiplexing >> + occurs for Transmit operations. The ordering and rate of Receive operations >> + is indirectly controlled by the order in which blocks are pushed into the DMA >> + on the Rx PSI-L interface. >> + >> + The UDMA-P also supports acting as both a UTC and UDMA-C for its internal >> + channels. Channels in the UDMA-P can be configured to be either Packet-Based >> + or Third-Party channels on a channel by channel basis. >> + >> + All transfers within NAVSS is done between PSI-L source and destination >> + threads. >> + The peripherals serviced by UDMA can be PSI-L native (sa2ul, cpsw, etc) or >> + legacy, non PSI-L native peripherals. In the later case a special, small PDMA >> + is tasked to act as a bridge between the PSI-L fabric and the legacy >> + peripheral. >> + >> + PDMAs can be configured via UDMAP peer registers to match with the >> + configuration of the legacy peripheral. >> + >> +allOf: >> + - $ref: "../dma-controller.yaml#" >> + >> +properties: >> + "#dma-cells": >> + const: 1 >> + description: | >> + The cell is the PSI-L thread ID of the remote (to UDMAP) end. >> + Valid ranges for thread ID depends on the data movement direction: >> + for source thread IDs (rx): 0 - 0x7fff >> + for destination thread IDs (tx): 0x8000 - 0xffff >> + >> + PLease refer to the device documentation for the PSI-L thread map and also >> + the PSI-L peripheral chapter for the correct thread ID. >> + >> + compatible: >> + oneOf: >> + - const: ti,am654-navss-main-udmap >> + - const: ti,am654-navss-mcu-udmap >> + - const: ti,j721e-navss-main-udmap >> + - const: ti,j721e-navss-mcu-udmap > > enum works better than oneOf+const. Better error messages. Like this: compatible: oneOf: - description: for AM654 items: - enum: - ti,am654-navss-main-udmap - ti,am654-navss-mcu-udmap - description: for J721E items: - enum: - ti,j721e-navss-main-udmap - ti,j721e-navss-mcu-udmap > >> + >> + reg: >> + maxItems: 3 >> + >> + reg-names: >> + items: >> + - const: gcfg >> + - const: rchanrt >> + - const: tchanrt >> + >> + msi-parent: true >> + >> + ti,sci: >> + description: | > > Doesn't need to be a literal block (can drop the '|'). OK > >> + phandle to TI-SCI compatible System controller node >> + maxItems: 1 > > Drop this, not an array. > >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/phandle >> + >> + ti,sci-dev-id: >> + description: | >> + TI-SCI device id of UDMAP >> + maxItems: 1 > > Drop this. > >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + >> + ti,ringacc: >> + description: | >> + phandle to the ring accelerator node >> + maxItems: 1 > > Drop this. > >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/phandle >> + >> + ti,sci-rm-range-tchan: >> + description: | >> + Array of UDMA tchan resource subtypes for resource allocation for this >> + host >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32-array >> + items: >> + minItems: 1 >> + # Should be enough >> + maxItems: 255 > > These should not be under 'items'. Drop 'items'. > > Any constraints on the values of the array elements? The subtype is usually smaller than 30 for the current K3 device line-up, but I would not set an upper limit, it all depends on system firmware for the given family member. I'll drop the items for the rm-ranges > >> + >> + ti,sci-rm-range-rchan: >> + description: | >> + Array of UDMA rchan resource subtypes for resource allocation for this >> + host >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32-array >> + items: >> + minItems: 1 >> + # Should be enough >> + maxItems: 255 > > Same here. > >> + >> + ti,sci-rm-range-rflow: >> + description: | >> + Array of UDMA rflow resource subtypes for resource allocation for this >> + host >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32-array >> + items: >> + minItems: 1 >> + # Should be enough >> + maxItems: 255 > > And here. > >> + >> +required: >> + - compatible >> + - "#dma-cells" >> + - reg >> + - reg-names >> + - msi-parent >> + - ti,sci >> + - ti,sci-dev-id >> + - ti,ringacc >> + - ti,sci-rm-range-tchan >> + - ti,sci-rm-range-rchan >> + - ti,sci-rm-range-rflow - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On Tue, Nov 5, 2019 at 4:07 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > > > > On 05/11/2019 4.19, Rob Herring wrote: > > On Fri, Nov 01, 2019 at 10:41:28AM +0200, Peter Ujfalusi wrote: > >> New binding document for > >> Texas Instruments K3 NAVSS Unified DMA – Peripheral Root Complex (UDMA-P). > >> > >> UDMA-P is introduced as part of the K3 architecture and can be found in > >> AM654 and j721e. > >> > >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > >> --- > >> Rob, > >> > >> can you give me some hint on how to fix these two warnings from dt_binding_check: > >> > >> DTC Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml > >> Documentation/devicetree/bindings/dma/ti/k3-udma.example.dts:23.13-72: Warning (ranges_format): /example-0/interconnect@30800000:ranges: "ranges" property has invalid length (24 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 2) > >> CHECK Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml > > > > The default #address-cells is 1 for examples. So you need to > > either override it or change ranges parent address size. > > wrapping the cbass_main_navss inside: > cbass_main { > #address-cells = <2>; > #size-cells = <2>; > ... > }; > > fixes it. > > >> > >> Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml: interconnect@30800000: $nodename:0: 'interconnect@30800000' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > > > > Use 'bus' for the node name of 'simple-bus'. > > I took the navss node from the upstream dts (I'm going to fix it there > as well). > It has simple-bus for the navss, which is not quite right as NAVSS is > not a bus, but a big subsystem with multiple components (UDMAP, ringacc, > INTA, INTR, timers, etc). > > What about to change the binding doc to simple-mfd like this That's really for things not memory-mapped (I'm sure you can probably find an example to contradict me), so better to keep simple-bus if all the child nodes have addresses. Do you need the node name to be 'navss' for some reason? If so, then better have a compatible string in there to identify it. If not, just use 'bus' and be done with it. > cbass_main_navss: navss@30800000 { > compatible = "simple-mfd"; > #address-cells = <2>; > #size-cells = <2>; > ... > }; > > and fix up the DT when I got to the point when I can send the patches to > enable DMA for am654 and j721e? There's no requirement yet for DTS files to not have warnings. > >> + compatible: > >> + oneOf: > >> + - const: ti,am654-navss-main-udmap > >> + - const: ti,am654-navss-mcu-udmap > >> + - const: ti,j721e-navss-main-udmap > >> + - const: ti,j721e-navss-mcu-udmap > > > > enum works better than oneOf+const. Better error messages. > > Like this: > compatible: > oneOf: > - description: for AM654 > items: > - enum: > - ti,am654-navss-main-udmap > - ti,am654-navss-mcu-udmap > > - description: for J721E > items: > - enum: > - ti,j721e-navss-main-udmap > - ti,j721e-navss-mcu-udmap If the 'description' was useful, but it's not. Just: compatible: enum: - ti,am654-navss-main-udmap - ti,am654-navss-mcu-udmap - ti,j721e-navss-main-udmap - ti,j721e-navss-mcu-udmap Rob
Rob, On 14/11/2019 19.53, Rob Herring wrote: > On Tue, Nov 5, 2019 at 4:07 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: >> >> >> >> On 05/11/2019 4.19, Rob Herring wrote: >>> On Fri, Nov 01, 2019 at 10:41:28AM +0200, Peter Ujfalusi wrote: >>>> New binding document for >>>> Texas Instruments K3 NAVSS Unified DMA – Peripheral Root Complex (UDMA-P). >>>> >>>> UDMA-P is introduced as part of the K3 architecture and can be found in >>>> AM654 and j721e. >>>> >>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >>>> --- >>>> Rob, >>>> >>>> can you give me some hint on how to fix these two warnings from dt_binding_check: >>>> >>>> DTC Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml >>>> Documentation/devicetree/bindings/dma/ti/k3-udma.example.dts:23.13-72: Warning (ranges_format): /example-0/interconnect@30800000:ranges: "ranges" property has invalid length (24 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 2) >>>> CHECK Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml >>> >>> The default #address-cells is 1 for examples. So you need to >>> either override it or change ranges parent address size. >> >> wrapping the cbass_main_navss inside: >> cbass_main { >> #address-cells = <2>; >> #size-cells = <2>; >> ... >> }; >> >> fixes it. >> >>>> >>>> Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml: interconnect@30800000: $nodename:0: 'interconnect@30800000' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' >>> >>> Use 'bus' for the node name of 'simple-bus'. >> >> I took the navss node from the upstream dts (I'm going to fix it there >> as well). >> It has simple-bus for the navss, which is not quite right as NAVSS is >> not a bus, but a big subsystem with multiple components (UDMAP, ringacc, >> INTA, INTR, timers, etc). >> >> What about to change the binding doc to simple-mfd like this > > That's really for things not memory-mapped (I'm sure you can probably > find an example to contradict me), so better to keep simple-bus if all > the child nodes have addresses. According to Documentation/devicetree/bindings/mfd/mfd.txt: - A range of memory registers containing "miscellaneous system registers" also known as a system controller "syscon" or any other memory range containing a mix of unrelated hardware devices. NAVSS (NAVigator SubSystem) falls in the later case, it contains unrelated blocks, like the UDMAP, ringacc, mailboxes, spinlocks, interrupt aggregator, interrupt router, etc. - compatible : "simple-mfd" - this signifies that the operating system should consider all subnodes of the MFD device as separate devices akin to how "simple-bus" indicates when to see subnodes as children for a simple memory-mapped bus. This is a bit confusing, but NAVSS is not really a bus, everything in it can be accessed by the CPU via memory mapped registers (some sub devices does not have registers defined, they are controlled via system firmware). > Do you need the node name to be 'navss' for some reason? If so, then > better have a compatible string in there to identify it. If not, just > use 'bus' and be done with it. We don't need unique compatible for the NAVSS itself as there is not much we can configure on the top level, it is 'just' a big subsystem with all sorts of things. I like to keep the 'navss' as node name as it gives human understandable representation of it in /sys for example, easier to see the topology. I just feel that the 'bus' does not really apply to what NAVSS is. Probably my view of simple-bus is not correct. >> cbass_main_navss: navss@30800000 { >> compatible = "simple-mfd"; >> #address-cells = <2>; >> #size-cells = <2>; >> ... >> }; >> >> and fix up the DT when I got to the point when I can send the patches to >> enable DMA for am654 and j721e? > > There's no requirement yet for DTS files to not have warnings. Sure, but it does not hurt if they are clean ;) >>>> + compatible: >>>> + oneOf: >>>> + - const: ti,am654-navss-main-udmap >>>> + - const: ti,am654-navss-mcu-udmap >>>> + - const: ti,j721e-navss-main-udmap >>>> + - const: ti,j721e-navss-mcu-udmap >>> >>> enum works better than oneOf+const. Better error messages. >> >> Like this: >> compatible: >> oneOf: >> - description: for AM654 >> items: >> - enum: >> - ti,am654-navss-main-udmap >> - ti,am654-navss-mcu-udmap >> >> - description: for J721E >> items: >> - enum: >> - ti,j721e-navss-main-udmap >> - ti,j721e-navss-mcu-udmap > > If the 'description' was useful, but it's not. Just: > > compatible: > enum: > - ti,am654-navss-main-udmap > - ti,am654-navss-mcu-udmap > - ti,j721e-navss-main-udmap > - ti,j721e-navss-mcu-udmap OK, can I keep your Reviewed-by you have given to v5 if I do this change for v6? > > > Rob > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Rob, On 15/11/2019 11.45, Peter Ujfalusi wrote: > Rob, > > On 14/11/2019 19.53, Rob Herring wrote: >> On Tue, Nov 5, 2019 at 4:07 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: >>> >>> >>> >>> On 05/11/2019 4.19, Rob Herring wrote: >>>> On Fri, Nov 01, 2019 at 10:41:28AM +0200, Peter Ujfalusi wrote: >>>>> New binding document for >>>>> Texas Instruments K3 NAVSS Unified DMA – Peripheral Root Complex (UDMA-P). >>>>> >>>>> UDMA-P is introduced as part of the K3 architecture and can be found in >>>>> AM654 and j721e. >>>>> >>>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >>>>> --- >>>>> Rob, >>>>> >>>>> can you give me some hint on how to fix these two warnings from dt_binding_check: >>>>> >>>>> DTC Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml >>>>> Documentation/devicetree/bindings/dma/ti/k3-udma.example.dts:23.13-72: Warning (ranges_format): /example-0/interconnect@30800000:ranges: "ranges" property has invalid length (24 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 2) >>>>> CHECK Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml >>>> >>>> The default #address-cells is 1 for examples. So you need to >>>> either override it or change ranges parent address size. >>> >>> wrapping the cbass_main_navss inside: >>> cbass_main { >>> #address-cells = <2>; >>> #size-cells = <2>; >>> ... >>> }; >>> >>> fixes it. >>> >>>>> >>>>> Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml: interconnect@30800000: $nodename:0: 'interconnect@30800000' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' >>>> >>>> Use 'bus' for the node name of 'simple-bus'. >>> >>> I took the navss node from the upstream dts (I'm going to fix it there >>> as well). >>> It has simple-bus for the navss, which is not quite right as NAVSS is >>> not a bus, but a big subsystem with multiple components (UDMAP, ringacc, >>> INTA, INTR, timers, etc). >>> >>> What about to change the binding doc to simple-mfd like this >> >> That's really for things not memory-mapped (I'm sure you can probably >> find an example to contradict me), so better to keep simple-bus if all >> the child nodes have addresses. > > According to Documentation/devicetree/bindings/mfd/mfd.txt: > - A range of memory registers containing "miscellaneous system > registers" also known as a system controller "syscon" or any other > memory range containing a mix of unrelated hardware devices. > > NAVSS (NAVigator SubSystem) falls in the later case, it contains > unrelated blocks, like the UDMAP, ringacc, mailboxes, spinlocks, > interrupt aggregator, interrupt router, etc. > > - compatible : "simple-mfd" - this signifies that the operating system > should consider all subnodes of the MFD device as separate devices > akin to how "simple-bus" indicates when to see subnodes as children > for a simple memory-mapped bus. > > This is a bit confusing, but NAVSS is not really a bus, everything in it > can be accessed by the CPU via memory mapped registers (some sub devices > does not have registers defined, they are controlled via system firmware). > >> Do you need the node name to be 'navss' for some reason? If so, then >> better have a compatible string in there to identify it. If not, just >> use 'bus' and be done with it. > > We don't need unique compatible for the NAVSS itself as there is not > much we can configure on the top level, it is 'just' a big subsystem > with all sorts of things. > > I like to keep the 'navss' as node name as it gives human understandable > representation of it in /sys for example, easier to see the topology. > > I just feel that the 'bus' does not really apply to what NAVSS is. > Probably my view of simple-bus is not correct. Can you advice on how to proceed? I would like to send v6 so Vinod can pick it for next after 5.5-rc1 is tagged. This is the only thing which I need to close on to be able to do that. > >>> cbass_main_navss: navss@30800000 { >>> compatible = "simple-mfd"; >>> #address-cells = <2>; >>> #size-cells = <2>; >>> ... >>> }; >>> >>> and fix up the DT when I got to the point when I can send the patches to >>> enable DMA for am654 and j721e? >> >> There's no requirement yet for DTS files to not have warnings. > > Sure, but it does not hurt if they are clean ;) > >>>>> + compatible: >>>>> + oneOf: >>>>> + - const: ti,am654-navss-main-udmap >>>>> + - const: ti,am654-navss-mcu-udmap >>>>> + - const: ti,j721e-navss-main-udmap >>>>> + - const: ti,j721e-navss-mcu-udmap >>>> >>>> enum works better than oneOf+const. Better error messages. >>> >>> Like this: >>> compatible: >>> oneOf: >>> - description: for AM654 >>> items: >>> - enum: >>> - ti,am654-navss-main-udmap >>> - ti,am654-navss-mcu-udmap >>> >>> - description: for J721E >>> items: >>> - enum: >>> - ti,j721e-navss-main-udmap >>> - ti,j721e-navss-mcu-udmap >> >> If the 'description' was useful, but it's not. Just: >> >> compatible: >> enum: >> - ti,am654-navss-main-udmap >> - ti,am654-navss-mcu-udmap >> - ti,j721e-navss-main-udmap >> - ti,j721e-navss-mcu-udmap > > OK, can I keep your Reviewed-by you have given to v5 if I do this change > for v6? > >> >> >> Rob >> > > - Péter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff --git a/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml new file mode 100644 index 000000000000..e00fe3b2364e --- /dev/null +++ b/Documentation/devicetree/bindings/dma/ti/k3-udma.yaml @@ -0,0 +1,190 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/dma/ti/k3-udma.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments K3 NAVSS Unified DMA Device Tree Bindings + +maintainers: + - Peter Ujfalusi <peter.ujfalusi@ti.com> + +description: | + The UDMA-P is intended to perform similar (but significantly upgraded) + functions as the packet-oriented DMA used on previous SoC devices. The UDMA-P + module supports the transmission and reception of various packet types. + The UDMA-P is architected to facilitate the segmentation and reassembly of + SoC DMA data structure compliant packets to/from smaller data blocks that are + natively compatible with the specific requirements of each connected + peripheral. + Multiple Tx and Rx channels are provided within the DMA which allow multiple + segmentation or reassembly operations to be ongoing. The DMA controller + maintains state information for each of the channels which allows packet + segmentation and reassembly operations to be time division multiplexed between + channels in order to share the underlying DMA hardware. An external DMA + scheduler is used to control the ordering and rate at which this multiplexing + occurs for Transmit operations. The ordering and rate of Receive operations + is indirectly controlled by the order in which blocks are pushed into the DMA + on the Rx PSI-L interface. + + The UDMA-P also supports acting as both a UTC and UDMA-C for its internal + channels. Channels in the UDMA-P can be configured to be either Packet-Based + or Third-Party channels on a channel by channel basis. + + All transfers within NAVSS is done between PSI-L source and destination + threads. + The peripherals serviced by UDMA can be PSI-L native (sa2ul, cpsw, etc) or + legacy, non PSI-L native peripherals. In the later case a special, small PDMA + is tasked to act as a bridge between the PSI-L fabric and the legacy + peripheral. + + PDMAs can be configured via UDMAP peer registers to match with the + configuration of the legacy peripheral. + +allOf: + - $ref: "../dma-controller.yaml#" + +properties: + "#dma-cells": + const: 1 + description: | + The cell is the PSI-L thread ID of the remote (to UDMAP) end. + Valid ranges for thread ID depends on the data movement direction: + for source thread IDs (rx): 0 - 0x7fff + for destination thread IDs (tx): 0x8000 - 0xffff + + PLease refer to the device documentation for the PSI-L thread map and also + the PSI-L peripheral chapter for the correct thread ID. + + compatible: + oneOf: + - const: ti,am654-navss-main-udmap + - const: ti,am654-navss-mcu-udmap + - const: ti,j721e-navss-main-udmap + - const: ti,j721e-navss-mcu-udmap + + reg: + maxItems: 3 + + reg-names: + items: + - const: gcfg + - const: rchanrt + - const: tchanrt + + msi-parent: true + + ti,sci: + description: | + phandle to TI-SCI compatible System controller node + maxItems: 1 + allOf: + - $ref: /schemas/types.yaml#/definitions/phandle + + ti,sci-dev-id: + description: | + TI-SCI device id of UDMAP + maxItems: 1 + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + + ti,ringacc: + description: | + phandle to the ring accelerator node + maxItems: 1 + allOf: + - $ref: /schemas/types.yaml#/definitions/phandle + + ti,sci-rm-range-tchan: + description: | + Array of UDMA tchan resource subtypes for resource allocation for this + host + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32-array + items: + minItems: 1 + # Should be enough + maxItems: 255 + + ti,sci-rm-range-rchan: + description: | + Array of UDMA rchan resource subtypes for resource allocation for this + host + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32-array + items: + minItems: 1 + # Should be enough + maxItems: 255 + + ti,sci-rm-range-rflow: + description: | + Array of UDMA rflow resource subtypes for resource allocation for this + host + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32-array + items: + minItems: 1 + # Should be enough + maxItems: 255 + +required: + - compatible + - "#dma-cells" + - reg + - reg-names + - msi-parent + - ti,sci + - ti,sci-dev-id + - ti,ringacc + - ti,sci-rm-range-tchan + - ti,sci-rm-range-rchan + - ti,sci-rm-range-rflow + +examples: + - |+ + cbass_main_navss: interconnect@30800000 { + compatible = "simple-bus"; + #address-cells = <2>; + #size-cells = <2>; + dma-coherent; + dma-ranges; + ranges = <0x00 0x30800000 0x00 0x30800000 0x00 0x0bc00000>; + + ti,sci-dev-id = <118>; + + main_udmap: dma-controller@31150000 { + compatible = "ti,am654-navss-main-udmap"; + reg = <0x0 0x31150000 0x0 0x100>, + <0x0 0x34000000 0x0 0x100000>, + <0x0 0x35000000 0x0 0x100000>; + reg-names = "gcfg", "rchanrt", "tchanrt"; + #dma-cells = <1>; + + ti,ringacc = <&ringacc>; + + msi-parent = <&inta_main_udmass>; + + ti,sci = <&dmsc>; + ti,sci-dev-id = <188>; + + ti,sci-rm-range-tchan = <0x1>, /* TX_HCHAN */ + <0x2>; /* TX_CHAN */ + ti,sci-rm-range-rchan = <0x4>, /* RX_HCHAN */ + <0x5>; /* RX_CHAN */ + ti,sci-rm-range-rflow = <0x6>; /* GP RFLOW */ + }; + }; + + mcasp0: mcasp@02B00000 { + dmas = <&main_udmap 0xc400>, <&main_udmap 0x4400>; + dma-names = "tx", "rx"; + }; + + crypto: crypto@4E00000 { + compatible = "ti,sa2ul-crypto"; + + dmas = <&main_udmap 0xc000>, <&main_udmap 0x4000>, <&main_udmap 0x4001>; + dma-names = "tx", "rx1", "rx2"; + }; +
New binding document for Texas Instruments K3 NAVSS Unified DMA – Peripheral Root Complex (UDMA-P). UDMA-P is introduced as part of the K3 architecture and can be found in AM654 and j721e. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- Rob, can you give me some hint on how to fix these two warnings from dt_binding_check: DTC Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml Documentation/devicetree/bindings/dma/ti/k3-udma.example.dts:23.13-72: Warning (ranges_format): /example-0/interconnect@30800000:ranges: "ranges" property has invalid length (24 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 2) CHECK Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml Documentation/devicetree/bindings/dma/ti/k3-udma.example.dt.yaml: interconnect@30800000: $nodename:0: 'interconnect@30800000' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' Thanks, Peter .../devicetree/bindings/dma/ti/k3-udma.yaml | 190 ++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/ti/k3-udma.yaml