All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH] PCI: switch remaining pci_printk(KERN_DEBUG users to pci_dbg
Date: Wed, 30 Jan 2019 13:29:31 -0600	[thread overview]
Message-ID: <20190130192931.GJ229773@google.com> (raw)
In-Reply-To: <44473b8c-c9d3-733c-a4b1-18904f4d62b2@gmail.com>

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@gmail.com>
> ---
>  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, &region);
> -		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, &region);
> -		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, &region);
> -		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
> 
> 

      reply	other threads:[~2019-01-30 19:29 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-21 15:20 [PATCH] PCI: switch remaining pci_printk(KERN_DEBUG users to pci_dbg Heiner Kallweit
2019-01-30 19:29 ` Bjorn Helgaas [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190130192931.GJ229773@google.com \
    --to=helgaas@kernel.org \
    --cc=hkallweit1@gmail.com \
    --cc=linux-pci@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.