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

[Rob Herring]

On Tue, May 8, 2018 at 10:40 AM, Suzuki K Poulose
<Suzuki.Poulose@xxxxxxx> wrote:
>
>
> 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 ?

Yeah, that's probably how we got into this problem with the port
numbering in the first place.


>>>>   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.

Better let Mathieu confirm he's okay with the first option because he
wasn't okay with changing the port reg when we discussed. But maybe
that was just on existing things like TPIU.

Rob