[PATCH 0/2] PCI: Manage hotplug attributes through group and make slot attributes const

[PATCH 0/2] PCI: Manage hotplug attributes through group and make slot attributes const

[Thomas Weißschuh]

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (2):
      PCI: hotplug: Manage hotplug slot attributes as group
      PCI: Constify slot attributes

 drivers/pci/hotplug/pci_hotplug_core.c | 125 +++++++++++++--------------------
 drivers/pci/pci.h                      |   2 +-
 drivers/pci/slot.c                     |  14 ++--
 3 files changed, 57 insertions(+), 84 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260513-sysfs-const-pci-hotplug-48b62047984e

Best regards,
--  
Thomas Weißschuh <linux@weissschuh.net>

[PATCH 1/2] PCI: hotplug: Manage hotplug slot attributes as group

[Thomas Weißschuh]

Make use of 'struct attribute_group' to manage all attribute together,
simplifying the lifecycle management.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 114 +++++++++++++--------------------
 1 file changed, 43 insertions(+), 71 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index fadcf98a8a66..7c4e197f5581 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -239,10 +239,46 @@ static bool has_test_file(struct hotplug_slot *slot)
 	return false;
 }
 
+static struct attribute *hotplug_slot_attrs[] = {
+	&hotplug_slot_attr_power.attr,
+	&hotplug_slot_attr_attention.attr,
+	&hotplug_slot_attr_latch.attr,
+	&hotplug_slot_attr_presence.attr,
+	&hotplug_slot_attr_test.attr,
+	NULL
+};
+
+static umode_t hotplug_slot_attr_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
+{
+	struct hotplug_slot *slot = to_pci_slot(kobj)->hotplug;
+
+	if (attr == &hotplug_slot_attr_power.attr && !has_power_file(slot))
+		return 0;
+
+	if (attr == &hotplug_slot_attr_attention.attr && !has_attention_file(slot))
+		return 0;
+
+	if (attr == &hotplug_slot_attr_latch.attr && !has_latch_file(slot))
+		return 0;
+
+	if (attr == &hotplug_slot_attr_presence.attr && !has_adapter_file(slot))
+		return 0;
+
+	if (attr == &hotplug_slot_attr_test.attr && !has_test_file(slot))
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group hotplug_slot_group = {
+	.is_visible = hotplug_slot_attr_is_visible,
+	.attrs = hotplug_slot_attrs,
+};
+
 static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
 {
 	struct kobject *kobj;
-	int retval = 0;
+	int retval;
 
 	/* Create symbolic link to the hotplug driver module */
 	kobj = kset_find_obj(module_kset, slot->mod_name);
@@ -254,82 +290,18 @@ static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
 		kobject_put(kobj);
 	}
 
-	if (has_power_file(slot)) {
-		retval = sysfs_create_file(&pci_slot->kobj,
-					   &hotplug_slot_attr_power.attr);
-		if (retval)
-			goto exit_power;
-	}
-
-	if (has_attention_file(slot)) {
-		retval = sysfs_create_file(&pci_slot->kobj,
-					   &hotplug_slot_attr_attention.attr);
-		if (retval)
-			goto exit_attention;
-	}
-
-	if (has_latch_file(slot)) {
-		retval = sysfs_create_file(&pci_slot->kobj,
-					   &hotplug_slot_attr_latch.attr);
-		if (retval)
-			goto exit_latch;
-	}
-
-	if (has_adapter_file(slot)) {
-		retval = sysfs_create_file(&pci_slot->kobj,
-					   &hotplug_slot_attr_presence.attr);
-		if (retval)
-			goto exit_adapter;
-	}
-
-	if (has_test_file(slot)) {
-		retval = sysfs_create_file(&pci_slot->kobj,
-					   &hotplug_slot_attr_test.attr);
-		if (retval)
-			goto exit_test;
+	retval = sysfs_create_group(&pci_slot->kobj, &hotplug_slot_group);
+	if (retval) {
+		sysfs_remove_link(&pci_slot->kobj, "module");
+		return retval;
 	}
 
-	goto exit;
-
-exit_test:
-	if (has_adapter_file(slot))
-		sysfs_remove_file(&pci_slot->kobj,
-				  &hotplug_slot_attr_presence.attr);
-exit_adapter:
-	if (has_latch_file(slot))
-		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_latch.attr);
-exit_latch:
-	if (has_attention_file(slot))
-		sysfs_remove_file(&pci_slot->kobj,
-				  &hotplug_slot_attr_attention.attr);
-exit_attention:
-	if (has_power_file(slot))
-		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
-exit_power:
-	sysfs_remove_link(&pci_slot->kobj, "module");
-exit:
-	return retval;
+	return 0;
 }
 
 static void fs_remove_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
 {
-	if (has_power_file(slot))
-		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
-
-	if (has_attention_file(slot))
-		sysfs_remove_file(&pci_slot->kobj,
-				  &hotplug_slot_attr_attention.attr);
-
-	if (has_latch_file(slot))
-		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_latch.attr);
-
-	if (has_adapter_file(slot))
-		sysfs_remove_file(&pci_slot->kobj,
-				  &hotplug_slot_attr_presence.attr);
-
-	if (has_test_file(slot))
-		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_test.attr);
-
+	sysfs_remove_group(&pci_slot->kobj, &hotplug_slot_group);
 	sysfs_remove_link(&pci_slot->kobj, "module");
 }
 

-- 
2.54.0

[PATCH 2/2] PCI: Constify slot attributes

[Thomas Weißschuh]

The attribute struct are never modified, mark them as const.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 19 ++++++++++---------
 drivers/pci/pci.h                      |  2 +-
 drivers/pci/slot.c                     | 14 +++++++-------
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 7c4e197f5581..cdf3293aade7 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -97,7 +97,7 @@ static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
 	return count;
 }
 
-static struct pci_slot_attribute hotplug_slot_attr_power = {
+static const struct pci_slot_attribute hotplug_slot_attr_power = {
 	.attr = {.name = "power", .mode = S_IFREG | S_IRUGO | S_IWUSR},
 	.show = power_read_file,
 	.store = power_write_file
@@ -136,7 +136,7 @@ static ssize_t attention_write_file(struct pci_slot *pci_slot, const char *buf,
 	return count;
 }
 
-static struct pci_slot_attribute hotplug_slot_attr_attention = {
+static const struct pci_slot_attribute hotplug_slot_attr_attention = {
 	.attr = {.name = "attention", .mode = S_IFREG | S_IRUGO | S_IWUSR},
 	.show = attention_read_file,
 	.store = attention_write_file
@@ -154,7 +154,7 @@ static ssize_t latch_read_file(struct pci_slot *pci_slot, char *buf)
 	return sysfs_emit(buf, "%d\n", value);
 }
 
-static struct pci_slot_attribute hotplug_slot_attr_latch = {
+static const struct pci_slot_attribute hotplug_slot_attr_latch = {
 	.attr = {.name = "latch", .mode = S_IFREG | S_IRUGO},
 	.show = latch_read_file,
 };
@@ -171,7 +171,7 @@ static ssize_t presence_read_file(struct pci_slot *pci_slot, char *buf)
 	return sysfs_emit(buf, "%d\n", value);
 }
 
-static struct pci_slot_attribute hotplug_slot_attr_presence = {
+static const struct pci_slot_attribute hotplug_slot_attr_presence = {
 	.attr = {.name = "adapter", .mode = S_IFREG | S_IRUGO},
 	.show = presence_read_file,
 };
@@ -196,7 +196,7 @@ static ssize_t test_write_file(struct pci_slot *pci_slot, const char *buf,
 	return count;
 }
 
-static struct pci_slot_attribute hotplug_slot_attr_test = {
+static const struct pci_slot_attribute hotplug_slot_attr_test = {
 	.attr = {.name = "test", .mode = S_IFREG | S_IRUGO | S_IWUSR},
 	.store = test_write_file
 };
@@ -239,7 +239,7 @@ static bool has_test_file(struct hotplug_slot *slot)
 	return false;
 }
 
-static struct attribute *hotplug_slot_attrs[] = {
+static const struct attribute *const hotplug_slot_attrs[] = {
 	&hotplug_slot_attr_power.attr,
 	&hotplug_slot_attr_attention.attr,
 	&hotplug_slot_attr_latch.attr,
@@ -248,7 +248,8 @@ static struct attribute *hotplug_slot_attrs[] = {
 	NULL
 };
 
-static umode_t hotplug_slot_attr_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
+static umode_t hotplug_slot_attr_is_visible(struct kobject *kobj, const struct attribute *attr,
+					    int idx)
 {
 	struct hotplug_slot *slot = to_pci_slot(kobj)->hotplug;
 
@@ -271,8 +272,8 @@ static umode_t hotplug_slot_attr_is_visible(struct kobject *kobj, struct attribu
 }
 
 static const struct attribute_group hotplug_slot_group = {
-	.is_visible = hotplug_slot_attr_is_visible,
-	.attrs = hotplug_slot_attrs,
+	.is_visible_const = hotplug_slot_attr_is_visible,
+	.attrs_const = hotplug_slot_attrs,
 };
 
 static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4a14f88e543a..3ca3832c7267 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -478,7 +478,7 @@ struct pci_slot_attribute {
 	ssize_t (*show)(struct pci_slot *, char *);
 	ssize_t (*store)(struct pci_slot *, const char *, size_t);
 };
-#define to_pci_slot_attr(s) container_of(s, struct pci_slot_attribute, attr)
+#define to_pci_slot_attr(s) container_of_const(s, struct pci_slot_attribute, attr)
 
 enum pci_bar_type {
 	pci_bar_unknown,	/* Standard PCI BAR probe */
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index 6d5cd37bfb1e..7929bb87b195 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -18,7 +18,7 @@ static ssize_t pci_slot_attr_show(struct kobject *kobj,
 					struct attribute *attr, char *buf)
 {
 	struct pci_slot *slot = to_pci_slot(kobj);
-	struct pci_slot_attribute *attribute = to_pci_slot_attr(attr);
+	const struct pci_slot_attribute *attribute = to_pci_slot_attr(attr);
 	return attribute->show ? attribute->show(slot, buf) : -EIO;
 }
 
@@ -26,7 +26,7 @@ static ssize_t pci_slot_attr_store(struct kobject *kobj,
 			struct attribute *attr, const char *buf, size_t len)
 {
 	struct pci_slot *slot = to_pci_slot(kobj);
-	struct pci_slot_attribute *attribute = to_pci_slot_attr(attr);
+	const struct pci_slot_attribute *attribute = to_pci_slot_attr(attr);
 	return attribute->store ? attribute->store(slot, buf, len) : -EIO;
 }
 
@@ -93,14 +93,14 @@ static void pci_slot_release(struct kobject *kobj)
 	kfree(slot);
 }
 
-static struct pci_slot_attribute pci_slot_attr_address =
+static const struct pci_slot_attribute pci_slot_attr_address =
 	__ATTR(address, S_IRUGO, address_read_file, NULL);
-static struct pci_slot_attribute pci_slot_attr_max_speed =
+static const struct pci_slot_attribute pci_slot_attr_max_speed =
 	__ATTR(max_bus_speed, S_IRUGO, max_speed_read_file, NULL);
-static struct pci_slot_attribute pci_slot_attr_cur_speed =
+static const struct pci_slot_attribute pci_slot_attr_cur_speed =
 	__ATTR(cur_bus_speed, S_IRUGO, cur_speed_read_file, NULL);
 
-static struct attribute *pci_slot_default_attrs[] = {
+static const struct attribute *const pci_slot_default_attrs[] = {
 	&pci_slot_attr_address.attr,
 	&pci_slot_attr_max_speed.attr,
 	&pci_slot_attr_cur_speed.attr,
@@ -108,7 +108,7 @@ static struct attribute *pci_slot_default_attrs[] = {
 };
 
 static const struct attribute_group pci_slot_default_group = {
-	.attrs = pci_slot_default_attrs,
+	.attrs_const = pci_slot_default_attrs,
 };
 
 static const struct attribute_group *pci_slot_default_groups[] = {

-- 
2.54.0

Re: [PATCH 0/2] PCI: Manage hotplug attributes through group and make slot attributes const

[Krzysztof Wilczyński]

Hello,

> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

No cover letter?

Thank you!

	Krzysztof

Re: [PATCH 2/2] PCI: Constify slot attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incomplete constification: missed `zpci_slot_attr_uid` in `arch/s390/pci/pci_sysfs.c`.
--

commit 81d437e9bb503bcf5cbb6fdfb3f55cfe7eaf8cde
Author: Thomas Weißschuh <linux@weissschuh.net>

PCI: Constify slot attributes
    
The attribute struct are never modified, mark them as const.

> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 6d5cd37bfb1e7..7929bb87b1959 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c

[ ... ]

> @@ -93,14 +93,14 @@ static void pci_slot_release(struct kobject *kobj)
>  	kfree(slot);
>  }
>  
> -static struct pci_slot_attribute pci_slot_attr_address =
> +static const struct pci_slot_attribute pci_slot_attr_address =
>  	__ATTR(address, S_IRUGO, address_read_file, NULL);
> -static struct pci_slot_attribute pci_slot_attr_max_speed =
> +static const struct pci_slot_attribute pci_slot_attr_max_speed =
>  	__ATTR(max_bus_speed, S_IRUGO, max_speed_read_file, NULL);
> -static struct pci_slot_attribute pci_slot_attr_cur_speed =
> +static const struct pci_slot_attribute pci_slot_attr_cur_speed =
>  	__ATTR(cur_bus_speed, S_IRUGO, cur_speed_read_file, NULL);

Since this constifies the core PCI slot attributes, should the s390
architecture-specific attributes also be marked as const?

Looking at arch/s390/pci/pci_sysfs.c, it defines a struct pci_slot_attribute
and an attribute array that appear to be missed in this patch:

arch/s390/pci/pci_sysfs.c:
static struct pci_slot_attribute zpci_slot_attr_uid =
	__ATTR(uid, 0444, zpci_uid_slot_show, NULL);

static struct attribute *zpci_slot_attrs[] = {
	&zpci_slot_attr_uid.attr,
	NULL,
};

Could these be updated to const as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514-sysfs-const-pci-hotplug-v1-0-3b0b6c6f7481@weissschuh.net?part=2