Messages in this thread | | | Date | Tue, 6 Mar 2012 13:16:16 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCHv5] EDAC core changes in order to properly report errors from all types of memory controllers |
| |
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
| |