diff mbox

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

Message ID 1487358790-2618-1-git-send-email-mike.leach@linaro.org (mailing list archive)
State New, archived
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(-)

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
>
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>;
>
Mike Leach Feb. 21, 2017, 12:34 p.m. UTC | #3
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.

>  Since it's too late for
> v4.11 (already in merge window now), I will queue this for v4.12
>

Thanks.

Mike



> > 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
> _______________________________________________
> CoreSight mailing list
> CoreSight@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/coresight
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Sudeep Holla Feb. 21, 2017, 12:37 p.m. UTC | #4
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.
Mike Leach Feb. 21, 2017, 12:46 p.m. UTC | #5
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
Sudeep Holla Feb. 21, 2017, 1:53 p.m. UTC | #6
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.
Mathieu Poirier Feb. 21, 2017, 3:47 p.m. UTC | #7
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
Sudeep Holla Feb. 21, 2017, 3:54 p.m. UTC | #8
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.
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>;