On Thu, Nov 27, 2025 at 04:44:53PM +0800, Kuan-Wei Chiu wrote:
[...]
I don't feel it is my place to say whether the etm3x driver should be removed entirely.
Sorry for confusion. Your fix patch is welcome, this is useful no matter if remove the ETMv3 driver or not.
However, if we decide to keep it, I agree that aligning cntr_val_show with the cntr_val_store behavior (using cntr_idx) makes more sense.
Here is my plan for v2:
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c index 762109307b86..77578885e8f3 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c @@ -717,26 +717,19 @@ static DEVICE_ATTR_RW(cntr_rld_event); static ssize_t cntr_val_show(struct device *dev, struct device_attribute *attr, char *buf) {
- int i, ret = 0; u32 val; struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent); struct etm_config *config = &drvdata->config;
if (!coresight_get_mode(drvdata->csdev)) { spin_lock(&drvdata->spinlock);
for (i = 0; i < drvdata->nr_cntr; i++)ret += sprintf(buf, "counter %d: %x\n",i, config->cntr_val[i]);
val = config->cntr_val[config->cntr_idx];
return ret;- }
- for (i = 0; i < drvdata->nr_cntr; i++) {
val = etm_readl(drvdata, ETMCNTVRn(i));ret += sprintf(buf, "counter %d: %x\n", i, val);
return sprintf(buf, "%x\n", val);
- return ret;
- val = etm_readl(drvdata, ETMCNTVRn(config->cntr_idx));
It is not right to read register at here (it cannot promise to read the CPU (cp14) register on the target CPU).
Please refer to the same function in coresight-etm4x-sysfs.c. I think we can do the same thing at here.
- return sprintf(buf, "%x\n", val);
} static ssize_t cntr_val_store(struct device *dev,
Given the upcoming merge window, I plan to submit this v2 after -rc1 is released.
Alternatively, if the consensus is to drop the driver, I am happy to submit a patch for that instead.
Please continue this patch. Thanks a lot!
Leo