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
>
prev parent 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.