linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Krzysztof Wilczyński" <kw@linux.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: "Pali Rohár" <pali@kernel.org>,
	"Oliver O'Halloran" <oohall@gmail.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Joe Perches" <joe@perches.com>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>,
	"David Sterba" <dsterba@suse.com>,
	linux-pci@vger.kernel.org
Subject: [PATCH 05/20] PCI: Convert dynamic "index" and "label" sysfs objects into static
Date: Fri, 16 Apr 2021 20:58:41 +0000	[thread overview]
Message-ID: <20210416205856.3234481-6-kw@linux.com> (raw)
In-Reply-To: <20210416205856.3234481-1-kw@linux.com>

The "index" and "label" sysfs objects, which are read-only attributes,
that reside at the "/sys/bus/pci/devices/.../index" (or "acpi_index"
with the ACPI _DSM support) and "/sys/bus/pci/devices/.../label" (when
using either SMBIOS or ACPI _DSM) paths and allow for accessing the PCI
device label, either from the BIOS (SMBIOS) or through the ACPI _DSM
(Device-Specific Methods), and the device function index.

At the moment, both the "index" (or "acpi_index") and "label" static
sysfs objects are dynamically created when the pci_create_sysfs_dev_files()
function executes that is part of the PCI sysfs objects initialisation
and when a device is added:

  late_initcall()
    pci_sysfs_init()
      pci_create_sysfs_dev_files()
        pci_create_firmware_label_files()
          pci_create_acpi_index_label_files()
            <--------------------------- Depending on what is supported.
          pci_create_smbiosname_file()

  pci_bus_add_devices()
    pci_bus_add_device()
      pci_create_sysfs_dev_files()
        ...

And dynamically removed when the pci_remove_sysfs_dev_files() function
executes when the PCI device is stopped and removed:

  pci_stop_bus_device()
    pci_stop_dev()
      pci_remove_sysfs_dev_files()
        pci_remove_firmware_label_files()
          pci_remove_acpi_index_label_files()
            <--------------------------- Depending on what is supported.
          pci_remove_smbiosname_file()

These attributes do not need to be created dynamically and removed
dynamically, and thus there is no need to also manage their create and
remove life cycle manually as the PCI driver core can do it when the
device is either added or removed.

Convert the "index", "acpi_index" and "label" sysfs objects created
dynamically to static and use the .is_visible callback to check whether
the SMBIOS or ACPI _DSM is available to decide if and which of the sysfs
objects should be made available.

Then, add a newly created attribute group to the existing group called
"pci_dev_groups" to ensure that the "config" sysfs object would be
automatically included when the PCI driver initializes.

And also rename the device_has_dsm() function to device_has_acpi_name()
to better relfect its purpose.

Suggested-by: Oliver O'Halloran <oohall@gmail.com>
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci-label.c | 212 +++++++++++++---------------------------
 drivers/pci/pci-sysfs.c |  17 ++--
 drivers/pci/pci.h       |  13 +--
 3 files changed, 80 insertions(+), 162 deletions(-)

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index 781e45cf60d1..31fedc4920bb 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -33,6 +33,22 @@
 #include <linux/pci-acpi.h>
 #include "pci.h"
 
