Hi Heiner,
Thank you for working on this and for cleaning this up!
On 21-02-03 09:50:13, Heiner Kallweit wrote:
> Since 104daa71b396 ("PCI: Determine actual VPD size on first access")
> there's nothing that keeps us from using a static attribute.
> This allows to use PCI sysfs core code for handling the attribute.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
> drivers/pci/pci-sysfs.c | 54 ++++++++++++++++++++++++++++++-----------
> drivers/pci/pci.h | 2 --
> drivers/pci/vpd.c | 54 -----------------------------------------
> 3 files changed, 40 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index fb072f4b3..ed2ded167 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -255,6 +255,25 @@ static ssize_t ari_enabled_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(ari_enabled);
>
> +static ssize_t vpd_read(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *bin_attr, char *buf,
> + loff_t off, size_t count)
> +{
> + struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj));
> +
> + return pci_read_vpd(dev, off, count, buf);
> +}
> +
> +static ssize_t vpd_write(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *bin_attr, char *buf,
> + loff_t off, size_t count)
> +{
> + struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj));
> +
> + return pci_write_vpd(dev, off, count, buf);
> +}
> +static BIN_ATTR_RW(vpd, 0);
> +
> static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> @@ -621,6 +640,11 @@ static struct attribute *pci_dev_attrs[] = {
> NULL,
> };
>
> +static struct bin_attribute *pci_dev_bin_attrs[] = {
> + &bin_attr_vpd,
> + NULL,
> +};
> +
> static struct attribute *pci_bridge_attrs[] = {
> &dev_attr_subordinate_bus_number.attr,
> &dev_attr_secondary_bus_number.attr,
> @@ -1323,20 +1347,10 @@ static DEVICE_ATTR(reset, 0200, NULL, reset_store);
>
> static int pci_create_capabilities_sysfs(struct pci_dev *dev)
> {
> - int retval;
> -
> - pcie_vpd_create_sysfs_dev_files(dev);
> -
> - if (dev->reset_fn) {
> - retval = device_create_file(&dev->dev, &dev_attr_reset);
> - if (retval)
> - goto error;
> - }
> - return 0;
> + if (!dev->reset_fn)
> + return 0;
>
> -error:
> - pcie_vpd_remove_sysfs_dev_files(dev);
> - return retval;
> + return device_create_file(&dev->dev, &dev_attr_reset);
> }
>
> int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
> @@ -1409,7 +1423,6 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>
> static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
> {
> - pcie_vpd_remove_sysfs_dev_files(dev);
> if (dev->reset_fn) {
> device_remove_file(&dev->dev, &dev_attr_reset);
> dev->reset_fn = 0;
> @@ -1523,8 +1536,21 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
> return 0;
> }
>
> +static umode_t pci_dev_bin_attrs_visible(struct kobject *kobj,
> + struct bin_attribute *a, int n)
> +{
> + struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
> +
> + if (a == &bin_attr_vpd && !pdev->vpd)
> + return 0;
> +
> + return a->attr.mode;
> +}
> +
> static const struct attribute_group pci_dev_group = {
> .attrs = pci_dev_attrs,
> + .bin_attrs = pci_dev_bin_attrs,
> + .is_bin_visible = pci_dev_bin_attrs_visible,
> };
>
> const struct attribute_group *pci_dev_groups[] = {
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index ef7c46613..d6ad1a3e2 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -143,8 +143,6 @@ static inline bool pcie_downstream_port(const struct pci_dev *dev)
>
> int pci_vpd_init(struct pci_dev *dev);
> void pci_vpd_release(struct pci_dev *dev);
> -void pcie_vpd_create_sysfs_dev_files(struct pci_dev *dev);
> -void pcie_vpd_remove_sysfs_dev_files(struct pci_dev *dev);
>
> /* PCI Virtual Channel */
> int pci_save_vc_state(struct pci_dev *dev);
> diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
> index 2096530ce..a33a8fd7e 100644
> --- a/drivers/pci/vpd.c
> +++ b/drivers/pci/vpd.c
> @@ -21,7 +21,6 @@ struct pci_vpd_ops {
>
> struct pci_vpd {
> const struct pci_vpd_ops *ops;
> - struct bin_attribute *attr; /* Descriptor for sysfs VPD entry */
> struct mutex lock;
> unsigned int len;
> u16 flag;
> @@ -397,59 +396,6 @@ void pci_vpd_release(struct pci_dev *dev)
> kfree(dev->vpd);
> }
>
> -static ssize_t read_vpd_attr(struct file *filp, struct kobject *kobj,
> - struct bin_attribute *bin_attr, char *buf,
> - loff_t off, size_t count)
> -{
> - struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj));
> -
> - return pci_read_vpd(dev, off, count, buf);
> -}
> -
> -static ssize_t write_vpd_attr(struct file *filp, struct kobject *kobj,
> - struct bin_attribute *bin_attr, char *buf,
> - loff_t off, size_t count)
> -{
> - struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj));
> -
> - return pci_write_vpd(dev, off, count, buf);
> -}
> -
> -void pcie_vpd_create_sysfs_dev_files(struct pci_dev *dev)
> -{
> - int retval;
> - struct bin_attribute *attr;
> -
> - if (!dev->vpd)
> - return;
> -
> - attr = kzalloc(sizeof(*attr), GFP_ATOMIC);
> - if (!attr)
> - return;
> -
> - sysfs_bin_attr_init(attr);
> - attr->size = 0;
> - attr->attr.name = "vpd";
> - attr->attr.mode = S_IRUSR | S_IWUSR;
> - attr->read = read_vpd_attr;
> - attr->write = write_vpd_attr;
> - retval = sysfs_create_bin_file(&dev->dev.kobj, attr);
> - if (retval) {
> - kfree(attr);
> - return;
> - }
> -
> - dev->vpd->attr = attr;
> -}
> -
> -void pcie_vpd_remove_sysfs_dev_files(struct pci_dev *dev)
> -{
> - if (dev->vpd && dev->vpd->attr) {
> - sysfs_remove_bin_file(&dev->dev.kobj, dev->vpd->attr);
> - kfree(dev->vpd->attr);
> - }
> -}
> -
> int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt)
> {
> int i;
> --
> 2.30.0
>
This looks so much better now.
Reviewed-by: Krzysztof Wilczyński <kw@xxxxxxxxx>
Krzysztof
