[PATCH v8 8/8] perf: ARM DynamIQ Shared Unit PMU support

On 18/10/17 10:20, Mark Rutland wrote:

Hi Suzuki,

This generally looks good. My comments below are mostly minor, modulo
the probing/hotplug bit at the end.

...

quoted

+static void dsu_pmu_probe_pmu(void *data)
+{
+	struct dsu_pmu *dsu_pmu = data;
+	u64 num_counters;
+	u32 cpmceid[2];
+
+	num_counters = (__dsu_pmu_read_pmcr() >> CLUSTERPMCR_N_SHIFT) &
+						CLUSTERPMCR_N_MASK;
+	/* We can only support upto 31 independent counters */

s/upto/up to/

Does the hardware spec allow for more than this?

No, the "counter" mask registers are 32bit wide, with Bit 31 reserved for the
Cycle counter.

quoted

+	if (WARN_ON(num_counters > 31))
+		num_counters = 31;
+	dsu_pmu->num_counters = num_counters;
+	if (!dsu_pmu->num_counters)
+		return;
+	cpmceid[0] = __dsu_pmu_read_pmceid(0);
+	cpmceid[1] = __dsu_pmu_read_pmceid(1);
+	bitmap_from_u32array(dsu_pmu->cpmceid_bitmap,
+				DSU_PMU_MAX_COMMON_EVENTS,
+				cpmceid,
+				ARRAY_SIZE(cpmceid));
+}

[...]

quoted

+
+static int dsu_pmu_device_probe(struct platform_device *pdev)
+{
+	int irq, rc, cpu;
+	struct dsu_pmu *dsu_pmu;
+	char *name;
+	static atomic_t pmu_idx = ATOMIC_INIT(-1);
+
+	dsu_pmu = dsu_pmu_alloc(pdev);
+	if (IS_ERR(dsu_pmu))
+		return PTR_ERR(dsu_pmu);
+
+	rc = dsu_pmu_dt_get_cpus(pdev->dev.of_node, &dsu_pmu->associated_cpus);
+	if (rc) {
+		dev_warn(&pdev->dev, "Failed to parse the CPUs\n");
+		return rc;
+	}
+
+	rc = smp_call_function_any(&dsu_pmu->associated_cpus,
+					dsu_pmu_probe_pmu,
+					dsu_pmu, 1);

Can we probe the relevant CPUs in the same was as the qcom l2 pmu, using
notifiers?

That way we'd work better with maxcpus=.

We have to do this cross-call in the arm_pmu driver because we need the
name at probe time, but that doesn't apply here. We should be able to
lazily initialize the set of events and number of counters.

OK, I will make the necessary changes.

Thanks a lot for the review.

Cheers
Suzuki