[PATCH 5/8] drivers/perf: hisi: Refactor the attributes creation

Yicong Yang yangyicong at huawei.com
Mon Oct 21 05:54:40 PDT 2024


On 2024/10/18 21:47, Jonathan Cameron wrote:
> On Fri, 18 Oct 2024 17:57:42 +0800
> Yicong Yang <yangyicong at huawei.com> wrote:
> 
>> From: Yicong Yang <yangyicong at hisilicon.com>
>>
>> Each type of HiSilicon Uncore PMU has the following sysfs attributes:
>>
>> - format: bitmask in perf_event_attr::config[012] of corresponding
>>   attribute
>> - event: events name and corresponding event code
>> - cpumask: range of CPUs the events can be opened on
>> - identifier: the version of this PMU
>>
>> Different types of PMU have different implementations of the "format"
>> and "event" but all share the same implementation of the "cpumask"
>> and "identifier". Thus we can move cpumask and identifier to the
>> hisi_uncore_pmu framework and drivers can use the generic
>> implementation.
>>
>> Signed-off-by: Yicong Yang <yangyicong at hisilicon.com>
> 
> Is there a reason to move to code setting all 4 elements vs
> just using the extern struct attribute_group in the static const arrays?
> 
> e.g.
> static const struct attribute_group *hisi_uc_pmu_attr_groups[] = {
> 	&hisi_uc_pmu_format_group,
> 	&hisi_uc_pmu_events_group,
> 	&hisi_pmu_cpumask_attr_group,
> 	&hisi_pmu_identifier_group,
> 	NULL
> };
> mixing attributes defined in each module with those coming from the core module?
> 
> For the cases where it varies between versions of the PMU, just have a bunch
> of such arrays to pick from.

ah I got the suggestion here. then "enum hisi_pmu_attr_groups" maybe unnecessary
and could be dropped. Only the generic groups like "cpumask" and "identifier"
are centralized and others keep the same. Will try.

Thanks.

