Re: [PATCH v4 1/2] drivers/perf: hisi: Add support for HiSilicon NoC PMU

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

On Thu, 17 Jul 2025 15:41:37 +0800
Yicong Yang <yangyicong@huawei.com> wrote:

> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> Adds the support for HiSilicon NoC (Network on Chip) PMU which
> will be used to monitor the events on the system bus. The PMU
> device will be named after the SCL ID (either Super CPU cluster
> or Super IO cluster) and the index ID, just similar to other
> HiSilicon Uncore PMUs. Below PMU formats are provided besides
> the event:
> 
> - ch: the transaction channel (data, request, response, etc) which
>   can be used to filter the counting.
> - tt_en: tracetag filtering enable. Just as other HiSilicon Uncore
>   PMUs the NoC PMU supports only counting the transactions with
>   tracetag.
> 
> The NoC PMU doesn't have an interrupt to indicate the overflow.
> However we have a 64 bit counter which is large enough and it's
> nearly impossible to overflow.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>

Hi.

Just one trivial thing that I missed in internal review yesterday.
Not worth a respin though!

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> diff --git a/drivers/perf/hisilicon/hisi_uncore_noc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_noc_pmu.c
> new file mode 100644
> index 000000000000..2e3eb7881caa
> --- /dev/null
> +++ b/drivers/perf/hisilicon/hisi_uncore_noc_pmu.c

> +
> +static struct hisi_noc_pmu_regs hisi_noc_v1_pmu_regs = {
> +	.version = NOC_PMU_VERSION,
> +	.pmu_ctrl = NOC_PMU_GLOBAL_CTRL,
> +	.event_ctrl0 = NOC_PMU_EVENT_CTRL0,
> +	.event_cntr0 = NOC_PMU_EVENT_COUNTER0,
> +	.ovflow_status = NOC_PMU_CNT_INFO,

Trivial but if you are spinning a v5 for some I'd just spell out
as .overflow_status.

I've never seen any consistency in abbreviations for overflow!

> +};
> +
> +static const struct hisi_pmu_dev_info hisi_noc_v1 = {
> +	.attr_groups = hisi_noc_pmu_attr_groups,
> +	.counter_bits = 64,
> +	.check_event = NOC_PMU_EVENT_CTRL_TYPE,
> +	.private = &hisi_noc_v1_pmu_regs,
> +};