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.