Re: [RFC PATCH v1 2/2] perf/smmuv3: To support the dts to get options
From: Robin Murphy <robin.murphy@arm.com>
Date: 2020-07-06 15:05:08
On 2020-07-06 12:22, Jay Chen wrote:
quoted hunk ↗ jump to hunk
For the smmuv3 pmu for support the dts to get the options Signed-off-by: Jay Chen <redacted> --- drivers/perf/arm_smmuv3_pmu.c | 42 ++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-)diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c index 2d09f3e47d12..44e9f4197444 100644 --- a/drivers/perf/arm_smmuv3_pmu.c +++ b/drivers/perf/arm_smmuv3_pmu.c@@ -115,6 +115,16 @@ struct smmu_pmu { bool global_filter; }; +struct smmu_pmu_prop { + u32 opt; + const char *prop; +}; + +static struct smmu_pmu_prop smmu_pmu_options[] = { + { SMMU_PMCG_EVCNTR_RDONLY, "hisilicon,smmu-pmcg-evcntr-rdonly"},
We know this kind of thing doesn't scale well - please define a compatible string for the specific PMU model. In fact my hope was that DT compatibles would map to the same model values we made up for IORT to pass, such that the *only* firmware-specific difference the driver needs to have is whether to retrieve the model from platdata or OF match data, then the actual quirks are applied commonly.
quoted hunk ↗ jump to hunk
+ { 0, NULL}, +}; + #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu)) #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start, _end) \@@ -708,6 +718,7 @@ static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu) smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0); } +#ifdef CONFIG_ACPI
Why bother stubbing this out? It shouldn't have any build-time dependency on ACPI code, and saving a whole 48 bytes from the object size (for my local build setup) struggles to justify the source-code clutter.
quoted hunk ↗ jump to hunk
static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu) { u32 model;@@ -723,6 +734,26 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu) dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options); } +#else +static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu) +{ + +} +#endif + +static void smmu_pmu_get_dt_options(struct smmu_pmu *smmu_pmu) +{ + int i = 0; + + do { + if (of_property_read_bool(smmu_pmu->dev->of_node, + smmu_pmu_options[i].prop)) { + smmu_pmu->options |= smmu_pmu_options[i].opt; + dev_notice(smmu_pmu->dev, "option mask 0x%x\n", + smmu_pmu->options); + } + } while (smmu_pmu_options[++i].opt); +} static int smmu_pmu_probe(struct platform_device *pdev) {@@ -801,7 +832,10 @@ static int smmu_pmu_probe(struct platform_device *pdev) return -EINVAL; } - smmu_pmu_get_acpi_options(smmu_pmu); + if (dev->of_node) + smmu_pmu_get_dt_options(smmu_pmu); + else + smmu_pmu_get_acpi_options(smmu_pmu); /* Pick one CPU to be the preferred one to use */ smmu_pmu->on_cpu = raw_smp_processor_id();@@ -855,9 +889,15 @@ static void smmu_pmu_shutdown(struct platform_device *pdev) smmu_pmu_disable(&smmu_pmu->pmu); } +static const struct of_device_id smmu_pmu_of_match[] = { + { .compatible = "arm-smmu-v3-pmcg", },
Please define the DT binding first. IIRC Jean-Philippe wrote some patches a while back that never got posted, but I suppose it should be YAML now...
+ { },
+};How about the thing for module autoloading too? Robin.
+
static struct platform_driver smmu_pmu_driver = {
.driver = {
.name = "arm-smmu-v3-pmcg",
+ .of_match_table = smmu_pmu_of_match,
},
.probe = smmu_pmu_probe,
.remove = smmu_pmu_remove,_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel