[LinuxBIOS] i440bx RAM initialization code (SPD)

[Uwe Hermann]

Hi,

thanks for your patch. However, please sign-off all patches you submit,
otherwise we cannot commit them.

http://linuxbios.org/Development_Guidelines#Sign-off_Procedure

Please re-send this patch with a sign-off.


On Mon, Apr 30, 2007 at 10:47:42AM -0400, Alfred Wanga wrote:
> * PMCR - When should it be updated?
> Looking at the assembly, it seems as though it's ok to just set the
> final value before the RAM refresh code instead of waiting until
> afterwards, but I don't know for sure, so I left the original code
> alone.

Yeah, I'm not sure either. Will look into that later...


> * sdram_enable delays
> I changed all the RAM timing delays (tRP, tRC, tMRD) to 1usec, since
> the timings are on the order of hundreds of nanoseconds (according to
> SPD values) and the smallest resolution timer available seems to be
> udelay() anyway. It should work for any SDRAM, and shaves a few
> milliseconds off previous code.

Yep, when everything is working fine (or maybe even before) we'll lower
the delays. I set them quite high to make sure that it's definately not
a too short delay which is causing problems...

If you want you can submit an extra patch with just the delay-fixes. I'll
test that on my hardware and commit if it still works with shorter delays.


> Anyway, enough verbosity... take a look and see what you think.
> Unfortunately, besides testing the SPD code on memory dumps, I haven't
> tested the image.

It doesn't compile, but that's not a problem in your code, but rather a
limitation of romcc ("too few registers").

I'll try to move the 440BX code over to Cache as RAM, that'll be needed for
LinuxBIOSv3 anyways. Expect a patch soonish...


After a few quick tests (after ripping out lots of code and debug
statements so that romcc compiles the code) it didn't seem to work on
my board. That may have lots of reasons (after all, I had to remove lots
of code and replace it with hardcoded values), but I'll look into it
in more detail.


Anyway, on the long run I don't want to merge this code as is (not a
pure translation of the v1 assembler code), but rather create a generic
framework for the RAM init on 440BX-ish chipsets.

It should be possible to share most of the code for, e.g.
 - 440BX/ZX/FX/...
 - 430TX/...
 - and maybe more such (probably quite similar) chipsets.

Thus, no spd_set_pgpol() etc., but more generic code (as far as possible).

But please re-send your patch with a sign-off, I think we can merge at
least some parts of it.


> Index: src/northbridge/intel/i440bx/raminit.c
> ===================================================================
> --- src/northbridge/intel/i440bx/raminit.c	(revision 2621)
> +++ src/northbridge/intel/i440bx/raminit.c	(working copy)
> @@ -377,7 +377,348 @@
>  DIMM-independant configuration functions.
>  -----------------------------------------------------------------------------*/
>  
> +/* Performs Bit Scan Right (MSB) Function (for SPD code) */
> +static inline uint8_t bsr(uint16_t val)
> +{
> +#if ASSEMBLY
> +	__asm__ __volatile__ ("bsr %1, %%ax;" : "=a"(val) : "a"(val));

No assembler, please. Apart from the very early stuff which _has_ to be
written in assembler, LinuxBIOS should be written completely in C.


>  	/* 2. Precharge all. Wait tRP. */
>  	PRINT_DEBUG("RAM Enable 2: Precharge all\r\n");
> -	do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0);
> -	mdelay(10);
> +	do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0x2000);

Why 0x2000 here?


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070501/aafe5253/attachment.sig>