Messages in this thread | | | Date | Wed, 29 Mar 2023 19:53:56 +0800 | Subject | Re: [PATCH RFC 1/4] driver/perf: Add identifier sysfs file for CMN | From | Jing Zhang <> |
| |
在 2023/3/27 下午3:55, John Garry 写道: > On 27/03/2023 03:46, 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 | 43 +++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 43 insertions(+) >> >> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c >> index c968986..0c138ad 100644 >> --- a/drivers/perf/arm-cmn.c >> +++ b/drivers/perf/arm-cmn.c >> @@ -1168,10 +1168,53 @@ 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)); >> + if (cmn->model == CMN700) { >> + return sysfs_emit(buf, "%s\n", "CMN700"); > > Is it possible to have a pointer to this string in struct arm_cmn, such that we don't have to do this model to identifier lookup here? If-else chains like this are not scalable. > Will do.
> BTW, does this HW have some HW identifier register, like iidr? I think that using that may be preferable. >
I didn't find the relevant identifier register.
Do Illka and Robin know that there is such a register that can identify different CMN versions? Looking forward to your suggestions.
>> + } >> + else if (cmn->model == CMN650) { >> + return sysfs_emit(buf, "%s\n", "CMN650"); > > I'd use lowercase names > Ok.
>> + } >> + else if (cmn->model == CMN600) { >> + return sysfs_emit(buf, "%s\n", "CMN600"); >> + } >> + else if (cmn->model == CI700) { >> + return sysfs_emit(buf, "%s\n", "CI700"); >> + } >> + return sysfs_emit(buf, "%s\n", "UNKNOWN"); > > can we have a "is_visble" attr to just no show this when unknown? >
Ok.
>> +} >> + >> +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->model <= 0) >> + return 0; >> + 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, > > nit: no need for trailing ',' on a sentinel >
Ok, Will do.
>> +}; >> + >> +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 >> }; >>
| |