Message-ID: <20161117073124.sfqiyojqnvjzibbh@phenom.ffwll.local>
Date:   Thu, 17 Nov 2016 08:31:24 +0100
From:   Daniel Vetter <daniel@...ll.ch>
To:     Bjorn Helgaas <bhelgaas@...gle.com>
Cc:     David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Thierry Reding <treding@...dia.com>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] vgaarb: Use dev_printk() when possible

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 <bhelgaas@...gle.com>

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 <linux/module.h>
>  #include <linux/kernel.h>
> @@ -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@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Powered by blists - more mailing lists