From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754626AbcAVUK4 (ORCPT ); Fri, 22 Jan 2016 15:10:56 -0500 Received: from mx2.suse.de ([195.135.220.15]:55930 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754525AbcAVUKP (ORCPT ); Fri, 22 Jan 2016 15:10:15 -0500 Date: Fri, 22 Jan 2016 21:10:09 +0100 From: "Luis R. Rodriguez" To: Kees Cook , Mimi Zohar Cc: "Luis R. Rodriguez" , Greg KH , Ming Lei , Josh Boyer , Johannes Berg , Andy Lutomirski , Jonathan Corbet , David Woodhouse , David Howells , Seth Forshee , Rusty Russell , Michal Marek , Matthew Garrett , Kyle McMartin , Mimi Zohar , Dmitry Kasatkin , Vivek Goyal , Brian Norris , Shuah Khan , Linus Torvalds , linux-security-module , keyrings@linux-nfs.org, LKML , Andrew Morton , Casey Schaufler , Takashi Iwai , =?utf-8?Q?Vojt=C4=9Bch_Pavl=C3=ADk?= Subject: Re: [PATCH v3 1/5] firmware: generalize "firmware" as "system data" helpers Message-ID: <20160122201009.GX20964@wotan.suse.de> References: <1450906497-24179-1-git-send-email-mcgrof@do-not-panic.com> <1450906497-24179-2-git-send-email-mcgrof@do-not-panic.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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 On Mon, Jan 04, 2016 at 12:41:07PM -0800, Kees Cook wrote: > On Wed, Dec 23, 2015 at 1:34 PM, Luis R. Rodriguez > wrote: > > From: "Luis R. Rodriguez" > > > > Historically firmware_class code was added to help > > get device driver firmware binaries but these days > > request_firmware*() helpers are being repurposed for > > general system data needed by the kernel. > > > > Annotate this before we extend firmare_class more, > > as this is expected. We want to generalize the code > > as much as possible. > > > > Cc: Rusty Russell > > Cc: Andrew Morton > > Cc: Greg Kroah-Hartman > > Cc: David Howells > > Cc: Kees Cook > > Cc: Casey Schaufler > > Cc: Ming Lei > > Cc: Takashi Iwai > > Cc: Vojtěch Pavlík > > Cc: Kyle McMartin > > Cc: Matthew Garrett > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Luis R. Rodriguez > > --- > > drivers/base/firmware_class.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > > index 8524450e75bd..6f5fcda71a60 100644 > > --- a/drivers/base/firmware_class.c > > +++ b/drivers/base/firmware_class.c > > @@ -353,15 +353,15 @@ static int fw_get_filesystem_firmware(struct device *device, > > rc = fw_read_file_contents(file, buf); > > fput(file); > > if (rc) > > - dev_warn(device, "firmware, attempted to load %s, but failed with error %d\n", > > - path, rc); > > + dev_warn(device, "system data, attempted to load %s, but failed with error %d\n", > > + path, rc); > > Since dev_warn should already be prefixing the string, I would think > "firmware, " should just be removed entirely instead of replaced? Its a good point, the messages could also be simplified further. Here's a more generic form and patch description that I'll wrap into a v4 iteration after Mimi's patches go in. [PATCH] firmware: simplify generic "firmware" helpers Historically firmware_class code was added to help get device driver firmware binaries but these days request_firmware*() helpers are being repurposed for general system data needed by the kernel. To make the generic set of routines that will be shared with generic system data helpers simplify a few of the shared debug and warn print outs, the prints already have the device associated as dev_*() helpers are used. This will help generalize shared code as much as possible. Cc: Rusty Russell Cc: Andrew Morton Cc: Greg Kroah-Hartman Cc: David Howells Cc: Kees Cook Cc: Casey Schaufler Cc: Ming Lei Cc: Takashi Iwai Cc: Vojtěch Pavlík Cc: Kyle McMartin Cc: Matthew Garrett Cc: linux-kernel@vger.kernel.org Signed-off-by: Luis R. Rodriguez --- drivers/base/firmware_class.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 8524450e75bd..3358f5df926f 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -353,15 +353,15 @@ static int fw_get_filesystem_firmware(struct device *device, rc = fw_read_file_contents(file, buf); fput(file); if (rc) - dev_warn(device, "firmware, attempted to load %s, but failed with error %d\n", - path, rc); + dev_warn(device, "loading %s failed with error %d\n", + path, rc); else break; } __putname(path); if (!rc) { - dev_dbg(device, "firmware: direct-loading firmware %s\n", + dev_dbg(device, "direct-loading %s\n", buf->fw_id); mutex_lock(&fw_lock); set_bit(FW_STATUS_DONE, &buf->status); @@ -1051,7 +1051,7 @@ _request_firmware_prepare(struct firmware **firmware_p, const char *name, } if (fw_get_builtin_firmware(firmware, name)) { - dev_dbg(device, "firmware: using built-in firmware %s\n", name); + dev_dbg(device, "using built-in %s\n", name); return 0; /* assigned */ } -- 2.7.0