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 <john.garry@xxxxxxxxxx>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@xxxxxxx>
> ---
> 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
>
