Messages in this thread |  | | Date | Wed, 04 Dec 2013 06:51:41 -0500 | From | Prarit Bhargava <> | Subject | Re: [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. > >
|  |