* [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 related [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 related [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;
as well as URLs for NNTP newsgroup(s).