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

Damien Le Moal
2025-04-11T00:00:36Z

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