[coreboot-gerrit] Change in coreboot[master]: nb/intel/x4x/raminit: Rework receive enable calibration

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/18692 )

Change subject: nb/intel/x4x/raminit: Rework receive enable calibration
......................................................................


Patch Set 20:

(4 comments)

https://review.coreboot.org/#/c/18692/20/src/northbridge/intel/x4x/rcven.c
File src/northbridge/intel/x4x/rcven.c:

PS20, Line 49: 	MCHBAR8(0x9d8) = MCHBAR8(0x9d8) & ~0x2;
             : 	udelay(2);
             : 	MCHBAR8(0x9d8) = MCHBAR8(0x9d8) | 0x2;
             : 	udelay(2);
> Looks like `0x400 * channel + 0x5d8`, shouldn't matter, though it
I'll try without. it could provide a speed up.


Line 54: 	strobe = read32((u32 *)addr);
> i945 repeats the memory access but not the whole sequence, maybe
I'll try.


PS20, Line 56: MCHBAR32(sample_offset)
> Unaligned access, not sure if it can cause any trouble.
it could just I use MCHBAR8 I suppose.


PS20, Line 319: timing[lane].coarse = (s->selected_timings.CAS - 1)
pineview which is a quite similar memory controller starts at CAS + 1, and so does gm45. Maybe this helps with those false results?


-- 
To view, visit https://review.coreboot.org/18692
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0c970455e609d3ce96a262cbf110336a2079da4d
Gerrit-PatchSet: 20
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Damien Zammit <damien at zamaudio.com>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude at gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Reviewer: coreboot org <coreboot.org at gmail.com>
Gerrit-HasComments: Yes