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

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

[Krzysztof Wilczynski]

Make the log facility used to print warnings to be KERN_WARNING
explicitly, rather than rely on the current (or default) value
of the MESSAGE_LOGLEVEL_DEFAULT set in Kconfig.  This will make
all the warnings in the arch/x86/pci/pcbios.c to be printed
consistently at the same log facility.

Replace printk(KERN_<level> ...) with corresponding pr_ macros,
while adding the missing log facility.

Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
---
 arch/x86/pci/pcbios.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/pci/pcbios.c b/arch/x86/pci/pcbios.c
index 9c97d814125e..0c3673f50bce 100644
--- a/arch/x86/pci/pcbios.c
+++ b/arch/x86/pci/pcbios.c
@@ -47,7 +47,7 @@ static inline void set_bios_x(void)
 	pcibios_enabled = 1;
 	set_memory_x(PAGE_OFFSET + BIOS_BEGIN, (BIOS_END - BIOS_BEGIN) >> PAGE_SHIFT);
 	if (__supported_pte_mask & _PAGE_NX)
-		printk(KERN_INFO "PCI: PCI BIOS area is rw and x. Use pci=nobios if you want it NX.\n");
+		pr_info("PCI: PCI BIOS area is rw and x. Use pci=nobios if you want it NX.\n");
 }
 
 /*
@@ -111,10 +111,10 @@ static unsigned long __init bios32_service(unsigned long service)
 		case 0:
 			return address + entry;
 		case 0x80:	/* Not present */
-			printk(KERN_WARNING "bios32_service(0x%lx): not present\n", service);
+			pr_warn("bios32_service(0x%lx): not present\n", service);
 			return 0;
 		default: /* Shouldn't happen */
-			printk(KERN_WARNING "bios32_service(0x%lx): returned 0x%x -- BIOS bug!\n",
+			pr_warn("bios32_service(0x%lx): returned 0x%x -- BIOS bug!\n",
 				service, return_code);
 			return 0;
 	}
