[PATCH 00/12] coresight: Support for ARM Coresight SoC-600

[PATCH 00/12] coresight: Support for ARM Coresight SoC-600

[Suzuki K Poulose]

This series adds support for ARM Coresight SoC-600 IP, which implements
Coresight V3 architecture. It also does some clean up of the replicator
driver namings used in the driver to prevent confusions to the user.

The SoC-600 comes with an improved TMC which supports new features,
including Save-Restore and Software FIFO2 mode (for streaming the trace
data over functional I/O like USB/PCI).
This series only supports Save-Restore feature of the new ETR by reusing
the driver to perform additional setups required in case we are dealing
with an IP which supports it. Towards this, we add a capability
description to the IP listings to make it easier.

Patches 1-8 contains general cleanups, introduction of some helper
routines.
Patch 9 introduces the capability tracking for TMCs.
Patch 10 adds the support for Save-Restore feature in ETR.
Patch 11 adds support for the Coresight SoC 600 TMC
Patch 12 adds the support for other components in SoC 600.

Tested on Juno (with Coresight SoC 400) and an FPGA based system
for SoC 600.

Suzuki K Poulose (12):
  coresight replicator: Cleanup programmable replicator naming
  arm64: dts: juno: Use the new coresight replicator string
  coresight: Extend the PIDR mask to cover relevant bits in PIDR2
  coresight: Add support for reading 64bit registers
  coresight tmc: Add helpers for accessing 64bit registers
  coresight tmc: Expose DBA and AXICTL
  coresight replicator: Expose replicator management registers
  coresight tmc: Handle configuration types properly
  coresight tmc: Add capability information
  coresight tmc: Support for save-restore in ETR
  coresight tmc: Add support for Coresight SoC 600 TMC
  coresight: Add support for Coresight SoC 600 components

 .../devicetree/bindings/arm/coresight.txt          |  4 +-
 arch/arm64/boot/dts/arm/juno-base.dtsi             |  2 +-
 drivers/hwtracing/coresight/Kconfig                | 10 ++--
 drivers/hwtracing/coresight/Makefile               |  2 +-
 drivers/hwtracing/coresight/coresight-funnel.c     |  9 +++-
 drivers/hwtracing/coresight/coresight-priv.h       | 35 +++++++++++--
 .../coresight/coresight-replicator-qcom.c          | 33 ++++++++++--
 drivers/hwtracing/coresight/coresight-stm.c        |  8 +--
 drivers/hwtracing/coresight/coresight-tmc-etf.c    |  8 +--
 drivers/hwtracing/coresight/coresight-tmc-etr.c    | 21 ++++++--
 drivers/hwtracing/coresight/coresight-tmc.c        | 58 ++++++++++++++++++----
 drivers/hwtracing/coresight/coresight-tmc.h        | 53 ++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tpiu.c       |  9 +++-
 13 files changed, 210 insertions(+), 42 deletions(-)

-- 
2.7.4

[PATCH 01/12] coresight replicator: Cleanup programmable replicator naming

[Suzuki K Poulose]

The Linux coresight drivers define the programmable ATB replicator as
Qualcom replicator, while this is designed by ARM. This can cause confusion
to a user selecting the driver. Cleanup all references to make it
 explicitly clear. This patch :

 1) Adds a new compatible string for the same, retaining the old one for
    compatibility.
 2) Changes the Kconfig symbol (since this is not part of any defconfigs)
	 CORESIGHT_QCOM_REPLICATOR => CORESIGHT_DYNAMIC_REPLICATOR
 3) Improves the help message in the Kconfig.
 4) Changes the name of the driver :
	coresight-replicator-qcom => coresight-dynamic-replicator

Cc: Pratik Patel <pratikp@codeaurora.org>
Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 Documentation/devicetree/bindings/arm/coresight.txt     |  4 +++-
 drivers/hwtracing/coresight/Kconfig                     | 10 +++++-----
 drivers/hwtracing/coresight/Makefile                    |  2 +-
 drivers/hwtracing/coresight/coresight-replicator-qcom.c |  2 +-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index fcbae6a..f77329f 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -34,7 +34,9 @@ its hardware characteristcs.
 		- Embedded Trace Macrocell (version 4.x):
 			"arm,coresight-etm4x", "arm,primecell";
 
-		- Qualcomm Configurable Replicator (version 1.x):
+		- Coresight programmable Replicator (version 1.x):
+			"arm,coresight-dynamic-replicator", "arm,primecell";
+				OR
 			"qcom,coresight-replicator1x", "arm,primecell";
 
 		- System Trace Macrocell:
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index 8d55d6d..e50ea56 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -70,13 +70,13 @@ config CORESIGHT_SOURCE_ETM4X
 	  for instruction level tracing. Depending on the implemented version
 	  data tracing may also be available.
 
-config CORESIGHT_QCOM_REPLICATOR
-	bool "Qualcomm CoreSight Replicator driver"
+config CORESIGHT_DYNAMIC_REPLICATOR
+	bool "Programmable CoreSight Replicator driver"
 	depends on CORESIGHT_LINKS_AND_SINKS
 	help
-	  This enables support for Qualcomm CoreSight link driver. The
-	  programmable ATB replicator sends the ATB trace stream from the
-	  ETB/ETF to the TPIUi and ETR.
+	  This enables support for dynamic CoreSight replicator link driver.
+	  The programmable ATB replicator allows independent filtering of the
+	  trace data based on the traceid.
 
 config CORESIGHT_STM
 	bool "CoreSight System Trace Macrocell driver"
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 433d590..c7638d4 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -14,6 +14,6 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \
 					coresight-etm3x-sysfs.o
 obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
 					coresight-etm4x-sysfs.o
-obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
+obj-$(CONFIG_CORESIGHT_DYNAMIC_REPLICATOR) += coresight-replicator-qcom.o
 obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
 obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
diff --git a/drivers/hwtracing/coresight/coresight-replicator-qcom.c b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
index 0a3d15f..b7e44d1 100644
--- a/drivers/hwtracing/coresight/coresight-replicator-qcom.c
+++ b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
@@ -186,7 +186,7 @@ static struct amba_id replicator_ids[] = {
 
 static struct amba_driver replicator_driver = {
 	.drv = {
-		.name	= "coresight-replicator-qcom",
+		.name	= "coresight-dynamic-replicator",
 		.pm	= &replicator_dev_pm_ops,
 		.suppress_bind_attrs = true,
 	},
-- 
2.7.4

Re: [PATCH 01/12] coresight replicator: Cleanup programmable replicator naming

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:40PM +0100, Suzuki K Poulose wrote:
> The Linux coresight drivers define the programmable ATB replicator as
> Qualcom replicator, while this is designed by ARM. This can cause confusion
> to a user selecting the driver. Cleanup all references to make it
>  explicitly clear. This patch :
> 
>  1) Adds a new compatible string for the same, retaining the old one for
>     compatibility.
>  2) Changes the Kconfig symbol (since this is not part of any defconfigs)
> 	 CORESIGHT_QCOM_REPLICATOR => CORESIGHT_DYNAMIC_REPLICATOR
>  3) Improves the help message in the Kconfig.
>  4) Changes the name of the driver :
> 	coresight-replicator-qcom => coresight-dynamic-replicator
> 
> Cc: Pratik Patel <pratikp@codeaurora.org>
> Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Hi Suzuki,

> ---
>  Documentation/devicetree/bindings/arm/coresight.txt     |  4 +++-
>  drivers/hwtracing/coresight/Kconfig                     | 10 +++++-----
>  drivers/hwtracing/coresight/Makefile                    |  2 +-
>  drivers/hwtracing/coresight/coresight-replicator-qcom.c |  2 +-
>  4 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index fcbae6a..f77329f 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -34,7 +34,9 @@ its hardware characteristcs.
>  		- Embedded Trace Macrocell (version 4.x):
>  			"arm,coresight-etm4x", "arm,primecell";
>  
> -		- Qualcomm Configurable Replicator (version 1.x):
> +		- Coresight programmable Replicator (version 1.x):
> +			"arm,coresight-dynamic-replicator", "arm,primecell";
> +				OR
>  			"qcom,coresight-replicator1x", "arm,primecell";

Rob, what's your view on keeping the old binding around?  We could simply change
the two occurences we find in the DTs (Juno and 410c) to the new name and be
done with the old one.

>  
>  		- System Trace Macrocell:
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 8d55d6d..e50ea56 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -70,13 +70,13 @@ config CORESIGHT_SOURCE_ETM4X
>  	  for instruction level tracing. Depending on the implemented version
>  	  data tracing may also be available.
>  
> -config CORESIGHT_QCOM_REPLICATOR
> -	bool "Qualcomm CoreSight Replicator driver"
> +config CORESIGHT_DYNAMIC_REPLICATOR
> +	bool "Programmable CoreSight Replicator driver"
>  	depends on CORESIGHT_LINKS_AND_SINKS
>  	help
> -	  This enables support for Qualcomm CoreSight link driver. The
> -	  programmable ATB replicator sends the ATB trace stream from the
> -	  ETB/ETF to the TPIUi and ETR.
> +	  This enables support for dynamic CoreSight replicator link driver.
> +	  The programmable ATB replicator allows independent filtering of the
> +	  trace data based on the traceid.
>  
>  config CORESIGHT_STM
>  	bool "CoreSight System Trace Macrocell driver"
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 433d590..c7638d4 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -14,6 +14,6 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \
>  					coresight-etm3x-sysfs.o
>  obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
>  					coresight-etm4x-sysfs.o
> -obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
> +obj-$(CONFIG_CORESIGHT_DYNAMIC_REPLICATOR) += coresight-replicator-qcom.o
>  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
>  obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-replicator-qcom.c b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
> index 0a3d15f..b7e44d1 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator-qcom.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator-qcom.c

While at it, I would also change the name of the file.

Thanks,
Mathieu

> @@ -186,7 +186,7 @@ static struct amba_id replicator_ids[] = {
>  
>  static struct amba_driver replicator_driver = {
>  	.drv = {
> -		.name	= "coresight-replicator-qcom",
> +		.name	= "coresight-dynamic-replicator",
>  		.pm	= &replicator_dev_pm_ops,
>  		.suppress_bind_attrs = true,
>  	},
> -- 
> 2.7.4
> 

Re: [PATCH 01/12] coresight replicator: Cleanup programmable replicator naming

[Suzuki K Poulose]

On 13/06/17 17:55, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:40PM +0100, Suzuki K Poulose wrote:
>> The Linux coresight drivers define the programmable ATB replicator as
>> Qualcom replicator, while this is designed by ARM. This can cause confusion
>> to a user selecting the driver. Cleanup all references to make it
>>  explicitly clear. This patch :
>>
>>  1) Adds a new compatible string for the same, retaining the old one for
>>     compatibility.
>>  2) Changes the Kconfig symbol (since this is not part of any defconfigs)
>> 	 CORESIGHT_QCOM_REPLICATOR => CORESIGHT_DYNAMIC_REPLICATOR
>>  3) Improves the help message in the Kconfig.
>>  4) Changes the name of the driver :
>> 	coresight-replicator-qcom => coresight-dynamic-replicator
>>
>> Cc: Pratik Patel <pratikp@codeaurora.org>
>> Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: devicetree@vger.kernel.org
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>
> Hi Suzuki,
>
>> ---
>>  Documentation/devicetree/bindings/arm/coresight.txt     |  4 +++-
>>  drivers/hwtracing/coresight/Kconfig                     | 10 +++++-----
>>  drivers/hwtracing/coresight/Makefile                    |  2 +-
>>  drivers/hwtracing/coresight/coresight-replicator-qcom.c |  2 +-
>>  4 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
>> index fcbae6a..f77329f 100644
>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>> @@ -34,7 +34,9 @@ its hardware characteristcs.
>>  		- Embedded Trace Macrocell (version 4.x):
>>  			"arm,coresight-etm4x", "arm,primecell";
>>
>> -		- Qualcomm Configurable Replicator (version 1.x):
>> +		- Coresight programmable Replicator (version 1.x):
>> +			"arm,coresight-dynamic-replicator", "arm,primecell";
>> +				OR
>>  			"qcom,coresight-replicator1x", "arm,primecell";
>
> Rob, what's your view on keeping the old binding around?  We could simply change
> the two occurences we find in the DTs (Juno and 410c) to the new name and be
> done with the old one.

...

>> --- a/drivers/hwtracing/coresight/coresight-replicator-qcom.c
>> +++ b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
>
> While at it, I would also change the name of the file.

Sure, will do.

Thanks
Suzuki

Re: [PATCH 01/12] coresight replicator: Cleanup programmable replicator naming

[Rob Herring]

