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

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

[Suzuki K Poulose]

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@vger.kernel.org
Cc: frowand.list@gmail.com
Cc: Rob Herring <robh@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mathieu Poirier <mathieu.poirier@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../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";
 
 	* 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
 
 Example:
 
@@ -118,6 +123,35 @@ Example:
 		};
 	};
 
+	etr@20070000 {
+		compatible = "arm,coresight-tmc", "arm,primecell";
+		reg = <0 0x20070000 0 0x1000>;
+
+		clocks = <&oscclk6a>;
+		clock-names = "apb_pclk";
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			/* 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>;
+				etr_out_port: endpoint {
+					remote-endpoint = <&catu_in_port>;
+				};
+			};
+		};
+	};
+
 2. Links
 	replicator {
 		/* non-configurable replicators don't show up on the
@@ -247,5 +281,23 @@ Example:
 		};
 	};
 
+5. CATU
+
+	catu@207e0000 {
+		compatible = "arm,coresight-catu", "arm,primecell";
+		reg = <0 0x207e0000 0 0x1000>;
+
+		clocks = <&oscclk6a>;
+		clock-names = "apb_pclk";
+
+		interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+		port {
+			catu_in_port: endpoint {
+				slave-mode;
+				remote-endpoint = <&etr_out_port>;
+			};
+		};
+	};
+
 [1]. There is currently two version of STM: STM32 and STM500.  Both
 have the same HW interface and as such don't need an explicit binding name.
-- 
2.7.4

[PATCH v2 10/27] dts: bindings: Restrict coresight tmc-etr scatter-gather mode

[Suzuki K Poulose]

We are about to add the support for ETR builtin scatter-gather mode
for dealing with large amount of trace buffers. However, on some of
the platforms, using the ETR SG mode can lock up the system due to
the way the ETR is connected to the memory subsystem.

In SG mode, the ETR performs READ from the scatter-gather table to
fetch the next page and regular WRITE of trace data. If the READ
operation doesn't complete(due to the memory subsystem issues,
which we have seen on a couple of platforms) the trace WRITE
cannot proceed leading to issues. So, we by default do not
use the SG mode, unless it is known to be safe on the platform.
We define a DT property for the TMC node to specify whether we
have a proper SG mode.

Cc: Mathieu Poirier <matheiu.poirier@linaro.org>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: John Horley <john.horley@arm.com>
Cc: Robert Walker <robert.walker@arm.com>
Cc: devicetree@vger.kernel.org
Cc: frowand.list@gmail.com
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 3 +++
 drivers/hwtracing/coresight/coresight-tmc.c         | 8 +++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index cdd84d0..7c0c8f0 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -88,6 +88,9 @@ its hardware characteristcs.
 	* arm,buffer-size: size of contiguous buffer space for TMC ETR
 	 (embedded trace router)
 
+	* scatter-gather: boolean. Indicates that the TMC-ETR can safely
+	  use the SG mode on this system.
+
 * Optional property for CATU :
 	* interrupts : Exactly one SPI may be listed for reporting the address
 	  error
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index 7a4e84f..e38379c 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -20,6 +20,7 @@
 #include <linux/err.h>
 #include <linux/fs.h>
 #include <linux/miscdevice.h>
+#include <linux/property.h>
 #include <linux/uaccess.h>
 #include <linux/slab.h>
 #include <linux/dma-mapping.h>
@@ -304,6 +305,11 @@ const struct attribute_group *coresight_tmc_groups[] = {
 	NULL,
 };
 
+static inline bool tmc_etr_can_use_sg(struct tmc_drvdata *drvdata)
+{
+	return fwnode_property_present(drvdata->dev->fwnode, "scatter-gather");
+}
+
 /* Detect and initialise the capabilities of a TMC ETR */
 static int tmc_etr_setup_caps(struct tmc_drvdata *drvdata,
 			     u32 devid, void *dev_caps)
@@ -313,7 +319,7 @@ static int tmc_etr_setup_caps(struct tmc_drvdata *drvdata,
 	/* Set the unadvertised capabilities */
 	tmc_etr_init_caps(drvdata, (u32)(unsigned long)dev_caps);
 
-	if (!(devid & TMC_DEVID_NOSCAT))
+	if (!(devid & TMC_DEVID_NOSCAT) && tmc_etr_can_use_sg(drvdata))
 		tmc_etr_set_cap(drvdata, TMC_ETR_SG);
 
 	/* Check if the AXI address width is available */
-- 
2.7.4

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

[Rob Herring]

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@vger.kernel.org
> Cc: frowand.list@gmail.com
> Cc: Rob Herring <robh@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mathieu Poirier <mathieu.poirier@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  .../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               ^

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

>  
>  Example:
>  
> @@ -118,6 +123,35 @@ Example:
>  		};
>  	};
>  
> +	etr@20070000 {
> +		compatible = "arm,coresight-tmc", "arm,primecell";
> +		reg = <0 0x20070000 0 0x1000>;
> +
> +		clocks = <&oscclk6a>;
> +		clock-names = "apb_pclk";
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			/* 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.

Rob

Re: [PATCH v2 10/27] dts: bindings: Restrict coresight tmc-etr scatter-gather mode

[Rob Herring]

On Tue, May 01, 2018 at 10:10:40AM +0100, Suzuki K Poulose wrote:
> We are about to add the support for ETR builtin scatter-gather mode
> for dealing with large amount of trace buffers. However, on some of
> the platforms, using the ETR SG mode can lock up the system due to
> the way the ETR is connected to the memory subsystem.
> 
> In SG mode, the ETR performs READ from the scatter-gather table to
> fetch the next page and regular WRITE of trace data. If the READ
> operation doesn't complete(due to the memory subsystem issues,
> which we have seen on a couple of platforms) the trace WRITE
> cannot proceed leading to issues. So, we by default do not
> use the SG mode, unless it is known to be safe on the platform.
> We define a DT property for the TMC node to specify whether we
> have a proper SG mode.
> 
> Cc: Mathieu Poirier <matheiu.poirier@linaro.org>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: John Horley <john.horley@arm.com>
> Cc: Robert Walker <robert.walker@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: frowand.list@gmail.com
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  Documentation/devicetree/bindings/arm/coresight.txt | 3 +++
>  drivers/hwtracing/coresight/coresight-tmc.c         | 8 +++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index cdd84d0..7c0c8f0 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -88,6 +88,9 @@ its hardware characteristcs.
>  	* arm,buffer-size: size of contiguous buffer space for TMC ETR
>  	 (embedded trace router)
>  
> +	* scatter-gather: boolean. Indicates that the TMC-ETR can safely
> +	  use the SG mode on this system.
> +

Needs a vendor prefix.

>  * Optional property for CATU :
>  	* interrupts : Exactly one SPI may be listed for reporting the address
>  	  error

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

[Mathieu Poirier]

On 1 May 2018 at 07:10, Rob Herring <robh@kernel.org> 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@vger.kernel.org
>> Cc: frowand.list@gmail.com
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Mathieu Poirier <mathieu.poirier@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  .../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               ^
>
>>
>>       * 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.
>
>>
>>  Example:
>>
>> @@ -118,6 +123,35 @@ Example:
>>               };
>>       };
>>
>> +     etr@20070000 {
>> +             compatible = "arm,coresight-tmc", "arm,primecell";
>> +             reg = <0 0x20070000 0 0x1000>;
>> +
>> +             clocks = <&oscclk6a>;
>> +             clock-names = "apb_pclk";
>> +             ports {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +
>> +                     /* 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.

>
> Rob

Re: [PATCH v2 10/27] dts: bindings: Restrict coresight tmc-etr scatter-gather mode

[Mathieu Poirier]

On 1 May 2018 at 07:13, Rob Herring <robh@kernel.org> wrote:
> On Tue, May 01, 2018 at 10:10:40AM +0100, Suzuki K Poulose wrote:
>> We are about to add the support for ETR builtin scatter-gather mode
>> for dealing with large amount of trace buffers. However, on some of
>> the platforms, using the ETR SG mode can lock up the system due to
>> the way the ETR is connected to the memory subsystem.
>>
>> In SG mode, the ETR performs READ from the scatter-gather table to
>> fetch the next page and regular WRITE of trace data. If the READ
>> operation doesn't complete(due to the memory subsystem issues,
>> which we have seen on a couple of platforms) the trace WRITE
>> cannot proceed leading to issues. So, we by default do not
>> use the SG mode, unless it is known to be safe on the platform.
>> We define a DT property for the TMC node to specify whether we
>> have a proper SG mode.
>>
>> Cc: Mathieu Poirier <matheiu.poirier@linaro.org>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: John Horley <john.horley@arm.com>
>> Cc: Robert Walker <robert.walker@arm.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: frowand.list@gmail.com
>> Cc: Rob Herring <robh@kernel.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  Documentation/devicetree/bindings/arm/coresight.txt | 3 +++
>>  drivers/hwtracing/coresight/coresight-tmc.c         | 8 +++++++-
>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
>> index cdd84d0..7c0c8f0 100644
>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>> @@ -88,6 +88,9 @@ its hardware characteristcs.
>>       * arm,buffer-size: size of contiguous buffer space for TMC ETR
>>        (embedded trace router)
>>
>> +     * scatter-gather: boolean. Indicates that the TMC-ETR can safely
>> +       use the SG mode on this system.
>> +
>
> Needs a vendor prefix.
>

Thinking further on this, do we need to make it device specific as
well - something like "arm,etr-scatter-gather"?  That way we don't
have to redefine "scatter-gather" for other ARM devices if they happen
to need the same property but for different reasons.

>>  * Optional property for CATU :
>>       * interrupts : Exactly one SPI may be listed for reporting the address
>>         error

Re: [PATCH v2 10/27] dts: bindings: Restrict coresight tmc-etr scatter-gather mode

[Rob Herring]

On Thu, May 3, 2018 at 3:32 PM, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> On 1 May 2018 at 07:13, Rob Herring <robh@kernel.org> wrote:
>> On Tue, May 01, 2018 at 10:10:40AM +0100, Suzuki K Poulose wrote:
>>> We are about to add the support for ETR builtin scatter-gather mode
>>> for dealing with large amount of trace buffers. However, on some of
>>> the platforms, using the ETR SG mode can lock up the system due to
>>> the way the ETR is connected to the memory subsystem.
>>>
>>> In SG mode, the ETR performs READ from the scatter-gather table to
>>> fetch the next page and regular WRITE of trace data. If the READ
>>> operation doesn't complete(due to the memory subsystem issues,
>>> which we have seen on a couple of platforms) the trace WRITE
>>> cannot proceed leading to issues. So, we by default do not
>>> use the SG mode, unless it is known to be safe on the platform.
>>> We define a DT property for the TMC node to specify whether we
>>> have a proper SG mode.
>>>
>>> Cc: Mathieu Poirier <matheiu.poirier@linaro.org>
>>> Cc: Mike Leach <mike.leach@linaro.org>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: John Horley <john.horley@arm.com>
>>> Cc: Robert Walker <robert.walker@arm.com>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: frowand.list@gmail.com
>>> Cc: Rob Herring <robh@kernel.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  Documentation/devicetree/bindings/arm/coresight.txt | 3 +++
>>>  drivers/hwtracing/coresight/coresight-tmc.c         | 8 +++++++-
>>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
>>> index cdd84d0..7c0c8f0 100644
>>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>>> @@ -88,6 +88,9 @@ its hardware characteristcs.
>>>       * arm,buffer-size: size of contiguous buffer space for TMC ETR
>>>        (embedded trace router)
>>>
>>> +     * scatter-gather: boolean. Indicates that the TMC-ETR can safely
>>> +       use the SG mode on this system.
>>> +
>>
>> Needs a vendor prefix.
>>
>
> Thinking further on this, do we need to make it device specific as
> well - something like "arm,etr-scatter-gather"?  That way we don't
> have to redefine "scatter-gather" for other ARM devices if they happen
> to need the same property but for different reasons.

No. If we had a bunch of cases, then we'd probably want to have just
'scatter-gather'.

BTW, if SG had already been supported, then I'd say this is a quirk
and we should invert this property. Otherwise, you'd be disabling once
enabled SG and require working platforms to update their dtb. Of
course, I shouldn't really let the state of an OS driver influence the
DT binding.

Rob

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@kernel.org> 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@vger.kernel.org
>>> Cc: frowand.list@gmail.com
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Mathieu Poirier <mathieu.poirier@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>   .../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

Re: [PATCH v2 10/27] dts: bindings: Restrict coresight tmc-etr scatter-gather mode

[Suzuki K Poulose]

On 04/05/18 23:56, Rob Herring wrote:
> On Thu, May 3, 2018 at 3:32 PM, Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
>> On 1 May 2018 at 07:13, Rob Herring <robh@kernel.org> wrote:
>>> On Tue, May 01, 2018 at 10:10:40AM +0100, Suzuki K Poulose wrote:
>>>> We are about to add the support for ETR builtin scatter-gather mode
>>>> for dealing with large amount of trace buffers. However, on some of
>>>> the platforms, using the ETR SG mode can lock up the system due to
>>>> the way the ETR is connected to the memory subsystem.
>>>>
>>>> In SG mode, the ETR performs READ from the scatter-gather table to
>>>> fetch the next page and regular WRITE of trace data. If the READ
>>>> operation doesn't complete(due to the memory subsystem issues,
>>>> which we have seen on a couple of platforms) the trace WRITE
>>>> cannot proceed leading to issues. So, we by default do not
>>>> use the SG mode, unless it is known to be safe on the platform.
>>>> We define a DT property for the TMC node to specify whether we
>>>> have a proper SG mode.


>>>> ---
>>>>   Documentation/devicetree/bindings/arm/coresight.txt | 3 +++
>>>>   drivers/hwtracing/coresight/coresight-tmc.c         | 8 +++++++-
>>>>   2 files changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
>>>> index cdd84d0..7c0c8f0 100644
>>>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>>>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>>>> @@ -88,6 +88,9 @@ its hardware characteristcs.
>>>>        * arm,buffer-size: size of contiguous buffer space for TMC ETR
>>>>         (embedded trace router)
>>>>
>>>> +     * scatter-gather: boolean. Indicates that the TMC-ETR can safely
>>>> +       use the SG mode on this system.
>>>> +
>>>
>>> Needs a vendor prefix.
>>>
>>
>> Thinking further on this, do we need to make it device specific as
>> well - something like "arm,etr-scatter-gather"?  That way we don't
>> have to redefine "scatter-gather" for other ARM devices if they happen
>> to need the same property but for different reasons.
> 
> No. If we had a bunch of cases, then we'd probably want to have just
> 'scatter-gather'.

Does it mean "arm,scatter-gather" ? If we ever wanted to add the device
specific information, I would prefer to go with "arm,tmc-scatter-gather"
and not "etr-scatter-gather". They both could mean different things.

> 
> BTW, if SG had already been supported, then I'd say this is a quirk
> and we should invert this property. Otherwise, you'd be disabling once
> enabled SG and require working platforms to update their dtb. Of
> course, I shouldn't really let the state of an OS driver influence the
> DT binding.
> 

The SG support is added with this series. So, the OS has never made use
of the feature.

Suzuki

Re: [PATCH v2 10/27] dts: bindings: Restrict coresight tmc-etr scatter-gather mode

[Rob Herring]

On Tue, May 8, 2018 at 10:48 AM, Suzuki K Poulose
<Suzuki.Poulose@arm.com> wrote:
> On 04/05/18 23:56, Rob Herring wrote:
>>
>> On Thu, May 3, 2018 at 3:32 PM, Mathieu Poirier
>> <mathieu.poirier@linaro.org> wrote:
>>>
>>> On 1 May 2018 at 07:13, Rob Herring <robh@kernel.org> wrote:
>>>>
>>>> On Tue, May 01, 2018 at 10:10:40AM +0100, Suzuki K Poulose wrote:
>>>>>
>>>>> We are about to add the support for ETR builtin scatter-gather mode
>>>>> for dealing with large amount of trace buffers. However, on some of
>>>>> the platforms, using the ETR SG mode can lock up the system due to
>>>>> the way the ETR is connected to the memory subsystem.
>>>>>
>>>>> In SG mode, the ETR performs READ from the scatter-gather table to
>>>>> fetch the next page and regular WRITE of trace data. If the READ
>>>>> operation doesn't complete(due to the memory subsystem issues,
>>>>> which we have seen on a couple of platforms) the trace WRITE
>>>>> cannot proceed leading to issues. So, we by default do not
>>>>> use the SG mode, unless it is known to be safe on the platform.
>>>>> We define a DT property for the TMC node to specify whether we
>>>>> have a proper SG mode.
>
>
>
>>>>> ---
>>>>>   Documentation/devicetree/bindings/arm/coresight.txt | 3 +++
>>>>>   drivers/hwtracing/coresight/coresight-tmc.c         | 8 +++++++-
>>>>>   2 files changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt
>>>>> b/Documentation/devicetree/bindings/arm/coresight.txt
>>>>> index cdd84d0..7c0c8f0 100644
>>>>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>>>>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>>>>> @@ -88,6 +88,9 @@ its hardware characteristcs.
>>>>>        * arm,buffer-size: size of contiguous buffer space for TMC ETR
>>>>>         (embedded trace router)
>>>>>
>>>>> +     * scatter-gather: boolean. Indicates that the TMC-ETR can safely
>>>>> +       use the SG mode on this system.
>>>>> +
>>>>
>>>>
>>>> Needs a vendor prefix.
>>>>
>>>
>>> Thinking further on this, do we need to make it device specific as
>>> well - something like "arm,etr-scatter-gather"?  That way we don't
>>> have to redefine "scatter-gather" for other ARM devices if they happen
>>> to need the same property but for different reasons.
>>
>>
>> No. If we had a bunch of cases, then we'd probably want to have just
>> 'scatter-gather'.
>
>
> Does it mean "arm,scatter-gather" ?

Yes. Use that.

> If we ever wanted to add the device
> specific information, I would prefer to go with "arm,tmc-scatter-gather"
> and not "etr-scatter-gather". They both could mean different things.
>
>>
>> BTW, if SG had already been supported, then I'd say this is a quirk
>> and we should invert this property. Otherwise, you'd be disabling once
>> enabled SG and require working platforms to update their dtb. Of
>> course, I shouldn't really let the state of an OS driver influence the
>> DT binding.
>>
>
> The SG support is added with this series. So, the OS has never made use
> of the feature.

Linux never did, but other OSs use DT, hence why I said "an OS
driver", not "the OS driver". But in reality, I'd guess only Linux has
Coresight support at all.

Rob

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@arm.com> wrote:
>
>
> Rob, Mathieu,
>
>
> On 03/05/18 18:42, Mathieu Poirier wrote:
>>
>> On 1 May 2018 at 07:10, Rob Herring <robh@kernel.org> 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@vger.kernel.org
>>>> Cc: frowand.list@gmail.com
>>>> Cc: Rob Herring <robh@kernel.org>
>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>> Cc: Mathieu Poirier <mathieu.poirier@arm.com>
>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> ---
>>>>   .../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

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

[Mathieu Poirier]

On Fri, May 11, 2018 at 11:05:56AM -0500, Rob Herring wrote:
> On Tue, May 8, 2018 at 10:40 AM, Suzuki K Poulose
> <Suzuki.Poulose@arm.com> wrote:
> >
> >
> > Rob, Mathieu,
> >
> >
> > On 03/05/18 18:42, Mathieu Poirier wrote:
> >>
> >> On 1 May 2018 at 07:10, Rob Herring <robh@kernel.org> 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@vger.kernel.org
> >>>> Cc: frowand.list@gmail.com
> >>>> Cc: Rob Herring <robh@kernel.org>
> >>>> Cc: Mark Rutland <mark.rutland@arm.com>
> >>>> Cc: Mathieu Poirier <mathieu.poirier@arm.com>
> >>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >>>> ---
> >>>>   .../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.

I'm good with this one as it is a new component and doesn't change anything
that people could be relying on.

> 
> Rob