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 8:31 PM, Jie Gan wrote:
> 
> 
> 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.
> 

Missed for explaining the mode_ctrl bit. Set value 0 to the mode_ctrl 
means reset the counter and set the mode to 0.

We have two modes:

mode = 0:
COUNT[11:0] value represents the approximate number of bytes moved 
between two ASYNC packet requests

mode = 1:
the bits COUNT[11:7] are used as a power of 2. for example, we could 
insert an async packet every 8K data by writing a value 13 to the 
COUNT[11:7] field.

Thanks,
Jie

>>
>>> +    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
>>
>>
>>>
>>
>