[PATCH 3/8] drivers/perf: hisi: Extract topology information to a separate structure

[Yicong Yang]

On 2024/10/18 20:54, Jonathan Cameron wrote:
> On Fri, 18 Oct 2024 17:57:40 +0800
> Yicong Yang <yangyicong at huawei.com> wrote:
> 
>> From: Yicong Yang <yangyicong at hisilicon.com>
>>
>> HiSilicon Uncore PMUs are identified by the IDs of the topology element
>> on which the PMUs are located. Add a new separate struct hisi_pmu_toplogy
>> to encapsulate this information. Add additional documentation on the
>> meaning of each ID.
>>
>> - make sccl_id and sicl_id into a union since they're exclusive. It can
>>   also be accessed by scl_id if the SICL/SCCL is unawared.
> 
> As below. Maybe "distinction is not relevant." as not sure what unwared means here.
> 
>> - make index_id and sub_id to int since we'll make them to -1 if the
>>   PMU doesn't have this topology or fail to retrieve them
> - make index_id and sub_id signed so -1 may be used to indicate
>   the PMU doesn't have this topology element or it could not be retrieved.
> 
>>
>> This patch should have no functional changes.
>>
>> Signed-off-by: Yicong Yang <yangyicong at hisilicon.com>
> With those small description updates (or similar)
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron at huawei.com>
> 
>> ---
>> +/**
>> + * struct hisi_pmu_topology - Describe the topology hierarchy on which the PMU
>> + *                            is located.
>> + * @sccl_id: ID of the SCCL on which the PMU locate is located.
>> + * @sicl_id: ID of the SICL on which the PMU locate is located.
>> + * @scl_id:  ID used by the core which is unaware of the SCCL/SICL.
>> + * @ccl_id: ID of the CCL (CPU cluster) on which the PMU is located.
>> + * @index_id: the ID of the PMU module if there're several PMUs at a
>> + *            particularly location in the topology.
>> + * @sub_id: submodule ID of the PMU. For example we use this for DDRC PMU v2
>> + *          since each DDRC has more than one DMC
>> + *
>> + * The ID will be -1 if the PMU isn't located on a certain topology.
>> + */
>> +struct hisi_pmu_topology {
>> +	/*
>> +	 * SCCL (Super CPU CLuster) and SICL (Super I/O Cluster) are parallel
>> +	 * so a PMU cannot locate on a SCCL and a SICL. If the SCCL/SICL is
>> +	 * not awared use scl_id instead.
> distinction is not relevant?  I' not quite sure what not awared means.
> 

The SICL or SCCL is only meaningful to the certain driver like L3C, since they
know whether this PMU locates on a SICL or SCCL. For the framework it's not
capable of inferring such information and don't care about it (not awared here).

Will make this piece of comment more clearer.

Thanks.

>> +	 */
>> +	union {
>> +		int sccl_id;
>> +		int sicl_id;
>> +		int scl_id;
>> +	};
>> +	int ccl_id;
>> +	int index_id;
>> +	int sub_id;
>> +};
>> +
> 
> .
>