* [PATCH v2 1/1] PCI: Introduce pci_bus_*() printing macros when device is not available @ 2021-12-09 18:27 Andy Shevchenko 2021-12-09 18:40 ` Joe Perches 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2021-12-09 18:27 UTC (permalink / raw) To: Bjorn Helgaas, linux-pci, linux-kernel; +Cc: Andy Shevchenko, Jean Delvare In some cases PCI device structure is not available and we want to print information based on the bus and devfn parameters. For this cases introduce pci_bus_*() printing macros and replace in existing users. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Jean Delvare <jdelvare@suse.de> --- v2: split out as a separate patch, added tag (Jean) Jean, for now it seems overkill to add pci_bus_dbg() since it's not as simple as the rest, it needs a separate full macro with pr_debug() called explicitly underneath. Hence, I tried and decided not to go until we have enough users. drivers/pci/probe.c | 12 +++--------- include/linux/pci.h | 8 ++++++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 2c91d3509d17..7208901fba70 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2334,16 +2334,12 @@ static bool pci_bus_wait_crs(struct pci_bus *bus, int devfn, u32 *l, */ while (pci_bus_crs_vendor_id(*l)) { if (delay > timeout) { - pr_warn("pci %04x:%02x:%02x.%d: not ready after %dms; giving up\n", - pci_domain_nr(bus), bus->number, - PCI_SLOT(devfn), PCI_FUNC(devfn), delay - 1); + pci_bus_warn(bus, devfn, "not ready after %dms; giving up\n", delay - 1); return false; } if (delay >= 1000) - pr_info("pci %04x:%02x:%02x.%d: not ready after %dms; waiting\n", - pci_domain_nr(bus), bus->number, - PCI_SLOT(devfn), PCI_FUNC(devfn), delay - 1); + pci_bus_info(bus, devfn, "not ready after %dms; waiting\n", delay - 1); msleep(delay); delay *= 2; @@ -2353,9 +2349,7 @@ static bool pci_bus_wait_crs(struct pci_bus *bus, int devfn, u32 *l, } if (delay >= 1000) - pr_info("pci %04x:%02x:%02x.%d: ready after %dms\n", - pci_domain_nr(bus), bus->number, - PCI_SLOT(devfn), PCI_FUNC(devfn), delay - 1); + pci_bus_info(bus, devfn, "ready after %dms\n", delay - 1); return true; } diff --git a/include/linux/pci.h b/include/linux/pci.h index 0ce26850470e..ea8736077d83 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2482,4 +2482,12 @@ void pci_uevent_ers(struct pci_dev *pdev, enum pci_ers_result err_type); WARN_ONCE(condition, "%s %s: " fmt, \ dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg) +#define pci_bus_printk(level, bus, devfn, fmt, arg...) \ + printk(level "pci %04x:%02x:%02x.%d: " fmt, \ + pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##arg) + +#define pci_bus_err(bus, devfn, fmt, arg...) pci_bus_printk(KERN_ERR, bus, devfn, fmt, ##arg) +#define pci_bus_warn(bus, devfn, fmt, arg...) pci_bus_printk(KERN_WARNING, bus, devfn, fmt, ##arg) +#define pci_bus_info(bus, devfn, fmt, arg...) pci_bus_printk(KERN_INFO, bus, devfn, fmt, ##arg) + #endif /* LINUX_PCI_H */ -- 2.33.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] PCI: Introduce pci_bus_*() printing macros when device is not available 2021-12-09 18:27 [PATCH v2 1/1] PCI: Introduce pci_bus_*() printing macros when device is not available Andy Shevchenko @ 2021-12-09 18:40 ` Joe Perches 2021-12-09 19:33 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Joe Perches @ 2021-12-09 18:40 UTC (permalink / raw) To: Andy Shevchenko, Bjorn Helgaas, linux-pci, linux-kernel; +Cc: Jean Delvare On Thu, 2021-12-09 at 20:27 +0200, Andy Shevchenko wrote: > In some cases PCI device structure is not available and we want to print > information based on the bus and devfn parameters. For this cases introduce > pci_bus_*() printing macros and replace in existing users. [] > diff --git a/include/linux/pci.h b/include/linux/pci.h [] > @@ -2482,4 +2482,12 @@ void pci_uevent_ers(struct pci_dev *pdev, enum pci_ers_result err_type); > WARN_ONCE(condition, "%s %s: " fmt, \ > dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg) > > +#define pci_bus_printk(level, bus, devfn, fmt, arg...) \ > + printk(level "pci %04x:%02x:%02x.%d: " fmt, \ > + pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##arg) I have a small preference for using ... and __VA_ARGS___ #define pci_bus_printk(level, bus, devfn, fmt, ...) \ printk(level "pci %04x:%02x:%02x.%d: " fmt, \ pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##__VA_ARGS__) and likely this should have parentheses around bus printk(level "pci %04x:%02x:%02x.%d: " fmt, \ pci_domain_nr(bus), (bus)->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##__VA_ARGS__) > +#define pci_bus_err(bus, devfn, fmt, arg...) pci_bus_printk(KERN_ERR, bus, devfn, fmt, ##arg) > +#define pci_bus_warn(bus, devfn, fmt, arg...) pci_bus_printk(KERN_WARNING, bus, devfn, fmt, ##arg) > +#define pci_bus_info(bus, devfn, fmt, arg...) pci_bus_printk(KERN_INFO, bus, devfn, fmt, ##arg) __VA_ARGS__ etc... ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] PCI: Introduce pci_bus_*() printing macros when device is not available 2021-12-09 18:40 ` Joe Perches @ 2021-12-09 19:33 ` Andy Shevchenko 2021-12-09 19:55 ` Joe Perches 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2021-12-09 19:33 UTC (permalink / raw) To: Joe Perches; +Cc: Bjorn Helgaas, linux-pci, linux-kernel, Jean Delvare On Thu, Dec 09, 2021 at 10:40:57AM -0800, Joe Perches wrote: > On Thu, 2021-12-09 at 20:27 +0200, Andy Shevchenko wrote: ... > > +#define pci_bus_printk(level, bus, devfn, fmt, arg...) \ > > + printk(level "pci %04x:%02x:%02x.%d: " fmt, \ > > + pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##arg) > > I have a small preference for using ... and __VA_ARGS___ It contradicts what other macros in the pci.h do. So I will stick with current solution for the sake of consistency. ... > and likely this should have parentheses around bus > > printk(level "pci %04x:%02x:%02x.%d: " fmt, \ > pci_domain_nr(bus), (bus)->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##__VA_ARGS__) This makes sense, thanks! -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] PCI: Introduce pci_bus_*() printing macros when device is not available 2021-12-09 19:33 ` Andy Shevchenko @ 2021-12-09 19:55 ` Joe Perches 2021-12-09 20:00 ` Krzysztof Wilczyński 0 siblings, 1 reply; 6+ messages in thread From: Joe Perches @ 2021-12-09 19:55 UTC (permalink / raw) To: Andy Shevchenko; +Cc: Bjorn Helgaas, linux-pci, linux-kernel, Jean Delvare On Thu, 2021-12-09 at 21:33 +0200, Andy Shevchenko wrote: > On Thu, Dec 09, 2021 at 10:40:57AM -0800, Joe Perches wrote: > > On Thu, 2021-12-09 at 20:27 +0200, Andy Shevchenko wrote: > > ... > > > > +#define pci_bus_printk(level, bus, devfn, fmt, arg...) \ > > > + printk(level "pci %04x:%02x:%02x.%d: " fmt, \ > > > + pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##arg) > > > > I have a small preference for using ... and __VA_ARGS___ > > It contradicts what other macros in the pci.h do. > So I will stick with current solution for the sake of consistency. There's always this possibility. And this: (cheers) --- include/linux/pci.h | 58 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/include/linux/pci.h b/include/linux/pci.h index 0ce26850470ef..1dc34f6eaeda7 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2456,30 +2456,38 @@ void pci_uevent_ers(struct pci_dev *pdev, enum pci_ers_result err_type); /* Provide the legacy pci_dma_* API */ #include <linux/pci-dma-compat.h> -#define pci_printk(level, pdev, fmt, arg...) \ - dev_printk(level, &(pdev)->dev, fmt, ##arg) - -#define pci_emerg(pdev, fmt, arg...) dev_emerg(&(pdev)->dev, fmt, ##arg) -#define pci_alert(pdev, fmt, arg...) dev_alert(&(pdev)->dev, fmt, ##arg) -#define pci_crit(pdev, fmt, arg...) dev_crit(&(pdev)->dev, fmt, ##arg) -#define pci_err(pdev, fmt, arg...) dev_err(&(pdev)->dev, fmt, ##arg) -#define pci_warn(pdev, fmt, arg...) dev_warn(&(pdev)->dev, fmt, ##arg) -#define pci_notice(pdev, fmt, arg...) dev_notice(&(pdev)->dev, fmt, ##arg) -#define pci_info(pdev, fmt, arg...) dev_info(&(pdev)->dev, fmt, ##arg) -#define pci_dbg(pdev, fmt, arg...) dev_dbg(&(pdev)->dev, fmt, ##arg) - -#define pci_notice_ratelimited(pdev, fmt, arg...) \ - dev_notice_ratelimited(&(pdev)->dev, fmt, ##arg) - -#define pci_info_ratelimited(pdev, fmt, arg...) \ - dev_info_ratelimited(&(pdev)->dev, fmt, ##arg) - -#define pci_WARN(pdev, condition, fmt, arg...) \ - WARN(condition, "%s %s: " fmt, \ - dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg) - -#define pci_WARN_ONCE(pdev, condition, fmt, arg...) \ - WARN_ONCE(condition, "%s %s: " fmt, \ - dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg) +#define pci_printk(level, pdev, fmt, ...) \ + dev_printk(level, &(pdev)->dev, fmt, ##__VA_ARGS__) + +#define pci_emerg(pdev, fmt, ...) \ + dev_emerg(&(pdev)->dev, fmt, ##__VA_ARGS__) +#define pci_alert(pdev, fmt, ...) \ + dev_alert(&(pdev)->dev, fmt, ##__VA_ARGS__) +#define pci_crit(pdev, fmt, ...) \ + dev_crit(&(pdev)->dev, fmt, ##__VA_ARGS__) +#define pci_err(pdev, fmt, ...) \ + dev_err(&(pdev)->dev, fmt, ##__VA_ARGS__) +#define pci_warn(pdev, fmt, ...) \ + dev_warn(&(pdev)->dev, fmt, ##__VA_ARGS__) +#define pci_notice(pdev, fmt, ...) \ + dev_notice(&(pdev)->dev, fmt, ##__VA_ARGS__) +#define pci_info(pdev, fmt, ...) \ + dev_info(&(pdev)->dev, fmt, ##__VA_ARGS__) +#define pci_dbg(pdev, fmt, ...) \ + dev_dbg(&(pdev)->dev, fmt, ##__VA_ARGS__) + +#define pci_notice_ratelimited(pdev, fmt, ...) \ + dev_notice_ratelimited(&(pdev)->dev, fmt, ##__VA_ARGS__) +#define pci_info_ratelimited(pdev, fmt, ...) \ + dev_info_ratelimited(&(pdev)->dev, fmt, ##__VA_ARGS__) + +#define pci_WARN(pdev, condition, fmt, ...) \ + WARN(condition, "%s %s: " fmt, \ + dev_driver_string(&(pdev)->dev), pci_name(pdev), \ + ##__VA_ARGS__) +#define pci_WARN_ONCE(pdev, condition, fmt, ...) \ + WARN_ONCE(condition, "%s %s: " fmt, \ + dev_driver_string(&(pdev)->dev), pci_name(pdev), \ + ##__VA_ARGS__) #endif /* LINUX_PCI_H */ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] PCI: Introduce pci_bus_*() printing macros when device is not available 2021-12-09 19:55 ` Joe Perches @ 2021-12-09 20:00 ` Krzysztof Wilczyński 2021-12-09 20:13 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Krzysztof Wilczyński @ 2021-12-09 20:00 UTC (permalink / raw) To: Joe Perches Cc: Andy Shevchenko, Bjorn Helgaas, linux-pci, linux-kernel, Jean Delvare Hi Andy and Joe, [...] > > > > +#define pci_bus_printk(level, bus, devfn, fmt, arg...) \ > > > > + printk(level "pci %04x:%02x:%02x.%d: " fmt, \ > > > > + pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##arg) > > > > > > I have a small preference for using ... and __VA_ARGS___ > > > > It contradicts what other macros in the pci.h do. > > So I will stick with current solution for the sake of consistency. > > There's always this possibility. > > And this: (cheers) > --- > include/linux/pci.h | 58 ++++++++++++++++++++++++++++++----------------------- > 1 file changed, 33 insertions(+), 25 deletions(-) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 0ce26850470ef..1dc34f6eaeda7 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -2456,30 +2456,38 @@ void pci_uevent_ers(struct pci_dev *pdev, enum pci_ers_result err_type); > /* Provide the legacy pci_dma_* API */ > #include <linux/pci-dma-compat.h> > > -#define pci_printk(level, pdev, fmt, arg...) \ > - dev_printk(level, &(pdev)->dev, fmt, ##arg) > - > -#define pci_emerg(pdev, fmt, arg...) dev_emerg(&(pdev)->dev, fmt, ##arg) > -#define pci_alert(pdev, fmt, arg...) dev_alert(&(pdev)->dev, fmt, ##arg) > -#define pci_crit(pdev, fmt, arg...) dev_crit(&(pdev)->dev, fmt, ##arg) > -#define pci_err(pdev, fmt, arg...) dev_err(&(pdev)->dev, fmt, ##arg) > -#define pci_warn(pdev, fmt, arg...) dev_warn(&(pdev)->dev, fmt, ##arg) > -#define pci_notice(pdev, fmt, arg...) dev_notice(&(pdev)->dev, fmt, ##arg) > -#define pci_info(pdev, fmt, arg...) dev_info(&(pdev)->dev, fmt, ##arg) > -#define pci_dbg(pdev, fmt, arg...) dev_dbg(&(pdev)->dev, fmt, ##arg) > - > -#define pci_notice_ratelimited(pdev, fmt, arg...) \ > - dev_notice_ratelimited(&(pdev)->dev, fmt, ##arg) > - > -#define pci_info_ratelimited(pdev, fmt, arg...) \ > - dev_info_ratelimited(&(pdev)->dev, fmt, ##arg) > - > -#define pci_WARN(pdev, condition, fmt, arg...) \ > - WARN(condition, "%s %s: " fmt, \ > - dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg) > - > -#define pci_WARN_ONCE(pdev, condition, fmt, arg...) \ > - WARN_ONCE(condition, "%s %s: " fmt, \ > - dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg) > +#define pci_printk(level, pdev, fmt, ...) \ > + dev_printk(level, &(pdev)->dev, fmt, ##__VA_ARGS__) > + > +#define pci_emerg(pdev, fmt, ...) \ > + dev_emerg(&(pdev)->dev, fmt, ##__VA_ARGS__) > +#define pci_alert(pdev, fmt, ...) \ > + dev_alert(&(pdev)->dev, fmt, ##__VA_ARGS__) > +#define pci_crit(pdev, fmt, ...) \ > + dev_crit(&(pdev)->dev, fmt, ##__VA_ARGS__) > +#define pci_err(pdev, fmt, ...) \ > + dev_err(&(pdev)->dev, fmt, ##__VA_ARGS__) > +#define pci_warn(pdev, fmt, ...) \ > + dev_warn(&(pdev)->dev, fmt, ##__VA_ARGS__) > +#define pci_notice(pdev, fmt, ...) \ > + dev_notice(&(pdev)->dev, fmt, ##__VA_ARGS__) > +#define pci_info(pdev, fmt, ...) \ > + dev_info(&(pdev)->dev, fmt, ##__VA_ARGS__) > +#define pci_dbg(pdev, fmt, ...) \ > + dev_dbg(&(pdev)->dev, fmt, ##__VA_ARGS__) > + > +#define pci_notice_ratelimited(pdev, fmt, ...) \ > + dev_notice_ratelimited(&(pdev)->dev, fmt, ##__VA_ARGS__) > +#define pci_info_ratelimited(pdev, fmt, ...) \ > + dev_info_ratelimited(&(pdev)->dev, fmt, ##__VA_ARGS__) > + > +#define pci_WARN(pdev, condition, fmt, ...) \ > + WARN(condition, "%s %s: " fmt, \ > + dev_driver_string(&(pdev)->dev), pci_name(pdev), \ > + ##__VA_ARGS__) > +#define pci_WARN_ONCE(pdev, condition, fmt, ...) \ > + WARN_ONCE(condition, "%s %s: " fmt, \ > + dev_driver_string(&(pdev)->dev), pci_name(pdev), \ > + ##__VA_ARGS__) > > #endif /* LINUX_PCI_H */ I think both things look nice! So perhaps meet in-between here? Two patches in a small series: one adds new useful macros from Andy, and the other updates current macros as per Joe's feedback/preference? I am sure Bjorn wouldn't mind (hopefully). Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] PCI: Introduce pci_bus_*() printing macros when device is not available 2021-12-09 20:00 ` Krzysztof Wilczyński @ 2021-12-09 20:13 ` Andy Shevchenko 0 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2021-12-09 20:13 UTC (permalink / raw) To: Krzysztof Wilczyński Cc: Joe Perches, Bjorn Helgaas, linux-pci, linux-kernel, Jean Delvare On Thu, Dec 09, 2021 at 09:00:28PM +0100, Krzysztof Wilczyński wrote: > Hi Andy and Joe, ... > I think both things look nice! > > So perhaps meet in-between here? Two patches in a small series: one adds > new useful macros from Andy, and the other updates current macros as per > Joe's feedback/preference? I am sure Bjorn wouldn't mind (hopefully). Thanks, I agree that these changes should be separated, otherwise I'm fine if pci.h switches to __VA_ARGS__. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-09 20:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-09 18:27 [PATCH v2 1/1] PCI: Introduce pci_bus_*() printing macros when device is not available Andy Shevchenko 2021-12-09 18:40 ` Joe Perches 2021-12-09 19:33 ` Andy Shevchenko 2021-12-09 19:55 ` Joe Perches 2021-12-09 20:00 ` Krzysztof Wilczyński 2021-12-09 20:13 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®