Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/22776 )
Change subject: intel/sandybridge: Make timC training more robust. ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/#/c/22776/3/src/northbridge/intel/sandybridge/ra... File src/northbridge/intel/sandybridge/raminit_common.c:
https://review.coreboot.org/#/c/22776/3/src/northbridge/intel/sandybridge/ra... PS3, Line 1577: rn.length < 8
Why? 0 errors is 0 errors...
if the lane is erroneous there might be a single good sample. In that case rn.all is not true, and the following raminit will fail as we didn't pick the optimal value, instead we pick a random "good" one. This check ensures that the area is big enough. It happens in 1 out of 10 boots that I don't hit rn.all and thus the PC won't boot.
https://review.coreboot.org/#/c/22776/3/src/northbridge/intel/sandybridge/ra... PS3, Line 1581: avarage
The average is a very high since most of the entries will have the max (4000) errors. […]
usually you will only see 4000 and 0, but as tests showed in an error case it can be anything. taking the average between min and max seems the best solution. As this is a recovery attempt that is seldom taken I'd not try to improve it any more, as it gives good results already.
-
Patrick Rudolph (Code Review)