[02/10] PCI/PME: Replace dev_printk(KERN_DEBUG) with dev_info()
diff mbox series

Message ID 20190509141456.223614-3-helgaas@kernel.org
State In Next
Commit 5232e2faa33efff05af1450e3a12357f072f448b
Headers show
Series
  • PCI: Log with pci_dev, not pcie_device
Related show

Commit Message

Bjorn Helgaas May 9, 2019, 2:14 p.m. UTC
From: Frederick Lawler <fred@fredlawl.com>

Replace dev_printk(KERN_DEBUG) with dev_info() or dev_err() to be more
consistent with other logging.

These could be converted to dev_dbg(), but that depends on
CONFIG_DYNAMIC_DEBUG and DEBUG, and we want most of these messages to
*always* be in the dmesg log.

Also, use dev_fmt() to add the service name.  Example output change:

  - pcieport 0000:80:10.0: Signaling PME with IRQ ...
  + pcieport 0000:80:10.0: PME: Signaling with IRQ ...

Link: https://lore.kernel.org/lkml/20190503035946.23608-4-fred@fredlawl.com
Signed-off-by: Frederick Lawler <fred@fredlawl.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/pme.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Andy Shevchenko May 9, 2019, 5:35 p.m. UTC | #1
On Thu, May 9, 2019 at 5:18 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> Replace dev_printk(KERN_DEBUG) with dev_info() or dev_err() to be more
> consistent with other logging.
>
> These could be converted to dev_dbg(), but that depends on
> CONFIG_DYNAMIC_DEBUG and DEBUG, and we want most of these messages to
> *always* be in the dmesg log.
>
> Also, use dev_fmt() to add the service name.  Example output change:
>
>   - pcieport 0000:80:10.0: Signaling PME with IRQ ...
>   + pcieport 0000:80:10.0: PME: Signaling with IRQ ...

> +               pci_info(port, "interrupt generated for non-existent device %02x:%02x.%d\n",

Can we be slightly more consistent here, i.e. start from Capital letter?

> +                        busnr, PCI_SLOT(devfn), PCI_FUNC(devfn));

> +               pci_info(port, "Spurious native interrupt!\n");

> +       pci_info(port, "Signaling with IRQ %d\n", srv->irq);
Joe Perches May 9, 2019, 6:31 p.m. UTC | #2
On Thu, 2019-05-09 at 20:35 +0300, Andy Shevchenko wrote:
> On Thu, May 9, 2019 at 5:18 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > Replace dev_printk(KERN_DEBUG) with dev_info() or dev_err() to be more
> > consistent with other logging.
> > 
> > These could be converted to dev_dbg(), but that depends on
> > CONFIG_DYNAMIC_DEBUG and DEBUG, and we want most of these messages to
> > *always* be in the dmesg log.
> > 
> > Also, use dev_fmt() to add the service name.  Example output change:
> > 
> >   - pcieport 0000:80:10.0: Signaling PME with IRQ ...
> >   + pcieport 0000:80:10.0: PME: Signaling with IRQ ...
> > +               pci_info(port, "interrupt generated for non-existent device %02x:%02x.%d\n",
> 
> Can we be slightly more consistent here, i.e. start from Capital letter?
> 
> > +                        busnr, PCI_SLOT(devfn), PCI_FUNC(devfn));
> > +               pci_info(port, "Spurious native interrupt!\n");
> > +       pci_info(port, "Signaling with IRQ %d\n", srv->irq);

Why change the logging level?
Why not use #define DEBUG and use pci_dbg ?
Bjorn Helgaas May 9, 2019, 9:12 p.m. UTC | #3
On Thu, May 09, 2019 at 11:31:04AM -0700, Joe Perches wrote:
> On Thu, 2019-05-09 at 20:35 +0300, Andy Shevchenko wrote:
> > On Thu, May 9, 2019 at 5:18 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > Replace dev_printk(KERN_DEBUG) with dev_info() or dev_err() to be more
> > > consistent with other logging.
> > > 
> > > These could be converted to dev_dbg(), but that depends on
> > > CONFIG_DYNAMIC_DEBUG and DEBUG, and we want most of these messages to
> > > *always* be in the dmesg log.
> > > 
> > > Also, use dev_fmt() to add the service name.  Example output change:
> > > 
> > >   - pcieport 0000:80:10.0: Signaling PME with IRQ ...
> > >   + pcieport 0000:80:10.0: PME: Signaling with IRQ ...
> > > +               pci_info(port, "interrupt generated for non-existent device %02x:%02x.%d\n",
> > > +                        busnr, PCI_SLOT(devfn), PCI_FUNC(devfn));
> > > +               pci_info(port, "Spurious native interrupt!\n");
> > > +       pci_info(port, "Signaling with IRQ %d\n", srv->irq);
> 
> Why change the logging level?
> Why not use #define DEBUG and use pci_dbg ?

