Messages in this thread Patch in this message | | | Date | Mon, 1 May 2023 13:38:00 +0100 | Subject | Re: [PATCH v2 1/5] driver/perf: Add identifier sysfs file for CMN | From | John Garry <> |
| |
On 24/04/2023 10:44, Jing Zhang wrote: > To allow userspace to identify the specific implementation of the device, > add an "identifier" sysfs file. > > The perf tool can match the arm CMN metric through the identifier. > > Signed-off-by: Jing Zhang<renyu.zj@linux.alibaba.com> > --- > drivers/perf/arm-cmn.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c > index c968986..f425610 100644 > --- a/drivers/perf/arm-cmn.c > +++ b/drivers/perf/arm-cmn.c > @@ -334,6 +334,7 @@ struct arm_cmn { > > struct pmu pmu; > struct dentry *debug; > + const char *identifier; > }; > > #define to_cmn(p) container_of(p, struct arm_cmn, pmu) > @@ -1168,10 +1169,41 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev, > .attrs = arm_cmn_cpumask_attrs, > }; > > +static ssize_t arm_cmn_identifier_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev)); > + > + return sysfs_emit(buf, "%s\n", cmn->identifier); > +} > + > +static umode_t arm_cmn_identifier_attr_visible(struct kobject *kobj, > + struct attribute *attr, int n) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev)); > + > + if (cmn->identifier == NULL) > + return 0;
nit: generally if (!val) is preferred
> + return attr->mode; > +}; > + > +static struct device_attribute arm_cmn_identifier_attr = > +__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL); > + > +static struct attribute *arm_cmn_identifier_attrs[] = { > + &arm_cmn_identifier_attr.attr, > + NULL > +}; > + > +static struct attribute_group arm_cmn_identifier_attr_group = { > + .attrs = arm_cmn_identifier_attrs, > + .is_visible = arm_cmn_identifier_attr_visible > +}; > + > static const struct attribute_group *arm_cmn_attr_groups[] = { > &arm_cmn_event_attrs_group, > &arm_cmn_format_attrs_group, > &arm_cmn_cpumask_attr_group, > + &arm_cmn_identifier_attr_group, > NULL > }; > > @@ -2241,6 +2273,22 @@ static int arm_cmn600_of_probe(struct device_node *np) > return of_property_read_u32(np, "arm,root-node", &rootnode) ?: rootnode; > } > > +const char *arm_cmn_identifier(unsigned long model) > +{ > + switch (model) { > + case CMN600: > + return "cmn600"; > + case CMN650: > + return "cmn650"; > + case CMN700: > + return "cmn700"; > + case CI700: > + return "ci700"; > + default: > + return NULL; > + }
nit: I think that it would be nicer to have this per-model string stored statically in arm_cmn_acpi_match[].driver_data and arm_cmn_of_match[].data, so we have a straight lookup
> +} > + > static int arm_cmn_probe(struct platform_device *pdev) > { > struct arm_cmn *cmn; > @@ -2254,6 +2302,7 @@ static int arm_cmn_probe(struct platform_device *pdev) > > cmn->dev = &pdev->dev; > cmn->model = (unsigned long)device_get_match_data(cmn->dev); > + cmn->identifier = arm_cmn_identifier(cmn->model); > platform_set_drvdata(pdev, cmn); >
| |