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)