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 <[EMAIL PROTECTED]>
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 <[EMAIL PROTECTED]>
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
signature.asc
Description: Digital signature
-- linuxbios mailing list linuxbios@linuxbios.org http://www.linuxbios.org/mailman/listinfo/linuxbios
