DBE62 v3 strangeness since v3 rev 649

[Carl-Daniel Hailfinger]

Hi Ron,

this one is for you: Your commit 649 had a few probably unintended side
effects:
- set_delay_control thinks zero DIMMs are installed because
SPD_PRIMARY_SDRAM_WIDTH does not exist in the SPD table.
- Due to that, the GLCP_DELAY_CONTROLS msr is set to 0.

Please confirm that you really wanted these settings.

Regards,
Carl-Daniel

[ron minnich]

are you running on a dbe62? good news!

That's probably a mistake. If you set it back does it work?

thanks

ron

[Carl-Daniel Hailfinger]

On 17.04.2008 18:08, ron minnich wrote:
> are you running on a dbe62? good news!

No, Mart Raudsepp ran it. I'm one of those strange people who actually
read logs and compare them to what would be expected by reading the code.
By the way, diffing coreboot output at maximum verbosity before and
after a patch is a great way to make sure the commit didn't do
unintended things.
> That's probably a mistake. If you set it back does it work?

You were the one making the changes (surely with a reason) and the new
code seemed to work for you. I can prepare a patch but someone would
have to test.

Marc Jones wrote that the current state of code is unlikely to work.
That's another reason why I would like to know how the
SPD_PRIMARY_SDRAM_WIDTH removal happened.

Regards,
Carl-Daniel

[ron minnich]

On Thu, Apr 17, 2008 at 4:25 PM, Carl-Daniel Hailfinger
 wrote:
> On 17.04.2008 18:08, ron minnich wrote:
> > are you running on a dbe62? good news!
>
> No, Mart Raudsepp ran it. I'm one of those strange people who actually
>  read logs and compare them to what would be expected by reading the code.
>  By the way, diffing coreboot output at maximum verbosity before and
>  after a patch is a great way to make sure the commit didn't do
>  unintended things.

it's weird as I test before every commit. So why this failed? I don't know.
> Marc Jones wrote that the current state of code is unlikely to work.
>  That's another reason why I would like to know how the
>  SPD_PRIMARY_SDRAM_WIDTH removal happened.

That puzzles me too. As of my last test it was all working. I will try
to do a burn tonight before and after svn up and see how it goes.

ron

[ron minnich]

On Tue, Apr 15, 2008 at 2:52 PM, Carl-Daniel Hailfinger
 wrote:
> Hi Ron,
>
> this one is for you: Your commit 649 had a few probably unintended side
>  effects:
> set_delay_control thinks zero DIMMs are installed because
>
> SPD_PRIMARY_SDRAM_WIDTH does not exist in the SPD table.
> Due to that, the GLCP_DELAY_CONTROLS msr is set to 0.
>
> Please confirm that you really wanted these settings.

This mistake arose out of my attempt to clean up the namings of things
in the original initram. It had names from two sources.

This patch cleans up that error, and is my attempt at cleaning up the
null pointer
issue in device/device.c, which I actually don't understand.

This patch allows me to almost boot a Plan 9 kernel from flash.

ron

Fwd: DBE62 v3 strangeness since v3 rev 649

[ron minnich]

If somebody could ack this I can close out the recent dbe62 problems.

ron

---------- Forwarded message ----------
From: ron minnich 
Date: Fri, Apr 18, 2008 at 10:09 PM
Subject: Re: DBE62 v3 strangeness since v3 rev 649
To: Carl-Daniel Hailfinger 
Cc: Coreboot

On Tue, Apr 15, 2008 at 2:52 PM, Carl-Daniel Hailfinger

 wrote:
> Hi Ron,
>
> this one is for you: Your commit 649 had a few probably unintended side
>  effects:
> set_delay_control thinks zero DIMMs are installed because
>
> SPD_PRIMARY_SDRAM_WIDTH does not exist in the SPD table.
> Due to that, the GLCP_DELAY_CONTROLS msr is set to 0.
>
> Please confirm that you really wanted these settings.

This mistake arose out of my attempt to clean up the namings of things
 in the original initram. It had names from two sources.

This patch cleans up that error, and is my attempt at cleaning up the
 null pointer
 issue in device/device.c, which I actually don't understand.

This patch allows me to almost boot a Plan 9 kernel from flash.

ron

Fwd: DBE62 v3 strangeness since v3 rev 649

[Carl-Daniel Hailfinger]

On 29.04.2008 05:55, ron minnich wrote:
> If somebody could ack this I can close out the recent dbe62 problems.

Sure, but see below.
> ron
>
> ---------- Forwarded message ----------
> From: ron minnich 
> Date: Fri, Apr 18, 2008 at 10:09 PM
> Subject: Re: DBE62 v3 strangeness since v3 rev 649
> To: Carl-Daniel Hailfinger 
> Cc: Coreboot
>
> On Tue, Apr 15, 2008 at 2:52 PM, Carl-Daniel Hailfinger
>
>  wrote:
> > Hi Ron,
> >
> > this one is for you: Your commit 649 had a few probably unintended side
> >  effects:
> > set_delay_control thinks zero DIMMs are installed because
> >
> > SPD_PRIMARY_SDRAM_WIDTH does not exist in the SPD table.
> > Due to that, the GLCP_DELAY_CONTROLS msr is set to 0.
> >
> > Please confirm that you really wanted these settings.
>
> This mistake arose out of my attempt to clean up the namings of things
>  in the original initram. It had names from two sources.
>
> This patch cleans up that error, and is my attempt at cleaning up the
>  null pointer
>  issue in device/device.c, which I actually don't understand.

That's the big problem. I have no idea whether it is allowed to have
bus->dev==NULL at that point in time. If it is not allowed, we should
add a big fat warning. If it is allowed, your fix is absolutely correct.
> This patch allows me to almost boot a Plan 9 kernel from flash.

Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

Fwd: DBE62 v3 strangeness since v3 rev 649

[Marc Jones]

ron minnich wrote:
> If somebody could ack this I can close out the recent dbe62 problems.
>
> ron

Acked-by: Marc Jones
> ---------- Forwarded message ----------
> From: ron minnich 
> Date: Fri, Apr 18, 2008 at 10:09 PM
> Subject: Re: DBE62 v3 strangeness since v3 rev 649
> To: Carl-Daniel Hailfinger 
> Cc: Coreboot
>
> On Tue, Apr 15, 2008 at 2:52 PM, Carl-Daniel Hailfinger
>
>  wrote:
> > Hi Ron,
> >
> > this one is for you: Your commit 649 had a few probably unintended side
> >  effects:
> > set_delay_control thinks zero DIMMs are installed because
> >
> > SPD_PRIMARY_SDRAM_WIDTH does not exist in the SPD table.
> > Due to that, the GLCP_DELAY_CONTROLS msr is set to 0.
> >
> > Please confirm that you really wanted these settings.
>
> This mistake arose out of my attempt to clean up the namings of things
>  in the original initram. It had names from two sources.
>
> This patch cleans up that error, and is my attempt at cleaning up the
>  null pointer
>  issue in device/device.c, which I actually don't understand.
>
> This patch allows me to almost boot a Plan 9 kernel from flash.
>
> ron

Fwd: DBE62 v3 strangeness since v3 rev 649

[ron minnich]

On Tue, Apr 29, 2008 at 8:44 AM, Marc Jones  wrote:
> ron minnich wrote:
> > If somebody could ack this I can close out the recent dbe62 problems.
> >
> > ron
>
> Acked-by: Marc Jones

Committed revision 672.