[coreboot] [PATCH] v3: fix bugs in Geode LX RAMinit

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> Marc? It would be nice if you could read through the code and verify the 
> things I did.
> 
> northbridge/amd/geodelx/raminit.c:auto_size_dimm() checks for the 
> mathematically impossible condition of a value being above and below the 
> specified range at the same time. Change it to check for out-of-range.
> arch/x86/geodelx/geodelx.c:set_delay_control() is missing a break, it 
> will keep going and mess up DRAM timings.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> 

Strange, The raminit in v2 was correct. Both changes look right to me. I 
didn't build or run it. I am a bit swamped in something else right now.

Ron, can you test this?

BTW, Thanks to Carl-Daniel, Ron, and Uwe for recent work on LX. You are 
making good improvements to the code.

Acked-by: Marc Jones <marc.jones at amd.com>




> Index: LinuxBIOSv3-geodelx_dram_fixes/northbridge/amd/geodelx/raminit.c
> ===================================================================
> --- LinuxBIOSv3-geodelx_dram_fixes/northbridge/amd/geodelx/raminit.c    
> (Revision 638)
> +++ LinuxBIOSv3-geodelx_dram_fixes/northbridge/amd/geodelx/raminit.c    
> (Arbeitskopie)
> @@ -79,7 +79,7 @@
>     /* EEPROM byte usage: (5) Number of DIMM Banks */
>     banner(BIOS_DEBUG, "MODBANKS");
>     spd_byte = spd_read_byte(dimm, SPD_NUM_DIMM_BANKS);
> -    if ((MIN_MOD_BANKS > spd_byte) && (spd_byte > MAX_MOD_BANKS)) {
> +    if ((MIN_MOD_BANKS > spd_byte) || (spd_byte > MAX_MOD_BANKS)) {
>         printk(BIOS_EMERG, "Number of module banks not compatible\n");
>         post_code(ERROR_BANK_SET);
>         hlt();
> @@ -90,7 +90,7 @@
>     /* EEPROM byte usage: (17) Number of Banks on SDRAM Device */
>     banner(BIOS_DEBUG, "FIELDBANKS");
>     spd_byte = spd_read_byte(dimm, SPD_NUM_BANKS_PER_SDRAM);
> -    if ((MIN_DEV_BANKS > spd_byte) && (spd_byte > MAX_DEV_BANKS)) {
> +    if ((MIN_DEV_BANKS > spd_byte) || (spd_byte > MAX_DEV_BANKS)) {
>         printk(BIOS_EMERG, "Number of device banks not compatible\n");
>         post_code(ERROR_BANK_SET);
>         hlt();
> Index: LinuxBIOSv3-geodelx_dram_fixes/arch/x86/geodelx/geodelx.c
> ===================================================================
> --- LinuxBIOSv3-geodelx_dram_fixes/arch/x86/geodelx/geodelx.c    
> (Revision 638)
> +++ LinuxBIOSv3-geodelx_dram_fixes/arch/x86/geodelx/geodelx.c    
> (Arbeitskopie)
> @@ -386,6 +386,7 @@
>                 msr.hi |= delay_control_table[i].fast_hi;
>                 msr.lo |= delay_control_table[i].fast_low;
>             }
> +            break;
>         }
>     }
>     wrmsr(GLCP_DELAY_CONTROLS, msr);
> 
> 

-- 
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors