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

On 05/01/2024 16:11, Mike Leach wrote:

Hi,

On Fri, 5 Jan 2024 at 05:59, Linu Cherian lcherian@marvell.com wrote:

...

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 lcherian@marvell.com

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) writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE); 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

...

     writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);

@@ -615,7 +615,13 @@ static int tmc_panic_sync_etf(struct coresight_device *csdev) 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?

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.

Or even a new sysfs handle to turn this on ? e.g.,

/sys/bus/coresight/devices/tmc_etfN/stop_on_flush

That can be honoured only in the sysfs mode and set accordingly by the user ?

e.g.,

for f in $(find /sys/bus/coresight/devices/ -maxdepth 1 -name tmc_*) do echo 1 > $f/stop_on_flush done

Suzuki

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

...

     /* 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) 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.

Regards

Mike

...

     /* Sync registers from hardware to metadata region */
     mdata->size = csdev_access_relaxed_read32(csa, TMC_RSZ);

diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 6e1e910d5ea4..cf9313b302c7 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -77,6 +77,9 @@ #define TMC_AXICTL_AXCACHE_OS (0xf << 2) #define TMC_AXICTL_ARCACHE_OS (0xf << 16)

+/* TMC_FFSR - 0x300 */ +#define TMC_FFSR_FT_STOPPED BIT(1)

• /* TMC_FFCR - 0x304 */ #define TMC_FFCR_FLUSHMAN_BIT 6 #define TMC_FFCR_EN_FMT BIT(0)

-- 2.34.1

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