Re: [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

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

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.