[PATCH v2 0/5] ata: libata-transport: series with further refactorings

[PATCH v2 0/5] ata: libata-transport: series with further refactorings

[Heiner Kallweit]

This series aims at further improving and simplifying the code.

v2:
- improve commit messages for patches 1 and 2

Heiner Kallweit (5):
  ata: libata-transport: instantiate struct ata_internal statically
  ata: libata-transport: inline ata_attach|release_transport
  ata: libata-transport: use static struct ata_transport_internal to
    simplify match functions
  ata: libata-transport: split struct ata_internal
  ata: libata-transport: remove static variable
    ata_scsi_transport_template

 drivers/ata/libata-core.c      |  11 +---
 drivers/ata/libata-scsi.c      |   2 +-
 drivers/ata/libata-transport.c | 117 +++++++++++++--------------------
 drivers/ata/libata-transport.h |   5 +-
 4 files changed, 49 insertions(+), 86 deletions(-)

-- 
2.53.0

[PATCH v2 1/5] ata: libata-transport: instantiate struct ata_internal statically

[Heiner Kallweit]

Struct ata_internal is only instantiated once, in module init code.
So we can also instantiate it statically, which allows simplifying
the code.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/ata/libata-core.c      | 11 ++-----
 drivers/ata/libata-transport.c | 57 +++++++++++++++-------------------
 drivers/ata/libata-transport.h |  2 +-
 3 files changed, 28 insertions(+), 42 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 81479ddb8..ae56567af 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6779,22 +6779,15 @@ static int __init ata_init(void)
 
 	libata_transport_init();
 	ata_scsi_transport_template = ata_attach_transport();
-	if (!ata_scsi_transport_template) {
-		ata_sff_exit();
-		rc = -ENOMEM;
-		goto err_out;
-	}
 
 	printk(KERN_DEBUG "libata version " DRV_VERSION " loaded.\n");
-	return 0;
 
-err_out:
-	return rc;
+	return 0;
 }
 
 static void __exit ata_exit(void)
 {
-	ata_release_transport(ata_scsi_transport_template);
+	ata_release_transport();
 	libata_transport_exit();
 	ata_sff_exit();
 	ata_free_force_param();
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index 7ad180265..0c7ce278e 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -745,6 +745,23 @@ int ata_tlink_add(struct ata_link *link)
 	return error;
 }
 
+static struct ata_internal ata_transport_internal = {
+	.t.eh_strategy_handler	= ata_scsi_error,
+	.t.user_scan		= ata_scsi_user_scan,
+
+	.t.host_attrs.ac.class	= &ata_port_class.class,
+	.t.host_attrs.ac.grp	= &ata_port_attr_group,
+	.t.host_attrs.ac.match	= ata_tport_match,
+
+	.link_attr_cont.ac.class = &ata_link_class.class,
+	.link_attr_cont.ac.grp   = &ata_link_attr_group,
+	.link_attr_cont.ac.match = ata_tlink_match,
+
+	.dev_attr_cont.ac.class	= &ata_dev_class.class,
+	.dev_attr_cont.ac.grp	= &ata_device_attr_group,
+	.dev_attr_cont.ac.match	= ata_tdev_match,
+};
+
 /*
  * Setup / Teardown code
  */
@@ -754,46 +771,22 @@ int ata_tlink_add(struct ata_link *link)
  */
 struct scsi_transport_template *ata_attach_transport(void)
 {
-	struct ata_internal *i;
-
-	i = kzalloc_obj(struct ata_internal);
-	if (!i)
-		return NULL;
+	transport_container_register(&ata_transport_internal.t.host_attrs);
+	transport_container_register(&ata_transport_internal.link_attr_cont);
+	transport_container_register(&ata_transport_internal.dev_attr_cont);
 
-	i->t.eh_strategy_handler	= ata_scsi_error;
-	i->t.user_scan			= ata_scsi_user_scan;
-
-	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.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.grp   = &ata_device_attr_group;
-	i->dev_attr_cont.ac.match = ata_tdev_match;
-	transport_container_register(&i->dev_attr_cont);
-
-	return &i->t;
+	return &ata_transport_internal.t;
 }
 
 /**
  * ata_release_transport  --  release ATA transport template instance
  * @t:		transport template instance
  */
-void ata_release_transport(struct scsi_transport_template *t)
+void ata_release_transport(void)
 {
-	struct ata_internal *i = to_ata_internal(t);
-
-	transport_container_unregister(&i->t.host_attrs);
-	transport_container_unregister(&i->link_attr_cont);
-	transport_container_unregister(&i->dev_attr_cont);
-
-	kfree(i);
+	transport_container_unregister(&ata_transport_internal.t.host_attrs);
+	transport_container_unregister(&ata_transport_internal.link_attr_cont);
+	transport_container_unregister(&ata_transport_internal.dev_attr_cont);
 }
 
 __init int libata_transport_init(void)
diff --git a/drivers/ata/libata-transport.h b/drivers/ata/libata-transport.h
index 50cd2cbe8..a464b8fcd 100644
--- a/drivers/ata/libata-transport.h
+++ b/drivers/ata/libata-transport.h
@@ -9,7 +9,7 @@ int ata_tlink_add(struct ata_link *link);
 void ata_tlink_delete(struct ata_link *link);
 
 struct scsi_transport_template *ata_attach_transport(void);
-void ata_release_transport(struct scsi_transport_template *t);
+void ata_release_transport(void);
 
 __init int libata_transport_init(void);
 void __exit libata_transport_exit(void);
-- 
2.53.0

[PATCH v2 2/5] ata: libata-transport: inline ata_attach|release_transport

[Heiner Kallweit]

Both functions are helpers which are used only once. So remove them and
merge their code into libata_transport_init() and libata_transport_exit()
respectively. 

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/ata/libata-core.c      |  2 --
 drivers/ata/libata-transport.c | 34 +++++++++++-----------------------
 drivers/ata/libata-transport.h |  3 ---
 3 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ae56567af..e76d15411 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6778,7 +6778,6 @@ static int __init ata_init(void)
 	}
 
 	libata_transport_init();
-	ata_scsi_transport_template = ata_attach_transport();
 
 	printk(KERN_DEBUG "libata version " DRV_VERSION " loaded.\n");
 
@@ -6787,7 +6786,6 @@ static int __init ata_init(void)
 
 static void __exit ata_exit(void)
 {
-	ata_release_transport();
 	libata_transport_exit();
 	ata_sff_exit();
 	ata_free_force_param();
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index 0c7ce278e..9d594562c 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -766,29 +766,6 @@ static struct ata_internal ata_transport_internal = {
  * Setup / Teardown code
  */
 
-/**
- * ata_attach_transport  --  instantiate ATA transport template
- */
-struct scsi_transport_template *ata_attach_transport(void)
-{
-	transport_container_register(&ata_transport_internal.t.host_attrs);
-	transport_container_register(&ata_transport_internal.link_attr_cont);
-	transport_container_register(&ata_transport_internal.dev_attr_cont);
-
-	return &ata_transport_internal.t;
-}
-
-/**
- * ata_release_transport  --  release ATA transport template instance
- * @t:		transport template instance
- */
-void ata_release_transport(void)
-{
-	transport_container_unregister(&ata_transport_internal.t.host_attrs);
-	transport_container_unregister(&ata_transport_internal.link_attr_cont);
-	transport_container_unregister(&ata_transport_internal.dev_attr_cont);
-}
-
 __init int libata_transport_init(void)
 {
 	int error;
@@ -802,6 +779,13 @@ __init int libata_transport_init(void)
 	error = transport_class_register(&ata_dev_class);
 	if (error)
 		goto out_unregister_port;
+
+	transport_container_register(&ata_transport_internal.t.host_attrs);
+	transport_container_register(&ata_transport_internal.link_attr_cont);
+	transport_container_register(&ata_transport_internal.dev_attr_cont);
+
+	ata_scsi_transport_template = &ata_transport_internal.t;
+
 	return 0;
 
  out_unregister_port:
@@ -815,6 +799,10 @@ __init int libata_transport_init(void)
 
 void __exit libata_transport_exit(void)
 {
+	transport_container_unregister(&ata_transport_internal.t.host_attrs);
+	transport_container_unregister(&ata_transport_internal.link_attr_cont);
+	transport_container_unregister(&ata_transport_internal.dev_attr_cont);
+
 	transport_class_unregister(&ata_link_class);
 	transport_class_unregister(&ata_port_class);
 	transport_class_unregister(&ata_dev_class);
diff --git a/drivers/ata/libata-transport.h b/drivers/ata/libata-transport.h
index a464b8fcd..fe5ca66fc 100644
--- a/drivers/ata/libata-transport.h
+++ b/drivers/ata/libata-transport.h
@@ -8,9 +8,6 @@ extern struct scsi_transport_template *ata_scsi_transport_template;
 int ata_tlink_add(struct ata_link *link);
 void ata_tlink_delete(struct ata_link *link);
 
-struct scsi_transport_template *ata_attach_transport(void);
-void ata_release_transport(void);
-
 __init int libata_transport_init(void);
 void __exit libata_transport_exit(void);
 #endif
-- 
2.53.0

[PATCH v2 3/5] ata: libata-transport: use static struct ata_transport_internal to simplify match functions

[Heiner Kallweit]

Both matching functions can make use of static struct
ata_transport_internal. This eliminates the dependency on static
variable ata_scsi_transport_template, and it allows to remove helper
to_ata_internal(). Small drawback is that a forward declaration of
both functions is needed.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/ata/libata-transport.c | 44 ++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index 9d594562c..68ae57a89 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -46,7 +46,11 @@ struct ata_internal {
 	struct transport_container link_attr_cont;
 	struct transport_container dev_attr_cont;
 };
-#define to_ata_internal(tmpl)	container_of(tmpl, struct ata_internal, t)
+
+static int ata_tlink_match(struct attribute_container *cont,
+			   struct device *dev);
+static int ata_tdev_match(struct attribute_container *cont,
+			  struct device *dev);
 
 #define tdev_to_device(d)					\
 	container_of((d), struct ata_device, tdev)
@@ -519,16 +523,6 @@ static bool ata_is_ata_dev(const struct device *dev)
 	return dev->release == ata_tdev_release;
 }
 
-static int ata_tdev_match(struct attribute_container *cont,
-			  struct device *dev)
-{
-	struct ata_internal *i = to_ata_internal(ata_scsi_transport_template);
-
-	if (!ata_is_ata_dev(dev))
-		return 0;
-	return &i->dev_attr_cont.ac == cont;
-}
-
 /**
  * ata_tdev_free  --  free an ATA transport device
  * @dev:	struct ata_device owning the transport device to free
@@ -660,16 +654,6 @@ static bool ata_is_link(const struct device *dev)
 	return dev->release == ata_tlink_release;
 }
 
-static int ata_tlink_match(struct attribute_container *cont,
-			    struct device *dev)
-{
-	struct ata_internal *i = to_ata_internal(ata_scsi_transport_template);
-
-	if (!ata_is_link(dev))
-		return 0;
-	return &i->link_attr_cont.ac == cont;
-}
-
 /**
  * ata_tlink_delete  --  remove an ATA link transport device
  * @link:	struct ata_link owning the link transport device to remove
@@ -762,6 +746,24 @@ static struct ata_internal ata_transport_internal = {
 	.dev_attr_cont.ac.match	= ata_tdev_match,
 };
 
+static int ata_tlink_match(struct attribute_container *cont,
+			    struct device *dev)
+{
+	if (!ata_is_link(dev))
+		return 0;
+
+	return &ata_transport_internal.link_attr_cont.ac == cont;
+}
+
+static int ata_tdev_match(struct attribute_container *cont,
+			  struct device *dev)
+{
+	if (!ata_is_ata_dev(dev))
+		return 0;
+
+	return &ata_transport_internal.dev_attr_cont.ac == cont;
+}
+
 /*
  * Setup / Teardown code
  */
-- 
2.53.0

[PATCH v2 4/5] ata: libata-transport: split struct ata_internal

[Heiner Kallweit]

There's no need for an umbrella struct, so remove it. It's also a
prerequisite for making the embedded struct scsi_transport_template
public.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/ata/libata-transport.c | 53 ++++++++++++++++------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index 68ae57a89..538743e88 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -40,13 +40,6 @@
 struct scsi_transport_template;
 struct scsi_transport_template *ata_scsi_transport_template;
 
-struct ata_internal {
-	struct scsi_transport_template t;
-
-	struct transport_container link_attr_cont;
-	struct transport_container dev_attr_cont;
-};
-
 static int ata_tlink_match(struct attribute_container *cont,
 			   struct device *dev);
 static int ata_tdev_match(struct attribute_container *cont,
@@ -729,21 +722,25 @@ int ata_tlink_add(struct ata_link *link)
 	return error;
 }
 
-static struct ata_internal ata_transport_internal = {
-	.t.eh_strategy_handler	= ata_scsi_error,
-	.t.user_scan		= ata_scsi_user_scan,
+static struct scsi_transport_template ata_scsi_transportt = {
+	.eh_strategy_handler	= ata_scsi_error,
+	.user_scan		= ata_scsi_user_scan,
 
-	.t.host_attrs.ac.class	= &ata_port_class.class,
-	.t.host_attrs.ac.grp	= &ata_port_attr_group,
-	.t.host_attrs.ac.match	= ata_tport_match,
+	.host_attrs.ac.class	= &ata_port_class.class,
+	.host_attrs.ac.grp	= &ata_port_attr_group,
+	.host_attrs.ac.match	= ata_tport_match,
+};
 
-	.link_attr_cont.ac.class = &ata_link_class.class,
-	.link_attr_cont.ac.grp   = &ata_link_attr_group,
-	.link_attr_cont.ac.match = ata_tlink_match,
+static struct transport_container ata_link_attr_cont = {
+	.ac.class	= &ata_link_class.class,
+	.ac.grp		= &ata_link_attr_group,
+	.ac.match	= ata_tlink_match,
+};
 
-	.dev_attr_cont.ac.class	= &ata_dev_class.class,
-	.dev_attr_cont.ac.grp	= &ata_device_attr_group,
-	.dev_attr_cont.ac.match	= ata_tdev_match,
+static struct transport_container ata_dev_attr_cont = {
+	.ac.class	= &ata_dev_class.class,
+	.ac.grp		= &ata_device_attr_group,
+	.ac.match	= ata_tdev_match,
 };
 
 static int ata_tlink_match(struct attribute_container *cont,
@@ -752,7 +749,7 @@ static int ata_tlink_match(struct attribute_container *cont,
 	if (!ata_is_link(dev))
 		return 0;
 
-	return &ata_transport_internal.link_attr_cont.ac == cont;
+	return &ata_link_attr_cont.ac == cont;
 }
 
 static int ata_tdev_match(struct attribute_container *cont,
@@ -761,7 +758,7 @@ static int ata_tdev_match(struct attribute_container *cont,
 	if (!ata_is_ata_dev(dev))
 		return 0;
 
-	return &ata_transport_internal.dev_attr_cont.ac == cont;
+	return &ata_dev_attr_cont.ac == cont;
 }
 
 /*
@@ -782,11 +779,11 @@ __init int libata_transport_init(void)
 	if (error)
 		goto out_unregister_port;
 
-	transport_container_register(&ata_transport_internal.t.host_attrs);
-	transport_container_register(&ata_transport_internal.link_attr_cont);
-	transport_container_register(&ata_transport_internal.dev_attr_cont);
+	transport_container_register(&ata_scsi_transportt.host_attrs);
+	transport_container_register(&ata_link_attr_cont);
+	transport_container_register(&ata_dev_attr_cont);
 
-	ata_scsi_transport_template = &ata_transport_internal.t;
+	ata_scsi_transport_template = &ata_scsi_transportt;
 
 	return 0;
 
@@ -801,9 +798,9 @@ __init int libata_transport_init(void)
 
 void __exit libata_transport_exit(void)
 {
-	transport_container_unregister(&ata_transport_internal.t.host_attrs);
-	transport_container_unregister(&ata_transport_internal.link_attr_cont);
-	transport_container_unregister(&ata_transport_internal.dev_attr_cont);
+	transport_container_unregister(&ata_scsi_transportt.host_attrs);
+	transport_container_unregister(&ata_link_attr_cont);
+	transport_container_unregister(&ata_dev_attr_cont);
 
 	transport_class_unregister(&ata_link_class);
 	transport_class_unregister(&ata_port_class);
-- 
2.53.0

[PATCH v2 5/5] ata: libata-transport: remove static variable ata_scsi_transport_template

[Heiner Kallweit]

Simplify the code by making struct ata_scsi_transportt public, instead
of using separate variable ata_scsi_transport_template.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/ata/libata-scsi.c      | 2 +-
 drivers/ata/libata-transport.c | 9 ++-------
 drivers/ata/libata-transport.h | 2 +-
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 06f3a243f..b15830bed 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4624,7 +4624,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
 		*(struct ata_port **)&shost->hostdata[0] = ap;
 		ap->scsi_host = shost;
 
-		shost->transportt = ata_scsi_transport_template;
+		shost->transportt = &ata_scsi_transportt;
 		shost->unique_id = ap->print_id;
 		shost->max_id = 16;
 		shost->max_lun = 1;
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index 538743e88..adf38e4bb 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -37,9 +37,6 @@
 #include "libata.h"
 #include "libata-transport.h"
 
-struct scsi_transport_template;
-struct scsi_transport_template *ata_scsi_transport_template;
-
 static int ata_tlink_match(struct attribute_container *cont,
 			   struct device *dev);
 static int ata_tdev_match(struct attribute_container *cont,
@@ -224,7 +221,7 @@ static int ata_tport_match(struct attribute_container *cont,
 {
 	if (!ata_is_port(dev))
 		return 0;
-	return &ata_scsi_transport_template->host_attrs.ac == cont;
+	return &ata_scsi_transportt.host_attrs.ac == cont;
 }
 
 /**
@@ -722,7 +719,7 @@ int ata_tlink_add(struct ata_link *link)
 	return error;
 }
 
-static struct scsi_transport_template ata_scsi_transportt = {
+struct scsi_transport_template ata_scsi_transportt = {
 	.eh_strategy_handler	= ata_scsi_error,
 	.user_scan		= ata_scsi_user_scan,
 
@@ -783,8 +780,6 @@ __init int libata_transport_init(void)
 	transport_container_register(&ata_link_attr_cont);
 	transport_container_register(&ata_dev_attr_cont);
 
-	ata_scsi_transport_template = &ata_scsi_transportt;
-
 	return 0;
 
  out_unregister_port:
diff --git a/drivers/ata/libata-transport.h b/drivers/ata/libata-transport.h
index fe5ca66fc..629ac843a 100644
--- a/drivers/ata/libata-transport.h
+++ b/drivers/ata/libata-transport.h
@@ -3,7 +3,7 @@
 #define _LIBATA_TRANSPORT_H
 
 
-extern struct scsi_transport_template *ata_scsi_transport_template;
+extern struct scsi_transport_template ata_scsi_transportt;
 
 int ata_tlink_add(struct ata_link *link);
 void ata_tlink_delete(struct ata_link *link);
-- 
2.53.0

Re: [PATCH v2 0/5] ata: libata-transport: series with further refactorings

[Niklas Cassel]

On Thu, 02 Apr 2026 15:28:29 +0200, Heiner Kallweit wrote:
> This series aims at further improving and simplifying the code.
> 
> v2:
> - improve commit messages for patches 1 and 2
> 
> Heiner Kallweit (5):
>   ata: libata-transport: instantiate struct ata_internal statically
>   ata: libata-transport: inline ata_attach|release_transport
>   ata: libata-transport: use static struct ata_transport_internal to
>     simplify match functions
>   ata: libata-transport: split struct ata_internal
>   ata: libata-transport: remove static variable
>     ata_scsi_transport_template
> 
> [...]

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

[1/5] ata: libata-transport: instantiate struct ata_internal statically
      https://git.kernel.org/libata/linux/c/ee1ed7a8
[2/5] ata: libata-transport: inline ata_attach|release_transport
      https://git.kernel.org/libata/linux/c/f2122465
[3/5] ata: libata-transport: use static struct ata_transport_internal to simplify match functions
      https://git.kernel.org/libata/linux/c/365da8c6
[4/5] ata: libata-transport: split struct ata_internal
      https://git.kernel.org/libata/linux/c/359942ba
[5/5] ata: libata-transport: remove static variable ata_scsi_transport_template
      https://git.kernel.org/libata/linux/c/7bf6ddc3

Kind regards,
Niklas