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

[Keith Busch <kbusch@kernel.org>]

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 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.