RE: [EXT] Re: [PATCH v6 6/7] coresight: tmc: Stop trace capture on FlIn

Hi Mike,

-----Original Message-----
From: Mike Leach <redacted>
Sent: Friday, January 5, 2024 9:41 PM
To: Linu Cherian <redacted>
Cc: suzuki.poulose@arm.com; james.clark@arm.com; leo.yan@linaro.org;
linux-arm-kernel@lists.infradead.org; coresight@lists.linaro.org; linux-
kernel@vger.kernel.org; robh+dt@kernel.org;
krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
devicetree@vger.kernel.org; Sunil Kovvuri Goutham
[off-list ref]; George Cherian [off-list ref]
Subject: [EXT] Re: [PATCH v6 6/7] coresight: tmc: Stop trace capture on FlIn

External Email

----------------------------------------------------------------------
Hi,

On Fri, 5 Jan 2024 at 05:59, Linu Cherian [off-list ref] wrote:

quoted

Configure TMC ETR and ETF to flush and stop trace capture on FlIn
event. As a side effect, do manual flush only if auto flush fails.

Signed-off-by: Linu Cherian <redacted>
---
Changelog from v5:
* No changes

 drivers/hwtracing/coresight/coresight-tmc-etf.c | 10 ++++++++--
drivers/hwtracing/coresight/coresight-tmc-etr.c | 10 ++++++++--
 drivers/hwtracing/coresight/coresight-tmc.h     |  3 +++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c

b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 72c2315f4e23..57a9a9300d36 100644

--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c

@@ -34,7 +34,7 @@ static int __tmc_etb_enable_hw(struct tmc_drvdata

*drvdata)

quoted

        writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base +

TMC_MODE);

quoted

        writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
                       TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
-                      TMC_FFCR_TRIGON_TRIGIN,
+                      TMC_FFCR_TRIGON_TRIGIN |
+ TMC_FFCR_STOP_ON_FLUSH,
                       drvdata->base + TMC_FFCR);

This is a problem. Setting TMC_FFCR_STOP_ON_FLUSH changes the
fundamentals of trigger event handling. Without this bit ETM can generate
multiple event  + triggers which are then embedded into the formatted trace
stream for later search.
With this new bit the capture will stop on the first event. Setting this bit should
be dependent on the mode being set to ETR_MODE_RESRV

Okay, will add a configuration dependency here. 

quoted

        writel_relaxed(drvdata->trigger_cntr, drvdata->base +
TMC_TRG); @@ -615,7 +615,13 @@ static int tmc_panic_sync_etf(struct

coresight_device *csdev)

quoted

        if (val != TMC_MODE_CIRCULAR_BUFFER)
                goto out;

-       tmc_flush_and_stop(drvdata);
+       val = readl(drvdata->base + TMC_FFSR);
+       /* Do manual flush and stop only if its not auto-stopped */
+       if (!(val & TMC_FFSR_FT_STOPPED)) {
+               dev_info(&csdev->dev,
+                        "%s: Triggering manual flush\n", __func__);
+               tmc_flush_and_stop(drvdata);
+       }

Is there some reason to believe that the stop on flush will not work?

CTI misconfiguration or even skipping the CTI configuration could be the reasons.

Using this conditional skips the  tmc_wait_for_tmcready() called by
tmc_flush_and_stop() if the formatter is stopped - which is a different
condition test on a different register.

Okay got it. While add a tmc_wait_for_tmcready() for the event based flush as well.

Why is this block of code not in the patch that introduces the
tmc_panic_sync_etf()

Ack. Will merge the above block of code alone to the patch that introduces tmc_panic_sync_xxx

quoted

        /* Sync registers from hardware to metadata region */
        mdata->sts = csdev_access_relaxed_read32(csa, TMC_STS); diff
--git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index ab7521bbb2f5..4b3c7ec7f62b 100644

--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c

@@ -1113,7 +1113,7 @@ static int __tmc_etr_enable_hw(struct

tmc_drvdata *drvdata)

        writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
                       TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
-                      TMC_FFCR_TRIGON_TRIGIN,
+                      TMC_FFCR_TRIGON_TRIGIN |
+ TMC_FFCR_STOP_ON_FLUSH,
                       drvdata->base + TMC_FFCR);
        writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
        tmc_enable_hw(drvdata);

@@ -1846,7 +1846,13 @@ static int tmc_panic_sync_etr(struct

coresight_device *csdev)

quoted

        if (!(val & TMC_CTL_CAPT_EN))
                goto out;

-       tmc_flush_and_stop(drvdata);
+       val = readl(drvdata->base + TMC_FFSR);
+       /* Do manual flush and stop only if its not auto-stopped */
+       if (!(val & TMC_FFSR_FT_STOPPED)) {
+               dev_info(&csdev->dev,
+                        "%s: Triggering manual flush\n", __func__);
+               tmc_flush_and_stop(drvdata);
+       }

Above comments for etf apply equally to etr.

Ack.

Linu Cherian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel