From: Tao Zhang <tao.zhang@oss.qualcomm.com>
Setting bit i in the TPDA_FLUSH_CR register initiates a flush request
for port i, forcing the data to synchronize and be transmitted to the
sink device.
Signed-off-by: Tao Zhang <tao.zhang@oss.qualcomm.com>
Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
.../testing/sysfs-bus-coresight-devices-tpda | 7 +++
drivers/hwtracing/coresight/coresight-tpda.c | 45 +++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpda.h | 1 +
3 files changed, 53 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
index e827396a0fa1..8803158ba42f 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
@@ -41,3 +41,10 @@ Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qu
Description:
(RW) Configure the CMB/MCMB channel mode for all enabled ports.
Value 0 means raw channel mapping mode. Value 1 means channel pair marking mode.
+
+What: /sys/bus/coresight/devices/<tpda-name>/port_flush_req
+Date: August 2025
+KernelVersion: 6.17
+Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qualcomm.com>, Jie Gan <jie.gan@oss.qualcomm.com>
+Description:
+ (RW) Configure the bit i to requests a flush operation of port i on the TPDA.
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 9e623732d1e7..c5f169facc51 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct device *dev,
}
static DEVICE_ATTR_RW(cmbchan_mode);
+static ssize_t port_flush_req_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ unsigned long val;
+
+ guard(spinlock)(&drvdata->spinlock);
+ if (!drvdata->csdev->refcnt)
+ return -EPERM;
+
+ val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
+ return sysfs_emit(buf, "%lx\n", val);
+}
+
+static ssize_t port_flush_req_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t size)
+{
+ struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ unsigned long val;
+
+ if (kstrtoul(buf, 0, &val))
+ return -EINVAL;
+
+ /* The valid value ranges from 0 to 127 */
+ if (val > 127)
+ return -EINVAL;
+
+ guard(spinlock)(&drvdata->spinlock);
+ if (!drvdata->csdev->refcnt)
+ return -EPERM;
+
+ if (val) {
+ CS_UNLOCK(drvdata->base);
+ writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
+ CS_LOCK(drvdata->base);
+ }
+
+ return size;
+}
+static DEVICE_ATTR_RW(port_flush_req);
+
static struct attribute *tpda_attrs[] = {
&dev_attr_trig_async_enable.attr,
&dev_attr_trig_flag_ts_enable.attr,
@@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = {
&dev_attr_freq_ts_enable.attr,
&dev_attr_global_flush_req.attr,
&dev_attr_cmbchan_mode.attr,
+ &dev_attr_port_flush_req.attr,
NULL,
};
diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h
index 00d146960d81..55a18d718357 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.h
+++ b/drivers/hwtracing/coresight/coresight-tpda.h
@@ -10,6 +10,7 @@
#define TPDA_Pn_CR(n) (0x004 + (n * 4))
#define TPDA_FPID_CR (0x084)
#define TPDA_SYNCR (0x08C)
+#define TPDA_FLUSH_CR (0x090)
/* Cross trigger FREQ packets timestamp bit */
#define TPDA_CR_FREQTS BIT(2)
--
2.34.1
On 26/08/2025 8:01 am, Jie Gan wrote:
> From: Tao Zhang <tao.zhang@oss.qualcomm.com>
>
> Setting bit i in the TPDA_FLUSH_CR register initiates a flush request
> for port i, forcing the data to synchronize and be transmitted to the
> sink device.
>
> Signed-off-by: Tao Zhang <tao.zhang@oss.qualcomm.com>
> Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
> .../testing/sysfs-bus-coresight-devices-tpda | 7 +++
> drivers/hwtracing/coresight/coresight-tpda.c | 45 +++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tpda.h | 1 +
> 3 files changed, 53 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
> index e827396a0fa1..8803158ba42f 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
> @@ -41,3 +41,10 @@ Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qu
> Description:
> (RW) Configure the CMB/MCMB channel mode for all enabled ports.
> Value 0 means raw channel mapping mode. Value 1 means channel pair marking mode.
> +
> +What: /sys/bus/coresight/devices/<tpda-name>/port_flush_req
> +Date: August 2025
> +KernelVersion: 6.17
> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qualcomm.com>, Jie Gan <jie.gan@oss.qualcomm.com>
> +Description:
> + (RW) Configure the bit i to requests a flush operation of port i on the TPDA.
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index 9e623732d1e7..c5f169facc51 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct device *dev,
> }
> static DEVICE_ATTR_RW(cmbchan_mode);
>
> +static ssize_t port_flush_req_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
> + unsigned long val;
> +
> + guard(spinlock)(&drvdata->spinlock);
> + if (!drvdata->csdev->refcnt)
> + return -EPERM;
> +
> + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
> + return sysfs_emit(buf, "%lx\n", val);
Decimal would be better for a port number that goes from 0 - 127. If you
really want to use hex then don't you need to prefix it with 0x?
Otherwise you can't tell the difference between decimal 10 and hex 10,
and it's not documented that it's hex either.
> +}
> +
> +static ssize_t port_flush_req_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t size)
> +{
> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
> + unsigned long val;
> +
> + if (kstrtoul(buf, 0, &val))
> + return -EINVAL;
> +
> + /* The valid value ranges from 0 to 127 */
> + if (val > 127)
> + return -EINVAL;
> +
> + guard(spinlock)(&drvdata->spinlock);
> + if (!drvdata->csdev->refcnt)
> + return -EPERM;
> +
> + if (val) {
If 0 - 127 are valid don't you want to write 0 too?
> + CS_UNLOCK(drvdata->base);
> + writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
> + CS_LOCK(drvdata->base);
> + }
> +
> + return size;
> +}
> +static DEVICE_ATTR_RW(port_flush_req);
> +
> static struct attribute *tpda_attrs[] = {
> &dev_attr_trig_async_enable.attr,
> &dev_attr_trig_flag_ts_enable.attr,
> @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = {
> &dev_attr_freq_ts_enable.attr,
> &dev_attr_global_flush_req.attr,
> &dev_attr_cmbchan_mode.attr,
> + &dev_attr_port_flush_req.attr,
> NULL,
> };
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h
> index 00d146960d81..55a18d718357 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.h
> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
> @@ -10,6 +10,7 @@
> #define TPDA_Pn_CR(n) (0x004 + (n * 4))
> #define TPDA_FPID_CR (0x084)
> #define TPDA_SYNCR (0x08C)
> +#define TPDA_FLUSH_CR (0x090)
>
> /* Cross trigger FREQ packets timestamp bit */
> #define TPDA_CR_FREQTS BIT(2)
On 8/26/2025 5:27 PM, James Clark wrote:
>
>
> On 26/08/2025 8:01 am, Jie Gan wrote:
>> From: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>
>> Setting bit i in the TPDA_FLUSH_CR register initiates a flush request
>> for port i, forcing the data to synchronize and be transmitted to the
>> sink device.
>>
>> Signed-off-by: Tao Zhang <tao.zhang@oss.qualcomm.com>
>> Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>> .../testing/sysfs-bus-coresight-devices-tpda | 7 +++
>> drivers/hwtracing/coresight/coresight-tpda.c | 45 +++++++++++++++++++
>> drivers/hwtracing/coresight/coresight-tpda.h | 1 +
>> 3 files changed, 53 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-
>> tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>> index e827396a0fa1..8803158ba42f 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>> @@ -41,3 +41,10 @@ Contact: Jinlong Mao
>> <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qu
>> Description:
>> (RW) Configure the CMB/MCMB channel mode for all enabled ports.
>> Value 0 means raw channel mapping mode. Value 1 means
>> channel pair marking mode.
>> +
>> +What: /sys/bus/coresight/devices/<tpda-name>/port_flush_req
>> +Date: August 2025
>> +KernelVersion: 6.17
>> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang
>> <tao.zhang@oss.qualcomm.com>, Jie Gan <jie.gan@oss.qualcomm.com>
>> +Description:
>> + (RW) Configure the bit i to requests a flush operation of
>> port i on the TPDA.
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/
>> hwtracing/coresight/coresight-tpda.c
>> index 9e623732d1e7..c5f169facc51 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>> @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct device
>> *dev,
>> }
>> static DEVICE_ATTR_RW(cmbchan_mode);
>> +static ssize_t port_flush_req_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + unsigned long val;
>> +
>> + guard(spinlock)(&drvdata->spinlock);
>> + if (!drvdata->csdev->refcnt)
>> + return -EPERM;
>> +
>> + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
>> + return sysfs_emit(buf, "%lx\n", val);
>
> Decimal would be better for a port number that goes from 0 - 127. If you
> really want to use hex then don't you need to prefix it with 0x?
> Otherwise you can't tell the difference between decimal 10 and hex 10,
> and it's not documented that it's hex either.
>
Got it. I will fix the code here, and update the description in document.
>> +}
>> +
>> +static ssize_t port_flush_req_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf,
>> + size_t size)
>> +{
>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + unsigned long val;
>> +
>> + if (kstrtoul(buf, 0, &val))
>> + return -EINVAL;
>> +
>> + /* The valid value ranges from 0 to 127 */
>> + if (val > 127)
>> + return -EINVAL;
>> +
>> + guard(spinlock)(&drvdata->spinlock);
>> + if (!drvdata->csdev->refcnt)
>> + return -EPERM;
>> +
>> + if (val) {
>
> If 0 - 127 are valid don't you want to write 0 too?
It's 1-127 here. 0 may leads to an unexpected issue here.
Thanks,
Jie
>
>> + CS_UNLOCK(drvdata->base);
>> + writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
>> + CS_LOCK(drvdata->base);
>> + }
>> +
>> + return size;
>> +}
>> +static DEVICE_ATTR_RW(port_flush_req);
>> +
>> static struct attribute *tpda_attrs[] = {
>> &dev_attr_trig_async_enable.attr,
>> &dev_attr_trig_flag_ts_enable.attr,
>> @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = {
>> &dev_attr_freq_ts_enable.attr,
>> &dev_attr_global_flush_req.attr,
>> &dev_attr_cmbchan_mode.attr,
>> + &dev_attr_port_flush_req.attr,
>> NULL,
>> };
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/
>> hwtracing/coresight/coresight-tpda.h
>> index 00d146960d81..55a18d718357 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpda.h
>> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
>> @@ -10,6 +10,7 @@
>> #define TPDA_Pn_CR(n) (0x004 + (n * 4))
>> #define TPDA_FPID_CR (0x084)
>> #define TPDA_SYNCR (0x08C)
>> +#define TPDA_FLUSH_CR (0x090)
>> /* Cross trigger FREQ packets timestamp bit */
>> #define TPDA_CR_FREQTS BIT(2)
>
>
On 26/08/2025 10:39 am, Jie Gan wrote:
>
>
> On 8/26/2025 5:27 PM, James Clark wrote:
>>
>>
>> On 26/08/2025 8:01 am, Jie Gan wrote:
>>> From: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>>
>>> Setting bit i in the TPDA_FLUSH_CR register initiates a flush request
>>> for port i, forcing the data to synchronize and be transmitted to the
>>> sink device.
>>>
>>> Signed-off-by: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>> Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>> ---
>>> .../testing/sysfs-bus-coresight-devices-tpda | 7 +++
>>> drivers/hwtracing/coresight/coresight-tpda.c | 45 +++++++++++++++++++
>>> drivers/hwtracing/coresight/coresight-tpda.h | 1 +
>>> 3 files changed, 53 insertions(+)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-
>>> tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>> index e827396a0fa1..8803158ba42f 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>> @@ -41,3 +41,10 @@ Contact: Jinlong Mao
>>> <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qu
>>> Description:
>>> (RW) Configure the CMB/MCMB channel mode for all enabled
>>> ports.
>>> Value 0 means raw channel mapping mode. Value 1 means
>>> channel pair marking mode.
>>> +
>>> +What: /sys/bus/coresight/devices/<tpda-name>/port_flush_req
>>> +Date: August 2025
>>> +KernelVersion: 6.17
>>> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang
>>> <tao.zhang@oss.qualcomm.com>, Jie Gan <jie.gan@oss.qualcomm.com>
>>> +Description:
>>> + (RW) Configure the bit i to requests a flush operation of
>>> port i on the TPDA.
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/
>>> hwtracing/coresight/coresight-tpda.c
>>> index 9e623732d1e7..c5f169facc51 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>>> @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct device
>>> *dev,
>>> }
>>> static DEVICE_ATTR_RW(cmbchan_mode);
>>> +static ssize_t port_flush_req_show(struct device *dev,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> + unsigned long val;
>>> +
>>> + guard(spinlock)(&drvdata->spinlock);
>>> + if (!drvdata->csdev->refcnt)
>>> + return -EPERM;
>>> +
>>> + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
>>> + return sysfs_emit(buf, "%lx\n", val);
>>
>> Decimal would be better for a port number that goes from 0 - 127. If
>> you really want to use hex then don't you need to prefix it with 0x?
>> Otherwise you can't tell the difference between decimal 10 and hex 10,
>> and it's not documented that it's hex either.
>>
>
> Got it. I will fix the code here, and update the description in document.
>
>>> +}
>>> +
>>> +static ssize_t port_flush_req_store(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf,
>>> + size_t size)
>>> +{
>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> + unsigned long val;
>>> +
>>> + if (kstrtoul(buf, 0, &val))
>>> + return -EINVAL;
>>> +
>>> + /* The valid value ranges from 0 to 127 */
>>> + if (val > 127)
>>> + return -EINVAL;
>>> +
>>> + guard(spinlock)(&drvdata->spinlock);
>>> + if (!drvdata->csdev->refcnt)
>>> + return -EPERM;
>>> +
>>> + if (val) {
>>
>> If 0 - 127 are valid don't you want to write 0 too?
>
> It's 1-127 here. 0 may leads to an unexpected issue here.
>
> Thanks,
> Jie
>
Then can't the above be this:
/* The valid value ranges from 1 to 127 */
if (val < 1 || val > 127)
return -EINVAL;
But I'm wondering how you flush port 0?
Isn't the default value 0? So if you never write to port_flush_req then
you'd flush port 0, but why can't you change it back to 0 after writing
a different value?
>>
>>> + CS_UNLOCK(drvdata->base);
>>> + writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
>>> + CS_LOCK(drvdata->base);
>>> + }
>>> +
>>> + return size;
>>> +}
>>> +static DEVICE_ATTR_RW(port_flush_req);
>>> +
>>> static struct attribute *tpda_attrs[] = {
>>> &dev_attr_trig_async_enable.attr,
>>> &dev_attr_trig_flag_ts_enable.attr,
>>> @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = {
>>> &dev_attr_freq_ts_enable.attr,
>>> &dev_attr_global_flush_req.attr,
>>> &dev_attr_cmbchan_mode.attr,
>>> + &dev_attr_port_flush_req.attr,
>>> NULL,
>>> };
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/
>>> hwtracing/coresight/coresight-tpda.h
>>> index 00d146960d81..55a18d718357 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpda.h
>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
>>> @@ -10,6 +10,7 @@
>>> #define TPDA_Pn_CR(n) (0x004 + (n * 4))
>>> #define TPDA_FPID_CR (0x084)
>>> #define TPDA_SYNCR (0x08C)
>>> +#define TPDA_FLUSH_CR (0x090)
>>> /* Cross trigger FREQ packets timestamp bit */
>>> #define TPDA_CR_FREQTS BIT(2)
>>
>>
>
On 8/26/2025 5:54 PM, James Clark wrote:
>
>
> On 26/08/2025 10:39 am, Jie Gan wrote:
>>
>>
>> On 8/26/2025 5:27 PM, James Clark wrote:
>>>
>>>
>>> On 26/08/2025 8:01 am, Jie Gan wrote:
>>>> From: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>>>
>>>> Setting bit i in the TPDA_FLUSH_CR register initiates a flush request
>>>> for port i, forcing the data to synchronize and be transmitted to the
>>>> sink device.
>>>>
>>>> Signed-off-by: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>>> Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>>> ---
>>>> .../testing/sysfs-bus-coresight-devices-tpda | 7 +++
>>>> drivers/hwtracing/coresight/coresight-tpda.c | 45 +++++++++++++++
>>>> ++++
>>>> drivers/hwtracing/coresight/coresight-tpda.h | 1 +
>>>> 3 files changed, 53 insertions(+)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-
>>>> tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>>> index e827396a0fa1..8803158ba42f 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>>> @@ -41,3 +41,10 @@ Contact: Jinlong Mao
>>>> <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qu
>>>> Description:
>>>> (RW) Configure the CMB/MCMB channel mode for all enabled
>>>> ports.
>>>> Value 0 means raw channel mapping mode. Value 1 means
>>>> channel pair marking mode.
>>>> +
>>>> +What: /sys/bus/coresight/devices/<tpda-name>/port_flush_req
>>>> +Date: August 2025
>>>> +KernelVersion: 6.17
>>>> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang
>>>> <tao.zhang@oss.qualcomm.com>, Jie Gan <jie.gan@oss.qualcomm.com>
>>>> +Description:
>>>> + (RW) Configure the bit i to requests a flush operation of
>>>> port i on the TPDA.
>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/
>>>> hwtracing/coresight/coresight-tpda.c
>>>> index 9e623732d1e7..c5f169facc51 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>>>> @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct device
>>>> *dev,
>>>> }
>>>> static DEVICE_ATTR_RW(cmbchan_mode);
>>>> +static ssize_t port_flush_req_show(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + char *buf)
>>>> +{
>>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>> + unsigned long val;
>>>> +
>>>> + guard(spinlock)(&drvdata->spinlock);
>>>> + if (!drvdata->csdev->refcnt)
>>>> + return -EPERM;
>>>> +
>>>> + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
>>>> + return sysfs_emit(buf, "%lx\n", val);
>>>
>>> Decimal would be better for a port number that goes from 0 - 127. If
>>> you really want to use hex then don't you need to prefix it with 0x?
>>> Otherwise you can't tell the difference between decimal 10 and hex
>>> 10, and it's not documented that it's hex either.
>>>
>>
>> Got it. I will fix the code here, and update the description in document.
>>
>>>> +}
>>>> +
>>>> +static ssize_t port_flush_req_store(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + const char *buf,
>>>> + size_t size)
>>>> +{
>>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>> + unsigned long val;
>>>> +
>>>> + if (kstrtoul(buf, 0, &val))
>>>> + return -EINVAL;
>>>> +
>>>> + /* The valid value ranges from 0 to 127 */
>>>> + if (val > 127)
>>>> + return -EINVAL;
>>>> +
>>>> + guard(spinlock)(&drvdata->spinlock);
>>>> + if (!drvdata->csdev->refcnt)
>>>> + return -EPERM;
>>>> +
>>>> + if (val) {
>>>
>>> If 0 - 127 are valid don't you want to write 0 too?
>>
>> It's 1-127 here. 0 may leads to an unexpected issue here.
>>
>> Thanks,
>> Jie
>>
>
> Then can't the above be this:
>
> /* The valid value ranges from 1 to 127 */
> if (val < 1 || val > 127)
> return -EINVAL;
>
> But I'm wondering how you flush port 0?
>
BIT(0) represents port 0 with value 1 and the default value 0 means
nothing will be triggered here.
> Isn't the default value 0? So if you never write to port_flush_req then
> you'd flush port 0, but why can't you change it back to 0 after writing
> a different value?
We can change the value back to 0 but I think we shouldn't do this
although I haven't suffer issue after I changed it back to 0(for bit).
Because the document mentioned: "Once set, the bit remains set until the
flush operation on port i completes and the bit then clears to 0". So I
think we should let the flush operation finish as expected and clear the
bit by itself? Or may suffer unexpected error when try to interrupt the
flush operation?
Thanks,
Jie
>>>
>>>> + CS_UNLOCK(drvdata->base);
>>>> + writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
>>>> + CS_LOCK(drvdata->base);
>>>> + }
>>>> +
>>>> + return size;
>>>> +}
>>>> +static DEVICE_ATTR_RW(port_flush_req);
>>>> +
>>>> static struct attribute *tpda_attrs[] = {
>>>> &dev_attr_trig_async_enable.attr,
>>>> &dev_attr_trig_flag_ts_enable.attr,
>>>> @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = {
>>>> &dev_attr_freq_ts_enable.attr,
>>>> &dev_attr_global_flush_req.attr,
>>>> &dev_attr_cmbchan_mode.attr,
>>>> + &dev_attr_port_flush_req.attr,
>>>> NULL,
>>>> };
>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/
>>>> hwtracing/coresight/coresight-tpda.h
>>>> index 00d146960d81..55a18d718357 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-tpda.h
>>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
>>>> @@ -10,6 +10,7 @@
>>>> #define TPDA_Pn_CR(n) (0x004 + (n * 4))
>>>> #define TPDA_FPID_CR (0x084)
>>>> #define TPDA_SYNCR (0x08C)
>>>> +#define TPDA_FLUSH_CR (0x090)
>>>> /* Cross trigger FREQ packets timestamp bit */
>>>> #define TPDA_CR_FREQTS BIT(2)
>>>
>>>
>>
>
>
On 26/08/2025 1:11 pm, Jie Gan wrote:
>
>
> On 8/26/2025 5:54 PM, James Clark wrote:
>>
>>
>> On 26/08/2025 10:39 am, Jie Gan wrote:
>>>
>>>
>>> On 8/26/2025 5:27 PM, James Clark wrote:
>>>>
>>>>
>>>> On 26/08/2025 8:01 am, Jie Gan wrote:
>>>>> From: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>>>>
>>>>> Setting bit i in the TPDA_FLUSH_CR register initiates a flush request
>>>>> for port i, forcing the data to synchronize and be transmitted to the
>>>>> sink device.
>>>>>
>>>>> Signed-off-by: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>>>> Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>>>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>>>> ---
>>>>> .../testing/sysfs-bus-coresight-devices-tpda | 7 +++
>>>>> drivers/hwtracing/coresight/coresight-tpda.c | 45 ++++++++++++++
>>>>> + ++++
>>>>> drivers/hwtracing/coresight/coresight-tpda.h | 1 +
>>>>> 3 files changed, 53 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-
>>>>> tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>>>> index e827396a0fa1..8803158ba42f 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>>>> @@ -41,3 +41,10 @@ Contact: Jinlong Mao
>>>>> <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qu
>>>>> Description:
>>>>> (RW) Configure the CMB/MCMB channel mode for all enabled
>>>>> ports.
>>>>> Value 0 means raw channel mapping mode. Value 1 means
>>>>> channel pair marking mode.
>>>>> +
>>>>> +What: /sys/bus/coresight/devices/<tpda-name>/port_flush_req
>>>>> +Date: August 2025
>>>>> +KernelVersion: 6.17
>>>>> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang
>>>>> <tao.zhang@oss.qualcomm.com>, Jie Gan <jie.gan@oss.qualcomm.com>
>>>>> +Description:
>>>>> + (RW) Configure the bit i to requests a flush operation of
>>>>> port i on the TPDA.
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/
>>>>> drivers/ hwtracing/coresight/coresight-tpda.c
>>>>> index 9e623732d1e7..c5f169facc51 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>>>>> @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct
>>>>> device *dev,
>>>>> }
>>>>> static DEVICE_ATTR_RW(cmbchan_mode);
>>>>> +static ssize_t port_flush_req_show(struct device *dev,
>>>>> + struct device_attribute *attr,
>>>>> + char *buf)
>>>>> +{
>>>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>> + unsigned long val;
>>>>> +
>>>>> + guard(spinlock)(&drvdata->spinlock);
>>>>> + if (!drvdata->csdev->refcnt)
>>>>> + return -EPERM;
>>>>> +
>>>>> + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
>>>>> + return sysfs_emit(buf, "%lx\n", val);
>>>>
>>>> Decimal would be better for a port number that goes from 0 - 127. If
>>>> you really want to use hex then don't you need to prefix it with 0x?
>>>> Otherwise you can't tell the difference between decimal 10 and hex
>>>> 10, and it's not documented that it's hex either.
>>>>
>>>
>>> Got it. I will fix the code here, and update the description in
>>> document.
>>>
>>>>> +}
>>>>> +
>>>>> +static ssize_t port_flush_req_store(struct device *dev,
>>>>> + struct device_attribute *attr,
>>>>> + const char *buf,
>>>>> + size_t size)
>>>>> +{
>>>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>> + unsigned long val;
>>>>> +
>>>>> + if (kstrtoul(buf, 0, &val))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + /* The valid value ranges from 0 to 127 */
>>>>> + if (val > 127)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + guard(spinlock)(&drvdata->spinlock);
>>>>> + if (!drvdata->csdev->refcnt)
>>>>> + return -EPERM;
>>>>> +
>>>>> + if (val) {
>>>>
>>>> If 0 - 127 are valid don't you want to write 0 too?
>>>
>>> It's 1-127 here. 0 may leads to an unexpected issue here.
>>>
>>> Thanks,
>>> Jie
>>>
>>
>> Then can't the above be this:
>>
>> /* The valid value ranges from 1 to 127 */
>> if (val < 1 || val > 127)
>> return -EINVAL;
>>
>> But I'm wondering how you flush port 0?
>>
>
> BIT(0) represents port 0 with value 1 and the default value 0 means
> nothing will be triggered here.
>
>> Isn't the default value 0? So if you never write to port_flush_req
>> then you'd flush port 0, but why can't you change it back to 0 after
>> writing a different value?
>
> We can change the value back to 0 but I think we shouldn't do this
> although I haven't suffer issue after I changed it back to 0(for bit).
> Because the document mentioned: "Once set, the bit remains set until the
> flush operation on port i completes and the bit then clears to 0". So I
> think we should let the flush operation finish as expected and clear the
> bit by itself? Or may suffer unexpected error when try to interrupt the
> flush operation?
>
> Thanks,
> Jie
Oh I see, I thought this was a port number, not a bit for each port.
That changes this and my other comment about changing the output to be
decimal then. Hex is probably better but it needs the 0x prefix.
I would also treat 0 as EINVAL. It doesn't do anything different to any
other out of range request so it should be treated the same way.
Then comparing to 127 isn't that obvious either. Something like
FIELD_FITS() more clearly states that values have to fit into a bitfield
rather than be less than some value:
if (!val || !FIELD_FIT(TPDA_FLUSH_CR_PORTNUM, val))
return -EINVAL;
> >>>
>>>>> + CS_UNLOCK(drvdata->base);
>>>>> + writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
>>>>> + CS_LOCK(drvdata->base);
>>>>> + }
>>>>> +
>>>>> + return size;
>>>>> +}
>>>>> +static DEVICE_ATTR_RW(port_flush_req);
>>>>> +
>>>>> static struct attribute *tpda_attrs[] = {
>>>>> &dev_attr_trig_async_enable.attr,
>>>>> &dev_attr_trig_flag_ts_enable.attr,
>>>>> @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = {
>>>>> &dev_attr_freq_ts_enable.attr,
>>>>> &dev_attr_global_flush_req.attr,
>>>>> &dev_attr_cmbchan_mode.attr,
>>>>> + &dev_attr_port_flush_req.attr,
>>>>> NULL,
>>>>> };
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/
>>>>> drivers/ hwtracing/coresight/coresight-tpda.h
>>>>> index 00d146960d81..55a18d718357 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-tpda.h
>>>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
>>>>> @@ -10,6 +10,7 @@
>>>>> #define TPDA_Pn_CR(n) (0x004 + (n * 4))
>>>>> #define TPDA_FPID_CR (0x084)
>>>>> #define TPDA_SYNCR (0x08C)
>>>>> +#define TPDA_FLUSH_CR (0x090)
>>>>> /* Cross trigger FREQ packets timestamp bit */
>>>>> #define TPDA_CR_FREQTS BIT(2)
>>>>
>>>>
>>>
>>
>>
>
On 8/26/2025 9:29 PM, James Clark wrote:
>
>
> On 26/08/2025 1:11 pm, Jie Gan wrote:
>>
>>
>> On 8/26/2025 5:54 PM, James Clark wrote:
>>>
>>>
>>> On 26/08/2025 10:39 am, Jie Gan wrote:
>>>>
>>>>
>>>> On 8/26/2025 5:27 PM, James Clark wrote:
>>>>>
>>>>>
>>>>> On 26/08/2025 8:01 am, Jie Gan wrote:
>>>>>> From: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>>>>>
>>>>>> Setting bit i in the TPDA_FLUSH_CR register initiates a flush request
>>>>>> for port i, forcing the data to synchronize and be transmitted to the
>>>>>> sink device.
>>>>>>
>>>>>> Signed-off-by: Tao Zhang <tao.zhang@oss.qualcomm.com>
>>>>>> Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>>>>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>>>>> ---
>>>>>> .../testing/sysfs-bus-coresight-devices-tpda | 7 +++
>>>>>> drivers/hwtracing/coresight/coresight-tpda.c | 45 +++++++++++++
>>>>>> + + ++++
>>>>>> drivers/hwtracing/coresight/coresight-tpda.h | 1 +
>>>>>> 3 files changed, 53 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-
>>>>>> devices- tpda b/Documentation/ABI/testing/sysfs-bus-coresight-
>>>>>> devices-tpda
>>>>>> index e827396a0fa1..8803158ba42f 100644
>>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
>>>>>> @@ -41,3 +41,10 @@ Contact: Jinlong Mao
>>>>>> <jinlong.mao@oss.qualcomm.com>, Tao Zhang <tao.zhang@oss.qu
>>>>>> Description:
>>>>>> (RW) Configure the CMB/MCMB channel mode for all enabled
>>>>>> ports.
>>>>>> Value 0 means raw channel mapping mode. Value 1 means
>>>>>> channel pair marking mode.
>>>>>> +
>>>>>> +What: /sys/bus/coresight/devices/<tpda-name>/port_flush_req
>>>>>> +Date: August 2025
>>>>>> +KernelVersion: 6.17
>>>>>> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Tao Zhang
>>>>>> <tao.zhang@oss.qualcomm.com>, Jie Gan <jie.gan@oss.qualcomm.com>
>>>>>> +Description:
>>>>>> + (RW) Configure the bit i to requests a flush operation of
>>>>>> port i on the TPDA.
>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/
>>>>>> drivers/ hwtracing/coresight/coresight-tpda.c
>>>>>> index 9e623732d1e7..c5f169facc51 100644
>>>>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>>>>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>>>>>> @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct
>>>>>> device *dev,
>>>>>> }
>>>>>> static DEVICE_ATTR_RW(cmbchan_mode);
>>>>>> +static ssize_t port_flush_req_show(struct device *dev,
>>>>>> + struct device_attribute *attr,
>>>>>> + char *buf)
>>>>>> +{
>>>>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>>> + unsigned long val;
>>>>>> +
>>>>>> + guard(spinlock)(&drvdata->spinlock);
>>>>>> + if (!drvdata->csdev->refcnt)
>>>>>> + return -EPERM;
>>>>>> +
>>>>>> + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
>>>>>> + return sysfs_emit(buf, "%lx\n", val);
>>>>>
>>>>> Decimal would be better for a port number that goes from 0 - 127.
>>>>> If you really want to use hex then don't you need to prefix it with
>>>>> 0x? Otherwise you can't tell the difference between decimal 10 and
>>>>> hex 10, and it's not documented that it's hex either.
>>>>>
>>>>
>>>> Got it. I will fix the code here, and update the description in
>>>> document.
>>>>
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t port_flush_req_store(struct device *dev,
>>>>>> + struct device_attribute *attr,
>>>>>> + const char *buf,
>>>>>> + size_t size)
>>>>>> +{
>>>>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>>> + unsigned long val;
>>>>>> +
>>>>>> + if (kstrtoul(buf, 0, &val))
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + /* The valid value ranges from 0 to 127 */
>>>>>> + if (val > 127)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + guard(spinlock)(&drvdata->spinlock);
>>>>>> + if (!drvdata->csdev->refcnt)
>>>>>> + return -EPERM;
>>>>>> +
>>>>>> + if (val) {
>>>>>
>>>>> If 0 - 127 are valid don't you want to write 0 too?
>>>>
>>>> It's 1-127 here. 0 may leads to an unexpected issue here.
>>>>
>>>> Thanks,
>>>> Jie
>>>>
>>>
>>> Then can't the above be this:
>>>
>>> /* The valid value ranges from 1 to 127 */
>>> if (val < 1 || val > 127)
>>> return -EINVAL;
>>>
>>> But I'm wondering how you flush port 0?
>>>
>>
>> BIT(0) represents port 0 with value 1 and the default value 0 means
>> nothing will be triggered here.
>>
>>> Isn't the default value 0? So if you never write to port_flush_req
>>> then you'd flush port 0, but why can't you change it back to 0 after
>>> writing a different value?
>>
>> We can change the value back to 0 but I think we shouldn't do this
>> although I haven't suffer issue after I changed it back to 0(for bit).
>> Because the document mentioned: "Once set, the bit remains set until
>> the flush operation on port i completes and the bit then clears to 0".
>> So I think we should let the flush operation finish as expected and
>> clear the bit by itself? Or may suffer unexpected error when try to
>> interrupt the flush operation?
>>
>> Thanks,
>> Jie
>
> Oh I see, I thought this was a port number, not a bit for each port.
> That changes this and my other comment about changing the output to be
> decimal then. Hex is probably better but it needs the 0x prefix.
>
> I would also treat 0 as EINVAL. It doesn't do anything different to any
> other out of range request so it should be treated the same way.
>
> Then comparing to 127 isn't that obvious either. Something like
> FIELD_FITS() more clearly states that values have to fit into a bitfield
> rather than be less than some value:
>
> if (!val || !FIELD_FIT(TPDA_FLUSH_CR_PORTNUM, val))
> return -EINVAL;
I found I made a mistake here for value range. 0-127 is for port 0 to
port 6. But the TPDA device could support up to 32 ports, means u32 here.
So the mask here, the TPDA_FLUSH_CR_PORTNUM, should be designed for 32
bits, like 0xffffffff.
Thanks,
Jie
>
>
>> >>>
>>>>>> + CS_UNLOCK(drvdata->base);
>>>>>> + writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
>>>>>> + CS_LOCK(drvdata->base);
>>>>>> + }
>>>>>> +
>>>>>> + return size;
>>>>>> +}
>>>>>> +static DEVICE_ATTR_RW(port_flush_req);
>>>>>> +
>>>>>> static struct attribute *tpda_attrs[] = {
>>>>>> &dev_attr_trig_async_enable.attr,
>>>>>> &dev_attr_trig_flag_ts_enable.attr,
>>>>>> @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = {
>>>>>> &dev_attr_freq_ts_enable.attr,
>>>>>> &dev_attr_global_flush_req.attr,
>>>>>> &dev_attr_cmbchan_mode.attr,
>>>>>> + &dev_attr_port_flush_req.attr,
>>>>>> NULL,
>>>>>> };
>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/
>>>>>> drivers/ hwtracing/coresight/coresight-tpda.h
>>>>>> index 00d146960d81..55a18d718357 100644
>>>>>> --- a/drivers/hwtracing/coresight/coresight-tpda.h
>>>>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
>>>>>> @@ -10,6 +10,7 @@
>>>>>> #define TPDA_Pn_CR(n) (0x004 + (n * 4))
>>>>>> #define TPDA_FPID_CR (0x084)
>>>>>> #define TPDA_SYNCR (0x08C)
>>>>>> +#define TPDA_FLUSH_CR (0x090)
>>>>>> /* Cross trigger FREQ packets timestamp bit */
>>>>>> #define TPDA_CR_FREQTS BIT(2)
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
© 2016 - 2025 Red Hat, Inc.