Hi Mike, > -----Original Message----- > From: Mike Leach <mike.leach@xxxxxxxxxx> > Sent: Friday, January 5, 2024 9:41 PM > To: Linu Cherian <lcherian@xxxxxxxxxxx> > Cc: suzuki.poulose@xxxxxxx; james.clark@xxxxxxx; leo.yan@xxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; coresight@xxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; > krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; Sunil Kovvuri Goutham > <sgoutham@xxxxxxxxxxx>; George Cherian <gcherian@xxxxxxxxxxx> > 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 <lcherian@xxxxxxxxxxx> 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@xxxxxxxxxxx> > > --- > > 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 > > > Okay, will add a configuration dependency here. > > 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? > 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 > > > /* 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. Ack. Linu Cherian