On Sun, Oct 21, 2018 at 05:20:05PM +0200, Heiner Kallweit wrote:
> pci_printk messages with level KERN_DEBUG are written to the syslog
> even if CONFIG_DEBUG isn't defined. This may confuse users, e.g. if
> the debug messages include "not supported" or "failed to". To avoid
> this switch the remaining pci_printk(KERN_DEBUG users to pci_dbg.
Yeah...
I always find the dev_dbg() stuff confusing because it depends on DEBUG,
CONFIG_DYNAMIC_DEBUG, and possibly some dynamic stuff that I always have to
look up, so you can't tell from the source whether to expect the message in
a dmesg log, and I personally never use it.
When we get bug reports, they often already contain a dmesg log, and it's
super nice when that has everything we need. If they don't, it's a lot
easier to just ask for a dmesg log than to figure out how to enable debug
output.
Some of these, e.g., the device ID, BAR and bridge window info, "PME#
supported from", etc., probably should be converted to pci_info() because
I think we really do always want them.
That said, some of the messages *are* potentially confusing. I would
rather change the text of the message to make them more useful.
Bjorn
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
> drivers/pci/bus.c | 3 +--
> drivers/pci/pci.c | 39 +++++++++++++++++++--------------------
> drivers/pci/probe.c | 17 ++++++++---------
> drivers/pci/setup-bus.c | 26 +++++++++++++-------------
> 4 files changed, 41 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 5cb40b251..ddee5f461 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -288,8 +288,7 @@ bool pci_bus_clip_resource(struct pci_dev *dev, int idx)
> res->end = end;
> res->flags &= ~IORESOURCE_UNSET;
> orig_res.flags &= ~IORESOURCE_UNSET;
> - pci_printk(KERN_DEBUG, dev, "%pR clipped to %pR\n",
> - &orig_res, res);
> + pci_dbg(dev, "%pR clipped to %pR\n", &orig_res, res);
>
> return true;
> }
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index fb63b3d89..e60fd652d 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2711,19 +2711,19 @@ void pci_pm_init(struct pci_dev *dev)
> dev->d2_support = true;
>
> if (dev->d1_support || dev->d2_support)
> - pci_printk(KERN_DEBUG, dev, "supports%s%s\n",
> - dev->d1_support ? " D1" : "",
> - dev->d2_support ? " D2" : "");
> + pci_dbg(dev, "supports%s%s\n",
> + dev->d1_support ? " D1" : "",
> + dev->d2_support ? " D2" : "");
> }
>
> pmc &= PCI_PM_CAP_PME_MASK;
> if (pmc) {
> - pci_printk(KERN_DEBUG, dev, "PME# supported from%s%s%s%s%s\n",
> - (pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
> - (pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
> - (pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
> - (pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
> - (pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
> + pci_dbg(dev, "PME# supported from%s%s%s%s%s\n",
> + (pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
> + (pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
> + (pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
> + (pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
> + (pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
> dev->pme_support = pmc >> PCI_PM_CAP_PME_SHIFT;
> dev->pme_poll = true;
> /*
> @@ -2886,18 +2886,17 @@ static int pci_ea_read(struct pci_dev *dev, int offset)
> res->flags = flags;
>
> if (bei <= PCI_EA_BEI_BAR5)
> - pci_printk(KERN_DEBUG, dev, "BAR %d: %pR (from Enhanced Allocation, properties %#02x)\n",
> - bei, res, prop);
> + pci_dbg(dev, "BAR %d: %pR (from Enhanced Allocation, properties %#02x)\n",
> + bei, res, prop);
> else if (bei == PCI_EA_BEI_ROM)
> - pci_printk(KERN_DEBUG, dev, "ROM: %pR (from Enhanced Allocation, properties %#02x)\n",
> - res, prop);
> + pci_dbg(dev, "ROM: %pR (from Enhanced Allocation, properties %#02x)\n",
> + res, prop);
> else if (bei >= PCI_EA_BEI_VF_BAR0 && bei <= PCI_EA_BEI_VF_BAR5)
> - pci_printk(KERN_DEBUG, dev, "VF BAR %d: %pR (from Enhanced Allocation, properties %#02x)\n",
> - bei - PCI_EA_BEI_VF_BAR0, res, prop);
> + pci_dbg(dev, "VF BAR %d: %pR (from Enhanced Allocation, properties %#02x)\n",
> + bei - PCI_EA_BEI_VF_BAR0, res, prop);
> else
> - pci_printk(KERN_DEBUG, dev, "BEI %d res: %pR (from Enhanced Allocation, properties %#02x)\n",
> - bei, res, prop);
> -
> + pci_dbg(dev, "BEI %d res: %pR (from Enhanced Allocation, properties %#02x)\n",
> + bei, res, prop);
> out:
> return offset + ent_size;
> }
> @@ -4114,8 +4113,8 @@ int pci_set_cacheline_size(struct pci_dev *dev)
> if (cacheline_size == pci_cache_line_size)
> return 0;
>
> - pci_printk(KERN_DEBUG, dev, "cache line size of %d is not supported\n",
> - pci_cache_line_size << 2);
> + pci_dbg(dev, "cache line size of %d is not supported\n",
> + pci_cache_line_size << 2);
>
> return -EINVAL;
> }
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index b1c05b505..bd986a1c1 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -317,7 +317,7 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
> res->flags = 0;
> out:
> if (res->flags)
> - pci_printk(KERN_DEBUG, dev, "reg 0x%x: %pR\n", pos, res);
> + pci_dbg(dev, "reg 0x%x: %pR\n", pos, res);
>
> return (res->flags & IORESOURCE_MEM_64) ? 1 : 0;
> }
> @@ -384,7 +384,7 @@ static void pci_read_bridge_io(struct pci_bus *child)
> region.start = base;
> region.end = limit + io_granularity - 1;
> pcibios_bus_to_resource(dev->bus, res, ®ion);
> - pci_printk(KERN_DEBUG, dev, " bridge window %pR\n", res);
> + pci_dbg(dev, " bridge window %pR\n", res);
> }
> }
>
> @@ -406,7 +406,7 @@ static void pci_read_bridge_mmio(struct pci_bus *child)
> region.start = base;
> region.end = limit + 0xfffff;
> pcibios_bus_to_resource(dev->bus, res, ®ion);
> - pci_printk(KERN_DEBUG, dev, " bridge window %pR\n", res);
> + pci_dbg(dev, " bridge window %pR\n", res);
> }
> }
>
> @@ -459,7 +459,7 @@ static void pci_read_bridge_mmio_pref(struct pci_bus *child)
> region.start = base;
> region.end = limit + 0xfffff;
> pcibios_bus_to_resource(dev->bus, res, ®ion);
> - pci_printk(KERN_DEBUG, dev, " bridge window %pR\n", res);
> + pci_dbg(dev, " bridge window %pR\n", res);
> }
> }
>
> @@ -489,9 +489,8 @@ void pci_read_bridge_bases(struct pci_bus *child)
> if (res && res->flags) {
> pci_bus_add_resource(child, res,
> PCI_SUBTRACTIVE_DECODE);
> - pci_printk(KERN_DEBUG, dev,
> - " bridge window %pR (subtractive decode)\n",
> - res);
> + pci_dbg(dev, " bridge window %pR (subtractive decode)\n",
> + res);
> }
> }
> }
> @@ -1626,8 +1625,8 @@ int pci_setup_device(struct pci_dev *dev)
> dev->revision = class & 0xff;
> dev->class = class >> 8; /* upper 3 bytes */
>
> - pci_printk(KERN_DEBUG, dev, "[%04x:%04x] type %02x class %#08x\n",
> - dev->vendor, dev->device, dev->hdr_type, dev->class);
> + pci_dbg(dev, "[%04x:%04x] type %02x class %#08x\n",
> + dev->vendor, dev->device, dev->hdr_type, dev->class);
>
> if (pci_early_dump)
> early_dump_pci_device(dev);
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index ed960436d..34c1d0e84 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -255,10 +255,10 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
> (IORESOURCE_STARTALIGN|IORESOURCE_SIZEALIGN);
> if (pci_reassign_resource(add_res->dev, idx,
> add_size, align))
> - pci_printk(KERN_DEBUG, add_res->dev,
> - "failed to add %llx res[%d]=%pR\n",
> - (unsigned long long)add_size,
> - idx, res);
> + pci_dbg(add_res->dev,
> + "failed to add %llx res[%d]=%pR\n",
> + (unsigned long long)add_size,
> + idx, res);
> }
> out:
> list_del(&add_res->list);
> @@ -955,9 +955,9 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
> if (size1 > size0 && realloc_head) {
> add_to_list(realloc_head, bus->self, b_res, size1-size0,
> min_align);
> - pci_printk(KERN_DEBUG, bus->self, "bridge window %pR to %pR add_size %llx\n",
> - b_res, &bus->busn_res,
> - (unsigned long long)size1-size0);
> + pci_dbg(bus->self, "bridge window %pR to %pR add_size %llx\n",
> + b_res, &bus->busn_res,
> + (unsigned long long)size1-size0);
> }
> }
>
> @@ -1100,10 +1100,10 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
> b_res->flags |= IORESOURCE_STARTALIGN;
> if (size1 > size0 && realloc_head) {
> add_to_list(realloc_head, bus->self, b_res, size1-size0, add_align);
> - pci_printk(KERN_DEBUG, bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
> - b_res, &bus->busn_res,
> - (unsigned long long) (size1 - size0),
> - (unsigned long long) add_align);
> + pci_dbg(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
> + b_res, &bus->busn_res,
> + (unsigned long long) (size1 - size0),
> + (unsigned long long) add_align);
> }
> return 0;
> }
> @@ -1568,8 +1568,8 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
> release_child_resources(r);
> if (!release_resource(r)) {
> type = old_flags = r->flags & PCI_RES_TYPE_MASK;
> - pci_printk(KERN_DEBUG, dev, "resource %d %pR released\n",
> - PCI_BRIDGE_RESOURCES + idx, r);
> + pci_dbg(dev, "resource %d %pR released\n",
> + PCI_BRIDGE_RESOURCES + idx, r);
> /* keep the old size */
> r->end = resource_size(r) - 1;
> r->start = 0;
> --
> 2.19.1
>
>
