Re: [PATCH 01/24] libata: move ata_{port,link,dev}_dbg to dynamic debugging

[Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>]

[ added Tejun to Cc: ]

On 12/13/18 11:46 AM, Hannes Reinecke wrote:
> Use dev_dbg() for ata_{port,link,dev}_dbg to allow for selective
> debugging during runtime.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  include/linux/libata.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 38c95d66ab12..7b2f039d3d21 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1436,7 +1436,7 @@ void ata_dev_printk(const struct ata_device *dev, const char *level,
>  #define ata_port_info(ap, fmt, ...)				\
>  	ata_port_printk(ap, KERN_INFO, fmt, ##__VA_ARGS__)
>  #define ata_port_dbg(ap, fmt, ...)				\
> -	ata_port_printk(ap, KERN_DEBUG, fmt, ##__VA_ARGS__)
> +	dev_dbg(&ap->tdev, fmt, ##__VA_ARGS__)
>  
>  #define ata_link_err(link, fmt, ...)				\
>  	ata_link_printk(link, KERN_ERR, fmt, ##__VA_ARGS__)
> @@ -1447,7 +1447,7 @@ void ata_dev_printk(const struct ata_device *dev, const char *level,
>  #define ata_link_info(link, fmt, ...)				\
>  	ata_link_printk(link, KERN_INFO, fmt, ##__VA_ARGS__)
>  #define ata_link_dbg(link, fmt, ...)				\
> -	ata_link_printk(link, KERN_DEBUG, fmt, ##__VA_ARGS__)
> +	dev_dbg(&link->tdev, fmt, ##__VA_ARGS__)
>  
>  #define ata_dev_err(dev, fmt, ...)				\
>  	ata_dev_printk(dev, KERN_ERR, fmt, ##__VA_ARGS__)
> @@ -1458,7 +1458,7 @@ void ata_dev_printk(const struct ata_device *dev, const char *level,
>  #define ata_dev_info(dev, fmt, ...)				\
>  	ata_dev_printk(dev, KERN_INFO, fmt, ##__VA_ARGS__)
>  #define ata_dev_dbg(dev, fmt, ...)				\
> -	ata_dev_printk(dev, KERN_DEBUG, fmt, ##__VA_ARGS__)
> +	dev_dbg(&dev->tdev, fmt, ##__VA_ARGS__)

While you are at it please remove ata_{port,link,dev}_printk()
altogether.

[ Since code in libata-transport.c sets valid device names using
  dev_set_name() we can simply use generic dev_*() helpers. ]

Please also note that ata_{link,dev}_printk() differs slightly in PMP
handling for links and devices names from code in libata-transport.c:

void ata_link_printk(const struct ata_link *link, const char *level,
		     const char *fmt, ...)
...
	if (sata_pmp_attached(link->ap) || link->ap->slave_link)
		printk("%sata%u.%02u: %pV",
		       level, link->ap->print_id, link->pmp, &vaf);
	else
		printk("%sata%u: %pV",
		       level, link->ap->print_id, &vaf);
...

int ata_tlink_add(struct ata_link *link)
...
	if (ata_is_host_link(link))
		dev_set_name(dev, "link%d", ap->print_id);
        else
		dev_set_name(dev, "link%d.%d", ap->print_id, link->pmp);
...


void ata_dev_printk(const struct ata_device *dev, const char *level,
		    const char *fmt, ...)
...
	printk("%sata%u.%02u: %pV",
	       level, dev->link->ap->print_id, dev->link->pmp + dev->devno,
	       &vaf);
...

static int ata_tdev_add(struct ata_device *ata_dev)
...
	if (ata_is_host_link(link))
		dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno);
        else
		dev_set_name(dev, "dev%d.%d.0", ap->print_id, link->pmp);
...

I assume that the code in libata-transport.c is the preferred one but
I would like Jens or Tejun to confirm this.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics