[PATCH 0/8] arm64:perf: Support for Hisilicon SoC Hardware event counters

[PATCH 0/8] arm64:perf: Support for Hisilicon SoC Hardware event counters

[Anurup M]

Provide Support for Hisilicon SoC Hardware event counters.
The Hisilicon SoC Hi161x series has many uncore or non-CPU performance
events and counters units.

This patch is implemented refering to arm-cci, Intel/AMD uncore and
also the cavium thunderX pmu patches.

Support for Hisilicon L3 cache(LLC) hardware events and counters are added
in this implementation.

The Hisilicon PMU datastructures are designed so as to support uncore
and also CPU specific events in future.

The Hisilicon LLC has four banks for a Super CPU Cluster(consists of
 16 CPU cores) and each LLC bank has separate hardware events and counters.
In the current implementation, the count from all these banks are summarized
and total count is output.

The Hisilicon uncore PMUs can be found under /sys/bus/event_source/devices.
The counters are exported via sysfs in the corresponding events files
under the PMU directory so the perf tool can list the event names.

Note:
This is very initial patchset for Hisilicon SoC PMU support shared for
review. Please review and share comments.
This patchset depends on the Hisilicon Djtag driver for r/w access to
Hisilicon SoC PMU registers, which is not upstreamed yet. The patches for
Djtag driver shall be send for review soon.

TODO:
        1. Counter overflow interrupt handling support to be added.
        2. CPU notifier to migrate to available online CPU's in case
           of CPU DOWN. Also mapping CPU cores to the current SCCL.
        3. Support for counting of individual LLC banks

Anurup M (8):
  arm64:perf: Add Devicetree bindings for Hisilicon SoC PMU
  arm64:MAINTAINERS:hisi: Add hisilicon SoC PMU support
  arm64:perf: Update Kconfig for Hisilicon PMU support
  arm64:perf: Add support for Hisilicon SoC event counters
  arm64:perf: L3 cache(LLC) event counting in perf
  arm64:perf: Makefile for Hisilicon ARMv8 PMU
  arm64:perf: Update Makefile for Hisilicon PMU support
  arm64:perf: L3 cache(LLC) event listing in perf

 .../devicetree/bindings/arm/hisilicon/pmu.txt      |  32 ++
 MAINTAINERS                                        |   8 +
 drivers/perf/Kconfig                               |   9 +
 drivers/perf/Makefile                              |   1 +
 drivers/perf/hisilicon/Makefile                    |   1 +
 drivers/perf/hisilicon/hisi_uncore_llc.c           | 613 +++++++++++++++++++++
 drivers/perf/hisilicon/hisi_uncore_llc.h           | 100 ++++
 drivers/perf/hisilicon/hisi_uncore_pmu.c           | 521 +++++++++++++++++
 drivers/perf/hisilicon/hisi_uncore_pmu.h           | 144 +++++
 9 files changed, 1429 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
 create mode 100644 drivers/perf/hisilicon/Makefile
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_llc.c
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_llc.h
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.c
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.h

-- 
2.1.4

[PATCH 1/8] arm64:perf: Add Devicetree bindings for Hisilicon SoC PMU

[Anurup M]

	1) Device tree bindings for Hisilicon PMU.
	2) Add example for Hisilicon LLC PMU.

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 .../devicetree/bindings/arm/hisilicon/pmu.txt      | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/pmu.txt

diff --git a/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
new file mode 100644
index 0000000..7584a81
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
@@ -0,0 +1,32 @@
+Hisilicon SoC HIP05 ARMv8 PMU
+
+Required Properties:
+	- compatible : This field contain two values. The first value is
+		always "hisilicon" and second value is the PMU type as shown
+		in below examples:
+		(a) "hisilicon,hip05-llc" for Hisiliocn SoC L3 cache PMU
+		(b) "hisilicon,hip05-ddrc" for Hisiliocn SoC DDRC PMU
+		(c) "hisilicon,hip05-mn" for Hisiliocn SoC MN PMU
+
+Optional Properties:
+
+	- djtag	: Some PMU registers are accessed via the Djtag interface
+		This field contains two values. The first value is the djtag
+		node phandle and second value is the Super CPU Cluster ID.
+
+	- interrupt-parent : A phandle indicating which interrupt controller
+		this PMU signals interrupts to.
+
+	- interrupts : Interrupt lines used by this PMU. If the PMU has
+		multiple banks, then all IRQ lines are listed in this
+		property.
+
+Example:
+	llc0: llc at 0 {
+		compatible = "hisilicon,hip05-llc";
+		djtag = <&djtag0 2>; /* DJTAG node for Super CPU Cluster 2
+				      * (starts from 1) */
+		interrupt-parent = <&mbigen_pc>;
+		interrupts = <141 4>,<142 4>,
+			 <143 4>,<144 4>; /* IRQ lines for 4 L3 cache banks */
+	};
-- 
2.1.4

[PATCH 2/8] arm64:MAINTAINERS:hisi: Add hisilicon SoC PMU support

[Anurup M]

	Add support for Hisilicon SoC hardware event counters.
	Support ARM64 architecture.

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c51c736..cf7dc0c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5405,6 +5405,14 @@ F:	include/uapi/linux/if_hippi.h
 F:	net/802/hippi.c
 F:	drivers/net/hippi/
 
+HISILICON PMU DRIVER
+M:	Anurup M <anurup.m@huawei.com>
+M:	Shaokun Zhang <zhangshaokun@hisilicon.com>
+W:	http://www.hisilicon.com
+S:	Supported
+F:	Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
+F:	drivers/perf/hisilicon/*
+
 HISILICON SAS Controller
 M:	John Garry <john.garry@huawei.com>
 W:	http://www.hisilicon.com
-- 
2.1.4

[PATCH 3/8] arm64:perf: Update Kconfig for Hisilicon PMU support

[Anurup M]

	1. Update Kconfig for Hisilicon SoC ARMv8 PMU support.

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 drivers/perf/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 04e2653..a90b2fc 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -12,4 +12,13 @@ config ARM_PMU
 	  Say y if you want to use CPU performance monitors on ARM-based
 	  systems.
 
+config HISI_PERFCTR
+	bool "Enable hardware event counter support for HiSilicon SoC"
+	depends on HW_PERF_EVENTS && ARM64
+	select HISI_DJTAG
+	default n
+	help
+	  Enable hardware event counter support for hardware event counters
+	  in Hisilicon Hi161x SoC. The hardware modules like LLC, MN1 and
+	  DDRC have hardware events and counters.
 endmenu
-- 
2.1.4

[PATCH 4/8] arm64:perf: Add support for Hisilicon SoC event counters

[Anurup M]

	1. Hisilicon SoC PMU to support different hardware event
	   counters.
	2. Register with Perf PMU as RAW events.
	3. Hisilicon PMU shall use the DJTAG hardware interface
	   to access hardware event counters and configuration
	   register.
	4. Routines to initialze and setup PMU.
	5. Routines to enable/disable/add/del/start/stop hardware
	   event counting.

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 465 +++++++++++++++++++++++++++++++
 drivers/perf/hisilicon/hisi_uncore_pmu.h | 125 +++++++++
 2 files changed, 590 insertions(+)
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.c
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.h

diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
new file mode 100644
index 0000000..40bdc23
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -0,0 +1,465 @@
+/*
+ * HiSilicon SoC Hardware event counters support
+ *
+ * Copyright (C) 2016 Huawei Technologies Limited
+ * Author: Anurup M <anurup.m@huawei.com>
+ *
+ * This code is based heavily on the ARMv7 perf event code.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/bitmap.h>
+#include <linux/of.h>
+#include <linux/perf_event.h>
+#include "hisi_uncore_pmu.h"
+
+/* djtag read interface - Call djtag driver to access SoC registers */
+int hisi_djtag_readreg(int module_id, int bank, u32 offset,
+				struct device_node *djtag_node, u32 *pvalue)
+{
+	int ret;
+	u32 chain_id = 0;
+
+	while (bank != 1) {
+		bank = (bank >> 0x1);
+		chain_id++;
+	}
+
+	ret = djtag_readl(djtag_node, offset, module_id, chain_id, pvalue);
+	if (ret)
+		pr_warn("Djtag:%s Read failed!\n", djtag_node->full_name);
+
+	return ret;
+}
+
+/* djtag write interface - Call djtag driver  to access SoC registers */
+int hisi_djtag_writereg(int module_id, int bank,
+				u32 offset, u32 value,
+				struct device_node *djtag_node)
+{
+	int ret;
+
+	ret = djtag_writel(djtag_node, offset, module_id, 0, value);
+	if (ret)
+		pr_warn("Djtag:%s Write failed!\n", djtag_node->full_name);
+
+	return ret;
+}
+
+void hisi_uncore_pmu_write_evtype(struct hisi_hwmod_unit *punit,
+						int idx, u32 val)
+{
+	/* TBD: Select event based on Hardware counter Module */
+}
+
+int hisi_pmu_get_event_idx(struct hw_perf_event *hwc,
+						struct hisi_hwmod_unit *punit)
+{
+	int event_idx = -1;
+
+	/* TBD - Get the available hardware event counter index */
+
+	return event_idx;
+}
+
+void hisi_pmu_clear_event_idx(struct hw_perf_event *hwc,
+					struct hisi_hwmod_unit *punit,
+								int idx)
+{
+	/* TBD - Release the hardware event counter index */
+}
+
+static int
+__hw_perf_event_init(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	int mapping;
+
+	mapping = pmu_map_event(event);
+	if (mapping < 0) {
+		pr_debug("event %x:%llx not supported\n", event->attr.type,
+							 event->attr.config);
+		return mapping;
+	}
+
+	/*
+	 * We don't assign an index until we actually place the event onto
+	 * hardware. Use -1 to signify that we haven't decided where to put it
+	 * yet.
+	 */
+	hwc->idx		= -1;
+	hwc->config_base	= 0;
+	hwc->config		= 0;
+	hwc->event_base		= 0;
+
+	/*
+	 * For HiSilicon SoC store the event encoding into the config_base
+	 * field.
+	 */
+	hwc->config_base = event->attr.config;
+
+	/*
+	 * Limit the sample_period to half of the counter width. That way, the
+	 * new counter value is far less likely to overtake the previous one
+	 * unless you have some serious IRQ latency issues.
+	 */
+	hwc->sample_period  = HISI_MAX_PERIOD >> 1;
+	hwc->last_period    = hwc->sample_period;
+	local64_set(&hwc->period_left, hwc->sample_period);
+
+	/* TBD: Initialize event counter variables to support multiple
+	 * HiSilicon Soc SCCL and banks
+	 */
+
+	return 0;
+}
+
+void hw_perf_event_destroy(struct perf_event *event)
+{
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit;
+	atomic_t *active_events;
+	struct mutex *reserve_mutex;
+	u32 unit_idx = GET_UNIT_IDX(event->hw.config_base);
+
+	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
+	active_events = &punit->active_events;
+	reserve_mutex = &punit->reserve_mutex;
+
+	if (atomic_dec_and_mutex_lock(active_events, reserve_mutex)) {
+		/* FIXME: Release IRQ here */
+		mutex_unlock(reserve_mutex);
+	}
+}
+
+int hisi_uncore_pmu_event_init(struct perf_event *event)
+{
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit;
+	u32 raw_event_code = event->attr.config;
+	u32 unit_idx = GET_UNIT_IDX(raw_event_code);
+	int err;
+
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/* we do not support sampling */
+	if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
+		return -EOPNOTSUPP;
+
+	/* counters do not have these bits */
+	if (event->attr.exclude_user	||
+	    event->attr.exclude_kernel	||
+	    event->attr.exclude_host	||
+	    event->attr.exclude_guest	||
+	    event->attr.exclude_hv	||
+	    event->attr.exclude_idle)
+		return -EINVAL;
+
+	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
+
+	event->destroy = hw_perf_event_destroy;
+
+	err = __hw_perf_event_init(event);
+	if (err)
+		hw_perf_event_destroy(event);
+
+	return err;
+}
+
+
+/* Read hardware counter and update the Perf counter statistics */
+u64 hisi_uncore_pmu_event_update(struct perf_event *event,
+					struct hw_perf_event *hwc,
+							int idx) {
+	u64 new_raw_count = 0;
+	/*
+	 * TBD: Identify Event type and read appropriate hardware
+	 * counter and sum the values
+	 */
+
+	return new_raw_count;
+}
+
+void hisi_uncore_pmu_enable(struct pmu *pmu)
+{
+	/* TBD: Enable all the PMU counters. */
+}
+
+void hisi_uncore_pmu_disable(struct pmu *pmu)
+{
+	/* TBD: Disable all the PMU counters. */
+}
+
+int hisi_pmu_enable_counter(struct hisi_hwmod_unit *punit,
+							int idx)
+{
+	int ret = 0;
+
+	/* TBD - Enable the hardware event counting */
+
+	return ret;
+}
+
+void hisi_pmu_disable_counter(struct hisi_hwmod_unit *punit,
+							int idx)
+{
+	/* TBD - Disable the hardware event counting */
+}
+
+void hisi_uncore_pmu_enable_event(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit;
+	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
+	int idx = hwc->idx;
+
+	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
+
+	/*
+	 * Enable counter and set the counter to count
+	 * the event that we're interested in.
+	 */
+
+	/*
+	 * Disable counter
+	 */
+	hisi_pmu_disable_counter(punit, idx);
+
+	/*
+	 * Set event (if destined for Hisilicon SoC counters).
+	 */
+	hisi_uncore_pmu_write_evtype(punit, idx, hwc->config_base);
+
+	/*
+	 * Enable counter
+	 */
+	hisi_pmu_enable_counter(punit, idx);
+
+}
+
+int hisi_pmu_write_counter(struct hisi_hwmod_unit *punit,
+						int idx,
+						u32 value)
+{
+	int ret = 0;
+
+	/* TBD - Write to the hardware event counter */
+
+	return ret;
+}
+
+void hisi_pmu_event_set_period(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit;
+	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
+	int idx = hwc->idx;
+
+	/*
+	 * The Hisilicon PMU counters have a period of 2^32. To account for the
+	 * possiblity of extreme interrupt latency we program for a period of
+	 * half that. Hopefully we can handle the interrupt before another 2^31
+	 * events occur and the counter overtakes its previous value.
+	 */
+	u64 val = 1ULL << 31;
+
+	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
+	local64_set(&hwc->prev_count, val);
+	hisi_pmu_write_counter(punit, idx, val);
+}
+
+void hisi_uncore_pmu_start(struct perf_event *event,
+						int pmu_flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit;
+	struct hisi_pmu_hw_events *hw_events;
+	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
+	unsigned long flags;
+
+	if (unit_idx >= (HISI_SCCL_MAX - 1)) {
+		pr_err("Invalid unitID=%d in event code=%lu!\n",
+					unit_idx + 1, hwc->config_base);
+		return;
+	}
+
+	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
+	hw_events = &punit->hw_events;
+
+	/*
+	 * To handle interrupt latency, we always reprogram the period
+	 * regardlesss of PERF_EF_RELOAD.
+	 */
+	if (pmu_flags & PERF_EF_RELOAD)
+		WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
+
+	hwc->state = 0;
+
+	raw_spin_lock_irqsave(&hw_events->pmu_lock, flags);
+
+	hisi_pmu_event_set_period(event);
+	hisi_uncore_pmu_enable_event(event);
+
+	raw_spin_unlock_irqrestore(&hw_events->pmu_lock, flags);
+}
+
+void hisi_uncore_pmu_stop(struct perf_event *event,
+						int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit = NULL;
+	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
+	int idx = hwc->idx;
+
+	if (hwc->state & PERF_HES_STOPPED)
+		return;
+
+	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
+
+	/*
+	 * We always reprogram the counter, so ignore PERF_EF_UPDATE. See
+	 * hisi_uncore_pmu_start()
+	 */
+	hisi_pmu_disable_counter(punit, idx);
+	hisi_uncore_pmu_event_update(event, hwc, idx);
+	hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
+
+}
+
+int hisi_uncore_pmu_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisipmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit;
+	struct hisi_pmu_hw_events *hw_events;
+	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
+	int idx, err = 0;
+
+	if (unit_idx >= (HISI_SCCL_MAX - 1)) {
+		pr_err("Invalid unitID=%d in event code=%lu\n",
+					unit_idx + 1, hwc->config_base);
+		return -EINVAL;
+	}
+
+	punit = &phisipmu->hwmod_pmu_unit[unit_idx];
+	hw_events = &punit->hw_events;
+
+	/* If we don't have a free counter then return early. */
+	idx = hisi_pmu_get_event_idx(hwc, punit);
+	if (idx < 0) {
+		err = idx;
+		goto out;
+	}
+
+	event->hw.idx = idx;
+	hw_events->events[idx] = event;
+
+	hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
+	if (flags & PERF_EF_START)
+		hisi_uncore_pmu_start(event, PERF_EF_RELOAD);
+
+	/* Propagate our changes to the userspace mapping. */
+	perf_event_update_userpage(event);
+
+out:
+	return err;
+}
+
+void hisi_uncore_pmu_del(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisipmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit;
+	struct hisi_pmu_hw_events *hw_events;
+	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
+	int idx = hwc->idx;
+
+	punit = &phisipmu->hwmod_pmu_unit[unit_idx];
+	hw_events = &punit->hw_events;
+
+	hisi_uncore_pmu_stop(event, PERF_EF_UPDATE);
+	hw_events->events[idx] = NULL;
+
+	hisi_pmu_clear_event_idx(hwc, punit, idx);
+
+	perf_event_update_userpage(event);
+}
+
+int pmu_map_event(struct perf_event *event)
+{
+	return (int)(event->attr.config & HISI_EVTYPE_EVENT);
+}
+
+struct hisi_pmu *hisi_pmu_alloc(struct platform_device *pdev)
+{
+	struct hisi_pmu *phisipmu;
+
+	phisipmu = devm_kzalloc(&pdev->dev, sizeof(*phisipmu), GFP_KERNEL);
+	if (!phisipmu)
+		return ERR_PTR(-ENOMEM);
+
+	return phisipmu;
+}
+
+int hisi_pmu_unit_init(struct platform_device *pdev,
+				struct hisi_hwmod_unit *punit,
+						int unit_id,
+						int num_counters)
+{
+	punit->hw_events.events = devm_kcalloc(&pdev->dev,
+				     num_counters,
+				     sizeof(*punit->hw_events.events),
+							     GFP_KERNEL);
+	if (!punit->hw_events.events)
+		return -ENOMEM;
+
+	raw_spin_lock_init(&punit->hw_events.pmu_lock);
+	atomic_set(&punit->active_events, 0);
+	mutex_init(&punit->reserve_mutex);
+
+	punit->unit_id = unit_id;
+
+	return 0;
+}
+
+void hisi_uncore_pmu_read(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	int idx = hwc->idx;
+
+	hisi_uncore_pmu_event_update(event, hwc, idx);
+}
+
+int hisi_uncore_pmu_setup(struct hisi_pmu *hisipmu,
+				struct platform_device *pdev,
+						char *pmu_name)
+{
+	int ret;
+
+	/* Register the events are RAW events to support RAW events too */
+	ret = perf_pmu_register(&hisipmu->pmu, pmu_name, PERF_TYPE_RAW);
+	if (ret)
+		goto fail;
+
+	return 0;
+
+fail:
+	return ret;
+}
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
new file mode 100644
index 0000000..7fbd09c
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -0,0 +1,125 @@
+/*
+ * HiSilicon SoC Hardware event counters support
+ *
+ * Copyright (C) 2016 Huawei Technologies Limited
+ * Author: Anurup M <anurup.m@huawei.com>
+ *
+ * This code is based heavily on the ARMv7 perf event code.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __HISI_UNCORE_PMU_H__
+#define __HISI_UNCORE_PMU_H__
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <asm/local64.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <asm/hisi-djtag.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt)     "hisi_pmu: " fmt
+
+#define HISI_SCCL_MAX	(1 << 4)
+#define HISI_SCCL_MASK	(0xF00000)
+#define HISI_SCCL_SHIFT 20
+
+#define HISI_EVTYPE_EVENT	0xfff
+#define HISI_MAX_PERIOD ((1LLU << 32) - 1)
+
+#define MAX_COUNTERS 30
+#define MAX_UNITS 8
+
+enum hisi_hwmod_type {
+	HISI_LLC = 0x0,
+};
+
+/* Event granularity */
+enum hisi_pmu_type {
+	CORE_SPECIFIC,
+	CCL_SPECIFIC,
+	SCCL_SPECIFIC,
+	SOCKET_SPECIFIC,
+};
+
+struct hisi_pmu_hw_events {
+	struct perf_event **events;
+	raw_spinlock_t pmu_lock;
+};
+
+/* Hardware module information */
+struct hisi_hwmod_unit {
+	   int unit_id;
+	   struct hisi_pmu_hw_events hw_events;
+	   atomic_t active_events;
+	   struct mutex reserve_mutex;
+	   void *hwmod_data;
+};
+
+/* Generic pmu struct for different pmu types */
+struct hisi_pmu {
+	const char *name;
+	enum hisi_pmu_type pmu_type;
+	enum hisi_hwmod_type hwmod_type;
+	int num_counters;
+	int	num_events;
+	struct perf_event *events[MAX_COUNTERS];
+	int num_units;
+	int (*check_event)(u64);
+	struct hisi_hwmod_unit hwmod_pmu_unit[MAX_UNITS];
+	struct pmu pmu; /* for custom pmu ops */
+	struct platform_device *plat_device;
+};
+
+#define to_hisi_pmu(c)	(container_of(c, struct hisi_pmu, pmu))
+
+#define GET_UNIT_IDX(event_code)		\
+	(((event_code & HISI_SCCL_MASK) >>	\
+			   HISI_SCCL_SHIFT) - 1)
+
+int pmu_map_event(struct perf_event *);
+u64 hisi_pmu_event_update(struct perf_event *,
+				struct hw_perf_event *, int);
+int hisi_pmu_enable_counter(struct hisi_hwmod_unit *, int);
+void hisi_pmu_disable_counter(struct hisi_hwmod_unit *, int);
+int hisi_pmu_write_counter(struct hisi_hwmod_unit *, int, u32);
+void hisi_pmu_write_evtype(int, u32);
+int hisi_pmu_get_event_idx(struct hw_perf_event *,
+				struct hisi_hwmod_unit *);
+void hisi_pmu_clear_event_idx(struct hw_perf_event *,
+				struct hisi_hwmod_unit *, int);
+void hisi_uncore_pmu_read(struct perf_event *);
+void hisi_uncore_pmu_del(struct perf_event *, int);
+int hisi_uncore_pmu_add(struct perf_event *, int);
+void hisi_uncore_pmu_start(struct perf_event *, int);
+void hisi_uncore_pmu_stop(struct perf_event *, int);
+void hisi_pmu_event_set_period(struct perf_event *);
+void hisi_uncore_pmu_enable_event(struct perf_event *);
+void hisi_uncore_pmu_disable_event(struct perf_event *);
+void hisi_uncore_pmu_enable(struct pmu *);
+void hisi_uncore_pmu_disable(struct pmu *);
+struct hisi_pmu *hisi_uncore_pmu_alloc(struct platform_device *);
+int hisi_uncore_pmu_setup(struct hisi_pmu *hisi_pmu,
+				struct platform_device *, char *);
+void hisi_uncore_pmu_write_evtype(struct hisi_hwmod_unit *, int, u32);
+int hisi_uncore_pmu_event_init(struct perf_event *);
+int hisi_djtag_readreg(int, int, u32, struct device_node *, u32 *);
+int hisi_djtag_writereg(int, int, u32, u32, struct device_node *);
+int hisi_pmu_unit_init(struct platform_device *,
+				struct hisi_hwmod_unit *,
+						int, int);
+struct hisi_pmu *hisi_pmu_alloc(struct platform_device *);
+#endif /* __HISI_UNCORE_PMU_H__ */
-- 
2.1.4

[PATCH 5/8] arm64:perf: L3 cache(LLC) event counting in perf

[Anurup M]

	1. Add support to count L3 cache(LLC) hardware events.
	2. Add events as RAW events.
	  The events to count can be selected as RAW event
	  referring to the hardware user manual.
	  e.g.: To input as RAW events the RAW event code is
		-e r124f301
	  The RAW event encoding followed is
	  <socket ID(2 bit)><SCCL ID(4 bit)><ModuleID(4 bit)>
	  <Bank(4 bit)><event code(12 bit)>
	  So 0x1 is SocketID, 0x2 is SCCL ID, 0x4 is the LLC
	  hardware module ID, 0xf is for sum of all LLC banks,
	  0x301 is the event code for LLC_READ_ALLOCATE.
	3. Routines to read/write/start/stop/count the hardware events
	   counting

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_llc.c | 542 +++++++++++++++++++++++++++++++
 drivers/perf/hisilicon/hisi_uncore_llc.h | 100 ++++++
 drivers/perf/hisilicon/hisi_uncore_pmu.c |  66 +++-
 drivers/perf/hisilicon/hisi_uncore_pmu.h |   3 +
 4 files changed, 700 insertions(+), 11 deletions(-)
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_llc.c
 create mode 100644 drivers/perf/hisilicon/hisi_uncore_llc.h

