Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Mon, 15 Sep 2025 16:23:07 -0600
Keith Busch <kbusch@kernel.org> wrote:

> On Sat, Sep 13, 2025 at 03:20:41PM +0900, Krzysztof Wilczy´nski wrote:
> > Hello,
> >   
> > > @@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
> > >  	struct device *dev = kobj_to_dev(kobj);
> > >  	struct pci_dev *pdev = to_pci_dev(dev);
> > >  
> > > -	if (pci_is_pcie(pdev))
> > > -		return a->mode;
> > > +	if (!pci_is_pcie(pdev))
> > > +		return 0;
> > >  
> > > -	return 0;
> > > +	if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
> > > +		return 0;
> > > +
> > > +	return a->mode;  
> > 
> > It would be fine to have this sysfs attribute present all the time, and
> > simply return error when the serial number is not available.  Not sure if
> > hiding it adds a lot of value.  This is how some of the existing attributes
> > currently behave.
> > 
> > But it does add extra code to pcie_dev_attrs_are_visible() where it is now
> > a special case, somewhat.  
> 
> You bring up a good point, but I think it seems odd that the existing
> pcie attributes are visible even if we know reading it will fail.

Perhaps historical. The is_visible infrastructure is I think somewhat
newer than a lot of that ABI.

> Perhaps the pcie link status visibility should be changed to follow this
> patch's example to hide when they don't exist. Applications might notice
> a different error, ENOENT vs EINVAL, if the device doesn't support the
> capability, but that is a more accurate errno.

As it is sysfs we can never be sure someone hasn't assumed existing files
are present even when they aren't useful.

So I doubt we can improve the existing cases without something breaking.
If we want to give it a go and see who screams I'm fine with that :)

Jonathan