On Tue, May 12, 2020 at 03:31:13PM +0800, Joakim Zhang wrote: > The DDR Perf for i.MX8 is a system PMU whose axi id would different from > SoC to SoC. Need expose system PMU identifier for userspace which refer > to /sys/bus/event_source/devices/<PMU DEVICE>/identifier.
Why not just expose the AXI ID if that's what's different? > > Reviewed-by: John Garry <[email protected]> > Signed-off-by: Joakim Zhang <[email protected]> > --- > drivers/perf/fsl_imx8_ddr_perf.c | 45 +++++++++++++++++++++++++++++--- > 1 file changed, 42 insertions(+), 3 deletions(-) > > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c > b/drivers/perf/fsl_imx8_ddr_perf.c > index 95dca2cb5265..88addbffbbd0 100644 > --- a/drivers/perf/fsl_imx8_ddr_perf.c > +++ b/drivers/perf/fsl_imx8_ddr_perf.c > @@ -50,21 +50,38 @@ static DEFINE_IDA(ddr_ida); > > struct fsl_ddr_devtype_data { > unsigned int quirks; /* quirks needed for different DDR Perf core */ > + const char *identifier; /* system PMU identifier for userspace */ > }; > > -static const struct fsl_ddr_devtype_data imx8_devtype_data; > +static const struct fsl_ddr_devtype_data imx8_devtype_data = { > + .identifier = "i.MX8", > +}; > + > +static const struct fsl_ddr_devtype_data imx8mq_devtype_data = { > + .quirks = DDR_CAP_AXI_ID_FILTER, > + .identifier = "i.MX8MQ", > +}; > + > +static const struct fsl_ddr_devtype_data imx8mm_devtype_data = { > + .quirks = DDR_CAP_AXI_ID_FILTER, > + .identifier = "i.MX8MM", > +}; > > -static const struct fsl_ddr_devtype_data imx8m_devtype_data = { > +static const struct fsl_ddr_devtype_data imx8mn_devtype_data = { > .quirks = DDR_CAP_AXI_ID_FILTER, > + .identifier = "i.MX8MN", > }; > > static const struct fsl_ddr_devtype_data imx8mp_devtype_data = { > .quirks = DDR_CAP_AXI_ID_FILTER_ENHANCED, > + .identifier = "i.MX8MP", > }; > > static const struct of_device_id imx_ddr_pmu_dt_ids[] = { > { .compatible = "fsl,imx8-ddr-pmu", .data = &imx8_devtype_data}, > - { .compatible = "fsl,imx8m-ddr-pmu", .data = &imx8m_devtype_data}, You need to keep the old one for compatibility. > + { .compatible = "fsl,imx8mq-ddr-pmu", .data = &imx8mq_devtype_data}, > + { .compatible = "fsl,imx8mm-ddr-pmu", .data = &imx8mm_devtype_data}, > + { .compatible = "fsl,imx8mn-ddr-pmu", .data = &imx8mn_devtype_data}, > { .compatible = "fsl,imx8mp-ddr-pmu", .data = &imx8mp_devtype_data}, > { /* sentinel */ } > }; > @@ -84,6 +101,27 @@ struct ddr_pmu { > int id; > }; > > +static ssize_t ddr_perf_identifier_show(struct device *dev, > + struct device_attribute *attr, > + char *page) > +{ > + struct ddr_pmu *pmu = dev_get_drvdata(dev); > + > + return sprintf(page, "%s\n", pmu->devtype_data->identifier); Why do we need yet another way to identify the SoC from userspace? > +} > + > +static struct device_attribute ddr_perf_identifier_attr = > + __ATTR(identifier, 0444, ddr_perf_identifier_show, NULL); sysfs attributes are supposed to be documented. > + > +static struct attribute *ddr_perf_identifier_attrs[] = { > + &ddr_perf_identifier_attr.attr, > + NULL, > +}; > + > +static struct attribute_group ddr_perf_identifier_attr_group = { > + .attrs = ddr_perf_identifier_attrs, > +}; > + > enum ddr_perf_filter_capabilities { > PERF_CAP_AXI_ID_FILTER = 0, > PERF_CAP_AXI_ID_FILTER_ENHANCED, > @@ -237,6 +275,7 @@ static const struct attribute_group *attr_groups[] = { > &ddr_perf_format_attr_group, > &ddr_perf_cpumask_attr_group, > &ddr_perf_filter_cap_attr_group, > + &ddr_perf_identifier_attr_group, > NULL, > }; > > -- > 2.17.1 >
