[coreboot-gerrit] Change in coreboot[master]: nb/intel/i945/raminit.c: Refactor tRD selection

[Arthur Heymans (Code Review)]

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

Change subject: nb/intel/i945/raminit.c: Refactor tRD selection
......................................................................


Patch Set 7: Code-Review-1

> Please mention for such changes if they affect mobile / desktop or
 > both
 > and if it was tested on both in case. Would have eased the review
 > if you
 > had mentioned that the values from the original (mobile) code are
 > still
 > the same.
 > 
 > Btw. in the comments you seem to assume that the register values
 > map
 > 1:1 to tRD (i.e. without any offset). I couldn't find any evidence
 > for
 > that. Did you?

Only mobile, 945gm, datasheet documents this, so it assumes indeed that those are the same on desktop. No deep understanding of how this should be set (I think pineview code has complicated code for how it ought to be). It seems to match vendor more or less ok...

Since dram CAS/freq selection is not yet ready I think it would be better to wait to submit it, since I have no idea if wrong CAS + this works fine...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8002daf25b7603131b78b01075f43fd23747dd94
Gerrit-PatchSet: 7
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
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: build bot (Jenkins)
Gerrit-HasComments: No