| Message ID | 20190825182557.23260-1-kw@linux.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | x86/PCI: Add missing log facility and move to use pr_ macros in pcbios.c | expand |
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 >
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
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);
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(-)