diff --git a/drivers/perf/hisilicon/hisi_uncore_llc.c b/drivers/perf/hisilicon/hisi_uncore_llc.c
new file mode 100644
index 0000000..a771e1a
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_llc.c
@@ -0,0 +1,542 @@
+/*
+ * HiSilicon SoC LLC Hardware event counters support
+ *
+ * Copyright (C) 2016 Huawei Technologies Limited
+ * Author: Anurup M <anurup.m@huawei.com>
+ *
+ * This code is based heavily on the ARMv7 perf event code.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/module.h>
+#include <linux/bitmap.h>
+#include <linux/of.h>
+#include <linux/perf_event.h>
+#include "hisi_uncore_llc.h"
+
+/* Map cfg_en values for LLC Banks */
+const int llc_cfgen_map[] = { HISI_LLC_BANK0_CFGEN, HISI_LLC_BANK1_CFGEN,
+				HISI_LLC_BANK2_CFGEN, HISI_LLC_BANK3_CFGEN
+};
+
+struct hisi_pmu *hisi_uncore_llc;
+
+static inline int hisi_llc_counter_valid(int idx)
+{
+	return (idx >= HISI_IDX_LLC_COUNTER0 &&
+			idx <= HISI_IDX_LLC_COUNTER_MAX);
+}
+
+u64 hisi_llc_event_update(struct perf_event *event,
+				struct hw_perf_event *hwc, int idx)
+{
+	struct device_node *djtag_node;
+	struct hisi_pmu *pllc_pmu = to_hisi_pmu(event->pmu);
+	struct hisi_hwmod_unit *punit;
+	struct hisi_llc_data *llc_hwmod_data;
+	u64 delta, prev_raw_count, new_raw_count = 0;
+	int cfg_en;
+	u32 raw_event_code = hwc->config_base;
+	u32 scclID = (raw_event_code & HISI_SCCL_MASK) >> 20;
+	u32 llc_idx = scclID - 1;
+	int i;
+
+	if (!scclID || (scclID >= HISI_SCCL_MASK)) {
+		pr_err("Invalid SCCL=%d in event code!\n", scclID);
+		return 0;
+	}
+
+	if (!hisi_llc_counter_valid(idx)) {
+		pr_err("Unsupported event index:%d!\n", idx);
+		return 0;
+	}
+
+	punit = &pllc_pmu->hwmod_pmu_unit[llc_idx];
+	llc_hwmod_data = punit->hwmod_data;
+
+	/* Check if the LLC data is initialized for this SCCL */
+	if (!llc_hwmod_data->djtag_node) {
+		pr_err("SCCL=%d not initialized!\n", scclID);
+		return 0;
+	}
+
+	/* Find the djtag device node of the SCCL */
+	djtag_node = llc_hwmod_data->djtag_node;
+
+	do {
+		prev_raw_count = local64_read(&hwc->prev_count);
+		for (i = 0; i < llc_hwmod_data->num_banks; i++) {
+			cfg_en = llc_hwmod_data->bank[i].cfg_en;
+
+			new_raw_count =
+					hisi_read_llc_counter(idx,
+							djtag_node, cfg_en);
+			delta = (new_raw_count - prev_raw_count) &
+							HISI_MAX_PERIOD;
+
+			local64_add(delta, &event->count);
+
+			pr_debug("delta for event:0x%x is %llu\n",
+						raw_event_code, delta);
+		}
+	} while (local64_cmpxchg(
+			&hwc->prev_count, prev_raw_count, new_raw_count) !=
+								prev_raw_count);
+
+	return new_raw_count;
+}
+
+/*
+ * Initialize LLC hardware before counting.
+ */
+int hisi_init_llc_hw_perf_event(struct hisi_pmu *pllc_pmu,
+					struct hw_perf_event *hwc)
+{
+	struct hisi_hwmod_unit *punit;
+	u32 raw_event_code = hwc->config_base;
+	atomic_t *active_events;
+	u32 scclID = (raw_event_code & HISI_SCCL_MASK) >> HISI_SCCL_SHIFT;
+	u32 llc_idx = scclID - 1;
+
+	if ((scclID == 0) || (scclID >= HISI_SCCL_MAX)) {
+		pr_err("Invalid SCCL=%d in event code=%d!\n",
+					scclID, raw_event_code);
+		return -EINVAL;
+	}
+
+	punit = &pllc_pmu->hwmod_pmu_unit[llc_idx];
+	active_events = &punit->active_events;
+
+	/* Increment the active_events for the first event counting */
+	if (!atomic_inc_not_zero(active_events)) {
+		mutex_lock(&punit->reserve_mutex);
+		if (atomic_read(active_events) == 0)
+			atomic_inc(active_events);
+		mutex_unlock(&punit->reserve_mutex);
+	}
+
+	return 0;
+}
+
+void hisi_set_llc_evtype(struct hisi_llc_data *llc_hwmod_data,
+						int idx, u32 val)
+{
+	struct device_node *djtag_node;
+	u32 reg_offset;
+	u32 value = 0;
+	int cfg_en;
+	u32 event_value;
+	int i;
+
+	event_value = (val -
+			HISI_HWEVENT_LLC_READ_ALLOCATE);
+
+	/* Select the appropriate Event select register */
+	if (idx <= 3)
+		reg_offset = HISI_LLC_EVENT_TYPE0_REG_OFF;
+	else
+		reg_offset = HISI_LLC_EVENT_TYPE1_REG_OFF;
+
+	/* Value to write to event type register */
+	val = event_value << (8 * idx);
+
+	/* Find the djtag device node of the Unit */
+	djtag_node = llc_hwmod_data->djtag_node;
+
+	/*
+	 * Set the event in LLC_EVENT_TYPEx Register
+	 * for all LLC banks
+	 */
+	for (i = 0; i < llc_hwmod_data->num_banks; i++) {
+		cfg_en = llc_hwmod_data->bank[i].cfg_en;
+		hisi_djtag_readreg(HISI_LLC_MODULE_ID,
+				cfg_en,
+				reg_offset,
+				djtag_node, &value);
+
+		value &= ~(0xff << (8 * idx));
+		value |= val;
+
+		hisi_djtag_writereg(HISI_LLC_MODULE_ID,
+				cfg_en,
+				reg_offset,
+				value,
+				djtag_node);
+	}
+}
+
+u32 hisi_write_llc_counter(struct hisi_llc_data *llc_hwmod_data,
+					int idx, u32 value)
+{
+	struct device_node *djtag_node;
+	int cfg_en;
+	u32 reg_offset = 0;
+	int i, ret = 0;
+
+	if (!hisi_llc_counter_valid(idx)) {
+		pr_err("Unsupported event index:%d!\n", idx);
+		return -EINVAL;
+	}
+
+	reg_offset = HISI_LLC_COUNTER0_REG_OFF +
+					(idx * 4);
+
+	/* Find the djtag device node of the Unit */
+	djtag_node = llc_hwmod_data->djtag_node;
+
+	for (i = 0; i < llc_hwmod_data->num_banks; i++) {
+		cfg_en = llc_hwmod_data->bank[i].cfg_en;
+		ret = hisi_djtag_writereg(HISI_LLC_MODULE_ID,
+					cfg_en,
+					reg_offset,
+					value,
+					djtag_node);
+		if (!ret)
+			ret = value;
+	}
+
+	return ret;
+}
+
+int hisi_enable_llc_counter(struct hisi_llc_data *llc_hwmod_data, int idx)
+{
+	struct device_node *djtag_node;
+	u32 value = 0;
+	int cfg_en;
+	int i, ret = 0;
+
+	if (!hisi_llc_counter_valid(idx)) {
+		pr_err("Unsupported event index:%d!\n", idx);
+		return -EINVAL;
+	}
+
+	/* Find the djtag device node of the Unit */
+	djtag_node = llc_hwmod_data->djtag_node;
+
+	/*
+	 * Set the event_bus_en bit in LLC AUCNTRL to enable counting
+	 * for all LLC banks
+	 */
+	for (i = 0; i < llc_hwmod_data->num_banks; i++) {
+		cfg_en = llc_hwmod_data->bank[i].cfg_en;
+		ret = hisi_djtag_readreg(HISI_LLC_MODULE_ID,
+				cfg_en,
+				HISI_LLC_AUCTRL_REG_OFF,
+				djtag_node, &value);
+
+		value |= HISI_LLC_AUCTRL_EVENT_BUS_EN;
+		ret = hisi_djtag_writereg(HISI_LLC_MODULE_ID,
+				cfg_en,
+				HISI_LLC_AUCTRL_REG_OFF,
+				value,
+				djtag_node);
+	}
+
+	return ret;
+}
+
+void hisi_disable_llc_counter(struct hisi_llc_data *llc_hwmod_data, int idx)
+{
+	struct device_node *djtag_node;
+	u32 value = 0;
+	int cfg_en;
+	int i;
+
+	if (!hisi_llc_counter_valid(idx)) {
+		pr_err("Unsupported event index:%d!\n", idx);
+		return;
+	}
+
+	/* Find the djtag device node of the Unit */
+	djtag_node = llc_hwmod_data->djtag_node;
+
+	/*
+	 * Clear the event_bus_en bit in LLC AUCNTRL if no other
+	 * event counting for all LLC banks
+	 */
+	for (i = 0; i < llc_hwmod_data->num_banks; i++) {
+		cfg_en = llc_hwmod_data->bank[i].cfg_en;
+		hisi_djtag_readreg(HISI_LLC_MODULE_ID,
+				cfg_en,
+				HISI_LLC_AUCTRL_REG_OFF,
+				djtag_node, &value);
+
+		value &= ~(HISI_LLC_AUCTRL_EVENT_BUS_EN);
+		hisi_djtag_writereg(HISI_LLC_MODULE_ID,
+				cfg_en,
+				HISI_LLC_AUCTRL_REG_OFF,
+				value,
+				djtag_node);
+	}
+}
+
+void hisi_clear_llc_event_idx(struct hisi_hwmod_unit *punit,
+							int idx)
+{
+	struct device_node *djtag_node;
+	void *bitmap_addr;
+	struct hisi_llc_data *llc_hwmod_data = punit->hwmod_data;
+	u32 cfg_en, value, reg_offset;
+	int i;
+
+	if (!hisi_llc_counter_valid(idx)) {
+		pr_err("Unsupported event index:%d!\n", idx);
+		return;
+	}
+
+	bitmap_addr = llc_hwmod_data->hisi_llc_event_used_mask;
+
+	__clear_bit(idx, bitmap_addr);
+
+	/* Clear Counting in LLC event config register */
+	if (idx <= 3)
+		reg_offset = HISI_LLC_EVENT_TYPE0_REG_OFF;
+	else
+		reg_offset = HISI_LLC_EVENT_TYPE1_REG_OFF;
+
+	djtag_node = llc_hwmod_data->djtag_node;
+
+	/*
+	 * Clear the event in LLC_EVENT_TYPEx Register
+	 * for all LLC banks
+	 */
+	for (i = 0; i < llc_hwmod_data->num_banks; i++) {
+		cfg_en = llc_hwmod_data->bank[i].cfg_en;
+
+		hisi_djtag_readreg(HISI_LLC_MODULE_ID,
+				cfg_en,
+				reg_offset,
+				djtag_node, &value);
+
+		value &= ~(0xff << (8 * idx));
+		value |= (0xff << (8 * idx));
+		hisi_djtag_writereg(HISI_LLC_MODULE_ID,
+				cfg_en,
+				reg_offset,
+				value,
+				djtag_node);
+	}
+}
+
+int hisi_llc_get_event_idx(struct hisi_hwmod_unit *punit)
+{
+	struct hisi_llc_data *llc_hwmod_data = punit->hwmod_data;
+	int event_idx;
+
+	event_idx =
+		find_first_zero_bit(
+			llc_hwmod_data->hisi_llc_event_used_mask,
+					HISI_MAX_CFG_LLC_CNTR);
+
+	if (event_idx == HISI_MAX_CFG_LLC_CNTR)
+		return -EAGAIN;
+
+	__set_bit(event_idx,
+		llc_hwmod_data->hisi_llc_event_used_mask);
+
+	pr_debug("event_idx=%d\n", event_idx);
+
+	return event_idx;
+}
+
+u32 hisi_read_llc_counter(int idx, struct device_node *djtag_node, int bank)
+{
+	u32 reg_offset = 0;
+	u32 value;
+
+	if (!hisi_llc_counter_valid(idx)) {
+		pr_err("Unsupported event index:%d!\n", idx);
+		return -EINVAL;
+	}
+
+	reg_offset = HISI_LLC_COUNTER0_REG_OFF + (idx * 4);
+
+	hisi_djtag_readreg(HISI_LLC_MODULE_ID, /* ModuleID  */
+			bank,
+			reg_offset, /* Register Offset */
+			djtag_node, &value);
+
+	return value;
+}
+
+static int init_hisi_llc_banks(struct hisi_llc_data *pllc_data,
+				struct platform_device *pdev)
+{
+	int i;
+
+	pllc_data->num_banks = NUM_LLC_BANKS;
+	for (i = 0; i < NUM_LLC_BANKS; i++)
+		pllc_data->bank[i].cfg_en = llc_cfgen_map[i];
+
+	return 0;
+}
+
+static int init_hisi_llc_data(struct platform_device *pdev,
+					struct hisi_pmu *pllc_pmu,
+							int *punit_id)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *djtag_node;
+	struct hisi_hwmod_unit *punit;
+	struct hisi_llc_data *llc_hwmod_data;
+	struct of_phandle_args arg;
+	int ret, sccl_id;
+
+	ret = of_parse_phandle_with_fixed_args(node,
+						"djtag", 1, 0, &arg);
+	if (!ret) {
+		if (arg.args[0] > 0  && arg.args[0] <= MAX_UNITS) {
+			sccl_id = arg.args[0];
+			djtag_node = arg.np;
+			if (sccl_id >= MAX_UNITS) {
+				pr_err("llc_device_probe-Invalid SCCL=%d!\n",
+						sccl_id);
+				return -EINVAL;
+			}
+		} else
+			return -EINVAL;
+	} else {
+		pr_err("llc_device_probe-node without djtag!\n");
+		return -EINVAL;
+	}
+
+	llc_hwmod_data = kzalloc(sizeof(struct hisi_llc_data), GFP_KERNEL);
+	if (!llc_hwmod_data)
+		return -ENOMEM;
+
+	llc_hwmod_data->djtag_node = djtag_node;
+	punit = &pllc_pmu->hwmod_pmu_unit[sccl_id - 1];
+
+	ret = hisi_pmu_unit_init(pdev, punit, sccl_id,
+					HISI_MAX_CFG_LLC_CNTR);
+	if (ret) {
+		kfree(llc_hwmod_data);
+		return ret;
+	}
+
+	ret = init_hisi_llc_banks(llc_hwmod_data, pdev);
+	if (ret) {
+		kfree(llc_hwmod_data);
+		return ret;
+	}
+
+	punit->hwmod_data = llc_hwmod_data;
+
+	*punit_id = sccl_id - 1;
+	return 0;
+}
+
+static void hisi_free_llc_data(struct hisi_hwmod_unit *punit)
+{
+	kfree(punit->hwmod_data);
+}
+
+void hisi_llc_pmu_init(struct platform_device *pdev,
+					struct hisi_pmu *pllc_pmu)
+{
+	pllc_pmu->pmu_type = SCCL_SPECIFIC;
+	pllc_pmu->name = "HISI_L3C";
+	pllc_pmu->num_counters = HISI_MAX_CFG_LLC_CNTR;
+	pllc_pmu->num_events = HISI_LLC_MAX_EVENTS;
+	pllc_pmu->hwmod_type = HISI_LLC;
+}
+
+static int hisi_pmu_llc_dev_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct hisi_pmu *pllc_pmu = hisi_uncore_llc;
+	struct hisi_hwmod_unit *punit;
+	int unit_id;
+
+	/* Allocate and Register PMU for the first time */
+	if (!hisi_uncore_llc) {
+		pllc_pmu = hisi_pmu_alloc(pdev);
+		if (IS_ERR(pllc_pmu))
+			return PTR_ERR(pllc_pmu);
+		hisi_llc_pmu_init(pdev, pllc_pmu);
+	}
+
+	ret = init_hisi_llc_data(pdev, pllc_pmu, &unit_id);
+	if (ret)
+		goto fail_init;
+
+	pllc_pmu->num_units++;
+
+	pllc_pmu->plat_device = pdev;
+	hisi_llc_pmu_init(pdev, pllc_pmu);
+
+	if (!hisi_uncore_llc) {
+		/* First active LLC in the chip registers the pmu */
+		pllc_pmu->pmu = (struct pmu) {
+				.name		= "HISI-L3C-PMU",
+		/*		.task_ctx_nr	= perf_invalid_context, */
+				.pmu_enable = hisi_uncore_pmu_enable,
+				.pmu_disable = hisi_uncore_pmu_disable,
+				.event_init = hisi_uncore_pmu_event_init,
+				.add = hisi_uncore_pmu_add,
+				.del = hisi_uncore_pmu_del,
+				.start = hisi_uncore_pmu_start,
+				.stop = hisi_uncore_pmu_stop,
+				.read = hisi_uncore_pmu_read,
+		};
+
+		ret = hisi_uncore_pmu_setup(pllc_pmu, pdev, "hisi_l3c");
+		if (ret) {
+			pr_err("hisi_uncore_pmu_init FAILED!!\n");
+			goto fail;
+		}
+
+		hisi_uncore_llc = pllc_pmu;
+	}
+
+	return 0;
+
+fail:
+	punit = &pllc_pmu->hwmod_pmu_unit[unit_id];
+	hisi_free_llc_data(punit);
+
+fail_init:
+	if (!hisi_uncore_llc)
+		devm_kfree(&pdev->dev, pllc_pmu);
+
+	return ret;
+}
+
+static int hisi_pmu_llc_dev_remove(struct platform_device *pdev)
+{
+	if (hisi_uncore_llc)
+		devm_kfree(&pdev->dev, hisi_uncore_llc);
+
+	return 0;
+}
+
+static const struct of_device_id llc_of_match[] = {
+	{ .compatible = "hisilicon,hip05-llc", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, llc_of_match);
+
+static struct platform_driver hisi_pmu_llc_driver = {
+	.driver = {
+		.name = "hisi-llc-perf",
+		.of_match_table = llc_of_match,
+	},
+	.probe = hisi_pmu_llc_dev_probe,
+	.remove = hisi_pmu_llc_dev_remove,
+};
+module_platform_driver(hisi_pmu_llc_driver);
+
+MODULE_DESCRIPTION("HiSilicon ARMv8 LLC PMU driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Anurup M");
diff --git a/drivers/perf/hisilicon/hisi_uncore_llc.h b/drivers/perf/hisilicon/hisi_uncore_llc.h
new file mode 100644
index 0000000..beb4219
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_llc.h
@@ -0,0 +1,100 @@
+/*
+ * HiSilicon SoC LLC Hardware event counters support
+ *
+ * Copyright (C) 2016 Huawei Technologies Limited
+ * Author: Anurup M <anurup.m@huawei.com>
+ *
+ * This code is based heavily on the ARMv7 perf event code.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __HISI_UNCORE_LLC_H__
+#define __HISI_UNCORE_LLC_H__
+
+#include "hisi_uncore_pmu.h"
+
+/*
+ * ARMv8 HiSilicon LLC RAW event types.
+ */
+enum armv8_hisi_llc_event_types {
+	HISI_HWEVENT_LLC_READ_ALLOCATE		= 0x300,
+	HISI_HWEVENT_LLC_WRITE_ALLOCATE		= 0x301,
+	HISI_HWEVENT_LLC_READ_NOALLOCATE	= 0x302,
+	HISI_HWEVENT_LLC_WRITE_NOALLOCATE	= 0x303,
+	HISI_HWEVENT_LLC_READ_HIT		= 0x304,
+	HISI_HWEVENT_LLC_WRITE_HIT		= 0x305,
+	HISI_HWEVENT_LLC_EVENT_MAX		= 0x315,
+};
+
+/*
+ * ARMv8 HiSilicon Hardware counter Index.
+ */
+enum armv8_hisi_llc_counters {
+	HISI_IDX_LLC_COUNTER0		= 0x0,
+	HISI_IDX_LLC_COUNTER_MAX	= 0x7,
+};
+
+#define HISI_LLC_MODULE_ID	0x04
+
+#define HISI_LLC_BANK0_CFGEN  0x02
+#define HISI_LLC_BANK1_CFGEN  0x04
+#define HISI_LLC_BANK2_CFGEN  0x01
+#define HISI_LLC_BANK3_CFGEN  0x08
+
+#define HISI_LLC_AUCTRL_REG_OFF 0x04
+#define HISI_LLC_AUCTRL_EVENT_BUS_EN 0x1000000
+
+#define HISI_LLC_EVENT_TYPE0_REG_OFF 0x140
+#define HISI_LLC_EVENT_TYPE1_REG_OFF 0x144
+
+#define HISI_MAX_CFG_LLC_CNTR	0x08
+
+#define HISI_LLC_COUNTER0_REG_OFF 0x170
+
+#define HISI_LLC_MAX_EVENTS 22
+
+#define NUM_LLC_BANKS 4
+
+struct hisi_hwc_prev_counter {
+	local64_t prev_count;
+};
+
+struct hisi_llc_hwc_data_info {
+	u32 num_banks;
+	struct hisi_hwc_prev_counter *hwc_prev_counters;
+};
+
+struct llc_bank_info {
+	u32 cfg_en;
+};
+
+struct hisi_llc_data {
+	struct device_node *djtag_node;
+	DECLARE_BITMAP(hisi_llc_event_used_mask,
+				HISI_MAX_CFG_LLC_CNTR);
+	u32 num_banks;
+	struct llc_bank_info bank[MAX_BANKS];
+};
+
+int hisi_llc_get_event_idx(struct hisi_hwmod_unit *);
+void hisi_clear_llc_event_idx(struct hisi_hwmod_unit *,	int);
+void hisi_set_llc_evtype(struct hisi_llc_data *, int, u32);
+u32 hisi_read_llc_counter(int, struct device_node *, int);
+u32 hisi_write_llc_counter(struct hisi_llc_data *, int, u32);
+int hisi_init_llc_hw_perf_event(struct hisi_pmu *, struct hw_perf_event *);
+u64 hisi_llc_event_update(struct perf_event *,
+					struct hw_perf_event *, int);
+void hisi_disable_llc_counter(struct hisi_llc_data *, int);
+int hisi_enable_llc_counter(struct hisi_llc_data *, int);
+
+#endif /* __HISI_UNCORE_LLC_H__ */
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 40bdc23..e02cb81 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -24,6 +24,7 @@
 #include <linux/of.h>
 #include <linux/perf_event.h>
 #include "hisi_uncore_pmu.h"
+#include "hisi_uncore_llc.h"
 
 /* djtag read interface - Call djtag driver to access SoC registers */
 int hisi_djtag_readreg(int module_id, int bank, u32 offset,
@@ -61,15 +62,24 @@ int hisi_djtag_writereg(int module_id, int bank,
 void hisi_uncore_pmu_write_evtype(struct hisi_hwmod_unit *punit,
 						int idx, u32 val)
 {
-	/* TBD: Select event based on Hardware counter Module */
+	/* Select event based on Hardware counter Module */
+	if (idx >= HISI_IDX_LLC_COUNTER0 &&
+		idx <= HISI_IDX_LLC_COUNTER_MAX)
+		hisi_set_llc_evtype(punit->hwmod_data, idx, val);
 }
 
 int hisi_pmu_get_event_idx(struct hw_perf_event *hwc,
 						struct hisi_hwmod_unit *punit)
 {
 	int event_idx = -1;
+	u32 raw_event_code = hwc->config_base;
+	unsigned long evtype = raw_event_code & HISI_EVTYPE_EVENT;
 
-	/* TBD - Get the available hardware event counter index */
+	/* If event type is LLC events */
+	if (evtype >= HISI_HWEVENT_LLC_READ_ALLOCATE &&
+			evtype <= HISI_HWEVENT_LLC_EVENT_MAX) {
+		event_idx = hisi_llc_get_event_idx(punit);
+	}
 
 	return event_idx;
 }
@@ -78,14 +88,22 @@ void hisi_pmu_clear_event_idx(struct hw_perf_event *hwc,
 					struct hisi_hwmod_unit *punit,
 								int idx)
 {
-	/* TBD - Release the hardware event counter index */
+	u32 raw_event_code = hwc->config_base;
+	unsigned long evtype = raw_event_code & HISI_EVTYPE_EVENT;
+
+	if (evtype >= HISI_HWEVENT_LLC_READ_ALLOCATE &&
+			evtype <= HISI_HWEVENT_LLC_EVENT_MAX) {
+		hisi_clear_llc_event_idx(punit, idx);
+	}
 }
 
 static int
 __hw_perf_event_init(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
+	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
 	int mapping;
+	int err;
 
 	mapping = pmu_map_event(event);
 	if (mapping < 0) {
@@ -108,7 +126,11 @@ __hw_perf_event_init(struct perf_event *event)
 	 * For HiSilicon SoC store the event encoding into the config_base
 	 * field.
 	 */
-	hwc->config_base = event->attr.config;
+	/* For HiSilicon SoC LLC update config_base based on event encoding */
+	if (mapping >= HISI_HWEVENT_LLC_READ_ALLOCATE &&
+				mapping <= HISI_HWEVENT_LLC_EVENT_MAX) {
+		hwc->config_base = event->attr.config;
+	}
 
 	/*
 	 * Limit the sample_period to half of the counter width. That way, the
@@ -119,9 +141,18 @@ __hw_perf_event_init(struct perf_event *event)
 	hwc->last_period    = hwc->sample_period;
 	local64_set(&hwc->period_left, hwc->sample_period);
 
-	/* TBD: Initialize event counter variables to support multiple
-	 * HiSilicon Soc SCCL and banks
+	/* Initialize event counter variables to support multiple
+	 * HiSilicon SoC SCCL and banks
 	 */
+	if (mapping >= HISI_HWEVENT_LLC_READ_ALLOCATE &&
+				mapping <= HISI_HWEVENT_LLC_EVENT_MAX) {
+		/* Init LLC hardware */
+		err = hisi_init_llc_hw_perf_event(phisi_pmu, hwc);
+		if (err)
+			return err;
+	} else {
+		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -185,10 +216,15 @@ u64 hisi_uncore_pmu_event_update(struct perf_event *event,
 					struct hw_perf_event *hwc,
 							int idx) {
 	u64 new_raw_count = 0;
+	int cntr_idx = idx & ~(HISI_CNTR_SCCL_MASK);
+
 	/*
-	 * TBD: Identify Event type and read appropriate hardware
+	 * Identify Event type and read appropriate hardware
 	 * counter and sum the values
 	 */
+	if (cntr_idx >= HISI_IDX_LLC_COUNTER0 &&
+		cntr_idx <= HISI_IDX_LLC_COUNTER_MAX)
+		new_raw_count = hisi_llc_event_update(event, hwc, idx);
 
 	return new_raw_count;
 }
@@ -208,7 +244,9 @@ int hisi_pmu_enable_counter(struct hisi_hwmod_unit *punit,
 {
 	int ret = 0;
 
-	/* TBD - Enable the hardware event counting */
+	if (idx >= HISI_IDX_LLC_COUNTER0 &&
+		idx <= HISI_IDX_LLC_COUNTER_MAX)
+		ret = hisi_enable_llc_counter(punit->hwmod_data, idx);
 
 	return ret;
 }
@@ -216,7 +254,11 @@ int hisi_pmu_enable_counter(struct hisi_hwmod_unit *punit,
 void hisi_pmu_disable_counter(struct hisi_hwmod_unit *punit,
 							int idx)
 {
-	/* TBD - Disable the hardware event counting */
+	int cntr_idx = idx & ~(HISI_CNTR_SCCL_MASK);
+
+	if (cntr_idx >= HISI_IDX_LLC_COUNTER0 &&
+		 cntr_idx <= HISI_IDX_LLC_COUNTER_MAX)
+		hisi_disable_llc_counter(punit->hwmod_data, idx);
 }
 
 void hisi_uncore_pmu_enable_event(struct perf_event *event)
@@ -257,7 +299,9 @@ int hisi_pmu_write_counter(struct hisi_hwmod_unit *punit,
 {
 	int ret = 0;
 
-	/* TBD - Write to the hardware event counter */
+	if (idx >= HISI_IDX_LLC_COUNTER0 &&
+		idx <= HISI_IDX_LLC_COUNTER_MAX)
+		ret = hisi_write_llc_counter(punit->hwmod_data, idx, value);
 
 	return ret;
 }
@@ -324,7 +368,7 @@ void hisi_uncore_pmu_stop(struct perf_event *event,
 {
 	struct hw_perf_event *hwc = &event->hw;
 	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
-	struct hisi_hwmod_unit *punit = NULL;
+	struct hisi_hwmod_unit *punit;
 	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
 	int idx = hwc->idx;
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index 7fbd09c..dfc8c83 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -33,6 +33,8 @@
 #undef pr_fmt
 #define pr_fmt(fmt)     "hisi_pmu: " fmt
 
+#define HISI_CNTR_SCCL_MASK    (0xF00)
+
 #define HISI_SCCL_MAX	(1 << 4)
 #define HISI_SCCL_MASK	(0xF00000)
 #define HISI_SCCL_SHIFT 20
@@ -40,6 +42,7 @@
 #define HISI_EVTYPE_EVENT	0xfff
 #define HISI_MAX_PERIOD ((1LLU << 32) - 1)
 
+#define MAX_BANKS 8
 #define MAX_COUNTERS 30
 #define MAX_UNITS 8
 
-- 
2.1.4

[PATCH 6/8] arm64:perf: Makefile for Hisilicon ARMv8 PMU

[Anurup M]

	1. Add Makefile for Hisilicon ARMv8 PMU support.

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 drivers/perf/hisilicon/Makefile | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 drivers/perf/hisilicon/Makefile

diff --git a/drivers/perf/hisilicon/Makefile b/drivers/perf/hisilicon/Makefile
new file mode 100644
index 0000000..82c75a1
--- /dev/null
+++ b/drivers/perf/hisilicon/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_HISI_PERFCTR) += hisi_uncore_pmu.o hisi_uncore_llc.o
-- 
2.1.4

[PATCH 7/8] arm64:perf: Update Makefile for Hisilicon PMU support

[Anurup M]

	1. Update Makefile for Hisilicon ARMv8 PMU support.

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 drivers/perf/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index acd2397..7f051ad 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ARM_PMU) += arm_pmu.o
+obj-$(CONFIG_HISI_PERFCTR) += hisilicon/
-- 
2.1.4

[PATCH 8/8] arm64:perf: L3 cache(LLC) event listing in perf

[Anurup M]

	1. Add L3 caches events to /sys/devices/hisi_l3c/events/
	   The events can be selected as shown in perf list
	   e.g.: For LLC_READ_ALLOCATE event for Super CPU cluster 2 the
		 event format is
		 -e "hisi_l3c/l3c_read_allocate,bank=0xf,cluster=0x2/"

Signed-off-by: Anurup M <anurup.m@huawei.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_llc.c | 75 +++++++++++++++++++++++++++++++-
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 12 +++++
 drivers/perf/hisilicon/hisi_uncore_pmu.h | 16 +++++++
 3 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_llc.c b/drivers/perf/hisilicon/hisi_uncore_llc.c
index a771e1a..316167a 100644
--- a/drivers/perf/hisilicon/hisi_uncore_llc.c
+++ b/drivers/perf/hisilicon/hisi_uncore_llc.c
@@ -25,11 +25,11 @@
 #include "hisi_uncore_llc.h"
 
 /* Map cfg_en values for LLC Banks */
-const int llc_cfgen_map[] = { HISI_LLC_BANK0_CFGEN, HISI_LLC_BANK1_CFGEN,
+static const int llc_cfgen_map[] = { HISI_LLC_BANK0_CFGEN, HISI_LLC_BANK1_CFGEN,
 				HISI_LLC_BANK2_CFGEN, HISI_LLC_BANK3_CFGEN
 };
 
-struct hisi_pmu *hisi_uncore_llc;
+static struct hisi_pmu *hisi_uncore_llc;
 
 static inline int hisi_llc_counter_valid(int idx)
 {
@@ -442,6 +442,76 @@ static void hisi_free_llc_data(struct hisi_hwmod_unit *punit)
 	kfree(punit->hwmod_data);
 }
 
+PMU_FORMAT_ATTR(event, "config:0-11");
+PMU_FORMAT_ATTR(bank, "config:12-15");
+PMU_FORMAT_ATTR(module, "config:16-19");
+PMU_FORMAT_ATTR(cluster, "config:20-23");
+PMU_FORMAT_ATTR(socket, "config:24-25");
+
+#define HISI_UNCORE_EVENT_DESC(_name, _config)			\
+{								\
+	.attr   = __ATTR(_name, 0444, uncore_event_show, NULL), \
+	.config = _config,					\
+}
+
+static struct attribute *hisi_llc_format_attr[] = {
+	&format_attr_event.attr,
+	&format_attr_bank.attr,
+	&format_attr_module.attr,
+	&format_attr_cluster.attr,
+	&format_attr_socket.attr,
+	NULL,
+};
+
+static struct attribute_group hisi_llc_format_group = {
+	.name = "format",
+	.attrs = hisi_llc_format_attr,
+};
+
+EVENT_ATTR_STR(l3c_read_allocate,
+			"event=0x301,bank=?,module=0x4,cluster=?,socket=0x1");
+EVENT_ATTR_STR(l3c_write_allocate,
+			"event=0x302,bank=?,module=0x4,cluster=?,socket=0x1");
+EVENT_ATTR_STR(l3c_read_noallocate,
+			"event=0x303,bank=?,module=0x4,cluster=?,socket=0x1");
+EVENT_ATTR_STR(l3c_write_noallocate,
+			"event=0x304,bank=?,module=0x4,cluster=?,socket=0x1");
+EVENT_ATTR_STR(l3c_read_hit,
+			"event=0x305,bank=?,module=0x4,cluster=?,socket=0x1");
+EVENT_ATTR_STR(l3c_write_hit,
+			"event=0x306,bank=?,module=0x4,cluster=?,socket=0x1");
+
+
+static struct attribute *hisi_llc_events_attr[] = {
+	EVENT_PTR(l3c_read_allocate),
+	EVENT_PTR(l3c_write_allocate),
+	EVENT_PTR(l3c_read_noallocate),
+	EVENT_PTR(l3c_write_noallocate),
+	EVENT_PTR(l3c_read_hit),
+	EVENT_PTR(l3c_write_hit),
+	NULL,
+};
+
+static struct attribute_group hisi_llc_events_group = {
+	.name = "events",
+	.attrs = hisi_llc_events_attr,
+};
+
+static struct attribute *hisi_llc_attrs[] = {
+	NULL,
+};
+
+struct attribute_group hisi_llc_attr_group = {
+	.attrs = hisi_llc_attrs,
+};
+
+static const struct attribute_group *hisi_llc_pmu_attr_groups[] = {
+	&hisi_llc_attr_group,
+	&hisi_llc_format_group,
+	&hisi_llc_events_group,
+	NULL
+};
+
 void hisi_llc_pmu_init(struct platform_device *pdev,
 					struct hisi_pmu *pllc_pmu)
 {
@@ -489,6 +559,7 @@ static int hisi_pmu_llc_dev_probe(struct platform_device *pdev)
 				.start = hisi_uncore_pmu_start,
 				.stop = hisi_uncore_pmu_stop,
 				.read = hisi_uncore_pmu_read,
+				.attr_groups = hisi_llc_pmu_attr_groups,
 		};
 
 		ret = hisi_uncore_pmu_setup(pllc_pmu, pdev, "hisi_l3c");
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index e02cb81..5f404a6 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -26,6 +26,18 @@
 #include "hisi_uncore_pmu.h"
 #include "hisi_uncore_llc.h"
 
+ssize_t hisi_events_sysfs_show(struct device *dev,
+				  struct device_attribute *attr, char *page)
+{
+	struct perf_pmu_events_attr *pmu_attr =
+		container_of(attr, struct perf_pmu_events_attr, attr);
+
+	if (pmu_attr->event_str)
+		return sprintf(page, "%s", pmu_attr->event_str);
+
+	return 0;
+}
+
 /* djtag read interface - Call djtag driver to access SoC registers */
 int hisi_djtag_readreg(int module_id, int bank, u32 offset,
 				struct device_node *djtag_node, u32 *pvalue)
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index dfc8c83..4bc60e0 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -46,6 +46,20 @@
 #define MAX_COUNTERS 30
 #define MAX_UNITS 8
 
+#define EVENT_PTR(_id) (&event_attr_##_id.attr.attr)
+
+#define EVENT_ATTR(_name, _val)						   \
+static struct perf_pmu_events_attr event_attr_##_name = {		   \
+	.attr	   = __ATTR(_name, 0444, hisi_events_sysfs_show, NULL),	   \
+	.event_str = "event=" __stringify(_val),			   \
+}
+
+#define EVENT_ATTR_STR(_name, _str)					   \
+static struct perf_pmu_events_attr event_attr_##_name = {		   \
+	.attr	   = __ATTR(_name, 0444, hisi_events_sysfs_show, NULL),    \
+	.event_str = _str,						   \
+}
+
 enum hisi_hwmod_type {
 	HISI_LLC = 0x0,
 };
@@ -125,4 +139,6 @@ int hisi_pmu_unit_init(struct platform_device *,
 				struct hisi_hwmod_unit *,
 						int, int);
 struct hisi_pmu *hisi_pmu_alloc(struct platform_device *);
+ssize_t hisi_events_sysfs_show(struct device *,
+				  struct device_attribute *, char *);
 #endif /* __HISI_UNCORE_PMU_H__ */
-- 
2.1.4

[PATCH 1/8] arm64:perf: Add Devicetree bindings for Hisilicon SoC PMU

[Mark Rutland]

Hi,

On Tue, Jun 28, 2016 at 05:50:22AM -0400, Anurup M wrote:
> 	1) Device tree bindings for Hisilicon PMU.
> 	2) Add example for Hisilicon LLC PMU.
> 
> Signed-off-by: Anurup M <anurup.m@huawei.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
>  .../devicetree/bindings/arm/hisilicon/pmu.txt      | 32 ++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
> new file mode 100644
> index 0000000..7584a81
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
> @@ -0,0 +1,32 @@
> +Hisilicon SoC HIP05 ARMv8 PMU
> +
> +Required Properties:
> +	- compatible : This field contain two values. The first value is
> +		always "hisilicon" and second value is the PMU type as shown
> +		in below examples:
> +		(a) "hisilicon,hip05-llc" for Hisiliocn SoC L3 cache PMU
> +		(b) "hisilicon,hip05-ddrc" for Hisiliocn SoC DDRC PMU
> +		(c) "hisilicon,hip05-mn" for Hisiliocn SoC MN PMU

That wording is rather confusing. Use:

	- compatible: must contain one of:
	  * "hisilicon,hip05-llc" for HIP05 L3 cache PMU
	  * "hisilicon,hip05-ddrc" for HIP05 DDRC PMU
	  * "hisilicon,hip05-mn" for HIP05 MN PMU

What exactly is an "MN"?

I assume that these nodes actually describe the whole interface for
controlling the L3/DDRC/MN, so it's probably worth dropping the "PMU"
from the description, even if we only support the PMU in Linux.

No reg properties?

> +
> +Optional Properties:
> +
> +	- djtag	: Some PMU registers are accessed via the Djtag interface
> +		This field contains two values. The first value is the djtag
> +		node phandle and second value is the Super CPU Cluster ID.

What is a Djtag node? What is a "Super CPU Cluster ID"?

I think you need additional bindings for these. I cannot understand the
binding without a description of those.

> +	- interrupt-parent : A phandle indicating which interrupt controller
> +		this PMU signals interrupts to.
> +
> +	- interrupts : Interrupt lines used by this PMU. If the PMU has
> +		multiple banks, then all IRQ lines are listed in this
> +		property.

In which order?

> +
> +Example:
> +	llc0: llc at 0 {

That unit address (the '@0') shouldn't be there, given the lack of a reg
property.

> +		compatible = "hisilicon,hip05-llc";
> +		djtag = <&djtag0 2>; /* DJTAG node for Super CPU Cluster 2
> +				      * (starts from 1) */
> +		interrupt-parent = <&mbigen_pc>;
> +		interrupts = <141 4>,<142 4>,
> +			 <143 4>,<144 4>; /* IRQ lines for 4 L3 cache banks */
> +	};
> -- 
> 2.1.4

Thanks,
Mark.

[PATCH 3/8] arm64:perf: Update Kconfig for Hisilicon PMU support

[Mark Rutland]

On Tue, Jun 28, 2016 at 05:50:24AM -0400, Anurup M wrote:
> 	1. Update Kconfig for Hisilicon SoC ARMv8 PMU support.
> 
> Signed-off-by: Anurup M <anurup.m@huawei.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
>  drivers/perf/Kconfig | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index 04e2653..a90b2fc 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -12,4 +12,13 @@ config ARM_PMU
>  	  Say y if you want to use CPU performance monitors on ARM-based
>  	  systems.
>  
> +config HISI_PERFCTR
> +	bool "Enable hardware event counter support for HiSilicon SoC"
> +	depends on HW_PERF_EVENTS && ARM64
> +	select HISI_DJTAG

This Kconfig symbol doesn't exist.

You need to post the relevant code, or at the very least refer to it.

This cannot be reviewed or merged in the absence of that code.

Thanks,
Mark.

> +	default n
> +	help
> +	  Enable hardware event counter support for hardware event counters
> +	  in Hisilicon Hi161x SoC. The hardware modules like LLC, MN1 and
> +	  DDRC have hardware events and counters.
>  endmenu
> -- 
> 2.1.4
> 

[PATCH 4/8] arm64:perf: Add support for Hisilicon SoC event counters

[Mark Rutland]

On Tue, Jun 28, 2016 at 05:50:25AM -0400, Anurup M wrote:
> 	1. Hisilicon SoC PMU to support different hardware event
> 	   counters.
> 	2. Register with Perf PMU as RAW events.

NAK to use of PERF_TYPE_RAW. This should be a dynamic, non-ABI id.
Register with -1 as the type, and have the perf core allocate it.

> 	3. Hisilicon PMU shall use the DJTAG hardware interface
> 	   to access hardware event counters and configuration
> 	   register.

As mentioend for earlier patches, I cannot find the DJTAG code, so it'sn
ot possible to review this fully.

> 	4. Routines to initialze and setup PMU.

Nit: initialize.

> 	5. Routines to enable/disable/add/del/start/stop hardware
> 	   event counting.
> 
> Signed-off-by: Anurup M <anurup.m@huawei.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
>  drivers/perf/hisilicon/hisi_uncore_pmu.c | 465 +++++++++++++++++++++++++++++++
>  drivers/perf/hisilicon/hisi_uncore_pmu.h | 125 +++++++++
>  2 files changed, 590 insertions(+)
>  create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.c
>  create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.h

>From a quick look at the code, there are a large number of "TBD"
comments, and the basic functionality required for this driver to
actually function is not present as of this patch.

This is not helpful for review.

> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
> new file mode 100644
> index 0000000..40bdc23
> --- /dev/null
> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
> @@ -0,0 +1,465 @@
> +/*
> + * HiSilicon SoC Hardware event counters support
> + *
> + * Copyright (C) 2016 Huawei Technologies Limited
> + * Author: Anurup M <anurup.m@huawei.com>
> + *
> + * This code is based heavily on the ARMv7 perf event code.

The CPU PMU code is very different to what's required for uncore/system
PMUs such as this. The arm-cci and arm-ccn drivers (currently under
drivers/bus/) are much better examples.

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/bitmap.h>
> +#include <linux/of.h>
> +#include <linux/perf_event.h>

Minor nit: please sort these alphabetically (the linux/bitmap.h include
should be first).

> +#include "hisi_uncore_pmu.h"
> +
> +/* djtag read interface - Call djtag driver to access SoC registers */
> +int hisi_djtag_readreg(int module_id, int bank, u32 offset,
> +				struct device_node *djtag_node, u32 *pvalue)
> +{
> +	int ret;
> +	u32 chain_id = 0;
> +
> +	while (bank != 1) {
> +		bank = (bank >> 0x1);
> +		chain_id++;
> +	}
> +
> +	ret = djtag_readl(djtag_node, offset, module_id, chain_id, pvalue);
> +	if (ret)
> +		pr_warn("Djtag:%s Read failed!\n", djtag_node->full_name);
> +
> +	return ret;
> +}
> +
> +/* djtag write interface - Call djtag driver  to access SoC registers */
> +int hisi_djtag_writereg(int module_id, int bank,
> +				u32 offset, u32 value,
> +				struct device_node *djtag_node)
> +{
> +	int ret;
> +
> +	ret = djtag_writel(djtag_node, offset, module_id, 0, value);
> +	if (ret)
> +		pr_warn("Djtag:%s Write failed!\n", djtag_node->full_name);
> +
> +	return ret;
> +}
> +
> +void hisi_uncore_pmu_write_evtype(struct hisi_hwmod_unit *punit,
> +						int idx, u32 val)
> +{
> +	/* TBD: Select event based on Hardware counter Module */
> +}
> +
> +int hisi_pmu_get_event_idx(struct hw_perf_event *hwc,
> +						struct hisi_hwmod_unit *punit)
> +{
> +	int event_idx = -1;
> +
> +	/* TBD - Get the available hardware event counter index */
> +
> +	return event_idx;
> +}
> +
> +void hisi_pmu_clear_event_idx(struct hw_perf_event *hwc,
> +					struct hisi_hwmod_unit *punit,
> +								int idx)
> +{
> +	/* TBD - Release the hardware event counter index */
> +}

Please have some version of these implemented when this code is added.
Having empty stubs like this is not helpful for review.

> +
> +static int
> +__hw_perf_event_init(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	int mapping;
> +
> +	mapping = pmu_map_event(event);
> +	if (mapping < 0) {
> +		pr_debug("event %x:%llx not supported\n", event->attr.type,
> +							 event->attr.config);
> +		return mapping;
> +	}
> +
> +	/*
> +	 * We don't assign an index until we actually place the event onto
> +	 * hardware. Use -1 to signify that we haven't decided where to put it
> +	 * yet.
> +	 */
> +	hwc->idx		= -1;
> +	hwc->config_base	= 0;
> +	hwc->config		= 0;
> +	hwc->event_base		= 0;
> +
> +	/*
> +	 * For HiSilicon SoC store the event encoding into the config_base
> +	 * field.
> +	 */
> +	hwc->config_base = event->attr.config;
> +
> +	/*
> +	 * Limit the sample_period to half of the counter width. That way, the
> +	 * new counter value is far less likely to overtake the previous one
> +	 * unless you have some serious IRQ latency issues.
> +	 */
> +	hwc->sample_period  = HISI_MAX_PERIOD >> 1;
> +	hwc->last_period    = hwc->sample_period;
> +	local64_set(&hwc->period_left, hwc->sample_period);
> +
> +	/* TBD: Initialize event counter variables to support multiple
> +	 * HiSilicon Soc SCCL and banks
> +	 */
> +
> +	return 0;
> +}
> +
> +void hw_perf_event_destroy(struct perf_event *event)
> +{
> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit;
> +	atomic_t *active_events;
> +	struct mutex *reserve_mutex;
> +	u32 unit_idx = GET_UNIT_IDX(event->hw.config_base);
> +
> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
> +	active_events = &punit->active_events;
> +	reserve_mutex = &punit->reserve_mutex;
> +
> +	if (atomic_dec_and_mutex_lock(active_events, reserve_mutex)) {
> +		/* FIXME: Release IRQ here */
> +		mutex_unlock(reserve_mutex);
> +	}
> +}

Please either implement this, or don't bother dynamically acquiring and
freeing the IRQ.

It looks like none of the other uncore PMU drivers bothers...

> +
> +int hisi_uncore_pmu_event_init(struct perf_event *event)
> +{
> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit;
> +	u32 raw_event_code = event->attr.config;
> +	u32 unit_idx = GET_UNIT_IDX(raw_event_code);
> +	int err;
> +
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +
> +	/* we do not support sampling */
> +	if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
> +		return -EOPNOTSUPP;
> +
> +	/* counters do not have these bits */
> +	if (event->attr.exclude_user	||
> +	    event->attr.exclude_kernel	||
> +	    event->attr.exclude_host	||
> +	    event->attr.exclude_guest	||
> +	    event->attr.exclude_hv	||
> +	    event->attr.exclude_idle)
> +		return -EINVAL;

All of this crops up so much we should have common code to handle all of
this. Either a library function or an uncore flag that the core code can
check.

> +
> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
> +
> +	event->destroy = hw_perf_event_destroy;
> +
> +	err = __hw_perf_event_init(event);
> +	if (err)
> +		hw_perf_event_destroy(event);
> +
> +	return err;
> +}
> +
> +
> +/* Read hardware counter and update the Perf counter statistics */
> +u64 hisi_uncore_pmu_event_update(struct perf_event *event,
> +					struct hw_perf_event *hwc,
> +							int idx) {
> +	u64 new_raw_count = 0;
> +	/*
> +	 * TBD: Identify Event type and read appropriate hardware
> +	 * counter and sum the values
> +	 */
> +
> +	return new_raw_count;
> +}
> +
> +void hisi_uncore_pmu_enable(struct pmu *pmu)
> +{
> +	/* TBD: Enable all the PMU counters. */
> +}
> +
> +void hisi_uncore_pmu_disable(struct pmu *pmu)
> +{
> +	/* TBD: Disable all the PMU counters. */
> +}
> +
> +int hisi_pmu_enable_counter(struct hisi_hwmod_unit *punit,
> +							int idx)
> +{
> +	int ret = 0;
> +
> +	/* TBD - Enable the hardware event counting */
> +
> +	return ret;
> +}
> +
> +void hisi_pmu_disable_counter(struct hisi_hwmod_unit *punit,
> +							int idx)
> +{
> +	/* TBD - Disable the hardware event counting */
> +}

Please implement all of these.

> +void hisi_uncore_pmu_enable_event(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit;
> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
> +	int idx = hwc->idx;
> +
> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
> +
> +	/*
> +	 * Enable counter and set the counter to count
> +	 * the event that we're interested in.
> +	 */
> +
> +	/*
> +	 * Disable counter
> +	 */
> +	hisi_pmu_disable_counter(punit, idx);
> +
> +	/*
> +	 * Set event (if destined for Hisilicon SoC counters).
> +	 */
> +	hisi_uncore_pmu_write_evtype(punit, idx, hwc->config_base);
> +
> +	/*
> +	 * Enable counter
> +	 */
> +	hisi_pmu_enable_counter(punit, idx);
> +
> +}
> +
> +int hisi_pmu_write_counter(struct hisi_hwmod_unit *punit,
> +						int idx,
> +						u32 value)
> +{
> +	int ret = 0;
> +
> +	/* TBD - Write to the hardware event counter */
> +
> +	return ret;
> +}
> +
> +void hisi_pmu_event_set_period(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit;
> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
> +	int idx = hwc->idx;
> +
> +	/*
> +	 * The Hisilicon PMU counters have a period of 2^32. To account for the
> +	 * possiblity of extreme interrupt latency we program for a period of
> +	 * half that. Hopefully we can handle the interrupt before another 2^31
> +	 * events occur and the counter overtakes its previous value.
> +	 */
> +	u64 val = 1ULL << 31;
> +
> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
> +	local64_set(&hwc->prev_count, val);
> +	hisi_pmu_write_counter(punit, idx, val);
> +}
> +
> +void hisi_uncore_pmu_start(struct perf_event *event,
> +						int pmu_flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit;
> +	struct hisi_pmu_hw_events *hw_events;
> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
> +	unsigned long flags;
> +
> +	if (unit_idx >= (HISI_SCCL_MAX - 1)) {
> +		pr_err("Invalid unitID=%d in event code=%lu!\n",
> +					unit_idx + 1, hwc->config_base);
> +		return;
> +	}
> +
> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
> +	hw_events = &punit->hw_events;
> +
> +	/*
> +	 * To handle interrupt latency, we always reprogram the period
> +	 * regardlesss of PERF_EF_RELOAD.
> +	 */
> +	if (pmu_flags & PERF_EF_RELOAD)
> +		WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> +
> +	hwc->state = 0;
> +
> +	raw_spin_lock_irqsave(&hw_events->pmu_lock, flags);
> +
> +	hisi_pmu_event_set_period(event);
> +	hisi_uncore_pmu_enable_event(event);
> +
> +	raw_spin_unlock_irqrestore(&hw_events->pmu_lock, flags);
> +}
> +
> +void hisi_uncore_pmu_stop(struct perf_event *event,
> +						int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit = NULL;
> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
> +	int idx = hwc->idx;
> +
> +	if (hwc->state & PERF_HES_STOPPED)
> +		return;
> +
> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
> +
> +	/*
> +	 * We always reprogram the counter, so ignore PERF_EF_UPDATE. See
> +	 * hisi_uncore_pmu_start()
> +	 */
> +	hisi_pmu_disable_counter(punit, idx);
> +	hisi_uncore_pmu_event_update(event, hwc, idx);
> +	hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
> +
> +}
> +
> +int hisi_uncore_pmu_add(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct hisi_pmu *phisipmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit;
> +	struct hisi_pmu_hw_events *hw_events;
> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
> +	int idx, err = 0;
> +
> +	if (unit_idx >= (HISI_SCCL_MAX - 1)) {
> +		pr_err("Invalid unitID=%d in event code=%lu\n",
> +					unit_idx + 1, hwc->config_base);
> +		return -EINVAL;
> +	}
> +
> +	punit = &phisipmu->hwmod_pmu_unit[unit_idx];
> +	hw_events = &punit->hw_events;
> +
> +	/* If we don't have a free counter then return early. */
> +	idx = hisi_pmu_get_event_idx(hwc, punit);
> +	if (idx < 0) {
> +		err = idx;
> +		goto out;
> +	}
> +
> +	event->hw.idx = idx;
> +	hw_events->events[idx] = event;
> +
> +	hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
> +	if (flags & PERF_EF_START)
> +		hisi_uncore_pmu_start(event, PERF_EF_RELOAD);
> +
> +	/* Propagate our changes to the userspace mapping. */
> +	perf_event_update_userpage(event);
> +
> +out:
> +	return err;
> +}
> +
> +void hisi_uncore_pmu_del(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct hisi_pmu *phisipmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit;
> +	struct hisi_pmu_hw_events *hw_events;
> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
> +	int idx = hwc->idx;
> +
> +	punit = &phisipmu->hwmod_pmu_unit[unit_idx];
> +	hw_events = &punit->hw_events;
> +
> +	hisi_uncore_pmu_stop(event, PERF_EF_UPDATE);
> +	hw_events->events[idx] = NULL;
> +
> +	hisi_pmu_clear_event_idx(hwc, punit, idx);
> +
> +	perf_event_update_userpage(event);
> +}
> +
> +int pmu_map_event(struct perf_event *event)
> +{
> +	return (int)(event->attr.config & HISI_EVTYPE_EVENT);
> +}
> +
> +struct hisi_pmu *hisi_pmu_alloc(struct platform_device *pdev)
> +{
> +	struct hisi_pmu *phisipmu;
> +
> +	phisipmu = devm_kzalloc(&pdev->dev, sizeof(*phisipmu), GFP_KERNEL);
> +	if (!phisipmu)
> +		return ERR_PTR(-ENOMEM);
> +
> +	return phisipmu;
> +}
> +
> +int hisi_pmu_unit_init(struct platform_device *pdev,
> +				struct hisi_hwmod_unit *punit,
> +						int unit_id,
> +						int num_counters)
> +{
> +	punit->hw_events.events = devm_kcalloc(&pdev->dev,
> +				     num_counters,
> +				     sizeof(*punit->hw_events.events),
> +							     GFP_KERNEL);
> +	if (!punit->hw_events.events)
> +		return -ENOMEM;
> +
> +	raw_spin_lock_init(&punit->hw_events.pmu_lock);
> +	atomic_set(&punit->active_events, 0);
> +	mutex_init(&punit->reserve_mutex);
> +
> +	punit->unit_id = unit_id;
> +
> +	return 0;
> +}
> +
> +void hisi_uncore_pmu_read(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	int idx = hwc->idx;
> +
> +	hisi_uncore_pmu_event_update(event, hwc, idx);
> +}
> +
> +int hisi_uncore_pmu_setup(struct hisi_pmu *hisipmu,
> +				struct platform_device *pdev,
> +						char *pmu_name)
> +{
> +	int ret;
> +
> +	/* Register the events are RAW events to support RAW events too */
> +	ret = perf_pmu_register(&hisipmu->pmu, pmu_name, PERF_TYPE_RAW);
> +	if (ret)
> +		goto fail;
> +
> +	return 0;
> +
> +fail:
> +	return ret;
> +}

Get rid of the goto and return ret in all cases. If you need the goto in
subsequent patches, you can introduce it as required.

This is never called currently, so it's unclear to me precisely what is
being registered...

Thanks,
Mark.

[PATCH 5/8] arm64:perf: L3 cache(LLC) event counting in perf

[Mark Rutland]

On Tue, Jun 28, 2016 at 05:50:26AM -0400, Anurup M wrote:
> 	1. Add support to count L3 cache(LLC) hardware events.
> 	2. Add events as RAW events.
> 	  The events to count can be selected as RAW event
> 	  referring to the hardware user manual.
> 	  e.g.: To input as RAW events the RAW event code is
> 		-e r124f301
> 	  The RAW event encoding followed is
> 	  <socket ID(2 bit)><SCCL ID(4 bit)><ModuleID(4 bit)>
> 	  <Bank(4 bit)><event code(12 bit)>
> 	  So 0x1 is SocketID, 0x2 is SCCL ID, 0x4 is the LLC
> 	  hardware module ID, 0xf is for sum of all LLC banks,
> 	  0x301 is the event code for LLC_READ_ALLOCATE.

If you have a special way of encoding events, please add documentation
for this, as we do for CCN, and as is being done for the X-Gene SoC PMU
driver [1].

The commit message is not an appropriate location to document a
user-facing ABI which the user needs to know about.

For example, I have no idea what a "SCCL ID" is, how the LLC HW module
ID corresponds topologically, what a djtag unit is, nor how a djtag unit
relates to other units in the system.

Given that, and the lack of information regarding the djtag unit(s),
I've skimmed over the rest of this patch, ignoring the portions I canot
review due to a lack of appropriate information.

> +u64 hisi_llc_event_update(struct perf_event *event,
> +				struct hw_perf_event *hwc, int idx)
> +{
> +	struct device_node *djtag_node;
> +	struct hisi_pmu *pllc_pmu = to_hisi_pmu(event->pmu);
> +	struct hisi_hwmod_unit *punit;
> +	struct hisi_llc_data *llc_hwmod_data;
> +	u64 delta, prev_raw_count, new_raw_count = 0;
> +	int cfg_en;
> +	u32 raw_event_code = hwc->config_base;
> +	u32 scclID = (raw_event_code & HISI_SCCL_MASK) >> 20;
> +	u32 llc_idx = scclID - 1;
> +	int i;
> +
> +	if (!scclID || (scclID >= HISI_SCCL_MASK)) {
> +		pr_err("Invalid SCCL=%d in event code!\n", scclID);
> +		return 0;
> +	}
> +
> +	if (!hisi_llc_counter_valid(idx)) {
> +		pr_err("Unsupported event index:%d!\n", idx);
> +		return 0;
> +	}
> +
> +	punit = &pllc_pmu->hwmod_pmu_unit[llc_idx];
> +	llc_hwmod_data = punit->hwmod_data;
> +
> +	/* Check if the LLC data is initialized for this SCCL */
> +	if (!llc_hwmod_data->djtag_node) {
> +		pr_err("SCCL=%d not initialized!\n", scclID);
> +		return 0;
> +	}
> +
> +	/* Find the djtag device node of the SCCL */
> +	djtag_node = llc_hwmod_data->djtag_node;

As above, I cannot follow this due to a lack of understanding of the HW.
You will need to add documentation such that this can be understood.

[...]

> +	/* Increment the active_events for the first event counting */
> +	if (!atomic_inc_not_zero(active_events)) {
> +		mutex_lock(&punit->reserve_mutex);
> +		if (atomic_read(active_events) == 0)
> +			atomic_inc(active_events);
> +		mutex_unlock(&punit->reserve_mutex);
> +	}

If you have no special work to do to activate the PMU HW, this reference
counting is pointless. Please get rid of it entirely.

[...]

> +int hisi_llc_get_event_idx(struct hisi_hwmod_unit *punit)
> +{
> +	struct hisi_llc_data *llc_hwmod_data = punit->hwmod_data;
> +	int event_idx;
> +
> +	event_idx =
> +		find_first_zero_bit(
> +			llc_hwmod_data->hisi_llc_event_used_mask,
> +					HISI_MAX_CFG_LLC_CNTR);
> +
> +	if (event_idx == HISI_MAX_CFG_LLC_CNTR)
> +		return -EAGAIN;
> +
> +	__set_bit(event_idx,
> +		llc_hwmod_data->hisi_llc_event_used_mask);

For reference, what mutual exclusion mechanism protects the used_mask?

[...]

> +void hisi_llc_pmu_init(struct platform_device *pdev,
> +					struct hisi_pmu *pllc_pmu)
> +{
> +	pllc_pmu->pmu_type = SCCL_SPECIFIC;
> +	pllc_pmu->name = "HISI_L3C";

Use lowercase. I suspect "hip05_l3c" is likely a better name, as
Hisilcon may produce a new, different L3C controller, and you already
use "hisilicon,hip05-llc" as the compatible string.

[...]

> +		pllc_pmu->pmu = (struct pmu) {
> +				.name		= "HISI-L3C-PMU",
> +		/*		.task_ctx_nr	= perf_invalid_context, */

Why is this commented out? You *must* use perf_invalid_context.

> +				.pmu_enable = hisi_uncore_pmu_enable,
> +				.pmu_disable = hisi_uncore_pmu_disable,
> +				.event_init = hisi_uncore_pmu_event_init,
> +				.add = hisi_uncore_pmu_add,
> +				.del = hisi_uncore_pmu_del,
> +				.start = hisi_uncore_pmu_start,
> +				.stop = hisi_uncore_pmu_stop,
> +				.read = hisi_uncore_pmu_read,
> +		};

[...]

> +MODULE_DESCRIPTION("HiSilicon ARMv8 LLC PMU driver");

s/ARMv8/HIP05/, the SoC matters far more than the architecture it
happens to use.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438607.html

[PATCH 8/8] arm64:perf: L3 cache(LLC) event listing in perf

[Mark Rutland]

On Tue, Jun 28, 2016 at 05:50:29AM -0400, Anurup M wrote:
> 	1. Add L3 caches events to /sys/devices/hisi_l3c/events/
> 	   The events can be selected as shown in perf list
> 	   e.g.: For LLC_READ_ALLOCATE event for Super CPU cluster 2 the
> 		 event format is
> 		 -e "hisi_l3c/l3c_read_allocate,bank=0xf,cluster=0x2/"
> 
> Signed-off-by: Anurup M <anurup.m@huawei.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
>  drivers/perf/hisilicon/hisi_uncore_llc.c | 75 +++++++++++++++++++++++++++++++-
>  drivers/perf/hisilicon/hisi_uncore_pmu.c | 12 +++++
>  drivers/perf/hisilicon/hisi_uncore_pmu.h | 16 +++++++
>  3 files changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/hisilicon/hisi_uncore_llc.c b/drivers/perf/hisilicon/hisi_uncore_llc.c
> index a771e1a..316167a 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_llc.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_llc.c
> @@ -25,11 +25,11 @@
>  #include "hisi_uncore_llc.h"
>  
>  /* Map cfg_en values for LLC Banks */
> -const int llc_cfgen_map[] = { HISI_LLC_BANK0_CFGEN, HISI_LLC_BANK1_CFGEN,
> +static const int llc_cfgen_map[] = { HISI_LLC_BANK0_CFGEN, HISI_LLC_BANK1_CFGEN,
>  				HISI_LLC_BANK2_CFGEN, HISI_LLC_BANK3_CFGEN
>  };
>  
> -struct hisi_pmu *hisi_uncore_llc;
> +static struct hisi_pmu *hisi_uncore_llc;
>  
>  static inline int hisi_llc_counter_valid(int idx)
>  {
> @@ -442,6 +442,76 @@ static void hisi_free_llc_data(struct hisi_hwmod_unit *punit)
>  	kfree(punit->hwmod_data);
>  }
>  
> +PMU_FORMAT_ATTR(event, "config:0-11");
> +PMU_FORMAT_ATTR(bank, "config:12-15");
> +PMU_FORMAT_ATTR(module, "config:16-19");
> +PMU_FORMAT_ATTR(cluster, "config:20-23");
> +PMU_FORMAT_ATTR(socket, "config:24-25");
> +
> +#define HISI_UNCORE_EVENT_DESC(_name, _config)			\
> +{								\
> +	.attr   = __ATTR(_name, 0444, uncore_event_show, NULL), \
> +	.config = _config,					\
> +}
> +
> +static struct attribute *hisi_llc_format_attr[] = {
> +	&format_attr_event.attr,
> +	&format_attr_bank.attr,
> +	&format_attr_module.attr,
> +	&format_attr_cluster.attr,
> +	&format_attr_socket.attr,
> +	NULL,
> +};

Pleas use the compound literal trick I describe in [1].

> +
> +static struct attribute_group hisi_llc_format_group = {
> +	.name = "format",
> +	.attrs = hisi_llc_format_attr,
> +};
> +
> +EVENT_ATTR_STR(l3c_read_allocate,
> +			"event=0x301,bank=?,module=0x4,cluster=?,socket=0x1");
> +EVENT_ATTR_STR(l3c_write_allocate,
> +			"event=0x302,bank=?,module=0x4,cluster=?,socket=0x1");
> +EVENT_ATTR_STR(l3c_read_noallocate,
> +			"event=0x303,bank=?,module=0x4,cluster=?,socket=0x1");
> +EVENT_ATTR_STR(l3c_write_noallocate,
> +			"event=0x304,bank=?,module=0x4,cluster=?,socket=0x1");
> +EVENT_ATTR_STR(l3c_read_hit,
> +			"event=0x305,bank=?,module=0x4,cluster=?,socket=0x1");
> +EVENT_ATTR_STR(l3c_write_hit,
> +			"event=0x306,bank=?,module=0x4,cluster=?,socket=0x1");
> +
> +
> +static struct attribute *hisi_llc_events_attr[] = {
> +	EVENT_PTR(l3c_read_allocate),
> +	EVENT_PTR(l3c_write_allocate),
> +	EVENT_PTR(l3c_read_noallocate),
> +	EVENT_PTR(l3c_write_noallocate),
> +	EVENT_PTR(l3c_read_hit),
> +	EVENT_PTR(l3c_write_hit),
> +	NULL,
> +};

Again, please use the compound literal trick.

Regardless of this sysfs data, the format needs documenting, as I
commented in the last patch.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438742.html

[PATCH 0/8] arm64:perf: Support for Hisilicon SoC Hardware event counters

[Mark Rutland]

On Tue, Jun 28, 2016 at 05:50:21AM -0400, Anurup M wrote:
> Provide Support for Hisilicon SoC Hardware event counters.
> The Hisilicon SoC Hi161x series has many uncore or non-CPU performance
> events and counters units.
> 
> This patch is implemented refering to arm-cci, Intel/AMD uncore and
> also the cavium thunderX pmu patches.
> 
> Support for Hisilicon L3 cache(LLC) hardware events and counters are added
> in this implementation.
> 
> The Hisilicon PMU datastructures are designed so as to support uncore
> and also CPU specific events in future.

The uncore and CPU event context are *very* different. These should
really be handled by separate drivers.

> 
> The Hisilicon LLC has four banks for a Super CPU Cluster(consists of
>  16 CPU cores) and each LLC bank has separate hardware events and counters.
> In the current implementation, the count from all these banks are summarized
> and total count is output.
> 
> The Hisilicon uncore PMUs can be found under /sys/bus/event_source/devices.
> The counters are exported via sysfs in the corresponding events files
> under the PMU directory so the perf tool can list the event names.
> 
> Note:
> This is very initial patchset for Hisilicon SoC PMU support shared for
> review. Please review and share comments.
> This patchset depends on the Hisilicon Djtag driver for r/w access to
> Hisilicon SoC PMU registers, which is not upstreamed yet. The patches for
> Djtag driver shall be send for review soon.

The Djtag portions are a prerequisite for this patch series, both for
review and merging.

> 
> TODO:
>         1. Counter overflow interrupt handling support to be added.
>         2. CPU notifier to migrate to available online CPU's in case
>            of CPU DOWN. Also mapping CPU cores to the current SCCL.
>         3. Support for counting of individual LLC banks

I note that bidnings were added for LLC, DDRC, and MN nodes. What's the
plan for the DDRC and MN nodes? Are they intended to be handled by the
same driver?

Thanks,
Mark.

> 
> Anurup M (8):
>   arm64:perf: Add Devicetree bindings for Hisilicon SoC PMU
>   arm64:MAINTAINERS:hisi: Add hisilicon SoC PMU support
>   arm64:perf: Update Kconfig for Hisilicon PMU support
>   arm64:perf: Add support for Hisilicon SoC event counters
>   arm64:perf: L3 cache(LLC) event counting in perf
>   arm64:perf: Makefile for Hisilicon ARMv8 PMU
>   arm64:perf: Update Makefile for Hisilicon PMU support
>   arm64:perf: L3 cache(LLC) event listing in perf
> 
>  .../devicetree/bindings/arm/hisilicon/pmu.txt      |  32 ++
>  MAINTAINERS                                        |   8 +
>  drivers/perf/Kconfig                               |   9 +
>  drivers/perf/Makefile                              |   1 +
>  drivers/perf/hisilicon/Makefile                    |   1 +
>  drivers/perf/hisilicon/hisi_uncore_llc.c           | 613 +++++++++++++++++++++
>  drivers/perf/hisilicon/hisi_uncore_llc.h           | 100 ++++
>  drivers/perf/hisilicon/hisi_uncore_pmu.c           | 521 +++++++++++++++++
>  drivers/perf/hisilicon/hisi_uncore_pmu.h           | 144 +++++
>  9 files changed, 1429 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
>  create mode 100644 drivers/perf/hisilicon/Makefile
>  create mode 100644 drivers/perf/hisilicon/hisi_uncore_llc.c
>  create mode 100644 drivers/perf/hisilicon/hisi_uncore_llc.h
>  create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.c
>  create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.h
> 
> -- 
> 2.1.4
> 

[PATCH 3/8] arm64:perf: Update Kconfig for Hisilicon PMU support

[Anurup M]

On Tuesday 28 June 2016 03:54 PM, Mark Rutland wrote:
> On Tue, Jun 28, 2016 at 05:50:24AM -0400, Anurup M wrote:
>> 	1. Update Kconfig for Hisilicon SoC ARMv8 PMU support.
>>
>> Signed-off-by: Anurup M <anurup.m@huawei.com>
>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>> ---
>>   drivers/perf/Kconfig | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
>> index 04e2653..a90b2fc 100644
>> --- a/drivers/perf/Kconfig
>> +++ b/drivers/perf/Kconfig
>> @@ -12,4 +12,13 @@ config ARM_PMU
>>   	  Say y if you want to use CPU performance monitors on ARM-based
>>   	  systems.
>>   
>> +config HISI_PERFCTR
>> +	bool "Enable hardware event counter support for HiSilicon SoC"
>> +	depends on HW_PERF_EVENTS && ARM64
>> +	select HISI_DJTAG
> This Kconfig symbol doesn't exist.
>
> You need to post the relevant code, or at the very least refer to it.
>
> This cannot be reviewed or merged in the absence of that code.
Sorry for this. We shall post the djtag driver patch for review next week.

Thanks,
Anurup
>
> Thanks,
> Mark.
>
>> +	default n
>> +	help
>> +	  Enable hardware event counter support for hardware event counters
>> +	  in Hisilicon Hi161x SoC. The hardware modules like LLC, MN1 and
>> +	  DDRC have hardware events and counters.
>>   endmenu
>> -- 
>> 2.1.4
>>

[PATCH 1/8] arm64:perf: Add Devicetree bindings for Hisilicon SoC PMU

[Anurup M]

On Tuesday 28 June 2016 03:53 PM, Mark Rutland wrote:
> Hi,
>
> On Tue, Jun 28, 2016 at 05:50:22AM -0400, Anurup M wrote:
>> 	1) Device tree bindings for Hisilicon PMU.
>> 	2) Add example for Hisilicon LLC PMU.
>>
>> Signed-off-by: Anurup M <anurup.m@huawei.com>
>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>> ---
>>   .../devicetree/bindings/arm/hisilicon/pmu.txt      | 32 ++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
>> new file mode 100644
>> index 0000000..7584a81
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
>> @@ -0,0 +1,32 @@
>> +Hisilicon SoC HIP05 ARMv8 PMU
>> +
>> +Required Properties:
>> +	- compatible : This field contain two values. The first value is
>> +		always "hisilicon" and second value is the PMU type as shown
>> +		in below examples:
>> +		(a) "hisilicon,hip05-llc" for Hisiliocn SoC L3 cache PMU
>> +		(b) "hisilicon,hip05-ddrc" for Hisiliocn SoC DDRC PMU
>> +		(c) "hisilicon,hip05-mn" for Hisiliocn SoC MN PMU
> That wording is rather confusing. Use:
>
> 	- compatible: must contain one of:
> 	  * "hisilicon,hip05-llc" for HIP05 L3 cache PMU
> 	  * "hisilicon,hip05-ddrc" for HIP05 DDRC PMU
> 	  * "hisilicon,hip05-mn" for HIP05 MN PMU
>
> What exactly is an "MN"?
I shall add the description in the bindings.
>
> I assume that these nodes actually describe the whole interface for
> controlling the L3/DDRC/MN, so it's probably worth dropping the "PMU"
> from the description, even if we only support the PMU in Linux.
Agreed. I shall remove "PMU".
>
> No reg properties?
These nodes are accessed via djtag interface which again refers to the 
system-controller node phandle.
I shall describe them in the bindings along with the djtag driver patch 
series.
>
>> +
>> +Optional Properties:
>> +
>> +	- djtag	: Some PMU registers are accessed via the Djtag interface
>> +		This field contains two values. The first value is the djtag
>> +		node phandle and second value is the Super CPU Cluster ID.
> What is a Djtag node? What is a "Super CPU Cluster ID"?
>
> I think you need additional bindings for these. I cannot understand the
> binding without a description of those.
I shall add them in the bindings along with the djtag driver patch series.
>
>> +	- interrupt-parent : A phandle indicating which interrupt controller
>> +		this PMU signals interrupts to.
>> +
>> +	- interrupts : Interrupt lines used by this PMU. If the PMU has
>> +		multiple banks, then all IRQ lines are listed in this
>> +		property.
> In which order?
If multiple (N) banks, then the order of IRQ lines to be from bank 0 to 
bank N.
I shall mention them more clearly in the description.
>
>> +
>> +Example:
>> +	llc0: llc at 0 {
> That unit address (the '@0') shouldn't be there, given the lack of a reg
> property.
Ok.

>
>> +		compatible = "hisilicon,hip05-llc";
>> +		djtag = <&djtag0 2>; /* DJTAG node for Super CPU Cluster 2
>> +				      * (starts from 1) */
>> +		interrupt-parent = <&mbigen_pc>;
>> +		interrupts = <141 4>,<142 4>,
>> +			 <143 4>,<144 4>; /* IRQ lines for 4 L3 cache banks */
>> +	};
>> -- 
>> 2.1.4
> Thanks,
> Mark.
Thanks,
Anurup

[PATCH 4/8] arm64:perf: Add support for Hisilicon SoC event counters

[Anurup M]

Hi Mark,

  sorry for delayed response.

On Tuesday 28 June 2016 04:12 PM, Mark Rutland wrote:
> On Tue, Jun 28, 2016 at 05:50:25AM -0400, Anurup M wrote:
>> 	1. Hisilicon SoC PMU to support different hardware event
>> 	   counters.
>> 	2. Register with Perf PMU as RAW events.
> NAK to use of PERF_TYPE_RAW. This should be a dynamic, non-ABI id.
> Register with -1 as the type, and have the perf core allocate it.
I shall change it.
>> 	3. Hisilicon PMU shall use the DJTAG hardware interface
>> 	   to access hardware event counters and configuration
>> 	   register.
> As mentioend for earlier patches, I cannot find the DJTAG code, so it'sn
> ot possible to review this fully.
I shall send the djtag and HIP05 L3 cache PMU patches together.
>
>> 	4. Routines to initialze and setup PMU.
> Nit: initialize.
OK.
>
>> 	5. Routines to enable/disable/add/del/start/stop hardware
>> 	   event counting.
>>
>> Signed-off-by: Anurup M <anurup.m@huawei.com>
>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>> ---
>>   drivers/perf/hisilicon/hisi_uncore_pmu.c | 465 +++++++++++++++++++++++++++++++
>>   drivers/perf/hisilicon/hisi_uncore_pmu.h | 125 +++++++++
>>   2 files changed, 590 insertions(+)
>>   create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.c
>>   create mode 100644 drivers/perf/hisilicon/hisi_uncore_pmu.h
>  From a quick look at the code, there are a large number of "TBD"
> comments, and the basic functionality required for this driver to
> actually function is not present as of this patch.
>
> This is not helpful for review.
OK.
>
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> new file mode 100644
>> index 0000000..40bdc23
>> --- /dev/null
>> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> @@ -0,0 +1,465 @@
>> +/*
>> + * HiSilicon SoC Hardware event counters support
>> + *
>> + * Copyright (C) 2016 Huawei Technologies Limited
>> + * Author: Anurup M <anurup.m@huawei.com>
>> + *
>> + * This code is based heavily on the ARMv7 perf event code.
> The CPU PMU code is very different to what's required for uncore/system
> PMUs such as this. The arm-cci and arm-ccn drivers (currently under
> drivers/bus/) are much better examples.
OK. I shall change it.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/of.h>
>> +#include <linux/perf_event.h>
> Minor nit: please sort these alphabetically (the linux/bitmap.h include
> should be first).
Ok.
>> +#include "hisi_uncore_pmu.h"
>> +
>> +/* djtag read interface - Call djtag driver to access SoC registers */
>> +int hisi_djtag_readreg(int module_id, int bank, u32 offset,
>> +				struct device_node *djtag_node, u32 *pvalue)
>> +{
>> +	int ret;
>> +	u32 chain_id = 0;
>> +
>> +	while (bank != 1) {
>> +		bank = (bank >> 0x1);
>> +		chain_id++;
>> +	}
>> +
>> +	ret = djtag_readl(djtag_node, offset, module_id, chain_id, pvalue);
>> +	if (ret)
>> +		pr_warn("Djtag:%s Read failed!\n", djtag_node->full_name);
>> +
>> +	return ret;
>> +}
>> +
>> +/* djtag write interface - Call djtag driver  to access SoC registers */
>> +int hisi_djtag_writereg(int module_id, int bank,
>> +				u32 offset, u32 value,
>> +				struct device_node *djtag_node)
>> +{
>> +	int ret;
>> +
>> +	ret = djtag_writel(djtag_node, offset, module_id, 0, value);
>> +	if (ret)
>> +		pr_warn("Djtag:%s Write failed!\n", djtag_node->full_name);
>> +
>> +	return ret;
>> +}
>> +
>> +void hisi_uncore_pmu_write_evtype(struct hisi_hwmod_unit *punit,
>> +						int idx, u32 val)
>> +{
>> +	/* TBD: Select event based on Hardware counter Module */
>> +}
>> +
>> +int hisi_pmu_get_event_idx(struct hw_perf_event *hwc,
>> +						struct hisi_hwmod_unit *punit)
>> +{
>> +	int event_idx = -1;
>> +
>> +	/* TBD - Get the available hardware event counter index */
>> +
>> +	return event_idx;
>> +}
>> +
>> +void hisi_pmu_clear_event_idx(struct hw_perf_event *hwc,
>> +					struct hisi_hwmod_unit *punit,
>> +								int idx)
>> +{
>> +	/* TBD - Release the hardware event counter index */
>> +}
> Please have some version of these implemented when this code is added.
> Having empty stubs like this is not helpful for review.
OK. shall modify it.
>
>> +
>> +static int
>> +__hw_perf_event_init(struct perf_event *event)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	int mapping;
>> +
>> +	mapping = pmu_map_event(event);
>> +	if (mapping < 0) {
>> +		pr_debug("event %x:%llx not supported\n", event->attr.type,
>> +							 event->attr.config);
>> +		return mapping;
>> +	}
>> +
>> +	/*
>> +	 * We don't assign an index until we actually place the event onto
>> +	 * hardware. Use -1 to signify that we haven't decided where to put it
>> +	 * yet.
>> +	 */
>> +	hwc->idx		= -1;
>> +	hwc->config_base	= 0;
>> +	hwc->config		= 0;
>> +	hwc->event_base		= 0;
>> +
>> +	/*
>> +	 * For HiSilicon SoC store the event encoding into the config_base
>> +	 * field.
>> +	 */
>> +	hwc->config_base = event->attr.config;
>> +
>> +	/*
>> +	 * Limit the sample_period to half of the counter width. That way, the
>> +	 * new counter value is far less likely to overtake the previous one
>> +	 * unless you have some serious IRQ latency issues.
>> +	 */
>> +	hwc->sample_period  = HISI_MAX_PERIOD >> 1;
>> +	hwc->last_period    = hwc->sample_period;
>> +	local64_set(&hwc->period_left, hwc->sample_period);
>> +
>> +	/* TBD: Initialize event counter variables to support multiple
>> +	 * HiSilicon Soc SCCL and banks
>> +	 */
>> +
>> +	return 0;
>> +}
>> +
>> +void hw_perf_event_destroy(struct perf_event *event)
>> +{
>> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit;
>> +	atomic_t *active_events;
>> +	struct mutex *reserve_mutex;
>> +	u32 unit_idx = GET_UNIT_IDX(event->hw.config_base);
>> +
>> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
>> +	active_events = &punit->active_events;
>> +	reserve_mutex = &punit->reserve_mutex;
>> +
>> +	if (atomic_dec_and_mutex_lock(active_events, reserve_mutex)) {
>> +		/* FIXME: Release IRQ here */
>> +		mutex_unlock(reserve_mutex);
>> +	}
>> +}
> Please either implement this, or don't bother dynamically acquiring and
> freeing the IRQ.
>
> It looks like none of the other uncore PMU drivers bothers...
Ok. shall add them with the counter overflow IRQ handling support.
>
>> +
>> +int hisi_uncore_pmu_event_init(struct perf_event *event)
>> +{
>> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit;
>> +	u32 raw_event_code = event->attr.config;
>> +	u32 unit_idx = GET_UNIT_IDX(raw_event_code);
>> +	int err;
>> +
>> +	if (event->attr.type != event->pmu->type)
>> +		return -ENOENT;
>> +
>> +	/* we do not support sampling */
>> +	if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
>> +		return -EOPNOTSUPP;
>> +
>> +	/* counters do not have these bits */
>> +	if (event->attr.exclude_user	||
>> +	    event->attr.exclude_kernel	||
>> +	    event->attr.exclude_host	||
>> +	    event->attr.exclude_guest	||
>> +	    event->attr.exclude_hv	||
>> +	    event->attr.exclude_idle)
>> +		return -EINVAL;
> All of this crops up so much we should have common code to handle all of
> this. Either a library function or an uncore flag that the core code can
> check.
>
>> +
>> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
>> +
>> +	event->destroy = hw_perf_event_destroy;
>> +
>> +	err = __hw_perf_event_init(event);
>> +	if (err)
>> +		hw_perf_event_destroy(event);
>> +
>> +	return err;
>> +}
>> +
>> +
>> +/* Read hardware counter and update the Perf counter statistics */
>> +u64 hisi_uncore_pmu_event_update(struct perf_event *event,
>> +					struct hw_perf_event *hwc,
>> +							int idx) {
>> +	u64 new_raw_count = 0;
>> +	/*
>> +	 * TBD: Identify Event type and read appropriate hardware
>> +	 * counter and sum the values
>> +	 */
>> +
>> +	return new_raw_count;
>> +}
>> +
>> +void hisi_uncore_pmu_enable(struct pmu *pmu)
>> +{
>> +	/* TBD: Enable all the PMU counters. */
>> +}
>> +
>> +void hisi_uncore_pmu_disable(struct pmu *pmu)
>> +{
>> +	/* TBD: Disable all the PMU counters. */
>> +}
>> +
>> +int hisi_pmu_enable_counter(struct hisi_hwmod_unit *punit,
>> +							int idx)
>> +{
>> +	int ret = 0;
>> +
>> +	/* TBD - Enable the hardware event counting */
>> +
>> +	return ret;
>> +}
>> +
>> +void hisi_pmu_disable_counter(struct hisi_hwmod_unit *punit,
>> +							int idx)
>> +{
>> +	/* TBD - Disable the hardware event counting */
>> +}
> Please implement all of these.
Ok.
>
>> +void hisi_uncore_pmu_enable_event(struct perf_event *event)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit;
>> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
>> +	int idx = hwc->idx;
>> +
>> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
>> +
>> +	/*
>> +	 * Enable counter and set the counter to count
>> +	 * the event that we're interested in.
>> +	 */
>> +
>> +	/*
>> +	 * Disable counter
>> +	 */
>> +	hisi_pmu_disable_counter(punit, idx);
>> +
>> +	/*
>> +	 * Set event (if destined for Hisilicon SoC counters).
>> +	 */
>> +	hisi_uncore_pmu_write_evtype(punit, idx, hwc->config_base);
>> +
>> +	/*
>> +	 * Enable counter
>> +	 */
>> +	hisi_pmu_enable_counter(punit, idx);
>> +
>> +}
>> +
>> +int hisi_pmu_write_counter(struct hisi_hwmod_unit *punit,
>> +						int idx,
>> +						u32 value)
>> +{
>> +	int ret = 0;
>> +
>> +	/* TBD - Write to the hardware event counter */
>> +
>> +	return ret;
>> +}
>> +
>> +void hisi_pmu_event_set_period(struct perf_event *event)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit;
>> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
>> +	int idx = hwc->idx;
>> +
>> +	/*
>> +	 * The Hisilicon PMU counters have a period of 2^32. To account for the
>> +	 * possiblity of extreme interrupt latency we program for a period of
>> +	 * half that. Hopefully we can handle the interrupt before another 2^31
>> +	 * events occur and the counter overtakes its previous value.
>> +	 */
>> +	u64 val = 1ULL << 31;
>> +
>> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
>> +	local64_set(&hwc->prev_count, val);
>> +	hisi_pmu_write_counter(punit, idx, val);
>> +}
>> +
>> +void hisi_uncore_pmu_start(struct perf_event *event,
>> +						int pmu_flags)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit;
>> +	struct hisi_pmu_hw_events *hw_events;
>> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
>> +	unsigned long flags;
>> +
>> +	if (unit_idx >= (HISI_SCCL_MAX - 1)) {
>> +		pr_err("Invalid unitID=%d in event code=%lu!\n",
>> +					unit_idx + 1, hwc->config_base);
>> +		return;
>> +	}
>> +
>> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
>> +	hw_events = &punit->hw_events;
>> +
>> +	/*
>> +	 * To handle interrupt latency, we always reprogram the period
>> +	 * regardlesss of PERF_EF_RELOAD.
>> +	 */
>> +	if (pmu_flags & PERF_EF_RELOAD)
>> +		WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
>> +
>> +	hwc->state = 0;
>> +
>> +	raw_spin_lock_irqsave(&hw_events->pmu_lock, flags);
>> +
>> +	hisi_pmu_event_set_period(event);
>> +	hisi_uncore_pmu_enable_event(event);
>> +
>> +	raw_spin_unlock_irqrestore(&hw_events->pmu_lock, flags);
>> +}
>> +
>> +void hisi_uncore_pmu_stop(struct perf_event *event,
>> +						int flags)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit = NULL;
>> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
>> +	int idx = hwc->idx;
>> +
>> +	if (hwc->state & PERF_HES_STOPPED)
>> +		return;
>> +
>> +	punit = &phisi_pmu->hwmod_pmu_unit[unit_idx];
>> +
>> +	/*
>> +	 * We always reprogram the counter, so ignore PERF_EF_UPDATE. See
>> +	 * hisi_uncore_pmu_start()
>> +	 */
>> +	hisi_pmu_disable_counter(punit, idx);
>> +	hisi_uncore_pmu_event_update(event, hwc, idx);
>> +	hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
>> +
>> +}
>> +
>> +int hisi_uncore_pmu_add(struct perf_event *event, int flags)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	struct hisi_pmu *phisipmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit;
>> +	struct hisi_pmu_hw_events *hw_events;
>> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
>> +	int idx, err = 0;
>> +
>> +	if (unit_idx >= (HISI_SCCL_MAX - 1)) {
>> +		pr_err("Invalid unitID=%d in event code=%lu\n",
>> +					unit_idx + 1, hwc->config_base);
>> +		return -EINVAL;
>> +	}
>> +
>> +	punit = &phisipmu->hwmod_pmu_unit[unit_idx];
>> +	hw_events = &punit->hw_events;
>> +
>> +	/* If we don't have a free counter then return early. */
>> +	idx = hisi_pmu_get_event_idx(hwc, punit);
>> +	if (idx < 0) {
>> +		err = idx;
>> +		goto out;
>> +	}
>> +
>> +	event->hw.idx = idx;
>> +	hw_events->events[idx] = event;
>> +
>> +	hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
>> +	if (flags & PERF_EF_START)
>> +		hisi_uncore_pmu_start(event, PERF_EF_RELOAD);
>> +
>> +	/* Propagate our changes to the userspace mapping. */
>> +	perf_event_update_userpage(event);
>> +
>> +out:
>> +	return err;
>> +}
>> +
>> +void hisi_uncore_pmu_del(struct perf_event *event, int flags)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	struct hisi_pmu *phisipmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit;
>> +	struct hisi_pmu_hw_events *hw_events;
>> +	u32 unit_idx = GET_UNIT_IDX(hwc->config_base);
>> +	int idx = hwc->idx;
>> +
>> +	punit = &phisipmu->hwmod_pmu_unit[unit_idx];
>> +	hw_events = &punit->hw_events;
>> +
>> +	hisi_uncore_pmu_stop(event, PERF_EF_UPDATE);
>> +	hw_events->events[idx] = NULL;
>> +
>> +	hisi_pmu_clear_event_idx(hwc, punit, idx);
>> +
>> +	perf_event_update_userpage(event);
>> +}
>> +
>> +int pmu_map_event(struct perf_event *event)
>> +{
>> +	return (int)(event->attr.config & HISI_EVTYPE_EVENT);
>> +}
>> +
>> +struct hisi_pmu *hisi_pmu_alloc(struct platform_device *pdev)
>> +{
>> +	struct hisi_pmu *phisipmu;
>> +
>> +	phisipmu = devm_kzalloc(&pdev->dev, sizeof(*phisipmu), GFP_KERNEL);
>> +	if (!phisipmu)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	return phisipmu;
>> +}
>> +
>> +int hisi_pmu_unit_init(struct platform_device *pdev,
>> +				struct hisi_hwmod_unit *punit,
>> +						int unit_id,
>> +						int num_counters)
>> +{
>> +	punit->hw_events.events = devm_kcalloc(&pdev->dev,
>> +				     num_counters,
>> +				     sizeof(*punit->hw_events.events),
>> +							     GFP_KERNEL);
>> +	if (!punit->hw_events.events)
>> +		return -ENOMEM;
>> +
>> +	raw_spin_lock_init(&punit->hw_events.pmu_lock);
>> +	atomic_set(&punit->active_events, 0);
>> +	mutex_init(&punit->reserve_mutex);
>> +
>> +	punit->unit_id = unit_id;
>> +
>> +	return 0;
>> +}
>> +
>> +void hisi_uncore_pmu_read(struct perf_event *event)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	int idx = hwc->idx;
>> +
>> +	hisi_uncore_pmu_event_update(event, hwc, idx);
>> +}
>> +
>> +int hisi_uncore_pmu_setup(struct hisi_pmu *hisipmu,
>> +				struct platform_device *pdev,
>> +						char *pmu_name)
>> +{
>> +	int ret;
>> +
>> +	/* Register the events are RAW events to support RAW events too */
>> +	ret = perf_pmu_register(&hisipmu->pmu, pmu_name, PERF_TYPE_RAW);
>> +	if (ret)
>> +		goto fail;
>> +
>> +	return 0;
>> +
>> +fail:
>> +	return ret;
>> +}
> Get rid of the goto and return ret in all cases. If you need the goto in
> subsequent patches, you can introduce it as required.
>
> This is never called currently, so it's unclear to me precisely what is
> being registered...
OK. I shall modify it accordingly. Thanks.

Thanks,
Anurup
> Thanks,
> Mark.

[PATCH 5/8] arm64:perf: L3 cache(LLC) event counting in perf

[Anurup M]

On Tuesday 28 June 2016 04:28 PM, Mark Rutland wrote:
> On Tue, Jun 28, 2016 at 05:50:26AM -0400, Anurup M wrote:
>> 	1. Add support to count L3 cache(LLC) hardware events.
>> 	2. Add events as RAW events.
>> 	  The events to count can be selected as RAW event
>> 	  referring to the hardware user manual.
>> 	  e.g.: To input as RAW events the RAW event code is
>> 		-e r124f301
>> 	  The RAW event encoding followed is
>> 	  <socket ID(2 bit)><SCCL ID(4 bit)><ModuleID(4 bit)>
>> 	  <Bank(4 bit)><event code(12 bit)>
>> 	  So 0x1 is SocketID, 0x2 is SCCL ID, 0x4 is the LLC
>> 	  hardware module ID, 0xf is for sum of all LLC banks,
>> 	  0x301 is the event code for LLC_READ_ALLOCATE.
> If you have a special way of encoding events, please add documentation
> for this, as we do for CCN, and as is being done for the X-Gene SoC PMU
> driver [1].
>
> The commit message is not an appropriate location to document a
> user-facing ABI which the user needs to know about.
>
> For example, I have no idea what a "SCCL ID" is, how the LLC HW module
> ID corresponds topologically, what a djtag unit is, nor how a djtag unit
> relates to other units in the system.
>
> Given that, and the lack of information regarding the djtag unit(s),
> I've skimmed over the rest of this patch, ignoring the portions I canot
> review due to a lack of appropriate information.
OK. I shall add the event details in Documentation/perf/hip05-pmu.txt
>
>> +u64 hisi_llc_event_update(struct perf_event *event,
>> +				struct hw_perf_event *hwc, int idx)
>> +{
>> +	struct device_node *djtag_node;
>> +	struct hisi_pmu *pllc_pmu = to_hisi_pmu(event->pmu);
>> +	struct hisi_hwmod_unit *punit;
>> +	struct hisi_llc_data *llc_hwmod_data;
>> +	u64 delta, prev_raw_count, new_raw_count = 0;
>> +	int cfg_en;
>> +	u32 raw_event_code = hwc->config_base;
>> +	u32 scclID = (raw_event_code & HISI_SCCL_MASK) >> 20;
>> +	u32 llc_idx = scclID - 1;
>> +	int i;
>> +
>> +	if (!scclID || (scclID >= HISI_SCCL_MASK)) {
>> +		pr_err("Invalid SCCL=%d in event code!\n", scclID);
>> +		return 0;
>> +	}
>> +
>> +	if (!hisi_llc_counter_valid(idx)) {
>> +		pr_err("Unsupported event index:%d!\n", idx);
>> +		return 0;
>> +	}
>> +
>> +	punit = &pllc_pmu->hwmod_pmu_unit[llc_idx];
>> +	llc_hwmod_data = punit->hwmod_data;
>> +
>> +	/* Check if the LLC data is initialized for this SCCL */
>> +	if (!llc_hwmod_data->djtag_node) {
>> +		pr_err("SCCL=%d not initialized!\n", scclID);
>> +		return 0;
>> +	}
>> +
>> +	/* Find the djtag device node of the SCCL */
>> +	djtag_node = llc_hwmod_data->djtag_node;
> As above, I cannot follow this due to a lack of understanding of the HW.
> You will need to add documentation such that this can be understood.
OK. shall add in the documentation.
>
> [...]
>
>> +	/* Increment the active_events for the first event counting */
>> +	if (!atomic_inc_not_zero(active_events)) {
>> +		mutex_lock(&punit->reserve_mutex);
>> +		if (atomic_read(active_events) == 0)
>> +			atomic_inc(active_events);
>> +		mutex_unlock(&punit->reserve_mutex);
>> +	}
> If you have no special work to do to activate the PMU HW, this reference
> counting is pointless. Please get rid of it entirely.
>
> [...]
OK. I shall remove them.
>
>> +int hisi_llc_get_event_idx(struct hisi_hwmod_unit *punit)
>> +{
>> +	struct hisi_llc_data *llc_hwmod_data = punit->hwmod_data;
>> +	int event_idx;
>> +
>> +	event_idx =
>> +		find_first_zero_bit(
>> +			llc_hwmod_data->hisi_llc_event_used_mask,
>> +					HISI_MAX_CFG_LLC_CNTR);
>> +
>> +	if (event_idx == HISI_MAX_CFG_LLC_CNTR)
>> +		return -EAGAIN;
>> +
>> +	__set_bit(event_idx,
>> +		llc_hwmod_data->hisi_llc_event_used_mask);
> For reference, what mutual exclusion mechanism protects the used_mask?
>
> [...]
I shall check it. Thanks.
>> +void hisi_llc_pmu_init(struct platform_device *pdev,
>> +					struct hisi_pmu *pllc_pmu)
>> +{
>> +	pllc_pmu->pmu_type = SCCL_SPECIFIC;
>> +	pllc_pmu->name = "HISI_L3C";
> Use lowercase. I suspect "hip05_l3c" is likely a better name, as
> Hisilcon may produce a new, different L3C controller, and you already
> use "hisilicon,hip05-llc" as the compatible string.
>
> [...]
OK. I shall use hip05_l3c for the pmu name,
>> +		pllc_pmu->pmu = (struct pmu) {
>> +				.name		= "HISI-L3C-PMU",
>> +		/*		.task_ctx_nr	= perf_invalid_context, */
> Why is this commented out? You *must* use perf_invalid_context.
>
>> +				.pmu_enable = hisi_uncore_pmu_enable,
>> +				.pmu_disable = hisi_uncore_pmu_disable,
>> +				.event_init = hisi_uncore_pmu_event_init,
>> +				.add = hisi_uncore_pmu_add,
>> +				.del = hisi_uncore_pmu_del,
>> +				.start = hisi_uncore_pmu_start,
>> +				.stop = hisi_uncore_pmu_stop,
>> +				.read = hisi_uncore_pmu_read,
>> +		};
> [...]
>
>> +MODULE_DESCRIPTION("HiSilicon ARMv8 LLC PMU driver");
> s/ARMv8/HIP05/, the SoC matters far more than the architecture it
> happens to use.
Ok.

Thanks,
Anurup
> Thanks,
> Mark.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438607.html

[PATCH 8/8] arm64:perf: L3 cache(LLC) event listing in perf

[Anurup M]

On Tuesday 28 June 2016 04:31 PM, Mark Rutland wrote:
> On Tue, Jun 28, 2016 at 05:50:29AM -0400, Anurup M wrote:
>> 	1. Add L3 caches events to /sys/devices/hisi_l3c/events/
>> 	   The events can be selected as shown in perf list
>> 	   e.g.: For LLC_READ_ALLOCATE event for Super CPU cluster 2 the
>> 		 event format is
>> 		 -e "hisi_l3c/l3c_read_allocate,bank=0xf,cluster=0x2/"
>>
>> Signed-off-by: Anurup M <anurup.m@huawei.com>
>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>> ---
>>   drivers/perf/hisilicon/hisi_uncore_llc.c | 75 +++++++++++++++++++++++++++++++-
>>   drivers/perf/hisilicon/hisi_uncore_pmu.c | 12 +++++
>>   drivers/perf/hisilicon/hisi_uncore_pmu.h | 16 +++++++
>>   3 files changed, 101 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_llc.c b/drivers/perf/hisilicon/hisi_uncore_llc.c
>> index a771e1a..316167a 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_llc.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_llc.c
>> @@ -25,11 +25,11 @@
>>   #include "hisi_uncore_llc.h"
>>   
>>   /* Map cfg_en values for LLC Banks */
>> -const int llc_cfgen_map[] = { HISI_LLC_BANK0_CFGEN, HISI_LLC_BANK1_CFGEN,
>> +static const int llc_cfgen_map[] = { HISI_LLC_BANK0_CFGEN, HISI_LLC_BANK1_CFGEN,
>>   				HISI_LLC_BANK2_CFGEN, HISI_LLC_BANK3_CFGEN
>>   };
>>   
>> -struct hisi_pmu *hisi_uncore_llc;
>> +static struct hisi_pmu *hisi_uncore_llc;
>>   
>>   static inline int hisi_llc_counter_valid(int idx)
>>   {
>> @@ -442,6 +442,76 @@ static void hisi_free_llc_data(struct hisi_hwmod_unit *punit)
>>   	kfree(punit->hwmod_data);
>>   }
>>   
>> +PMU_FORMAT_ATTR(event, "config:0-11");
>> +PMU_FORMAT_ATTR(bank, "config:12-15");
>> +PMU_FORMAT_ATTR(module, "config:16-19");
>> +PMU_FORMAT_ATTR(cluster, "config:20-23");
>> +PMU_FORMAT_ATTR(socket, "config:24-25");
>> +
>> +#define HISI_UNCORE_EVENT_DESC(_name, _config)			\
>> +{								\
>> +	.attr   = __ATTR(_name, 0444, uncore_event_show, NULL), \
>> +	.config = _config,					\
>> +}
>> +
>> +static struct attribute *hisi_llc_format_attr[] = {
>> +	&format_attr_event.attr,
>> +	&format_attr_bank.attr,
>> +	&format_attr_module.attr,
>> +	&format_attr_cluster.attr,
>> +	&format_attr_socket.attr,
>> +	NULL,
>> +};
> Pleas use the compound literal trick I describe in [1].
Thanks, I have modified them, shall send the revised patchset.

Thanks,
Anurup
>> +
>> +static struct attribute_group hisi_llc_format_group = {
>> +	.name = "format",
>> +	.attrs = hisi_llc_format_attr,
>> +};
>> +
>> +EVENT_ATTR_STR(l3c_read_allocate,
>> +			"event=0x301,bank=?,module=0x4,cluster=?,socket=0x1");
>> +EVENT_ATTR_STR(l3c_write_allocate,
>> +			"event=0x302,bank=?,module=0x4,cluster=?,socket=0x1");
>> +EVENT_ATTR_STR(l3c_read_noallocate,
>> +			"event=0x303,bank=?,module=0x4,cluster=?,socket=0x1");
>> +EVENT_ATTR_STR(l3c_write_noallocate,
>> +			"event=0x304,bank=?,module=0x4,cluster=?,socket=0x1");
>> +EVENT_ATTR_STR(l3c_read_hit,
>> +			"event=0x305,bank=?,module=0x4,cluster=?,socket=0x1");
>> +EVENT_ATTR_STR(l3c_write_hit,
>> +			"event=0x306,bank=?,module=0x4,cluster=?,socket=0x1");
>> +
>> +
>> +static struct attribute *hisi_llc_events_attr[] = {
>> +	EVENT_PTR(l3c_read_allocate),
>> +	EVENT_PTR(l3c_write_allocate),
>> +	EVENT_PTR(l3c_read_noallocate),
>> +	EVENT_PTR(l3c_write_noallocate),
>> +	EVENT_PTR(l3c_read_hit),
>> +	EVENT_PTR(l3c_write_hit),
>> +	NULL,
>> +};
> Again, please use the compound literal trick.
>
> Regardless of this sysfs data, the format needs documenting, as I
> commented in the last patch.
>
> Thanks,
> Mark.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438742.html