What would the benefit of using DEBUG be?  I don't want these
particular messages to be conditional.

For messages that *should* be conditional, there's a later patch in
the series to use pci_dbg() and convert them to dyndbg so we have a
single mechanism for turning them off/on.

Bjorn
Joe Perches May 10, 2019, 2:22 a.m. UTC | #4
On Thu, 2019-05-09 at 16:12 -0500, Bjorn Helgaas wrote:
> On Thu, May 09, 2019 at 11:31:04AM -0700, Joe Perches wrote:
> > On Thu, 2019-05-09 at 20:35 +0300, Andy Shevchenko wrote:
> > > On Thu, May 9, 2019 at 5:18 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > Replace dev_printk(KERN_DEBUG) with dev_info() or dev_err() to be more
> > > > consistent with other logging.
> > > > 
> > > > These could be converted to dev_dbg(), but that depends on
> > > > CONFIG_DYNAMIC_DEBUG and DEBUG, and we want most of these messages to
> > > > *always* be in the dmesg log.
> > > > 
> > > > Also, use dev_fmt() to add the service name.  Example output change:
> > > > 
> > > >   - pcieport 0000:80:10.0: Signaling PME with IRQ ...
> > > >   + pcieport 0000:80:10.0: PME: Signaling with IRQ ...
> > > > +               pci_info(port, "interrupt generated for non-existent device %02x:%02x.%d\n",
> > > > +                        busnr, PCI_SLOT(devfn), PCI_FUNC(devfn));
> > > > +               pci_info(port, "Spurious native interrupt!\n");
> > > > +       pci_info(port, "Signaling with IRQ %d\n", srv->irq);
> > 
> > Why change the logging level?
> > Why not use #define DEBUG and use pci_dbg ?
> 
> What would the benefit of using DEBUG be?

It makes the <foo>_dbg output unconditional or
possible to be disabled via dynamic_debug

> I don't want these
> particular messages to be conditional.

Patch
diff mbox series

diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
index 54d593d10396..f38e6c19dd50 100644
--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -7,6 +7,8 @@ 
  * Copyright (C) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
  */
 
+#define dev_fmt(fmt) "PME: " fmt
+
 #include <linux/pci.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -194,14 +196,14 @@  static void pcie_pme_handle_request(struct pci_dev *port, u16 req_id)
 		 * assuming that the PME was reported by a PCIe-PCI bridge that
 		 * used devfn different from zero.
 		 */
-		pci_dbg(port, "PME interrupt generated for non-existent device %02x:%02x.%d\n",
-			busnr, PCI_SLOT(devfn), PCI_FUNC(devfn));
+		pci_info(port, "interrupt generated for non-existent device %02x:%02x.%d\n",
+			 busnr, PCI_SLOT(devfn), PCI_FUNC(devfn));
 		found = pcie_pme_from_pci_bridge(bus, 0);
 	}
 
  out:
 	if (!found)
-		pci_dbg(port, "Spurious native PME interrupt!\n");
+		pci_info(port, "Spurious native interrupt!\n");
 }
 
 /**
@@ -341,7 +343,7 @@  static int pcie_pme_probe(struct pcie_device *srv)
 		return ret;
 	}
 
-	pci_info(port, "Signaling PME with IRQ %d\n", srv->irq);
+	pci_info(port, "Signaling with IRQ %d\n", srv->irq);
 
 	pcie_pme_mark_devices(port);
 	pcie_pme_interrupt_enable(port, true);