| Message ID | 20161117174758.16810.67625.stgit@bhelgaas-glaptop.roam.corp.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 17, 2016 at 11:47:58AM -0600, Bjorn Helgaas wrote: > Use dev_printk() when possible. This makes messages more consistent with > other device-related messages and, in some cases, adds useful information. > This changes messages like this: > > vgaarb: failed to allocate pci device > vgaarb: setting as boot device: PCI:0000:01:00.0 > vgaarb: device added: PCI:0000:01:00.0,decodes=io+mem,owns=io+mem,locks=none > vgaarb: bridge control possible 0000:01:00.0 > > to this: > > pci 0000:01:00.0: vgaarb: failed to allocate VGA arbiter data > pci 0000:01:00.0: vgaarb: setting as boot VGA device > pci 0000:01:00.0: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none > pci 0000:01:00.0: vgaarb: bridge control possible > > No functional change intended. Found one more nit below where there was one relevant change that shouldn't be here. -Daniel > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/gpu/vga/vgaarb.c | 66 ++++++++++++++++++++++++---------------------- > 1 file changed, 35 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > index 1887f19..3a8e9c6 100644 > --- a/drivers/gpu/vga/vgaarb.c > +++ b/drivers/gpu/vga/vgaarb.c > @@ -31,6 +31,10 @@ > > #define pr_fmt(fmt) "vgaarb: " fmt > > +#define vgaarb_dbg(dev, fmt, arg...) dev_dbg(dev, "vgaarb: " fmt, ##arg) > +#define vgaarb_info(dev, fmt, arg...) dev_info(dev, "vgaarb: " fmt, ##arg) > +#define vgaarb_err(dev, fmt, arg...) dev_err(dev, "vgaarb: " fmt, ##arg) > + > #include <linux/module.h> > #include <linux/kernel.h> > #include <linux/pci.h> > @@ -188,6 +192,7 @@ static void vga_check_first_use(void) > static struct vga_device *__vga_tryget(struct vga_device *vgadev, > unsigned int rsrc) > { > + struct device *dev = &vgadev->pdev->dev; > unsigned int wants, legacy_wants, match; > struct vga_device *conflict; > unsigned int pci_bits; > @@ -203,8 +208,8 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev, > (vgadev->decodes & VGA_RSRC_LEGACY_MEM)) > rsrc |= VGA_RSRC_LEGACY_MEM; > > - pr_debug("%s: %d\n", __func__, rsrc); > - pr_debug("%s: owns: %d\n", __func__, vgadev->owns); > + vgaarb_dbg(dev, "%s: %d\n", __func__, rsrc); > + vgaarb_dbg(dev, "%s: owns: %d\n", __func__, vgadev->owns); > > /* Check what resources we need to acquire */ > wants = rsrc & ~vgadev->owns; > @@ -336,9 +341,10 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev, > > static void __vga_put(struct vga_device *vgadev, unsigned int rsrc) > { > + struct device *dev = &vgadev->pdev->dev; > unsigned int old_locks = vgadev->locks; > > - pr_debug("%s\n", __func__); > + vgaarb_dbg(dev, "%s\n", __func__); > > /* Update our counters, and account for equivalent legacy resources > * if we decode them > @@ -611,7 +617,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) > /* Allocate structure */ > vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL); > if (vgadev == NULL) { > - pr_err("failed to allocate pci device\n"); > + vgaarb_err(&pdev->dev, "failed to allocate VGA arbiter data\n"); > /* > * What to do on allocation failure ? For now, let's just do > * nothing, I'm not sure there is anything saner to be done. > @@ -663,7 +669,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) > */ > if (vga_default == NULL && > ((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK)) { > - pr_info("setting as boot device: PCI:%s\n", pci_name(pdev)); > + vgaarb_info(&pdev->dev, "setting as boot VGA device\n"); > vga_set_default_device(pdev); > } > > @@ -672,8 +678,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) > /* Add to the list */ > list_add(&vgadev->list, &vga_list); > vga_count++; > - pr_info("device added: PCI:%s,decodes=%s,owns=%s,locks=%s\n", > - pci_name(pdev), > + vgaarb_info(&pdev->dev, "VGA device added: decodes=%s,owns=%s,locks=%s\n", > vga_iostate_to_str(vgadev->decodes), > vga_iostate_to_str(vgadev->owns), > vga_iostate_to_str(vgadev->locks)); > @@ -725,6 +730,7 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev) > static inline void vga_update_device_decodes(struct vga_device *vgadev, > int new_decodes) > { > + struct device *dev = &vgadev->pdev->dev; > int old_decodes, decodes_removed, decodes_unlocked; > > old_decodes = vgadev->decodes; > @@ -732,8 +738,7 @@ static inline void vga_update_device_decodes(struct vga_device *vgadev, > decodes_unlocked = vgadev->locks & decodes_removed; > vgadev->decodes = new_decodes; > > - pr_info("device changed decodes: PCI:%s,olddecodes=%s,decodes=%s:owns=%s\n", > - pci_name(vgadev->pdev), > + vgaarb_info(dev, "changed VGA decodes: olddecodes=%s,decodes=%s:owns=%s\n", > vga_iostate_to_str(old_decodes), > vga_iostate_to_str(vgadev->decodes), > vga_iostate_to_str(vgadev->owns)); > @@ -754,7 +759,7 @@ static inline void vga_update_device_decodes(struct vga_device *vgadev, > if (!(old_decodes & VGA_RSRC_LEGACY_MASK) && > new_decodes & VGA_RSRC_LEGACY_MASK) > vga_decode_count++; > - pr_debug("decoding count now is: %d\n", vga_decode_count); > + vgaarb_dbg(dev, "decoding count now is: %d\n", vga_decode_count); > } > > static void __vga_set_legacy_decoding(struct pci_dev *pdev, > @@ -1189,24 +1194,25 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, > ret_val = -EPROTO; > goto done; > } > - pr_debug("%s ==> %x:%x:%x.%x\n", curr_pos, > - domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); > - > pdev = pci_get_domain_bus_and_slot(domain, bus, devfn); > - pr_debug("pdev %p\n", pdev); > if (!pdev) { > - pr_err("invalid PCI address %x:%x:%x\n", > - domain, bus, devfn); > + pr_err("invalid PCI address %04x:%02x:%02x.%x\n", > + domain, bus, PCI_SLOT(devfn), > + PCI_FUNC(devfn)); Userspace-triggerable dmesg spam is imo not good, this needs to stay at debug level. > ret_val = -ENODEV; > goto done; > } > + > + pr_debug("%s ==> %04x:%02x:%02x.%x pdev %p\n", curr_pos, > + domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), > + pdev); > } > > vgadev = vgadev_find(pdev); > pr_debug("vgadev %p\n", vgadev); > if (vgadev == NULL) { > if (pdev) { > - pr_err("this pci device is not a vga device\n"); > + vgaarb_err(&pdev->dev, "not a VGA device\n"); > pci_dev_put(pdev); > } > > @@ -1226,7 +1232,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, > } > } > if (i == MAX_USER_CARDS) { > - pr_err("maximum user cards (%d) number reached!\n", > + vgaarb_err(&pdev->dev, "maximum user cards (%d) number reached, ignoring this one!\n", > MAX_USER_CARDS); > pci_dev_put(pdev); > /* XXX: which value to return? */ > @@ -1317,8 +1323,8 @@ static int vga_arb_release(struct inode *inode, struct file *file) > uc = &priv->cards[i]; > if (uc->pdev == NULL) > continue; > - pr_debug("uc->io_cnt == %d, uc->mem_cnt == %d\n", > - uc->io_cnt, uc->mem_cnt); > + vgaarb_dbg(&uc->pdev->dev, "uc->io_cnt == %d, uc->mem_cnt == %d\n", > + uc->io_cnt, uc->mem_cnt); > while (uc->io_cnt--) > vga_put(uc->pdev, VGA_RSRC_LEGACY_IO); > while (uc->mem_cnt--) > @@ -1371,7 +1377,7 @@ static int pci_notify(struct notifier_block *nb, unsigned long action, > struct pci_dev *pdev = to_pci_dev(dev); > bool notify = false; > > - pr_debug("%s\n", __func__); > + vgaarb_dbg(dev, "%s\n", __func__); > > /* For now we're only intereted in devices added and removed. I didn't > * test this thing here, so someone needs to double check for the > @@ -1408,6 +1414,7 @@ static int __init vga_arb_device_init(void) > int rc; > struct pci_dev *pdev; > struct vga_device *vgadev; > + struct device *dev; > > rc = misc_register(&vga_arb_device); > if (rc < 0) > @@ -1423,8 +1430,6 @@ static int __init vga_arb_device_init(void) > PCI_ANY_ID, pdev)) != NULL) > vga_arbiter_add_pci_device(pdev); > > - pr_info("loaded\n"); > - > list_for_each_entry(vgadev, &vga_list, list) { > #if defined(CONFIG_X86) || defined(CONFIG_IA64) > /* > @@ -1440,6 +1445,7 @@ static int __init vga_arb_device_init(void) > int i; > > limit = screen_info.lfb_base + screen_info.lfb_size; > + dev = &vgadev->pdev->dev; > > /* Does firmware framebuffer belong to us? */ > for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) { > @@ -1458,21 +1464,19 @@ static int __init vga_arb_device_init(void) > continue; > > if (!vga_default_device()) > - pr_info("setting as boot device: PCI:%s\n", > - pci_name(vgadev->pdev)); > + vgaarb_info(dev, "setting as boot device\n"); > else if (vgadev->pdev != vga_default_device()) > - pr_info("overriding boot device: PCI:%s\n", > - pci_name(vgadev->pdev)); > + vgaarb_info(dev, "overriding boot device\n"); > vga_set_default_device(vgadev->pdev); > } > #endif > if (vgadev->bridge_has_one_vga) > - pr_info("bridge control possible %s\n", > - pci_name(vgadev->pdev)); > + vgaarb_info(dev, "bridge control possible\n"); > else > - pr_info("no bridge control possible %s\n", > - pci_name(vgadev->pdev)); > + vgaarb_info(dev, "no bridge control possible\n"); > } > + > + pr_info("loaded\n"); > return rc; > } > subsys_initcall(vga_arb_device_init); > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Nov 17, 2016 at 07:59:21PM +0100, Daniel Vetter wrote: > On Thu, Nov 17, 2016 at 11:47:58AM -0600, Bjorn Helgaas wrote: > > Use dev_printk() when possible. This makes messages more consistent with > > other device-related messages and, in some cases, adds useful information. > > This changes messages like this: > > > > vgaarb: failed to allocate pci device > > vgaarb: setting as boot device: PCI:0000:01:00.0 > > vgaarb: device added: PCI:0000:01:00.0,decodes=io+mem,owns=io+mem,locks=none > > vgaarb: bridge control possible 0000:01:00.0 > > > > to this: > > > > pci 0000:01:00.0: vgaarb: failed to allocate VGA arbiter data > > pci 0000:01:00.0: vgaarb: setting as boot VGA device > > pci 0000:01:00.0: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none > > pci 0000:01:00.0: vgaarb: bridge control possible > > > > No functional change intended. > > Found one more nit below where there was one relevant change that > shouldn't be here. > > @@ -1189,24 +1194,25 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, > > ret_val = -EPROTO; > > goto done; > > } > > - pr_debug("%s ==> %x:%x:%x.%x\n", curr_pos, > > - domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); > > - > > pdev = pci_get_domain_bus_and_slot(domain, bus, devfn); > > - pr_debug("pdev %p\n", pdev); > > if (!pdev) { > > - pr_err("invalid PCI address %x:%x:%x\n", > > - domain, bus, devfn); > > + pr_err("invalid PCI address %04x:%02x:%02x.%x\n", > > + domain, bus, PCI_SLOT(devfn), > > + PCI_FUNC(devfn)); > > Userspace-triggerable dmesg spam is imo not good, this needs to stay at > debug level. I did move these around slightly, but I don't *think* I changed anything from debug to err level. Previously: pr_debug("%s ==> %x:%x:%x.%x\n", ...); pdev = pci_get_domain_bus_and_slot(...); pr_debug("pdev %p\n", pdev); if (!pdev) { pr_err("invalid PCI address %x:%x:%x\n", ...); } after my patch: pdev = pci_get_domain_bus_and_slot(...); if (!pdev) { pr_err("invalid PCI address %x:%x:%x\n", ...); } pr_debug("%s ==> %x:%x:%x.%x pdev %p\n", ...); The pr_err() was there before. I'd be glad to change that to a pr_debug() if you prefer. Bjorn
On Thu, Nov 17, 2016 at 02:20:59PM -0600, Bjorn Helgaas wrote: > On Thu, Nov 17, 2016 at 07:59:21PM +0100, Daniel Vetter wrote: > > On Thu, Nov 17, 2016 at 11:47:58AM -0600, Bjorn Helgaas wrote: > > > Use dev_printk() when possible. This makes messages more consistent with > > > other device-related messages and, in some cases, adds useful information. > > > This changes messages like this: > > > > > > vgaarb: failed to allocate pci device > > > vgaarb: setting as boot device: PCI:0000:01:00.0 > > > vgaarb: device added: PCI:0000:01:00.0,decodes=io+mem,owns=io+mem,locks=none > > > vgaarb: bridge control possible 0000:01:00.0 > > > > > > to this: > > > > > > pci 0000:01:00.0: vgaarb: failed to allocate VGA arbiter data > > > pci 0000:01:00.0: vgaarb: setting as boot VGA device > > > pci 0000:01:00.0: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none > > > pci 0000:01:00.0: vgaarb: bridge control possible > > > > > > No functional change intended. > > > > Found one more nit below where there was one relevant change that > > shouldn't be here. > > > > @@ -1189,24 +1194,25 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, > > > ret_val = -EPROTO; > > > goto done; > > > } > > > - pr_debug("%s ==> %x:%x:%x.%x\n", curr_pos, > > > - domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); > > > - > > > pdev = pci_get_domain_bus_and_slot(domain, bus, devfn); > > > - pr_debug("pdev %p\n", pdev); > > > if (!pdev) { > > > - pr_err("invalid PCI address %x:%x:%x\n", > > > - domain, bus, devfn); > > > + pr_err("invalid PCI address %04x:%02x:%02x.%x\n", > > > + domain, bus, PCI_SLOT(devfn), > > > + PCI_FUNC(devfn)); > > > > Userspace-triggerable dmesg spam is imo not good, this needs to stay at > > debug level. > > I did move these around slightly, but I don't *think* I changed > anything from debug to err level. Previously: > > pr_debug("%s ==> %x:%x:%x.%x\n", ...); > pdev = pci_get_domain_bus_and_slot(...); > pr_debug("pdev %p\n", pdev); > if (!pdev) { > pr_err("invalid PCI address %x:%x:%x\n", ...); > } > > after my patch: > > pdev = pci_get_domain_bus_and_slot(...); > if (!pdev) { > pr_err("invalid PCI address %x:%x:%x\n", ...); > } > pr_debug("%s ==> %x:%x:%x.%x pdev %p\n", ...); > > The pr_err() was there before. I'd be glad to change that to a > pr_debug() if you prefer. Oh right, misread the diff. Applied your patch, but a s/pr_err/pr_debug for anything userspace can trigger in the string parsing would be nice. Thanks, Daniel
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c index 1887f19..3a8e9c6 100644 --- a/drivers/gpu/vga/vgaarb.c +++ b/drivers/gpu/vga/vgaarb.c @@ -31,6 +31,10 @@ #define pr_fmt(fmt) "vgaarb: " fmt +#define vgaarb_dbg(dev, fmt, arg...) dev_dbg(dev, "vgaarb: " fmt, ##arg) +#define vgaarb_info(dev, fmt, arg...) dev_info(dev, "vgaarb: " fmt, ##arg) +#define vgaarb_err(dev, fmt, arg...) dev_err(dev, "vgaarb: " fmt, ##arg) + #include <linux/module.h> #include <linux/kernel.h> #include <linux/pci.h> @@ -188,6 +192,7 @@ static void vga_check_first_use(void) static struct vga_device *__vga_tryget(struct vga_device *vgadev, unsigned int rsrc) { + struct device *dev = &vgadev->pdev->dev; unsigned int wants, legacy_wants, match; struct vga_device *conflict; unsigned int pci_bits; @@ -203,8 +208,8 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev, (vgadev->decodes & VGA_RSRC_LEGACY_MEM)) rsrc |= VGA_RSRC_LEGACY_MEM; - pr_debug("%s: %d\n", __func__, rsrc); - pr_debug("%s: owns: %d\n", __func__, vgadev->owns); + vgaarb_dbg(dev, "%s: %d\n", __func__, rsrc); + vgaarb_dbg(dev, "%s: owns: %d\n", __func__, vgadev->owns); /* Check what resources we need to acquire */ wants = rsrc & ~vgadev->owns; @@ -336,9 +341,10 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev, static void __vga_put(struct vga_device *vgadev, unsigned int rsrc) { + struct device *dev = &vgadev->pdev->dev; unsigned int old_locks = vgadev->locks; - pr_debug("%s\n", __func__); + vgaarb_dbg(dev, "%s\n", __func__); /* Update our counters, and account for equivalent legacy resources * if we decode them @@ -611,7 +617,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) /* Allocate structure */ vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL); if (vgadev == NULL) { - pr_err("failed to allocate pci device\n"); + vgaarb_err(&pdev->dev, "failed to allocate VGA arbiter data\n"); /* * What to do on allocation failure ? For now, let's just do * nothing, I'm not sure there is anything saner to be done. @@ -663,7 +669,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) */ if (vga_default == NULL && ((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK)) { - pr_info("setting as boot device: PCI:%s\n", pci_name(pdev)); + vgaarb_info(&pdev->dev, "setting as boot VGA device\n"); vga_set_default_device(pdev); } @@ -672,8 +678,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) /* Add to the list */ list_add(&vgadev->list, &vga_list); vga_count++; - pr_info("device added: PCI:%s,decodes=%s,owns=%s,locks=%s\n", - pci_name(pdev), + vgaarb_info(&pdev->dev, "VGA device added: decodes=%s,owns=%s,locks=%s\n", vga_iostate_to_str(vgadev->decodes), vga_iostate_to_str(vgadev->owns), vga_iostate_to_str(vgadev->locks)); @@ -725,6 +730,7 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev) static inline void vga_update_device_decodes(struct vga_device *vgadev, int new_decodes) { + struct device *dev = &vgadev->pdev->dev; int old_decodes, decodes_removed, decodes_unlocked; old_decodes = vgadev->decodes; @@ -732,8 +738,7 @@ static inline void vga_update_device_decodes(struct vga_device *vgadev, decodes_unlocked = vgadev->locks & decodes_removed; vgadev->decodes = new_decodes; - pr_info("device changed decodes: PCI:%s,olddecodes=%s,decodes=%s:owns=%s\n", - pci_name(vgadev->pdev), + vgaarb_info(dev, "changed VGA decodes: olddecodes=%s,decodes=%s:owns=%s\n", vga_iostate_to_str(old_decodes), vga_iostate_to_str(vgadev->decodes), vga_iostate_to_str(vgadev->owns)); @@ -754,7 +759,7 @@ static inline void vga_update_device_decodes(struct vga_device *vgadev, if (!(old_decodes & VGA_RSRC_LEGACY_MASK) && new_decodes & VGA_RSRC_LEGACY_MASK) vga_decode_count++; - pr_debug("decoding count now is: %d\n", vga_decode_count); + vgaarb_dbg(dev, "decoding count now is: %d\n", vga_decode_count); } static void __vga_set_legacy_decoding(struct pci_dev *pdev, @@ -1189,24 +1194,25 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, ret_val = -EPROTO; goto done; } - pr_debug("%s ==> %x:%x:%x.%x\n", curr_pos, - domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); - pdev = pci_get_domain_bus_and_slot(domain, bus, devfn); - pr_debug("pdev %p\n", pdev); if (!pdev) { - pr_err("invalid PCI address %x:%x:%x\n", - domain, bus, devfn); + pr_err("invalid PCI address %04x:%02x:%02x.%x\n", + domain, bus, PCI_SLOT(devfn), + PCI_FUNC(devfn)); ret_val = -ENODEV; goto done; } + + pr_debug("%s ==> %04x:%02x:%02x.%x pdev %p\n", curr_pos, + domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), + pdev); } vgadev = vgadev_find(pdev); pr_debug("vgadev %p\n", vgadev); if (vgadev == NULL) { if (pdev) { - pr_err("this pci device is not a vga device\n"); + vgaarb_err(&pdev->dev, "not a VGA device\n"); pci_dev_put(pdev); } @@ -1226,7 +1232,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, } } if (i == MAX_USER_CARDS) { - pr_err("maximum user cards (%d) number reached!\n", + vgaarb_err(&pdev->dev, "maximum user cards (%d) number reached, ignoring this one!\n", MAX_USER_CARDS); pci_dev_put(pdev); /* XXX: which value to return? */ @@ -1317,8 +1323,8 @@ static int vga_arb_release(struct inode *inode, struct file *file) uc = &priv->cards[i]; if (uc->pdev == NULL) continue; - pr_debug("uc->io_cnt == %d, uc->mem_cnt == %d\n", - uc->io_cnt, uc->mem_cnt); + vgaarb_dbg(&uc->pdev->dev, "uc->io_cnt == %d, uc->mem_cnt == %d\n", + uc->io_cnt, uc->mem_cnt); while (uc->io_cnt--) vga_put(uc->pdev, VGA_RSRC_LEGACY_IO); while (uc->mem_cnt--) @@ -1371,7 +1377,7 @@ static int pci_notify(struct notifier_block *nb, unsigned long action, struct pci_dev *pdev = to_pci_dev(dev); bool notify = false; - pr_debug("%s\n", __func__); + vgaarb_dbg(dev, "%s\n", __func__); /* For now we're only intereted in devices added and removed. I didn't * test this thing here, so someone needs to double check for the @@ -1408,6 +1414,7 @@ static int __init vga_arb_device_init(void) int rc; struct pci_dev *pdev; struct vga_device *vgadev; + struct device *dev; rc = misc_register(&vga_arb_device); if (rc < 0) @@ -1423,8 +1430,6 @@ static int __init vga_arb_device_init(void) PCI_ANY_ID, pdev)) != NULL) vga_arbiter_add_pci_device(pdev); - pr_info("loaded\n"); - list_for_each_entry(vgadev, &vga_list, list) { #if defined(CONFIG_X86) || defined(CONFIG_IA64) /* @@ -1440,6 +1445,7 @@ static int __init vga_arb_device_init(void) int i; limit = screen_info.lfb_base + screen_info.lfb_size; + dev = &vgadev->pdev->dev; /* Does firmware framebuffer belong to us? */ for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) { @@ -1458,21 +1464,19 @@ static int __init vga_arb_device_init(void) continue; if (!vga_default_device()) - pr_info("setting as boot device: PCI:%s\n", - pci_name(vgadev->pdev)); + vgaarb_info(dev, "setting as boot device\n"); else if (vgadev->pdev != vga_default_device()) - pr_info("overriding boot device: PCI:%s\n", - pci_name(vgadev->pdev)); + vgaarb_info(dev, "overriding boot device\n"); vga_set_default_device(vgadev->pdev); } #endif if (vgadev->bridge_has_one_vga) - pr_info("bridge control possible %s\n", - pci_name(vgadev->pdev)); + vgaarb_info(dev, "bridge control possible\n"); else - pr_info("no bridge control possible %s\n", - pci_name(vgadev->pdev)); + vgaarb_info(dev, "no bridge control possible\n"); } + + pr_info("loaded\n"); return rc; } subsys_initcall(vga_arb_device_init);
Use dev_printk() when possible. This makes messages more consistent with other device-related messages and, in some cases, adds useful information. This changes messages like this: vgaarb: failed to allocate pci device vgaarb: setting as boot device: PCI:0000:01:00.0 vgaarb: device added: PCI:0000:01:00.0,decodes=io+mem,owns=io+mem,locks=none vgaarb: bridge control possible 0000:01:00.0 to this: pci 0000:01:00.0: vgaarb: failed to allocate VGA arbiter data pci 0000:01:00.0: vgaarb: setting as boot VGA device pci 0000:01:00.0: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none pci 0000:01:00.0: vgaarb: bridge control possible No functional change intended. Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> --- drivers/gpu/vga/vgaarb.c | 66 ++++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 31 deletions(-)