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

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

[Heiner Kallweit]

This series aims at further improving and simplifying the code.

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 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, what allows to simplify the
code.

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

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

[Damien Le Moal]

On 3/31/26 12:18 AM, Heiner Kallweit wrote:
> Struct ata_internal is only instantiated once, in module init code.
> So we can also instantiate it statically, what allows to simplify the
> code.

s/what allows to simplify/which allows simplifying

With that,

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

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

[Hannes Reinecke]

On 3/30/26 17:18, Heiner Kallweit wrote:
> Struct ata_internal is only instantiated once, in module init code.
> So we can also instantiate it statically, what allows to simplify the
> code.
> 
> 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(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Heiner Kallweit]

Both functions can be merged into libata_transport_init() and
libata_transport_exit() respectively, thus simplifying the code.

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

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

[Damien Le Moal]

On 3/31/26 12:18 AM, Heiner Kallweit wrote:
> Both functions can be merged into libata_transport_init() and
> libata_transport_exit() respectively, thus simplifying the code.

That is not really simplifying anything but rather removing somewhat
unnecessary helper functions that are being called once only. So let's just say
that here.

Other than this, looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

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

[Hannes Reinecke]

On 3/30/26 17:18, Heiner Kallweit wrote:
> Both functions can be merged into libata_transport_init() and
> libata_transport_exit() respectively, thus simplifying the code.
> 
> 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(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

[PATCH 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.

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

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

[Damien Le Moal]

On 3/31/26 12:19 AM, Heiner Kallweit wrote:
> 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.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Looks OK. Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

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

[Hannes Reinecke]

On 3/30/26 17:19, Heiner Kallweit wrote:
> 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.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>   drivers/ata/libata-transport.c | 44 ++++++++++++++++++----------------
>   1 file changed, 23 insertions(+), 21 deletions(-)
> 
Hmm. Exchanging one set of helpers with another set of helpers is
not really gaining us much, but if you must...

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

[PATCH 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.

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

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

[Damien Le Moal]

On 3/31/26 12:20 AM, Heiner Kallweit wrote:
> 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.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

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

[Hannes Reinecke]

On 3/30/26 17:20, Heiner Kallweit wrote:
> 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.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>   drivers/ata/libata-transport.c | 53 ++++++++++++++++------------------
>   1 file changed, 25 insertions(+), 28 deletions(-)
> 

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

[PATCH 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.

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 5/5] ata: libata-transport: remove static variable ata_scsi_transport_template

[Damien Le Moal]

On 3/31/26 12:21 AM, Heiner Kallweit wrote:
> Simplify the code by making struct ata_scsi_transportt public, instead
> of using separate variable ata_scsi_transport_template.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

One nit below. With that fixed,

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

> 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;

Drop the extern please.

>  
>  int ata_tlink_add(struct ata_link *link);
>  void ata_tlink_delete(struct ata_link *link);


-- 
Damien Le Moal
Western Digital Research

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

[Sergey Shtylyov]

On 3/31/26 4:42 AM, Damien Le Moal wrote:
[...]
>> Simplify the code by making struct ata_scsi_transportt public, instead
>> of using separate variable ata_scsi_transport_template.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> One nit below. With that fixed,
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> 
>> 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;
> 
> Drop the extern please.

   I'm afraid this will turn the variable declaration into definition,
and I don't think we want that in a header file... Unlike the functions,
*extern* with variable declarations is not implied, IIRC.

[...]

MBR, Sergey

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

[Damien Le Moal]

On 3/31/26 20:41, Sergey Shtylyov wrote:
> On 3/31/26 4:42 AM, Damien Le Moal wrote:
> [...]
>>> Simplify the code by making struct ata_scsi_transportt public, instead
>>> of using separate variable ata_scsi_transport_template.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> One nit below. With that fixed,
>>
>> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
>>
>>> 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;
>>
>> Drop the extern please.
> 
>    I'm afraid this will turn the variable declaration into definition,
> and I don't think we want that in a header file... Unlike the functions,
> *extern* with variable declarations is not implied, IIRC.

Arg... You are of course absolutely right. I was thinking "function" and
overlooked that it is a variable. Apologies for the noise.

> 
> [...]
> 
> MBR, Sergey
> 


-- 
Damien Le Moal
Western Digital Research

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

[Hannes Reinecke]

On 3/30/26 17:21, Heiner Kallweit wrote:
> Simplify the code by making struct ata_scsi_transportt public, instead
> of using separate variable ata_scsi_transport_template.
> 
> 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(-)
> 
Huh. Not a big fan of this.

The idea of using a pointer is that it'll be initialized to 'NULL'
if the module / file isn't present.
Exposing a static pointer is tricky in that regard.

Not sure if that is the direction we want to go, but I'll leave it
to the maintainers to decide.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Heiner Kallweit]

On 07.04.2026 12:39, Hannes Reinecke wrote:
> On 3/30/26 17:21, Heiner Kallweit wrote:
>> Simplify the code by making struct ata_scsi_transportt public, instead
>> of using separate variable ata_scsi_transport_template.
>>
>> 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(-)
>>
> Huh. Not a big fan of this.
> 
> The idea of using a pointer is that it'll be initialized to 'NULL'
> if the module / file isn't present.
> Exposing a static pointer is tricky in that regard.
> 
Provider (libata-transport) and consumer (libata-scsi) of this static
struct are always built together:

libata-y	:= libata-core.o libata-scsi.o libata-eh.o \
	libata-transport.o libata-trace.o

> Not sure if that is the direction we want to go, but I'll leave it
> to the maintainers to decide.
> 
> Cheers,
> 
> Hannes

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

[Niklas Cassel]

Hello Heiner,

Once again, thank you for the nice cleanup!

It would be nice if you could send a V2 within a few days,
so that this will make it for 7.1.


Kind regards,
Niklas

On Mon, Mar 30, 2026 at 05:17:12PM +0200, Heiner Kallweit wrote:
> This series aims at further improving and simplifying the code.
> 
> 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
>