Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934174AbcKQHbb (ORCPT ); Thu, 17 Nov 2016 02:31:31 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:35714 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751378AbcKQHb3 (ORCPT ); Thu, 17 Nov 2016 02:31:29 -0500 Date: Thu, 17 Nov 2016 08:31:24 +0100 From: Daniel Vetter To: Bjorn Helgaas Cc: David Airlie , Daniel Vetter , Thierry Reding , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH] vgaarb: Use dev_printk() when possible Message-ID: <20161117073124.sfqiyojqnvjzibbh@phenom.ffwll.local> Mail-Followup-To: Bjorn Helgaas , David Airlie , Thierry Reding , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org References: <20161116214522.12138.50745.stgit@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161116214522.12138.50745.stgit@bhelgaas-glaptop.roam.corp.google.com> X-Operating-System: Linux phenom 4.6.0-1-amd64 User-Agent: NeoMutt/20161104 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4740 Lines: 135 On Wed, Nov 16, 2016 at 03:45:22PM -0600, Bjorn Helgaas wrote: > For consistency with other device-related messages, use dev_printk() when > possible instead of pr_*() and pci_name(). This changes messages like > this: > > 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: setting as boot device > pci 0000:01:00.0: vgaarb: 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 Hm, not convinced this is better (vgaarb is kinda not a device driver), and the code becomes a bit more unpretty imo with placing DRV all over. I guess if you go with a #define vgaarb_info(dev, fmt) dev_info(dev, "vgaarg: " fmt) or so I can be convinced. -Daniel > --- > drivers/gpu/vga/vgaarb.c | 27 ++++++++++++--------------- > 1 file changed, 12 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > index 1887f19..8a3f212 100644 > --- a/drivers/gpu/vga/vgaarb.c > +++ b/drivers/gpu/vga/vgaarb.c > @@ -29,7 +29,8 @@ > * > */ > > -#define pr_fmt(fmt) "vgaarb: " fmt > +#define DRV "vgaarb: " > +#define pr_fmt(fmt) DRV fmt > > #include > #include > @@ -663,7 +664,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)); > + dev_info(&pdev->dev, DRV "setting as boot device\n"); > vga_set_default_device(pdev); > } > > @@ -672,8 +673,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), > + dev_info(&pdev->dev, DRV "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)); > @@ -732,8 +732,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), > + dev_info(&vgadev->pdev->dev, DRV "device changed 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)); > @@ -1206,7 +1205,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, > pr_debug("vgadev %p\n", vgadev); > if (vgadev == NULL) { > if (pdev) { > - pr_err("this pci device is not a vga device\n"); > + dev_err(&pdev->dev, DRV "this device is not a vga device\n"); > pci_dev_put(pdev); > } > > @@ -1408,6 +1407,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) > @@ -1440,6 +1440,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,20 +1459,16 @@ 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)); > + dev_info(dev, DRV "setting as boot device\n"); > else if (vgadev->pdev != vga_default_device()) > - pr_info("overriding boot device: PCI:%s\n", > - pci_name(vgadev->pdev)); > + dev_info(dev, DRV "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)); > + dev_info(dev, DRV "bridge control possible\n"); > else > - pr_info("no bridge control possible %s\n", > - pci_name(vgadev->pdev)); > + dev_info(dev, DRV "no bridge control possible\n"); > } > return rc; > } > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch