linux-kernel - Re: [PATCH v4 08/15] dt-bindings: dma: ti: Add document for K3 UDMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqJbV7Zd40admW-x2SSveMqMkG0tM6RFTwjCJyYxX4Cxtw@mail.gmail.com>
Date:   Thu, 14 Nov 2019 11:53:41 -0600
From:   Rob Herring <robh@...nel.org>
To:     Peter Ujfalusi <peter.ujfalusi@...com>
Cc:     Vinod <vkoul@...nel.org>, Nishanth Menon <nm@...com>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        "open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM" 
        <dmaengine@...r.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>, devicetree@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Grygorii Strashko <grygorii.strashko@...com>,
        Lokesh Vutla <lokeshvutla@...com>,
        Tero Kristo <t-kristo@...com>,
        Tony Lindgren <tony@...mide.com>, Keerthy <j-keerthy@...com>
Subject: Re: [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@...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@...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@...00000: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@...00000: $nodename:0: 'interconnect@...00000' 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@...00000 {
>     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