Re: [PATCH] PCI probing debug message uniformization

Previous thread: Re: Faster getcpu() and sched_getcpu() by Andi Kleen on Monday, September 29, 2008 - 1:50 pm. (1 message)

Next thread: [PATCH] x86: export set_memory_ro and set_memory_rw by Jesse Brandeburg on Monday, September 29, 2008 - 2:38 pm. (1 message)
From: Vincent Legoll
Date: Monday, September 29, 2008 - 2:03 pm

Hello,

Here is a patch to uniformize PCI probing debug messages at
boot with dev_printk() intead of manual printk()

for example, it goes from the mixed-style:

PCI: 0000:00:1b.0 reg 10 64bit mmio: [f4280000, f4283fff]
pci 0000:00:1b.0: PME# supported from D0 D3hot D3cold

to uniform:

pci 0000:00:1b.0: reg 10 64bit mmio: [f4280000, f4283fff]
pci 0000:00:1b.0: PME# supported from D0 D3hot D3cold

If people prefer the UPPERCASE "PCI:" prefix from dev_printk(),
that would be matter for another patch...

I'm currently running the kernel with that patch applied, and
diffed bootlog, everything looks OK

Signed-off-by: Vincent Legoll <vincent.legoll@gmail.com>

-- 
Vincent Legoll

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 36698e5..2acb43d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -304,8 +304,9 @@ static int __pci_read_base(struct pci_dev *dev,
enum pci_bar_type type,
 		} else {
 			res->start = l64;
 			res->end = l64 + sz64;
-			printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
-				pci_name(dev), pos, (unsigned long long)res->start,
+			dev_printk(KERN_DEBUG, &dev->dev,
+				"reg %x 64bit mmio: [%llx, %llx]\n", pos,
+				(unsigned long long)res->start,
 				(unsigned long long)res->end);
 		}
 	} else {
@@ -316,8 +317,8 @@ static int __pci_read_base(struct pci_dev *dev,
enum pci_bar_type type,

 		res->start = l;
 		res->end = l + sz;
-		printk(KERN_DEBUG "PCI: %s reg %x %s: [%llx, %llx]\n", pci_name(dev),
-			pos, (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
+		dev_printk(KERN_DEBUG, &dev->dev, "reg %x %s: [%llx, %llx]\n", pos,
+			(res->flags & IORESOURCE_IO) ? "io port" : "32bit mmio",
 			(unsigned long long)res->start, (unsigned long long)res->end);
 	}

@@ -389,8 +390,8 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
 			res->start = base;
 		if (!res->end)
 			res->end = limit + 0xfff;
-		printk(KERN_DEBUG "PCI: bridge %s io port: [%llx, %llx]\n",
-			pci_name(dev), ...
From: Jesse Barnes
Date: Thursday, October 2, 2008 - 11:46 am

Bjorn, how does this jive with the various other debug harmonization patches 
you've been putting together & reviewing?

Thanks,
Jesse

-- 
Jesse Barnes, Intel Open Source Technology Center
--

From: Bjorn Helgaas
Date: Thursday, October 2, 2008 - 11:59 am

I think it's great.  The only nit I would change is to use
"[%#llx-%#llx]" as we do in pci_request_region().

Bjorn
--

From: Vincent Legoll
Date: Friday, October 3, 2008 - 2:14 am

Thanks for the review, I'll post an updated version this evening.

-- 
Vincent Legoll
--

From: Vincent Legoll
Date: Friday, October 3, 2008 - 11:13 am

On Fri, Oct 3, 2008 at 11:14 AM, Vincent Legoll

Here is the updated version, with an extra case from drivers/pci/pcie/aspm.c,
please review for the slight wording change in the message. It's currently
running, so is partly tested (It didn't ran through all cases on my HW)

Producing the following dmesg extract:

[    0.330756] pci 0000:00:02.0: reg 10 32bit mmio: [0xf4200000-0xf427ffff]
[    0.330762] pci 0000:00:02.0: reg 14 io port: [0xe100-0xe107]
[    0.330768] pci 0000:00:02.0: reg 18 32bit mmio: [0xe0000000-0xefffffff]
[    0.330773] pci 0000:00:02.0: reg 1c 32bit mmio: [0xf4000000-0xf40fffff]
[    0.330875] pci 0000:00:1a.0: reg 20 io port: [0xe200-0xe21f]
[    0.330953] pci 0000:00:1a.1: reg 20 io port: [0xe600-0xe61f]
[    0.331025] pci 0000:00:1a.2: reg 20 io port: [0xe000-0xe01f]
[    0.331090] pci 0000:00:1a.7: reg 10 32bit mmio: [0xf4285000-0xf42853ff]
[    0.331179] pci 0000:00:1b.0: reg 10 64bit mmio: [0xf4280000-0xf4283fff]
[    0.331222] pci 0000:00:1b.0: PME# supported from D0 D3hot D3cold
[    0.332010] pci 0000:00:1b.0: PME# disabled
[    0.332165] pci 0000:00:1c.0: PME# supported from D0 D3hot D3cold
[    0.332256] pci 0000:00:1c.0: PME# disabled
[    0.332401] pci 0000:00:1c.3: PME# supported from D0 D3hot D3cold
[    0.333004] pci 0000:00:1c.3: PME# disabled
[    0.333146] pci 0000:00:1c.4: PME# supported from D0 D3hot D3cold
[    0.333238] pci 0000:00:1c.4: PME# disabled
[    0.333417] pci 0000:00:1d.0: reg 20 io port: [0xe300-0xe31f]
[    0.333495] pci 0000:00:1d.1: reg 20 io port: [0xe400-0xe41f]
[    0.333571] pci 0000:00:1d.2: reg 20 io port: [0xe500-0xe51f]
[    0.333637] pci 0000:00:1d.7: reg 10 32bit mmio: [0xf4284000-0xf42843ff]
[    0.333820] pci 0000:00:1f.0: quirk: region 0400-047f claimed by
ICH6 ACPI/GPIO/TCO
[    0.334004] pci 0000:00:1f.0: quirk: region 0480-04bf claimed by ICH6 GPIO
[    0.334152] pci 0000:00:1f.2: reg 10 io port: [0xe700-0xe707]
[    0.334159] pci 0000:00:1f.2: reg 14 io port: [0xe800-0xe803]
[    0.334165] pci ...
From: Bjorn Helgaas
Date: Friday, October 3, 2008 - 11:57 am

It'll be easier for Jesse if you include the proper changelog again
with just a 1-2 line sample of the changed messages.  BTW, the "--"
before your sig confused my mailer into not quoting the patch itself,

dev_info() is exactly equivalent to dev_printk(KERN_INFO).  I usually
use dev_info(), though I'm a bit ambivalent because it's nice to be
able to grep for "printk".  Anyway, maybe you can correct the grammar
of "enabled forcedly" to something like "you can enable with ..." when
you re-post with the changelog.

(Note that dev_dbg() is NOT exactly equivalent to dev_printk(KERN_DEBUG),
so you can't change all of them.  dev_printk(KERN_DEBUG) is always
compiled in, while dev_dbg() is only compiled in when "DEBUG" is defined.)

Bjorn
--

From: Vincent Legoll
Date: Friday, October 3, 2008 - 3:50 pm

The "-- " line is the start-of-.sig marker have I been taught in my



Thanks for the hint, for I may have jumped the gun on those "other" cleanups
;-)

So here is the new version cut'n'pasted from git-format-patch

######################### CUT HERE #####################

From 6250265aded9adcc2bdd5f62977c02a936b641f0 Mon Sep 17 00:00:00 2001
From: Vincent Legoll <vincent.legoll@gmail.com>
Date: Fri, 3 Oct 2008 23:56:02 +0200
Subject: [PATCH] PCI probing debug message uniformization

This patch uniformizes PCI probing debug boot messages
with dev_printk() intead of manual printk()

It changes adress range output from [%llx, %llx] to
[%#llx-%#llx], like in pci_request_region().

For example, it goes from the mixed-style:

PCI: 0000:00:1b.0 reg 10 64bit mmio: [f4280000, f4283fff]
pci 0000:00:1b.0: PME# supported from D0 D3hot D3cold

to uniform:

pci 0000:00:1b.0: reg 10 64bit mmio: [0xf4280000-0xf4283fff]
pci 0000:00:1b.0: PME# supported from D0 D3hot D3cold

This patch has been runtime tested, boot log messages diffed,
everything looks OK.

Signed-off-by: Vincent Legoll <vincent.legoll@gmail.com>
---
 drivers/pci/pcie/aspm.c |    6 +++---
 drivers/pci/probe.c     |   21 +++++++++++----------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 851f5b8..fa0d1a4 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -528,9 +528,9 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 		pci_read_config_dword(child_dev, child_pos + PCI_EXP_DEVCAP,
 			&reg32);
 		if (!(reg32 & PCI_EXP_DEVCAP_RBER) && !aspm_force) {
-			printk("Pre-1.1 PCIe device detected, "
-				"disable ASPM for %s. It can be enabled forcedly"
-				" with 'pcie_aspm=force'\n", pci_name(pdev));
+			dev_printk(KERN_INFO, &child_dev->dev, "disabling ASPM"
+				" on pre-1.1 PCIe device. You can enable it"
+				" back with 'pcie_aspm=force'\n");
 			return -EINVAL;
 		}
 	}
diff --git ...
From: Jesse Barnes
Date: Friday, October 10, 2008 - 9:07 am

Can you respin against linux-next and send me a fresh copy?  This one seems to 
have been corrupted somehow (sorry for the slow reply, it's been sitting in 
my 'to apply' mbox for awhile now).

Thanks,
Jesse
--

From: Vincent Legoll
Date: Sunday, October 12, 2008 - 3:26 am

Here it is, attached, rebased on top of -next

-- 
Vincent Legoll
From: Jesse Barnes
Date: Wednesday, October 15, 2008 - 3:49 am

Applied, thanks.

Jesse
--

Previous thread: Re: Faster getcpu() and sched_getcpu() by Andi Kleen on Monday, September 29, 2008 - 1:50 pm. (1 message)

Next thread: [PATCH] x86: export set_memory_ro and set_memory_rw by Jesse Brandeburg on Monday, September 29, 2008 - 2:38 pm. (1 message)