On Fri, Sep 25, 2009 at 07:37:19PM +0200, Patrick Georgi wrote: > - via/pc2500e
Neat, I'll give this one a test run on hardware soonish. > Current status: > The tree has 114 boards, of which 46 build with kconfig now, so there > are 68 missing. > > As the patch has grown a bit, see > http://www.coresystems.de/~patrick/current.patch Please post to the list nevertheless (with Signed-off-by), it's much easier to review that way. Some parts inlined here for review: > Index: src/southbridge/via/Makefile.inc > =================================================================== > --- src/southbridge/via/Makefile.inc (Revision 4672) > +++ src/southbridge/via/Makefile.inc (Arbeitskopie) > @@ -1,5 +1,4 @@ > -#subdirs-y += k8t890 > -#subdirs-y += vt8231 > -#subdirs-y += vt8235 > +subdirs-$(CONFIG_SOUTHBRIDGE_VIA_K8T890) += k8t890 > +subdirs-$(CONFIG_SOUTHBRIDGE_VIA_VT8231) += vt8231 > +subdirs-$(CONFIG_SOUTHBRIDGE_VIA_VT8235) += vt8235 > subdirs-$(CONFIG_SOUTHBRIDGE_VIA_VT8237R) += vt8237r > -#subdirs-y += vt82c686 This will still be required later I guess (?). > Index: src/southbridge/via/k8t890/Makefile.inc > =================================================================== > --- src/southbridge/via/k8t890/Makefile.inc (Revision 0) > +++ src/southbridge/via/k8t890/Makefile.inc (Revision 0) [...] > -# arg. How does the linux kconfig handle compound conditionals? > -ifeq ($(CONFIG_HAVE_SMI_HANDLER),y) > - object-$(CONFIG_SOUTHBRIDGE_INTEL_I82801GX) += i82801gx_smi.o > - smmobj-$(CONFIG_SOUTHBRIDGE_INTEL_I82801GX) += i82801gx_smihandler.o > -endif > +object-$(CONFIG_HAVE_SMI_HANDLER) += i82801gx_smi.o > +smmobj-$(CONFIG_HAVE_SMI_HANDLER) += i82801gx_smihandler.o What's the proposed resolution for these "compound conditionals"? > Index: src/cpu/intel/ep80579/Kconfig > =================================================================== > --- src/cpu/intel/ep80579/Kconfig (Revision 0) > +++ src/cpu/intel/ep80579/Kconfig (Revision 0) > @@ -0,0 +1,3 @@ > +config CPU_INTEL_EP80579 > + bool > + default false "default n" for consistency. > =================================================================== > --- src/cpu/intel/socket_mPGA603/Kconfig (Revision 0) > +++ src/cpu/intel/socket_mPGA603/Kconfig (Revision 0) > @@ -0,0 +1,6 @@ > +config CPU_INTEL_SOCKET_MPGA603 > + bool > + default false Ditto. > Index: src/cpu/intel/socket_mPGA604/Kconfig > =================================================================== > --- src/cpu/intel/socket_mPGA604/Kconfig (Revision 0) > +++ src/cpu/intel/socket_mPGA604/Kconfig (Revision 0) > @@ -0,0 +1,6 @@ > +config CPU_INTEL_SOCKET_MPGA604 > + bool > + default false Ditto. > Index: src/cpu/intel/socket_mPGA478/Kconfig > =================================================================== > --- src/cpu/intel/socket_mPGA478/Kconfig (Revision 0) > +++ src/cpu/intel/socket_mPGA478/Kconfig (Revision 0) > @@ -0,0 +1,5 @@ > +config CPU_INTEL_SOCKET_MPGA478 > + bool > + default false Ditto. > Index: src/cpu/intel/model_1067x/Kconfig > =================================================================== > --- src/cpu/intel/model_1067x/Kconfig (Revision 0) > +++ src/cpu/intel/model_1067x/Kconfig (Revision 0) > @@ -0,0 +1,5 @@ > +config CPU_INTEL_CORE2 > + bool > + default y > + select SMP > + select HAVE_MOVNTI The syntax (i.e. using default y) means that all model_1067x are Core2? Is that correct? > Index: src/cpu/intel/socket_mPGA479M/Kconfig > =================================================================== > --- src/cpu/intel/socket_mPGA479M/Kconfig (Revision 0) > +++ src/cpu/intel/socket_mPGA479M/Kconfig (Revision 0) > @@ -0,0 +1,5 @@ > +config CPU_INTEL_SOCKET_MPGA479M > + bool > + default false See above. > Index: src/mainboard/supermicro/h8dme/Kconfig > =================================================================== > --- src/mainboard/supermicro/h8dme/Kconfig (Revision 0) > +++ src/mainboard/supermicro/h8dme/Kconfig (Revision 0) [...] > +config HAVE_HARD_RESET > + bool > + default y > + depends on BOARD_SUPERMICRO_H8DME This can be a simple "select HAVE_HARD_RESET" above, but I assume you'll do this kind of changes with a script later? > Index: src/mainboard/supermicro/h8dme/Makefile.inc > =================================================================== > --- src/mainboard/supermicro/h8dme/Makefile.inc (Revision 0) > +++ src/mainboard/supermicro/h8dme/Makefile.inc (Revision 0) [...] > +ifdef POST_EVALUATION > + > +MAINBOARD_OPTIONS=\ > + -DCONFIG_AP_IN_SIPI_WAIT=0 \ > + -DCONFIG_USE_PRINTK_IN_CAR=1 \ > + -DCONFIG_HAVE_HIGH_TABLES=1 What's the difference between using MAINBOARD_OPTIONS here vs. adding these settings in the Kconfig file for the respective mainboard? Can't we eliminate MAINBOARD_OPTIONS? > Index: src/mainboard/tyan/s2850/Kconfig > =================================================================== > --- src/mainboard/tyan/s2850/Kconfig (Revision 0) > +++ src/mainboard/tyan/s2850/Kconfig (Revision 0) > @@ -0,0 +1,65 @@ > +config BOARD_TYAN_S2850 > + bool "Tyan S2850" Nope, only "S2850" here, vendor name should not be in here. Maybe "Tomcat K8S (S2850)" as per wiki / vendor website, see http://www.coreboot.org/Supported_Motherboards Same for some other Tyan boards, and maybe also some non-Tyan, didn't check. > Index: src/mainboard/via/epia-m/auto.c > =================================================================== > --- src/mainboard/via/epia-m/auto.c (Revision 4672) > +++ src/mainboard/via/epia-m/auto.c (Arbeitskopie) > @@ -20,7 +20,7 @@ > > /* > */ > -void udelay(int usecs) > +void udelay(unsigned usecs) "unsigned int" then, for consistency. > Index: src/mainboard/via/epia/auto.c > =================================================================== > --- src/mainboard/via/epia/auto.c (Revision 4672) > +++ src/mainboard/via/epia/auto.c (Arbeitskopie) > @@ -16,7 +16,7 @@ > > /* > */ > -void udelay(int usecs) > +void udelay(unsigned usecs) Ditto. > Index: src/mainboard/via/pc2500e/Kconfig > =================================================================== > --- src/mainboard/via/pc2500e/Kconfig (Revision 0) > +++ src/mainboard/via/pc2500e/Kconfig (Revision 0) > @@ -0,0 +1,45 @@ > +config BOARD_VIA_PC2500E > + bool "PC2500E" pc2500e in this case, the vendor also writes it in all-lowercase. Maybe I'll fix this type of strings in one big committ soonish. > Index: src/mainboard/Makefile.romccboard.inc > =================================================================== > --- src/mainboard/Makefile.romccboard.inc (Revision 4672) > +++ src/mainboard/Makefile.romccboard.inc (Arbeitskopie) > @@ -44,15 +44,17 @@ > > ifdef POST_EVALUATION > > +CPUTYPE ?= p2 Yep, this kind of fix was on my TODO list also. Maybe ROMCC_MCPU (a bit more descriptive I guess)? > + > $(obj)/mainboard/$(MAINBOARDDIR)/failover.inc: $(obj)/romcc > $(src)/arch/i386/lib/failover.c > - $(obj)/romcc -mcpu=p2 -O2 --label-prefix=failover $(INCLUDES) > $(src)/arch/i386/lib/failover.c -o $@ > + $(obj)/romcc -mcpu=$(CPUTYPE) -O2 --label-prefix=failover $(INCLUDES) > $(src)/arch/i386/lib/failover.c -o $@ > > ifeq ($(CONFIG_HAVE_OPTION_TABLE),y) > $(obj)/mainboard/$(MAINBOARDDIR)/auto.inc: $(obj)/romcc > $(src)/mainboard/$(MAINBOARDDIR)/auto.c $(obj)/option_table.h > - $(obj)/romcc -mcpu=p2 -O2 $(INCLUDES) > $(src)/mainboard/$(MAINBOARDDIR)/auto.c -o $@ > + $(obj)/romcc -mcpu=$(CPUTYPE) -O2 $(INCLUDES) > $(src)/mainboard/$(MAINBOARDDIR)/auto.c -o $@ > else > $(obj)/mainboard/$(MAINBOARDDIR)/auto.inc: $(obj)/romcc > $(src)/mainboard/$(MAINBOARDDIR)/auto.c > - $(obj)/romcc -mcpu=p2 -O2 $(INCLUDES) > $(src)/mainboard/$(MAINBOARDDIR)/auto.c -o $@ > + $(obj)/romcc -mcpu=$(CPUTYPE) -O2 $(INCLUDES) > $(src)/mainboard/$(MAINBOARDDIR)/auto.c -o $@ > endif > Index: src/mainboard/intel/xe7501devkit/Makefile.inc > =================================================================== > --- src/mainboard/intel/xe7501devkit/Makefile.inc (Revision 0) > +++ src/mainboard/intel/xe7501devkit/Makefile.inc (Revision 0) > @@ -0,0 +1,13 @@ > +CPUTYPE := p4 > +obj-$(CONFIG_HAVE_ACPI_TABLES) += acpi_tables.o > +ifeq ($(CONFIG_PCI_ROM_RUN),y) > + ifeq ($(CONFIG_PCI_ROM_RUN),y) This looks incorrect. Wrong variable name in the second ifeq? > + obj-y += vgarom.o > + else > + obj-y += no_vgarom.o > + endif > +else > + obj-y += no_vgarom.o > +endif > +include $(src)/mainboard/Makefile.romccboard.inc > Index: src/mainboard/intel/truxton/Makefile.inc > =================================================================== > --- src/mainboard/intel/truxton/Makefile.inc (Revision 0) > +++ src/mainboard/intel/truxton/Makefile.inc (Revision 0) > @@ -0,0 +1,3 @@ > +CPUTYPE := p4 -fno-simplify-phi Hm, this is a bit of a hack, maybe make a generic ROMCC_CFLAGS then like this? ROMCC_CFLGAGS = -mcpu=p4 -fno-simplify-phi Or add another variable in addition to CPUTYPE. > Index: src/mainboard/intel/jarrell/Makefile.inc > =================================================================== > --- src/mainboard/intel/jarrell/Makefile.inc (Revision 0) > +++ src/mainboard/intel/jarrell/Makefile.inc (Revision 0) > @@ -0,0 +1,4 @@ > +obj-y += reset.o This should probably be protected via CONFIG_HAVE_HARD_RESET and can then go into the generic Makefile.romccboard.inc, I think. > +CPUTYPE := p4 > +include $(src)/mainboard/Makefile.romccboard.inc > Index: src/northbridge/via/cn700/Kconfig > =================================================================== > --- src/northbridge/via/cn700/Kconfig (Revision 0) > +++ src/northbridge/via/cn700/Kconfig (Revision 0) [...] > +config HAVE_HIGH_TABLES > + bool > + default y > + depends on NORTHBRIDGE_VIA_CN700 I think HAVE_HIGH_TABLES is y per default already, so this can be omitted (?) The interesting question is whether or not all boards/chipsets fully work with HAVE_HIGH_TABLES already (I don't know the answer)... The same issue also applies to some other boards. > Index: src/northbridge/amd/Makefile.inc > =================================================================== > --- src/northbridge/amd/Makefile.inc (Revision 4672) > +++ src/northbridge/amd/Makefile.inc (Arbeitskopie) > @@ -1,7 +1,5 @@ > subdirs-$(CONFIG_NORTHBRIDGE_AMD_AMDFAM10) += amdfam10 > -subdirs-$(CONFIG_NORTHBRIDGE_AMD_AMDHT) += amdht > subdirs-$(CONFIG_NORTHBRIDGE_AMD_AMDK8) += amdk8 > -subdirs-$(CONFIG_NORTHBRIDGE_AMD_AMDMCT) += amdmct Why are these two removed? > Index: src/northbridge/intel/i440bx/Kconfig > =================================================================== > --- src/northbridge/intel/i440bx/Kconfig (Revision 4672) > +++ src/northbridge/intel/i440bx/Kconfig (Arbeitskopie) > +config HAVE_HIGH_TABLES > + bool > + default y No depends on 440BX NB here? > Index: src/northbridge/intel/i945/Makefile.inc > =================================================================== > --- src/northbridge/intel/i945/Makefile.inc (Revision 4672) > +++ src/northbridge/intel/i945/Makefile.inc (Arbeitskopie) > @@ -17,8 +17,6 @@ > # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > # > > -driver-$(CONFIG_NORTHBRIDGE_INTEL_I945) += northbridge.o > -driver-$(CONFIG_NORTHBRIDGE_INTEL_I945) += gma.o > -ifeq ($(CONFIG_HAVE_ACPI_TABLES),y) > - obj-$(CONFIG_NORTHBRIDGE_INTEL_I945) += acpi.o > -endif > +driver-y += northbridge.o > +driver-y += gma.o > +obj-$(CONFIG_HAVE_ACPI_TABLES) += acpi.o Are the two versions really equivalent? > @@ -233,7 +242,7 @@ > CFLAGS = $(STACKPROTECT) $(INCLUDES) $(MAINBOARD_OPTIONS) -Os -nostdinc > CFLAGS += -nostdlib -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes > CFLAGS +=-Wwrite-strings -Wredundant-decls -Wno-trigraphs > -CFLAGS += -Werror-implicit-function-declaration -Wstrict-aliasing -Wshadow > +CFLAGS += -Wstrict-aliasing -Wshadow > CFLAGS += -fno-common -ffreestanding -fno-builtin -fomit-frame-pointer Needed for romcc boards which have lots of these warnings/errors I assume? Uwe. -- http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
