[PATCH] v3: fix bugs in Geode LX RAMinit

[Carl-Daniel Hailfinger]

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

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]

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

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
> 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);

[Carl-Daniel Hailfinger]

On 07.03.2008 19:16, Marc Jones wrote:
> 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
>
> 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.

The raminit in v2 was fixed in r2899 | rminnich | 2007-10-26 with this log:
> The lxraminit change fixes a bug (&& used instead of ||) [...]
> Signed-off-by: Ronald G. Minnich 
> Acked-by: Peter Stuge

On 07.03.2008 19:16, Marc Jones wrote:
> 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.

You're welcome.
> Acked-by: Marc Jones

Thanks! I will wait a bit before committing to give Ron the chance to test.

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

On 07.03.2008 20:15, Carl-Daniel Hailfinger wrote:
> On 07.03.2008 19:16, Marc Jones wrote:
> > 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
> >
> > 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.
>
> The raminit in v2 was fixed in r2899 | rminnich | 2007-10-26 with this log:
> > The lxraminit change fixes a bug (&& used instead of ||) [...]
> > Signed-off-by: Ronald G. Minnich 
> > Acked-by: Peter Stuge
>
> On 07.03.2008 19:16, Marc Jones wrote:
> > 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.
>
> You're welcome.
> > Acked-by: Marc Jones
>
> Thanks! I will wait a bit before committing to give Ron the chance to test.

I saw no further responses and committed this in r659.

Regards,
Carl-Daniel

[ron minnich]

On Wed, Apr 16, 2008 at 9:41 AM, Carl-Daniel Hailfinger
 wrote:
> I saw no further responses and committed this in r659.

OK, will try to test tonight.

I need some help on the LX, if anyone else out there has one .... I am
stuck on MFGPT issues.

ron

[Peter Stuge]

On Wed, Apr 16, 2008 at 12:11:38PM -0700, ron minnich wrote:
> I need some help on the LX, if anyone else out there has one ....
> I am stuck on MFGPT issues.

Am waiting for delivery. Can you describe the problem?

//Peter

[ron minnich]

On Fri, Apr 18, 2008 at 7:35 AM, Peter Stuge  wrote:
> On Wed, Apr 16, 2008 at 12:11:38PM -0700, ron minnich wrote:
> > I need some help on the LX, if anyone else out there has one ....
> > I am stuck on MFGPT issues.
>
> Am waiting for delivery. Can you describe the problem?

it's easy. Enable MFGPT, and watch the machine hang on boot. It's not
a hard one to exercise. What system are you getting again?

Also, for dbe62, we need to have usb or ethernet working from whatever
booter we choose. We need a way to load or reload the dbe62. I am
thinking etherboot may be best, save that I have never succeeded for
years in building a working etherboot ;-)

comments? (I don't see FILO+USB as practical at this point but am
willing to be proven wrong)

ron

[Ward Vandewege]

On Fri, Apr 18, 2008 at 07:45:20AM -0700, ron minnich wrote:
> On Fri, Apr 18, 2008 at 7:35 AM, Peter Stuge  wrote:
> > On Wed, Apr 16, 2008 at 12:11:38PM -0700, ron minnich wrote:
> > > I need some help on the LX, if anyone else out there has one ....
> > > I am stuck on MFGPT issues.
> >
> > Am waiting for delivery. Can you describe the problem?
>
> it's easy. Enable MFGPT, and watch the machine hang on boot. It's not
> a hard one to exercise. What system are you getting again?
>
> Also, for dbe62, we need to have usb or ethernet working from whatever
> booter we choose. We need a way to load or reload the dbe62. I am
> thinking etherboot may be best, save that I have never succeeded for
> years in building a working etherboot ;-)

Buildrom generates a working etherboot payload. We've got half a dozen
workstations that boot that way.

I don't have a dbe62 but I do have alix.1c and 2c3. Have we established if
the MFGPT problem manifests itself on all LX boards?

Thanks,
Ward.

[ron minnich]

On Fri, Apr 18, 2008 at 7:57 AM, Ward Vandewege  wrote:
> I don't have a dbe62 but I do have alix.1c and 2c3. Have we established if
>  the MFGPT problem manifests itself on all LX boards?

only seen on my alix so far but that's just because I'm not really
booting on the dbe62 yet; still trying to figure out how we load the
dbe62 up when the onboard flash is empty (and using the factory bios
to load it and then switching to coreboot is not an acceptable answer
:-)

ron

v3: fix bugs in Geode LX RAMinit

[Jordan Crouse]

On 18/04/08 10:57 -0400, Ward Vandewege wrote:
> Buildrom generates a working etherboot payload. We've got half a dozen
> workstations that boot that way.
>
> I don't have a dbe62 but I do have alix.1c and 2c3. Have we established if
> the MFGPT problem manifests itself on all LX boards?

Unclear.  They work flawlessly with v2.

Jordan

[Peter Stuge]

On Fri, Apr 18, 2008 at 07:45:20AM -0700, ron minnich wrote:
> > > I need some help on the LX, if anyone else out there has one ....
> > > I am stuck on MFGPT issues.
> >
> > Am waiting for delivery. Can you describe the problem?
>
> it's easy. Enable MFGPT, and watch the machine hang on boot. It's
> not a hard one to exercise.

Any idea as to the cause?
> What system are you getting again?

1c and 3c3

//Peter

[ron minnich]

On Sat, Apr 19, 2008 at 11:09 AM, Peter Stuge  wrote:
> On Fri, Apr 18, 2008 at 07:45:20AM -0700, ron minnich wrote:
> > > > I need some help on the LX, if anyone else out there has one ....
> > > > I am stuck on MFGPT issues.
> > >
> > > Am waiting for delivery. Can you describe the problem?
> >
> > it's easy. Enable MFGPT, and watch the machine hang on boot. It's
> > not a hard one to exercise.
>
> Any idea as to the cause?

Current theory is it's waiting forever for the MFGPT IRQ and it's not
getting it. So, misconfigured interrupts.

ron