On Tue, Jun 13, 2017 at 10:55:28AM -0600, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:40PM +0100, Suzuki K Poulose wrote:
> > The Linux coresight drivers define the programmable ATB replicator as
> > Qualcom replicator, while this is designed by ARM. This can cause confusion
> > to a user selecting the driver. Cleanup all references to make it
> >  explicitly clear. This patch :
> > 
> >  1) Adds a new compatible string for the same, retaining the old one for
> >     compatibility.
> >  2) Changes the Kconfig symbol (since this is not part of any defconfigs)
> > 	 CORESIGHT_QCOM_REPLICATOR => CORESIGHT_DYNAMIC_REPLICATOR
> >  3) Improves the help message in the Kconfig.
> >  4) Changes the name of the driver :
> > 	coresight-replicator-qcom => coresight-dynamic-replicator
> > 
> > Cc: Pratik Patel <pratikp@codeaurora.org>
> > Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org>
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: devicetree@vger.kernel.org
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> Hi Suzuki,
> 
> > ---
> >  Documentation/devicetree/bindings/arm/coresight.txt     |  4 +++-
> >  drivers/hwtracing/coresight/Kconfig                     | 10 +++++-----
> >  drivers/hwtracing/coresight/Makefile                    |  2 +-
> >  drivers/hwtracing/coresight/coresight-replicator-qcom.c |  2 +-
> >  4 files changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> > index fcbae6a..f77329f 100644
> > --- a/Documentation/devicetree/bindings/arm/coresight.txt
> > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> > @@ -34,7 +34,9 @@ its hardware characteristcs.
> >  		- Embedded Trace Macrocell (version 4.x):
> >  			"arm,coresight-etm4x", "arm,primecell";
> >  
> > -		- Qualcomm Configurable Replicator (version 1.x):
> > +		- Coresight programmable Replicator (version 1.x):
> > +			"arm,coresight-dynamic-replicator", "arm,primecell";
> > +				OR
> >  			"qcom,coresight-replicator1x", "arm,primecell";
> 
> Rob, what's your view on keeping the old binding around?  We could simply change
> the two occurences we find in the DTs (Juno and 410c) to the new name and be
> done with the old one.

Juno uses the Qcom string? We should keep the old string. You can switch 
the dts files, but the driver should support the old name.

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 01/12] coresight replicator: Cleanup programmable replicator naming

[Mathieu Poirier]

On 18 June 2017 at 08:04, Rob Herring <robh@kernel.org> wrote:
> On Tue, Jun 13, 2017 at 10:55:28AM -0600, Mathieu Poirier wrote:
>> On Mon, Jun 12, 2017 at 03:36:40PM +0100, Suzuki K Poulose wrote:
>> > The Linux coresight drivers define the programmable ATB replicator as
>> > Qualcom replicator, while this is designed by ARM. This can cause confusion
>> > to a user selecting the driver. Cleanup all references to make it
>> >  explicitly clear. This patch :
>> >
>> >  1) Adds a new compatible string for the same, retaining the old one for
>> >     compatibility.
>> >  2) Changes the Kconfig symbol (since this is not part of any defconfigs)
>> >      CORESIGHT_QCOM_REPLICATOR => CORESIGHT_DYNAMIC_REPLICATOR
>> >  3) Improves the help message in the Kconfig.
>> >  4) Changes the name of the driver :
>> >     coresight-replicator-qcom => coresight-dynamic-replicator
>> >
>> > Cc: Pratik Patel <pratikp@codeaurora.org>
>> > Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org>
>> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> > Cc: devicetree@vger.kernel.org
>> > Cc: Rob Herring <robh+dt@kernel.org>
>> > Cc: Mark Rutland <mark.rutland@arm.com>
>> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>
>> Hi Suzuki,
>>
>> > ---
>> >  Documentation/devicetree/bindings/arm/coresight.txt     |  4 +++-
>> >  drivers/hwtracing/coresight/Kconfig                     | 10 +++++-----
>> >  drivers/hwtracing/coresight/Makefile                    |  2 +-
>> >  drivers/hwtracing/coresight/coresight-replicator-qcom.c |  2 +-
>> >  4 files changed, 10 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
>> > index fcbae6a..f77329f 100644
>> > --- a/Documentation/devicetree/bindings/arm/coresight.txt
>> > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>> > @@ -34,7 +34,9 @@ its hardware characteristcs.
>> >             - Embedded Trace Macrocell (version 4.x):
>> >                     "arm,coresight-etm4x", "arm,primecell";
>> >
>> > -           - Qualcomm Configurable Replicator (version 1.x):
>> > +           - Coresight programmable Replicator (version 1.x):
>> > +                   "arm,coresight-dynamic-replicator", "arm,primecell";
>> > +                           OR
>> >                     "qcom,coresight-replicator1x", "arm,primecell";
>>
>> Rob, what's your view on keeping the old binding around?  We could simply change
>> the two occurences we find in the DTs (Juno and 410c) to the new name and be
>> done with the old one.
>
> Juno uses the Qcom string? We should keep the old string. You can switch
> the dts files, but the driver should support the old name.

When we first started working on CoreSight programmable replicators
were available but the documentation wasn't public.  As such when I
saw Qualcomm's design I mistakenly thought it was a custom IP block
and came up with a compatible string that reflected that reality.
Fast forward 3 years the documentation is available and Juno has used
the same IP block in their design.  Suzuki's patch rectifies history
by changing the programmable replicator naming convention to what it
should have been from the start.

That being said, we can keep the old compatible string around but it
won't change anything.  CoreSight devices are discovered on the AMBA
bus and don't use the compatible string - drivers are probed based on
AMBA IDs laid out in the drivers and device IDs found in HW ID
registers.

In light of the above let me know what you want to do.

Thanks,
Mathieu

>
> Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 01/12] coresight replicator: Cleanup programmable replicator naming

[Rob Herring]

On Tue, Jun 20, 2017 at 11:44 AM, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> On 18 June 2017 at 08:04, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Jun 13, 2017 at 10:55:28AM -0600, Mathieu Poirier wrote:
>>> On Mon, Jun 12, 2017 at 03:36:40PM +0100, Suzuki K Poulose wrote:
>>> > The Linux coresight drivers define the programmable ATB replicator as
>>> > Qualcom replicator, while this is designed by ARM. This can cause confusion
>>> > to a user selecting the driver. Cleanup all references to make it
>>> >  explicitly clear. This patch :
>>> >
>>> >  1) Adds a new compatible string for the same, retaining the old one for
>>> >     compatibility.
>>> >  2) Changes the Kconfig symbol (since this is not part of any defconfigs)
>>> >      CORESIGHT_QCOM_REPLICATOR => CORESIGHT_DYNAMIC_REPLICATOR
>>> >  3) Improves the help message in the Kconfig.
>>> >  4) Changes the name of the driver :
>>> >     coresight-replicator-qcom => coresight-dynamic-replicator
>>> >
>>> > Cc: Pratik Patel <pratikp@codeaurora.org>
>>> > Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org>
>>> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> > Cc: devicetree@vger.kernel.org
>>> > Cc: Rob Herring <robh+dt@kernel.org>
>>> > Cc: Mark Rutland <mark.rutland@arm.com>
>>> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>
>>> Hi Suzuki,
>>>
>>> > ---
>>> >  Documentation/devicetree/bindings/arm/coresight.txt     |  4 +++-
>>> >  drivers/hwtracing/coresight/Kconfig                     | 10 +++++-----
>>> >  drivers/hwtracing/coresight/Makefile                    |  2 +-
>>> >  drivers/hwtracing/coresight/coresight-replicator-qcom.c |  2 +-
>>> >  4 files changed, 10 insertions(+), 8 deletions(-)
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
>>> > index fcbae6a..f77329f 100644
>>> > --- a/Documentation/devicetree/bindings/arm/coresight.txt
>>> > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>>> > @@ -34,7 +34,9 @@ its hardware characteristcs.
>>> >             - Embedded Trace Macrocell (version 4.x):
>>> >                     "arm,coresight-etm4x", "arm,primecell";
>>> >
>>> > -           - Qualcomm Configurable Replicator (version 1.x):
>>> > +           - Coresight programmable Replicator (version 1.x):
>>> > +                   "arm,coresight-dynamic-replicator", "arm,primecell";
>>> > +                           OR
>>> >                     "qcom,coresight-replicator1x", "arm,primecell";
>>>
>>> Rob, what's your view on keeping the old binding around?  We could simply change
>>> the two occurences we find in the DTs (Juno and 410c) to the new name and be
>>> done with the old one.
>>
>> Juno uses the Qcom string? We should keep the old string. You can switch
>> the dts files, but the driver should support the old name.
>
> When we first started working on CoreSight programmable replicators
> were available but the documentation wasn't public.  As such when I
> saw Qualcomm's design I mistakenly thought it was a custom IP block
> and came up with a compatible string that reflected that reality.
> Fast forward 3 years the documentation is available and Juno has used
> the same IP block in their design.  Suzuki's patch rectifies history
> by changing the programmable replicator naming convention to what it
> should have been from the start.
>
> That being said, we can keep the old compatible string around but it
> won't change anything.  CoreSight devices are discovered on the AMBA
> bus and don't use the compatible string - drivers are probed based on
> AMBA IDs laid out in the drivers and device IDs found in HW ID
> registers.
>
> In light of the above let me know what you want to do.

Well, if drivers don't use the string, then there is nothing to keep around.

Rob

[PATCH 02/12] arm64: dts: juno: Use the new coresight replicator string

[Suzuki K Poulose]