> 
> I might be missing some subtle issue, but a really quick hack shows it builds
> fine.
> 
> Jonathan
> 
> 
>> ---
>>  drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c  | 41 +++----------
>>  drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 55 +++++-------------
>>  drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  | 57 ++++++-------------
>>  drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c  | 55 +++++-------------
>>  drivers/perf/hisilicon/hisi_uncore_pa_pmu.c   | 39 +++----------
>>  drivers/perf/hisilicon/hisi_uncore_pmu.c      | 45 +++++++++++----
>>  drivers/perf/hisilicon/hisi_uncore_pmu.h      | 14 ++++-
>>  drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 41 +++----------
>>  drivers/perf/hisilicon/hisi_uncore_uc_pmu.c   | 41 +++----------
>>  9 files changed, 128 insertions(+), 260 deletions(-)
>>
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
>> index bd1c1799f935..4dd52eba9f27 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
>> @@ -225,37 +225,6 @@ static const struct attribute_group hisi_cpa_pmu_events_group = {
>>  	.attrs = hisi_cpa_pmu_events_attr,
>>  };
>>  
>> -static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
>> -
>> -static struct attribute *hisi_cpa_pmu_cpumask_attrs[] = {
>> -	&dev_attr_cpumask.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_cpa_pmu_cpumask_attr_group = {
>> -	.attrs = hisi_cpa_pmu_cpumask_attrs,
>> -};
>> -
>> -static struct device_attribute hisi_cpa_pmu_identifier_attr =
>> -	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
>> -
>> -static struct attribute *hisi_cpa_pmu_identifier_attrs[] = {
>> -	&hisi_cpa_pmu_identifier_attr.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_cpa_pmu_identifier_group = {
>> -	.attrs = hisi_cpa_pmu_identifier_attrs,
>> -};
>> -
>> -static const struct attribute_group *hisi_cpa_pmu_attr_groups[] = {
>> -	&hisi_cpa_pmu_format_group,
>> -	&hisi_cpa_pmu_events_group,
>> -	&hisi_cpa_pmu_cpumask_attr_group,
>> -	&hisi_cpa_pmu_identifier_group,
>> -	NULL
>> -};
>> -
>>  static const struct hisi_uncore_ops hisi_uncore_cpa_pmu_ops = {
>>  	.write_evtype           = hisi_cpa_pmu_write_evtype,
>>  	.get_event_idx		= hisi_uncore_pmu_get_event_idx,
>> @@ -274,6 +243,7 @@ static const struct hisi_uncore_ops hisi_uncore_cpa_pmu_ops = {
>>  static int hisi_cpa_pmu_dev_probe(struct platform_device *pdev,
>>  				  struct hisi_pmu *cpa_pmu)
>>  {
>> +	struct hisi_pmu_hwevents *pmu_events = &cpa_pmu->pmu_events;
>>  	int ret;
>>  
>>  	ret = hisi_cpa_pmu_init_data(pdev, cpa_pmu);
>> @@ -286,7 +256,14 @@ static int hisi_cpa_pmu_dev_probe(struct platform_device *pdev,
>>  
>>  	cpa_pmu->counter_bits = CPA_COUNTER_BITS;
>>  	cpa_pmu->check_event = CPA_NR_EVENTS;
>> -	cpa_pmu->pmu_events.attr_groups = hisi_cpa_pmu_attr_groups;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						&hisi_cpa_pmu_format_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						&hisi_cpa_pmu_events_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_CPUMASK] =
>> +						&hisi_pmu_cpumask_attr_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_IDENTIFIER] =
>> +						&hisi_pmu_identifier_group;
>>  	cpa_pmu->ops = &hisi_uncore_cpa_pmu_ops;
>>  	cpa_pmu->num_counters = CPA_NR_COUNTERS;
>>  	cpa_pmu->dev = &pdev->dev;
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
>> index 415a95e24993..6d805ca4562f 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
>> @@ -380,45 +380,6 @@ static const struct attribute_group hisi_ddrc_pmu_v2_events_group = {
>>  	.attrs = hisi_ddrc_pmu_v2_events_attr,
>>  };
>>  
>> -static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
>> -
>> -static struct attribute *hisi_ddrc_pmu_cpumask_attrs[] = {
>> -	&dev_attr_cpumask.attr,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute_group hisi_ddrc_pmu_cpumask_attr_group = {
>> -	.attrs = hisi_ddrc_pmu_cpumask_attrs,
>> -};
>> -
>> -static struct device_attribute hisi_ddrc_pmu_identifier_attr =
>> -	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
>> -
>> -static struct attribute *hisi_ddrc_pmu_identifier_attrs[] = {
>> -	&hisi_ddrc_pmu_identifier_attr.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_ddrc_pmu_identifier_group = {
>> -	.attrs = hisi_ddrc_pmu_identifier_attrs,
>> -};
>> -
>> -static const struct attribute_group *hisi_ddrc_pmu_v1_attr_groups[] = {
>> -	&hisi_ddrc_pmu_v1_format_group,
>> -	&hisi_ddrc_pmu_v1_events_group,
>> -	&hisi_ddrc_pmu_cpumask_attr_group,
>> -	&hisi_ddrc_pmu_identifier_group,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute_group *hisi_ddrc_pmu_v2_attr_groups[] = {
>> -	&hisi_ddrc_pmu_v2_format_group,
>> -	&hisi_ddrc_pmu_v2_events_group,
>> -	&hisi_ddrc_pmu_cpumask_attr_group,
>> -	&hisi_ddrc_pmu_identifier_group,
>> -	NULL
>> -};
>> -
>>  static const struct hisi_uncore_ops hisi_uncore_ddrc_v1_ops = {
>>  	.write_evtype           = hisi_ddrc_pmu_write_evtype,
>>  	.get_event_idx		= hisi_ddrc_pmu_v1_get_event_idx,
>> @@ -452,6 +413,7 @@ static const struct hisi_uncore_ops hisi_uncore_ddrc_v2_ops = {
>>  static int hisi_ddrc_pmu_dev_probe(struct platform_device *pdev,
>>  				   struct hisi_pmu *ddrc_pmu)
>>  {
>> +	struct hisi_pmu_hwevents *pmu_events = &ddrc_pmu->pmu_events;
>>  	int ret;
>>  
>>  	ret = hisi_ddrc_pmu_init_data(pdev, ddrc_pmu);
>> @@ -465,15 +427,26 @@ static int hisi_ddrc_pmu_dev_probe(struct platform_device *pdev,
>>  	if (ddrc_pmu->identifier >= HISI_PMU_V2) {
>>  		ddrc_pmu->counter_bits = 48;
>>  		ddrc_pmu->check_event = DDRC_V2_NR_EVENTS;
>> -		ddrc_pmu->pmu_events.attr_groups = hisi_ddrc_pmu_v2_attr_groups;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						&hisi_ddrc_pmu_v2_format_group;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						&hisi_ddrc_pmu_v2_events_group;
>>  		ddrc_pmu->ops = &hisi_uncore_ddrc_v2_ops;
>>  	} else {
>>  		ddrc_pmu->counter_bits = 32;
>>  		ddrc_pmu->check_event = DDRC_V1_NR_EVENTS;
>> -		ddrc_pmu->pmu_events.attr_groups = hisi_ddrc_pmu_v1_attr_groups;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						&hisi_ddrc_pmu_v1_format_group;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						&hisi_ddrc_pmu_v1_events_group;
>>  		ddrc_pmu->ops = &hisi_uncore_ddrc_v1_ops;
>>  	}
>>  
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_CPUMASK] =
>> +						&hisi_pmu_cpumask_attr_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_IDENTIFIER] =
>> +						&hisi_pmu_identifier_group;
>> +
>>  	ddrc_pmu->num_counters = DDRC_NR_COUNTERS;
>>  	ddrc_pmu->dev = &pdev->dev;
>>  	ddrc_pmu->on_cpu = -1;
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
>> index 43554e4f8a36..07cab6cf4897 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
>> @@ -405,45 +405,6 @@ static const struct attribute_group hisi_hha_pmu_v2_events_group = {
>>  	.attrs = hisi_hha_pmu_v2_events_attr,
>>  };
>>  
>> -static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
>> -
>> -static struct attribute *hisi_hha_pmu_cpumask_attrs[] = {
>> -	&dev_attr_cpumask.attr,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute_group hisi_hha_pmu_cpumask_attr_group = {
>> -	.attrs = hisi_hha_pmu_cpumask_attrs,
>> -};
>> -
>> -static struct device_attribute hisi_hha_pmu_identifier_attr =
>> -	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
>> -
>> -static struct attribute *hisi_hha_pmu_identifier_attrs[] = {
>> -	&hisi_hha_pmu_identifier_attr.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_hha_pmu_identifier_group = {
>> -	.attrs = hisi_hha_pmu_identifier_attrs,
>> -};
>> -
>> -static const struct attribute_group *hisi_hha_pmu_v1_attr_groups[] = {
>> -	&hisi_hha_pmu_v1_format_group,
>> -	&hisi_hha_pmu_v1_events_group,
>> -	&hisi_hha_pmu_cpumask_attr_group,
>> -	&hisi_hha_pmu_identifier_group,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute_group *hisi_hha_pmu_v2_attr_groups[] = {
>> -	&hisi_hha_pmu_v2_format_group,
>> -	&hisi_hha_pmu_v2_events_group,
>> -	&hisi_hha_pmu_cpumask_attr_group,
>> -	&hisi_hha_pmu_identifier_group,
>> -	NULL
>> -};
>> -
>>  static const struct hisi_uncore_ops hisi_uncore_hha_ops = {
>>  	.write_evtype		= hisi_hha_pmu_write_evtype,
>>  	.get_event_idx		= hisi_uncore_pmu_get_event_idx,
>> @@ -464,6 +425,7 @@ static const struct hisi_uncore_ops hisi_uncore_hha_ops = {
>>  static int hisi_hha_pmu_dev_probe(struct platform_device *pdev,
>>  				  struct hisi_pmu *hha_pmu)
>>  {
>> +	struct hisi_pmu_hwevents *pmu_events = &hha_pmu->pmu_events;
>>  	int ret;
>>  
>>  	ret = hisi_hha_pmu_init_data(pdev, hha_pmu);
>> @@ -477,14 +439,27 @@ static int hisi_hha_pmu_dev_probe(struct platform_device *pdev,
>>  	if (hha_pmu->identifier >= HISI_PMU_V2) {
>>  		hha_pmu->counter_bits = 64;
>>  		hha_pmu->check_event = HHA_V2_NR_EVENT;
>> -		hha_pmu->pmu_events.attr_groups = hisi_hha_pmu_v2_attr_groups;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						&hisi_hha_pmu_v2_format_group;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						&hisi_hha_pmu_v2_events_group;
>>  		hha_pmu->num_counters = HHA_V2_NR_COUNTERS;
>>  	} else {
>>  		hha_pmu->counter_bits = 48;
>>  		hha_pmu->check_event = HHA_V1_NR_EVENT;
>> -		hha_pmu->pmu_events.attr_groups = hisi_hha_pmu_v1_attr_groups;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						&hisi_hha_pmu_v1_format_group;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						&hisi_hha_pmu_v1_events_group;
>>  		hha_pmu->num_counters = HHA_V1_NR_COUNTERS;
>>  	}
>> +
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_CPUMASK] =
>> +						&hisi_pmu_cpumask_attr_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_IDENTIFIER] =
>> +						&hisi_pmu_identifier_group;
>> +
>> +
>>  	hha_pmu->ops = &hisi_uncore_hha_ops;
>>  	hha_pmu->dev = &pdev->dev;
>>  	hha_pmu->on_cpu = -1;
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
>> index b113620d27f9..6f540ab1f451 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
>> @@ -441,45 +441,6 @@ static const struct attribute_group hisi_l3c_pmu_v2_events_group = {
>>  	.attrs = hisi_l3c_pmu_v2_events_attr,
>>  };
>>  
>> -static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
>> -
>> -static struct attribute *hisi_l3c_pmu_cpumask_attrs[] = {
>> -	&dev_attr_cpumask.attr,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute_group hisi_l3c_pmu_cpumask_attr_group = {
>> -	.attrs = hisi_l3c_pmu_cpumask_attrs,
>> -};
>> -
>> -static struct device_attribute hisi_l3c_pmu_identifier_attr =
>> -	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
>> -
>> -static struct attribute *hisi_l3c_pmu_identifier_attrs[] = {
>> -	&hisi_l3c_pmu_identifier_attr.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_l3c_pmu_identifier_group = {
>> -	.attrs = hisi_l3c_pmu_identifier_attrs,
>> -};
>> -
>> -static const struct attribute_group *hisi_l3c_pmu_v1_attr_groups[] = {
>> -	&hisi_l3c_pmu_v1_format_group,
>> -	&hisi_l3c_pmu_v1_events_group,
>> -	&hisi_l3c_pmu_cpumask_attr_group,
>> -	&hisi_l3c_pmu_identifier_group,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute_group *hisi_l3c_pmu_v2_attr_groups[] = {
>> -	&hisi_l3c_pmu_v2_format_group,
>> -	&hisi_l3c_pmu_v2_events_group,
>> -	&hisi_l3c_pmu_cpumask_attr_group,
>> -	&hisi_l3c_pmu_identifier_group,
>> -	NULL
>> -};
>> -
>>  static const struct hisi_uncore_ops hisi_uncore_l3c_ops = {
>>  	.write_evtype		= hisi_l3c_pmu_write_evtype,
>>  	.get_event_idx		= hisi_uncore_pmu_get_event_idx,
>> @@ -500,6 +461,7 @@ static const struct hisi_uncore_ops hisi_uncore_l3c_ops = {
>>  static int hisi_l3c_pmu_dev_probe(struct platform_device *pdev,
>>  				  struct hisi_pmu *l3c_pmu)
>>  {
>> +	struct hisi_pmu_hwevents *pmu_events = &l3c_pmu->pmu_events;
>>  	int ret;
>>  
>>  	ret = hisi_l3c_pmu_init_data(pdev, l3c_pmu);
>> @@ -513,13 +475,24 @@ static int hisi_l3c_pmu_dev_probe(struct platform_device *pdev,
>>  	if (l3c_pmu->identifier >= HISI_PMU_V2) {
>>  		l3c_pmu->counter_bits = 64;
>>  		l3c_pmu->check_event = L3C_V2_NR_EVENTS;
>> -		l3c_pmu->pmu_events.attr_groups = hisi_l3c_pmu_v2_attr_groups;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						 &hisi_l3c_pmu_v2_format_group;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						 &hisi_l3c_pmu_v2_events_group;
>>  	} else {
>>  		l3c_pmu->counter_bits = 48;
>>  		l3c_pmu->check_event = L3C_V1_NR_EVENTS;
>> -		l3c_pmu->pmu_events.attr_groups = hisi_l3c_pmu_v1_attr_groups;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						&hisi_l3c_pmu_v1_format_group;
>> +		pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						&hisi_l3c_pmu_v1_events_group;
>>  	}
>>  
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_CPUMASK] =
>> +						&hisi_pmu_cpumask_attr_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_IDENTIFIER] =
>> +						&hisi_pmu_identifier_group;
>> +
>>  	l3c_pmu->num_counters = L3C_NR_COUNTERS;
>>  	l3c_pmu->ops = &hisi_uncore_l3c_ops;
>>  	l3c_pmu->dev = &pdev->dev;
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
>> index dbe4d7b97b2b..69931e73a3cd 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
>> @@ -353,29 +353,6 @@ static const struct attribute_group hisi_h60pa_pmu_events_group = {
>>  	.attrs = hisi_h60pa_pmu_events_attr,
>>  };
>>  
>> -static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
>> -
>> -static struct attribute *hisi_pa_pmu_cpumask_attrs[] = {
>> -	&dev_attr_cpumask.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_pa_pmu_cpumask_attr_group = {
>> -	.attrs = hisi_pa_pmu_cpumask_attrs,
>> -};
>> -
>> -static struct device_attribute hisi_pa_pmu_identifier_attr =
>> -	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
>> -
>> -static struct attribute *hisi_pa_pmu_identifier_attrs[] = {
>> -	&hisi_pa_pmu_identifier_attr.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_pa_pmu_identifier_group = {
>> -	.attrs = hisi_pa_pmu_identifier_attrs,
>> -};
>> -
>>  static struct hisi_pa_pmu_int_regs hisi_pa_pmu_regs = {
>>  	.mask_offset = PA_INT_MASK,
>>  	.clear_offset = PA_INT_CLEAR,
>> @@ -385,8 +362,6 @@ static struct hisi_pa_pmu_int_regs hisi_pa_pmu_regs = {
>>  static const struct attribute_group *hisi_pa_pmu_v2_attr_groups[] = {
>>  	&hisi_pa_pmu_v2_format_group,
>>  	&hisi_pa_pmu_v2_events_group,
>> -	&hisi_pa_pmu_cpumask_attr_group,
>> -	&hisi_pa_pmu_identifier_group,
>>  	NULL
>>  };
>>  
>> @@ -399,8 +374,6 @@ static const struct hisi_pmu_dev_info hisi_h32pa_v2 = {
>>  static const struct attribute_group *hisi_pa_pmu_v3_attr_groups[] = {
>>  	&hisi_pa_pmu_v2_format_group,
>>  	&hisi_pa_pmu_v3_events_group,
>> -	&hisi_pa_pmu_cpumask_attr_group,
>> -	&hisi_pa_pmu_identifier_group,
>>  	NULL
>>  };
>>  
>> @@ -419,8 +392,6 @@ static struct hisi_pa_pmu_int_regs hisi_h60pa_pmu_regs = {
>>  static const struct attribute_group *hisi_h60pa_pmu_attr_groups[] = {
>>  	&hisi_pa_pmu_v2_format_group,
>>  	&hisi_h60pa_pmu_events_group,
>> -	&hisi_pa_pmu_cpumask_attr_group,
>> -	&hisi_pa_pmu_identifier_group,
>>  	NULL
>>  };
>>  
>> @@ -450,6 +421,7 @@ static const struct hisi_uncore_ops hisi_uncore_pa_ops = {
>>  static int hisi_pa_pmu_dev_probe(struct platform_device *pdev,
>>  				 struct hisi_pmu *pa_pmu)
>>  {
>> +	struct hisi_pmu_hwevents *pmu_events = &pa_pmu->pmu_events;
>>  	int ret;
>>  
>>  	ret = hisi_pa_pmu_init_data(pdev, pa_pmu);
>> @@ -460,7 +432,14 @@ static int hisi_pa_pmu_dev_probe(struct platform_device *pdev,
>>  	if (ret)
>>  		return ret;
>>  
>> -	pa_pmu->pmu_events.attr_groups = pa_pmu->dev_info->attr_groups;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +		pa_pmu->dev_info->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT];
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +		pa_pmu->dev_info->attr_groups[HISI_PMU_ATTR_GROUP_EVENT];
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_CPUMASK] =
>> +						&hisi_pmu_cpumask_attr_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_IDENTIFIER] =
>> +						&hisi_pmu_identifier_group;
>>  	pa_pmu->num_counters = PA_NR_COUNTERS;
>>  	pa_pmu->ops = &hisi_uncore_pa_ops;
>>  	pa_pmu->check_event = 0xB0;
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> index e1756e639784..648d0e5e08ed 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> @@ -49,6 +49,41 @@ ssize_t hisi_cpumask_sysfs_show(struct device *dev,
>>  }
>>  EXPORT_SYMBOL_NS_GPL(hisi_cpumask_sysfs_show, HISI_PMU);
>>  
>> +static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
>> +
>> +static struct attribute *hisi_pmu_cpumask_attrs[] = {
>> +	&dev_attr_cpumask.attr,
>> +	NULL
>> +};
>> +
>> +const struct attribute_group hisi_pmu_cpumask_attr_group = {
>> +	.attrs = hisi_pmu_cpumask_attrs,
>> +};
>> +EXPORT_SYMBOL_NS_GPL(hisi_pmu_cpumask_attr_group, HISI_PMU);
>> +
>> +ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
>> +					     struct device_attribute *attr,
>> +					     char *page)
>> +{
>> +	struct hisi_pmu *hisi_pmu = to_hisi_pmu(dev_get_drvdata(dev));
>> +
>> +	return sysfs_emit(page, "0x%08x\n", hisi_pmu->identifier);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_identifier_attr_show, HISI_PMU);
>> +
>> +static struct device_attribute hisi_pmu_identifier_attr =
>> +	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
>> +
>> +static struct attribute *hisi_pmu_identifier_attrs[] = {
>> +	&hisi_pmu_identifier_attr.attr,
>> +	NULL
>> +};
>> +
>> +const struct attribute_group hisi_pmu_identifier_group = {
>> +	.attrs = hisi_pmu_identifier_attrs,
>> +};
>> +EXPORT_SYMBOL_NS_GPL(hisi_pmu_identifier_group, HISI_PMU);
>> +
>>  static bool hisi_validate_event_group(struct perf_event *event)
>>  {
>>  	struct perf_event *sibling, *leader = event->group_leader;
>> @@ -99,16 +134,6 @@ int hisi_uncore_pmu_get_event_idx(struct perf_event *event)
>>  }
>>  EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_get_event_idx, HISI_PMU);
>>  
>> -ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
>> -					     struct device_attribute *attr,
>> -					     char *page)
>> -{
>> -	struct hisi_pmu *hisi_pmu = to_hisi_pmu(dev_get_drvdata(dev));
>> -
>> -	return sysfs_emit(page, "0x%08x\n", hisi_pmu->identifier);
>> -}
>> -EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_identifier_attr_show, HISI_PMU);
>> -
>>  static void hisi_uncore_pmu_clear_event_idx(struct hisi_pmu *hisi_pmu, int idx)
>>  {
>>  	clear_bit(idx, hisi_pmu->pmu_events.used_mask);
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
>> index 239c45d847b3..ac2be8c337b7 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
>> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
>> @@ -77,10 +77,22 @@ struct hisi_pmu_dev_info {
>>  	void *private;
>>  };
>>  
>> +enum hisi_pmu_attr_groups {
>> +	HISI_PMU_ATTR_GROUP_FORMAT,
>> +	HISI_PMU_ATTR_GROUP_EVENT,
>> +	HISI_PMU_ATTR_GROUP_CPUMASK,
>> +	HISI_PMU_ATTR_GROUP_IDENTIFIER,
>> +	HISI_PMU_ATTR_GROUP_NR
>> +};
>> +
>> +/* Generic implementation of cpumask/identifier group */
>> +extern const struct attribute_group hisi_pmu_cpumask_attr_group;
>> +extern const struct attribute_group hisi_pmu_identifier_group;
>> +
>>  struct hisi_pmu_hwevents {
>>  	struct perf_event *hw_events[HISI_MAX_COUNTERS];
>>  	DECLARE_BITMAP(used_mask, HISI_MAX_COUNTERS);
>> -	const struct attribute_group **attr_groups;
>> +	const struct attribute_group *attr_groups[HISI_PMU_ATTR_GROUP_NR + 1];
>>  };
>>  
>>  /**
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
>> index 43cbdf0fb7c7..676a7078f2ef 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
>> @@ -344,37 +344,6 @@ static const struct attribute_group hisi_sllc_pmu_v2_events_group = {
>>  	.attrs = hisi_sllc_pmu_v2_events_attr,
>>  };
>>  
>> -static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
>> -
>> -static struct attribute *hisi_sllc_pmu_cpumask_attrs[] = {
>> -	&dev_attr_cpumask.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_sllc_pmu_cpumask_attr_group = {
>> -	.attrs = hisi_sllc_pmu_cpumask_attrs,
>> -};
>> -
>> -static struct device_attribute hisi_sllc_pmu_identifier_attr =
>> -	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
>> -
>> -static struct attribute *hisi_sllc_pmu_identifier_attrs[] = {
>> -	&hisi_sllc_pmu_identifier_attr.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_sllc_pmu_identifier_group = {
>> -	.attrs = hisi_sllc_pmu_identifier_attrs,
>> -};
>> -
>> -static const struct attribute_group *hisi_sllc_pmu_v2_attr_groups[] = {
>> -	&hisi_sllc_pmu_v2_format_group,
>> -	&hisi_sllc_pmu_v2_events_group,
>> -	&hisi_sllc_pmu_cpumask_attr_group,
>> -	&hisi_sllc_pmu_identifier_group,
>> -	NULL
>> -};
>> -
>>  static const struct hisi_uncore_ops hisi_uncore_sllc_ops = {
>>  	.write_evtype		= hisi_sllc_pmu_write_evtype,
>>  	.get_event_idx		= hisi_uncore_pmu_get_event_idx,
>> @@ -395,6 +364,7 @@ static const struct hisi_uncore_ops hisi_uncore_sllc_ops = {
>>  static int hisi_sllc_pmu_dev_probe(struct platform_device *pdev,
>>  				   struct hisi_pmu *sllc_pmu)
>>  {
>> +	struct hisi_pmu_hwevents *pmu_events = &sllc_pmu->pmu_events;
>>  	int ret;
>>  
>>  	ret = hisi_sllc_pmu_init_data(pdev, sllc_pmu);
>> @@ -405,7 +375,14 @@ static int hisi_sllc_pmu_dev_probe(struct platform_device *pdev,
>>  	if (ret)
>>  		return ret;
>>  
>> -	sllc_pmu->pmu_events.attr_groups = hisi_sllc_pmu_v2_attr_groups;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						&hisi_sllc_pmu_v2_format_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						&hisi_sllc_pmu_v2_events_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_CPUMASK] =
>> +						&hisi_pmu_cpumask_attr_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_IDENTIFIER] =
>> +						&hisi_pmu_identifier_group;
>>  	sllc_pmu->ops = &hisi_uncore_sllc_ops;
>>  	sllc_pmu->check_event = SLLC_NR_EVENTS;
>>  	sllc_pmu->counter_bits = 64;
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
>> index 2040f6a8871e..c2ae852f6b19 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
>> @@ -437,37 +437,6 @@ static const struct attribute_group hisi_uc_pmu_events_group = {
>>  	.attrs = hisi_uc_pmu_events_attr,
>>  };
>>  
>> -static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
>> -
>> -static struct attribute *hisi_uc_pmu_cpumask_attrs[] = {
>> -	&dev_attr_cpumask.attr,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute_group hisi_uc_pmu_cpumask_attr_group = {
>> -	.attrs = hisi_uc_pmu_cpumask_attrs,
>> -};
>> -
>> -static struct device_attribute hisi_uc_pmu_identifier_attr =
>> -	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
>> -
>> -static struct attribute *hisi_uc_pmu_identifier_attrs[] = {
>> -	&hisi_uc_pmu_identifier_attr.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group hisi_uc_pmu_identifier_group = {
>> -	.attrs = hisi_uc_pmu_identifier_attrs,
>> -};
>> -
>> -static const struct attribute_group *hisi_uc_pmu_attr_groups[] = {
>> -	&hisi_uc_pmu_format_group,
>> -	&hisi_uc_pmu_events_group,
>> -	&hisi_uc_pmu_cpumask_attr_group,
>> -	&hisi_uc_pmu_identifier_group,
>> -	NULL
>> -};
>> -
>>  static const struct hisi_uncore_ops hisi_uncore_uc_pmu_ops = {
>>  	.check_filter		= hisi_uc_pmu_check_filter,
>>  	.write_evtype		= hisi_uc_pmu_write_evtype,
>> @@ -489,6 +458,7 @@ static const struct hisi_uncore_ops hisi_uncore_uc_pmu_ops = {
>>  static int hisi_uc_pmu_dev_probe(struct platform_device *pdev,
>>  				 struct hisi_pmu *uc_pmu)
>>  {
>> +	struct hisi_pmu_hwevents *pmu_events = &uc_pmu->pmu_events;
>>  	int ret;
>>  
>>  	ret = hisi_uc_pmu_init_data(pdev, uc_pmu);
>> @@ -499,7 +469,14 @@ static int hisi_uc_pmu_dev_probe(struct platform_device *pdev,
>>  	if (ret)
>>  		return ret;
>>  
>> -	uc_pmu->pmu_events.attr_groups = hisi_uc_pmu_attr_groups;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_FORMAT] =
>> +						&hisi_uc_pmu_format_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_EVENT] =
>> +						&hisi_uc_pmu_events_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_CPUMASK] =
>> +						&hisi_pmu_cpumask_attr_group;
>> +	pmu_events->attr_groups[HISI_PMU_ATTR_GROUP_IDENTIFIER] =
>> +						&hisi_pmu_identifier_group;
>>  	uc_pmu->check_event = HISI_UC_EVTYPE_MASK;
>>  	uc_pmu->ops = &hisi_uncore_uc_pmu_ops;
>>  	uc_pmu->counter_bits = HISI_UC_CNTR_REG_BITS;
> 
> .
> 



More information about the linux-arm-kernel mailing list