From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Yicong Yang <yangyicong@huawei.com>
Cc: <will@kernel.org>, <mark.rutland@arm.com>,
<linux-arm-kernel@lists.infradead.org>,
<yangyicong@hisilicon.com>, <hejunhao3@huawei.com>,
<linuxarm@huawei.com>, <wangyushan12@huawei.com>,
<prime.zeng@hisilicon.com>
Subject: Re: [PATCH 5/8] drivers/perf: hisi: Refactor the attributes creation
Date: Fri, 18 Oct 2024 14:47:29 +0100 [thread overview]
Message-ID: <20241018144729.00005ea3@Huawei.com> (raw)
In-Reply-To: <20241018095745.57057-6-yangyicong@huawei.com>
On Fri, 18 Oct 2024 17:57:42 +0800
Yicong Yang <yangyicong@huawei.com> wrote:
> From: Yicong Yang <yangyicong@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@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.
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;
next prev parent reply other threads:[~2024-10-18 13:52 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-18 9:57 [PATCH 0/8] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
2024-10-18 9:57 ` [PATCH 1/8] drivers/perf: hisi: Define a symbol namespace for HiSilicon Uncore PMUs Yicong Yang
2024-10-18 12:43 ` Jonathan Cameron
2024-10-21 12:39 ` Yicong Yang
2024-10-18 9:57 ` [PATCH 2/8] drivers/perf: hisi: Improve the detection of associated CPUs Yicong Yang
2024-10-18 12:48 ` Jonathan Cameron
2024-10-21 12:41 ` Yicong Yang
2024-10-18 9:57 ` [PATCH 3/8] drivers/perf: hisi: Extract topology information to a separate structure Yicong Yang
2024-10-18 12:54 ` Jonathan Cameron
2024-10-21 12:44 ` Yicong Yang
2024-10-18 9:57 ` [PATCH 4/8] drivers/perf: hisi: Add a common function to retrieve topology from firmware Yicong Yang
2024-10-18 12:58 ` Jonathan Cameron
2024-10-18 9:57 ` [PATCH 5/8] drivers/perf: hisi: Refactor the attributes creation Yicong Yang
2024-10-18 13:47 ` Jonathan Cameron [this message]
2024-10-21 12:54 ` Yicong Yang
2024-10-18 9:57 ` [PATCH 6/8] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs Yicong Yang
2024-10-18 13:19 ` Jonathan Cameron
2024-10-18 9:57 ` [PATCH 7/8] drivers/perf: hisi: Fix incorrect variable name "hha_pmu" in DDRC PMU driver Yicong Yang
2024-10-18 13:20 ` Jonathan Cameron
2024-10-18 9:57 ` [PATCH 8/8] drivers/perf: hisi: Delete redundant blank line of DDRC PMU Yicong Yang
2024-10-18 13:21 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241018144729.00005ea3@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=hejunhao3@huawei.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linuxarm@huawei.com \
--cc=mark.rutland@arm.com \
--cc=prime.zeng@hisilicon.com \
--cc=wangyushan12@huawei.com \
--cc=will@kernel.org \
--cc=yangyicong@hisilicon.com \
--cc=yangyicong@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).