lkml.org 
[lkml]   [2013]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] Introduce FW_INFO* functions and messages


On 12/03/2013 04:21 PM, Andrew Morton wrote:
> On Mon, 2 Dec 2013 10:19:37 -0500 Prarit Bhargava <prarit@redhat.com> wrote:
> A slight simplification:
>
>> +static inline char *dump_hadware_arch_desc(void)
>> +{
>> + return NULL;
>> +}
>
> return "unavailable";
>
>> +void warn_slowpath_fmt_dev(const char *file, int line,
>> + struct device *dev, int level, const char *fmt, ...)
>> +{
>> + struct slowpath_args args;
>> + static char fw_str[16] = "\0";
>> +
>> + switch (level) {
>> + case 1:
>> + strcpy(fw_str, "[Firmware Info]");
>> + break;
>> + case 2:
>> + strcpy(fw_str, "[Firmware Warn]");
>> + break;
>> + case 3:
>> + strcpy(fw_str, "[Firmware Bug]");
>
> What's With The Crazy Capitalization In This Code? It's Illiterate!

Heh :) it's what the original FW_* strings were defined as. I was hoping it was
okay to change them but wasn't 100% sure if I should. I'll definitely take that
(and the suggestion above) into v2.

<snip>

>
> The general thrust of the patchset seems useful and important. I do
> agree with Joe's suggestion regarding the presentation, although it
> isn't a show-stopper.

In V2, I'll take Joe's suggestion into account. It isn't that difficult to
rework those chunks of the patch into a single patch.

>
> I do wonder if it all should be generalised a bit - it creates a bunch
> of infrastructure which is specific to system firmware issues, but
> what's so special about firmware? Why can't I use this infrastructure
> to log netdev errors or acpi errors or PM errors or...? But I didn't
> think about it much ;)

Well ... I was thinking about this. Some drivers do keep track of firmware
versions for their devices and I think there is probably a way to unify all that.

Also, I think after this initial change goes in I'm going to grep through the
PCI & ACPI code to see what FW_* changes need to be made there. AFAICT there
are many locations in those areas which should be FW_*.

I'll get a [v2] out in a little while. Thanks Joe & Andrew for looking.

P.
>
>


\
 
 \ /
  Last update: 2013-12-04 13:21    [W:0.135 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site