[PATCH] ata: libata-transport: remove redundant dynamic sysfs attributes

[PATCH] ata: libata-transport: remove redundant dynamic sysfs attributes

[Heiner Kallweit]

Use the static sysfs attributes directly, this allows to significantly
simplify the code. See attribute_container_add_attrs() for why member
grp can be used instead of attrs.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/ata/libata-transport.c | 113 ++++++++++++---------------------
 1 file changed, 42 insertions(+), 71 deletions(-)

diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index 2099d94c4..7ad180265 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -37,34 +37,17 @@
 #include "libata.h"
 #include "libata-transport.h"
 
-#define ATA_PORT_ATTRS		3
-#define ATA_LINK_ATTRS		3
-#define ATA_DEV_ATTRS		9
-
 struct scsi_transport_template;
 struct scsi_transport_template *ata_scsi_transport_template;
 
 struct ata_internal {
 	struct scsi_transport_template t;
 
-	struct device_attribute private_port_attrs[ATA_PORT_ATTRS];
-	struct device_attribute private_link_attrs[ATA_LINK_ATTRS];
-	struct device_attribute private_dev_attrs[ATA_DEV_ATTRS];
-
 	struct transport_container link_attr_cont;
 	struct transport_container dev_attr_cont;
-
-	/*
-	 * The array of null terminated pointers to attributes
-	 * needed by scsi_sysfs.c
-	 */
-	struct device_attribute *link_attrs[ATA_LINK_ATTRS + 1];
-	struct device_attribute *port_attrs[ATA_PORT_ATTRS + 1];
-	struct device_attribute *dev_attrs[ATA_DEV_ATTRS + 1];
 };
 #define to_ata_internal(tmpl)	container_of(tmpl, struct ata_internal, t)
 
-
 #define tdev_to_device(d)					\
 	container_of((d), struct ata_device, tdev)
 #define transport_class_to_dev(dev)				\
@@ -80,13 +63,6 @@ struct ata_internal {
 #define transport_class_to_port(dev)				\
 	tdev_to_port((dev)->parent)
 
-/*
- * Hack to allow attributes of the same name in different objects.
- */
-#define ATA_DEVICE_ATTR(_prefix,_name,_mode,_show,_store) \
-	struct device_attribute device_attr_##_prefix##_##_name = \
-	__ATTR(_name,_mode,_show,_store)
-
 #define ata_bitfield_name_match(title, table)			\
 static ssize_t							\
 get_ata_##title##_names(u32 table_key, char *buf)		\
@@ -214,6 +190,17 @@ ata_port_simple_attr(stats.idle_irq, idle_irq, "%ld\n", unsigned long);
 /* We want the port_no sysfs attibute to start at 1 (ap->port_no starts at 0) */
 ata_port_simple_attr(port_no + 1, port_no, "%u\n", unsigned int);
 
+static const struct attribute *const ata_port_attr_attrs[] = {
+	&dev_attr_nr_pmp_links.attr,
+	&dev_attr_idle_irq.attr,
+	&dev_attr_port_no.attr,
+	NULL
+};
+
+static const struct attribute_group ata_port_attr_group = {
+	.attrs_const = ata_port_attr_attrs,
+};
+
 static DECLARE_TRANSPORT_CLASS(ata_port_class,
 			       "ata_port", NULL, NULL, NULL);
 
@@ -496,6 +483,23 @@ show_ata_dev_trim(struct device *dev,
 
 static DEVICE_ATTR(trim, S_IRUGO, show_ata_dev_trim, NULL);
 
+static const struct attribute *const ata_device_attr_attrs[] = {
+	&dev_attr_class.attr,
+	&dev_attr_pio_mode.attr,
+	&dev_attr_dma_mode.attr,
+	&dev_attr_xfer_mode.attr,
+	&dev_attr_spdn_cnt.attr,
+	&dev_attr_ering.attr,
+	&dev_attr_id.attr,
+	&dev_attr_gscr.attr,
+	&dev_attr_trim.attr,
+	NULL
+};
+
+static const struct attribute_group ata_device_attr_group = {
+	.attrs_const = ata_device_attr_attrs,
+};
+
 static DECLARE_TRANSPORT_CLASS(ata_dev_class,
 			       "ata_device", NULL, NULL, NULL);
 
@@ -626,6 +630,17 @@ ata_link_linkspeed_attr(hw_sata_spd_limit, fls);
 ata_link_linkspeed_attr(sata_spd_limit, fls);
 ata_link_linkspeed_attr(sata_spd, noop);
 
+static const struct attribute *const ata_link_attr_attrs[] = {
+	&dev_attr_hw_sata_spd_limit.attr,
+	&dev_attr_sata_spd_limit.attr,
+	&dev_attr_sata_spd.attr,
+	NULL
+};
+
+static const struct attribute_group ata_link_attr_group = {
+	.attrs_const = ata_link_attr_attrs,
+};
+
 static DECLARE_TRANSPORT_CLASS(ata_link_class,
 		"ata_link", NULL, NULL, NULL);
 
@@ -734,29 +749,12 @@ int ata_tlink_add(struct ata_link *link)
  * Setup / Teardown code
  */
 
-#define SETUP_TEMPLATE(attrb, field, perm, test)			\
-	i->private_##attrb[count] = dev_attr_##field;			\
-	i->private_##attrb[count].attr.mode = perm;			\
-	i->attrb[count] = &i->private_##attrb[count];			\
-	if (test)							\
-		count++
-
-#define SETUP_LINK_ATTRIBUTE(field)					\
-	SETUP_TEMPLATE(link_attrs, field, S_IRUGO, 1)
-
-#define SETUP_PORT_ATTRIBUTE(field)					\
-	SETUP_TEMPLATE(port_attrs, field, S_IRUGO, 1)
-
-#define SETUP_DEV_ATTRIBUTE(field)					\
-	SETUP_TEMPLATE(dev_attrs, field, S_IRUGO, 1)
-
 /**
  * ata_attach_transport  --  instantiate ATA transport template
  */
 struct scsi_transport_template *ata_attach_transport(void)
 {
 	struct ata_internal *i;
-	int count;
 
 	i = kzalloc_obj(struct ata_internal);
 	if (!i)
@@ -765,48 +763,21 @@ struct scsi_transport_template *ata_attach_transport(void)
 	i->t.eh_strategy_handler	= ata_scsi_error;
 	i->t.user_scan			= ata_scsi_user_scan;
 
-	i->t.host_attrs.ac.attrs = &i->port_attrs[0];
 	i->t.host_attrs.ac.class = &ata_port_class.class;
+	i->t.host_attrs.ac.grp   = &ata_port_attr_group;
 	i->t.host_attrs.ac.match = ata_tport_match;
 	transport_container_register(&i->t.host_attrs);
 
 	i->link_attr_cont.ac.class = &ata_link_class.class;
-	i->link_attr_cont.ac.attrs = &i->link_attrs[0];
+	i->link_attr_cont.ac.grp   = &ata_link_attr_group;
 	i->link_attr_cont.ac.match = ata_tlink_match;
 	transport_container_register(&i->link_attr_cont);
 
 	i->dev_attr_cont.ac.class = &ata_dev_class.class;
-	i->dev_attr_cont.ac.attrs = &i->dev_attrs[0];
+	i->dev_attr_cont.ac.grp   = &ata_device_attr_group;
 	i->dev_attr_cont.ac.match = ata_tdev_match;
 	transport_container_register(&i->dev_attr_cont);
 
-	count = 0;
-	SETUP_PORT_ATTRIBUTE(nr_pmp_links);
-	SETUP_PORT_ATTRIBUTE(idle_irq);
-	SETUP_PORT_ATTRIBUTE(port_no);
-	BUG_ON(count > ATA_PORT_ATTRS);
-	i->port_attrs[count] = NULL;
-
-	count = 0;
-	SETUP_LINK_ATTRIBUTE(hw_sata_spd_limit);
-	SETUP_LINK_ATTRIBUTE(sata_spd_limit);
-	SETUP_LINK_ATTRIBUTE(sata_spd);
-	BUG_ON(count > ATA_LINK_ATTRS);
-	i->link_attrs[count] = NULL;
-
-	count = 0;
-	SETUP_DEV_ATTRIBUTE(class);
-	SETUP_DEV_ATTRIBUTE(pio_mode);
-	SETUP_DEV_ATTRIBUTE(dma_mode);
-	SETUP_DEV_ATTRIBUTE(xfer_mode);
-	SETUP_DEV_ATTRIBUTE(spdn_cnt);
-	SETUP_DEV_ATTRIBUTE(ering);
-	SETUP_DEV_ATTRIBUTE(id);
-	SETUP_DEV_ATTRIBUTE(gscr);
-	SETUP_DEV_ATTRIBUTE(trim);
-	BUG_ON(count > ATA_DEV_ATTRS);
-	i->dev_attrs[count] = NULL;
-
 	return &i->t;
 }
 
-- 
2.53.0

Re: [PATCH] ata: libata-transport: remove redundant dynamic sysfs attributes

[Niklas Cassel]

On Fri, Mar 20, 2026 at 11:10:59PM +0100, Heiner Kallweit wrote:
> Use the static sysfs attributes directly, this allows to significantly
> simplify the code. See attribute_container_add_attrs() for why member
> grp can be used instead of attrs.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---

The sysfs attributes are still accessible from the file paths as defined in:
Documentation/ABI/testing/sysfs-ata

The sysfs attributes still have S_IRUGO file permissions.

Thus:
Tested-by: Niklas Cassel <cassel@kernel.org>

Re: [PATCH] ata: libata-transport: remove redundant dynamic sysfs attributes

[Niklas Cassel]

On Fri, 20 Mar 2026 23:10:59 +0100, Heiner Kallweit wrote:
> Use the static sysfs attributes directly, this allows to significantly
> simplify the code. See attribute_container_add_attrs() for why member
> grp can be used instead of attrs.
> 
> 

Applied to libata/linux.git (for-7.1), thanks!

[1/1] ata: libata-transport: remove redundant dynamic sysfs attributes
      https://git.kernel.org/libata/linux/c/78ec0699

Kind regards,
Niklas