Re: [PATCH v15 05/10] Coresight: Allocate trace ID after building the path

[Jie Gan <quic_jiegan@quicinc.com>]

On 3/4/2025 10:58 PM, Suzuki K Poulose wrote:
> On 03/03/2025 03:29, Jie Gan wrote:
>> The trace_id will be stored in coresight_path instead of being declared
>> everywhere and allocated after building the path.
>>
>> Co-developed-by: James Clark <james.clark@linaro.org>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c  | 44 +++++++++++++++++++
>>   .../hwtracing/coresight/coresight-etm-perf.c  |  5 +--
>>   drivers/hwtracing/coresight/coresight-priv.h  |  2 +
>>   drivers/hwtracing/coresight/coresight-sysfs.c |  4 ++
>>   4 files changed, 52 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ 
>> hwtracing/coresight/coresight-core.c
>> index ed0e9368324d..6adc06995d76 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -655,6 +655,50 @@ static void coresight_drop_device(struct 
>> coresight_device *csdev)
>>       }
>>   }
>> +/*
>> + * coresight device will read their existing or alloc a trace ID, if 
>> their trace_id
>> + * callback is set.
>> + *
>> + * Return 0 if the trace_id callback is not set.
>> + * Return the result of the trace_id callback if it is set. The 
>> return value
>> + * will be the trace_id if successful, and an error number if it fails.
>> + */
>> +static int coresight_get_trace_id(struct coresight_device *csdev,
>> +                  enum cs_mode mode,
>> +                  struct coresight_device *sink)
>> +{
>> +    if (coresight_ops(csdev)->trace_id)
>> +        return coresight_ops(csdev)->trace_id(csdev, mode, sink);
>> +
>> +    return 0;
>> +}
>> +
>> +/*
>> + * 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,
>> +                    enum cs_mode mode)
>> +{
>> +    struct coresight_device *sink = coresight_get_sink(&path- 
>> >path_list);
>> +    struct coresight_node *nd;
>> +    int trace_id;
>> +
>> +    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 */
>> +        trace_id = coresight_get_trace_id(nd->csdev, mode, sink);
>> +
>> +        /*
>> +         * 0 in this context is that it didn't want to assign so keep 
>> searching.
>> +         * Non 0 is either success or fail.
>> +        */
> 
> checkpatch complains:
> 
> WARNING: Block comments should align the * on each line
> #65: FILE: drivers/hwtracing/coresight/coresight-core.c:694:
> +                * Non 0 is either success or fail.
> +               */
> 
> 
> Please make sure to run the checkpatch on individual patches before 
> submitting in the future. I will fix this up locally for now.
> 
> Kind regards
> Suzuki
> 

Hi Suzuki,

Sure. Thanks for help to deal with the error this time. I will take care 
in future.

Jie

> 
> 
>> +        if (trace_id != 0) {
>> +            path->trace_id = trace_id;
>> +            return;
>> +        }
>> +    }
>> +}
>> +
>>   /**
>>    * _coresight_build_path - recursively build a path from a @csdev to 
>> a sink.
>>    * @csdev:    The device to start from.
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/ 
>> drivers/hwtracing/coresight/coresight-etm-perf.c
>> index b0426792f08a..134290ab622e 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -319,7 +319,6 @@ static void *etm_setup_aux(struct perf_event 
>> *event, void **pages,
>>   {
>>       u32 id, cfg_hash;
>>       int cpu = event->cpu;
>> -    int trace_id;
>>       cpumask_t *mask;
>>       struct coresight_device *sink = NULL;
>>       struct coresight_device *user_sink = NULL, *last_sink = NULL;
>> @@ -409,8 +408,8 @@ static void *etm_setup_aux(struct perf_event 
>> *event, void **pages,
>>           }
>>           /* ensure we can allocate a trace ID for this CPU */
>> -        trace_id = coresight_trace_id_get_cpu_id_map(cpu, &sink- 
>> >perf_sink_id_map);
>> -        if (!IS_VALID_CS_TRACE_ID(trace_id)) {
>> +        coresight_path_assign_trace_id(path, CS_MODE_PERF);
>> +        if (!IS_VALID_CS_TRACE_ID(path->trace_id)) {
>>               cpumask_clear_cpu(cpu, mask);
>>               coresight_release_path(path);
>>               continue;
>> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/ 
>> hwtracing/coresight/coresight-priv.h
>> index 27b7dc348d4a..2bea35bae0d4 100644
>> --- a/drivers/hwtracing/coresight/coresight-priv.h
>> +++ b/drivers/hwtracing/coresight/coresight-priv.h
>> @@ -152,6 +152,8 @@ 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,
>> +                   enum cs_mode mode);
>>   #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
>>   extern int etm_readl_cp14(u32 off, unsigned int *val);
>> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/ 
>> hwtracing/coresight/coresight-sysfs.c
>> index cb4c39732d26..d03751bf3d8a 100644
>> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
>> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
>> @@ -209,6 +209,10 @@ 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))
>> +        goto err_path;
>> +
>>       ret = coresight_enable_path(&path->path_list, CS_MODE_SYSFS, NULL);
>>       if (ret)
>>           goto err_path;
>