Re: [PATCH v2 05/27] dts: bindings: Document device tree binding for CATU

[Suzuki K Poulose]

Rob, Mathieu,

On 03/05/18 18:42, Mathieu Poirier wrote:
> On 1 May 2018 at 07:10, Rob Herring <robh@xxxxxxxxxx> wrote:
> > On Tue, May 01, 2018 at 10:10:35AM +0100, Suzuki K Poulose wrote:
> > > Document CATU device-tree bindings. CATU augments the TMC-ETR
> > > by providing an improved Scatter Gather mechanism for streaming
> > > trace data to non-contiguous system RAM pages.
> > >
> > > Cc: devicetree@xxxxxxxxxxxxxxx
> > > Cc: frowand.list@xxxxxxxxx
> > > Cc: Rob Herring <robh@xxxxxxxxxx>
> > > Cc: Mark Rutland <mark.rutland@xxxxxxx>
> > > Cc: Mathieu Poirier <mathieu.poirier@xxxxxxx>
> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> > > ---
> > >   .../devicetree/bindings/arm/coresight.txt          | 52 ++++++++++++++++++++++
> > >   1 file changed, 52 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> > > index 15ac8e8..cdd84d0 100644
> > > --- a/Documentation/devicetree/bindings/arm/coresight.txt
> > > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> > > @@ -39,6 +39,8 @@ its hardware characteristcs.
> > >
> > >                - System Trace Macrocell:
> > >                        "arm,coresight-stm", "arm,primecell"; [1]
> > > +             - Coresight Address Translation Unit (CATU)
> > > +                     "arm, coresight-catu", "arm,primecell";
> >
> > spurious space               ^

Thanks for spotting, will fix it.

> > >        * reg: physical base address and length of the register
> > >          set(s) of the component.
> > > @@ -86,6 +88,9 @@ its hardware characteristcs.
> > >        * arm,buffer-size: size of contiguous buffer space for TMC ETR
> > >         (embedded trace router)
> > >
> > > +* Optional property for CATU :
> > > +     * interrupts : Exactly one SPI may be listed for reporting the address
> > > +       error
> >
> > Somewhere you need to define the ports for the CATU.

The ports are defined common to all the coresight components. Would you
like it to be added just for the CATU ?

> > >   Example:
> > >
> > > @@ -118,6 +123,35 @@ Example:
> > >                };
> > >        };
> > >
> > > +     etr@20070000 {
> > > +             compatible = "arm,coresight-tmc", "arm,primecell";
> > > +             reg = <0 0x20070000 0 0x1000>;
> > > +
> > > +                     /* input port */
> > > +                     port@0 {
> > > +                             reg =  <0>;
> > > +                             etr_in_port: endpoint {
> > > +                                     slave-mode;
> > > +                                     remote-endpoint = <&replicator2_out_port0>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     /* CATU link represented by output port */
> > > +                     port@1 {
> > > +                             reg = <0>;
> >
> > While common in the Coresight bindings, having unit-address and reg not
> > match is an error. Mathieu and I discussed this a bit as dtc now warns
> > on these.
> >
> > Either reg should be 1 here, or 'ports' needs to be split into input and
> > output ports. My preference would be the former, but Mathieu objected to
> > this not reflecting the the h/w numbering.
>
> Suzuki, as we discuss this is related to your work on revamping CS
> bindings for ACPI.  Until that gets done and to move forward with this
> set I suggest you abide to Rob's request.

Ok, I can change it to <1>, as we don't expect any other output port for an
ETR.


Thanks
Suzuki