lkml.org 
[lkml]   [2016]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] vgaarb: Use dev_printk() when possible
    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

    \
     
     \ /
      Last update: 2016-11-17 21:21    [W:2.375 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site