Messages in this thread | | | Date | Wed, 16 Jun 2021 10:23:43 -0500 | From | Bjorn Helgaas <> | Subject | Re: [PATCH v6 2/2] drivers/perf: hisi: Add driver for HiSilicon PCIe PMU |
| |
On Wed, Jun 16, 2021 at 09:09:40AM +0800, liuqi (BA) wrote: > On 2021/6/12 7:33, Krzysztof Wilczyński wrote:
> > > +static ssize_t hisi_pcie_event_sysfs_show(struct device *dev, > > > + struct device_attribute *attr, char *buf) > > > +{ > > > + struct dev_ext_attribute *eattr; > > > + > > > + eattr = container_of(attr, struct dev_ext_attribute, attr); > > > + > > > + return sysfs_emit(buf, "config=0x%lx\n", (unsigned long)eattr->var); > > > +} > > > > I am not that familiar with the perf drivers, thus I might be completely > > wrong here, but usually for sysfs objects a single value is preferred, > > so that this "config=" technically would not be needed, unless this is > > somewhat essential to the consumers of this attribute to know what the > > value is? What do you think? > > "config=" is a supported for userspace tool, it is a kind of alias, so > cannot be remover here, thanks.
I don't understand this part. This is brand-new functionality for the kernel, so there's no requirement to maintain compatibility for existing userspace tools.
If there's a similar sysfs show function for other perf drivers, and you need to be compatible with *that*, fine. But if this is merely about being compatible with userspace that uses out-of-tree kernel functionality, that's not a real factor.
Bjorn
| |