Re: [PATCH v5 2/3] coresight: tpda: add logic to configure TPDA_SYNCR register

[Jie Gan <jie.gan@oss.qualcomm.com>]

On 12/19/2025 6:24 PM, Suzuki K Poulose wrote:
> On 19/12/2025 10:04, Jie Gan wrote:
>> From: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>
>> The TPDA_SYNC counter tracks the number of bytes transferred from the
>> aggregator. When this count reaches the value programmed in the
>> TPDA_SYNCR register, an ASYNC request is triggered, allowing userspace
>> tools to accurately parse each valid packet.
>>
>> Signed-off-by: Tao Zhang <tao.zhang@oss.qualcomm.com>
>> Reviewed-by: James Clark <james.clark@linaro.org>
>> Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-tpda.c | 7 +++++++
>>   drivers/hwtracing/coresight/coresight-tpda.h | 5 +++++
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/ 
>> hwtracing/coresight/coresight-tpda.c
>> index d25a8bcfb3d4..d378ff8ad77d 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>> @@ -163,6 +163,13 @@ static void tpda_enable_pre_port(struct 
>> tpda_drvdata *drvdata)
>>        */
>>       if (drvdata->trig_flag_ts)
>>           writel_relaxed(0x0, drvdata->base + TPDA_FPID_CR);
>> +
>> +    val = readl_relaxed(drvdata->base + TPDA_SYNCR);
>> +    /* Reset the mode ctrl */
>> +    val &= ~TPDA_SYNCR_MODE_CTRL;
>> +    /* Program the counter value for TPDA_SYNCR */
>> +    val |= TPDA_SYNCR_COUNTER_MASK;
> 
> Do we plan to change this value via sysfs ? If not whats the point of
> clearing the field. Why not simply set it (as it is all 1s anyways).

We configure the value only once during enablement and do not intend to 
modify it through sysfs. The mode_ctrl is reset to reinitialize the counter.
For example, if the TPDA device has been enabled and then disabled, the 
counter may have already counted some packets. This could result in an 
async packet being triggered with an unexpected data size.

> 
>> +    writel_relaxed(val, drvdata->base + TPDA_SYNCR);
>>   }
>>   static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/ 
>> hwtracing/coresight/coresight-tpda.h
>> index 8a075cfbc3cc..97e2729c15c9 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpda.h
>> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
>> @@ -9,6 +9,7 @@
>>   #define TPDA_CR            (0x000)
>>   #define TPDA_Pn_CR(n)        (0x004 + (n * 4))
>>   #define TPDA_FPID_CR        (0x084)
>> +#define TPDA_SYNCR        (0x08C)
>>   /* Cross trigger Global (all ports) flush request bit */
>>   #define TPDA_CR_FLREQ        BIT(0)
>> @@ -38,6 +39,10 @@
>>   #define TPDA_Pn_CR_CMBSIZE        GENMASK(7, 6)
>>   /* Aggregator port DSB data set element size bit */
>>   #define TPDA_Pn_CR_DSBSIZE        BIT(8)
> 
> Newline to separate the defintions of different registers, please.

will fix it.

Thanks,
Jie

> 
>> +/* TPDA_SYNCR mode control bit */
>> +#define TPDA_SYNCR_MODE_CTRL        BIT(12)
>> +/* TPDA_SYNCR counter mask */
>> +#define TPDA_SYNCR_COUNTER_MASK        GENMASK(11, 0)
>>   #define TPDA_MAX_INPORTS    32
> 
> Suzuki
> 
> 
>>
>