Re: [PATCH v2] x86/PCI: Add missing log facility and move to use pr_ macros in pcbios.c

From: Krzysztof Wilczynski
Date: Wed Aug 28 2019 - 14:40:35 EST


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