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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Fri, 18 Oct 2024 17:57:40 +0800
Yicong Yang <yangyicong@huawei.com> wrote:

> From: Yicong Yang <yangyicong@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@hisilicon.com>
With those small description updates (or similar)

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@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.

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