/* The memory is now setup, use it */ +#if USE_DCACHE_RAM == 0 cache_lbmem(MTRR_TYPE_WRBACK); +#endif }
Could you explain why you had to add this?
This board uses CAR, like the kontron. When we initialize the memory, we are still in CAR, and cache_lbmem will setup MTRR its ways. From what I understand, this will break CAR and we'll get stuck here. So cache_lbmem is OK for boards using ROMCC (mtarvon, truxton...), but is bad with CAR.
OK.
I've refactored two of your patches so that they use svn cp from kontron
and
model_6fx sources to show the differences. If you based your work on different sources, let me know.
I'm really I wanted to do that but I did not find the time (lot of work right now). I used a mix of 2 board : for the CAR skeleton I used the Kontron 986LCDM mainboard, for the 3100 stuff the mt arvon board.
Great.
My comments: There is some #if 0 code in acpi_tables. Will it ever be enabled? If
not,
remove it.
Yes indeed. As I said I started from the kontron mainboard but in my case I didn't fill the OEMB table so they can safely be removed. We can still fill table later if needed. I also found another #if 0 / #endif pair in auto.c. Its about the MSR_THERM2_CTL. I remember I add to disable it on a core 2 duo L7400 otherwise I was stuck. I'll check it today if we can re-enable this one.
Thanks.
I'm confused why we need eagleheights_fixups. Can we remove it?
Yes indeed. I always add it in case but here I did not had to use it, so trash.
Will do.
Index: svn/src/cpu/intel/model_1067x/cache_as_ram_post.c
--- svn.orig/src/cpu/intel/model_1067x/cache_as_ram_post.c +++ svn/src/cpu/intel/model_1067x/cache_as_ram_post.c @@ -50,9 +50,9 @@ "wrmsr\n" "movl $MTRRphysMask_MSR(1), %ecx\n" "wrmsr\n" -#endif
"movb $0x33, %al\noutb %al, $0x80\n" +#endif #ifdef CLEAR_FIRST_1M_RAM "movb $0x34, %al\noutb %al, $0x80\n" /* Enable Write Combining and Speculative Reads for the first 1MB
*/
@@ -120,7 +120,7 @@ "movb $0x3b, %al\noutb %al, $0x80\n"
/* Enable prefetchers */ - "movl $0x01a0, %eax\n" + "movl $0x01a0, %ecx\n" "rdmsr\n" "andl $~((1 << 9) | (1 << 19)), %eax\n" "andl $~((1 << 5) | (1 << 7)), %edx\n"
These changes were surprising. Is there a bug in the original code?
In the code, we execute a block a instructions to setup stuffs, then output a post code. But if the instructions are disabled (ifdef CLEAR_FIRST_1M_RAM), it's useless to output the post code no ? For the last modifications (enable prefetchers : eax becomes ecx), there is indeed a bug. I already sent a patch for this one few months ago but it has been lost in the mailing list.
Sorry about that.
The rdmsr instruction will read the msr specified by ecx into edx:eax (Intel Software Dev Manual, volume 2B, 4-322).
So I think we should fix the original code and not duplicate it.
Stefan: What do you think about unifying src/cpu/intel/model_*/cache_as_ram_post.c?
There are a couple of the files that are very similar.
Thanks again for taking the time to review my patch.
No problem. Could you send me the corrected Config.lb now that you removed the disabled devices?
Thanks, Myles