diff mbox

coresight: Fix disabling of CoreSight TPIU

Message ID 1511517525-3270-1-git-send-email-robert.walker@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Robert Walker Nov. 24, 2017, 9:58 a.m. UTC 
The CoreSight TPIU should be disabled when tracing to other sinks to allow
them to operate at full bandwidth.

This patch fixes tpiu_disable_hw() to correctly disable the TPIU by
configuring the TPIU to stop on flush, initiating a manual flush, waiting
for the flush to complete and then waits for the TPIU to indicate it has
stopped.

Signed-off-by: Robert Walker <robert.walker@arm.com>
---
 drivers/hwtracing/coresight/coresight-tpiu.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Mathieu Poirier Nov. 27, 2017, 5:30 p.m. UTC | #1 
Hi Robert,

On 24 November 2017 at 02:58, Robert Walker <robert.walker@arm.com> wrote:
> The CoreSight TPIU should be disabled when tracing to other sinks to allow
> them to operate at full bandwidth.
>
> This patch fixes tpiu_disable_hw() to correctly disable the TPIU by
> configuring the TPIU to stop on flush, initiating a manual flush, waiting
> for the flush to complete and then waits for the TPIU to indicate it has
> stopped.
>
> Signed-off-by: Robert Walker <robert.walker@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tpiu.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
> index bef49a3..4b46c49 100644
> --- a/drivers/hwtracing/coresight/coresight-tpiu.c
> +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
> @@ -46,8 +46,11 @@
>  #define TPIU_ITATBCTR0         0xef8
>
>  /** register definition **/
> +/* FFSR - 0x300 */
> +#define FFSR_FT_STOPPED                BIT(1)
>  /* FFCR - 0x304 */
>  #define FFCR_FON_MAN           BIT(6)
> +#define FFCR_STOP_FI           BIT(12)
>
>  /**
>   * @base:      memory mapped base address for this component.
> @@ -85,10 +88,14 @@ static void tpiu_disable_hw(struct tpiu_drvdata *drvdata)
>  {
>         CS_UNLOCK(drvdata->base);
>
> -       /* Clear formatter controle reg. */
> -       writel_relaxed(0x0, drvdata->base + TPIU_FFCR);
> +       /* Clear formatter and stop on flush */
> +       writel_relaxed(FFCR_STOP_FI, drvdata->base + TPIU_FFCR);
>         /* Generate manual flush */
> -       writel_relaxed(FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
> +       writel_relaxed(FFCR_STOP_FI | FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
> +       /* Wait for flush to complete */
> +       coresight_timeout(drvdata->base, TPIU_FFCR, FFCR_FON_MAN, 0);
> +       /* Wait for formatter to stop */
> +       coresight_timeout(drvdata->base, TPIU_FFSR, FFSR_FT_STOPPED, 1);
>
>         CS_LOCK(drvdata->base);
>  }

Shouldn't we also do this at boot time when the TPIU in _probed() ?
That way the TPIU doesn't have to explicitly be enabled and disabled
prior to running other tracing sessions.

Thanks,
Mathieu


> --
> 1.9.1
>
Robert Walker Nov. 28, 2017, 8:55 a.m. UTC | #2 
Hi Mathieu,

> -----Original Message-----
> From: Mathieu Poirier [mailto:mathieu.poirier@linaro.org]
> Sent: 27 November 2017 17:30
> To: Robert Walker <Robert.Walker@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org; CoreSight@lists.linaro.org
> Subject: Re: [PATCH] coresight: Fix disabling of CoreSight TPIU
> 
> Hi Robert,
> 
> On 24 November 2017 at 02:58, Robert Walker <robert.walker@arm.com>
> wrote:
> > The CoreSight TPIU should be disabled when tracing to other sinks to
> > allow them to operate at full bandwidth.
> >
> > This patch fixes tpiu_disable_hw() to correctly disable the TPIU by
> > configuring the TPIU to stop on flush, initiating a manual flush,
> > waiting for the flush to complete and then waits for the TPIU to
> > indicate it has stopped.
> >
> > Signed-off-by: Robert Walker <robert.walker@arm.com>
> > ---
> >  drivers/hwtracing/coresight/coresight-tpiu.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c
> > b/drivers/hwtracing/coresight/coresight-tpiu.c
> > index bef49a3..4b46c49 100644
> > --- a/drivers/hwtracing/coresight/coresight-tpiu.c
> > +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
> > @@ -46,8 +46,11 @@
> >  #define TPIU_ITATBCTR0         0xef8
> >
> >  /** register definition **/
> > +/* FFSR - 0x300 */
> > +#define FFSR_FT_STOPPED                BIT(1)
> >  /* FFCR - 0x304 */
> >  #define FFCR_FON_MAN           BIT(6)
> > +#define FFCR_STOP_FI           BIT(12)
> >
> >  /**
> >   * @base:      memory mapped base address for this component.
> > @@ -85,10 +88,14 @@ static void tpiu_disable_hw(struct tpiu_drvdata
> > *drvdata)  {
> >         CS_UNLOCK(drvdata->base);
> >
> > -       /* Clear formatter controle reg. */
> > -       writel_relaxed(0x0, drvdata->base + TPIU_FFCR);
> > +       /* Clear formatter and stop on flush */
> > +       writel_relaxed(FFCR_STOP_FI, drvdata->base + TPIU_FFCR);
> >         /* Generate manual flush */
> > -       writel_relaxed(FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
> > +       writel_relaxed(FFCR_STOP_FI | FFCR_FON_MAN, drvdata->base +
> TPIU_FFCR);
> > +       /* Wait for flush to complete */
> > +       coresight_timeout(drvdata->base, TPIU_FFCR, FFCR_FON_MAN, 0);
> > +       /* Wait for formatter to stop */
> > +       coresight_timeout(drvdata->base, TPIU_FFSR, FFSR_FT_STOPPED,
> > + 1);
> >
> >         CS_LOCK(drvdata->base);
> >  }
> 
> Shouldn't we also do this at boot time when the TPIU in _probed() ?
> That way the TPIU doesn't have to explicitly be enabled and disabled prior to
> running other tracing sessions.
> 
> Thanks,
> Mathieu

There already is a call to tpiu_disable_hw() in tpiu_probe(), so the TPIU is disabled on boot.
When tracing to another sink (e.g. ETR), the TPIU won't be enabled / disabled (i.e. tpiu_enable_hw() and tpiu_disable_hw() won't be called) as it's not on the direct path from source to sink.

Regards

Rob

> 
> 
> > --
> > 1.9.1
> >
Mathieu Poirier Nov. 28, 2017, 3:24 p.m. UTC | #3 
On 28 November 2017 at 01:55, Robert Walker <robert.walker@arm.com> wrote:
> Hi Mathieu,
>
>> -----Original Message-----
>> From: Mathieu Poirier [mailto:mathieu.poirier@linaro.org]
>> Sent: 27 November 2017 17:30
>> To: Robert Walker <Robert.Walker@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org; CoreSight@lists.linaro.org
>> Subject: Re: [PATCH] coresight: Fix disabling of CoreSight TPIU
>>
>> Hi Robert,
>>
>> On 24 November 2017 at 02:58, Robert Walker <robert.walker@arm.com>
>> wrote:
>> > The CoreSight TPIU should be disabled when tracing to other sinks to
>> > allow them to operate at full bandwidth.
>> >
>> > This patch fixes tpiu_disable_hw() to correctly disable the TPIU by
>> > configuring the TPIU to stop on flush, initiating a manual flush,
>> > waiting for the flush to complete and then waits for the TPIU to
>> > indicate it has stopped.
>> >
>> > Signed-off-by: Robert Walker <robert.walker@arm.com>
>> > ---
>> >  drivers/hwtracing/coresight/coresight-tpiu.c | 13 ++++++++++---
>> >  1 file changed, 10 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c
>> > b/drivers/hwtracing/coresight/coresight-tpiu.c
>> > index bef49a3..4b46c49 100644
>> > --- a/drivers/hwtracing/coresight/coresight-tpiu.c
>> > +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
>> > @@ -46,8 +46,11 @@
>> >  #define TPIU_ITATBCTR0         0xef8
>> >
>> >  /** register definition **/
>> > +/* FFSR - 0x300 */
>> > +#define FFSR_FT_STOPPED                BIT(1)
>> >  /* FFCR - 0x304 */
>> >  #define FFCR_FON_MAN           BIT(6)
>> > +#define FFCR_STOP_FI           BIT(12)
>> >
>> >  /**
>> >   * @base:      memory mapped base address for this component.
>> > @@ -85,10 +88,14 @@ static void tpiu_disable_hw(struct tpiu_drvdata
>> > *drvdata)  {
>> >         CS_UNLOCK(drvdata->base);
>> >
>> > -       /* Clear formatter controle reg. */
>> > -       writel_relaxed(0x0, drvdata->base + TPIU_FFCR);
>> > +       /* Clear formatter and stop on flush */
>> > +       writel_relaxed(FFCR_STOP_FI, drvdata->base + TPIU_FFCR);
>> >         /* Generate manual flush */
>> > -       writel_relaxed(FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
>> > +       writel_relaxed(FFCR_STOP_FI | FFCR_FON_MAN, drvdata->base +
>> TPIU_FFCR);
>> > +       /* Wait for flush to complete */
>> > +       coresight_timeout(drvdata->base, TPIU_FFCR, FFCR_FON_MAN, 0);
>> > +       /* Wait for formatter to stop */
>> > +       coresight_timeout(drvdata->base, TPIU_FFSR, FFSR_FT_STOPPED,
>> > + 1);
>> >
>> >         CS_LOCK(drvdata->base);
>> >  }
>>
>> Shouldn't we also do this at boot time when the TPIU in _probed() ?
>> That way the TPIU doesn't have to explicitly be enabled and disabled prior to
>> running other tracing sessions.
>>
>> Thanks,
>> Mathieu
>
> There already is a call to tpiu_disable_hw() in tpiu_probe(), so the TPIU is disabled on boot.