Use the new compatible for ATB programmable replicator in Juno.

Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index bfe7d68..c82c706 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -373,7 +373,7 @@
 	};
 
 	replicator@20120000 {
-		compatible = "qcom,coresight-replicator1x", "arm,primecell";
+		compatible = "arm,coresight-dynamic-replicator", "arm,primecell";
 		reg = <0 0x20120000 0 0x1000>;
 
 		clocks = <&soc_smc50mhz>;
-- 
2.7.4

[PATCH 03/12] coresight: Extend the PIDR mask to cover relevant bits in PIDR2

[Suzuki K Poulose]

As per coresight standards, PIDR2 register has the following format :

 [2-0]	- JEP106_bits6to4
 [3]	- JEDEC, designer ID is specified by JEDEC.

However some of the drivers only use mask of 0x3 for the PIDR2 leaving
bits [3-2] unchecked, which could potentially match the component for
a different device altogether. This patch fixes the mask and the
corresponding id bits for the existing devices.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
I have not touched the TPIU ids for Ux500 (see commit: 4339b699),
as I don't have a platform to fix/correct the ids.
---
 drivers/hwtracing/coresight/coresight-funnel.c          | 4 ++--
 drivers/hwtracing/coresight/coresight-replicator-qcom.c | 4 ++--
 drivers/hwtracing/coresight/coresight-stm.c             | 8 ++++----
 drivers/hwtracing/coresight/coresight-tmc.c             | 4 ++--
 drivers/hwtracing/coresight/coresight-tpiu.c            | 4 ++--
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index 860fe6e..6f7f3d3 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -248,8 +248,8 @@ static const struct dev_pm_ops funnel_dev_pm_ops = {
 
 static struct amba_id funnel_ids[] = {
 	{
-		.id     = 0x0003b908,
-		.mask   = 0x0003ffff,
+		.id     = 0x000bb908,
+		.mask   = 0x000fffff,
 	},
 	{ 0, 0},
 };
diff --git a/drivers/hwtracing/coresight/coresight-replicator-qcom.c b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
index b7e44d1..b029a5f 100644
--- a/drivers/hwtracing/coresight/coresight-replicator-qcom.c
+++ b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
@@ -177,8 +177,8 @@ static const struct dev_pm_ops replicator_dev_pm_ops = {
 
 static struct amba_id replicator_ids[] = {
 	{
-		.id     = 0x0003b909,
-		.mask   = 0x0003ffff,
+		.id     = 0x000bb909,
+		.mask   = 0x000bffff,
 		.data	= "REPLICATOR 1.0",
 	},
 	{ 0, 0 },
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index 93fc26f..1bcda80 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -916,13 +916,13 @@ static const struct dev_pm_ops stm_dev_pm_ops = {
 
 static struct amba_id stm_ids[] = {
 	{
-		.id     = 0x0003b962,
-		.mask   = 0x0003ffff,
+		.id     = 0x000bb962,
+		.mask   = 0x000fffff,
 		.data	= "STM32",
 	},
 	{
-		.id	= 0x0003b963,
-		.mask	= 0x0003ffff,
+		.id	= 0x000bb963,
+		.mask	= 0x000fffff,
 		.data	= "STM500",
 	},
 	{ 0, 0},
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index 8644887..eb0c7b3 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -393,8 +393,8 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
 
 static struct amba_id tmc_ids[] = {
 	{
-		.id     = 0x0003b961,
-		.mask   = 0x0003ffff,
+		.id     = 0x000bb961,
+		.mask   = 0x000fffff,
 	},
 	{ 0, 0},
 };
diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
index 0673baf..59c1510 100644
--- a/drivers/hwtracing/coresight/coresight-tpiu.c
+++ b/drivers/hwtracing/coresight/coresight-tpiu.c
@@ -194,8 +194,8 @@ static const struct dev_pm_ops tpiu_dev_pm_ops = {
 
 static struct amba_id tpiu_ids[] = {
 	{
-		.id	= 0x0003b912,
-		.mask	= 0x0003ffff,
+		.id	= 0x000bb912,
+		.mask	= 0x000fffff,
 	},
 	{
 		.id	= 0x0004b912,
-- 
2.7.4

Re: [PATCH 03/12] coresight: Extend the PIDR mask to cover relevant bits in PIDR2

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:42PM +0100, Suzuki K Poulose wrote:
> As per coresight standards, PIDR2 register has the following format :
> 
>  [2-0]	- JEP106_bits6to4
>  [3]	- JEDEC, designer ID is specified by JEDEC.
> 
> However some of the drivers only use mask of 0x3 for the PIDR2 leaving
> bits [3-2] unchecked, which could potentially match the component for
> a different device altogether. This patch fixes the mask and the
> corresponding id bits for the existing devices.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> I have not touched the TPIU ids for Ux500 (see commit: 4339b699),
> as I don't have a platform to fix/correct the ids.
> ---
>  drivers/hwtracing/coresight/coresight-funnel.c          | 4 ++--
>  drivers/hwtracing/coresight/coresight-replicator-qcom.c | 4 ++--
>  drivers/hwtracing/coresight/coresight-stm.c             | 8 ++++----
>  drivers/hwtracing/coresight/coresight-tmc.c             | 4 ++--
>  drivers/hwtracing/coresight/coresight-tpiu.c            | 4 ++--

Any reason for not adding ETMv3 to the list?  From what I see in the
documentation bit [2-0] need to 0b011 and the JEDEC bit is always 1.

>  5 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
> index 860fe6e..6f7f3d3 100644
> --- a/drivers/hwtracing/coresight/coresight-funnel.c
> +++ b/drivers/hwtracing/coresight/coresight-funnel.c
> @@ -248,8 +248,8 @@ static const struct dev_pm_ops funnel_dev_pm_ops = {
>  
>  static struct amba_id funnel_ids[] = {
>  	{
> -		.id     = 0x0003b908,
> -		.mask   = 0x0003ffff,
> +		.id     = 0x000bb908,
> +		.mask   = 0x000fffff,
>  	},
>  	{ 0, 0},
>  };
> diff --git a/drivers/hwtracing/coresight/coresight-replicator-qcom.c b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
> index b7e44d1..b029a5f 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator-qcom.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
> @@ -177,8 +177,8 @@ static const struct dev_pm_ops replicator_dev_pm_ops = {
>  
>  static struct amba_id replicator_ids[] = {
>  	{
> -		.id     = 0x0003b909,
> -		.mask   = 0x0003ffff,
> +		.id     = 0x000bb909,
> +		.mask   = 0x000bffff,
>  		.data	= "REPLICATOR 1.0",
>  	},
>  	{ 0, 0 },
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index 93fc26f..1bcda80 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -916,13 +916,13 @@ static const struct dev_pm_ops stm_dev_pm_ops = {
>  
>  static struct amba_id stm_ids[] = {
>  	{
> -		.id     = 0x0003b962,
> -		.mask   = 0x0003ffff,
> +		.id     = 0x000bb962,
> +		.mask   = 0x000fffff,
>  		.data	= "STM32",
>  	},
>  	{
> -		.id	= 0x0003b963,
> -		.mask	= 0x0003ffff,
> +		.id	= 0x000bb963,
> +		.mask	= 0x000fffff,
>  		.data	= "STM500",
>  	},
>  	{ 0, 0},
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index 8644887..eb0c7b3 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -393,8 +393,8 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>  
>  static struct amba_id tmc_ids[] = {
>  	{
> -		.id     = 0x0003b961,
> -		.mask   = 0x0003ffff,
> +		.id     = 0x000bb961,
> +		.mask   = 0x000fffff,
>  	},
>  	{ 0, 0},
>  };
> diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
> index 0673baf..59c1510 100644
> --- a/drivers/hwtracing/coresight/coresight-tpiu.c
> +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
> @@ -194,8 +194,8 @@ static const struct dev_pm_ops tpiu_dev_pm_ops = {
>  
>  static struct amba_id tpiu_ids[] = {
>  	{
> -		.id	= 0x0003b912,
> -		.mask	= 0x0003ffff,
> +		.id	= 0x000bb912,
> +		.mask	= 0x000fffff,
>  	},
>  	{
>  		.id	= 0x0004b912,
> -- 
> 2.7.4
> 

Re: [PATCH 03/12] coresight: Extend the PIDR mask to cover relevant bits in PIDR2

[Suzuki K Poulose]

On 13/06/17 18:53, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:42PM +0100, Suzuki K Poulose wrote:
>> As per coresight standards, PIDR2 register has the following format :
>>
>>  [2-0]	- JEP106_bits6to4
>>  [3]	- JEDEC, designer ID is specified by JEDEC.
>>
>> However some of the drivers only use mask of 0x3 for the PIDR2 leaving
>> bits [3-2] unchecked, which could potentially match the component for
>> a different device altogether. This patch fixes the mask and the
>> corresponding id bits for the existing devices.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> I have not touched the TPIU ids for Ux500 (see commit: 4339b699),
>> as I don't have a platform to fix/correct the ids.
>> ---
>>  drivers/hwtracing/coresight/coresight-funnel.c          | 4 ++--
>>  drivers/hwtracing/coresight/coresight-replicator-qcom.c | 4 ++--
>>  drivers/hwtracing/coresight/coresight-stm.c             | 8 ++++----
>>  drivers/hwtracing/coresight/coresight-tmc.c             | 4 ++--
>>  drivers/hwtracing/coresight/coresight-tpiu.c            | 4 ++--
>
> Any reason for not adding ETMv3 to the list?  From what I see in the
> documentation bit [2-0] need to 0b011 and the JEDEC bit is always 1.

I don't have a platform to test it easily. Hence the exclusion. Same for
etbv10.


Suzuki

Re: [PATCH 03/12] coresight: Extend the PIDR mask to cover relevant bits in PIDR2

[Mathieu Poirier]

On 13 June 2017 at 11:55, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 13/06/17 18:53, Mathieu Poirier wrote:
>>
>> On Mon, Jun 12, 2017 at 03:36:42PM +0100, Suzuki K Poulose wrote:
>>>
>>> As per coresight standards, PIDR2 register has the following format :
>>>
>>>  [2-0]  - JEP106_bits6to4
>>>  [3]    - JEDEC, designer ID is specified by JEDEC.
>>>
>>> However some of the drivers only use mask of 0x3 for the PIDR2 leaving
>>> bits [3-2] unchecked, which could potentially match the component for
>>> a different device altogether. This patch fixes the mask and the
>>> corresponding id bits for the existing devices.
>>>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>> I have not touched the TPIU ids for Ux500 (see commit: 4339b699),
>>> as I don't have a platform to fix/correct the ids.
>>> ---
>>>  drivers/hwtracing/coresight/coresight-funnel.c          | 4 ++--
>>>  drivers/hwtracing/coresight/coresight-replicator-qcom.c | 4 ++--
>>>  drivers/hwtracing/coresight/coresight-stm.c             | 8 ++++----
>>>  drivers/hwtracing/coresight/coresight-tmc.c             | 4 ++--
>>>  drivers/hwtracing/coresight/coresight-tpiu.c            | 4 ++--
>>
>>
>> Any reason for not adding ETMv3 to the list?  From what I see in the
>> documentation bit [2-0] need to 0b011 and the JEDEC bit is always 1.
>
>
> I don't have a platform to test it easily. Hence the exclusion. Same for
> etbv10.

I have a TC2 - add them in and I'll test it.

>
>
> Suzuki
>

[PATCH 04/12] coresight: Add support for reading 64bit registers

[Suzuki K Poulose]

Add support for reading a lower and upper 32bits of a register
as a single 64bit register.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-priv.h | 27 ++++++++++++++++++++++-----
 drivers/hwtracing/coresight/coresight-tmc.c  |  6 +++---
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 5f662d8..b62dc6a 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -39,23 +39,29 @@
 #define ETM_MODE_EXCL_USER	BIT(31)
 
 typedef u32 (*coresight_read_fn)(const struct device *, u32 offset);
-#define coresight_simple_func(type, func, name, offset)			\
+#define __coresight_simple_func(type, func, name, lo_off, hi_off)	\
 static ssize_t name##_show(struct device *_dev,				\
 			   struct device_attribute *attr, char *buf)	\
 {									\
 	type *drvdata = dev_get_drvdata(_dev->parent);			\
 	coresight_read_fn fn = func;					\
-	u32 val;							\
+	u64 val;							\
 	pm_runtime_get_sync(_dev->parent);				\
 	if (fn)								\
-		val = fn(_dev->parent, offset);				\
+		val = (u64)fn(_dev->parent, lo_off);			\
 	else								\
-		val = readl_relaxed(drvdata->base + offset);		\
+		val = coresight_read_reg_pair(drvdata->base,		\
+						 lo_off, hi_off);	\
 	pm_runtime_put_sync(_dev->parent);				\
-	return scnprintf(buf, PAGE_SIZE, "0x%x\n", val);		\
+	return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val);		\
 }									\
 static DEVICE_ATTR_RO(name)
 
+#define coresight_simple_func(type, func, name, offset)			\
+	__coresight_simple_func(type, func, name, offset, -1)
+#define coresight_simple_reg64(type, name, lo_off, hi_off)		\
+	__coresight_simple_func(type, NULL, name, lo_off, hi_off)
+
 enum etm_addr_type {
 	ETM_ADDR_TYPE_NONE,
 	ETM_ADDR_TYPE_SINGLE,
@@ -106,6 +112,17 @@ static inline void CS_UNLOCK(void __iomem *addr)
 	} while (0);
 }
 
+static inline u64
+coresight_read_reg_pair(void __iomem *addr, s32 lo_offset, s32 hi_offset)
+{
+	u64 val;
+
+	val = readl_relaxed(addr + lo_offset);
+	val |= (hi_offset < 0) ? 0 :
+		(u64)readl_relaxed(addr + hi_offset) << 32;
+	return val;
+}
+
 void coresight_disable_path(struct list_head *path);
 int coresight_enable_path(struct list_head *path, u32 mode);
 struct coresight_device *coresight_get_sink(struct list_head *path);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index eb0c7b3..7025982 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -219,11 +219,8 @@ static enum tmc_mem_intf_width tmc_get_memwidth(u32 devid)
 
 #define coresight_tmc_simple_func(name, offset)			\
 	coresight_simple_func(struct tmc_drvdata, NULL, name, offset)
-
 coresight_tmc_simple_func(rsz, TMC_RSZ);
 coresight_tmc_simple_func(sts, TMC_STS);
-coresight_tmc_simple_func(rrp, TMC_RRP);
-coresight_tmc_simple_func(rwp, TMC_RWP);
 coresight_tmc_simple_func(trg, TMC_TRG);
 coresight_tmc_simple_func(ctl, TMC_CTL);
 coresight_tmc_simple_func(ffsr, TMC_FFSR);
@@ -232,6 +229,9 @@ coresight_tmc_simple_func(mode, TMC_MODE);
 coresight_tmc_simple_func(pscr, TMC_PSCR);
 coresight_tmc_simple_func(devid, CORESIGHT_DEVID);
 
+coresight_simple_reg64(struct tmc_drvdata, rrp, TMC_RRP, TMC_RRPHI);
+coresight_simple_reg64(struct tmc_drvdata, rwp, TMC_RWP, TMC_RWPHI);
+
 static struct attribute *coresight_tmc_mgmt_attrs[] = {
 	&dev_attr_rsz.attr,
 	&dev_attr_sts.attr,
-- 
2.7.4

Re: [PATCH 04/12] coresight: Add support for reading 64bit registers

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:43PM +0100, Suzuki K Poulose wrote:
> Add support for reading a lower and upper 32bits of a register
> as a single 64bit register.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Just a few cosmetic details...

> ---
>  drivers/hwtracing/coresight/coresight-priv.h | 27 ++++++++++++++++++++++-----
>  drivers/hwtracing/coresight/coresight-tmc.c  |  6 +++---
>  2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 5f662d8..b62dc6a 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -39,23 +39,29 @@
>  #define ETM_MODE_EXCL_USER	BIT(31)
>  
>  typedef u32 (*coresight_read_fn)(const struct device *, u32 offset);
> -#define coresight_simple_func(type, func, name, offset)			\
> +#define __coresight_simple_func(type, func, name, lo_off, hi_off)	\
>  static ssize_t name##_show(struct device *_dev,				\
>  			   struct device_attribute *attr, char *buf)	\
>  {									\
>  	type *drvdata = dev_get_drvdata(_dev->parent);			\
>  	coresight_read_fn fn = func;					\
> -	u32 val;							\
> +	u64 val;							\
>  	pm_runtime_get_sync(_dev->parent);				\
>  	if (fn)								\
> -		val = fn(_dev->parent, offset);				\
> +		val = (u64)fn(_dev->parent, lo_off);			\
>  	else								\
> -		val = readl_relaxed(drvdata->base + offset);		\
> +		val = coresight_read_reg_pair(drvdata->base,		\
> +						 lo_off, hi_off);	\
>  	pm_runtime_put_sync(_dev->parent);				\
> -	return scnprintf(buf, PAGE_SIZE, "0x%x\n", val);		\
> +	return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val);		\
>  }									\
>  static DEVICE_ATTR_RO(name)
>  
> +#define coresight_simple_func(type, func, name, offset)			\
> +	__coresight_simple_func(type, func, name, offset, -1)
> +#define coresight_simple_reg64(type, name, lo_off, hi_off)		\
> +	__coresight_simple_func(type, NULL, name, lo_off, hi_off)
> +
>  enum etm_addr_type {
>  	ETM_ADDR_TYPE_NONE,
>  	ETM_ADDR_TYPE_SINGLE,
> @@ -106,6 +112,17 @@ static inline void CS_UNLOCK(void __iomem *addr)
>  	} while (0);
>  }
>  
> +static inline u64
> +coresight_read_reg_pair(void __iomem *addr, s32 lo_offset, s32 hi_offset)
> +{
> +	u64 val;
> +
> +	val = readl_relaxed(addr + lo_offset);
> +	val |= (hi_offset < 0) ? 0 :
> +		(u64)readl_relaxed(addr + hi_offset) << 32;

This should probably be aligned with the other '('.

> +	return val;
> +}
> +
>  void coresight_disable_path(struct list_head *path);
>  int coresight_enable_path(struct list_head *path, u32 mode);
>  struct coresight_device *coresight_get_sink(struct list_head *path);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index eb0c7b3..7025982 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -219,11 +219,8 @@ static enum tmc_mem_intf_width tmc_get_memwidth(u32 devid)
>  
>  #define coresight_tmc_simple_func(name, offset)			\
>  	coresight_simple_func(struct tmc_drvdata, NULL, name, offset)
> -

Please restore.

>  coresight_tmc_simple_func(rsz, TMC_RSZ);
>  coresight_tmc_simple_func(sts, TMC_STS);
> -coresight_tmc_simple_func(rrp, TMC_RRP);
> -coresight_tmc_simple_func(rwp, TMC_RWP);
>  coresight_tmc_simple_func(trg, TMC_TRG);
>  coresight_tmc_simple_func(ctl, TMC_CTL);
>  coresight_tmc_simple_func(ffsr, TMC_FFSR);
> @@ -232,6 +229,9 @@ coresight_tmc_simple_func(mode, TMC_MODE);
>  coresight_tmc_simple_func(pscr, TMC_PSCR);
>  coresight_tmc_simple_func(devid, CORESIGHT_DEVID);
>  

No need for a new line here.  In fact I would put the following two lines in
place of the ones you've replaced above.

> +coresight_simple_reg64(struct tmc_drvdata, rrp, TMC_RRP, TMC_RRPHI);
> +coresight_simple_reg64(struct tmc_drvdata, rwp, TMC_RWP, TMC_RWPHI);
> +
>  static struct attribute *coresight_tmc_mgmt_attrs[] = {
>  	&dev_attr_rsz.attr,
>  	&dev_attr_sts.attr,
> -- 
> 2.7.4
> 

Re: [PATCH 04/12] coresight: Add support for reading 64bit registers

[Suzuki K Poulose]

On 13/06/17 18:45, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:43PM +0100, Suzuki K Poulose wrote:
>> Add support for reading a lower and upper 32bits of a register
>> as a single 64bit register.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>
> Just a few cosmetic details...

Will fix all of them in the next version.

Thanks for the review.

Suzuki

[PATCH 05/12] coresight tmc: Add helpers for accessing 64bit registers

[Suzuki K Poulose]

Coresight TMC splits 64bit registers into a pair of 32bit registers
(e.g DBA, RRP, RWP). Provide helpers to read/write to these registers.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-priv.h    |  8 ++++++++
 drivers/hwtracing/coresight/coresight-tmc-etf.c |  8 ++++----
 drivers/hwtracing/coresight/coresight-tmc-etr.c |  8 ++++----
 drivers/hwtracing/coresight/coresight-tmc.h     | 19 +++++++++++++++++++
 4 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index b62dc6a..1a16964 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -123,6 +123,14 @@ coresight_read_reg_pair(void __iomem *addr, s32 lo_offset, s32 hi_offset)
 	return val;
 }
 
+static inline void coresight_write_reg_pair(void __iomem *addr, u64 val,
+						 s32 lo_offset, s32 hi_offset)
+{
+	writel_relaxed((u32)val, addr + lo_offset);
+	if (hi_offset >= 0)
+		writel_relaxed((u32)(val >> 32), addr + hi_offset);
+}
+
 void coresight_disable_path(struct list_head *path);
 int coresight_enable_path(struct list_head *path, u32 mode);
 struct coresight_device *coresight_get_sink(struct list_head *path);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index e3b9fb8..aecd712 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -371,7 +371,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 {
 	int i, cur;
 	u32 *buf_ptr;
-	u32 read_ptr, write_ptr;
+	u64 read_ptr, write_ptr;
 	u32 status, to_read;
 	unsigned long offset;
 	struct cs_buffers *buf = sink_config;
@@ -388,8 +388,8 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 
 	tmc_flush_and_stop(drvdata);
 
-	read_ptr = readl_relaxed(drvdata->base + TMC_RRP);
-	write_ptr = readl_relaxed(drvdata->base + TMC_RWP);
+	read_ptr = tmc_read_rrp(drvdata);
+	write_ptr = tmc_read_rwp(drvdata);
 
 	/*
 	 * Get a hold of the status register and see if a wrap around
@@ -441,7 +441,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 		if (read_ptr > (drvdata->size - 1))
 			read_ptr -= drvdata->size;
 		/* Tell the HW */
-		writel_relaxed(read_ptr, drvdata->base + TMC_RRP);
+		tmc_write_rrp(drvdata, read_ptr);
 		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 	}
 
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 5d31269..ff11b92 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -44,9 +44,8 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
 		  ~(TMC_AXICTL_PROT_CTL_B0 | TMC_AXICTL_PROT_CTL_B1)) |
 		  TMC_AXICTL_PROT_CTL_B1;
 	writel_relaxed(axictl, drvdata->base + TMC_AXICTL);
+	tmc_write_dba(drvdata, drvdata->paddr);
 
-	writel_relaxed(drvdata->paddr, drvdata->base + TMC_DBALO);
-	writel_relaxed(0x0, drvdata->base + TMC_DBAHI);
 	writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
 		       TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
 		       TMC_FFCR_TRIGON_TRIGIN,
@@ -59,9 +58,10 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
 
 static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata)
 {
-	u32 rwp, val;
+	u64 rwp;
+	u32 val;
 
-	rwp = readl_relaxed(drvdata->base + TMC_RWP);
+	rwp = tmc_read_rwp(drvdata);
 	val = readl_relaxed(drvdata->base + TMC_STS);
 
 	/*
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 51c0185..c78de00 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -18,6 +18,7 @@
 #ifndef _CORESIGHT_TMC_H
 #define _CORESIGHT_TMC_H
 
+#include <linux/io.h>
 #include <linux/miscdevice.h>
 
 #define TMC_RSZ			0x004
@@ -139,4 +140,22 @@ extern const struct coresight_ops tmc_etf_cs_ops;
 int tmc_read_prepare_etr(struct tmc_drvdata *drvdata);
 int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata);
 extern const struct coresight_ops tmc_etr_cs_ops;
+
+
+#define TMC_REG_PAIR(name, lo_off, hi_off)				\
+static inline u64							\
+tmc_read_##name(struct tmc_drvdata *drvdata)				\
+{									\
+	return coresight_read_reg_pair(drvdata->base, lo_off, hi_off);	\
+}									\
+static inline void							\
+tmc_write_##name(struct tmc_drvdata *drvdata, u64 val)			\
+{									\
+	coresight_write_reg_pair(drvdata->base, val, lo_off, hi_off);	\
+}
+
+TMC_REG_PAIR(rrp, TMC_RRP, TMC_RRPHI)
+TMC_REG_PAIR(rwp, TMC_RWP, TMC_RWPHI)
+TMC_REG_PAIR(dba, TMC_DBALO, TMC_DBAHI)
+
 #endif
-- 
2.7.4

Re: [PATCH 05/12] coresight tmc: Add helpers for accessing 64bit registers

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:44PM +0100, Suzuki K Poulose wrote:
> Coresight TMC splits 64bit registers into a pair of 32bit registers
> (e.g DBA, RRP, RWP). Provide helpers to read/write to these registers.

I'm good with this patch but please specify these changes are to support the
SoC-600 suite.  That way when we look back at this set in a couple of years we
don't loose hair thinking we've been carrying bugs all this time.

> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-priv.h    |  8 ++++++++
>  drivers/hwtracing/coresight/coresight-tmc-etf.c |  8 ++++----
>  drivers/hwtracing/coresight/coresight-tmc-etr.c |  8 ++++----
>  drivers/hwtracing/coresight/coresight-tmc.h     | 19 +++++++++++++++++++
>  4 files changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index b62dc6a..1a16964 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -123,6 +123,14 @@ coresight_read_reg_pair(void __iomem *addr, s32 lo_offset, s32 hi_offset)
>  	return val;
>  }
>  
> +static inline void coresight_write_reg_pair(void __iomem *addr, u64 val,
> +						 s32 lo_offset, s32 hi_offset)
> +{
> +	writel_relaxed((u32)val, addr + lo_offset);
> +	if (hi_offset >= 0)
> +		writel_relaxed((u32)(val >> 32), addr + hi_offset);
> +}
> +
>  void coresight_disable_path(struct list_head *path);
>  int coresight_enable_path(struct list_head *path, u32 mode);
>  struct coresight_device *coresight_get_sink(struct list_head *path);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index e3b9fb8..aecd712 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -371,7 +371,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>  {
>  	int i, cur;
>  	u32 *buf_ptr;
> -	u32 read_ptr, write_ptr;
> +	u64 read_ptr, write_ptr;
>  	u32 status, to_read;
>  	unsigned long offset;
>  	struct cs_buffers *buf = sink_config;
> @@ -388,8 +388,8 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>  
>  	tmc_flush_and_stop(drvdata);
>  
> -	read_ptr = readl_relaxed(drvdata->base + TMC_RRP);
> -	write_ptr = readl_relaxed(drvdata->base + TMC_RWP);
> +	read_ptr = tmc_read_rrp(drvdata);
> +	write_ptr = tmc_read_rwp(drvdata);
>  
>  	/*
>  	 * Get a hold of the status register and see if a wrap around
> @@ -441,7 +441,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>  		if (read_ptr > (drvdata->size - 1))
>  			read_ptr -= drvdata->size;
>  		/* Tell the HW */
> -		writel_relaxed(read_ptr, drvdata->base + TMC_RRP);
> +		tmc_write_rrp(drvdata, read_ptr);
>  		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>  	}
>  
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 5d31269..ff11b92 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -44,9 +44,8 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>  		  ~(TMC_AXICTL_PROT_CTL_B0 | TMC_AXICTL_PROT_CTL_B1)) |
>  		  TMC_AXICTL_PROT_CTL_B1;
>  	writel_relaxed(axictl, drvdata->base + TMC_AXICTL);
> +	tmc_write_dba(drvdata, drvdata->paddr);
>  
> -	writel_relaxed(drvdata->paddr, drvdata->base + TMC_DBALO);
> -	writel_relaxed(0x0, drvdata->base + TMC_DBAHI);
>  	writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
>  		       TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
>  		       TMC_FFCR_TRIGON_TRIGIN,
> @@ -59,9 +58,10 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>  
>  static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata)
>  {
> -	u32 rwp, val;
> +	u64 rwp;
> +	u32 val;
>  
> -	rwp = readl_relaxed(drvdata->base + TMC_RWP);
> +	rwp = tmc_read_rwp(drvdata);
>  	val = readl_relaxed(drvdata->base + TMC_STS);
>  
>  	/*
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 51c0185..c78de00 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -18,6 +18,7 @@
>  #ifndef _CORESIGHT_TMC_H
>  #define _CORESIGHT_TMC_H
>  
> +#include <linux/io.h>
>  #include <linux/miscdevice.h>
>  
>  #define TMC_RSZ			0x004
> @@ -139,4 +140,22 @@ extern const struct coresight_ops tmc_etf_cs_ops;
>  int tmc_read_prepare_etr(struct tmc_drvdata *drvdata);
>  int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata);
>  extern const struct coresight_ops tmc_etr_cs_ops;
> +
> +
> +#define TMC_REG_PAIR(name, lo_off, hi_off)				\
> +static inline u64							\
> +tmc_read_##name(struct tmc_drvdata *drvdata)				\
> +{									\
> +	return coresight_read_reg_pair(drvdata->base, lo_off, hi_off);	\
> +}									\
> +static inline void							\
> +tmc_write_##name(struct tmc_drvdata *drvdata, u64 val)			\
> +{									\
> +	coresight_write_reg_pair(drvdata->base, val, lo_off, hi_off);	\
> +}
> +
> +TMC_REG_PAIR(rrp, TMC_RRP, TMC_RRPHI)
> +TMC_REG_PAIR(rwp, TMC_RWP, TMC_RWPHI)
> +TMC_REG_PAIR(dba, TMC_DBALO, TMC_DBAHI)
> +
>  #endif
> -- 
> 2.7.4
> 

Re: [PATCH 05/12] coresight tmc: Add helpers for accessing 64bit registers

[Suzuki K Poulose]

On 14/06/17 18:49, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:44PM +0100, Suzuki K Poulose wrote:
>> Coresight TMC splits 64bit registers into a pair of 32bit registers
>> (e.g DBA, RRP, RWP). Provide helpers to read/write to these registers.
>
> I'm good with this patch but please specify these changes are to support the
> SoC-600 suite.  That way when we look back at this set in a couple of years we
> don't loose hair thinking we've been carrying bugs all this time.

To be honest, these are not necessarily just for the support of SoC-600.
It is applies to the current driver with SoC-400, as you could see below,
where we have always assumed that the RRP/RWP/DBA HI bits are always
0. Technically, the TMC supports upto 40bits and hence we have been doing
it wrong.

>> -	read_ptr = readl_relaxed(drvdata->base + TMC_RRP);
>> -	write_ptr = readl_relaxed(drvdata->base + TMC_RWP);
>> +	read_ptr = tmc_read_rrp(drvdata);
>> +	write_ptr = tmc_read_rwp(drvdata);



>> -		writel_relaxed(read_ptr, drvdata->base + TMC_RRP);
>> +		tmc_write_rrp(drvdata, read_ptr);
>>  		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);

>>  	writel_relaxed(axictl, drvdata->base + TMC_AXICTL);
>> +	tmc_write_dba(drvdata, drvdata->paddr);
>>
>> -	writel_relaxed(drvdata->paddr, drvdata->base + TMC_DBALO);
>> -	writel_relaxed(0x0, drvdata->base + TMC_DBAHI);

>>
>> -	rwp = readl_relaxed(drvdata->base + TMC_RWP);
>> +	rwp = tmc_read_rwp(drvdata);
>>  	val = readl_relaxed(drvdata->base + TMC_STS);
>>


Suzuki

Re: [PATCH 05/12] coresight tmc: Add helpers for accessing 64bit registers

[Mike Leach]

Part of the perf / ETR SG work I have been doing use the HI bits -
which makes these additions useful for that too.
I'd be aiming to use these as part of the re-spin
Mike

On 15 June 2017 at 11:13, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 14/06/17 18:49, Mathieu Poirier wrote:
>>
>> On Mon, Jun 12, 2017 at 03:36:44PM +0100, Suzuki K Poulose wrote:
>>>
>>> Coresight TMC splits 64bit registers into a pair of 32bit registers
>>> (e.g DBA, RRP, RWP). Provide helpers to read/write to these registers.
>>
>>
>> I'm good with this patch but please specify these changes are to support
>> the
>> SoC-600 suite.  That way when we look back at this set in a couple of
>> years we
>> don't loose hair thinking we've been carrying bugs all this time.
>
>
> To be honest, these are not necessarily just for the support of SoC-600.
> It is applies to the current driver with SoC-400, as you could see below,
> where we have always assumed that the RRP/RWP/DBA HI bits are always
> 0. Technically, the TMC supports upto 40bits and hence we have been doing
> it wrong.
>
>>> -       read_ptr = readl_relaxed(drvdata->base + TMC_RRP);
>>> -       write_ptr = readl_relaxed(drvdata->base + TMC_RWP);
>>> +       read_ptr = tmc_read_rrp(drvdata);
>>> +       write_ptr = tmc_read_rwp(drvdata);
>
>
>
>
>>> -               writel_relaxed(read_ptr, drvdata->base + TMC_RRP);
>>> +               tmc_write_rrp(drvdata, read_ptr);
>>>                 perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>
>
>>>         writel_relaxed(axictl, drvdata->base + TMC_AXICTL);
>>> +       tmc_write_dba(drvdata, drvdata->paddr);
>>>
>>> -       writel_relaxed(drvdata->paddr, drvdata->base + TMC_DBALO);
>>> -       writel_relaxed(0x0, drvdata->base + TMC_DBAHI);
>
>
>>>
>>> -       rwp = readl_relaxed(drvdata->base + TMC_RWP);
>>> +       rwp = tmc_read_rwp(drvdata);
>>>         val = readl_relaxed(drvdata->base + TMC_STS);
>>>
>
>
> Suzuki
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Blackburn Design Centre. UK

Re: [PATCH 05/12] coresight tmc: Add helpers for accessing 64bit registers

[Mathieu Poirier]

On 15 June 2017 at 04:13, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 14/06/17 18:49, Mathieu Poirier wrote:
>>
>> On Mon, Jun 12, 2017 at 03:36:44PM +0100, Suzuki K Poulose wrote:
>>>
>>> Coresight TMC splits 64bit registers into a pair of 32bit registers
>>> (e.g DBA, RRP, RWP). Provide helpers to read/write to these registers.
>>
>>
>> I'm good with this patch but please specify these changes are to support
>> the
>> SoC-600 suite.  That way when we look back at this set in a couple of
>> years we
>> don't loose hair thinking we've been carrying bugs all this time.
>
>
> To be honest, these are not necessarily just for the support of SoC-600.
> It is applies to the current driver with SoC-400, as you could see below,
> where we have always assumed that the RRP/RWP/DBA HI bits are always
> 0. Technically, the TMC supports upto 40bits and hence we have been doing
> it wrong.

I just had another look at the documentation and you are correct -
disregard my comment for this patch.

>
>>> -       read_ptr = readl_relaxed(drvdata->base + TMC_RRP);
>>> -       write_ptr = readl_relaxed(drvdata->base + TMC_RWP);
>>> +       read_ptr = tmc_read_rrp(drvdata);
>>> +       write_ptr = tmc_read_rwp(drvdata);
>
>
>
>
>>> -               writel_relaxed(read_ptr, drvdata->base + TMC_RRP);
>>> +               tmc_write_rrp(drvdata, read_ptr);
>>>                 perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>
>
>>>         writel_relaxed(axictl, drvdata->base + TMC_AXICTL);
>>> +       tmc_write_dba(drvdata, drvdata->paddr);
>>>
>>> -       writel_relaxed(drvdata->paddr, drvdata->base + TMC_DBALO);
>>> -       writel_relaxed(0x0, drvdata->base + TMC_DBAHI);
>
>
>>>
>>> -       rwp = readl_relaxed(drvdata->base + TMC_RWP);
>>> +       rwp = tmc_read_rwp(drvdata);
>>>         val = readl_relaxed(drvdata->base + TMC_STS);
>>>
>
>
> Suzuki

[PATCH 06/12] coresight tmc: Expose DBA and AXICTL

[Suzuki K Poulose]

Expose DBALO,DBAHI and AXICTL registers

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index 7025982..fd5a2e0 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -228,9 +228,11 @@ coresight_tmc_simple_func(ffcr, TMC_FFCR);
 coresight_tmc_simple_func(mode, TMC_MODE);
 coresight_tmc_simple_func(pscr, TMC_PSCR);
 coresight_tmc_simple_func(devid, CORESIGHT_DEVID);
+coresight_tmc_simple_func(axictl, TMC_AXICTL);
 
 coresight_simple_reg64(struct tmc_drvdata, rrp, TMC_RRP, TMC_RRPHI);
 coresight_simple_reg64(struct tmc_drvdata, rwp, TMC_RWP, TMC_RWPHI);
+coresight_simple_reg64(struct tmc_drvdata, dba, TMC_DBALO, TMC_DBAHI);
 
 static struct attribute *coresight_tmc_mgmt_attrs[] = {
 	&dev_attr_rsz.attr,
@@ -244,6 +246,8 @@ static struct attribute *coresight_tmc_mgmt_attrs[] = {
 	&dev_attr_mode.attr,
 	&dev_attr_pscr.attr,
 	&dev_attr_devid.attr,
+	&dev_attr_dba.attr,
+	&dev_attr_axictl.attr,
 	NULL,
 };
 
-- 
2.7.4

Re: [PATCH 06/12] coresight tmc: Expose DBA and AXICTL

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:45PM +0100, Suzuki K Poulose wrote:
> Expose DBALO,DBAHI and AXICTL registers

Why is this needed?  I fear we are exposing internal kernel information.

> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index 7025982..fd5a2e0 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -228,9 +228,11 @@ coresight_tmc_simple_func(ffcr, TMC_FFCR);
>  coresight_tmc_simple_func(mode, TMC_MODE);
>  coresight_tmc_simple_func(pscr, TMC_PSCR);
>  coresight_tmc_simple_func(devid, CORESIGHT_DEVID);
> +coresight_tmc_simple_func(axictl, TMC_AXICTL);
>  
>  coresight_simple_reg64(struct tmc_drvdata, rrp, TMC_RRP, TMC_RRPHI);
>  coresight_simple_reg64(struct tmc_drvdata, rwp, TMC_RWP, TMC_RWPHI);
> +coresight_simple_reg64(struct tmc_drvdata, dba, TMC_DBALO, TMC_DBAHI);
>  
>  static struct attribute *coresight_tmc_mgmt_attrs[] = {
>  	&dev_attr_rsz.attr,
> @@ -244,6 +246,8 @@ static struct attribute *coresight_tmc_mgmt_attrs[] = {
>  	&dev_attr_mode.attr,
>  	&dev_attr_pscr.attr,
>  	&dev_attr_devid.attr,
> +	&dev_attr_dba.attr,
> +	&dev_attr_axictl.attr,
>  	NULL,
>  };
>  
> -- 
> 2.7.4
> 

Re: [PATCH 06/12] coresight tmc: Expose DBA and AXICTL

[Suzuki K Poulose]

On 14/06/17 18:50, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:45PM +0100, Suzuki K Poulose wrote:
>> Expose DBALO,DBAHI and AXICTL registers
>
> Why is this needed?  I fear we are exposing internal kernel information.
>

Mathieu,

These are useful to analyse/debug the trace session. e.g, we could compare
RRP/RWP with DBA to see if the trace is actually generated or not.
As such, we already expose RRP/RWP. Exposing DBA doesn't expose
more information than we do already. Also, when we add SG support
this will be useful, as DBA will never match (or be in the range of) RRP/RWP.

The AXICTL may not be of any significance right now, but will be
useful once we add scatter gather support, to check which mode
are we operating in.

Let me know if you still have concerns.

Suzuki

>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  drivers/hwtracing/coresight/coresight-tmc.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
>> index 7025982..fd5a2e0 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
>> @@ -228,9 +228,11 @@ coresight_tmc_simple_func(ffcr, TMC_FFCR);
>>  coresight_tmc_simple_func(mode, TMC_MODE);
>>  coresight_tmc_simple_func(pscr, TMC_PSCR);
>>  coresight_tmc_simple_func(devid, CORESIGHT_DEVID);
>> +coresight_tmc_simple_func(axictl, TMC_AXICTL);
>>
>>  coresight_simple_reg64(struct tmc_drvdata, rrp, TMC_RRP, TMC_RRPHI);
>>  coresight_simple_reg64(struct tmc_drvdata, rwp, TMC_RWP, TMC_RWPHI);
>> +coresight_simple_reg64(struct tmc_drvdata, dba, TMC_DBALO, TMC_DBAHI);
>>
>>  static struct attribute *coresight_tmc_mgmt_attrs[] = {
>>  	&dev_attr_rsz.attr,
>> @@ -244,6 +246,8 @@ static struct attribute *coresight_tmc_mgmt_attrs[] = {
>>  	&dev_attr_mode.attr,
>>  	&dev_attr_pscr.attr,
>>  	&dev_attr_devid.attr,
>> +	&dev_attr_dba.attr,
>> +	&dev_attr_axictl.attr,
>>  	NULL,
>>  };
>>
>> --
>> 2.7.4
>>

[PATCH 07/12] coresight replicator: Expose replicator management registers

[Suzuki K Poulose]

Expose the idfilter* registers of the programmable replicator.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../hwtracing/coresight/coresight-replicator-qcom.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-replicator-qcom.c b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
index b029a5f..4dd18e0 100644
--- a/drivers/hwtracing/coresight/coresight-replicator-qcom.c
+++ b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
@@ -95,6 +95,26 @@ static const struct coresight_ops replicator_cs_ops = {
 	.link_ops	= &replicator_link_ops,
 };
 
+coresight_simple_func(struct replicator_state, NULL, idfilter0,
+				 REPLICATOR_IDFILTER0);
+coresight_simple_func(struct replicator_state, NULL, idfilter1,
+				REPLICATOR_IDFILTER1);
+static struct attribute *replicator_mgmt_attrs[] = {
+	&dev_attr_idfilter0.attr,
+	&dev_attr_idfilter1.attr,
+	NULL,
+};
+
+static const struct attribute_group replicator_mgmt_group = {
+	.attrs = replicator_mgmt_attrs,
+	.name = "mgmt",
+};
+
+static const struct attribute_group *replicator_groups[] = {
+	&replicator_mgmt_group,
+	NULL,
+};
+
 static int replicator_probe(struct amba_device *adev, const struct amba_id *id)
 {
 	int ret;
@@ -139,6 +159,7 @@ static int replicator_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.ops = &replicator_cs_ops;
 	desc.pdata = adev->dev.platform_data;
 	desc.dev = &adev->dev;
+	desc.groups = replicator_groups;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
-- 
2.7.4

Re: [PATCH 07/12] coresight replicator: Expose replicator management registers

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:46PM +0100, Suzuki K Poulose wrote:
> Expose the idfilter* registers of the programmable replicator.

Is this for SoC600 only?  If so we need to make sure these are not visible when
operating an SoC400 replicator.  Otherwise simply disregard my statement.

> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  .../hwtracing/coresight/coresight-replicator-qcom.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-replicator-qcom.c b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
> index b029a5f..4dd18e0 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator-qcom.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
> @@ -95,6 +95,26 @@ static const struct coresight_ops replicator_cs_ops = {
>  	.link_ops	= &replicator_link_ops,
>  };
>  
> +coresight_simple_func(struct replicator_state, NULL, idfilter0,
> +				 REPLICATOR_IDFILTER0);
> +coresight_simple_func(struct replicator_state, NULL, idfilter1,
> +				REPLICATOR_IDFILTER1);
> +static struct attribute *replicator_mgmt_attrs[] = {
> +	&dev_attr_idfilter0.attr,
> +	&dev_attr_idfilter1.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group replicator_mgmt_group = {
> +	.attrs = replicator_mgmt_attrs,
> +	.name = "mgmt",
> +};
> +
> +static const struct attribute_group *replicator_groups[] = {
> +	&replicator_mgmt_group,
> +	NULL,
> +};
> +
>  static int replicator_probe(struct amba_device *adev, const struct amba_id *id)
>  {
>  	int ret;
> @@ -139,6 +159,7 @@ static int replicator_probe(struct amba_device *adev, const struct amba_id *id)
>  	desc.ops = &replicator_cs_ops;
>  	desc.pdata = adev->dev.platform_data;
>  	desc.dev = &adev->dev;
> +	desc.groups = replicator_groups;
>  	drvdata->csdev = coresight_register(&desc);
>  	if (IS_ERR(drvdata->csdev))
>  		return PTR_ERR(drvdata->csdev);
> -- 
> 2.7.4
> 

Re: [PATCH 07/12] coresight replicator: Expose replicator management registers

[Suzuki K Poulose]

On 14/06/17 18:54, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:46PM +0100, Suzuki K Poulose wrote:
>> Expose the idfilter* registers of the programmable replicator.
>
> Is this for SoC600 only?  If so we need to make sure these are not visible when
> operating an SoC400 replicator.  Otherwise simply disregard my statement.

No, this is for the existing dynamic replicator, which is also compatible with the
one in SoC-600. In the future, we may be able to support multiple trace sessions
(i.e, distinct source-sink pairs using a shared dynamic replicator to filter the
data based on the trace-id). Right now we simple switch off the other side of the
replicator, when we enable the port connected to the sink.

I found these useful while debugging trace generation issue on one of the
platforms.

Suzuki

[PATCH 08/12] coresight tmc: Handle configuration types properly

[Suzuki K Poulose]

Coresight SoC 600 defines a new configuration for TMC, Embedded Trace
Streamer (ETS), indicated by 0x3 in MODE:CONFIG_TYPE. Make sure
the driver handles the new type properly.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index fd5a2e0..7152656 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -358,11 +358,13 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.dev = dev;
 	desc.groups = coresight_tmc_groups;
 
-	if (drvdata->config_type == TMC_CONFIG_TYPE_ETB) {
+	switch (drvdata->config_type) {
+	case TMC_CONFIG_TYPE_ETB:
 		desc.type = CORESIGHT_DEV_TYPE_SINK;
 		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
 		desc.ops = &tmc_etb_cs_ops;
-	} else if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) {
+		break;
+	case TMC_CONFIG_TYPE_ETR:
 		desc.type = CORESIGHT_DEV_TYPE_SINK;
 		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
 		desc.ops = &tmc_etr_cs_ops;
@@ -373,10 +375,16 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
 		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
 		if (ret)
 			goto out;
-	} else {
+		break;
+	case TMC_CONFIG_TYPE_ETF:
 		desc.type = CORESIGHT_DEV_TYPE_LINKSINK;
 		desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO;
 		desc.ops = &tmc_etf_cs_ops;
+		break;
+	default:
+		pr_err("%s: Unsupported TMC config\n", pdata->name);
+		ret = -EINVAL;
+		goto out;
 	}
 
 	drvdata->csdev = coresight_register(&desc);
-- 
2.7.4

Re: [PATCH 08/12] coresight tmc: Handle configuration types properly

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:47PM +0100, Suzuki K Poulose wrote:
> Coresight SoC 600 defines a new configuration for TMC, Embedded Trace
> Streamer (ETS), indicated by 0x3 in MODE:CONFIG_TYPE. Make sure
> the driver handles the new type properly.

>From what I see below this patch only improves readability by moving the if/else
compound to a case statement - there is no trace of the new ETS block.
Readability enhancement are good but the changelog must match the code.
 
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index fd5a2e0..7152656 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -358,11 +358,13 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>  	desc.dev = dev;
>  	desc.groups = coresight_tmc_groups;
>  
> -	if (drvdata->config_type == TMC_CONFIG_TYPE_ETB) {
> +	switch (drvdata->config_type) {
> +	case TMC_CONFIG_TYPE_ETB:
>  		desc.type = CORESIGHT_DEV_TYPE_SINK;
>  		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
>  		desc.ops = &tmc_etb_cs_ops;
> -	} else if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) {
> +		break;
> +	case TMC_CONFIG_TYPE_ETR:
>  		desc.type = CORESIGHT_DEV_TYPE_SINK;
>  		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
>  		desc.ops = &tmc_etr_cs_ops;
> @@ -373,10 +375,16 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>  		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
>  		if (ret)
>  			goto out;
> -	} else {
> +		break;
> +	case TMC_CONFIG_TYPE_ETF:
>  		desc.type = CORESIGHT_DEV_TYPE_LINKSINK;
>  		desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO;
>  		desc.ops = &tmc_etf_cs_ops;
> +		break;
> +	default:
> +		pr_err("%s: Unsupported TMC config\n", pdata->name);
> +		ret = -EINVAL;
> +		goto out;
>  	}
>  
>  	drvdata->csdev = coresight_register(&desc);
> -- 
> 2.7.4
> 

Re: [PATCH 08/12] coresight tmc: Handle configuration types properly

[Suzuki K Poulose]

On 14/06/17 18:59, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:47PM +0100, Suzuki K Poulose wrote:
>> Coresight SoC 600 defines a new configuration for TMC, Embedded Trace
>> Streamer (ETS), indicated by 0x3 in MODE:CONFIG_TYPE. Make sure
>> the driver handles the new type properly.
>
> From what I see below this patch only improves readability by moving the if/else
> compound to a case statement - there is no trace of the new ETS block.
> Readability enhancement are good but the changelog must match the code.

Mathieu,

Not really. It fixes an issue with the current code. We check for ETB and ETR,
if it doesn't match either of those, we assume it is ETF which is fine with SoC-400.
But with SoC-600, we could have an ETS with id 0x3 and the driver could treat it
as an ETF and go on with the initialisation. This patch makes sure that we
do check for the ETF and report that ETS is not a supported configuration.

Suzuki

>
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  drivers/hwtracing/coresight/coresight-tmc.c | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
>> index fd5a2e0..7152656 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
>> @@ -358,11 +358,13 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>>  	desc.dev = dev;
>>  	desc.groups = coresight_tmc_groups;
>>
>> -	if (drvdata->config_type == TMC_CONFIG_TYPE_ETB) {
>> +	switch (drvdata->config_type) {
>> +	case TMC_CONFIG_TYPE_ETB:
>>  		desc.type = CORESIGHT_DEV_TYPE_SINK;
>>  		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
>>  		desc.ops = &tmc_etb_cs_ops;
>> -	} else if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) {
>> +		break;
>> +	case TMC_CONFIG_TYPE_ETR:
>>  		desc.type = CORESIGHT_DEV_TYPE_SINK;
>>  		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
>>  		desc.ops = &tmc_etr_cs_ops;
>> @@ -373,10 +375,16 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>>  		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
>>  		if (ret)
>>  			goto out;
>> -	} else {
>> +		break;
>> +	case TMC_CONFIG_TYPE_ETF:
>>  		desc.type = CORESIGHT_DEV_TYPE_LINKSINK;
>>  		desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO;
>>  		desc.ops = &tmc_etf_cs_ops;
>> +		break;
>> +	default:
>> +		pr_err("%s: Unsupported TMC config\n", pdata->name);
>> +		ret = -EINVAL;
>> +		goto out;
>>  	}
>>
>>  	drvdata->csdev = coresight_register(&desc);
>> --
>> 2.7.4
>>

Re: [PATCH 08/12] coresight tmc: Handle configuration types properly

[Mathieu Poirier]

On 15 June 2017 at 04:25, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 14/06/17 18:59, Mathieu Poirier wrote:
>>
>> On Mon, Jun 12, 2017 at 03:36:47PM +0100, Suzuki K Poulose wrote:
>>>
>>> Coresight SoC 600 defines a new configuration for TMC, Embedded Trace
>>> Streamer (ETS), indicated by 0x3 in MODE:CONFIG_TYPE. Make sure
>>> the driver handles the new type properly.
>>
>>
>> From what I see below this patch only improves readability by moving the
>> if/else
>> compound to a case statement - there is no trace of the new ETS block.
>> Readability enhancement are good but the changelog must match the code.
>
>
> Mathieu,
>
> Not really. It fixes an issue with the current code. We check for ETB and
> ETR,
> if it doesn't match either of those, we assume it is ETF which is fine with
> SoC-400.
> But with SoC-600, we could have an ETS with id 0x3 and the driver could
> treat it
> as an ETF and go on with the initialisation. This patch makes sure that we
> do check for the ETF and report that ETS is not a supported configuration.

That is the description that should go in the changelog - it reflects
exactly what this patch is doing.

Mathieu.

>
> Suzuki
>
>>
>>>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  drivers/hwtracing/coresight/coresight-tmc.c | 14 +++++++++++---
>>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
>>> b/drivers/hwtracing/coresight/coresight-tmc.c
>>> index fd5a2e0..7152656 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tmc.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
>>> @@ -358,11 +358,13 @@ static int tmc_probe(struct amba_device *adev,
>>> const struct amba_id *id)
>>>         desc.dev = dev;
>>>         desc.groups = coresight_tmc_groups;
>>>
>>> -       if (drvdata->config_type == TMC_CONFIG_TYPE_ETB) {
>>> +       switch (drvdata->config_type) {
>>> +       case TMC_CONFIG_TYPE_ETB:
>>>                 desc.type = CORESIGHT_DEV_TYPE_SINK;
>>>                 desc.subtype.sink_subtype =
>>> CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
>>>                 desc.ops = &tmc_etb_cs_ops;
>>> -       } else if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) {
>>> +               break;
>>> +       case TMC_CONFIG_TYPE_ETR:
>>>                 desc.type = CORESIGHT_DEV_TYPE_SINK;
>>>                 desc.subtype.sink_subtype =
>>> CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
>>>                 desc.ops = &tmc_etr_cs_ops;
>>> @@ -373,10 +375,16 @@ static int tmc_probe(struct amba_device *adev,
>>> const struct amba_id *id)
>>>                 ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
>>>                 if (ret)
>>>                         goto out;
>>> -       } else {
>>> +               break;
>>> +       case TMC_CONFIG_TYPE_ETF:
>>>                 desc.type = CORESIGHT_DEV_TYPE_LINKSINK;
>>>                 desc.subtype.link_subtype =
>>> CORESIGHT_DEV_SUBTYPE_LINK_FIFO;
>>>                 desc.ops = &tmc_etf_cs_ops;
>>> +               break;
>>> +       default:
>>> +               pr_err("%s: Unsupported TMC config\n", pdata->name);
>>> +               ret = -EINVAL;
>>> +               goto out;
>>>         }
>>>
>>>         drvdata->csdev = coresight_register(&desc);
>>> --
>>> 2.7.4
>>>
>

[PATCH 09/12] coresight tmc: Add capability information

[Suzuki K Poulose]

This patch adds description of the capabilities of a given TMC.
This will help us to handle different versions of the TMC in the
same driver by checking the capabilities.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc.c | 10 +++++++++-
 drivers/hwtracing/coresight/coresight-tmc.h | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index 7152656..e88f2f3 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -399,16 +399,24 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
 	ret = misc_register(&drvdata->miscdev);
 	if (ret)
 		coresight_unregister(drvdata->csdev);
+	else if (id->data)
+		drvdata->caps = *(struct tmc_caps *)id->data;
 out:
 	return ret;
 }
 
+static struct tmc_caps coresight_soc_400_tmc_caps = {
+	.caps = CORESIGHT_SOC_400_TMC_CAPS,
+};
+
 static struct amba_id tmc_ids[] = {
 	{
+		/* Coresight SoC 400 TMC */
 		.id     = 0x000bb961,
 		.mask   = 0x000fffff,
+		.data	= &coresight_soc_400_tmc_caps,
 	},
-	{ 0, 0},
+	{},
 };
 
 static struct amba_driver tmc_driver = {
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index c78de00..87e4561 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -89,6 +89,18 @@ enum tmc_mem_intf_width {
 	TMC_MEM_INTF_WIDTH_256BITS	= 8,
 };
 
+#define TMC_CAP_ETR_SG_UNIT			(1U << 0)
+
+/**
+ * struct tmc_cap - Describes the capabilities of the TMC.
+ * @caps:	- Bitmask of the capacities
+ */
+struct tmc_caps {
+	u32	caps;
+};
+
+#define CORESIGHT_SOC_400_TMC_CAPS	(TMC_CAP_ETR_SG_UNIT)
+
 /**
  * struct tmc_drvdata - specifics associated to an TMC component
  * @base:	memory mapped base address for this component.
@@ -110,6 +122,7 @@ struct tmc_drvdata {
 	void __iomem		*base;
 	struct device		*dev;
 	struct coresight_device	*csdev;
+	struct tmc_caps		caps;
 	struct miscdevice	miscdev;
 	spinlock_t		spinlock;
 	bool			reading;
@@ -158,4 +171,9 @@ TMC_REG_PAIR(rrp, TMC_RRP, TMC_RRPHI)
 TMC_REG_PAIR(rwp, TMC_RWP, TMC_RWPHI)
 TMC_REG_PAIR(dba, TMC_DBALO, TMC_DBAHI)
 
+static inline bool tmc_has_cap(struct tmc_drvdata *drvdata, u32 cap)
+{
+	return !!(drvdata->caps.caps & cap);
+}
+
 #endif
-- 
2.7.4

Re: [PATCH 09/12] coresight tmc: Add capability information

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:48PM +0100, Suzuki K Poulose wrote:
> This patch adds description of the capabilities of a given TMC.
> This will help us to handle different versions of the TMC in the
> same driver by checking the capabilities.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc.c | 10 +++++++++-
>  drivers/hwtracing/coresight/coresight-tmc.h | 18 ++++++++++++++++++
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index 7152656..e88f2f3 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -399,16 +399,24 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>  	ret = misc_register(&drvdata->miscdev);
>  	if (ret)
>  		coresight_unregister(drvdata->csdev);
> +	else if (id->data)
> +		drvdata->caps = *(struct tmc_caps *)id->data;
>  out:
>  	return ret;
>  }
>  
> +static struct tmc_caps coresight_soc_400_tmc_caps = {
> +	.caps = CORESIGHT_SOC_400_TMC_CAPS,
> +};
> +
>  static struct amba_id tmc_ids[] = {
>  	{
> +		/* Coresight SoC 400 TMC */
>  		.id     = 0x000bb961,
>  		.mask   = 0x000fffff,
> +		.data	= &coresight_soc_400_tmc_caps,

Do we need this?  I don't see anywhere a check for TMC_CAP_ETR_SG_UNIT.  And
I also suppose that the SoC600 suite also supports scatter-gather - is there a
need to differenciate both that may not be implemented in this set?

I'm also wondering if capabilities for SoC600 could not be retrieved from HW
registers rather than hard coded?

>  	},
> -	{ 0, 0},
> +	{},
>  };
>  
>  static struct amba_driver tmc_driver = {
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index c78de00..87e4561 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -89,6 +89,18 @@ enum tmc_mem_intf_width {
>  	TMC_MEM_INTF_WIDTH_256BITS	= 8,
>  };
>  
> +#define TMC_CAP_ETR_SG_UNIT			(1U << 0)
> +
> +/**
> + * struct tmc_cap - Describes the capabilities of the TMC.
> + * @caps:	- Bitmask of the capacities
> + */
> +struct tmc_caps {
> +	u32	caps;
> +};
> +
> +#define CORESIGHT_SOC_400_TMC_CAPS	(TMC_CAP_ETR_SG_UNIT)
> +
>  /**
>   * struct tmc_drvdata - specifics associated to an TMC component
>   * @base:	memory mapped base address for this component.
> @@ -110,6 +122,7 @@ struct tmc_drvdata {
>  	void __iomem		*base;
>  	struct device		*dev;
>  	struct coresight_device	*csdev;
> +	struct tmc_caps		caps;

A simple u32 is probably best here rather than introducing a new structure.  If
capabilites can't be retrieved from HW and have to be declared statically, a
*u32 referencing ->data is sufficient rather than copying memory.

>  	struct miscdevice	miscdev;
>  	spinlock_t		spinlock;
>  	bool			reading;
> @@ -158,4 +171,9 @@ TMC_REG_PAIR(rrp, TMC_RRP, TMC_RRPHI)
>  TMC_REG_PAIR(rwp, TMC_RWP, TMC_RWPHI)
>  TMC_REG_PAIR(dba, TMC_DBALO, TMC_DBAHI)
>  
> +static inline bool tmc_has_cap(struct tmc_drvdata *drvdata, u32 cap)
> +{
> +	return !!(drvdata->caps.caps & cap);
> +}
> +
>  #endif
> -- 
> 2.7.4
> 

Re: [PATCH 09/12] coresight tmc: Add capability information

[Suzuki K Poulose]

On 14/06/17 19:22, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:48PM +0100, Suzuki K Poulose wrote:
>> This patch adds description of the capabilities of a given TMC.
>> This will help us to handle different versions of the TMC in the
>> same driver by checking the capabilities.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  drivers/hwtracing/coresight/coresight-tmc.c | 10 +++++++++-
>>  drivers/hwtracing/coresight/coresight-tmc.h | 18 ++++++++++++++++++
>>  2 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
>> index 7152656..e88f2f3 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
>> @@ -399,16 +399,24 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>>  	ret = misc_register(&drvdata->miscdev);
>>  	if (ret)
>>  		coresight_unregister(drvdata->csdev);
>> +	else if (id->data)
>> +		drvdata->caps = *(struct tmc_caps *)id->data;
>>  out:
>>  	return ret;
>>  }
>>
>> +static struct tmc_caps coresight_soc_400_tmc_caps = {
>> +	.caps = CORESIGHT_SOC_400_TMC_CAPS,
>> +};
>> +
>>  static struct amba_id tmc_ids[] = {
>>  	{
>> +		/* Coresight SoC 400 TMC */
>>  		.id     = 0x000bb961,
>>  		.mask   = 0x000fffff,
>> +		.data	= &coresight_soc_400_tmc_caps,
>
> Do we need this?  I don't see anywhere a check for TMC_CAP_ETR_SG_UNIT.  And
> I also suppose that the SoC600 suite also supports scatter-gather - is there a
> need to differenciate both that may not be implemented in this set?

Yes, the coresight SoC-600 doesn't come with an in built Scatter Gather unit.
Instead there is a dedicated component (Coresight Address Translation UNIT, CATU)
to do the Scatter Gather, which needs a driver. This is to make sure that if
somebody wants to use the SG, they should check it in the caps.


>
> I'm also wondering if capabilities for SoC600 could not be retrieved from HW
> registers rather than hard coded?

Unfortunately, no. There is no hardware description for the feature. So, we need
to depend on the PIDs to detect the features.

>> +#define TMC_CAP_ETR_SG_UNIT			(1U << 0)
>> +
>> +/**
>> + * struct tmc_cap - Describes the capabilities of the TMC.
>> + * @caps:	- Bitmask of the capacities
>> + */
>> +struct tmc_caps {
>> +	u32	caps;
>> +};
>> +
>> +#define CORESIGHT_SOC_400_TMC_CAPS	(TMC_CAP_ETR_SG_UNIT)
>> +
>>  /**
>>   * struct tmc_drvdata - specifics associated to an TMC component
>>   * @base:	memory mapped base address for this component.
>> @@ -110,6 +122,7 @@ struct tmc_drvdata {
>>  	void __iomem		*base;
>>  	struct device		*dev;
>>  	struct coresight_device	*csdev;
>> +	struct tmc_caps		caps;
>
> A simple u32 is probably best here rather than introducing a new structure.  If
> capabilites can't be retrieved from HW and have to be declared statically, a
> *u32 referencing ->data is sufficient rather than copying memory.

I think eventually the compiler may be able to do a register move to copy a 32bit
data. We could potentially add more fields there (e.g, whether a CATU is connected
to the device or not etc). Hence the abstraction.


Suzuki

Re: [PATCH 09/12] coresight tmc: Add capability information

[Mathieu Poirier]

On 15 June 2017 at 04:30, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 14/06/17 19:22, Mathieu Poirier wrote:
>>
>> On Mon, Jun 12, 2017 at 03:36:48PM +0100, Suzuki K Poulose wrote:
>>>
>>> This patch adds description of the capabilities of a given TMC.
>>> This will help us to handle different versions of the TMC in the
>>> same driver by checking the capabilities.
>>>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  drivers/hwtracing/coresight/coresight-tmc.c | 10 +++++++++-
>>>  drivers/hwtracing/coresight/coresight-tmc.h | 18 ++++++++++++++++++
>>>  2 files changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
>>> b/drivers/hwtracing/coresight/coresight-tmc.c
>>> index 7152656..e88f2f3 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tmc.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
>>> @@ -399,16 +399,24 @@ static int tmc_probe(struct amba_device *adev,
>>> const struct amba_id *id)
>>>         ret = misc_register(&drvdata->miscdev);
>>>         if (ret)
>>>                 coresight_unregister(drvdata->csdev);
>>> +       else if (id->data)
>>> +               drvdata->caps = *(struct tmc_caps *)id->data;
>>>  out:
>>>         return ret;
>>>  }
>>>
>>> +static struct tmc_caps coresight_soc_400_tmc_caps = {
>>> +       .caps = CORESIGHT_SOC_400_TMC_CAPS,
>>> +};
>>> +
>>>  static struct amba_id tmc_ids[] = {
>>>         {
>>> +               /* Coresight SoC 400 TMC */
>>>                 .id     = 0x000bb961,
>>>                 .mask   = 0x000fffff,
>>> +               .data   = &coresight_soc_400_tmc_caps,
>>
>>
>> Do we need this?  I don't see anywhere a check for TMC_CAP_ETR_SG_UNIT.
>> And
>> I also suppose that the SoC600 suite also supports scatter-gather - is
>> there a
>> need to differenciate both that may not be implemented in this set?
>
>
> Yes, the coresight SoC-600 doesn't come with an in built Scatter Gather
> unit.
> Instead there is a dedicated component (Coresight Address Translation UNIT,
> CATU)
> to do the Scatter Gather, which needs a driver. This is to make sure that if
> somebody wants to use the SG, they should check it in the caps.
>
>
>>
>> I'm also wondering if capabilities for SoC600 could not be retrieved from
>> HW
>> registers rather than hard coded?
>
>
> Unfortunately, no. There is no hardware description for the feature. So, we
> need
> to depend on the PIDs to detect the features.

I suspected that much - thanks for the clarification.

>
>>> +#define TMC_CAP_ETR_SG_UNIT                    (1U << 0)
>>> +
>>> +/**
>>> + * struct tmc_cap - Describes the capabilities of the TMC.
>>> + * @caps:      - Bitmask of the capacities
>>> + */
>>> +struct tmc_caps {
>>> +       u32     caps;
>>> +};
>>> +
>>> +#define CORESIGHT_SOC_400_TMC_CAPS     (TMC_CAP_ETR_SG_UNIT)
>>> +
>>>  /**
>>>   * struct tmc_drvdata - specifics associated to an TMC component
>>>   * @base:      memory mapped base address for this component.
>>> @@ -110,6 +122,7 @@ struct tmc_drvdata {
>>>         void __iomem            *base;
>>>         struct device           *dev;
>>>         struct coresight_device *csdev;
>>> +       struct tmc_caps         caps;
>>
>>
>> A simple u32 is probably best here rather than introducing a new
>> structure.  If
>> capabilites can't be retrieved from HW and have to be declared statically,
>> a
>> *u32 referencing ->data is sufficient rather than copying memory.
>
>
> I think eventually the compiler may be able to do a register move to copy a
> 32bit
> data. We could potentially add more fields there (e.g, whether a CATU is
> connected
> to the device or not etc). Hence the abstraction.

Ok

>
>
> Suzuki

[PATCH 10/12] coresight tmc: Support for save-restore in ETR

[Suzuki K Poulose]

The Coresight SoC 600 TMC ETR supports save-restore feature,
where the values of the RRP/RWP and STS.Full are retained
when it leaves the Disabled state. Hence, we must program the
RRP/RWP and STS.Full to a proper value. For now, set the RRP/RWP
to the base address of the buffer and clear the STS.Full register.
This can be later exploited for proper save-restore of ETR
trace contexts (e.g, perf).

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 13 ++++++++++++-
 drivers/hwtracing/coresight/coresight-tmc.h     |  8 ++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index ff11b92..7294fb7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -22,7 +22,7 @@
 
 static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
 {
-	u32 axictl;
+	u32 axictl, sts;
 
 	/* Zero out the memory to help with debug */
 	memset(drvdata->vaddr, 0, drvdata->size);
@@ -45,6 +45,17 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
 		  TMC_AXICTL_PROT_CTL_B1;
 	writel_relaxed(axictl, drvdata->base + TMC_AXICTL);
 	tmc_write_dba(drvdata, drvdata->paddr);
+	/*
+	 * If the TMC pointers must be programmed before the session,
+	 * we have to set it properly (i.e, RRP/RWP to base address and
+	 * STS to "not full").
+	 */
+	if (tmc_has_cap(drvdata, TMC_CAP_ETR_SAVE_RESTORE)) {
+		tmc_write_rrp(drvdata, drvdata->paddr);
+		tmc_write_rwp(drvdata, drvdata->paddr);
+		sts = readl_relaxed(drvdata->base + TMC_STS) & ~TMC_STS_FULL;
+		writel_relaxed(sts, drvdata->base + TMC_STS);
+	}
 
 	writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
 		       TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 87e4561..d5ef51e 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -90,6 +90,14 @@ enum tmc_mem_intf_width {
 };
 
 #define TMC_CAP_ETR_SG_UNIT			(1U << 0)
+/*
+ * TMC_CAP_ETR_SAVE_RESTORE - Values of RRP/RWP/STS.Full are
+ * retained when TMC leaves Disabled state, allowing us to continue
+ * the tracing from a point where we stopped. This also implies that
+ * the RRP/RWP/STS.Full should always be programmed to the correct
+ * value.
+ */
+#define TMC_CAP_ETR_SAVE_RESTORE	(1U << 1)
 
 /**
  * struct tmc_cap - Describes the capabilities of the TMC.
-- 
2.7.4

[PATCH 11/12] coresight tmc: Add support for Coresight SoC 600 TMC

[Suzuki K Poulose]

The coresight SoC 600 supports ETR save-restore and also supports
a new mode, SWFIFO2, which helps to streaming the trace data through
a functional I/O (e.g, USB).

Also, TMCs have different PIDs in different configurations (ETF,
ETB & ETR), unlike the previous generation.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc.c | 20 ++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tmc.h |  8 ++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index e88f2f3..03cafa7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -409,6 +409,10 @@ static struct tmc_caps coresight_soc_400_tmc_caps = {
 	.caps = CORESIGHT_SOC_400_TMC_CAPS,
 };
 
+static struct tmc_caps coresight_soc_600_etr_caps = {
+	.caps = CORESIGHT_SOC_600_ETR_CAPS,
+};
+
 static struct amba_id tmc_ids[] = {
 	{
 		/* Coresight SoC 400 TMC */
@@ -416,6 +420,22 @@ static struct amba_id tmc_ids[] = {
 		.mask   = 0x000fffff,
 		.data	= &coresight_soc_400_tmc_caps,
 	},
+	{
+		/* Coresight SoC 600 TMC-ETR/ETS */
+		.id	= 0x000bb9e8,
+		.mask	= 0x000fffff,
+		.data	= &coresight_soc_600_etr_caps,
+	},
+	{
+		/* Coresight SoC 600 TMC-ETB */
+		.id	= 0x000bb9e9,
+		.mask	= 0x000fffff,
+	},
+	{
+		/* Coresight SoC 600 TMC-ETF */
+		.id	= 0x000bb9ea,
+		.mask	= 0x000fffff,
+	},
 	{},
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index d5ef51e..8c74e1e 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -98,6 +98,12 @@ enum tmc_mem_intf_width {
  * value.
  */
 #define TMC_CAP_ETR_SAVE_RESTORE	(1U << 1)
+/*
+ * TMC_CAP_ETR_SWFIFO2_MODE - ETR supports a new mode, SWFIFO2, which
+ * allows streaming the trace data with optionally raising an interrupt
+ * when the buffer fill level reaches a programmed watermark.
+ */
+#define TMC_CAP_ETR_SWFIFO2_MODE	(1U << 2)
 
 /**
  * struct tmc_cap - Describes the capabilities of the TMC.
@@ -108,6 +114,8 @@ struct tmc_caps {
 };
 
 #define CORESIGHT_SOC_400_TMC_CAPS	(TMC_CAP_ETR_SG_UNIT)
+#define CORESIGHT_SOC_600_ETR_CAPS	(TMC_CAP_ETR_SAVE_RESTORE | \
+					 TMC_CAP_ETR_SWFIFO2_MODE)
 
 /**
  * struct tmc_drvdata - specifics associated to an TMC component
-- 
2.7.4

Re: [PATCH 11/12] coresight tmc: Add support for Coresight SoC 600 TMC

[Mathieu Poirier]

On Mon, Jun 12, 2017 at 03:36:50PM +0100, Suzuki K Poulose wrote:
> The coresight SoC 600 supports ETR save-restore and also supports
> a new mode, SWFIFO2, which helps to streaming the trace data through
> a functional I/O (e.g, USB).
> 
> Also, TMCs have different PIDs in different configurations (ETF,
> ETB & ETR), unlike the previous generation.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc.c | 20 ++++++++++++++++++++
>  drivers/hwtracing/coresight/coresight-tmc.h |  8 ++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index e88f2f3..03cafa7 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -409,6 +409,10 @@ static struct tmc_caps coresight_soc_400_tmc_caps = {
>  	.caps = CORESIGHT_SOC_400_TMC_CAPS,
>  };
>  
> +static struct tmc_caps coresight_soc_600_etr_caps = {
> +	.caps = CORESIGHT_SOC_600_ETR_CAPS,
> +};
> +
>  static struct amba_id tmc_ids[] = {
>  	{
>  		/* Coresight SoC 400 TMC */
> @@ -416,6 +420,22 @@ static struct amba_id tmc_ids[] = {
>  		.mask   = 0x000fffff,
>  		.data	= &coresight_soc_400_tmc_caps,
>  	},
> +	{
> +		/* Coresight SoC 600 TMC-ETR/ETS */
> +		.id	= 0x000bb9e8,
> +		.mask	= 0x000fffff,
> +		.data	= &coresight_soc_600_etr_caps,
> +	},
> +	{
> +		/* Coresight SoC 600 TMC-ETB */
> +		.id	= 0x000bb9e9,
> +		.mask	= 0x000fffff,
> +	},
> +	{
> +		/* Coresight SoC 600 TMC-ETF */
> +		.id	= 0x000bb9ea,
> +		.mask	= 0x000fffff,
> +	},
>  	{},
>  };
>  
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index d5ef51e..8c74e1e 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -98,6 +98,12 @@ enum tmc_mem_intf_width {
>   * value.
>   */
>  #define TMC_CAP_ETR_SAVE_RESTORE	(1U << 1)
> +/*
> + * TMC_CAP_ETR_SWFIFO2_MODE - ETR supports a new mode, SWFIFO2, which
> + * allows streaming the trace data with optionally raising an interrupt
> + * when the buffer fill level reaches a programmed watermark.
> + */
> +#define TMC_CAP_ETR_SWFIFO2_MODE	(1U << 2)
>  
>  /**
>   * struct tmc_cap - Describes the capabilities of the TMC.
> @@ -108,6 +114,8 @@ struct tmc_caps {
>  };
>  
>  #define CORESIGHT_SOC_400_TMC_CAPS	(TMC_CAP_ETR_SG_UNIT)
> +#define CORESIGHT_SOC_600_ETR_CAPS	(TMC_CAP_ETR_SAVE_RESTORE | \
> +					 TMC_CAP_ETR_SWFIFO2_MODE)

TMC_CAP_ETR_SWFIFO2_MODE isn't used anywhere - please remove.  It can be added
when code that uses the feature is introduced.

Thanks,
Mathieu

>  
>  /**
>   * struct tmc_drvdata - specifics associated to an TMC component
> -- 
> 2.7.4
> 

Re: [PATCH 11/12] coresight tmc: Add support for Coresight SoC 600 TMC

[Suzuki K Poulose]

On 14/06/17 19:25, Mathieu Poirier wrote:
> On Mon, Jun 12, 2017 at 03:36:50PM +0100, Suzuki K Poulose wrote:
>> The coresight SoC 600 supports ETR save-restore and also supports
>> a new mode, SWFIFO2, which helps to streaming the trace data through
>> a functional I/O (e.g, USB).
>>
>> Also, TMCs have different PIDs in different configurations (ETF,
>> ETB & ETR), unlike the previous generation.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>

...

>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
>> index d5ef51e..8c74e1e 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
>> @@ -98,6 +98,12 @@ enum tmc_mem_intf_width {
>>   * value.
>>   */
>>  #define TMC_CAP_ETR_SAVE_RESTORE	(1U << 1)
>> +/*
>> + * TMC_CAP_ETR_SWFIFO2_MODE - ETR supports a new mode, SWFIFO2, which
>> + * allows streaming the trace data with optionally raising an interrupt
>> + * when the buffer fill level reaches a programmed watermark.
>> + */
>> +#define TMC_CAP_ETR_SWFIFO2_MODE	(1U << 2)
>>
>>  /**
>>   * struct tmc_cap - Describes the capabilities of the TMC.
>> @@ -108,6 +114,8 @@ struct tmc_caps {
>>  };
>>
>>  #define CORESIGHT_SOC_400_TMC_CAPS	(TMC_CAP_ETR_SG_UNIT)
>> +#define CORESIGHT_SOC_600_ETR_CAPS	(TMC_CAP_ETR_SAVE_RESTORE | \
>> +					 TMC_CAP_ETR_SWFIFO2_MODE)
>
> TMC_CAP_ETR_SWFIFO2_MODE isn't used anywhere - please remove.  It can be added
> when code that uses the feature is introduced.

Sure, will do.

Suzuki

[PATCH 12/12] coresight: Add support for Coresight SoC 600 components

[Suzuki K Poulose]

Add the peripheral ids for the Coresight SoC 600 TPIU, replicator
and funnel.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-funnel.c          | 5 +++++
 drivers/hwtracing/coresight/coresight-replicator-qcom.c | 6 ++++++
 drivers/hwtracing/coresight/coresight-tpiu.c            | 5 +++++
 3 files changed, 16 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index 6f7f3d3..df1cf73 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -251,6 +251,11 @@ static struct amba_id funnel_ids[] = {
 		.id     = 0x000bb908,
 		.mask   = 0x000fffff,
 	},
+	{
+		/* Coresight SoC-600 */
+		.id     = 0x000bb9eb,
+		.mask   = 0x000fffff,
+	},
 	{ 0, 0},
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-replicator-qcom.c b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
index 4dd18e0..d09b124 100644
--- a/drivers/hwtracing/coresight/coresight-replicator-qcom.c
+++ b/drivers/hwtracing/coresight/coresight-replicator-qcom.c
@@ -202,6 +202,12 @@ static struct amba_id replicator_ids[] = {
 		.mask   = 0x000bffff,
 		.data	= "REPLICATOR 1.0",
 	},
+	{
+		/* Coresight SoC-600 */
+		.id     = 0x000bb9ec,
+		.mask   = 0x000fffff,
+		.data	= "REPLICATOR 1.0",
+	},
 	{ 0, 0 },
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
index 59c1510..eda5d5a 100644
--- a/drivers/hwtracing/coresight/coresight-tpiu.c
+++ b/drivers/hwtracing/coresight/coresight-tpiu.c
@@ -201,6 +201,11 @@ static struct amba_id tpiu_ids[] = {
 		.id	= 0x0004b912,
 		.mask	= 0x0007ffff,
 	},
+	{
+		/* Coresight SoC-600 */
+		.id	= 0x000bb9e7,
+		.mask	= 0x000fffff,
+	},
 	{ 0, 0},
 };
 
-- 
2.7.4