[lkml]   [2012]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
Em 30-04-2012 09:38, Borislav Petkov escreveu:
> On Mon, Apr 30, 2012 at 08:45:09AM -0300, Mauro Carvalho Chehab wrote:
>> Em 30-04-2012 08:11, Borislav Petkov escreveu:
>>> On Mon, Apr 30, 2012 at 07:58:33AM -0300, Mauro Carvalho Chehab wrote:

>>> This way it says "initializing 12 dimms" and the user thinks there are
>>> 12 DIMMs on his system where this might not be true.
>> I'm OK to remove the "initializing 12 dimms" message. It doesn't add anything
>> new.
>> With regards do the other messages, if the debug messages are not clear,
>> then let's fix them, instead of removing. What if we print, instead,
>> on a message like:
>> "row 1, chan 1 will represent dimm5 (1:2:0) if not empty"
> How about the following instead: the specific driver calls
> edac_mc_alloc(), it gets the allocated dimm array in mci->dimms
> _without_ dumping each dimm%d line. Then, each driver figures out which
> subset of that dimms array actually has populated slots and prints only
> the populated rank/slot/...
> This information is much more valuable than saying how many _possible_
> slots the edac core has allocated.
> Then, each driver can decide whether it makes sense to dump that info or
> not.

No, that would add extra complexity at the drivers level just due to debug
messages. I think that the better is to move this printk to the debug-specific
routine that is called only when the dimm is filled (edac_mc_dump_dimm).

With this cange, the message will be printed only for the filled dimms.

This is a cleanup patch, so I'll write it, together with the change that
will get rid of the loop that uses KERN_CONT. It will use a function added
by a latter patch at edac_mc_sysfs so it can't be merged on this patch


 \ /
  Last update: 2012-04-30 15:41    [W:0.171 / U:11.568 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site