diff mbox

arm64: dts: juno: update definition for programmable replicator.

Message ID 1487358790-2618-1-git-send-email-mike.leach@linaro.org
State Accepted
Commit 7e6a69ee954caf8dd8c98f70c8ee894c8ebfdcf0
Headers show

Commit Message

Mike Leach Feb. 17, 2017, 7:13 p.m. UTC 
Juno platforms have a programmable replicator splitting the trace output to
TPIU and ETR. Currently this is not being programmed as it is being treated
as a none-programmable replicator - which is the default operational mode
for these devices. The TPIU in the system is enabled by default, and this
combination is causing back-pressure in the trace system resulting in
overflows at the source.

Replaces the existing definition with one that defines the programmable
replicator, using the "qcom,coresight-replicator1x" driver that provides
the correct functionality for CoreSight programmable replicators.

Signed-off-by: Mike Leach <mike.leach@linaro.org>

---
 arch/arm64/boot/dts/arm/juno-base.dtsi | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mathieu Poirier Feb. 20, 2017, 6:50 p.m. UTC | #1 
On 17 February 2017 at 12:13, Mike Leach <mike.leach@linaro.org> wrote:
> Juno platforms have a programmable replicator splitting the trace output to

> TPIU and ETR. Currently this is not being programmed as it is being treated

> as a none-programmable replicator - which is the default operational mode

> for these devices. The TPIU in the system is enabled by default, and this

> combination is causing back-pressure in the trace system resulting in

> overflows at the source.

>

> Replaces the existing definition with one that defines the programmable

> replicator, using the "qcom,coresight-replicator1x" driver that provides

> the correct functionality for CoreSight programmable replicators.

>

> Signed-off-by: Mike Leach <mike.leach@linaro.org>

> ---

>  arch/arm64/boot/dts/arm/juno-base.dtsi | 14 ++++++++------

>  1 file changed, 8 insertions(+), 6 deletions(-)

>

> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi

> index 9d799d9..6546e23 100644

> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi

> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi

> @@ -372,12 +372,14 @@

>                 };

>         };

>

> -       coresight-replicator {

> -               /*

> -                * Non-configurable replicators don't show up on the

> -                * AMBA bus.  As such no need to add "arm,primecell".

> -                */

> -               compatible = "arm,coresight-replicator";

> +       coresight-replicator@20120000 {

> +


There is an extra line that is not needed here.  And we can probably
change "coresight-replicator" to "replicator" now that it can be
discovered on the AMBA bus.

Other than that:

Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>


> +               compatible = "qcom,coresight-replicator1x", "arm,primecell";

> +               reg = <0 0x20120000 0 0x1000>;

> +

> +               clocks = <&soc_smc50mhz>;

> +               clock-names = "apb_pclk";

> +               power-domains = <&scpi_devpd 0>;

>

>                 ports {

>                         #address-cells = <1>;

> --

> 2.7.4

>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sudeep Holla Feb. 21, 2017, 11:02 a.m. UTC | #2 
Hi Mike,

On 17/02/17 19:13, Mike Leach wrote:
> Juno platforms have a programmable replicator splitting the trace output to

> TPIU and ETR. Currently this is not being programmed as it is being treated

> as a none-programmable replicator - which is the default operational mode

> for these devices. The TPIU in the system is enabled by default, and this

> combination is causing back-pressure in the trace system resulting in

> overflows at the source.

> 

> Replaces the existing definition with one that defines the programmable

> replicator, using the "qcom,coresight-replicator1x" driver that provides

> the correct functionality for CoreSight programmable replicators.

> 


I assume this is just enhancement and not a fix. Since it's too late for
v4.11 (already in merge window now), I will queue this for v4.12

> Signed-off-by: Mike Leach <mike.leach@linaro.org>

> ---

>  arch/arm64/boot/dts/arm/juno-base.dtsi | 14 ++++++++------

>  1 file changed, 8 insertions(+), 6 deletions(-)

> 

> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi

> index 9d799d9..6546e23 100644

> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi

> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi

> @@ -372,12 +372,14 @@

>  		};

>  	};

>  

> -	coresight-replicator {

> -		/*

> -		 * Non-configurable replicators don't show up on the

> -		 * AMBA bus.  As such no need to add "arm,primecell".

> -		 */

> -		compatible = "arm,coresight-replicator";

> +	coresight-replicator@20120000 {

> +

> +		compatible = "qcom,coresight-replicator1x", "arm,primecell";

> +		reg = <0 0x20120000 0 0x1000>;

> +

> +		clocks = <&soc_smc50mhz>;

> +		clock-names = "apb_pclk";

> +		power-domains = <&scpi_devpd 0>;

>  

>  		ports {

>  			#address-cells = <1>;

> 


-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sudeep Holla Feb. 21, 2017, 12:37 p.m. UTC | #3 
On 21/02/17 12:34, Mike Leach wrote:
> Hi Sudeep

> 

>> -----Original Message-----

>> From: CoreSight [mailto:coresight-bounces@lists.linaro.org] On Behalf Of

>> Sudeep Holla

>> Sent: 21 February 2017 11:03

>> To: Mike Leach; linux-arm-kernel@lists.infradead.org

>> Cc: devicetree@vger.kernel.org; coresight@lists.linaro.org;

>> robh+dt@kernel.org; Sudeep Holla

>> Subject: Re: [PATCH] arm64: dts: juno: update definition for programmable

>> replicator.

>>

>> Hi Mike,

>>

>> On 17/02/17 19:13, Mike Leach wrote:

>>> Juno platforms have a programmable replicator splitting the trace

>>> output to TPIU and ETR. Currently this is not being programmed as it

>>> is being treated as a none-programmable replicator - which is the

>>> default operational mode for these devices. The TPIU in the system is

>>> enabled by default, and this combination is causing back-pressure in

>>> the trace system resulting in overflows at the source.

>>>

>>> Replaces the existing definition with one that defines the

>>> programmable replicator, using the "qcom,coresight-replicator1x"

>>> driver that provides the correct functionality for CoreSight programmable

>> replicators.

>>>

>>

>> I assume this is just enhancement and not a fix.

> 

> I guess it depends on your point of view - with this update the trace

> overflows I was seeing disappear, as the trace path to TPIU is

> blocked. So it affects the quality of collected trace using the ETR

> rather than a binary didn't work / works now change.

> 


OK, could you add the warning overflow messages if you get, so that it's
very clear from the change log. Then I can see if it can be pushed as fix.

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Leach Feb. 21, 2017, 12:46 p.m. UTC | #4 
The messages are not a normal linux / kernel warning message, but a
result of the offline trace decode process:-

  110739: I_OVERFLOW : Overflow.
  110741: I_ASYNC : Alignment Synchronisation.
  110754: I_TRACE_INFO : Trace Info.; PCTL=0x0
  110757: I_TRACE_ON : Trace On.

If this is sufficient with an explanation I'm happy to add it to the patch.

Mike


On 21 February 2017 at 12:37, Sudeep Holla <sudeep.holla@arm.com> wrote:
>

>

> On 21/02/17 12:34, Mike Leach wrote:

>> Hi Sudeep

>>

>>> -----Original Message-----

>>> From: CoreSight [mailto:coresight-bounces@lists.linaro.org] On Behalf Of

>>> Sudeep Holla

>>> Sent: 21 February 2017 11:03

>>> To: Mike Leach; linux-arm-kernel@lists.infradead.org

>>> Cc: devicetree@vger.kernel.org; coresight@lists.linaro.org;

>>> robh+dt@kernel.org; Sudeep Holla

>>> Subject: Re: [PATCH] arm64: dts: juno: update definition for programmable

>>> replicator.

>>>

>>> Hi Mike,

>>>

>>> On 17/02/17 19:13, Mike Leach wrote:

>>>> Juno platforms have a programmable replicator splitting the trace

>>>> output to TPIU and ETR. Currently this is not being programmed as it

>>>> is being treated as a none-programmable replicator - which is the

>>>> default operational mode for these devices. The TPIU in the system is

>>>> enabled by default, and this combination is causing back-pressure in

>>>> the trace system resulting in overflows at the source.

>>>>

>>>> Replaces the existing definition with one that defines the

>>>> programmable replicator, using the "qcom,coresight-replicator1x"

>>>> driver that provides the correct functionality for CoreSight programmable

>>> replicators.

>>>>

>>>

>>> I assume this is just enhancement and not a fix.

>>

>> I guess it depends on your point of view - with this update the trace

>> overflows I was seeing disappear, as the trace path to TPIU is

>> blocked. So it affects the quality of collected trace using the ETR

>> rather than a binary didn't work / works now change.

>>

>

> OK, could you add the warning overflow messages if you get, so that it's

> very clear from the change log. Then I can see if it can be pushed as fix.

>

> --

> Regards,

> Sudeep




-- 
Mike Leach
Principal Engineer, ARM Ltd.
Blackburn Design Centre. UK
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sudeep Holla Feb. 21, 2017, 1:53 p.m. UTC | #5 
Hi Mike,

On 21/02/17 12:46, Mike Leach wrote:
> The messages are not a normal linux / kernel warning message, but a

> result of the offline trace decode process:-

> 

>   110739: I_OVERFLOW : Overflow.

>   110741: I_ASYNC : Alignment Synchronisation.

>   110754: I_TRACE_INFO : Trace Info.; PCTL=0x0

>   110757: I_TRACE_ON : Trace On.

> 

> If this is sufficient with an explanation I'm happy to add it to the patch.

> 


Oh OK, then no need to add to changelog as it requires the knowledge of
this tool to understand which is not part of the kernel tree. I will try
to push it as a fix and lets see how that goes.

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mathieu Poirier Feb. 21, 2017, 3:47 p.m. UTC | #6 
On 21 February 2017 at 04:02, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Hi Mike,

>

> On 17/02/17 19:13, Mike Leach wrote:

>> Juno platforms have a programmable replicator splitting the trace output to

>> TPIU and ETR. Currently this is not being programmed as it is being treated

>> as a none-programmable replicator - which is the default operational mode

>> for these devices. The TPIU in the system is enabled by default, and this

>> combination is causing back-pressure in the trace system resulting in

>> overflows at the source.

>>

>> Replaces the existing definition with one that defines the programmable

>> replicator, using the "qcom,coresight-replicator1x" driver that provides

>> the correct functionality for CoreSight programmable replicators.

>>

>

> I assume this is just enhancement and not a fix. Since it's too late for

> v4.11 (already in merge window now), I will queue this for v4.12

>

>> Signed-off-by: Mike Leach <mike.leach@linaro.org>

>> ---

>>  arch/arm64/boot/dts/arm/juno-base.dtsi | 14 ++++++++------

>>  1 file changed, 8 insertions(+), 6 deletions(-)

>>

>> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi

>> index 9d799d9..6546e23 100644

>> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi

>> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi

>> @@ -372,12 +372,14 @@

>>               };

>>       };

>>

>> -     coresight-replicator {

>> -             /*

>> -              * Non-configurable replicators don't show up on the

>> -              * AMBA bus.  As such no need to add "arm,primecell".

>> -              */

>> -             compatible = "arm,coresight-replicator";

>> +     coresight-replicator@20120000 {

>> +


As stated in a previous post please remove the extra line and the
"coresight-".  Since programmable replicators show up on the AMBA bus
there is no need to differentiate them from other CoreSight devices.

Thanks,
Mathieu

>> +             compatible = "qcom,coresight-replicator1x", "arm,primecell";

>> +             reg = <0 0x20120000 0 0x1000>;

>> +

>> +             clocks = <&soc_smc50mhz>;

>> +             clock-names = "apb_pclk";

>> +             power-domains = <&scpi_devpd 0>;

>>

>>               ports {

>>                       #address-cells = <1>;

>>

>

> --

> Regards,

> Sudeep

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sudeep Holla Feb. 21, 2017, 3:54 p.m. UTC | #7 
On 21/02/17 15:47, Mathieu Poirier wrote:
> On 21 February 2017 at 04:02, Sudeep Holla <sudeep.holla@arm.com> wrote:

>> Hi Mike,

>>

>> On 17/02/17 19:13, Mike Leach wrote:

>>> Juno platforms have a programmable replicator splitting the trace output to

>>> TPIU and ETR. Currently this is not being programmed as it is being treated

>>> as a none-programmable replicator - which is the default operational mode

>>> for these devices. The TPIU in the system is enabled by default, and this

>>> combination is causing back-pressure in the trace system resulting in

>>> overflows at the source.

>>>

>>> Replaces the existing definition with one that defines the programmable

>>> replicator, using the "qcom,coresight-replicator1x" driver that provides

>>> the correct functionality for CoreSight programmable replicators.

>>>

>>

>> I assume this is just enhancement and not a fix. Since it's too late for

>> v4.11 (already in merge window now), I will queue this for v4.12

>>

>>> Signed-off-by: Mike Leach <mike.leach@linaro.org>

>>> ---

>>>  arch/arm64/boot/dts/arm/juno-base.dtsi | 14 ++++++++------

>>>  1 file changed, 8 insertions(+), 6 deletions(-)

>>>

>>> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi

>>> index 9d799d9..6546e23 100644

>>> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi

>>> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi

>>> @@ -372,12 +372,14 @@

>>>               };

>>>       };

>>>

>>> -     coresight-replicator {

>>> -             /*

>>> -              * Non-configurable replicators don't show up on the

>>> -              * AMBA bus.  As such no need to add "arm,primecell".

>>> -              */

>>> -             compatible = "arm,coresight-replicator";

>>> +     coresight-replicator@20120000 {

>>> +

> 

> As stated in a previous post please remove the extra line and the

> "coresight-".  Since programmable replicators show up on the AMBA bus

> there is no need to differentiate them from other CoreSight devices.

> 


Already done in my local branch ;) along with you tags.

Also I away when merge window closes, so I will be able to send only at
-rc2. ARM SoC team generally prefer to rebase fixes on top of -rc1.

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index 9d799d9..6546e23 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -372,12 +372,14 @@ 
 		};
 	};
 
-	coresight-replicator {
-		/*
-		 * Non-configurable replicators don't show up on the
-		 * AMBA bus.  As such no need to add "arm,primecell".
-		 */
-		compatible = "arm,coresight-replicator";
+	coresight-replicator@20120000 {
+
+		compatible = "qcom,coresight-replicator1x", "arm,primecell";
+		reg = <0 0x20120000 0 0x1000>;
+
+		clocks = <&soc_smc50mhz>;
+		clock-names = "apb_pclk";
+		power-domains = <&scpi_devpd 0>;
 
 		ports {
 			#address-cells = <1>;