Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752295AbcKQUVF (ORCPT ); Thu, 17 Nov 2016 15:21:05 -0500 Received: from mail.kernel.org ([198.145.29.136]:44434 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752219AbcKQUVD (ORCPT ); Thu, 17 Nov 2016 15:21:03 -0500 Date: Thu, 17 Nov 2016 14:20:59 -0600 From: Bjorn Helgaas To: Bjorn Helgaas , David Airlie , Thierry Reding , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v2] vgaarb: Use dev_printk() when possible Message-ID: <20161117202059.GA22428@bhelgaas-glaptop.roam.corp.google.com> References: <20161117174758.16810.67625.stgit@bhelgaas-glaptop.roam.corp.google.com> <20161117185921.r3fcd4dhfdlahb6b@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161117185921.r3fcd4dhfdlahb6b@phenom.ffwll.local> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2257 Lines: 64 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