On 12/03/2013 04:21 PM, Andrew Morton wrote:
> On Mon, 2 Dec 2013 10:19:37 -0500 Prarit Bhargava <pra...@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.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
