[coreboot] [PATCH] More Kconfig changes

Myles Watson mylesgw at gmail.com
Mon Oct 26 16:19:02 CET 2009


On Sat, Oct 24, 2009 at 5:11 PM, Uwe Hermann <uwe at hermann-uwe.de> 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 <uwe at hermann-uwe.de> 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 <mylesgw at gmail.com>
>
> Acked-by: Uwe Hermann <uwe at hermann-uwe.de>
>
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20091026/d3f70277/attachment.html>


More information about the coreboot mailing list