Re: [coreboot] [PATCH] 440BX registered SDRAM support

[Keith Hui]

> > if ((edosd & 0x84) == 0x84) {
> > edosd = 0x10; // Registered SDRAM
> > } else {
> > // Clear [4:3] in case it's EDO.
> > edosd &= 0x07;
> >
> > +//    } else if (edosd & 0x02) {
>
> Besides being commented out, this piece of code would never be executed,
> as there already is an else case.
> Also, modifying edosd in place is semi nice.

So is this good, not so good, or bad?

I want to know if I should split up edosd.
> Please clean this up before committing. Maybe consider using switch/case?
>
> Looks good otherwise.
> > +//        edosd |= 0x00;
> > if (edosd & 0x04) {
> > edosd |= 0x08; // SDRAM
> > }
> >
> > }
> > // Keep only [4:3].
> >
> > edosd &= 0x18;

Thanks
Keith