@@ -163,11 +163,11 @@ static int __init check_pcibios(void)
 		DBG("PCI: BIOS probe returned s=%02x hw=%02x ver=%02x.%02x l=%02x\n",
 			status, hw_mech, major_ver, minor_ver, pcibios_last_bus);
 		if (status || signature != PCI_SIGNATURE) {
-			printk (KERN_ERR "PCI: BIOS BUG #%x[%08x] found\n",
+			pr_err("PCI: BIOS BUG #%x[%08x] found\n",
 				status, signature);
 			return 0;
 		}
-		printk(KERN_INFO "PCI: PCI BIOS revision %x.%02x entry at 0x%lx, last bus=%d\n",
+		pr_info("PCI: PCI BIOS revision %x.%02x entry at 0x%lx, last bus=%d\n",
 			major_ver, minor_ver, pcibios_entry, pcibios_last_bus);
 #ifdef CONFIG_PCI_DIRECT
 		if (!(hw_mech & PCIBIOS_HW_TYPE1))
@@ -316,13 +316,13 @@ static const struct pci_raw_ops *__init pci_find_bios(void)
 		if (sum != 0)
 			continue;
 		if (check->fields.revision != 0) {
-			printk("PCI: unsupported BIOS32 revision %d at 0x%p\n",
+			pr_warn("PCI: unsupported BIOS32 revision %d at 0x%p\n",
 				check->fields.revision, check);
 			continue;
 		}
 		DBG("PCI: BIOS32 Service Directory structure at 0x%p\n", check);
 		if (check->fields.entry >= 0x100000) {
-			printk("PCI: BIOS32 entry (0x%p) in high memory, "
+			pr_warn("PCI: BIOS32 entry (0x%p) in high memory, "
 					"cannot use.\n", check);
 			return NULL;
 		} else {
@@ -386,7 +386,7 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
 		: "memory");
 	DBG("OK  ret=%d, size=%d, map=%x\n", ret, opt.size, map);
 	if (ret & 0xff00)
-		printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
+		pr_err("PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
 	else if (opt.size) {
 		rt = kmalloc(sizeof(struct irq_routing_table) + opt.size, GFP_KERNEL);
 		if (rt) {
@@ -394,7 +394,7 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
 			rt->size = opt.size + sizeof(struct irq_routing_table);
 			rt->exclusive_irqs = map;
 			memcpy(rt->slots, (void *) page, opt.size);
-			printk(KERN_INFO "PCI: Using BIOS Interrupt Routing Table\n");
+			pr_info("PCI: Using BIOS Interrupt Routing Table\n");
 		}
 	}
 	free_page(page);
-- 
2.22.1

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

[Bjorn Helgaas]

On Sun, Aug 25, 2019 at 08:25:57PM +0200, Krzysztof Wilczynski wrote:
> Make the log facility used to print warnings to be KERN_WARNING
> explicitly, rather than rely on the current (or default) value
> of the MESSAGE_LOGLEVEL_DEFAULT set in Kconfig.  This will make
> all the warnings in the arch/x86/pci/pcbios.c to be printed
> consistently at the same log facility.

This is slightly confusing.  There are only two messages that didn't
supply a log level, so the avoidance of MESSAGE_LOGLEVEL_DEFAULT
applies to those.

The rest already supplied a log level, so converting printk(KERN_INFO)
to pr_info() is purely simplification.

> Replace printk(KERN_<level> ...) with corresponding pr_ macros,
> while adding the missing log facility.

Might be worth doing this as well:

  #define pr_fmt(fmt) "PCI: " fmt

and removing the "PCI: " prefix from the messages.  This would change
the "bios32_service" output slightly, but I think the change would be
a good one.

> Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
> ---
>  arch/x86/pci/pcbios.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/pci/pcbios.c b/arch/x86/pci/pcbios.c
> index 9c97d814125e..0c3673f50bce 100644
> --- a/arch/x86/pci/pcbios.c
> +++ b/arch/x86/pci/pcbios.c
> @@ -47,7 +47,7 @@ static inline void set_bios_x(void)
>  	pcibios_enabled = 1;
>  	set_memory_x(PAGE_OFFSET + BIOS_BEGIN, (BIOS_END - BIOS_BEGIN) >> PAGE_SHIFT);
>  	if (__supported_pte_mask & _PAGE_NX)
> -		printk(KERN_INFO "PCI: PCI BIOS area is rw and x. Use pci=nobios if you want it NX.\n");
> +		pr_info("PCI: PCI BIOS area is rw and x. Use pci=nobios if you want it NX.\n");
>  }
>  
>  /*
> @@ -111,10 +111,10 @@ static unsigned long __init bios32_service(unsigned long service)
>  		case 0:
>  			return address + entry;
>  		case 0x80:	/* Not present */
> -			printk(KERN_WARNING "bios32_service(0x%lx): not present\n", service);
> +			pr_warn("bios32_service(0x%lx): not present\n", service);
>  			return 0;
>  		default: /* Shouldn't happen */
> -			printk(KERN_WARNING "bios32_service(0x%lx): returned 0x%x -- BIOS bug!\n",
> +			pr_warn("bios32_service(0x%lx): returned 0x%x -- BIOS bug!\n",
>  				service, return_code);
>  			return 0;
>  	}
> @@ -163,11 +163,11 @@ static int __init check_pcibios(void)
>  		DBG("PCI: BIOS probe returned s=%02x hw=%02x ver=%02x.%02x l=%02x\n",
>  			status, hw_mech, major_ver, minor_ver, pcibios_last_bus);
>  		if (status || signature != PCI_SIGNATURE) {
> -			printk (KERN_ERR "PCI: BIOS BUG #%x[%08x] found\n",
> +			pr_err("PCI: BIOS BUG #%x[%08x] found\n",
>  				status, signature);
>  			return 0;
>  		}
> -		printk(KERN_INFO "PCI: PCI BIOS revision %x.%02x entry at 0x%lx, last bus=%d\n",
> +		pr_info("PCI: PCI BIOS revision %x.%02x entry at 0x%lx, last bus=%d\n",
>  			major_ver, minor_ver, pcibios_entry, pcibios_last_bus);
>  #ifdef CONFIG_PCI_DIRECT
>  		if (!(hw_mech & PCIBIOS_HW_TYPE1))
> @@ -316,13 +316,13 @@ static const struct pci_raw_ops *__init pci_find_bios(void)
>  		if (sum != 0)
>  			continue;
>  		if (check->fields.revision != 0) {
> -			printk("PCI: unsupported BIOS32 revision %d at 0x%p\n",
> +			pr_warn("PCI: unsupported BIOS32 revision %d at 0x%p\n",
>  				check->fields.revision, check);
>  			continue;
>  		}
>  		DBG("PCI: BIOS32 Service Directory structure at 0x%p\n", check);
>  		if (check->fields.entry >= 0x100000) {
> -			printk("PCI: BIOS32 entry (0x%p) in high memory, "
> +			pr_warn("PCI: BIOS32 entry (0x%p) in high memory, "
>  					"cannot use.\n", check);
>  			return NULL;
>  		} else {
> @@ -386,7 +386,7 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
>  		: "memory");
>  	DBG("OK  ret=%d, size=%d, map=%x\n", ret, opt.size, map);
>  	if (ret & 0xff00)
> -		printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
> +		pr_err("PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
>  	else if (opt.size) {
>  		rt = kmalloc(sizeof(struct irq_routing_table) + opt.size, GFP_KERNEL);
>  		if (rt) {
> @@ -394,7 +394,7 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
>  			rt->size = opt.size + sizeof(struct irq_routing_table);
>  			rt->exclusive_irqs = map;
>  			memcpy(rt->slots, (void *) page, opt.size);
> -			printk(KERN_INFO "PCI: Using BIOS Interrupt Routing Table\n");
> +			pr_info("PCI: Using BIOS Interrupt Routing Table\n");
>  		}
>  	}
>  	free_page(page);
> -- 
> 2.22.1
> 

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

[Krzysztof Wilczynski]

Hello Bjorn,

Thank you for the feedback.
[...]
>>  Make the log facility used to print warnings to be KERN_WARNING
>>  explicitly, rather than rely on the current (or default) value
>>  of the MESSAGE_LOGLEVEL_DEFAULT set in Kconfig.  This will make
>>  all the warnings in the arch/x86/pci/pcbios.c to be printed
>>  consistently at the same log facility.
> 
> This is slightly confusing.  There are only two messages that didn't
> supply a log level, so the avoidance of MESSAGE_LOGLEVEL_DEFAULT
> applies to those.

Good point.  I will update both the wording and the explanation so that
it would be more accurate and make a whole lot more sense.

[...]
> Might be worth doing this as well:
> 
>   #define pr_fmt(fmt) "PCI: " fmt
> 
> and removing the "PCI: " prefix from the messages.  This would change
> the "bios32_service" output slightly, but I think the change would be
> a good one.

Will do. The v2 have all the improvements. Thank you!

Krzysztof

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

[Krzysztof Wilczynski]

Add missing log facility where two instances of printk() that did not
use any (so it would be using MESSAGE_LOGLEVEL_DEFAULT set in Kconfig)
to make all the warnings in the arch/x86/pci/pcbios.c to be printed
consistently at the same log facility.  Also resolve the following
checkpatch.pl script warning:

WARNING: printk() should include KERN_<LEVEL> facility level

While adding the missing log facility move over to using pr_ macros
over using printk(KERN_<level> ...) and DBG().

Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
---
Changes in v2:
  Change wording and include checkpatch.pl script warning.
  Leverage pr_fmt and remove "PCI: " prefix used throught.
  Move to pr_debug() over using DBG() from arch/x86/include/asm/pci_x86.h.

 arch/x86/pci/pcbios.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/x86/pci/pcbios.c b/arch/x86/pci/pcbios.c
index 9c97d814125e..ddcefb25c55e 100644
--- a/arch/x86/pci/pcbios.c
+++ b/arch/x86/pci/pcbios.c
@@ -3,6 +3,8 @@
  * BIOS32 and PCI BIOS handling.
  */
 
+#define pr_fmt(fmt) "PCI: " fmt
+
 #include <linux/pci.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -47,7 +49,7 @@ static inline void set_bios_x(void)
 	pcibios_enabled = 1;
 	set_memory_x(PAGE_OFFSET + BIOS_BEGIN, (BIOS_END - BIOS_BEGIN) >> PAGE_SHIFT);
 	if (__supported_pte_mask & _PAGE_NX)
-		printk(KERN_INFO "PCI: PCI BIOS area is rw and x. Use pci=nobios if you want it NX.\n");
+		pr_info("PCI BIOS area is rw and x. Use pci=nobios if you want it NX.\n");
 }
 
 /*
@@ -111,10 +113,10 @@ static unsigned long __init bios32_service(unsigned long service)
 		case 0:
 			return address + entry;
 		case 0x80:	/* Not present */
-			printk(KERN_WARNING "bios32_service(0x%lx): not present\n", service);
+			pr_warn("bios32_service(0x%lx): not present\n", service);
 			return 0;
 		default: /* Shouldn't happen */
-			printk(KERN_WARNING "bios32_service(0x%lx): returned 0x%x -- BIOS bug!\n",
+			pr_warn("bios32_service(0x%lx): returned 0x%x -- BIOS bug!\n",
 				service, return_code);
 			return 0;
 	}
@@ -160,14 +162,14 @@ static int __init check_pcibios(void)
 		minor_ver = ebx & 0xff;
 		if (pcibios_last_bus < 0)
 			pcibios_last_bus = ecx & 0xff;
-		DBG("PCI: BIOS probe returned s=%02x hw=%02x ver=%02x.%02x l=%02x\n",
+		pr_debug("BIOS probe returned s=%02x hw=%02x ver=%02x.%02x l=%02x\n",
 			status, hw_mech, major_ver, minor_ver, pcibios_last_bus);
 		if (status || signature != PCI_SIGNATURE) {
-			printk (KERN_ERR "PCI: BIOS BUG #%x[%08x] found\n",
+			pr_err("BIOS BUG #%x[%08x] found\n",
 				status, signature);
 			return 0;
 		}
-		printk(KERN_INFO "PCI: PCI BIOS revision %x.%02x entry at 0x%lx, last bus=%d\n",
+		pr_info("PCI BIOS revision %x.%02x entry at 0x%lx, last bus=%d\n",
 			major_ver, minor_ver, pcibios_entry, pcibios_last_bus);
 #ifdef CONFIG_PCI_DIRECT
 		if (!(hw_mech & PCIBIOS_HW_TYPE1))
@@ -316,18 +318,18 @@ static const struct pci_raw_ops *__init pci_find_bios(void)
 		if (sum != 0)
 			continue;
 		if (check->fields.revision != 0) {
-			printk("PCI: unsupported BIOS32 revision %d at 0x%p\n",
+			pr_warn("unsupported BIOS32 revision %d at 0x%p\n",
 				check->fields.revision, check);
 			continue;
 		}
-		DBG("PCI: BIOS32 Service Directory structure at 0x%p\n", check);
+		pr_debug("BIOS32 Service Directory structure at 0x%p\n", check);
 		if (check->fields.entry >= 0x100000) {
-			printk("PCI: BIOS32 entry (0x%p) in high memory, "
+			pr_warn("BIOS32 entry (0x%p) in high memory, "
 					"cannot use.\n", check);
 			return NULL;
 		} else {
 			unsigned long bios32_entry = check->fields.entry;
-			DBG("PCI: BIOS32 Service Directory entry at 0x%lx\n",
+			pr_debug("BIOS32 Service Directory entry at 0x%lx\n",
 					bios32_entry);
 			bios32_indirect.address = bios32_entry + PAGE_OFFSET;
 			set_bios_x();
@@ -366,7 +368,7 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
 	opt.size = PAGE_SIZE;
 	opt.segment = __KERNEL_DS;
 
-	DBG("PCI: Fetching IRQ routing table... ");
+	pr_debug("Fetching IRQ routing table... ");
 	__asm__("push %%es\n\t"
 		"push %%ds\n\t"
 		"pop  %%es\n\t"
@@ -384,9 +386,9 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
 		  "S" (&pci_indirect),
 		  "m" (opt)
 		: "memory");
-	DBG("OK  ret=%d, size=%d, map=%x\n", ret, opt.size, map);
+	pr_debug("OK  ret=%d, size=%d, map=%x\n", ret, opt.size, map);
 	if (ret & 0xff00)
-		printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
+		pr_err("Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
 	else if (opt.size) {
 		rt = kmalloc(sizeof(struct irq_routing_table) + opt.size, GFP_KERNEL);
 		if (rt) {
@@ -394,7 +396,7 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
 			rt->size = opt.size + sizeof(struct irq_routing_table);
 			rt->exclusive_irqs = map;
 			memcpy(rt->slots, (void *) page, opt.size);
-			printk(KERN_INFO "PCI: Using BIOS Interrupt Routing Table\n");
+			pr_info("Using BIOS Interrupt Routing Table\n");
 		}
 	}
 	free_page(page);
-- 
2.22.1

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

[Joe Perches]

On Wed, 2019-08-28 at 19:51 +0200, Krzysztof Wilczynski wrote:
> Add missing log facility where two instances of printk() that did not
> use any (so it would be using MESSAGE_LOGLEVEL_DEFAULT set in Kconfig)
> to make all the warnings in the arch/x86/pci/pcbios.c to be printed
> consistently at the same log facility.  Also resolve the following
> checkpatch.pl script warning:
> 
> WARNING: printk() should include KERN_<LEVEL> facility level
> 
> While adding the missing log facility move over to using pr_ macros
> over using printk(KERN_<level> ...) and DBG().
> 
> Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
> ---
> Changes in v2:
>   Change wording and include checkpatch.pl script warning.
>   Leverage pr_fmt and remove "PCI: " prefix used throught.
>   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

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

[Krzysztof Wilczynski]

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

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

[Joe Perches]

On Wed, 2019-08-28 at 20:40 +0200, Krzysztof Wilczynski wrote:
> 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.

Not really, those were the warnings checkpatch
emits on your actual patch.

> 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?

Fine with me, your patch, your choices.

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

[Krzysztof Wilczynski]

Hello Joe,
[...]
>>  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.
> 
> Not really, those were the warnings checkpatch
> emits on your actual patch.

Ah! Yes, very true.  Sorry about that.  I will address these in v3,
of course.

Thank you for correcting me!

Krzysztof

[PATCH v3] x86/PCI: Add missing log facility and move to use pr_ macros

[Krzysztof Wilczynski]

Add missing log facility where two instances of printk() that did not
use any (so it would be using MESSAGE_LOGLEVEL_DEFAULT set in Kconfig)
to make all the warnings in the arch/x86/pci/pcbios.c to be printed
consistently at the same log facility.  This resolves the following
checkpatch.pl script warning:

WARNING: printk() should include KERN_<LEVEL> facility level

While adding the missing log facility move over to using pr_ macros
over using printk(KERN_<level> ...) and DBG().

Also resolve the additional errors and warnings reported by the
checkpatch.pl script:

ERROR: trailing whitespace
ERROR: "foo * bar" should be "foo *bar"
ERROR: switch and case should be at the same indent

WARNING: please, no space before tabs
WARNING: line over 80 characters
WARNING: quoted string split across lines
WARNING: __packed is preferred over __attribute__((packed))
WARNING: Prefer using '"%s...", __func__' to using 'bios32_service',
  this function's name, in a string

Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
---
Changes in v3:
  Remove name of the file from the subject.
  Address review feedback of v2, and resolve more checkpatch.pl
  script errors and warnings.

Changes in v2:
  Change wording and include checkpatch.pl script warning.
  Leverage pr_fmt and remove "PCI: " prefix used throught.
  Move to pr_debug() over using DBG() from arch/x86/include/asm/pci_x86.h.

 arch/x86/pci/pcbios.c | 77 +++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 36 deletions(-)

diff --git a/arch/x86/pci/pcbios.c b/arch/x86/pci/pcbios.c
index 9c97d814125e..dd8ca5636953 100644
--- a/arch/x86/pci/pcbios.c
+++ b/arch/x86/pci/pcbios.c
@@ -3,6 +3,8 @@
  * BIOS32 and PCI BIOS handling.
  */
 
+#define pr_fmt(fmt) "PCI: " fmt
+
 #include <linux/pci.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -47,15 +49,15 @@ static inline void set_bios_x(void)
 	pcibios_enabled = 1;
 	set_memory_x(PAGE_OFFSET + BIOS_BEGIN, (BIOS_END - BIOS_BEGIN) >> PAGE_SHIFT);
 	if (__supported_pte_mask & _PAGE_NX)
-		printk(KERN_INFO "PCI: PCI BIOS area is rw and x. Use pci=nobios if you want it NX.\n");
+		pr_info("PCI BIOS area is rw and x. Use pci=nobios if you want it NX.\n");
 }
 
 /*
  * This is the standard structure used to identify the entry point
  * to the BIOS32 Service Directory, as documented in
- * 	Standard BIOS 32-bit Service Directory Proposal
- * 	Revision 0.4 May 24, 1993
- * 	Phoenix Technologies Ltd.
+ *	Standard BIOS 32-bit Service Directory Proposal
+ *	Revision 0.4 May 24, 1993
+ *	Phoenix Technologies Ltd.
  *	Norwood, MA
  * and the PCI BIOS specification.
  */
@@ -67,7 +69,7 @@ union bios32 {
 		unsigned char revision;		/* Revision level, 0 */
 		unsigned char length;		/* Length in paragraphs should be 01 */
 		unsigned char checksum;		/* All bytes must add up to zero */
-		unsigned char reserved[5]; 	/* Must be zero */
+		unsigned char reserved[5];	/* Must be zero */
 	} fields;
 	char chars[16];
 };
@@ -108,15 +110,16 @@ static unsigned long __init bios32_service(unsigned long service)
 	local_irq_restore(flags);
 
 	switch (return_code) {
-		case 0:
-			return address + entry;
-		case 0x80:	/* Not present */
-			printk(KERN_WARNING "bios32_service(0x%lx): not present\n", service);
-			return 0;
-		default: /* Shouldn't happen */
-			printk(KERN_WARNING "bios32_service(0x%lx): returned 0x%x -- BIOS bug!\n",
-				service, return_code);
-			return 0;
+	case 0:
+		return address + entry;
+	case 0x80:	/* Not present */
+		pr_warn("%s(0x%lx): not present\n",
+			__func__, service);
+		return 0;
+	default: /* Shouldn't happen */
+		pr_warn("%s(0x%lx): returned 0x%x -- BIOS bug!\n",
+			__func__, service, return_code);
+		return 0;
 	}
 }
 
@@ -140,8 +143,7 @@ static int __init check_pcibios(void)
 		pci_indirect.address = pcibios_entry + PAGE_OFFSET;
 
 		local_irq_save(flags);
-		__asm__(
-			"lcall *(%%edi); cld\n\t"
+		__asm__("lcall *(%%edi); cld\n\t"
 			"jc 1f\n\t"
 			"xor %%ah, %%ah\n"
 			"1:"
@@ -160,14 +162,15 @@ static int __init check_pcibios(void)
 		minor_ver = ebx & 0xff;
 		if (pcibios_last_bus < 0)
 			pcibios_last_bus = ecx & 0xff;
-		DBG("PCI: BIOS probe returned s=%02x hw=%02x ver=%02x.%02x l=%02x\n",
-			status, hw_mech, major_ver, minor_ver, pcibios_last_bus);
+		pr_debug("BIOS probe returned s=%02x hw=%02x ver=%02x.%02x l=%02x\n",
+			 status, hw_mech, major_ver, minor_ver,
+			 pcibios_last_bus);
 		if (status || signature != PCI_SIGNATURE) {
-			printk (KERN_ERR "PCI: BIOS BUG #%x[%08x] found\n",
-				status, signature);
+			pr_err("BIOS BUG #%x[%08x] found\n",
+			       status, signature);
 			return 0;
 		}
-		printk(KERN_INFO "PCI: PCI BIOS revision %x.%02x entry at 0x%lx, last bus=%d\n",
+		pr_info("PCI BIOS revision %x.%02x entry at 0x%lx, last bus=%d\n",
 			major_ver, minor_ver, pcibios_entry, pcibios_last_bus);
 #ifdef CONFIG_PCI_DIRECT
 		if (!(hw_mech & PCIBIOS_HW_TYPE1))
@@ -239,7 +242,7 @@ static int pci_bios_write(unsigned int seg, unsigned int bus,
 	u16 number = 0;
 
 	WARN_ON(seg);
-	if ((bus > 255) || (devfn > 255) || (reg > 255)) 
+	if ((bus > 255) || (devfn > 255) || (reg > 255))
 		return -EINVAL;
 
 	raw_spin_lock_irqsave(&pci_config_lock, flags);
@@ -316,19 +319,19 @@ static const struct pci_raw_ops *__init pci_find_bios(void)
 		if (sum != 0)
 			continue;
 		if (check->fields.revision != 0) {
-			printk("PCI: unsupported BIOS32 revision %d at 0x%p\n",
+			pr_warn("unsupported BIOS32 revision %d at 0x%p\n",
 				check->fields.revision, check);
 			continue;
 		}
-		DBG("PCI: BIOS32 Service Directory structure at 0x%p\n", check);
+		pr_debug("BIOS32 Service Directory structure at 0x%p\n", check);
 		if (check->fields.entry >= 0x100000) {
-			printk("PCI: BIOS32 entry (0x%p) in high memory, "
-					"cannot use.\n", check);
+			pr_warn("BIOS32 entry (0x%p) in high memory, cannot use.\n",
+				check);
 			return NULL;
 		} else {
 			unsigned long bios32_entry = check->fields.entry;
-			DBG("PCI: BIOS32 Service Directory entry at 0x%lx\n",
-					bios32_entry);
+			pr_debug("BIOS32 Service Directory entry at 0x%lx\n",
+				 bios32_entry);
 			bios32_indirect.address = bios32_entry + PAGE_OFFSET;
 			set_bios_x();
 			if (check_pcibios())
@@ -348,9 +351,9 @@ struct irq_routing_options {
 	u16 size;
 	struct irq_info *table;
 	u16 segment;
-} __attribute__((packed));
+} __packed;
 
-struct irq_routing_table * pcibios_get_irq_routing_table(void)
+struct irq_routing_table *pcibios_get_irq_routing_table(void)
 {
 	struct irq_routing_options opt;
 	struct irq_routing_table *rt = NULL;
@@ -366,7 +369,7 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
 	opt.size = PAGE_SIZE;
 	opt.segment = __KERNEL_DS;
 
-	DBG("PCI: Fetching IRQ routing table... ");
+	pr_debug("Fetching IRQ routing table... ");
 	__asm__("push %%es\n\t"
 		"push %%ds\n\t"
 		"pop  %%es\n\t"
@@ -384,17 +387,19 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
 		  "S" (&pci_indirect),
 		  "m" (opt)
 		: "memory");
-	DBG("OK  ret=%d, size=%d, map=%x\n", ret, opt.size, map);
+	pr_debug("OK  ret=%d, size=%d, map=%x\n", ret, opt.size, map);
 	if (ret & 0xff00)
-		printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
+		pr_err("Error %02x when fetching IRQ routing table.\n",
+		       (ret >> 8) & 0xff);
 	else if (opt.size) {
-		rt = kmalloc(sizeof(struct irq_routing_table) + opt.size, GFP_KERNEL);
+		rt = kmalloc(sizeof(struct irq_routing_table) + opt.size,
+			     GFP_KERNEL);
 		if (rt) {
 			memset(rt, 0, sizeof(struct irq_routing_table));
 			rt->size = opt.size + sizeof(struct irq_routing_table);
 			rt->exclusive_irqs = map;
 			memcpy(rt->slots, (void *) page, opt.size);
-			printk(KERN_INFO "PCI: Using BIOS Interrupt Routing Table\n");
+			pr_info("Using BIOS Interrupt Routing Table\n");
 		}
 	}
 	free_page(page);
@@ -421,7 +426,7 @@ EXPORT_SYMBOL(pcibios_set_irq_routing);
 
 void __init pci_pcbios_init(void)
 {
-	if ((pci_probe & PCI_PROBE_BIOS) 
+	if ((pci_probe & PCI_PROBE_BIOS)
 		&& ((raw_pci_ops = pci_find_bios()))) {
 		pci_bios_present = 1;
 	}
-- 
2.22.1