As CPA PMU is on SICL, and id of SICL is 1 smaller than id of adjacent SCCL. SCCL id in our Hip09 platform is set as 1, 3.... so, a reasonable sccl-id will never be 0.
+ name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sicl%d_cpa%u", cpa_pmu->sccl_id - 1,
why subtract 1? Looks dangerous if sccl_id == 0
Parameters sccl_id is read from ACPI, and is checked in hisi_pmu_cpu_is_associated_pmu(), so we could make sure that sccl_id here is reasonable and is not 0.
+ cpa_pmu->index_id);
+
+ cpa_pmu->pmu = (struct pmu) {
+ .name = name,
+ .module = THIS_MODULE,
+ .task_ctx_nr = perf_invalid_context,
+ .event_init = hisi_uncore_pmu_event_init,
+ .pmu_enable = hisi_uncore_pmu_enable,
+ .pmu_disable = hisi_uncore_pmu_disable,
+ .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,
+ .attr_groups = cpa_pmu->pmu_events.attr_groups,
+ .capabilities = PERF_PMU_CAP_NO_EXCLUDE,
+ };
+
+ ret = perf_pmu_register(&cpa_pmu->pmu, name, -1);
+ if (ret) {
+ dev_err(cpa_pmu->dev, "CPA PMU register failed\n");
+ cpuhp_state_remove_instance_nocalls(
+ CPUHP_AP_PERF_ARM_HISI_CPA_ONLINE, &cpa_pmu->node);
+ }
+
+ return ret;
+}
+
+static int hisi_cpa_pmu_remove(struct platform_device *pdev)
+{
+ struct hisi_pmu *cpa_pmu = platform_get_drvdata(pdev);
+
+ perf_pmu_unregister(&cpa_pmu->pmu);
+ cpuhp_state_remove_instance_nocalls(CPUHP_AP_PERF_ARM_HISI_CPA_ONLINE,
+ &cpa_pmu->node);
+ return 0;
+}
+
+static struct platform_driver hisi_cpa_pmu_driver = {
+ .driver = {
+ .name = "hisi_cpa_pmu",
+ .acpi_match_table = ACPI_PTR(hisi_cpa_pmu_acpi_match),
+ .suppress_bind_attrs = true,
+ },
+ .probe = hisi_cpa_pmu_probe,
+ .remove = hisi_cpa_pmu_remove,
+};
+
+static int __init hisi_cpa_pmu_module_init(void)
+{
+ int ret;
+
+ ret = cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_HISI_CPA_ONLINE,
+ "AP_PERF_ARM_HISI_CPA_ONLINE",
+ hisi_uncore_pmu_online_cpu,
+ hisi_uncore_pmu_offline_cpu);
+ if (ret) {
+ pr_err("CPA PMU: setup hotplug: %d\n", ret);
+ return ret;
+ }
+
+ ret = platform_driver_register(&hisi_cpa_pmu_driver);
+ if (ret)
+ cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_HISI_CPA_ONLINE);
I am not being totally serious, but this pattern of registering a CPU hotplug handler and then a driver is so common that we could nearly add a wrapper for it.
Hi John, do you mean somthing like this?
in hisi_uncore_pmu.c:
int hisi_uncore_pmu_module_init(enum cpuhp_state state, const char *name, struct platform_driver *drv)
{
int ret;
ret = cpuhp_setup_state_multi(state, name, hisi_uncore_pmu_online_cpu,
hisi_uncore_pmu_offline_cpu);
if (ret) {
pr_err("%s: setup hotplug: %d\n", drv->driver.name, ret);
return ret;
}
ret = platform_driver_register(drv);
if (ret)
cpuhp_remove_multi_state(state);
return ret;
}
in hisi_uncore_cpa_pmu.c:
static int __init hisi_cpa_pmu_module_init(void)
{
int ret;
ret = hisi_uncore_pmu_module_init(CPUHP_AP_PERF_ARM_HISI_CPA_ONLINE,
"AP_PERF_ARM_HISI_CPA_ONLINE",
&hisi_cpa_pmu_driver);
return ret;
}
module_init(hisi_cpa_pmu_module_init);