[coreboot] [PATCH] Geode GX2 auto DRAM detect patch

Myles Watson mylesgw at gmail.com
Thu Oct 14 18:53:43 CEST 2010


On Wed, Oct 13, 2010 at 2:51 PM, Nils <njacobs8 at hetnet.nl> wrote:
>
> ***Ping***
>
> It would be nice if someone finds the time to ack/commit or comment my patch.

Your patch is rather long.  It would be easier to review if you split
out the white space & comment fixes, then the usage of msr names
instead of magic values.  It looks like this would make your patch
much smaller.  Another thing that would help would be if you used svn
cp so that it's obvious how you changed the lx code to adapt it for
the gx2.  When a patch looks like it has hundreds of lines of new
code, it's understandable that it could take longer to review.

+#define DIMM0 0xA0
+#define DIMM1 0xA2

-#include "northbridge/amd/gx2/raminit.h"
-
-	/* This is needed because ROMCC doesn`t now the ctz bitop */
-static inline unsigned int ctz(unsigned int n)
+static inline int spd_read_byte(unsigned int device, unsigned int address)
 {
-	int zeros;
+	if (device != DIMM0)
+		return 0xFF;	/* No DIMM1, don't even try. */

Why do you define DIMM1 to be 0xA2 (a reasonable value) if there is no
DIMM1?  Could we leave it undefined or define it to be something
obviously bogus?

Thanks,
Myles




More information about the coreboot mailing list