[PATCH] ata: libata-core: Simplify ata_print_version_once

[PATCH] ata: libata-core: Simplify ata_print_version_once

[Heiner Kallweit]

Use dev_dbg_once() instead of open-coding the once functionality.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/ata/libata-core.c |  6 +++---
 include/linux/libata.h    | 13 +------------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 773799cfd..e58a22e83 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6682,11 +6682,11 @@ const struct ata_port_info ata_dummy_port_info = {
 };
 EXPORT_SYMBOL_GPL(ata_dummy_port_info);
 
-void ata_print_version(const struct device *dev, const char *version)
+void ata_print_version_once(const struct device *dev, const char *version)
 {
-	dev_printk(KERN_DEBUG, dev, "version %s\n", version);
+	dev_dbg_once(dev, "version %s\n", version);
 }
-EXPORT_SYMBOL(ata_print_version);
+EXPORT_SYMBOL(ata_print_version_once);
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(ata_tf_load);
 EXPORT_TRACEPOINT_SYMBOL_GPL(ata_exec_command);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e5695998a..02c9f621e 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -41,16 +41,7 @@
  */
 #undef ATA_IRQ_TRAP		/* define to ack screaming irqs */
 
-
-#define ata_print_version_once(dev, version)			\
-({								\
-	static bool __print_once;				\
-								\
-	if (!__print_once) {					\
-		__print_once = true;				\
-		ata_print_version(dev, version);		\
-	}							\
-})
+void ata_print_version_once(const struct device *dev, const char *version);
 
 /* defines only for the constants which don't work well as enums */
 #define ATA_TAG_POISON		0xfafbfcfdU
@@ -1593,8 +1584,6 @@ do {								\
 #define ata_dev_dbg(dev, fmt, ...)				\
 	ata_dev_printk(debug, dev, fmt, ##__VA_ARGS__)
 
-void ata_print_version(const struct device *dev, const char *version);
-
 /*
  * ata_eh_info helpers
  */
-- 
2.49.0

Re: [PATCH] ata: libata-core: Simplify ata_print_version_once

[Damien Le Moal]

On 4/11/25 06:45, Heiner Kallweit wrote:
> Use dev_dbg_once() instead of open-coding the once functionality.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/ata/libata-core.c |  6 +++---
>  include/linux/libata.h    | 13 +------------
>  2 files changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 773799cfd..e58a22e83 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6682,11 +6682,11 @@ const struct ata_port_info ata_dummy_port_info = {
>  };
>  EXPORT_SYMBOL_GPL(ata_dummy_port_info);
>  
> -void ata_print_version(const struct device *dev, const char *version)
> +void ata_print_version_once(const struct device *dev, const char *version)
>  {
> -	dev_printk(KERN_DEBUG, dev, "version %s\n", version);
> +	dev_dbg_once(dev, "version %s\n", version);
>  }
> -EXPORT_SYMBOL(ata_print_version);
> +EXPORT_SYMBOL(ata_print_version_once);

Overall, looks good, but I really do not see the point of having this as an
exported function. So while at it, can you change it to be inline in libata.h ?

>  
>  EXPORT_TRACEPOINT_SYMBOL_GPL(ata_tf_load);
>  EXPORT_TRACEPOINT_SYMBOL_GPL(ata_exec_command);
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index e5695998a..02c9f621e 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -41,16 +41,7 @@
>   */
>  #undef ATA_IRQ_TRAP		/* define to ack screaming irqs */
>  
> -
> -#define ata_print_version_once(dev, version)			\
> -({								\
> -	static bool __print_once;				\
> -								\
> -	if (!__print_once) {					\
> -		__print_once = true;				\
> -		ata_print_version(dev, version);		\
> -	}							\
> -})
> +void ata_print_version_once(const struct device *dev, const char *version);
>  
>  /* defines only for the constants which don't work well as enums */
>  #define ATA_TAG_POISON		0xfafbfcfdU
> @@ -1593,8 +1584,6 @@ do {								\
>  #define ata_dev_dbg(dev, fmt, ...)				\
>  	ata_dev_printk(debug, dev, fmt, ##__VA_ARGS__)
>  
> -void ata_print_version(const struct device *dev, const char *version);
> -
>  /*
>   * ata_eh_info helpers
>   */


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libata-core: Simplify ata_print_version_once

[Heiner Kallweit]

On 11.04.2025 02:00, Damien Le Moal wrote:
> On 4/11/25 06:45, Heiner Kallweit wrote:
>> Use dev_dbg_once() instead of open-coding the once functionality.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/ata/libata-core.c |  6 +++---
>>  include/linux/libata.h    | 13 +------------
>>  2 files changed, 4 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 773799cfd..e58a22e83 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -6682,11 +6682,11 @@ const struct ata_port_info ata_dummy_port_info = {
>>  };
>>  EXPORT_SYMBOL_GPL(ata_dummy_port_info);
>>  
>> -void ata_print_version(const struct device *dev, const char *version)
>> +void ata_print_version_once(const struct device *dev, const char *version)
>>  {
>> -	dev_printk(KERN_DEBUG, dev, "version %s\n", version);
>> +	dev_dbg_once(dev, "version %s\n", version);
>>  }
>> -EXPORT_SYMBOL(ata_print_version);
>> +EXPORT_SYMBOL(ata_print_version_once);
> 
> Overall, looks good, but I really do not see the point of having this as an
> exported function. So while at it, can you change it to be inline in libata.h ?
> 
Right, this would be better. I'll submit a v2.