lkml.org 
[lkml]   [2012]   [Dec]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v7 2/3] aerdrv: Enhanced AER logging
On Wed, Dec 5, 2012 at 9:30 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Dec 05, 2012 at 04:14:14PM +0000, Ortiz, Lance E wrote:
>> I removed the prefix argument because it was never used by its caller
>> and never set. The reason I added the prefix variable and set it to
>> NULL was to help in breaking up the patch and adding it would help the
>> intermittent patch build without changing too much code. I knew I was
>> actually going to use the variable in patch 3.
>
> No, the correct way to do that is to keep all changes that belong
> logically together in a single patch for ease of reviewing and avoid
> breakages. And in your case this should be pretty easy: simply move all
> the 'prefix' touching code to patch #3 and you're done, AFAICT.

Lance, you didn't add all the "prefix" stuff in AER, but since you're
touching it, I think things will make more sense if you clean it up at
the same time. There are a lot of printk() calls there that should be
converted to dev_printk().

I think I see why it was done that way -- it sticks either
KERN_WARNING or KERN_ERR at the beginning of the prefix, then uses
plain printk() later. I guess that means you only have to pass around
the prefix argument, rather than both a "level" and a "struct pci_dev
*". But I think it will be simpler overall to pass both and take
advantage of dev_printk() and stop emulating it.

Bjorn


\
 
 \ /
  Last update: 2012-12-27 03:21    [W:0.044 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site