Re: [PATCH RFC 1/3] coresight: tmc: add the handle of the event to the path

[Jie Gan <jie.gan@oss.qualcomm.com>]

On 9/22/2025 4:29 PM, Leo Yan wrote:
> On Mon, Sep 22, 2025 at 03:31:39PM +0800, Jie Gan wrote:
>> From: Carl Worth <carl@os.amperecomputing.com>
>>
>> The handle is essential for retrieving the AUX_EVENT of each CPU and is
>> required in perf mode. It has been added to the coresight_path so that
>> dependent devices can access it from the path when needed.
>>
>> Fixes: 080ee83cc361 ("Coresight: Change functions to accept the coresight_path")
>> Signed-off-by: Carl Worth <carl@os.amperecomputing.com>
>> Co-developed-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm-perf.c |  1 +
>>   drivers/hwtracing/coresight/coresight-tmc-etr.c  |  3 ++-
>>   include/linux/coresight.h                        | 10 ++++++----
>>   3 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> index f677c08233ba..5c256af6e54a 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -520,6 +520,7 @@ static void etm_event_start(struct perf_event *event, int flags)
>>   		goto out;
>>   
>>   	path = etm_event_cpu_path(event_data, cpu);
>> +	path->handle = handle;
>>   	/* We need a sink, no need to continue without one */
>>   	sink = coresight_get_sink(path);
>>   	if (WARN_ON_ONCE(!sink))
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index b07fcdb3fe1a..1040f73f0537 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1327,7 +1327,8 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>>   struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
>>   				   enum cs_mode mode, void *data)
>>   {
>> -	struct perf_output_handle *handle = data;
>> +	struct coresight_path *path = data;
>> +	struct perf_output_handle *handle = path->handle;
>>   	struct etr_perf_buffer *etr_perf;
>>   
>>   	switch (mode) {
>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>> index 6de59ce8ef8c..4591121ae1d4 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -332,12 +332,14 @@ static struct coresight_dev_list (var) = {				\
>>   
>>   /**
>>    * struct coresight_path - data needed by enable/disable path
>> - * @path_list:              path from source to sink.
>> - * @trace_id:          trace_id of the whole path.
>> + * @path_list:			path from source to sink.
>> + * @trace_id:			trace_id of the whole path.
>> + * struct perf_output_handle:	handle of the aux_event.
> 
> s/struct perf_output_handle/@handle/

Hi Leo,

Thanks for pointing out. I actually missed this error during the code 
review...

Jie

> 
> Otherwise, LGTM:
> 
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> 
>>    */
>>   struct coresight_path {
>> -	struct list_head	path_list;
>> -	u8			trace_id;
>> +	struct list_head		path_list;
>> +	u8				trace_id;
>> +	struct perf_output_handle	*handle;
>>   };
>>   
>>   enum cs_mode {
>>
>> -- 
>> 2.34.1
>>
>>