* [PATCH v3] coresight: etm3x: Fix cntr_val_show() to match cntr_val_store() behavior
@ 2025-12-02 8:26 Kuan-Wei Chiu
2025-12-02 9:26 ` James Clark
0 siblings, 1 reply; 4+ messages in thread
From: Kuan-Wei Chiu @ 2025-12-02 8:26 UTC (permalink / raw)
To: suzuki.poulose
Cc: mike.leach, james.clark, alexander.shishkin, gregkh,
mathieu.poirier, leo.yan, Al.Grant, jserv, marscheng, ericchancf,
milesjiang, nickpan, coresight, linux-arm-kernel, linux-kernel,
Kuan-Wei Chiu, stable
The cntr_val_show() function was intended to print the values of all
counters using a loop. However, due to a buffer overwrite issue with
sprintf(), it effectively only displayed the value of the last counter.
The companion function, cntr_val_store(), allows users to modify a
specific counter selected by 'cntr_idx'. To maintain consistency
between read and write operations and to align with the ETM4x driver
behavior, modify cntr_val_show() to report only the value of the
currently selected counter.
This change removes the loop and the "counter %d:" prefix, printing
only the hexadecimal value. It also adopts sysfs_emit() for standard
sysfs output formatting.
Fixes: a939fc5a71ad ("coresight-etm: add CoreSight ETM/PTM driver")
Cc: stable@vger.kernel.org
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Build test only.
Changes in v3:
- Switch format specifier to %#x to include the 0x prefix.
- Add Cc stable
v2: https://lore.kernel.org/lkml/20251201095228.1905489-1-visitorckw@gmail.com/
.../hwtracing/coresight/coresight-etm3x-sysfs.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 762109307b86..b3c67e96a82a 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];
spin_unlock(&drvdata->spinlock);
- 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);
+ } else {
+ val = etm_readl(drvdata, ETMCNTVRn(config->cntr_idx));
}
- return ret;
+ return sysfs_emit(buf, "%#x\n", val);
}
static ssize_t cntr_val_store(struct device *dev,
--
2.52.0.158.g65b55ccf14-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] coresight: etm3x: Fix cntr_val_show() to match cntr_val_store() behavior
2025-12-02 8:26 [PATCH v3] coresight: etm3x: Fix cntr_val_show() to match cntr_val_store() behavior Kuan-Wei Chiu
@ 2025-12-02 9:26 ` James Clark
2026-02-02 5:09 ` Kuan-Wei Chiu
0 siblings, 1 reply; 4+ messages in thread
From: James Clark @ 2025-12-02 9:26 UTC (permalink / raw)
To: Kuan-Wei Chiu, suzuki.poulose
Cc: mike.leach, alexander.shishkin, gregkh, mathieu.poirier, leo.yan,
Al.Grant, jserv, marscheng, ericchancf, milesjiang, nickpan,
coresight, linux-arm-kernel, linux-kernel, stable
On 02/12/2025 8:26 am, Kuan-Wei Chiu wrote:
> The cntr_val_show() function was intended to print the values of all
> counters using a loop. However, due to a buffer overwrite issue with
> sprintf(), it effectively only displayed the value of the last counter.
>
> The companion function, cntr_val_store(), allows users to modify a
> specific counter selected by 'cntr_idx'. To maintain consistency
> between read and write operations and to align with the ETM4x driver
> behavior, modify cntr_val_show() to report only the value of the
> currently selected counter.
>
> This change removes the loop and the "counter %d:" prefix, printing
> only the hexadecimal value. It also adopts sysfs_emit() for standard
> sysfs output formatting.
>
> Fixes: a939fc5a71ad ("coresight-etm: add CoreSight ETM/PTM driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> ---
> Build test only.
>
> Changes in v3:
> - Switch format specifier to %#x to include the 0x prefix.
> - Add Cc stable
>
> v2: https://lore.kernel.org/lkml/20251201095228.1905489-1-visitorckw@gmail.com/
>
> .../hwtracing/coresight/coresight-etm3x-sysfs.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index 762109307b86..b3c67e96a82a 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];
> spin_unlock(&drvdata->spinlock);
> - 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);
> + } else {
> + val = etm_readl(drvdata, ETMCNTVRn(config->cntr_idx));
> }
>
> - return ret;
> + return sysfs_emit(buf, "%#x\n", val);
> }
>
> static ssize_t cntr_val_store(struct device *dev,
Reviewed-by: James Clark <james.clark@linaro.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] coresight: etm3x: Fix cntr_val_show() to match cntr_val_store() behavior
2025-12-02 9:26 ` James Clark
@ 2026-02-02 5:09 ` Kuan-Wei Chiu
2026-02-02 9:33 ` Suzuki K Poulose
0 siblings, 1 reply; 4+ messages in thread
From: Kuan-Wei Chiu @ 2026-02-02 5:09 UTC (permalink / raw)
To: James Clark
Cc: suzuki.poulose, mike.leach, alexander.shishkin, gregkh,
mathieu.poirier, leo.yan, Al.Grant, jserv, marscheng, ericchancf,
milesjiang, nickpan, coresight, linux-arm-kernel, linux-kernel,
stable
On Tue, Dec 02, 2025 at 09:26:19AM +0000, James Clark wrote:
>
>
> On 02/12/2025 8:26 am, Kuan-Wei Chiu wrote:
> > The cntr_val_show() function was intended to print the values of all
> > counters using a loop. However, due to a buffer overwrite issue with
> > sprintf(), it effectively only displayed the value of the last counter.
> >
> > The companion function, cntr_val_store(), allows users to modify a
> > specific counter selected by 'cntr_idx'. To maintain consistency
> > between read and write operations and to align with the ETM4x driver
> > behavior, modify cntr_val_show() to report only the value of the
> > currently selected counter.
> >
> > This change removes the loop and the "counter %d:" prefix, printing
> > only the hexadecimal value. It also adopts sysfs_emit() for standard
> > sysfs output formatting.
> >
> > Fixes: a939fc5a71ad ("coresight-etm: add CoreSight ETM/PTM driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > ---
> > Build test only.
> >
> > Changes in v3:
> > - Switch format specifier to %#x to include the 0x prefix.
> > - Add Cc stable
> >
> > v2: https://lore.kernel.org/lkml/20251201095228.1905489-1-visitorckw@gmail.com/
> >
> > .../hwtracing/coresight/coresight-etm3x-sysfs.c | 15 ++++-----------
> > 1 file changed, 4 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> > index 762109307b86..b3c67e96a82a 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];
> > spin_unlock(&drvdata->spinlock);
> > - 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);
> > + } else {
> > + val = etm_readl(drvdata, ETMCNTVRn(config->cntr_idx));
> > }
> > - return ret;
> > + return sysfs_emit(buf, "%#x\n", val);
> > }
> > static ssize_t cntr_val_store(struct device *dev,
>
> Reviewed-by: James Clark <james.clark@linaro.org>
>
Thanks for the review!
Is there anything else I need to do for this fix to land?
Regards,
Kuan-Wei
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] coresight: etm3x: Fix cntr_val_show() to match cntr_val_store() behavior
2026-02-02 5:09 ` Kuan-Wei Chiu
@ 2026-02-02 9:33 ` Suzuki K Poulose
0 siblings, 0 replies; 4+ messages in thread
From: Suzuki K Poulose @ 2026-02-02 9:33 UTC (permalink / raw)
To: Kuan-Wei Chiu, James Clark
Cc: mike.leach, alexander.shishkin, gregkh, mathieu.poirier, leo.yan,
Al.Grant, jserv, marscheng, ericchancf, milesjiang, nickpan,
coresight, linux-arm-kernel, linux-kernel, stable
Hello
On 02/02/2026 05:09, Kuan-Wei Chiu wrote:
> On Tue, Dec 02, 2025 at 09:26:19AM +0000, James Clark wrote:
>>
>>
>> On 02/12/2025 8:26 am, Kuan-Wei Chiu wrote:
>>> The cntr_val_show() function was intended to print the values of all
>>> counters using a loop. However, due to a buffer overwrite issue with
>>> sprintf(), it effectively only displayed the value of the last counter.
>>>
>>> The companion function, cntr_val_store(), allows users to modify a
>>> specific counter selected by 'cntr_idx'. To maintain consistency
>>> between read and write operations and to align with the ETM4x driver
>>> behavior, modify cntr_val_show() to report only the value of the
>>> currently selected counter.
>>>
>>> This change removes the loop and the "counter %d:" prefix, printing
>>> only the hexadecimal value. It also adopts sysfs_emit() for standard
>>> sysfs output formatting.
>>>
>>> Fixes: a939fc5a71ad ("coresight-etm: add CoreSight ETM/PTM driver")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>>> ---
>>> Build test only.
>>>
>>> Changes in v3:
>>> - Switch format specifier to %#x to include the 0x prefix.
>>> - Add Cc stable
>>>
>>> v2: https://lore.kernel.org/lkml/20251201095228.1905489-1-visitorckw@gmail.com/
>>>
>>> .../hwtracing/coresight/coresight-etm3x-sysfs.c | 15 ++++-----------
>>> 1 file changed, 4 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
>>> index 762109307b86..b3c67e96a82a 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];
>>> spin_unlock(&drvdata->spinlock);
>>> - 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);
>>> + } else {
>>> + val = etm_readl(drvdata, ETMCNTVRn(config->cntr_idx));
>>> }
>>> - return ret;
>>> + return sysfs_emit(buf, "%#x\n", val);
>>> }
>>> static ssize_t cntr_val_store(struct device *dev,
>>
>> Reviewed-by: James Clark <james.clark@linaro.org>
>>
> Thanks for the review!
> Is there anything else I need to do for this fix to land?
Thanks for the patch, I will queue this for the next release (v7.1).
Suzuki
>
> Regards,
> Kuan-Wei
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-02-02 9:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 8:26 [PATCH v3] coresight: etm3x: Fix cntr_val_show() to match cntr_val_store() behavior Kuan-Wei Chiu
2025-12-02 9:26 ` James Clark
2026-02-02 5:09 ` Kuan-Wei Chiu
2026-02-02 9:33 ` Suzuki K Poulose
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox