From: Krzysztof Wilczynski <kswilczynski@gmail.com>
To: Joe Perches <joe@perches.com>
Cc: Krzysztof Wilczynski <kw@linux.com>,
Bjorn Helgaas <helgaas@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"H. Peter Anvin" <hpa@zytor.com>,
x86@kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] x86/PCI: Add missing log facility and move to use pr_ macros in pcbios.c
Date: Wed, 28 Aug 2019 20:40:27 +0200 [thread overview]
Message-ID: <1567017627.3507.0@gmail.com> (raw)
In-Reply-To: <a13a086c2dd6dd6259d28e5d1d360e2b4d04ca83.camel@perches.com>
Hello Joe,
Thank you for feedback.
[...]
>> Move to pr_debug() over using DBG() from
>> arch/x86/include/asm/pci_x86.h.
>
> You might also consider the checkpatch output for this patch.
>
> arch/x86/pci/pcbios.c:116: WARNING: line over 80 characters
> arch/x86/pci/pcbios.c:116: WARNING: Prefer using '"%s...", __func__'
> to using 'bios32_service', this function's name, in a string
> arch/x86/pci/pcbios.c:119: WARNING: Prefer using '"%s...", __func__'
> to using 'bios32_service', this function's name, in a string
> arch/x86/pci/pcbios.c:391: WARNING: line over 80 characters
Good point.
The lines over 80 characters wide would be taken care of when
moving to using the pr_ macros as the line length will now be
shorter contrary to when the e.g., printk(KERNEL_INFO ...),
etc., was used.
The other warnings I am going to address in v3. I was thinking
of replacing the following:
pr_warn("bios32_service(0x%lx): not present\n", service);
With something that looks like this:
pr_warn("BIOS32 Service(0x%lx): not present\n", service);
Using "bios32_service" name directly or even moving to __func__
feels a lot like an implementation detail is exposed to the
end user. I am not sure how useful that could be. Also,
we are already using log lines starting with "BIOS32", thus
it seemed like following them would be the most sensible
choice, especially to keep messages consistent.
What do you think?
Krzysztof
next prev parent reply other threads:[~2019-08-28 18:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-25 18:25 [PATCH] x86/PCI: Add missing log facility and move to use pr_ macros in pcbios.c Krzysztof Wilczynski
2019-08-27 22:47 ` Bjorn Helgaas
2019-08-28 10:59 ` Krzysztof Wilczynski
2019-08-28 17:51 ` [PATCH v2] " Krzysztof Wilczynski
2019-08-28 18:02 ` Joe Perches
2019-08-28 18:40 ` Krzysztof Wilczynski [this message]
2019-08-28 18:43 ` Joe Perches
2019-08-28 18:58 ` Krzysztof Wilczynski
2019-08-29 17:11 ` [PATCH v3] x86/PCI: Add missing log facility and move to use pr_ macros Krzysztof Wilczynski
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=1567017627.3507.0@gmail.com \
--to=kswilczynski@gmail.com \
--cc=bp@alien8.de \
--cc=helgaas@kernel.org \
--cc=hpa@zytor.com \
--cc=joe@perches.com \
--cc=kw@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox