Re: [coreboot] r1157 - coreboot-v3/northbridge/amd/geodelx

[Peter Stuge]

[e-mail] wrote:
> speed = 2 * ((10000 / (((spd_byte0 >> 4) * 10) + (spd_byte0 & 0x0F))));
> speed = 20000 / (((spd_byte0 >> 4) * 10) + (spd_byte0 & 0x0F));

..
> 	dimm_speed = 2 * (10000 / (((spd_byte >> 4) * 10) +
> 				(spd_byte & 0x0F)));
> 	dimm_speed = 20000 / (((spd_byte >> 4) * 10) + (spd_byte & 0x0F));

..
> 			dimm_speed = 2 * (10000 / (((spd_byte >> 4) * 10) + (spd_byte & 0x0F)));
> 			dimm_speed = 20000 / (((spd_byte >> 4) * 10) + (spd_byte & 0x0F));

..
..
> 	dimm_speed = 2 * (10000 / (((spd_byte >> 4) * 10) + (spd_byte & 0x0F)));
> 	dimm_speed = 20000 / (((spd_byte >> 4) * 10) + (spd_byte & 0x0F));

..
> 			dimm_speed = 2 * (10000 / (((spd_byte >> 4) * 10) + (spd_byte & 0x0F)));
> 			dimm_speed = 20000 / (((spd_byte >> 4) * 10) + (spd_byte & 0x0F));

Anyone else thinks this calls for a macro? Maybe even a generic SPD
macro?

//Peter

r1157 - coreboot-v3/northbridge/amd/geodelx

[Peter Stuge]

Peter Stuge wrote:
> [e-mail] wrote:
> > speed = 2 * ((10000 / (((spd_byte0 >> 4) * 10) + (spd_byte0 & 0x0F))));
> > speed = 20000 / (((spd_byte0 >> 4) * 10) + (spd_byte0 & 0x0F));
>
> ..
>
> Anyone else thinks this calls for a macro? Maybe even a generic SPD
> macro?

As it turns out, some restructuring here would be nice.

Making use of goto in a select few places makes it very obvious that
this function isn't completely consistent. Is that a potential
problem for RAM init or is it just an academic discovery? I don't
have enough experience from SPD data..

I would think that every occurence of goto dimm1 in my patch should
be preceded by casmap0=0. Do you agree?

I suggest that this patch does not change how the code works.

//Peter

r1157 - coreboot-v3/northbridge/amd/geodelx

[Peter Stuge]

Peter Stuge wrote:
> I suggest that this patch does not change how the code works.

This time with attachment.

//Peter

r1157 - coreboot-v3/northbridge/amd/geodelx

[Marc Jones]

2009/3/19 Peter Stuge :
> Peter Stuge wrote:
> > I suggest that this patch does not change how the code works.

I think that there is a mistake in the v3 code. I don't think that it
does the same thing as the v2 code. I think that the v2 code is easier
to understand and doesn't require the gotos. I would like to know what
you think about it.

Marc