Nothing has happened on the TPIU side since I first re-worked the
driver, thanks for the reminder.


> When tracing to another sink (e.g. ETR), the TPIU won't be enabled / disabled (i.e. tpiu_enable_hw() and tpiu_disable_hw() won't be called) as it's not on the direct path from source to sink.

Right, that part is of no concern to me.

>
> Regards
>
> Rob
>
>>
>>
>> > --
>> > 1.9.1
>> >
>
Mike Leach Dec. 7, 2017, 4:14 p.m. UTC | #4 
Tested by: Mike Leach <mike.leach@linaro.org>

On 24 November 2017 at 09:58, Robert Walker <robert.walker@arm.com> wrote:
> The CoreSight TPIU should be disabled when tracing to other sinks to allow
> them to operate at full bandwidth.
>
> This patch fixes tpiu_disable_hw() to correctly disable the TPIU by
> configuring the TPIU to stop on flush, initiating a manual flush, waiting
> for the flush to complete and then waits for the TPIU to indicate it has
> stopped.
>
> Signed-off-by: Robert Walker <robert.walker@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tpiu.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
> index bef49a3..4b46c49 100644
> --- a/drivers/hwtracing/coresight/coresight-tpiu.c
> +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
> @@ -46,8 +46,11 @@
>  #define TPIU_ITATBCTR0         0xef8
>
>  /** register definition **/
> +/* FFSR - 0x300 */
> +#define FFSR_FT_STOPPED                BIT(1)
>  /* FFCR - 0x304 */
>  #define FFCR_FON_MAN           BIT(6)
> +#define FFCR_STOP_FI           BIT(12)
>
>  /**
>   * @base:      memory mapped base address for this component.
> @@ -85,10 +88,14 @@ static void tpiu_disable_hw(struct tpiu_drvdata *drvdata)
>  {
>         CS_UNLOCK(drvdata->base);
>
> -       /* Clear formatter controle reg. */
> -       writel_relaxed(0x0, drvdata->base + TPIU_FFCR);
> +       /* Clear formatter and stop on flush */
> +       writel_relaxed(FFCR_STOP_FI, drvdata->base + TPIU_FFCR);
>         /* Generate manual flush */
> -       writel_relaxed(FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
> +       writel_relaxed(FFCR_STOP_FI | FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
> +       /* Wait for flush to complete */
> +       coresight_timeout(drvdata->base, TPIU_FFCR, FFCR_FON_MAN, 0);
> +       /* Wait for formatter to stop */
> +       coresight_timeout(drvdata->base, TPIU_FFSR, FFSR_FT_STOPPED, 1);
>
>         CS_LOCK(drvdata->base);
>  }
> --
> 1.9.1
>
> _______________________________________________
> CoreSight mailing list
> CoreSight@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/coresight
Mathieu Poirier Dec. 7, 2017, 4:39 p.m. UTC | #5 
On 7 December 2017 at 09:14, Mike Leach <mike.leach@linaro.org> wrote:
> Tested by: Mike Leach <mike.leach@linaro.org>
>
> On 24 November 2017 at 09:58, Robert Walker <robert.walker@arm.com> wrote:
>> The CoreSight TPIU should be disabled when tracing to other sinks to allow
>> them to operate at full bandwidth.
>>
>> This patch fixes tpiu_disable_hw() to correctly disable the TPIU by
>> configuring the TPIU to stop on flush, initiating a manual flush, waiting
>> for the flush to complete and then waits for the TPIU to indicate it has
>> stopped.
>>
>> Signed-off-by: Robert Walker <robert.walker@arm.com>
>> ---
>>  drivers/hwtracing/coresight/coresight-tpiu.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
>> index bef49a3..4b46c49 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpiu.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
>> @@ -46,8 +46,11 @@
>>  #define TPIU_ITATBCTR0         0xef8
>>
>>  /** register definition **/
>> +/* FFSR - 0x300 */
>> +#define FFSR_FT_STOPPED                BIT(1)
>>  /* FFCR - 0x304 */
>>  #define FFCR_FON_MAN           BIT(6)
>> +#define FFCR_STOP_FI           BIT(12)
>>
>>  /**
>>   * @base:      memory mapped base address for this component.
>> @@ -85,10 +88,14 @@ static void tpiu_disable_hw(struct tpiu_drvdata *drvdata)
>>  {
>>         CS_UNLOCK(drvdata->base);
>>
>> -       /* Clear formatter controle reg. */
>> -       writel_relaxed(0x0, drvdata->base + TPIU_FFCR);
>> +       /* Clear formatter and stop on flush */
>> +       writel_relaxed(FFCR_STOP_FI, drvdata->base + TPIU_FFCR);
>>         /* Generate manual flush */
>> -       writel_relaxed(FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
>> +       writel_relaxed(FFCR_STOP_FI | FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
>> +       /* Wait for flush to complete */
>> +       coresight_timeout(drvdata->base, TPIU_FFCR, FFCR_FON_MAN, 0);
>> +       /* Wait for formatter to stop */
>> +       coresight_timeout(drvdata->base, TPIU_FFSR, FFSR_FT_STOPPED, 1);
>>
>>         CS_LOCK(drvdata->base);
>>  }
>> --
>> 1.9.1

Applied - thanks.
Mathieu

>>
>> _______________________________________________
>> CoreSight mailing list
>> CoreSight@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/coresight
>
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Blackburn Design Centre. UK
diff mbox

Patch

diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
index bef49a3..4b46c49 100644
--- a/drivers/hwtracing/coresight/coresight-tpiu.c
+++ b/drivers/hwtracing/coresight/coresight-tpiu.c
@@ -46,8 +46,11 @@ 
 #define TPIU_ITATBCTR0		0xef8
 
 /** register definition **/
+/* FFSR - 0x300 */
+#define FFSR_FT_STOPPED		BIT(1)
 /* FFCR - 0x304 */
 #define FFCR_FON_MAN		BIT(6)
+#define FFCR_STOP_FI		BIT(12)
 
 /**
  * @base:	memory mapped base address for this component.
@@ -85,10 +88,14 @@  static void tpiu_disable_hw(struct tpiu_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
 
-	/* Clear formatter controle reg. */
-	writel_relaxed(0x0, drvdata->base + TPIU_FFCR);
+	/* Clear formatter and stop on flush */
+	writel_relaxed(FFCR_STOP_FI, drvdata->base + TPIU_FFCR);
 	/* Generate manual flush */
-	writel_relaxed(FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
+	writel_relaxed(FFCR_STOP_FI | FFCR_FON_MAN, drvdata->base + TPIU_FFCR);
+	/* Wait for flush to complete */
+	coresight_timeout(drvdata->base, TPIU_FFCR, FFCR_FON_MAN, 0);
+	/* Wait for formatter to stop */
+	coresight_timeout(drvdata->base, TPIU_FFSR, FFSR_FT_STOPPED, 1);
 
 	CS_LOCK(drvdata->base);
 }