[coreboot] [PATCH] Geode GX2 auto DRAM detect patch
Nils
njacobs8 at hetnet.nl
Thu Oct 14 23:02:10 CEST 2010
Hello Myles,
Thanks for the review.
Op donderdag 14 oktober 2010 18:53:43 schreef u:
> 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.
Sorry! I will try to do better next time.
> +#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?
I am not the designer of that code i copied it from lippert/roadrunner-lx .
When i leave DIMM1 undefined i get following errors/warnings:
In file included from src/mainboard/wyse/s50/romstage.c:50:
src/northbridge/amd/gx2/raminit.c: In function ‘checkDDRMax’:
src/northbridge/amd/gx2/raminit.c:176: error: ‘DIMM1’ undeclared (first use in
this function)
src/northbridge/amd/gx2/raminit.c:176: error: (Each undeclared identifier is
reported only once
src/northbridge/amd/gx2/raminit.c:176: error: for each function it appears
in.)
src/northbridge/amd/gx2/raminit.c: In function ‘set_refresh_rate’:
src/northbridge/amd/gx2/raminit.c:212: error: ‘DIMM1’ undeclared (first use in
this function)
src/northbridge/amd/gx2/raminit.c: In function ‘setCAS’:
src/northbridge/amd/gx2/raminit.c:299: error: ‘DIMM1’ undeclared (first use in
this function)
src/northbridge/amd/gx2/raminit.c: In function ‘set_latencies’:
src/northbridge/amd/gx2/raminit.c:335: error: ‘DIMM1’ undeclared (first use in
this function)
src/northbridge/amd/gx2/raminit.c: In function ‘set_extended_mode_registers’:
src/northbridge/amd/gx2/raminit.c:431: error: ‘DIMM1’ undeclared (first use in
this function)
src/northbridge/amd/gx2/raminit.c: In function ‘sdram_set_spd_registers’:
src/northbridge/amd/gx2/raminit.c:482: error: ‘DIMM1’ undeclared (first use in
this function)
make: *** [build/mainboard/wyse/s50/romstage.pre.inc] Fout 1
n
Thanks,Nils.
More information about the coreboot
mailing list