public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <bjorn.helgaas@hp.com>
To: "Vincent Legoll" <vincent.legoll@gmail.com>
Cc: "Jesse Barnes" <jbarnes@virtuousgeek.org>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PCI probing debug message uniformization
Date: Fri, 3 Oct 2008 12:57:42 -0600	[thread overview]
Message-ID: <200810031257.42457.bjorn.helgaas@hp.com> (raw)
In-Reply-To: <4727185d0810031113w2967fef7qa102d6ec4a89311e@mail.gmail.com>

On Friday 03 October 2008 12:13:14 pm Vincent Legoll wrote:
> On Fri, Oct 3, 2008 at 11:14 AM, Vincent Legoll
> <vincent.legoll@gmail.com> wrote:
> > On Thu, Oct 2, 2008 at 8:59 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> >> On Thursday 02 October 2008 12:46:28 pm Jesse Barnes wrote:
> >>> Bjorn, how does this jive with the various other debug harmonization patches
> >>> you've been putting together & reviewing?
> >>
> >> I think it's great.  The only nit I would change is to use
> >> "[%#llx-%#llx]" as we do in pci_request_region().
> >>
> >> Bjorn
> >
> > Thanks for the review, I'll post an updated version this evening.
> 
> 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)

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,
hence the screwed up formatting below.

> -                       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. It can be enabled" +                               "
> forcedly with 'pcie_aspm=force'\n");

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

  reply	other threads:[~2008-10-03 18:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-29 21:03 [PATCH] PCI probing debug message uniformization Vincent Legoll
2008-10-02 18:46 ` Jesse Barnes
2008-10-02 18:59   ` Bjorn Helgaas
2008-10-03  9:14     ` Vincent Legoll
2008-10-03 18:13       ` Vincent Legoll
2008-10-03 18:57         ` Bjorn Helgaas [this message]
2008-10-03 22:50           ` Vincent Legoll
2008-10-10 16:07             ` Jesse Barnes
2008-10-12 10:26               ` Vincent Legoll
2008-10-15 10:49                 ` Jesse Barnes

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=200810031257.42457.bjorn.helgaas@hp.com \
    --to=bjorn.helgaas@hp.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=vincent.legoll@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox