[LinuxBIOS] i440bx RAM initialization code (SPD)
Alfred Wanga
awanga at gmail.com
Wed May 30 04:36:33 CEST 2007
Apologies everyone... I meant to tweak it after the feedback, but never could
seem to get around to it. Here is the original patch again.
Signed-off-by: Alfred Wanga <awanga at gmail.com>
Uwe Hermann wrote:
> 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.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: i440bx_raminit_spd.patch
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070529/b073c6a8/attachment.ksh>
More information about the coreboot
mailing list