On Mon, Mar 10, 2025 at 12:01:25PM +0000, Suzuki Kuruppassery Poulose wrote:
On 10/03/2025 10:49, Leo Yan wrote:
Introduce APIs for pausing and resuming trace source and export as GPL symbols.
Signed-off-by: Leo Yan leo.yan@arm.com
drivers/hwtracing/coresight/coresight-core.c | 20 ++++++++++++++++++++ drivers/hwtracing/coresight/coresight-priv.h | 2 ++ include/linux/coresight.h | 4 ++++ 3 files changed, 26 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 0a9380350fb5..eb7b83a7bfa2 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -365,6 +365,26 @@ void coresight_disable_source(struct coresight_device *csdev, void *data) } EXPORT_SYMBOL_GPL(coresight_disable_source); +void coresight_pause_source(struct coresight_device *csdev) +{
- if (!csdev || !coresight_is_percpu_source(csdev))
You may remove the !csdev check, coresight_is_percpu_source() already checks that.
Will do.
return;- if (source_ops(csdev)->pause)
source_ops(csdev)->pause(csdev);+} +EXPORT_SYMBOL_GPL(coresight_pause_source);
+void coresight_resume_source(struct coresight_device *csdev) +{
- if (!csdev || !coresight_is_percpu_source(csdev))
return;Same as above.
Will do.
- if (source_ops(csdev)->resume)
source_ops(csdev)->resume(csdev);+} +EXPORT_SYMBOL_GPL(coresight_resume_source);
- /*
- coresight_disable_path_from : Disable components in the given path beyond
- @nd in the list. If @nd is NULL, all the components, except the SOURCE are
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 76403530f33e..a9f14c075e91 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -247,5 +247,7 @@ void coresight_add_helper(struct coresight_device *csdev, void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev); struct coresight_device *coresight_get_percpu_sink(int cpu); void coresight_disable_source(struct coresight_device *csdev, void *data); +void coresight_pause_source(struct coresight_device *csdev); +void coresight_resume_source(struct coresight_device *csdev); #endif diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 17276965ff1d..703e1b8dbe22 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -385,6 +385,8 @@ struct coresight_ops_link {
is associated to.- @enable: enables tracing for a source.
- @disable: disables tracing for a source.
- @resume: resumes tracing for a source.
*/ struct coresight_ops_source { int (*cpu_id)(struct coresight_device *csdev);
- @pause: pauses tracing for a source.
@@ -392,6 +394,8 @@ struct coresight_ops_source { enum cs_mode mode, struct coresight_trace_id_map *id_map); void (*disable)(struct coresight_device *csdev, struct perf_event *event);
- int (*resume)(struct coresight_device *csdev);
- void (*pause)(struct coresight_device *csdev);
Given these callbacks are for perf mode, does it make sense to pass the parameters similar to enable() ?
It is not necessary to pass perf event handle to resume() and pause() callbacks.
The main reason is perf related operations have been processed in the coresight-etm-perf.c layer, the low level's callbacks (pause/resume) should simply take actions on hardware logic.
Also, should we rename these as :
resume_perf pause_perf
To explicitly mark them perf mode specific ?
This makes sense to me. The naming "resume" might be confused with CPUIdle's terminology (CPU resume). pause_perf/resume_perf can perfectly avoid any confusion.
Thanks, Leo