Re: [PATCHv5] EDAC core changes in order to properly report errors from all types of memory controllers

[Borislav Petkov <bp@amd64.org>]

On Tue, Mar 06, 2012 at 08:31:36AM -0300, Mauro Carvalho Chehab wrote:

[..]

> Breaking it into smaller patchsets is not trivial, as the analysis of the
> individual changesets only makes sense in the light of the big change on
> the edac core structures.

I don't believe that and LKML contains countless examples of how patches
can be split into simple hunks containing only one logical change.
Simply put yourself in the reviewer's place and try to imagine what kind
of patch format you'd like to see.

[..]

> On a csrow-based MC, dual-rank memories would be mapped as two separate 
> (csrow, channel) addresses. Each will have half of the DIMM size on each
> address. The above "emulation" creates two (csrow, channel) addreses for every
> FB-DIMM, filling just one, but only if the memory is dual-rank.
> 
> For a FB-DIMM controller, the number of ranks is just a detail associated with
> a given DIMM slot, as the memory is selected by slot, and not by rank.
> 
> So, the logic is completely broken for single-rank memories and half-broken for 
> double-rank ones.

I'm still wondering whether FBDIMM-based drivers should get their own
EDAC infrastructure and own nomenclature instead of fitting them in the
existing scheme...

> 
> This is an example of a patch that should not be fold.
> 
> After this patch, the memories on the i5000 machine I'm testing are properly
> reported, being single-rank or dual-rank.

How about instead of verbosely explaining in a mail why you're doing
what you're doing, you add that explanation to your patches so that even
the unlightened one can understand what you're doing? I think that will
benefit all.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551