[coreboot] v2[PATCH]RCA RM4100 i82830 support

[Peter Stuge]

On Mon, Feb 25, 2008 at 11:12:58PM -0500, joe at smittys.pointclark.net wrote:
> >> +	print_err("are not supported on this board\r\n");
> >
> > board -> northbridge or chipset, in all of these.
> 
> Not sure what you mean here.

Change the word "board" to either "northbridge" or "chipset" because
this isn't board-specific.


> >> +				die("HALT\r\n");
> >
> > Why die?
> 
> Die becaause the memory is not supported, right?

Maybe it is possible to degrade and carry on?


> >> +			if ((sz.side2 != 0) && (sz.side1 != sz.side2)) {
> >> +				print_err("This board only supports\r\n");
> >> +				print_err("symmetrical dual-sided DIMMs\r\n");
> >> +				die("HALT\r\n");
> >> +			}
> >
> > This should be moved right into the detection routine. If the   
> > northbridge can't handle it, then just die if sz.side1 |= sz.side2.
> > The other option is to set sz.side2 to 0, and try to use it like a   
> > single-sided dimm.
> 
> Hmm above is saying that IF THERE IS A SECOND SIDE and it does not  
> equal the first side than die, otherwise it is assumed it is single  
> sided and moves on.
> 
> "if sz.side1 |= sz.side2" does not work for single sided so-dimms,
> correct?

Yes. I think that was a typo, it should probably have been != instead.


> >> +				drb1 = sz.side1 >> 5;
> >
> > Please don't use bitwise shifts for multiplication/division,
> 
> ok will change, but there was quite a discussion about this a while
> ago and half thought it was ok and half didn't. So which is correct
> in "C" standards?

I say depends on the case. If the algorithm is mathematical I say use
/ and * but if it is primarily about moving bits then shift is fine.


//Peter