[LinuxBIOS] [PATCH] Fix previous patch for Asus MEW-VM

[Uwe Hermann]

On Thu, Jun 14, 2007 at 04:36:05AM -0400, Joseph Smith wrote:
> > Thanks guys. Somehow I let the i82810 slip by completely, and missed a
> > couple lines when I was manually editing the i82801. Attached patch
> > fixes those.
> >
> > Signed-off-by: Corey Osgood <corey.osgood at gmail.com>

Committed in r2720 and r2721 with some minor changes, thanks!

One more thing I noticed in the 82810 code (but didn't fix, yet):

 In spd_set_dram_size() the nesting level is quite deep (you notice
 immediately because the lines exceed 79 characters).

 If you change

        /* First check if a DIMM is actually present. */
        if (smbus_read_byte(ctrl->channel0[i], 2) == 4) {
		// FOO
	} else {
		// BAR
	}

  to:

        /* First check if a DIMM is actually present. */
        if (smbus_read_byte(ctrl->channel0[i], 2) != 4) {
		// BAR
		continue;	// or break? need to check.
	}

	// FOO

  that will make the code a lot more readable, IMO (FOO is a huge
  amount of code which needs to be indented quite a lot otherwise).


> Oh your right Corey, Oops. We should probibly get your patch commited  
> asap right?
> 
> Acked-by: Joseph Smith <joe at smittys.pointclark.net>

Did you review and/or test the patch? Acked-by is not merely a "I like it"
but should always involve at least some degree of review of the code
or even better testing the build and testing on real hardware (if possible).


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070614/6c992986/attachment.sig>