+static bool device_has_acpi_name(struct device *dev)
+{
+#ifdef CONFIG_ACPI
+	acpi_handle handle;
+
+	handle = ACPI_HANDLE(dev);
+	if (!handle)
+		return false;
+
+	return !!acpi_check_dsm(handle, &pci_acpi_dsm_guid, 0x2,
+				1 << DSM_PCI_DEVICE_NAME);
+#else
+	return false;
+#endif
+}
+
 #ifdef CONFIG_DMI
 enum smbios_attr_enum {
 	SMBIOS_ATTR_NONE = 0,
@@ -45,13 +61,9 @@ static size_t find_smbios_instance_string(struct pci_dev *pdev, char *buf,
 {
 	const struct dmi_device *dmi;
 	struct dmi_dev_onboard *donboard;
-	int domain_nr;
-	int bus;
-	int devfn;
-
-	domain_nr = pci_domain_nr(pdev->bus);
-	bus = pdev->bus->number;
-	devfn = pdev->devfn;
+	int domain_nr = pci_domain_nr(pdev->bus);
+	int bus = pdev->bus->number;
+	int devfn = pdev->devfn;
 
 	dmi = NULL;
 	while ((dmi = dmi_find_device(DMI_DEV_TYPE_DEV_ONBOARD,
@@ -73,81 +85,57 @@ static size_t find_smbios_instance_string(struct pci_dev *pdev, char *buf,
 			return strlen(dmi->name);
 		}
 	}
-	return 0;
-}
-
-static umode_t smbios_instance_string_exist(struct kobject *kobj,
-					    struct attribute *attr, int n)
-{
-	struct device *dev;
-	struct pci_dev *pdev;
 
-	dev = kobj_to_dev(kobj);
-	pdev = to_pci_dev(dev);
-
-	return find_smbios_instance_string(pdev, NULL, SMBIOS_ATTR_NONE) ?
-					   S_IRUGO : 0;
+	return 0;
 }
 
-static ssize_t smbioslabel_show(struct device *dev,
-				struct device_attribute *attr, char *buf)
+static ssize_t smbios_label_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
 {
-	struct pci_dev *pdev;
-	pdev = to_pci_dev(dev);
+	struct pci_dev *pdev = to_pci_dev(dev);
 
 	return find_smbios_instance_string(pdev, buf,
 					   SMBIOS_ATTR_LABEL_SHOW);
 }
+static struct device_attribute dev_attr_smbios_label = __ATTR(label, 0444,
+							      smbios_label_show,
+							      NULL);
 
-static ssize_t smbiosinstance_show(struct device *dev,
-				   struct device_attribute *attr, char *buf)
+static ssize_t index_show(struct device *dev,
+			  struct device_attribute *attr, char *buf)
 {
-	struct pci_dev *pdev;
-	pdev = to_pci_dev(dev);
+	struct pci_dev *pdev = to_pci_dev(dev);
 
 	return find_smbios_instance_string(pdev, buf,
 					   SMBIOS_ATTR_INSTANCE_SHOW);
 }
+static DEVICE_ATTR_RO(index);
 
-static struct device_attribute smbios_attr_label = {
-	.attr = {.name = "label", .mode = 0444},
-	.show = smbioslabel_show,
-};
-
-static struct device_attribute smbios_attr_instance = {
-	.attr = {.name = "index", .mode = 0444},
-	.show = smbiosinstance_show,
-};
-
-static struct attribute *smbios_attributes[] = {
-	&smbios_attr_label.attr,
-	&smbios_attr_instance.attr,
+static struct attribute *smbios_attrs[] = {
+	&dev_attr_smbios_label.attr,
+	&dev_attr_index.attr,
 	NULL,
 };
 
-static const struct attribute_group smbios_attr_group = {
-	.attrs = smbios_attributes,
-	.is_visible = smbios_instance_string_exist,
-};
-
-static int pci_create_smbiosname_file(struct pci_dev *pdev)
+static umode_t smbios_attr_is_visible(struct kobject *kobj,
+				      struct attribute *a, int n)
 {
-	return sysfs_create_group(&pdev->dev.kobj, &smbios_attr_group);
-}
+	struct device *dev = kobj_to_dev(kobj);
+	struct pci_dev *pdev = to_pci_dev(dev);
 
-static void pci_remove_smbiosname_file(struct pci_dev *pdev)
-{
-	sysfs_remove_group(&pdev->dev.kobj, &smbios_attr_group);
-}
-#else
-static inline int pci_create_smbiosname_file(struct pci_dev *pdev)
-{
-	return -1;
-}
+	if (device_has_acpi_name(dev))
+		return 0;
 
-static inline void pci_remove_smbiosname_file(struct pci_dev *pdev)
-{
+	if (!find_smbios_instance_string(pdev, NULL, SMBIOS_ATTR_NONE))
+		return 0;
+
+	return a->mode;
 }
+
+const struct attribute_group smbios_attr_group = {
+	.attrs = smbios_attrs,
+	.is_visible = smbios_attr_is_visible,
+};
 #endif
 
 #ifdef CONFIG_ACPI
@@ -209,103 +197,41 @@ static int dsm_get_label(struct device *dev, char *buf,
 	return len;
 }
 
-static bool device_has_dsm(struct device *dev)
-{
-	acpi_handle handle;
-
-	handle = ACPI_HANDLE(dev);
-	if (!handle)
-		return false;
-
-	return !!acpi_check_dsm(handle, &pci_acpi_dsm_guid, 0x2,
-				1 << DSM_PCI_DEVICE_NAME);
-}
-
-static umode_t acpi_index_string_exist(struct kobject *kobj,
-				       struct attribute *attr, int n)
-{
-	struct device *dev;
-
-	dev = kobj_to_dev(kobj);
-
-	if (device_has_dsm(dev))
-		return S_IRUGO;
-
-	return 0;
-}
-
-static ssize_t acpilabel_show(struct device *dev,
-			      struct device_attribute *attr, char *buf)
+static ssize_t acpi_label_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
 {
 	return dsm_get_label(dev, buf, ACPI_ATTR_LABEL_SHOW);
 }
+static struct device_attribute dev_attr_acpi_label = __ATTR(label, 0444,
+							    acpi_label_show,
+							    NULL);
 
-static ssize_t acpiindex_show(struct device *dev,
-			      struct device_attribute *attr, char *buf)
+static ssize_t acpi_index_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
 {
 	return dsm_get_label(dev, buf, ACPI_ATTR_INDEX_SHOW);
 }
+static DEVICE_ATTR_RO(acpi_index);
 
-static struct device_attribute acpi_attr_label = {
-	.attr = {.name = "label", .mode = 0444},
-	.show = acpilabel_show,
-};
-
-static struct device_attribute acpi_attr_index = {
-	.attr = {.name = "acpi_index", .mode = 0444},
-	.show = acpiindex_show,
-};
-
-static struct attribute *acpi_attributes[] = {
-	&acpi_attr_label.attr,
-	&acpi_attr_index.attr,
+static struct attribute *acpi_attrs[] = {
+	&dev_attr_acpi_label.attr,
+	&dev_attr_acpi_index.attr,
 	NULL,
 };
 
-static const struct attribute_group acpi_attr_group = {
-	.attrs = acpi_attributes,
-	.is_visible = acpi_index_string_exist,
-};
-
-static int pci_create_acpi_index_label_files(struct pci_dev *pdev)
+static umode_t acpi_attr_is_visible(struct kobject *kobj,
+				    struct attribute *a, int n)
 {
-	return sysfs_create_group(&pdev->dev.kobj, &acpi_attr_group);
-}
+	struct device *dev = kobj_to_dev(kobj);
 
-static int pci_remove_acpi_index_label_files(struct pci_dev *pdev)
-{
-	sysfs_remove_group(&pdev->dev.kobj, &acpi_attr_group);
-	return 0;
-}
-#else
-static inline int pci_create_acpi_index_label_files(struct pci_dev *pdev)
-{
-	return -1;
-}
+	if (!device_has_acpi_name(dev))
+		return 0;
 
-static inline int pci_remove_acpi_index_label_files(struct pci_dev *pdev)
-{
-	return -1;
+	return a->mode;
 }
 
-static inline bool device_has_dsm(struct device *dev)
-{
-	return false;
-}
+const struct attribute_group acpi_attr_group = {
+	.attrs = acpi_attrs,
+	.is_visible = acpi_attr_is_visible,
+};
 #endif
-
-void pci_create_firmware_label_files(struct pci_dev *pdev)
-{
-	if (device_has_dsm(&pdev->dev))
-		pci_create_acpi_index_label_files(pdev);
-	else
-		pci_create_smbiosname_file(pdev);
-}
-
-void pci_remove_firmware_label_files(struct pci_dev *pdev)
-{
-	if (device_has_dsm(&pdev->dev))
-		pci_remove_acpi_index_label_files(pdev);
-	else
-		pci_remove_smbiosname_file(pdev);
-}
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 501d120e268b..6a8a44d44127 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1380,18 +1380,10 @@ static const struct attribute_group pci_dev_reset_attr_group = {
 
 int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
 {
-	int retval;
-
 	if (!sysfs_initialized)
 		return -EACCES;
 
-	retval = pci_create_resource_files(pdev);
-	if (retval)
-		return retval;
-
-	pci_create_firmware_label_files(pdev);
-
-	return 0;
+	return pci_create_resource_files(pdev);
 }
 
 /**
@@ -1406,7 +1398,6 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
 		return;
 
 	pci_remove_resource_files(pdev);
-	pci_remove_firmware_label_files(pdev);
 }
 
 static int __init pci_sysfs_init(void)
@@ -1501,6 +1492,12 @@ const struct attribute_group *pci_dev_groups[] = {
 	&pci_dev_rom_attr_group,
 	&pci_dev_reset_attr_group,
 	&vpd_attr_group,
+#ifdef CONFIG_DMI
+	&smbios_attr_group,
+#endif
+#ifdef CONFIG_ACPI
+	&acpi_attr_group,
+#endif
 	NULL,
 };
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0dc52e60d03c..d6678124e8ea 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -21,16 +21,10 @@ bool pcie_cap_has_rtctl(const struct pci_dev *dev);
 
 int pci_create_sysfs_dev_files(struct pci_dev *pdev);
 void pci_remove_sysfs_dev_files(struct pci_dev *pdev);
-#if !defined(CONFIG_DMI) && !defined(CONFIG_ACPI)
-static inline void pci_create_firmware_label_files(struct pci_dev *pdev)
-{ return; }
-static inline void pci_remove_firmware_label_files(struct pci_dev *pdev)
-{ return; }
-#else
-void pci_create_firmware_label_files(struct pci_dev *pdev);
-void pci_remove_firmware_label_files(struct pci_dev *pdev);
-#endif
 void pci_cleanup_rom(struct pci_dev *dev);
+#ifdef CONFIG_DMI
+extern const struct attribute_group smbios_attr_group;
+#endif
 
 enum pci_mmap_api {
 	PCI_MMAP_SYSFS,	/* mmap on /sys/bus/pci/devices/<BDF>/resource<N> */
@@ -695,6 +689,7 @@ static inline int pci_aer_raw_clear_status(struct pci_dev *dev) { return -EINVAL
 
 #ifdef CONFIG_ACPI
 int pci_acpi_program_hp_params(struct pci_dev *dev);
+extern const struct attribute_group acpi_attr_group;
 #else
 static inline int pci_acpi_program_hp_params(struct pci_dev *dev)
 {
-- 
2.31.0


  parent reply	other threads:[~2021-04-16 20:59 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16 20:58 [PATCH 00/20] PCI: Convert dynamic sysfs objects into static Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 01/20] PCI: Convert dynamic "config" sysfs object " Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 02/20] PCI: Convert dynamic "rom" " Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 03/20] PCI: Convert dynamic "reset" " Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 04/20] PCI/VPD: Convert dynamic "vpd" " Krzysztof Wilczyński
2021-04-16 20:58 ` Krzysztof Wilczyński [this message]
2021-04-16 20:58 ` [PATCH 06/20] sysfs: Introduce BIN_ATTR_ADMIN_RO and BIN_ATTR_ADMIN_RW Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 07/20] PCI: Convert PCI sysfs objects to use BIN_ATTR_ADMIN_RW macro Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 08/20] PCI: Move to kstrtobool() to handle user input Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 09/20] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 10/20] PCI: Update style to be more consistent Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 11/20] PCI: Rearrange attributes from the pci_dev_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 12/20] PCI: Rearrange attributes from the pci_dev_config_attr_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 13/20] PCI: Rearrange attributes from the pci_dev_rom_attr_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 14/20] PCI: Rearrange attributes from the pci_dev_reset_attr_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 15/20] PCI: Rearrange attributes from the pci_dev_attr_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 16/20] PCI: Rearrange attributes from the pci_dev_hp_attr_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 17/20] PCI: Rearrange attributes from the pci_bridge_attr_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 18/20] PCI: Rearrange attributes from the pcie_dev_attr_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 19/20] PCI: Rearrange attributes from the pci_bus_group Krzysztof Wilczyński
2021-04-16 20:58 ` [PATCH 20/20] PCI: Rearrange attributes from the pcibus_group Krzysztof Wilczyński

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210416205856.3234481-6-kw@linux.com \
    --to=kw@linux.com \
    --cc=bhelgaas@google.com \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dsterba@suse.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=mchehab+huawei@kernel.org \
    --cc=oohall@gmail.com \
    --cc=pali@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).