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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.