On Sat, Oct 24, 2009 at 5:11 PM, Uwe Hermann <[email protected]> wrote:
> On Fri, Oct 23, 2009 at 05:06:35PM -0600, Myles Watson wrote: > > On Fri, Oct 23, 2009 at 4:56 PM, Uwe Hermann <[email protected]> wrote: > > > > > On Fri, Oct 23, 2009 at 02:43:27PM -0600, Myles Watson wrote: > > > > Ping. I think I've addressed all of Peter and Uwe's concerns. > > > > > > I Think so, but please repost the updated patch so we can have a look > > > at the final version. > > > > > I don't think the first patches have changed much, but there are two new > > ones that apply on top: peter.diff and uwe.diff. > > > > Signed-off-by: Myles Watson <[email protected]> > > Acked-by: Uwe Hermann <[email protected]> > Rev 4856. > > > +config HAVE_LOW_TABLES > > + bool > > + default y > > + help > > + This Option is unused in the code. Since two boards try to set > it to > > + 'n', they may be broken. We either need to make the option > useful or > > + get rid of it. The broken boards are: > > + asus/m2v-mx_se > > + supermicro/h8dme > > Please add a "# TODO" comment on top of that for easy grepping. > Done. > > +config MEM_TRAIN_SEQ > > + int > > + default 2 > > We should add a MEM_TRAIN_SEQ comment or help text, it's unclear to me what > it is > supposed to do. Is it a per-chipset, per-cpu, or per-board option? What > do the values of the variable mean? > All good questions. It's unclear to me. > Regardless of that, it seems like it should be a user-visible option in > menuconfig with useful option names for the values of 0, 1, and 2 (which > seem to be the only valid ones). > The more things we make "user configurable", the more ways a user can break the build/ brick his board. Thanks, Myles
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot

