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

[Damien Le Moal <dlemoal@kernel.org>]

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