2016-11-16 21:45:27

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH] vgaarb: Use dev_printk() when possible

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 <[email protected]>
---
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;
}


2016-11-17 07:31:31

by Daniel Vetter

[permalink] [raw]
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 <[email protected]>

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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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