[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8d37d907-68c3-4e77-995b-bc85a0c3b8f0@arm.com>
Date: Tue, 9 Jan 2024 10:59:00 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Mike Leach <mike.leach@...aro.org>, Linu Cherian <lcherian@...vell.com>
Cc: james.clark@....com, leo.yan@...aro.org,
linux-arm-kernel@...ts.infradead.org, coresight@...ts.linaro.org,
linux-kernel@...r.kernel.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
devicetree@...r.kernel.org, sgoutham@...vell.com, gcherian@...vell.com
Subject: 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@...vell.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@...vell.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
Powered by blists - more mailing lists