Re: [coreboot] [PATCH] First part of heterogenous dualchannel support

[Carl-Daniel Hailfinger]

On 16.01.2009 04:33, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
> > Calculate tCL related settings per DIMM in a dual channel setup.
>
> ..
> > +++ LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/raminit_f.c	(Arbeitskopie)
> >    /* Make a second pass through the dimms and disable
> >     * any that cannot support the selected memclk and cas latency.
>
> This (existing) comment is reassuring.

Unfortunately, it only checks one DIMM per channel, so the comment is a
bit misleading. I'll update that code as a next step, but probably won't
finish it before the end of the week, so if anybody has time...
> > +int get_dimm_Trc_clocks(u32 spd_device, const struct mem_param *param)
>
> ..
> > if (clocks < DTL_TRC_MIN) {
> > +#warning We should die here or at least disable this bank.
> > printk_notice("update_dimm_Trc: can't refresh fast enough, "
> > 	"want %i clocks, can %i clocks\n", clocks, DTL_TRC_MIN);
> > clocks = DTL_TRC_MIN;
> > }
> > return clocks;
> >
> > +}
>
> This (existing) code not so much.

I thought a warning would be in order instead of ignoring a possibly
catastrophical misconfiguration. However, I am not totally sure, so
adding a die() there was too extreme for my taste.
I'd appreciate any insight on the code in question.
> How to combine the two?

Unfortunately, they are unrelated code paths, so combining them is not
feasible.
> Acked-by: Peter Stuge

Thanks, r3867.

Regards,
Carl-Daniel