* [PATCH v2] coresight: fix missing error code when trace ID is invalid
@ 2026-05-09 0:58 Jie Gan
2026-05-11 8:16 ` James Clark
0 siblings, 1 reply; 3+ messages in thread
From: Jie Gan @ 2026-05-09 0:58 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
Alexander Shishkin, Tingwei Zhang
Cc: coresight, linux-arm-kernel, linux-kernel, Jie Gan
When coresight_path_assign_trace_id() cannot assign a valid trace ID,
coresight_enable_sysfs() takes the err_path goto with ret still 0,
returning success to the caller despite no trace session being started.
Fix this by changing coresight_path_assign_trace_id() to return int.
Move the IS_VALID_CS_TRACE_ID() check inside the function so it returns
-EINVAL on failure and 0 on success. Update coresight_enable_sysfs() to
check the return value directly instead of inspecting path->trace_id
after the call.
The other caller in coresight-etm-perf.c discards the return value and
continues to check path->trace_id via IS_VALID_CS_TRACE_ID() directly.
This is unaffected: on failure path->trace_id is no longer written, so
it remains 0, which IS_VALID_CS_TRACE_ID() rejects the same as before.
Fixes: d87d76d823d1 ("Coresight: Allocate trace ID after building the path")
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
Changes in v2:
- Refactor the coresight_path_assign_trace_id function.
- Link to v1: https://lore.kernel.org/r/20260508-fix-trace-id-error-v1-1-5f11a5456fdf@oss.qualcomm.com
---
drivers/hwtracing/coresight/coresight-core.c | 14 ++++++++++----
drivers/hwtracing/coresight/coresight-priv.h | 2 +-
drivers/hwtracing/coresight/coresight-sysfs.c | 4 ++--
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 46f247f73cf6..cabdc0c72f38 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -739,12 +739,12 @@ static int coresight_get_trace_id(struct coresight_device *csdev,
* Call this after creating the path and before enabling it. This leaves
* the trace ID set on the path, or it remains 0 if it couldn't be assigned.
*/
-void coresight_path_assign_trace_id(struct coresight_path *path,
+int coresight_path_assign_trace_id(struct coresight_path *path,
enum cs_mode mode)
{
struct coresight_device *sink = coresight_get_sink(path);
struct coresight_node *nd;
- int trace_id;
+ int trace_id, ret = -EINVAL;
list_for_each_entry(nd, &path->path_list, link) {
/* Assign a trace ID to the path for the first device that wants to do it */
@@ -755,10 +755,16 @@ void coresight_path_assign_trace_id(struct coresight_path *path,
* Non 0 is either success or fail.
*/
if (trace_id != 0) {
- path->trace_id = trace_id;
- return;
+ if (IS_VALID_CS_TRACE_ID(trace_id)) {
+ path->trace_id = trace_id;
+ ret = 0;
+ }
+
+ return ret;
}
}
+
+ return ret;
}
/**
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 1ea882dffd70..34c7e792adbd 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -153,7 +153,7 @@ int coresight_make_links(struct coresight_device *orig,
void coresight_remove_links(struct coresight_device *orig,
struct coresight_connection *conn);
u32 coresight_get_sink_id(struct coresight_device *csdev);
-void coresight_path_assign_trace_id(struct coresight_path *path,
+int coresight_path_assign_trace_id(struct coresight_path *path,
enum cs_mode mode);
#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index d2a6ed8bcc74..b6a870399e83 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -211,8 +211,8 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
goto out;
}
- coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
- if (!IS_VALID_CS_TRACE_ID(path->trace_id))
+ ret = coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
+ if (ret)
goto err_path;
ret = coresight_enable_path(path, CS_MODE_SYSFS);
---
base-commit: 17c7841d09ee7d33557fd075562d9289b6018c90
change-id: 20260508-fix-trace-id-error-dbfdd4d8f2d1
Best regards,
--
Jie Gan <jie.gan@oss.qualcomm.com>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] coresight: fix missing error code when trace ID is invalid
2026-05-09 0:58 [PATCH v2] coresight: fix missing error code when trace ID is invalid Jie Gan
@ 2026-05-11 8:16 ` James Clark
2026-05-11 8:19 ` Jie Gan
0 siblings, 1 reply; 3+ messages in thread
From: James Clark @ 2026-05-11 8:16 UTC (permalink / raw)
To: Jie Gan
Cc: coresight, linux-arm-kernel, linux-kernel, Suzuki K Poulose,
Mike Leach, Leo Yan, Alexander Shishkin, Tingwei Zhang
On 09/05/2026 1:58 am, Jie Gan wrote:
> When coresight_path_assign_trace_id() cannot assign a valid trace ID,
> coresight_enable_sysfs() takes the err_path goto with ret still 0,
> returning success to the caller despite no trace session being started.
>
> Fix this by changing coresight_path_assign_trace_id() to return int.
> Move the IS_VALID_CS_TRACE_ID() check inside the function so it returns
> -EINVAL on failure and 0 on success. Update coresight_enable_sysfs() to
> check the return value directly instead of inspecting path->trace_id
> after the call.
>
> The other caller in coresight-etm-perf.c discards the return value and
> continues to check path->trace_id via IS_VALID_CS_TRACE_ID() directly.
> This is unaffected: on failure path->trace_id is no longer written, so
> it remains 0, which IS_VALID_CS_TRACE_ID() rejects the same as before.
>
> Fixes: d87d76d823d1 ("Coresight: Allocate trace ID after building the path")
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
> Changes in v2:
> - Refactor the coresight_path_assign_trace_id function.
> - Link to v1: https://lore.kernel.org/r/20260508-fix-trace-id-error-v1-1-5f11a5456fdf@oss.qualcomm.com
> ---
> drivers/hwtracing/coresight/coresight-core.c | 14 ++++++++++----
> drivers/hwtracing/coresight/coresight-priv.h | 2 +-
> drivers/hwtracing/coresight/coresight-sysfs.c | 4 ++--
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 46f247f73cf6..cabdc0c72f38 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -739,12 +739,12 @@ static int coresight_get_trace_id(struct coresight_device *csdev,
> * Call this after creating the path and before enabling it. This leaves
> * the trace ID set on the path, or it remains 0 if it couldn't be assigned.
> */
> -void coresight_path_assign_trace_id(struct coresight_path *path,
> +int coresight_path_assign_trace_id(struct coresight_path *path,
> enum cs_mode mode)
> {
> struct coresight_device *sink = coresight_get_sink(path);
> struct coresight_node *nd;
> - int trace_id;
> + int trace_id, ret = -EINVAL;
>
> list_for_each_entry(nd, &path->path_list, link) {
> /* Assign a trace ID to the path for the first device that wants to do it */
> @@ -755,10 +755,16 @@ void coresight_path_assign_trace_id(struct coresight_path *path,
> * Non 0 is either success or fail.
> */
> if (trace_id != 0) {
> - path->trace_id = trace_id;
> - return;
> + if (IS_VALID_CS_TRACE_ID(trace_id)) {
> + path->trace_id = trace_id;
> + ret = 0;
Reviewed-by: James Clark <james.clark@linaro.org>
Minor nit, but just "return 0" here is a bit simpler.
> + }
> +
> + return ret;
And then "return -EINVAL" for these ones.
> }
> }
> +
> + return ret; > }
>
> /**
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 1ea882dffd70..34c7e792adbd 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -153,7 +153,7 @@ int coresight_make_links(struct coresight_device *orig,
> void coresight_remove_links(struct coresight_device *orig,
> struct coresight_connection *conn);
> u32 coresight_get_sink_id(struct coresight_device *csdev);
> -void coresight_path_assign_trace_id(struct coresight_path *path,
> +int coresight_path_assign_trace_id(struct coresight_path *path,
> enum cs_mode mode);
>
> #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index d2a6ed8bcc74..b6a870399e83 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -211,8 +211,8 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
> goto out;
> }
>
> - coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
> - if (!IS_VALID_CS_TRACE_ID(path->trace_id))
> + ret = coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
> + if (ret)
> goto err_path;
>
> ret = coresight_enable_path(path, CS_MODE_SYSFS);
>
> ---
> base-commit: 17c7841d09ee7d33557fd075562d9289b6018c90
> change-id: 20260508-fix-trace-id-error-dbfdd4d8f2d1
>
> Best regards,
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] coresight: fix missing error code when trace ID is invalid
2026-05-11 8:16 ` James Clark
@ 2026-05-11 8:19 ` Jie Gan
0 siblings, 0 replies; 3+ messages in thread
From: Jie Gan @ 2026-05-11 8:19 UTC (permalink / raw)
To: James Clark
Cc: coresight, linux-arm-kernel, linux-kernel, Suzuki K Poulose,
Mike Leach, Leo Yan, Alexander Shishkin, Tingwei Zhang
On 5/11/2026 4:16 PM, James Clark wrote:
>
>
> On 09/05/2026 1:58 am, Jie Gan wrote:
>> When coresight_path_assign_trace_id() cannot assign a valid trace ID,
>> coresight_enable_sysfs() takes the err_path goto with ret still 0,
>> returning success to the caller despite no trace session being started.
>>
>> Fix this by changing coresight_path_assign_trace_id() to return int.
>> Move the IS_VALID_CS_TRACE_ID() check inside the function so it returns
>> -EINVAL on failure and 0 on success. Update coresight_enable_sysfs() to
>> check the return value directly instead of inspecting path->trace_id
>> after the call.
>>
>> The other caller in coresight-etm-perf.c discards the return value and
>> continues to check path->trace_id via IS_VALID_CS_TRACE_ID() directly.
>> This is unaffected: on failure path->trace_id is no longer written, so
>> it remains 0, which IS_VALID_CS_TRACE_ID() rejects the same as before.
>>
>> Fixes: d87d76d823d1 ("Coresight: Allocate trace ID after building the
>> path")
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>> Changes in v2:
>> - Refactor the coresight_path_assign_trace_id function.
>> - Link to v1: https://lore.kernel.org/r/20260508-fix-trace-id-error-
>> v1-1-5f11a5456fdf@oss.qualcomm.com
>> ---
>> drivers/hwtracing/coresight/coresight-core.c | 14 ++++++++++----
>> drivers/hwtracing/coresight/coresight-priv.h | 2 +-
>> drivers/hwtracing/coresight/coresight-sysfs.c | 4 ++--
>> 3 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/
>> hwtracing/coresight/coresight-core.c
>> index 46f247f73cf6..cabdc0c72f38 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -739,12 +739,12 @@ static int coresight_get_trace_id(struct
>> coresight_device *csdev,
>> * Call this after creating the path and before enabling it. This
>> leaves
>> * the trace ID set on the path, or it remains 0 if it couldn't be
>> assigned.
>> */
>> -void coresight_path_assign_trace_id(struct coresight_path *path,
>> +int coresight_path_assign_trace_id(struct coresight_path *path,
>> enum cs_mode mode)
>> {
>> struct coresight_device *sink = coresight_get_sink(path);
>> struct coresight_node *nd;
>> - int trace_id;
>> + int trace_id, ret = -EINVAL;
>> list_for_each_entry(nd, &path->path_list, link) {
>> /* Assign a trace ID to the path for the first device that
>> wants to do it */
>> @@ -755,10 +755,16 @@ void coresight_path_assign_trace_id(struct
>> coresight_path *path,
>> * Non 0 is either success or fail.
>> */
>> if (trace_id != 0) {
>> - path->trace_id = trace_id;
>> - return;
>> + if (IS_VALID_CS_TRACE_ID(trace_id)) {
>> + path->trace_id = trace_id;
>> + ret = 0;
>
> Reviewed-by: James Clark <james.clark@linaro.org>
>
> Minor nit, but just "return 0" here is a bit simpler.
>
Will remove the ret, and directly return the value for clearly expression.
Thanks,
Jie
>> + }
>> +
>> + return ret;
>
> And then "return -EINVAL" for these ones.
>
>> }
>> }
>> +
>> + return ret; > }
>> /**
>> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/
>> hwtracing/coresight/coresight-priv.h
>> index 1ea882dffd70..34c7e792adbd 100644
>> --- a/drivers/hwtracing/coresight/coresight-priv.h
>> +++ b/drivers/hwtracing/coresight/coresight-priv.h
>> @@ -153,7 +153,7 @@ int coresight_make_links(struct coresight_device
>> *orig,
>> void coresight_remove_links(struct coresight_device *orig,
>> struct coresight_connection *conn);
>> u32 coresight_get_sink_id(struct coresight_device *csdev);
>> -void coresight_path_assign_trace_id(struct coresight_path *path,
>> +int coresight_path_assign_trace_id(struct coresight_path *path,
>> enum cs_mode mode);
>> #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
>> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/
>> hwtracing/coresight/coresight-sysfs.c
>> index d2a6ed8bcc74..b6a870399e83 100644
>> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
>> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
>> @@ -211,8 +211,8 @@ int coresight_enable_sysfs(struct coresight_device
>> *csdev)
>> goto out;
>> }
>> - coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
>> - if (!IS_VALID_CS_TRACE_ID(path->trace_id))
>> + ret = coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
>> + if (ret)
>> goto err_path;
>> ret = coresight_enable_path(path, CS_MODE_SYSFS);
>>
>> ---
>> base-commit: 17c7841d09ee7d33557fd075562d9289b6018c90
>> change-id: 20260508-fix-trace-id-error-dbfdd4d8f2d1
>>
>> Best regards,
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-11 8:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-09 0:58 [PATCH v2] coresight: fix missing error code when trace ID is invalid Jie Gan
2026-05-11 8:16 ` James Clark
2026-05-11 8:19 ` Jie Gan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox