[LinuxBIOS] PATCH: add the pcengines ALIX1

Peter Stuge peter at stuge.se
Sat Sep 8 20:52:57 CEST 2007


Hey,

On Sat, Sep 08, 2007 at 08:37:21PM +0200, Uwe Hermann wrote:
> On Sat, Sep 08, 2007 at 09:28:34AM -0700, ron minnich wrote:
> > Hi, here is try 2. This one builds :-)
> > +++ src/mainboard/pcengines/alix1c/reset.c	(revision 0)
> > @@ -0,0 +1,43 @@
> > +#if 0
> 
> As it's commented out (and likely copied from some other target),
> please drop the file completely for now. We can add a working
> version (or preferrably use a global/common file if possible) if
> and when we need it.
> I don't like adding dummy files which aren't really necessary.

Agreed. I don't think this file can be made generic though - but I'm
not completely sure.


> Is this file even used ATM? I see a
>   default HAVE_HARD_RESET=0
> in Options.lb.

Right - this means the file is not used, and it should be removed.


..

> HTH, Uwe.

Thanks for the review Uwe. You found a lot of (IMHO) important issues
in the patch.

It would be great if all of these issues could be investigated and
fixed now that the patch has been committed already.

(Or we could decide that we don't care at all about the state or
quality of v2 but then code will rot and we will lose support for
more boards - like we did with v1. Let's not repeat that mistake.)

Thanks!


//Peter




More information about the coreboot mailing list