All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Mohan Kumar <mohankumar718@gmail.com>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drivers: pci: This patch fix the following checkpatch warning.
Date: Wed, 17 Apr 2019 16:35:42 -0500	[thread overview]
Message-ID: <20190417213542.GU126710@google.com> (raw)
In-Reply-To: <1555433020-3830-1-git-send-email-mohankumar718@gmail.com>

Hi Mohan,

On Tue, Apr 16, 2019 at 07:43:40PM +0300, Mohan Kumar wrote:
> Use pr_err instead of printk

I don't mind taking patches like these, but they do tend to clutter
the revision history if we're not careful.  So here are some hints:

  - Make the subject line useful.  The one here really doesn't tell me
    anything.  Run "git log --oneline drivers/pci/bus.c" and follow
    the conventions there.  Something like:

      PCI: Convert printk(KERN_*) to pr_*()

  - Do all the similar changes at once.  I notice similar things in
    these files:

      drivers/pci/pci-acpi.c
      drivers/pci/pci-stub.c
      drivers/pci/pci-sysfs.c
      drivers/pci/pci.c
      drivers/pci/pcie/aspm.c
      drivers/pci/quirks.c
      drivers/pci/setup-bus.c
      drivers/pci/slot.c

    plus several in drivers/pci/hotplug/.  These could all be done in
    a single patch.

  - If there are several in a file that use the same prefix, add a
    "#define pr_fmt()".  This should be a separate patch so each patch
    does only one type of change, which makes them easier to review.

  - There are several uses of "printk(KERN_DEBUG)",
    "dev_printk(KERN_DEBUG)", and "pci_printk(KERN_DEBUG)".  The
    obvious thing would be to convert those to pr_debug(), dev_dbg(),
    and pci_dbg(), but I don't like that approach because pr_debug()
    and friends do different things depending on how the kernel is
    built.  The uses in PCI provide information that I want to appear
    in the dmesg log *always*.  So they could be (a) left alone or (b)
    converted from KERN_DEBUG to KERN_INFO (i.e., converted to
    pr_info(), dev_info(), etc).

> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ...
> then dev_err(dev, ... then pr_err(...  to printk(KERN_ERR ...
> 
> Signed-off-by: Mohan Kumar <mohankumar718@gmail.com>
> ---
>  drivers/pci/bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 5cb40b2..2179a8b 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -23,7 +23,7 @@ void pci_add_resource_offset(struct list_head *resources, struct resource *res,
>  
>  	entry = resource_list_create_entry(res, 0);
>  	if (!entry) {
> -		printk(KERN_ERR "PCI: can't add host bridge window %pR\n", res);
> +		pr_err("PCI: can't add host bridge window %pR\n", res);
>  		return;
>  	}
>  
> -- 
> 2.7.4
> 

      reply	other threads:[~2019-04-17 21:35 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-16 16:43 [PATCH] drivers: pci: This patch fix the following checkpatch warning Mohan Kumar
2019-04-17 21:35 ` 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=20190417213542.GU126710@google.com \
    --to=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mohankumar718@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 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.