lkml.org 
[lkml]   [2019]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v4 08/15] dt-bindings: dma: ti: Add document for K3 UDMA
    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

    \
     
     \ /
      Last update: 2019-11-14 18:55    [W:3.643 / U:0.216 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site