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 > <[email protected]> wrote: > > From: "Luis R. Rodriguez" <[email protected]> > > > > 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 <[email protected]> > > Cc: Andrew Morton <[email protected]> > > Cc: Greg Kroah-Hartman <[email protected]> > > Cc: David Howells <[email protected]> > > Cc: Kees Cook <[email protected]> > > Cc: Casey Schaufler <[email protected]> > > Cc: Ming Lei <[email protected]> > > Cc: Takashi Iwai <[email protected]> > > Cc: Vojtěch Pavlík <[email protected]> > > Cc: Kyle McMartin <[email protected]> > > Cc: Matthew Garrett <[email protected]> > > Cc: [email protected] > > Signed-off-by: Luis R. Rodriguez <[email protected]> > > --- > > 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 <[email protected]> Cc: Andrew Morton <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: David Howells <[email protected]> Cc: Kees Cook <[email protected]> Cc: Casey Schaufler <[email protected]> Cc: Ming Lei <[email protected]> Cc: Takashi Iwai <[email protected]> Cc: Vojtěch Pavlík <[email protected]> Cc: Kyle McMartin <[email protected]> Cc: Matthew Garrett <[email protected]> Cc: [email protected] Signed-off-by: Luis R. Rodriguez <[email protected]> --- 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
