| Message ID | 20181213104716.31930-2-hare@suse.de |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | [01/24] libata: move ata_{port,link,dev}_dbg to dynamic debugging | expand |
[ 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
On 1/30/20 11:42 AM, Bartlomiej Zolnierkiewicz wrote: > > [ 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 [ .. ] > > 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: > I know. I hated it, too. One should try to keep the naming consistent, and these 'link' devices mess up everything. > 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. > I'll go with the libata-transport names; one should really prefix the subsystem with the names otherwise things really get messy if one tries to analyse or debug issues. Cheers, Hannes
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__) void ata_print_version(const struct device *dev, const char *version);
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(-)