[coreboot] [PATCH]more kconfig

[Patrick Georgi]

Hi,

attached patch enables kconfig based builds for a couple more boards. As
usual, this merely passes build and a validity check that all variables
are defined that are defined by newconfig (but not necessarily the same
value).

The larger part of this (which is why I stopped porting more boards and
want to get this out first) is that I moved the -O2 flag for romcc
invocations into ROMCCFLAGS, as rca/rm4100 wants -O only.

Signed-off-by: Patrick Georgi 
Index: src/mainboard/axus/tc320/Makefile.inc
===
--- src/mainboard/axus/tc320/Makefile.inc	(Revision 4729)
+++ src/mainboard/axus/tc320/Makefile.inc	(Arbeitskopie)
@@ -18,6 +18,6 @@
 ## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 ##
 
-ROMCCFLAGS := -mcpu=i386
+ROMCCFLAGS := -mcpu=i386 -O2
 include $(src)/mainboard/Makefile.romccboard.inc
 
Index: src/mainboard/bcom/Kconfig
===
--- src/mainboard/bcom/Kconfig	(Revision 4729)
+++ src/mainboard/bcom/Kconfig	(Arbeitskopie)
@@ -23,7 +23,7 @@
 	depends on VENDOR_BCOM
 
 source "src/mainboard/bcom/winnet100/Kconfig"
-# source "src/mainboard/bcom/winnetp680/Kconfig"
+source "src/mainboard/bcom/winnetp680/Kconfig"
 
 endchoice
 
Index: src/mainboard/bcom/winnetp680/Kconfig
===
--- src/mainboard/bcom/winnetp680/Kconfig	(Revision 0)
+++ src/mainboard/bcom/winnetp680/Kconfig	(Revision 0)
@@ -0,0 +1,36 @@
+config BOARD_BCOM_WINNETP680
+	bool "WinNET P680"
+	select ARCH_X86
+	select CPU_VIA_C7
+	select NORTHBRIDGE_VIA_CN700
+	select SOUTHBRIDGE_VIA_VT8237R
+	select SUPERIO_WINBOND_W83697HF
+	select HAVE_PIRQ_TABLE
+	select UDELAY_TSC
+	select TSC_X86RDTSC_CALIBRATE_WITH_TIMER2
+
+config MAINBOARD_DIR
+	string
+	default bcom/winnetp680
+	depends on BOARD_BCOM_WINNETP680
+
+config MAINBOARD_PART_NUMBER
+	string
+	default "WinNET P680"
+	depends on BOARD_BCOM_WINNETP680
+
+config IRQ_SLOT_COUNT
+	int
+	default 10
+	depends on BOARD_BCOM_WINNETP680
+
+config VIDEO_MB
+	int
+	default 32
+	depends on BOARD_BCOM_WINNETP680
+
+config RAMBASE
+	hex
+	default 0x4000
+	depends on BOARD_BCOM_WINNETP680
+
Index: src/mainboard/bcom/winnetp680/Makefile.inc
===
--- src/mainboard/bcom/winnetp680/Makefile.inc	(Revision 0)
+++ src/mainboard/bcom/winnetp680/Makefile.inc	(Revision 0)
@@ -0,0 +1,61 @@
+##
+## This file is part of the coreboot project.
+##
+## Copyright (C) 2008 VIA Technologies, Inc.
+## (Written by Aaron Lwe  for VIA)
+##
+## This program is free software; you can redistribute it and/or modify
+## it under the terms of the GNU General Public License as published by
+## the Free Software Foundation; either version 2 of the License, or
+## (at your option) any later version.
+##
+## This program is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+## GNU General Public License for more details.
+##
+## You should have received a copy of the GNU General Public License
+## along with this program; if not, write to the Free Software
+## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
+##
+
+initobj-y += crt0.o
+obj-y += mainboard.o
+obj-$(CONFIG_HAVE_PIRQ_TABLE) += irq_tables.o
+obj-$(CONFIG_HAVE_MP_TABLE) += mptable.o
+
+obj-$(CONFIG_HAVE_ACPI_TABLES) += fadt.o
+obj-$(CONFIG_HAVE_ACPI_TABLES) += dsdt.o
+obj-$(CONFIG_HAVE_ACPI_TABLES) += acpi_tables.o
+
+ldscript-y += ../../../../src/arch/i386/init/ldscript_fallback_cbfs.lb
+ldscript-y += ../../../../src/cpu/x86/16bit/entry16.lds
+ldscript-y += ../../../../src/cpu/x86/16bit/reset16.lds
+ldscript-y += ../../../../src/arch/i386/lib/id.lds
+ldscript-y += ../../../../src/arch/i386/lib/failover.lds
+
+crt0-y += ../../../../src/cpu/x86/16bit/entry16.inc
+crt0-y += ../../../../src/cpu/x86/32bit/entry32.inc
+crt0-y += ../../../../src/cpu/x86/16bit/reset16.inc
+crt0-y += ../../../../src/arch/i386/lib/id.inc
+crt0-y += ../../../../src/cpu/x86/fpu/enable_fpu.inc
+crt0-y += ../../../../src/cpu/x86/mmx/enable_mmx.inc
+crt0-y += auto.inc
+crt0-y += ../../../../src/cpu/x86/mmx/disable_mmx.inc
+
+ifdef POST_EVALUATION
+
+$(obj)/mainboard/$(MAINBOARDDIR)/dsdt.o: $(obj)/dsdt.c
+	$(CC) $(DISTRO_CFLAGS) $(CFLAGS) $(CPPFLAGS) $(DEBUG_CFLAGS) -I$(src) -I. -c $< -o $@
+
+$(obj)/dsdt.c: $(src)/mainboard/$(MAINBOARDDIR)/dsdt.asl
+	iasl -p dsdt -tc $(src)/mainboard/$(MAINBOARDDIR)/dsdt.asl
+	mv dsdt.hex $@
+
+$(obj)/mainboard/$(MAINBOARDDIR)/auto.inc: $(src)/mainboard/$(MAINBOARDDIR)/auto.c $(obj)/option_table.h
+	$(CC) $(DISTRO_CFLAGS) $(CFLAGS) $(CPPFLAGS) $(DEBUG_CFLAGS) -I$(src) -I. -c -S  $(src)/mainboard/$(MAINBOARDDIR)/auto.c -o $@
+	perl -e 's/\.rodata/.rom.data/g' -pi $@
+	perl -e 's/\.text/.section .rom.text/g' -pi $@
+
+endif
+
Index: src/mainboard/bcom/wi

[coreboot] [PATCH]more kconfig

[Patrick Georgi]

Hi,

this time: AMD LX, AMD SC520, board by
iei, pcengines, technexion, technologics, thomson

All of them build, and pass my variables test.

Signed-off-by: Patrick Georgi 


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

[coreboot] [PATCH]more kconfig

[Patrick Georgi]

Hi,

attached patch makes all boards build with kconfig (incl. fam10). None
boot-tested.

Details:
- move declaration of romstrap files that must be added to the bootblock
into the related southbridge's Makefiles.
- amd/model_fxx, amd/model_10xxx provide their own init_timer and udelay
functions. disable udelay_io default, declare init_timer presence
- amd/sc520 provides its own udelay function (no init_timer). Declare as
such
- supermicro/h8dmr_fam10:
-- enable tinybootblock
-- align configuration a bit with newconfig's
-- move acpi object files to $(obj)
-- fix copy&paste bug in devicetree.cb
- emulation/qemu-x86 provides its own timer. Declare as such
- tyan/s2912_fam10:
-- enable tinybootblock
-- align configuration a bit with newconfig's

The remaining issues are:
- Make configurations match newconfig's
- Find bugs

Signed-off-by: Patrick Georgi 
Index: src/southbridge/via/k8t890/Makefile.inc
===
--- src/southbridge/via/k8t890/Makefile.inc (revision 4999)
+++ src/southbridge/via/k8t890/Makefile.inc (working copy)
@@ -7,3 +7,6 @@
 driver-y += k8t890_traf_ctrl.o
 driver-y += k8t890_error.o
 driver-y += k8m890_chrome.o
+
+chipset_bootblock_inc += $(src)/southbridge/via/k8t890/romstrap.inc
+chipset_bootblock_lds += $(src)/southbridge/via/k8t890/romstrap.lds
Index: src/southbridge/nvidia/mcp55/Makefile.inc
===
--- src/southbridge/nvidia/mcp55/Makefile.inc   (revision 4999)
+++ src/southbridge/nvidia/mcp55/Makefile.inc   (working copy)
@@ -14,3 +14,6 @@
 driver-$(CONFIG_GENERATE_ACPI_TABLES) += mcp55_fadt.o
 
 obj-y += mcp55_reset.o
+
+chipset_bootblock_inc += $(src)/southbridge/nvidia/mcp55/romstrap.inc
+chipset_bootblock_lds += $(src)/southbridge/nvidia/mcp55/romstrap.lds
Index: src/cpu/amd/model_fxx/Kconfig
===
--- src/cpu/amd/model_fxx/Kconfig   (revision 4999)
+++ src/cpu/amd/model_fxx/Kconfig   (working copy)
@@ -4,6 +4,7 @@
select USE_PRINTK_IN_CAR
select USE_DCACHE_RAM
select SSE
+   select HAVE_INIT_TIMER
 
 config CPU_ADDR_BITS
int
@@ -25,3 +26,8 @@
default 0x01000
depends on CPU_AMD_MODEL_FXX
 
+
+config UDELAY_IO
+   bool
+   default n
+   depends on CPU_AMD_MODEL_FXX
Index: src/cpu/amd/sc520/Kconfig
===
--- src/cpu/amd/sc520/Kconfig   (revision 4999)
+++ src/cpu/amd/sc520/Kconfig   (working copy)
@@ -1,3 +1,8 @@
 config CPU_AMD_SC520
bool
 
+config UDELAY_IO
+   bool
+   default n
+   depends on CPU_AMD_SC520
+
Index: src/cpu/amd/model_10xxx/Kconfig
===
--- src/cpu/amd/model_10xxx/Kconfig (revision 4999)
+++ src/cpu/amd/model_10xxx/Kconfig (working copy)
@@ -4,6 +4,7 @@
select USE_PRINTK_IN_CAR
select USE_DCACHE_RAM
select SSE
+   select HAVE_INIT_TIMER
 
 config CPU_ADDR_BITS
int
@@ -25,3 +26,7 @@
default 0x04000
depends on CPU_AMD_MODEL_10XXX
 
+config UDELAY_IO
+   bool
+   default n
+   depends on CPU_AMD_MODEL_10XXX
Index: src/mainboard/supermicro/h8dmr_fam10/Kconfig
===
--- src/mainboard/supermicro/h8dmr_fam10/Kconfig(revision 4999)
+++ src/mainboard/supermicro/h8dmr_fam10/Kconfig(working copy)
@@ -15,6 +15,8 @@
select LIFT_BSP_APIC_ID
select AMDMCT
select BOARD_ROMSIZE_KB_1024
+   select TINY_BOOTBLOCK
+   select ENABLE_APIC_EXT_ID
 
 config MAINBOARD_DIR
string
@@ -23,27 +25,42 @@
 
 config DCACHE_RAM_BASE
hex
-   default 0xc8000
+   default 0xc4000
depends on BOARD_SUPERMICRO_H8DMR_FAM10
 
 config DCACHE_RAM_SIZE
hex
-   default 0x08000
+   default 0x0c000
depends on BOARD_SUPERMICRO_H8DMR_FAM10
 
 config DCACHE_RAM_GLOBAL_VAR_SIZE
hex
-   default 0x01000
+   default 0x04000
depends on BOARD_SUPERMICRO_H8DMR_FAM10
 
+config RAMBASE
+   hex
+   default 0x20
+   depends on BOARD_SUPERMICRO_H8DMR_FAM10
+
+config RAMTOP
+   hex
+   default 0x100
+   depends on BOARD_SUPERMICRO_H8DMR_FAM10
+
+config HEAP_SIZE
+   hex
+   default 0xc
+   depends on BOARD_SUPERMICRO_H8DMR_FAM10
+
 config APIC_ID_OFFSET
hex
-   default 0x10
+   default 0x0
depends on BOARD_SUPERMICRO_H8DMR_FAM10
 
 config MEM_TRAIN_SEQ
int
-   default 1
+   default 2
depends on BOARD_SUPERMICRO_H8DMR_FAM10
 
 config SB_HT_CHAIN_ON_BUS0
@@ -78,7 +95,7 @@
 
 config MAX_CPUS
int
-   default 4
+   default 8
depends on BOARD_SUPERMICRO_H8DMR_FAM10
 
 config MAX_PHYSICAL_CPUS
@@ -88,12 +105,12 @@
 
 config HT_CHAIN_END_UNITID_BASE
hex
-

[coreboot] [PATCH]more kconfig fixes

[Patrick Georgi]

Hi,

with this patch (and my previous one), all kconfig enabled boards build
for me.

Changes this time:
- Fix for superio/smsc/lpc47b272
- disable option table for asus/mew-vm
- change superio to use for compaq/deskpro_en_sff_p600 to be in line
  with what newconfig/buildtarget does


Signed-off-by: Patrick Georgi 

Regards,
Patrick
Index: src/superio/smsc/lpc47b272/Makefile.inc
===
--- src/superio/smsc/lpc47b272/Makefile.inc	(Revision 4665)
+++ src/superio/smsc/lpc47b272/Makefile.inc	(Arbeitskopie)
@@ -19,5 +19,5 @@
 ##
 
 #config chip.h
-obj-$(CONFIG_SUPERIO_SMSC_DEVICE) += superio.o
+obj-$(CONFIG_SUPERIO_SMSC_LPC47B272) += superio.o
 
Index: src/mainboard/asus/mew-vm/Kconfig
===
--- src/mainboard/asus/mew-vm/Kconfig	(Revision 4665)
+++ src/mainboard/asus/mew-vm/Kconfig	(Arbeitskopie)
@@ -24,7 +24,7 @@
 	select CPU_INTEL_SOCKET_PGA370
 	select NORTHBRIDGE_INTEL_I82810
 	select SOUTHBRIDGE_INTEL_I82801XX
-	select SUPERIO_SMSC_SMSCSUPERIO
+	select SUPERIO_SMSC_LPC47B272
 	select HAVE_PIRQ_TABLE
 	select UDELAY_IO
 	select PCI_ROM_RUN
@@ -42,6 +42,11 @@
 	default "MEW-VM"
 	depends on BOARD_ASUS_MEW_VM
 
+config HAVE_OPTION_TABLE
+	bool
+	default n
+	depends on BOARD_ASUS_MEW_VM
+
 config IRQ_SLOT_COUNT
 	int
 	default 11
Index: src/mainboard/compaq/deskpro_en_sff_p600/Kconfig
===
--- src/mainboard/compaq/deskpro_en_sff_p600/Kconfig	(Revision 4665)
+++ src/mainboard/compaq/deskpro_en_sff_p600/Kconfig	(Arbeitskopie)
@@ -24,7 +24,8 @@
 	select CPU_INTEL_SLOT_2
 	select NORTHBRIDGE_INTEL_I440BX
 	select SOUTHBRIDGE_INTEL_I82371EB
-	select SUPERIO_NSC_PC97307
+	# should be SUPERIO_NSC_PC97307!
+	select SUPERIO_NSC_PC97317
 	select HAVE_PIRQ_TABLE
 	select UDELAY_IO
 	select PCI_ROM_RUN
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

[coreboot] [PATCH] more kconfig support

[Patrick Georgi]

Hi,

it's not going as fast as I thought, but still, some progress:
(all of this excluding PPC hardware, and I might have missed something)
- northbridges are done
- southbridges are done
- Intel CPUs are done, with a design that the board only has to specify
  the socket it has, and the CPUs are pulled in automatically. There is
  some more cleanup possible in that area, but I'll do that later
- a couple more mainboards compile:
  - intel/eagleheights
  - intel/jarrell
  - intel/mtarvon
  - intel/truxton
  - intel/xe7501devkit
  - sunw/ultra40
  - supermicro/h8dme
  - tyan/s2850
  - tyan/s2875
  - via/epia
  - via/epia-cn
  - via/epia-m
  - via/epia-m700
  - via/epia-n
  - via/pc2500e

Given that pretty much all devices should be kconfig'd now, I guess that
I'll be faster with the others.
All of them only _build_, I guess some options are still completely
wrong, I'll handle that in one big pass (with tool support) when all
boards build.

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


Regards,
Patrick


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Myles Watson]

> Signed-off-by: Patrick Georgi 
Acked-by: Myles Watson 

Thanks,
Myles


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Uwe Hermann]

On Wed, Oct 07, 2009 at 02:06:46PM +0200, Patrick Georgi wrote:
> attached patch enables kconfig based builds for a couple more boards. As
> usual, this merely passes build and a validity check that all variables
> are defined that are defined by newconfig (but not necessarily the same
> value).
> 
> The larger part of this (which is why I stopped porting more boards and
> want to get this out first) is that I moved the -O2 flag for romcc
> invocations into ROMCCFLAGS,

That's OK.


> as rca/rm4100 wants -O only.

This should be investigated, though. I believe pretty much _all_ those
-O or -O2 lines are copy-paste anyway, nobody really ever thought about
which of them is appropriate for the resp. board.

Apart from possible romcc bugs I cannot think about a reason to not use
the -O2 option for all boards per default. Is there such a bug?
I can test -O vs. -O2 on some boards later, if needed.

(the -mcpu option is board-/cpu-dependent of course, and must stay
configurable)


> Index: src/mainboard/bcom/winnetp680/Kconfig
> ===
> --- src/mainboard/bcom/winnetp680/Kconfig (Revision 0)
> +++ src/mainboard/bcom/winnetp680/Kconfig (Revision 0)
[...]
> +config RAMBASE
> + hex
> + default 0x4000
> + depends on BOARD_BCOM_WINNETP680

This is also something we should look into later. I'd use the new
default for RAMBASE whereever possible, unless some boards have problems
with that (which we should find out).

I can do some tests on boards which used 0x4000 earlier.


> Index: src/mainboard/bcom/winnetp680/Makefile.inc
> ===
> --- src/mainboard/bcom/winnetp680/Makefile.inc(Revision 0)
> +++ src/mainboard/bcom/winnetp680/Makefile.inc(Revision 0)
> @@ -0,0 +1,61 @@
[...]
> +initobj-y += crt0.o
> +obj-y += mainboard.o
> +obj-$(CONFIG_HAVE_PIRQ_TABLE) += irq_tables.o
> +obj-$(CONFIG_HAVE_MP_TABLE) += mptable.o
> +
> +obj-$(CONFIG_HAVE_ACPI_TABLES) += fadt.o
> +obj-$(CONFIG_HAVE_ACPI_TABLES) += dsdt.o
> +obj-$(CONFIG_HAVE_ACPI_TABLES) += acpi_tables.o
> +
> +ldscript-y += ../../../../src/arch/i386/init/ldscript_fallback_cbfs.lb
> +ldscript-y += ../../../../src/cpu/x86/16bit/entry16.lds
> +ldscript-y += ../../../../src/cpu/x86/16bit/reset16.lds
> +ldscript-y += ../../../../src/arch/i386/lib/id.lds
> +ldscript-y += ../../../../src/arch/i386/lib/failover.lds
> +
> +crt0-y += ../../../../src/cpu/x86/16bit/entry16.inc
> +crt0-y += ../../../../src/cpu/x86/32bit/entry32.inc
> +crt0-y += ../../../../src/cpu/x86/16bit/reset16.inc
> +crt0-y += ../../../../src/arch/i386/lib/id.inc
> +crt0-y += ../../../../src/cpu/x86/fpu/enable_fpu.inc
> +crt0-y += ../../../../src/cpu/x86/mmx/enable_mmx.inc
> +crt0-y += auto.inc
> +crt0-y += ../../../../src/cpu/x86/mmx/disable_mmx.inc
> +
> +ifdef POST_EVALUATION
> +
> +$(obj)/mainboard/$(MAINBOARDDIR)/dsdt.o: $(obj)/dsdt.c
> + $(CC) $(DISTRO_CFLAGS) $(CFLAGS) $(CPPFLAGS) $(DEBUG_CFLAGS) -I$(src) 
> -I. -c $< -o $@
> +
> +$(obj)/dsdt.c: $(src)/mainboard/$(MAINBOARDDIR)/dsdt.asl
> + iasl -p dsdt -tc $(src)/mainboard/$(MAINBOARDDIR)/dsdt.asl
> + mv dsdt.hex $@
> +
> +$(obj)/mainboard/$(MAINBOARDDIR)/auto.inc: 
> $(src)/mainboard/$(MAINBOARDDIR)/auto.c $(obj)/option_table.h
> + $(CC) $(DISTRO_CFLAGS) $(CFLAGS) $(CPPFLAGS) $(DEBUG_CFLAGS) -I$(src) 
> -I. -c -S  $(src)/mainboard/$(MAINBOARDDIR)/auto.c -o $@
> + perl -e 's/\.rodata/.rom.data/g' -pi $@
> + perl -e 's/\.text/.section .rom.text/g' -pi $@
> +
> +endif

Long-term this should move into a global file as it's pretty much
the same for all boards.


> Index: src/mainboard/gigabyte/ga_2761gxdk/Kconfig
> ===
> --- src/mainboard/gigabyte/ga_2761gxdk/Kconfig(Revision 0)
> +++ src/mainboard/gigabyte/ga_2761gxdk/Kconfig(Revision 0)
> @@ -0,0 +1,148 @@
[...]
> + select HAVE_HIGH_TABLES

Global option, should probably not be per-board.


  
> Index: src/mainboard/newisys/khepri/Kconfig
> ===
> --- src/mainboard/newisys/khepri/Kconfig  (Revision 0)
> +++ src/mainboard/newisys/khepri/Kconfig  (Revision 0)
> @@ -0,0 +1,123 @@
> +config BOARD_NEWISYS_KHEPRI
> + bool "Khepri"

"2100 Server (SUN Fire v20z)" as per wiki (not sure if "Khepri" is the
official vendor name, if yes we should also adapt the wiki table).


> +config CONSOLE_VGA
> + bool
> + default y
> + depends on BOARD_NEWISYS_KHEPRI
> +
> +config PCI_ROM_RUN
> + bool
> + default y
> + depends on BOARD_NEWISYS_KHEPRI

Should be dropped, global options.


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: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Patrick Georgi]

Am Mittwoch, den 07.10.2009, 16:18 +0200 schrieb Uwe Hermann:
> > as rca/rm4100 wants -O only.
> 
> This should be investigated, though. I believe pretty much _all_ those
> -O or -O2 lines are copy-paste anyway, nobody really ever thought about
> which of them is appropriate for the resp. board.
> 
> Apart from possible romcc bugs I cannot think about a reason to not use
> the -O2 option for all boards per default. Is there such a bug?
romcc crashes, so yes, a bug. Some other board requires quite a specific
romcc option, so there are more such special cases. Fixing romcc bugs
will be a nice task once the tree settled down a bit.

> > Index: src/mainboard/bcom/winnetp680/Kconfig
> > ===
> > --- src/mainboard/bcom/winnetp680/Kconfig   (Revision 0)
> > +++ src/mainboard/bcom/winnetp680/Kconfig   (Revision 0)
> [...]
> > +config RAMBASE
> > +   hex
> > +   default 0x4000
> > +   depends on BOARD_BCOM_WINNETP680
> 
> This is also something we should look into later. I'd use the new
> default for RAMBASE whereever possible, unless some boards have problems
> with that (which we should find out).
> 
> I can do some tests on boards which used 0x4000 earlier.
At least those boards with their own vgabios.c (pretty much all via
chipsets) must be <1MB due to the way their VGABIOS stuff is
implemented.
Stefan and I tried to replace that with x86emu (or any of the common vga
things), but didn't succeed so far.
> 
> > Index: src/mainboard/bcom/winnetp680/Makefile.inc
> > ===
> > --- src/mainboard/bcom/winnetp680/Makefile.inc  (Revision 0)
> > +++ src/mainboard/bcom/winnetp680/Makefile.inc  (Revision 0)
> > @@ -0,0 +1,61 @@
> [...]
> > +initobj-y += crt0.o
> > +obj-y += mainboard.o
> > +obj-$(CONFIG_HAVE_PIRQ_TABLE) += irq_tables.o
> > +obj-$(CONFIG_HAVE_MP_TABLE) += mptable.o
> > +
> > +obj-$(CONFIG_HAVE_ACPI_TABLES) += fadt.o
> > +obj-$(CONFIG_HAVE_ACPI_TABLES) += dsdt.o
> > +obj-$(CONFIG_HAVE_ACPI_TABLES) += acpi_tables.o
> > +
> > +ldscript-y += ../../../../src/arch/i386/init/ldscript_fallback_cbfs.lb
> > +ldscript-y += ../../../../src/cpu/x86/16bit/entry16.lds
> > +ldscript-y += ../../../../src/cpu/x86/16bit/reset16.lds
> > +ldscript-y += ../../../../src/arch/i386/lib/id.lds
> > +ldscript-y += ../../../../src/arch/i386/lib/failover.lds
> > +
> > +crt0-y += ../../../../src/cpu/x86/16bit/entry16.inc
> > +crt0-y += ../../../../src/cpu/x86/32bit/entry32.inc
> > +crt0-y += ../../../../src/cpu/x86/16bit/reset16.inc
> > +crt0-y += ../../../../src/arch/i386/lib/id.inc
> > +crt0-y += ../../../../src/cpu/x86/fpu/enable_fpu.inc
> > +crt0-y += ../../../../src/cpu/x86/mmx/enable_mmx.inc
> > +crt0-y += auto.inc
> > +crt0-y += ../../../../src/cpu/x86/mmx/disable_mmx.inc
> > +
> > +ifdef POST_EVALUATION
> > +
> > +$(obj)/mainboard/$(MAINBOARDDIR)/dsdt.o: $(obj)/dsdt.c
> > +   $(CC) $(DISTRO_CFLAGS) $(CFLAGS) $(CPPFLAGS) $(DEBUG_CFLAGS) -I$(src) 
> > -I. -c $< -o $@
> > +
> > +$(obj)/dsdt.c: $(src)/mainboard/$(MAINBOARDDIR)/dsdt.asl
> > +   iasl -p dsdt -tc $(src)/mainboard/$(MAINBOARDDIR)/dsdt.asl
> > +   mv dsdt.hex $@
> > +
> > +$(obj)/mainboard/$(MAINBOARDDIR)/auto.inc: 
> > $(src)/mainboard/$(MAINBOARDDIR)/auto.c $(obj)/option_table.h
> > +   $(CC) $(DISTRO_CFLAGS) $(CFLAGS) $(CPPFLAGS) $(DEBUG_CFLAGS) -I$(src) 
> > -I. -c -S  $(src)/mainboard/$(MAINBOARDDIR)/auto.c -o $@
> > +   perl -e 's/\.rodata/.rom.data/g' -pi $@
> > +   perl -e 's/\.text/.section .rom.text/g' -pi $@
> > +
> > +endif
> 
> Long-term this should move into a global file as it's pretty much
> the same for all boards.
Yes, that's on my TODO for a while.
The ACPI related things require some file renaming, which I wanted to
wait for until newconfig is gone (no need to waste time on adapting that
one, too)


Patrick


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Peter Stuge]

Patrick Georgi wrote:
> +++ src/mainboard/gigabyte/ga_2761gxdk/Kconfig(Revision 0)
> @@ -0,0 +1,148 @@
> +config BOARD_GIGABYTE_GA_2761GXDK
> + bool "GA-2761GXDK"
> + select ARCH_X86
> + select CPU_AMD_K8
> + select CPU_AMD_SOCKET_AM2
> + select NORTHBRIDGE_AMD_AMDK8
> + select NORTHBRIDGE_AMD_AMDK8_ROOT_COMPLEX
> + select SOUTHBRIDGE_SIS_SIS966
> + select SUPERIO_ITE_IT8716F
..
> +config MAX_CPUS
> + int
> + default 2
> + depends on BOARD_GIGABYTE_GA_2761GXDK
> +
> +config MAX_PHYSICAL_CPUS
> + int
> + default 1
> + depends on BOARD_GIGABYTE_GA_2761GXDK

Does it make sense to set these CPU options in socket Kconfigs?


> +++ src/mainboard/rca/rm4100/Makefile.inc (Revision 0)
> @@ -0,0 +1,4 @@
> +ROMCCFLAGS=-mcpu=p3 -O
..

> +++ src/mainboard/iei/nova4899r/Makefile.inc  (Arbeitskopie)
> @@ -18,6 +18,6 @@
>  ## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
>  ##
>  
> -ROMCCFLAGS := -mcpu=i386
> +ROMCCFLAGS := -mcpu=i386 -O2

The same question for -mcpu in ROMCCFLAGS? Couldn't that also be set
in socket Kconfigs?


//Peter

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Patrick Georgi]

Am Donnerstag, den 08.10.2009, 16:05 +0200 schrieb Patrick Georgi:
> Hi,
> 
> this time: AMD LX, AMD SC520, board by
> iei, pcengines, technexion, technologics, thomson
> 
> All of them build, and pass my variables test.
> 
> Signed-off-by: Patrick Georgi 
With patch
Index: src/cpu/amd/Kconfig
===
--- src/cpu/amd/Kconfig	(Revision 4741)
+++ src/cpu/amd/Kconfig	(Arbeitskopie)
@@ -15,3 +15,6 @@
 source src/cpu/amd/model_10xxx/Kconfig
 source src/cpu/amd/model_gx1/Kconfig
 source src/cpu/amd/model_gx2/Kconfig
+source src/cpu/amd/model_lx/Kconfig
+
+source src/cpu/amd/sc520/Kconfig
Index: src/cpu/amd/socket_S1G1/Makefile.inc
===
--- src/cpu/amd/socket_S1G1/Makefile.inc	(Revision 4741)
+++ src/cpu/amd/socket_S1G1/Makefile.inc	(Arbeitskopie)
@@ -11,4 +11,5 @@
 subdirs-y += ../../x86/cache
 subdirs-y += ../../x86/pae
 subdirs-y += ../../x86/smm
+subdirs-y += ../../x86/mtrr
 
Index: src/cpu/amd/Makefile.inc
===
--- src/cpu/amd/Makefile.inc	(Revision 4741)
+++ src/cpu/amd/Makefile.inc	(Arbeitskopie)
@@ -4,4 +4,6 @@
 subdirs-$(CONFIG_CPU_AMD_SOCKET_AM2) += socket_AM2
 subdirs-$(CONFIG_CPU_AMD_GX1) += model_gx1
 subdirs-$(CONFIG_CPU_AMD_GX2) += model_gx2
+subdirs-$(CONFIG_CPU_AMD_LX) += model_lx
+subdirs-$(CONFIG_CPU_AMD_SC520) += sc520
 subdirs-$(CONFIG_CPU_AMD_SOCKET_S1G1) += socket_S1G1
Index: src/cpu/amd/sc520/Kconfig
===
--- src/cpu/amd/sc520/Kconfig	(Revision 0)
+++ src/cpu/amd/sc520/Kconfig	(Revision 0)
@@ -0,0 +1,4 @@
+config CPU_AMD_SC520
+	bool
+	default n
+
Index: src/cpu/amd/sc520/Makefile.inc
===
--- src/cpu/amd/sc520/Makefile.inc	(Revision 0)
+++ src/cpu/amd/sc520/Makefile.inc	(Revision 0)
@@ -0,0 +1 @@
+obj-y += sc520.o
Index: src/cpu/amd/model_lx/Kconfig
===
--- src/cpu/amd/model_lx/Kconfig	(Revision 0)
+++ src/cpu/amd/model_lx/Kconfig	(Revision 0)
@@ -0,0 +1,4 @@
+config CPU_AMD_LX
+	bool
+	default n
+
Index: src/cpu/amd/model_lx/Makefile.inc
===
--- src/cpu/amd/model_lx/Makefile.inc	(Revision 0)
+++ src/cpu/amd/model_lx/Makefile.inc	(Revision 0)
@@ -0,0 +1,10 @@
+subdirs-y += ../../x86/tsc
+subdirs-y += ../../x86/fpu
+subdirs-y += ../../x86/mmx
+subdirs-y += ../../x86/lapic
+subdirs-y += ../../x86/cache
+subdirs-y += ../../x86/smm
+
+driver-y += model_lx_init.o
+obj-y += cpubug.o
+obj-y += vsmsetup.o
Index: src/mainboard/thomson/Kconfig
===
--- src/mainboard/thomson/Kconfig	(Revision 4741)
+++ src/mainboard/thomson/Kconfig	(Arbeitskopie)
@@ -1 +1,8 @@
-#
+choice
+	prompt "Mainboard model"
+	depends on VENDOR_THOMSON
+
+source "src/mainboard/thomson/ip1000/Kconfig"
+
+endchoice
+
Index: src/mainboard/thomson/ip1000/Kconfig
===
--- src/mainboard/thomson/ip1000/Kconfig	(Revision 0)
+++ src/mainboard/thomson/ip1000/Kconfig	(Revision 0)
@@ -0,0 +1,35 @@
+config BOARD_THOMSON_IP1000
+	bool "ip1000"
+	select ARCH_X86
+	select CPU_INTEL_SOCKET_PGA370
+	select NORTHBRIDGE_INTEL_I82830
+	select SOUTHBRIDGE_INTEL_I82801XX
+	select SUPERIO_SMSC_SMSCSUPERIO
+	select HAVE_PIRQ_TABLE
+	select UDELAY_TSC
+
+config MAINBOARD_DIR
+	string
+	default thomson/ip1000
+	depends on BOARD_THOMSON_IP1000
+
+config MAINBOARD_PART_NUMBER
+	string
+	default "IP1000"
+	depends on BOARD_THOMSON_IP1000
+
+config HAVE_OPTION_TABLE
+	bool
+	default n
+	depends on BOARD_THOMSON_IP1000
+
+config IRQ_SLOT_COUNT
+	int
+	default 7
+	depends on BOARD_THOMSON_IP1000
+
+config VIDEO_MB
+	int
+	default 0
+	depends on BOARD_THOMSON_IP1000
+
Index: src/mainboard/thomson/ip1000/Makefile.inc
===
--- src/mainboard/thomson/ip1000/Makefile.inc	(Revision 0)
+++ src/mainboard/thomson/ip1000/Makefile.inc	(Revision 0)
@@ -0,0 +1,4 @@
+ROMCCFLAGS=-mcpu=p3 -O
+
+include $(src)/mainboard/Makefile.romccboard.inc
+
Index: src/mainboard/technologic/Kconfig
===
--- src/mainboard/technologic/Kconfig	(Revision 4741)
+++ src/mainboard/technologic/Kconfig	(Arbeitskopie)
@@ -1 +1,8 @@
-#
+choice
+	prompt "Mainboard model"
+	depends on VENDOR_TECHNOLOGIC
+
+source "src/mainboard/technologic/ts5300/Kconfig"
+
+endchoice
+
Index: src/mainboard/technologic/ts5300/Kconfig
===
--- src/mainboard/technologic/ts5300/Kconfig	(Revision 0)
+++ src/mainboard/technologic/ts5300/Kconfig	(Revision 0)
@@ -0,0 +1,21 @@
+config BOARD_TECHNOLOGIC_TS5300
+	bool "ts5300"
+	select ARCH_X86
+	select CPU_AMD_SC520
+	sel

Re: [coreboot] [PATCH]more kconfig

[Myles Watson]

> > All of them build, and pass my variables test.
> >
> > Signed-off-by: Patrick Georgi 
Acked-by: Myles Watson 

Thanks,
Myles


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Uwe Hermann]

On Thu, Oct 08, 2009 at 04:05:35PM +0200, Patrick Georgi wrote:
> Am Donnerstag, den 08.10.2009, 16:05 +0200 schrieb Patrick Georgi:
> > Hi,
> > 
> > this time: AMD LX, AMD SC520, board by
> > iei, pcengines, technexion, technologics, thomson
> > 
> > All of them build, and pass my variables test.
> > 
> > Signed-off-by: Patrick Georgi 

Acked-by: Uwe Hermann 

  
> Index: src/mainboard/thomson/ip1000/Kconfig
> ===
> --- src/mainboard/thomson/ip1000/Kconfig  (Revision 0)
> +++ src/mainboard/thomson/ip1000/Kconfig  (Revision 0)
> @@ -0,0 +1,35 @@
> +config BOARD_THOMSON_IP1000
> + bool "ip1000"

Will fix all boardnames later when Kconfig conversion is done, there are
multiple entries which are not quite correct (as per wiki), but this
can wait.


> + select ARCH_X86
> + select CPU_INTEL_SOCKET_PGA370
> + select NORTHBRIDGE_INTEL_I82830
> + select SOUTHBRIDGE_INTEL_I82801XX
> + select SUPERIO_SMSC_SMSCSUPERIO
> + select HAVE_PIRQ_TABLE
> + select UDELAY_TSC

Why no "select CONFIG_TSC_X86RDTSC_CALIBRATE_WITH_TIMER2"? That's in the
Options.lb file too:

  default CONFIG_TSC_X86RDTSC_CALIBRATE_WITH_TIMER2 = 1


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: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

[coreboot] [PATCH] More kconfig fixes

[Uwe Hermann]

See patch.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
More kconfig cleanups:
 
 - Use "default n" for all components that shall be "select"ed.

 - Use "0x0" instead of "0" for hex variables for clarity and to reduce
   the risk of people passing integer instead of hex values to such variables.

 - Set CONFIG_PIRQ_ROUTE=1 for boards which have an irq_tables.c but
   for some reason had CONFIG_PIRQ_ROUTE=0.
   For EPIA-M700 I'm not sure it should be used at all, as this board has
   ACPI tables (?)
 
 - ASUS M2V-MX SE doesn't have irq_tables.c so don't define
   IRQ_SLOT_COUNT in its Kconfig file.

Signed-off-by: Uwe Hermann 

Index: southbridge/intel/i82371eb/Kconfig
===
--- southbridge/intel/i82371eb/Kconfig	(Revision 4742)
+++ southbridge/intel/i82371eb/Kconfig	(Arbeitskopie)
@@ -1,3 +1,4 @@
 config SOUTHBRIDGE_INTEL_I82371EB
 	bool
+	default n
 
Index: Kconfig
===
--- Kconfig	(Revision 4742)
+++ Kconfig	(Arbeitskopie)
@@ -56,11 +56,11 @@
 
 config MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID
 	hex
-	default 0
+	default 0x0
 
 config MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID
 	hex
-	default 0
+	default 0x0
 
 config CPU_ADDR_BITS
 	int
@@ -68,7 +68,7 @@
 
 config AGP_APERTURE_SIZE
 	hex
-	default 0
+	default 0x0
 
 config XIP_ROM_BASE
 	hex
@@ -174,7 +174,7 @@
 
 config HAVE_HARD_RESET
 	bool
-	default 0
+	default n
 
 config HAVE_INIT_TIMER
 	bool
Index: superio/serverengines/Kconfig
===
--- superio/serverengines/Kconfig	(Revision 4742)
+++ superio/serverengines/Kconfig	(Arbeitskopie)
@@ -1,2 +1,3 @@
 config SUPERIO_SERVERENGINES_PILOT
 	bool
+	default n
Index: superio/via/Kconfig
===
--- superio/via/Kconfig	(Revision 4742)
+++ superio/via/Kconfig	(Arbeitskopie)
@@ -1,2 +1,3 @@
 config SUPERIO_VIA_VT1211
 	bool
+	default n
Index: superio/fintek/Kconfig
===
--- superio/fintek/Kconfig	(Revision 4742)
+++ superio/fintek/Kconfig	(Arbeitskopie)
@@ -1,2 +1,3 @@
 config SUPERIO_FINTEK_F71805F
 	bool
+	default n
Index: superio/winbond/Kconfig
===
--- superio/winbond/Kconfig	(Revision 4742)
+++ superio/winbond/Kconfig	(Arbeitskopie)
@@ -1,18 +1,27 @@
 config SUPERIO_WINBOND_W83627DHG
 	bool
+	default n
 config SUPERIO_WINBOND_W83627EHG
 	bool
+	default n
 config SUPERIO_WINBOND_W83627HF
 	bool
+	default n
 config SUPERIO_WINBOND_W83627THF
 	bool
+	default n
 config SUPERIO_WINBOND_W83627THG
 	bool
+	default n
 config SUPERIO_WINBOND_W83627UHG
 	bool
+	default n
 config SUPERIO_WINBOND_W83697HF
 	bool
+	default n
 config SUPERIO_WINBOND_W83977F
 	bool
+	default n
 config SUPERIO_WINBOND_W83977TF
 	bool
+	default n
Index: superio/ite/Kconfig
===
--- superio/ite/Kconfig	(Revision 4742)
+++ superio/ite/Kconfig	(Arbeitskopie)
@@ -1,14 +1,21 @@
 config SUPERIO_ITE_IT8661F
 	bool
+	default n
 config SUPERIO_ITE_IT8671F
 	bool
+	default n
 config SUPERIO_ITE_IT8673F
 	bool
+	default n
 config SUPERIO_ITE_IT8705F
 	bool
+	default n
 config SUPERIO_ITE_IT8712F
 	bool
+	default n
 config SUPERIO_ITE_IT8716F
 	bool
+	default n
 config SUPERIO_ITE_IT8718F
 	bool
+	default n
Index: superio/nsc/Kconfig
===
--- superio/nsc/Kconfig	(Revision 4742)
+++ superio/nsc/Kconfig	(Arbeitskopie)
@@ -1,18 +1,27 @@
 config SUPERIO_NSC_PC8374
 	bool
+	default n
 config SUPERIO_NSC_PC87309
 	bool
+	default n
 config SUPERIO_NSC_PC87351
 	bool
+	default n
 config SUPERIO_NSC_PC87360
 	bool
+	default n
 config SUPERIO_NSC_PC87366
 	bool
+	default n
 config SUPERIO_NSC_PC87417
 	bool
+	default n
 config SUPERIO_NSC_PC87427
 	bool
+	default n
 config SUPERIO_NSC_PC97307
 	bool
+	default n
 config SUPERIO_NSC_PC97317
 	bool
+	default n
Index: superio/smsc/Kconfig
===
--- superio/smsc/Kconfig	(Revision 4742)
+++ superio/smsc/Kconfig	(Arbeitskopie)
@@ -1,12 +1,18 @@
 config SUPERIO_SMSC_FDC37M60X
 	bool
+	default n
 config SUPERIO_SMSC_LPC47B272
 	bool
+	default n
 config SUPERIO_SMSC_LPC47B397
 	bool
+	default n
 config SUPERIO_SMSC_LPC47M10X
 	bool
+	default n
 config SUPERIO_SMSC_LPC47N217
 	bool
+	default n
 config SUPERIO_SMSC_SMSCSUPERIO
 	bool
+	default n
Index: superio/intel/Kconfig
===
--- superio/intel/Kconfig	(Revision 4742)
+++ superio/intel/Kconfig	(Arbeitskopie)
@@ -1,2 +1,3 @@
 config SUPERIO_INTEL_I3100
 	bool
+	default n
Index: cpu/Kconfig
===
--- cpu/Kconfig	(Revis

Re: [coreboot] [PATCH]more kconfig

[Stefan Reinauer]

Uwe Hermann wrote:
>> Index: src/mainboard/newisys/khepri/Kconfig
>> ===
>> --- src/mainboard/newisys/khepri/Kconfig (Revision 0)
>> +++ src/mainboard/newisys/khepri/Kconfig (Revision 0)
>> @@ -0,0 +1,123 @@
>> +config BOARD_NEWISYS_KHEPRI
>> +bool "Khepri"
>> 
>
> "2100 Server (SUN Fire v20z)" as per wiki (not sure if "Khepri" is the
> official vendor name, if yes we should also adapt the wiki table).
>   
The vendor name is Newisys.
http://www.amd.com/us-en/assets/content_type/DownloadableAssets/2100-e1.pdf

The company was bought by Sanmina-SCI a couple of years back. I believe
they designed the SUN x4100, too.

Stefan

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Uwe Hermann]

On Fri, Oct 09, 2009 at 02:05:56AM +0200, Stefan Reinauer wrote:
> Uwe Hermann wrote:
> >> Index: src/mainboard/newisys/khepri/Kconfig
> >> ===
> >> --- src/mainboard/newisys/khepri/Kconfig   (Revision 0)
> >> +++ src/mainboard/newisys/khepri/Kconfig   (Revision 0)
> >> @@ -0,0 +1,123 @@
> >> +config BOARD_NEWISYS_KHEPRI
> >> +  bool "Khepri"
> >> 
> >
> > "2100 Server (SUN Fire v20z)" as per wiki (not sure if "Khepri" is the
> > official vendor name, if yes we should also adapt the wiki table).
> >   
> The vendor name is Newisys.
> http://www.amd.com/us-en/assets/content_type/DownloadableAssets/2100-e1.pdf

Oops, sorry, I meant to say "official board name" not "official vendor name".

Is "2100 Server (SUN Fire v20z)" the full, correct name, or should we
change it?


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: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Stefan Reinauer]

Uwe Hermann wrote:
> On Fri, Oct 09, 2009 at 02:05:56AM +0200, Stefan Reinauer wrote:
>   
>> Uwe Hermann wrote:
>> 
 Index: src/mainboard/newisys/khepri/Kconfig
 ===
 --- src/mainboard/newisys/khepri/Kconfig   (Revision 0)
 +++ src/mainboard/newisys/khepri/Kconfig   (Revision 0)
 @@ -0,0 +1,123 @@
 +config BOARD_NEWISYS_KHEPRI
 +  bool "Khepri"
 
 
>>> "2100 Server (SUN Fire v20z)" as per wiki (not sure if "Khepri" is the
>>> official vendor name, if yes we should also adapt the wiki table).
>>>   
>>>   
>> The vendor name is Newisys.
>> http://www.amd.com/us-en/assets/content_type/DownloadableAssets/2100-e1.pdf
>> 
>
> Oops, sorry, I meant to say "official board name" not "official vendor name".
>
> Is "2100 Server (SUN Fire v20z)" the full, correct name, or should we
> change it?
>   

I think 2100 (Sun Fire v20z) would be enough, but it's also fine as it is..

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
  Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: i...@coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

[coreboot] [PATCH] More Kconfig changes

[Myles Watson]

Define some variables that were not defined.  There are a couple left.  Do
kbuildall then grep not.defined kbuildall.results/*
The interesting ones were GENERATE_*  I had to put them in twice to make it
work correctly.  Once outside the menu setting the defaults, and once inside
the menu.  Now they show up when they should, and are always defined

Define HAVE_INIT_TIMER to only exclude the three boards that define it to be
0 in newconfig.
Define MEM_TRAIN_SEQ to be an integer and set it correctly.
Remove CAR_FAM10 and just depend on NORTHBRIDGE_AMD_AMDFAM10
MOVNTI is a performance enhancement, and should default to 0 so it doesn't
break boards that forget to define it.

Signed-off-by: Myles Watson 

Thanks,
Myles
Index: svn/src/Kconfig
===
--- svn.orig/src/Kconfig
+++ svn/src/Kconfig
@@ -74,10 +74,6 @@ config CPU_ADDR_BITS
 	int
 	default 36
 
-config AGP_APERTURE_SIZE
-	hex
-	default 0x0
-
 config XIP_ROM_BASE
 	hex
 	default 0xfffe
@@ -216,6 +212,7 @@ config IOAPIC
 	default n
 
 config VIDEO_MB
+	default 0
 	int
 
 config USE_WATCHDOG_ON_BOOT
@@ -230,6 +227,7 @@ config VGA
 
 config GFXUMA
 	bool
+	default n
 	help
 	  Enable Unified Memory Architecture for graphics.
 
@@ -238,6 +236,26 @@ config GFXUMA
 #
 # endmenu
 
+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
+
+config HAVE_HIGH_TABLES
+	bool
+	default n
+	help
+	  This variable specifies whether a given northbridge has high table
+	  support.
+	  It is set in northbridge/*/Kconfig.
+	  Whether or not the high tables are actually written by coreboot is
+	  configurable by the user via WRITE_HIGH_TABLES.
+
 config HAVE_ACPI_TABLES
 	bool
 	help
@@ -262,14 +280,30 @@ config HAVE_PIRQ_TABLE
 	  Whether or not the PIRQ table is actually generated by coreboot
 	  is configurable by the user via GENERATE_PIRQ_TABLE.
 
-config HAVE_HIGH_TABLES
+#These Options are here to avoid "undefined" warnings.
+#The actual selection and help texts are in the following menu.
+
+config GENERATE_ACPI_TABLES
 	bool
+	default y if HAVE_ACPI_TABLES
+	default n
 
-menu "System tables"
+config GENERATE_MP_TABLE
+	bool
+	default y if HAVE_MP_TABLE
+	default n
 
-config HAVE_LOW_TABLES
+config GENERATE_PIRQ_TABLE
 	bool
-	default y
+	default y if HAVE_PIRQ_TABLE
+	default n
+
+config WRITE_HIGH_TABLES
+	bool
+	default y if HAVE_HIGH_TABLES
+	default n
+
+menu "System tables"
 
 config WRITE_HIGH_TABLES
 	bool "Write 'high' tables to avoid being overwritten in F segment"
@@ -406,3 +440,15 @@ config GDB_STUB
 
 endmenu
 
+# These probably belong somewhere else, but they are needed somewhere.
+config AP_CODE_IN_CAR
+	bool
+	default n
+
+config USE_INIT
+	bool
+	default n
+
+config ENABLE_APIC_EXT_ID
+	bool
+	default n
Index: svn/src/console/Kconfig
===
--- svn.orig/src/console/Kconfig
+++ svn/src/console/Kconfig
@@ -275,3 +275,14 @@ config DEFAULT_CONSOLE_LOGLEVEL
 
 endmenu
 
+config CONSOLE_BTEXT
+	bool
+	default n
+
+config CONSOLE_SROM
+	bool
+	default n
+
+config CONSOLE_LOGBUF
+	bool
+	default n
Index: svn/src/northbridge/amd/amdfam10/Kconfig
===
--- svn.orig/src/northbridge/amd/amdfam10/Kconfig
+++ svn/src/northbridge/amd/amdfam10/Kconfig
@@ -21,11 +21,35 @@ config NORTHBRIDGE_AMD_AMDFAM10
 	bool
 	select HAVE_HIGH_TABLES
 	select HYPERTRANSPORT_PLUGIN_SUPPORT
-	select HT3_SUPPORT
 
 config AGP_APERTURE_SIZE
 	hex
 	default 0x400
 	depends on NORTHBRIDGE_AMD_AMDFAM10
 
+config HT3_SUPPORT
+	bool
+	default y
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
+config AMDMCT
+	bool
+	default y
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
+config MEM_TRAIN_SEQ
+	int
+	default 0
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
+config HW_MEM_HOLE_SIZEK
+	hex
+	default 0x10
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
+config HW_MEM_HOLE_SIZE_AUTO_INC
+	bool
+	default n
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
 source src/northbridge/amd/amdfam10/root_complex/Kconfig
Index: svn/src/northbridge/amd/amdk8/Kconfig
===
--- svn.orig/src/northbridge/amd/amdk8/Kconfig
+++ svn/src/northbridge/amd/amdk8/Kconfig
@@ -32,4 +32,19 @@ config K8_HT_FREQ_1G_SUPPORT
 	default n
 	depends on NORTHBRIDGE_AMD_AMDK8
 
+config MEM_TRAIN_SEQ
+	int
+	default 0
+	depends on NORTHBRIDGE_AMD_AMDK8
+
+config HW_MEM_HOLE_SIZEK
+	hex
+	default 0x10
+	depends on NORTHBRIDGE_AMD_AMDK8
+
+config HW_MEM_HOLE_SIZE_AUTO_INC
+	bool
+	default n
+	depends on NORTHBRIDGE_AMD_AMDK8
+
 source src/northbridge/amd/amdk8/root_complex/Kconfig
Index: svn/src/southbridge/via/vt8237r/vt8237r.h
===

[coreboot] [PATCH] more Kconfig selects

[Myles Watson]

I compared the config variables that we select with the list that we define.

I removed CONFIG_CPU_AMD_FAM10 & CONFIG_CPU_AMDK8 from mainboards.  They
should be selected in sockets, and they aren't used yet.

I added a couple of variables to src/Kconfig for lack of a better place so
that their selects would work.
I added select statements according to newconfig for some variables that
were defined but never selected in mainboard configs.

Signed-off-by: Myles Watson 

Thanks,
Myles
Index: svn/src/mainboard/amd/serengeti_cheetah_fam10/Kconfig
===
--- svn.orig/src/mainboard/amd/serengeti_cheetah_fam10/Kconfig
+++ svn/src/mainboard/amd/serengeti_cheetah_fam10/Kconfig
@@ -1,7 +1,6 @@
 config BOARD_AMD_SERENGETI_CHEETAH_FAM10
 	bool "Serengeti Cheetah (Fam10)"
 	select ARCH_X86
-	select CPU_AMD_FAM10
 	select CPU_AMD_SOCKET_F_1207
 	select NORTHBRIDGE_AMD_AMDFAM10
 	select NORTHBRIDGE_AMD_AMDFAM10_ROOT_COMPLEX
Index: svn/src/mainboard/supermicro/h8dmr_fam10/Kconfig
===
--- svn.orig/src/mainboard/supermicro/h8dmr_fam10/Kconfig
+++ svn/src/mainboard/supermicro/h8dmr_fam10/Kconfig
@@ -1,7 +1,6 @@
 config BOARD_SUPERMICRO_H8DMR_FAM10
 	bool "H8DMR_FAM10 (Fam10)"
 	select ARCH_X86
-	select CPU_AMD_FAM10
 	select CPU_AMD_SOCKET_F_1207
 	select NORTHBRIDGE_AMD_AMDFAM10
 	select NORTHBRIDGE_AMD_AMDFAM10_ROOT_COMPLEX
@@ -13,6 +12,7 @@ config BOARD_SUPERMICRO_H8DMR_FAM10
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
+	select LIFT_BSP_APIC_ID
 	select AMDMCT
 	select BOARD_ROMSIZE_KB_1024
 
Index: svn/src/mainboard/tyan/s2912_fam10/Kconfig
===
--- svn.orig/src/mainboard/tyan/s2912_fam10/Kconfig
+++ svn/src/mainboard/tyan/s2912_fam10/Kconfig
@@ -1,7 +1,6 @@
 config BOARD_TYAN_S2912_FAM10
 	bool "S2912_FAM10"
 	select ARCH_X86
-	select CPU_AMD_K8
 	select CPU_AMD_SOCKET_F_1207
 	select NORTHBRIDGE_AMD_AMDFAM10
 	select NORTHBRIDGE_AMD_AMDFAM10_ROOT_COMPLEX
@@ -13,6 +12,7 @@ config BOARD_TYAN_S2912_FAM10
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
+	select LIFT_BSP_APIC_ID
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select BOARD_ROMSIZE_KB_1024
 	select ENABLE_APIC_EXT_ID
Index: svn/src/Kconfig
===
--- svn.orig/src/Kconfig
+++ svn/src/Kconfig
@@ -448,3 +448,7 @@ config USE_INIT
 config ENABLE_APIC_EXT_ID
 	bool
 	default n
+
+config LIFT_BSP_APIC_ID
+	bool
+	default n
Index: svn/src/mainboard/amd/dbm690t/Kconfig
===
--- svn.orig/src/mainboard/amd/dbm690t/Kconfig
+++ svn/src/mainboard/amd/dbm690t/Kconfig
@@ -1,7 +1,6 @@
 config BOARD_AMD_DBM690T
 	bool "DBM690T"
 	select ARCH_X86
-	select CPU_AMD_K8
 	select CPU_AMD_SOCKET_S1G1
 	select NORTHBRIDGE_AMD_AMDK8
 	select NORTHBRIDGE_AMD_AMDK8_ROOT_COMPLEX
Index: svn/src/mainboard/amd/pistachio/Kconfig
===
--- svn.orig/src/mainboard/amd/pistachio/Kconfig
+++ svn/src/mainboard/amd/pistachio/Kconfig
@@ -1,7 +1,6 @@
 config BOARD_AMD_PISTACHIO
 	bool "Pistachio"
 	select ARCH_X86
-	select CPU_AMD_K8
 	select CPU_AMD_SOCKET_AM2
 	select NORTHBRIDGE_AMD_AMDK8
 	select NORTHBRIDGE_AMD_AMDK8_ROOT_COMPLEX
Index: svn/src/mainboard/amd/serengeti_cheetah/Kconfig
===
--- svn.orig/src/mainboard/amd/serengeti_cheetah/Kconfig
+++ svn/src/mainboard/amd/serengeti_cheetah/Kconfig
@@ -1,7 +1,6 @@
 config BOARD_AMD_SERENGETI_CHEETAH
 	bool "Serengeti Cheetah"
 	select ARCH_X86
-	select CPU_AMD_K8
 	select CPU_AMD_SOCKET_F
 	select NORTHBRIDGE_AMD_AMDK8
 	select NORTHBRIDGE_AMD_AMDK8_ROOT_COMPLEX
@@ -14,6 +13,7 @@ config BOARD_AMD_SERENGETI_CHEETAH
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
+	select LIFT_BSP_APIC_ID
 	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
Index: svn/src/mainboard/arima/hdama/Kconfig
===
--- svn.orig/src/mainboard/arima/hdama/Kconfig
+++ svn/src/mainboard/arima/hdama/Kconfig
@@ -1,7 +1,6 @@
 config BOARD_ARIMA_HDAMA
 	bool "HDAMA"
 	select ARCH_X86
-	select CPU_AMD_K8
 	select CPU_AMD_SOCKET_940
 	select NORTHBRIDGE_AMD_AMDK8
 	select NORTHBRIDGE_AMD_AMDK8_ROOT_COMPLEX
Index: svn/src/mainboard/asus/a8n_e/Kconfig
===
--- svn.orig/src/mainboard/asus/a8n_e/Kconfig
+++ svn/src/mainboard/asus/a8n_e/Kconfig
@@ -1,7 +1,6 @@
 config BOARD_ASUS_A8N_E
 	bool "A8N-E"
 	select ARCH_X86
-	select CPU_AMD_K8
 	select CPU_AMD_SOCKET_939
 	select NORTHBRIDGE_AMD_AMDK8
 	select NORTHBRIDGE_AMD_AMDK8_ROOT_COMPLEX
Index: svn/src/mainboard/asus/a8v-e_se/Kconfig
===

Re: [coreboot] [PATCH]more kconfig

[Myles Watson]

On Tue, Jan 5, 2010 at 1:37 PM, Patrick Georgi wrote:

> Hi,
>
> attached patch makes all boards build with kconfig (incl. fam10). None
> boot-tested.
>
> Details:
> - move declaration of romstrap files that must be added to the bootblock
> into the related southbridge's Makefiles.
> - amd/model_fxx, amd/model_10xxx provide their own init_timer and udelay
> functions. disable udelay_io default, declare init_timer presence
> - amd/sc520 provides its own udelay function (no init_timer). Declare as
> such
> - supermicro/h8dmr_fam10:
> -- enable tinybootblock
> -- align configuration a bit with newconfig's
> -- move acpi object files to $(obj)
> -- fix copy&paste bug in devicetree.cb
> - emulation/qemu-x86 provides its own timer. Declare as such
> - tyan/s2912_fam10:
> -- enable tinybootblock
> -- align configuration a bit with newconfig's
>
> The remaining issues are:
> - Make configurations match newconfig's
> - Find bugs
>
> Signed-off-by: Patrick Georgi 
>
Acked-by: Myles Watson 

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Kevin O'Connor]

On Tue, Jan 05, 2010 at 09:37:21PM +0100, Patrick Georgi wrote:
> Hi,
> 
> attached patch makes all boards build with kconfig (incl. fam10). None
> boot-tested.

Nice!

I was going to test boot my epia-cn with a kconfig build, but I
noticed that auto.c got compiled with gcc instead of romcc.

-Kevin

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Patrick Georgi]

Am 07.01.2010 06:11, schrieb Kevin O'Connor:
> I was going to test boot my epia-cn with a kconfig build, but I
> noticed that auto.c got compiled with gcc instead of romcc.
>   
Oops, must have been an error when doing the mass conversion. Attached
patch should fix that.
Thanks for the report.


Signed-off-by: Patrick Georgi 
Index: src/mainboard/via/epia-cn/Makefile.inc
===
--- src/mainboard/via/epia-cn/Makefile.inc  (revision 5003)
+++ src/mainboard/via/epia-cn/Makefile.inc  (working copy)
@@ -1,60 +1,3 @@
-##
-## This file is part of the coreboot project.
-##
-## Copyright (C) 2008 VIA Technologies, Inc.
-## (Written by Aaron Lwe  for VIA)
-##
-## This program is free software; you can redistribute it and/or modify
-## it under the terms of the GNU General Public License as published by
-## the Free Software Foundation; either version 2 of the License, or
-## (at your option) any later version.
-##
-## This program is distributed in the hope that it will be useful,
-## but WITHOUT ANY WARRANTY; without even the implied warranty of
-## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-## GNU General Public License for more details.
-##
-## You should have received a copy of the GNU General Public License
-## along with this program; if not, write to the Free Software
-## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
-##
+ROMCCFLAGS := -mcpu=c3 -O
+include $(src)/mainboard/Makefile.romccboard.inc
 
-initobj-y += crt0.o
-obj-y += mainboard.o
-obj-$(CONFIG_GENERATE_PIRQ_TABLE) += irq_tables.o
-obj-$(CONFIG_GENERATE_MP_TABLE) += mptable.o
-
-obj-$(CONFIG_GENERATE_ACPI_TABLES) += fadt.o
-obj-$(CONFIG_GENERATE_ACPI_TABLES) += dsdt.o
-obj-$(CONFIG_GENERATE_ACPI_TABLES) += acpi_tables.o
-
-ldscript-y += ../../../../src/arch/i386/init/ldscript_fallback_cbfs.lb
-ldscript-y += ../../../../src/cpu/x86/16bit/entry16.lds
-ldscript-y += ../../../../src/cpu/x86/16bit/reset16.lds
-ldscript-y += ../../../../src/arch/i386/lib/id.lds
-ldscript-y += ../../../../src/arch/i386/lib/failover.lds
-
-crt0-y += ../../../../src/cpu/x86/16bit/entry16.inc
-crt0-y += ../../../../src/cpu/x86/32bit/entry32.inc
-crt0-y += ../../../../src/cpu/x86/16bit/reset16.inc
-crt0-y += ../../../../src/arch/i386/lib/id.inc
-crt0-y += ../../../../src/cpu/x86/fpu_enable.inc
-crt0-y += auto.inc
-crt0-y += ../../../../src/cpu/x86/mmx_disable.inc
-
-ifdef POST_EVALUATION
-
-$(obj)/mainboard/$(MAINBOARDDIR)/dsdt.o: $(obj)/dsdt.c
-   $(CC) $(DISTRO_CFLAGS) $(CFLAGS) $(CPPFLAGS) $(DEBUG_CFLAGS) -I$(src) 
-I. -c $< -o $@
-
-$(obj)/dsdt.c: $(src)/mainboard/$(MAINBOARDDIR)/dsdt.asl
-   iasl -p dsdt -tc $(src)/mainboard/$(MAINBOARDDIR)/dsdt.asl
-   mv dsdt.hex $@
-
-$(obj)/mainboard/$(MAINBOARDDIR)/auto.inc: 
$(src)/mainboard/$(MAINBOARDDIR)/auto.c $(obj)/option_table.h
-   $(CC) $(DISTRO_CFLAGS) $(CFLAGS) $(CPPFLAGS) $(DEBUG_CFLAGS) -I$(src) 
-I. -c -S  $(src)/mainboard/$(MAINBOARDDIR)/auto.c -o $@
-   perl -e 's/\.rodata/.rom.data/g' -pi $@
-   perl -e 's/\.text/.section .rom.text/g' -pi $@
-
-endif
-
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig

[Peter Stuge]

Patrick Georgi wrote:
> > I was going to test boot my epia-cn with a kconfig build, but I
> > noticed that auto.c got compiled with gcc instead of romcc.
> 
> Oops, must have been an error when doing the mass conversion.
> Attached patch should fix that.
> Thanks for the report.
> 
> Signed-off-by: Patrick Georgi 

Acked-by: Peter Stuge 

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH]more kconfig fixes

[Stefan Reinauer]

Patrick Georgi wrote:
> Hi,
>
> with this patch (and my previous one), all kconfig enabled boards build
> for me.
>
> Changes this time:
> - Fix for superio/smsc/lpc47b272
> - disable option table for asus/mew-vm
> - change superio to use for compaq/deskpro_en_sff_p600 to be in line
>   with what newconfig/buildtarget does
>
>
> Signed-off-by: Patrick Georgi 
>
> Regards,
> Patrick
>   
Acked-by: Stefan Reinauer 

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
  Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: i...@coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more kconfig support

[ron minnich]

Acked-by: Ronald G. Minnich 

Wonderful.

ron

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more kconfig support

[Uwe Hermann]

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
> ===

Re: [coreboot] [PATCH] more kconfig support

[Patrick Georgi]

Am Montag, den 28.09.2009, 15:23 +0200 schrieb Uwe Hermann:
> 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.
I committed it, so just using HEAD should do it.
The configuration options might still be way off, I'll handle them all
once all boards are supported.

> Some parts inlined here for review:
Thanks


> > 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 (?).
vt82c686 is a weird stub of southbridge support: I found no use of it,
and it seems to handle serial enable, only. My proposal? Unless it will
actually be used, get rid of it.


> > 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"?
In this case, CONFIG_SOUTHBRIDGE_INTEL_I82801GX is already covered by
the statement that includes this very Makefile.inc.
In general, the ifeq(..) thing is our solution for compound
conditionals, but this time we got away with it.


> > 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.
right. I think that might be the result of copy&paste from other,
similar files.


> > 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?
I think so. If not, it should be renamed to CPU_INTEL_MODEL_1067X in my
opinion.


> > 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?
Not sure if these can be scripted. I had this default to "n" first, but
that didn't compile, and I'm not sure yet what the right selection here
is.
And defaulting to "n" requires this more verbose statement.


> > 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?
It might be possible to eliminate it by now. I _think_ it started out as
a workaround in the early stages of kconfig, but I didn't dare to remove
it, yet.
That might be a good, isolated, low-risk issue to look into: Do we still
need it? Could we kill it by changing the makefile?


> > Index: src/mainboard/tyan/s2850/Kconfig
> > ===
> > --- src/mainboard/tyan/s2850/Kconfig(Revision 0)
> > +++ src/mainboard/tyan/s2850/Kconfig(Revision 0)
> > @@ -0,0 +1,65 @@

Re: [coreboot] [PATCH] more kconfig support

[Stefan Reinauer]

Patrick Georgi wrote:
>>> 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 (?).
>> 
> vt82c686 is a weird stub of southbridge support: I found no use of it,
> and it seems to handle serial enable, only. My proposal? Unless it will
> actually be used, get rid of it.
>   

Acked-by: Stefan Reinauer 


>>> 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?
>> 
> I think so. If not, it should be renamed to CPU_INTEL_MODEL_1067X in my
> opinion.
>   

Is that really the implication? I thought it means that if
model_1067x/Kconfig is loaded, code that belongs to core2 is also
selected. That's good, and it should be different from the assumption
that all mode_1067x are core2, even though I think this is correct (some
might be Atom, though, not 100% sure)

Stefan


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more kconfig support

[Peter Stuge]

Uwe Hermann wrote:
> > 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?

PLEASE don't wait until later!

New code being developed between first introduction of Kconfig and
"later" will inherit improper use of Kconfig - I so don't want that.

If a board is not using Kconfig perfectly I think it's better that it
does not build!

That is one (IMO good) way to encourage proper Kconfig use.

At this point only very few developers really know how Kconfig is
supposed to be used, and the rest of us are fighting the learning
curve - with delight. However, it is very easy to use bad examples -
which then causes more things that need fixing.

Can we avoid this?


> > +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?

Yes, I'd also like to know.


> > +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.

Go for it.


> >  ifdef POST_EVALUATION

Again with POST_EVALUATION. What is that?


> > +CPUTYPE ?= p2
> 
> Yep, this kind of fix was on my TODO list also.
> 
> Maybe ROMCC_MCPU (a bit more descriptive I guess)?

Yes please!


> > +++ 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

Yes please!


Thanks for finding this Uwe.


//Peter

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more kconfig support

[Peter Stuge]

Stefan Reinauer wrote:
> >>> +++ 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?
> >
> > I think so. If not, it should be renamed to CPU_INTEL_MODEL_1067X in
> > my opinion.
> 
> Is that really the implication? I thought it means that if
> model_1067x/Kconfig is loaded, code that belongs to core2 is also
> selected. That's good, and it should be different from the
> assumption that all mode_1067x are core2, even though I think this
> is correct (some might be Atom, though, not 100% sure)

Example from Linux:

--8<-- drivers/parport/Kconfig
if PARPORT
config PARPORT_PC
tristate "PC-style hardware"
depends on (!SPARC64 || PCI) && !SPARC32 && !M32R && !FRV && \
   (!M68K || ISA) && !MN10300 && !AVR32 && !BLACKFIN
..
config PARPORT_PC_FIFO
bool "Use FIFO/DMA if available (EXPERIMENTAL)"
depends on PARPORT_PC && EXPERIMENTAL
..
config PARPORT_PC_PCMCIA
tristate "Support for PCMCIA management for PC-style ports"
depends on PCMCIA && PARPORT_PC
..
config PARPORT_AMIGA
tristate "Amiga builtin port"
depends on AMIGA
select PARPORT_NOT_PC
..
config PARPORT_AX88796
tristate "AX88796 Parallel Port"
select PARPORT_NOT_PC
..
config PARPORT_NOT_PC
bool

endif # PARPORT
-->8--

--8<-- include/linux/parport.h
/* If PC hardware is the only type supported, we can optimise a bit. 
*/
#if !defined(CONFIG_PARPORT_NOT_PC)

#include 
#define parport_write_data(p,x)parport_pc_write_data(p,x)
#define parport_read_data(p)   parport_pc_read_data(p)
..
#else  /*  !CONFIG_PARPORT_NOT_PC  */

/* Generic operations vector through the dispatch table. */
#define parport_write_data(p,x)(p)->ops->write_data(p,x)
#define parport_read_data(p)   (p)->ops->read_data(p)
..
#endif /*  !CONFIG_PARPORT_NOT_PC  */
-->8--

We might make more use of Kconfig if in our Kconfigs. But for select,
it seems it does mean that stuff will be defined for the source code.


//Peter

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more kconfig support

[Uwe Hermann]

On Tue, Sep 29, 2009 at 01:30:04AM +0200, Stefan Reinauer wrote:
> Patrick Georgi wrote:
> >>> 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 (?).
> >> 
> > vt82c686 is a weird stub of southbridge support: I found no use of it,
> > and it seems to handle serial enable, only. My proposal? Unless it will
> > actually be used, get rid of it.
> >   
> 
> Acked-by: Stefan Reinauer 

Please don't drop this. I have some boards with this southbridge, and
the code that's there looks OK (though unfinished, of course). I'll
put this on my TODO list and improve the SB code one of these days.


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: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more kconfig support

[Stefan Reinauer]

Uwe Hermann wrote:
> On Tue, Sep 29, 2009 at 01:30:04AM +0200, Stefan Reinauer wrote:
>   
>> Patrick Georgi wrote:
>> 
> 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 (?).
 
 
>>> vt82c686 is a weird stub of southbridge support: I found no use of it,
>>> and it seems to handle serial enable, only. My proposal? Unless it will
>>> actually be used, get rid of it.
>>>   
>>>   
>> Acked-by: Stefan Reinauer 
>> 
>
> Please don't drop this. I have some boards with this southbridge, and
> the code that's there looks OK (though unfinished, of course). I'll
> put this on my TODO list and improve the SB code one of these days.
>   
I think we should not keep around broken components like that. An early
serial init like that is trivial and not more than half an hour of
effort. On the other side, the existence of the component in the tree
will create a wrong impression about what coreboot can do or can not do.

This code is not unfinished, as it's not being worked on..  It's in svn
history in case we'll some day have the time to support this.

Same reasoning as goes for the ppc port...

Stefan


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more kconfig support

[Patrick Georgi]

Am Dienstag, den 29.09.2009, 01:38 +0200 schrieb Peter Stuge:
> > This can be a simple "select HAVE_HARD_RESET" above, but I assume
> > you'll do this kind of changes with a script later?
> 
> PLEASE don't wait until later!
"later" means immediately after all boards are kconfig-aware.
My plan is:
1. get them to build
2. compare old and new build, and make new match old in terms of
   configuration options
3. let other people fix up the remaining issues as they run into them
   with their boards (I don't have all boards around)

I won't fight fixes to existing boards, but I'm not working
board-by-board, that's just too much a hassle.

> New code being developed between first introduction of Kconfig and
> "later" will inherit improper use of Kconfig - I so don't want that.
> 
> If a board is not using Kconfig perfectly I think it's better that it
> does not build!
I can easily add a typo everywhere - that won't help you much.

> Can we avoid this?
The issue is mostly that configuration options are missing, but nothing
"structural".
While there are many things I want to clean up once the old build system
is gone, that will be _later_ steps. I don't want kconfig be killed by
too-many-things-at-once.

> > Can't we eliminate MAINBOARD_OPTIONS?
> 
> Yes, I'd also like to know.
Legacy stuff from early development. Soon to be gone (see other mail).

> > Hm, this is a bit of a hack, maybe make a generic ROMCC_CFLAGS then
> > like this?
> > 
> >   ROMCC_CFLGAGS = -mcpu=p4 -fno-simplify-phi
> 
> Yes please!
Done, called ROMCCFLAGS, with -mcpu=p2 as default




-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More kconfig fixes

[Myles Watson]

> - Set CONFIG_PIRQ_ROUTE=1 for boards which have an irq_tables.c but
>for some reason had CONFIG_PIRQ_ROUTE=0.
>For EPIA-M700 I'm not sure it should be used at all, as this board has
>ACPI tables (?)
I don't think we should do this, unless it's been boot tested.  If the
default was not to use it with newconfig, it could be broken.  Right now
we're trying to match newconfig.

The other parts:
Acked-by: Myles Watson 

Thanks,
Myles


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More kconfig fixes

[Uwe Hermann]

On Thu, Oct 08, 2009 at 09:07:40AM -0600, Myles Watson wrote:
> > - Set CONFIG_PIRQ_ROUTE=1 for boards which have an irq_tables.c but
> >for some reason had CONFIG_PIRQ_ROUTE=0.
> >For EPIA-M700 I'm not sure it should be used at all, as this board has
> >ACPI tables (?)
> I don't think we should do this, unless it's been boot tested.  If the
> default was not to use it with newconfig, it could be broken.  Right now
> we're trying to match newconfig.

Yep, converted to TODO comments for now (Except for one board which
actually set the variable first to 0, then to 1 in the same file, where
I assume the second overrides the first).


> Acked-by: Myles Watson 

Thanks, r4749.


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: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

[coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

Fix AP_CODE_IN_CAR (only selected for two boards), STACK_SIZE, and HEAP_SIZE.

Signed-off-by: Myles Watson 

At this point my Tyan s2895 is happy with Kconfig.  Boot tested.

Thanks,
Myles
Index: svn/src/mainboard/amd/dbm690t/Kconfig
===
--- svn.orig/src/mainboard/amd/dbm690t/Kconfig
+++ svn/src/mainboard/amd/dbm690t/Kconfig
@@ -13,7 +13,6 @@ config BOARD_AMD_DBM690T
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select HAVE_ACPI_TABLES
Index: svn/src/mainboard/amd/pistachio/Kconfig
===
--- svn.orig/src/mainboard/amd/pistachio/Kconfig
+++ svn/src/mainboard/amd/pistachio/Kconfig
@@ -12,7 +12,6 @@ config BOARD_AMD_PISTACHIO
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select HAVE_ACPI_TABLES
Index: svn/src/mainboard/amd/serengeti_cheetah_fam10/Kconfig
===
--- svn.orig/src/mainboard/amd/serengeti_cheetah_fam10/Kconfig
+++ svn/src/mainboard/amd/serengeti_cheetah_fam10/Kconfig
@@ -13,7 +13,6 @@ config BOARD_AMD_SERENGETI_CHEETAH_FAM10
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select AMDMCT
Index: svn/src/mainboard/arima/hdama/Kconfig
===
--- svn.orig/src/mainboard/arima/hdama/Kconfig
+++ svn/src/mainboard/arima/hdama/Kconfig
@@ -13,7 +13,6 @@ config BOARD_ARIMA_HDAMA
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 
Index: svn/src/mainboard/gigabyte/ga_2761gxdk/Kconfig
===
--- svn.orig/src/mainboard/gigabyte/ga_2761gxdk/Kconfig
+++ svn/src/mainboard/gigabyte/ga_2761gxdk/Kconfig
@@ -97,11 +97,6 @@ config MAX_PHYSICAL_CPUS
 	default 1
 	depends on BOARD_GIGABYTE_GA_2761GXDK
 
-config AP_CODE_IN_CAR
-	bool	
-	default n
-	depends on BOARD_GIGABYTE_GA_2761GXDK
-
 config HW_MEM_HOLE_SIZE_AUTO_INC
 	bool	
 	default n
Index: svn/src/mainboard/gigabyte/m57sli/Kconfig
===
--- svn.orig/src/mainboard/gigabyte/m57sli/Kconfig
+++ svn/src/mainboard/gigabyte/m57sli/Kconfig
@@ -99,11 +99,6 @@ config MAX_PHYSICAL_CPUS
 	default 1
 	depends on BOARD_GIGABYTE_M57SLI
 
-config AP_CODE_IN_CAR
-	bool	
-	default n
-	depends on BOARD_GIGABYTE_M57SLI
-
 config HW_MEM_HOLE_SIZE_AUTO_INC
 	bool	
 	default n
Index: svn/src/mainboard/ibm/e325/Kconfig
===
--- svn.orig/src/mainboard/ibm/e325/Kconfig
+++ svn/src/mainboard/ibm/e325/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IBM_E325
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 
Index: svn/src/mainboard/ibm/e326/Kconfig
===
--- svn.orig/src/mainboard/ibm/e326/Kconfig
+++ svn/src/mainboard/ibm/e326/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IBM_E326
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 
Index: svn/src/mainboard/iwill/dk8_htx/Kconfig
===
--- svn.orig/src/mainboard/iwill/dk8_htx/Kconfig
+++ svn/src/mainboard/iwill/dk8_htx/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IWILL_DK8_HTX
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select HAVE_ACPI_TABLES
Index: svn/src/mainboard/iwill/dk8s2/Kconfig
===
--- svn.orig/src/mainboard/iwill/dk8s2/Kconfig
+++ svn/src/mainboard/iwill/dk8s2/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IWILL_DK8S2
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select ATI_RAGE_XL
Index: svn/src/mainboard/iwill/dk8x/Kconfig
===
--- svn.orig/src/mainboard/iwill/dk8x/Kconfig
+++ svn/src/mainboard/iwill/dk8x/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IWILL_DK8X
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 
Index: svn/src/mainboard/msi/ms7135/Kconfig
===

Re: [coreboot] [PATCH] More Kconfig changes

[Peter Stuge]

Myles Watson wrote:
> +config GENERATE_ACPI_TABLES
>   bool
> + default y if HAVE_ACPI_TABLES
> + default n

Can it be simply:

default HAVE_ACPI_TABLES

?


> +++ svn/src/northbridge/amd/amdfam10/Kconfig
> @@ -21,11 +21,35 @@ config NORTHBRIDGE_AMD_AMDFAM10
>   bool
>   select HAVE_HIGH_TABLES
>   select HYPERTRANSPORT_PLUGIN_SUPPORT
> - select HT3_SUPPORT
>  
>  config AGP_APERTURE_SIZE
>   hex
>   default 0x400
>   depends on NORTHBRIDGE_AMD_AMDFAM10
>  
> +config HT3_SUPPORT
> + bool
> + default y
> + depends on NORTHBRIDGE_AMD_AMDFAM10

Is this good?

I find having single line selects instead of repeating the horribly
long NB option name over and over very appealing but I don't know
enough about Kconfig to say if it makes a difference?

Essentially this is a doubly linked list where both links must be
explicitly described by Kconfig file authors, and I would so prefer
if only one link needed to be described explicitly.


//Peter

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Myles Watson]

On Mon, Oct 19, 2009 at 3:39 PM, Peter Stuge  wrote:

> Myles Watson wrote:
> > +config GENERATE_ACPI_TABLES
> >   bool
> > + default y if HAVE_ACPI_TABLES
> > + default n
>
> Can it be simply:
>
> default HAVE_ACPI_TABLES
>
It looks like it could.  We have a mix of the two styles right now.

src/console/Kconfig:default MAXIMUM_CONSOLE_LOGLEVEL_8
src/console/Kconfig:default DEFAULT_CONSOLE_LOGLEVEL_8
src/mainboard/Kconfig:default VENDOR_EMULATION
src/mainboard/Kconfig:default COREBOOT_ROMSIZE_KB_128 if
BOARD_ROMSIZE_KB_128
src/mainboard/Kconfig:default COREBOOT_ROMSIZE_KB_256 if
BOARD_ROMSIZE_KB_256

> +++ svn/src/northbridge/amd/amdfam10/Kconfig
> > @@ -21,11 +21,35 @@ config NORTHBRIDGE_AMD_AMDFAM10
> >   bool
> >   select HAVE_HIGH_TABLES
> >   select HYPERTRANSPORT_PLUGIN_SUPPORT
> > - select HT3_SUPPORT
> >
> >  config AGP_APERTURE_SIZE
> >   hex
> >   default 0x400
> >   depends on NORTHBRIDGE_AMD_AMDFAM10
> >
> > +config HT3_SUPPORT
> > + bool
> > + default y
> > + depends on NORTHBRIDGE_AMD_AMDFAM10
>
> Is this good?
>
It has to be defined somewhere.  The alternative is to put config
HT3_SUPPORT somewhere else, then use select in this file.  Since there are
no other devices with HT3_SUPPORT at this time, I think this is the place to
put it.

If you use select and HT3_SUPPORT isn't defined somewhere else, it silently
fails.


> I find having single line selects instead of repeating the horribly
> long NB option name over and over very appealing

Me too.


> but I don't know
> enough about Kconfig to say if it makes a difference?
>
In this case it does.

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Myles Watson]

On Mon, Oct 19, 2009 at 3:51 PM, Myles Watson  wrote:

>
>
> On Mon, Oct 19, 2009 at 3:39 PM, Peter Stuge  wrote:
>
>> Myles Watson wrote:
>> > +config GENERATE_ACPI_TABLES
>> >   bool
>> > + default y if HAVE_ACPI_TABLES
>> > + default n
>>
>> Can it be simply:
>>
>> default HAVE_ACPI_TABLES
>>
> It looks like it could.  We have a mix of the two styles right now.
>
No.  You're right about the defaults.  Fixed.

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Uwe Hermann]

On Mon, Oct 19, 2009 at 03:18:07PM -0600, Myles Watson wrote:
> Define HAVE_INIT_TIMER to only exclude the three boards that define it to be
> 0 in newconfig.

What exactly does HAVE_INIT_TIMER configure? Why do those three boards
_not_ set it? It sounds like all boards would want to set it?


> MOVNTI is a performance enhancement, and should default to 0 so it doesn't
> break boards that forget to define it.

Are all Kconfig files setting to y (via "select" preferrably) where needed?

 
> Index: svn/src/Kconfig
> ===
> --- svn.orig/src/Kconfig
> +++ svn/src/Kconfig
> @@ -74,10 +74,6 @@ config CPU_ADDR_BITS
>   int
>   default 36
>  
> -config AGP_APERTURE_SIZE
> - hex
> - default 0x0
> -

Why is this removed?


> +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

Hm, indeed. Why was this added? Are there situations where we might not
want LOW_TABLES on?


> +config HAVE_HIGH_TABLES
> + bool
> + default n
> + help
> +   This variable specifies whether a given northbridge has high table
> +   support.
> +   It is set in northbridge/*/Kconfig.
> +   Whether or not the high tables are actually written by coreboot is
> +   configurable by the user via WRITE_HIGH_TABLES.
> +
>  config HAVE_ACPI_TABLES
>   bool
>   help
> @@ -262,14 +280,30 @@ config HAVE_PIRQ_TABLE
> Whether or not the PIRQ table is actually generated by coreboot
> is configurable by the user via GENERATE_PIRQ_TABLE.
>  
> -config HAVE_HIGH_TABLES
> +#These Options are here to avoid "undefined" warnings.
> +#The actual selection and help texts are in the following menu.
  

> Index: svn/src/northbridge/amd/amdfam10/Kconfig
> ===
> --- svn.orig/src/northbridge/amd/amdfam10/Kconfig
> +++ svn/src/northbridge/amd/amdfam10/Kconfig
> @@ -21,11 +21,35 @@ config NORTHBRIDGE_AMD_AMDFAM10
>   bool
>   select HAVE_HIGH_TABLES
>   select HYPERTRANSPORT_PLUGIN_SUPPORT
> - select HT3_SUPPORT
[...]
> +config HT3_SUPPORT
> + bool
> + default y
> + depends on NORTHBRIDGE_AMD_AMDFAM10

Why this? "select HT3_SUPPORT" should work just fine, no?
Even if that's not the case I think it would be nicer to set it in some
global Kconfig file so we can "select" it. IMHO _all_ y variables should
be "select"ed so we avoid the 4-line chunks...


> +config AMDMCT
> + bool
> + default y
> + depends on NORTHBRIDGE_AMD_AMDFAM10

Ditto.


> +config MEM_TRAIN_SEQ
> +config HW_MEM_HOLE_SIZEK
> +config HW_MEM_HOLE_SIZE_AUTO_INC
> Index: svn/src/northbridge/amd/amdk8/Kconfig
> ===
> --- svn.orig/src/northbridge/amd/amdk8/Kconfig
> +++ svn/src/northbridge/amd/amdk8/Kconfig
> +config MEM_TRAIN_SEQ
> +config HW_MEM_HOLE_SIZEK
> +config HW_MEM_HOLE_SIZE_AUTO_INC

Why are these three defined twice?


> Index: svn/src/southbridge/via/vt8237r/vt8237r.h
> ===
> --- svn.orig/src/southbridge/via/vt8237r/vt8237r.h
> +++ svn/src/southbridge/via/vt8237r/vt8237r.h
> @@ -65,7 +65,7 @@
>  #define I2C_TRANS_CMD0x40
>  #define CLOCK_SLAVE_ADDRESS  0x69
>  
> -#if DEBUG_SMBUS == 1
> +#if defined(DEBUG_SMBUS) && DEBUG_SMBUS == 1
>  #define PRINT_DEBUG(x)   print_debug(x)
>  #define PRINT_DEBUG_HEX16(x) print_debug_hex16(x)
>  #else

Please drop this hunk, I have a patch in the queue to redo DEBUG_SMBUS
and other DEBUG options globally anyway.


> Index: svn/src/cpu/amd/model_10xxx/init_cpus.c
> ===
> --- svn.orig/src/cpu/amd/model_10xxx/init_cpus.c
> +++ svn/src/cpu/amd/model_10xxx/init_cpus.c
> @@ -473,8 +473,8 @@ static void start_node(u8 node)
>   /* Enable routing table */
>   printk_debug("Start node %02x", node);
>  
> -#if CONFIG_CAR_FAM10 == 1
> - /* For CONFIG_CAR_FAM10 support, we need to set Dram base/limit for the 
> new node */
> +#if CONFIG_NORTHBRIDGE_AMD_AMDFAM10 == 1

#if CONFIG_NORTHBRIDGE_AMD_AMDFAM10
should suffice, I think.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.randomprojects.org
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Myles Watson]

On Mon, Oct 19, 2009 at 4:17 PM, Uwe Hermann  wrote:

> On Mon, Oct 19, 2009 at 03:18:07PM -0600, Myles Watson wrote:
> > Define HAVE_INIT_TIMER to only exclude the three boards that define it to
> be
> > 0 in newconfig.
>
> What exactly does HAVE_INIT_TIMER configure? Why do those three boards
> _not_ set it? It sounds like all boards would want to set it?
>
I don't know.  Since one of the boards is qemu, I figured that it might not
have some functionality that hardware has.


> > MOVNTI is a performance enhancement, and should default to 0 so it
> doesn't
> > break boards that forget to define it.
>
> Are all Kconfig files setting to y (via "select" preferrably) where needed?
>
It's hard to check since this option is set in sockets.  That is a
worthwhile follow up patch.  It just doesn't make sense to break booting for
performance.

> Index: svn/src/Kconfig
> > ===
> > --- svn.orig/src/Kconfig
> > +++ svn/src/Kconfig
> > @@ -74,10 +74,6 @@ config CPU_ADDR_BITS
> >   int
> >   default 36
> >
> > -config AGP_APERTURE_SIZE
> > - hex
> > - default 0x0
> > -
>
> Why is this removed?
>
It shouldn't be a global option.  It's only used by amd northbridges.
Setting global defaults for specific options makes it harder to see when
something is broken.


> > +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
>
> Hm, indeed. Why was this added? Are there situations where we might not
> want LOW_TABLES on?
>
Good question.  I think there might be times where you don't want to corrupt
low memory.


>
> > Index: svn/src/northbridge/amd/amdfam10/Kconfig
> > ===
> > --- svn.orig/src/northbridge/amd/amdfam10/Kconfig
> > +++ svn/src/northbridge/amd/amdfam10/Kconfig
> > @@ -21,11 +21,35 @@ config NORTHBRIDGE_AMD_AMDFAM10
> >   bool
> >   select HAVE_HIGH_TABLES
> >   select HYPERTRANSPORT_PLUGIN_SUPPORT
> > - select HT3_SUPPORT
> [...]
> > +config HT3_SUPPORT
> > + bool
> > + default y
> > + depends on NORTHBRIDGE_AMD_AMDFAM10
>
> Why this? "select HT3_SUPPORT" should work just fine, no?
> Even if that's not the case I think it would be nicer to set it in some
> global Kconfig file so we can "select" it. IMHO _all_ y variables should
> be "select"ed so we avoid the 4-line chunks...
>
It doesn't work unless HT3_SUPPORT is defined somewhere else.  It doesn't
make sense to define it anywhere else to me, since only FAM10 uses it.


>
> > +config AMDMCT
> > + bool
> > + default y
> > + depends on NORTHBRIDGE_AMD_AMDFAM10
>
> Ditto.
>
Yep.


> > +config MEM_TRAIN_SEQ
> > +config HW_MEM_HOLE_SIZEK
> > +config HW_MEM_HOLE_SIZE_AUTO_INC
> > Index: svn/src/northbridge/amd/amdk8/Kconfig
> > ===
> > --- svn.orig/src/northbridge/amd/amdk8/Kconfig
> > +++ svn/src/northbridge/amd/amdk8/Kconfig
> > +config MEM_TRAIN_SEQ
> > +config HW_MEM_HOLE_SIZEK
> > +config HW_MEM_HOLE_SIZE_AUTO_INC
>
> Why are these three defined twice?
>
So that they only show up when they're used.


> > Index: svn/src/southbridge/via/vt8237r/vt8237r.h
> > ===
> > --- svn.orig/src/southbridge/via/vt8237r/vt8237r.h
> > +++ svn/src/southbridge/via/vt8237r/vt8237r.h
> > @@ -65,7 +65,7 @@
> >  #define I2C_TRANS_CMD0x40
> >  #define CLOCK_SLAVE_ADDRESS  0x69
> >
> > -#if DEBUG_SMBUS == 1
> > +#if defined(DEBUG_SMBUS) && DEBUG_SMBUS == 1
> >  #define PRINT_DEBUG(x)   print_debug(x)
> >  #define PRINT_DEBUG_HEX16(x) print_debug_hex16(x)
> >  #else
>
> Please drop this hunk, I have a patch in the queue to redo DEBUG_SMBUS
> and other DEBUG options globally anyway.
>
Sure.  I was just trying to get most of the not defined warnings.  Dropped.


> > Index: svn/src/cpu/amd/model_10xxx/init_cpus.c
> > ===
> > --- svn.orig/src/cpu/amd/model_10xxx/init_cpus.c
> > +++ svn/src/cpu/amd/model_10xxx/init_cpus.c
> > @@ -473,8 +473,8 @@ static void start_node(u8 node)
> >   /* Enable routing table */
> >   printk_debug("Start node %02x", node);
> >
> > -#if CONFIG_CAR_FAM10 == 1
> > - /* For CONFIG_CAR_FAM10 support, we need to set Dram base/limit for
> the new node */
> > +#if CONFIG_NORTHBRIDGE_AMD_AMDFAM10 == 1
>
> #if CONFIG_NORTHBRIDGE_AMD_AMDFAM10
> should suffice, I think.
>
Yes.  Fixed.

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig selects

[Uwe Hermann]

On Tue, Oct 20, 2009 at 09:19:37AM -0600, Myles Watson wrote:
> I compared the config variables that we select with the list that we define.
> 
> I removed CONFIG_CPU_AMD_FAM10 & CONFIG_CPU_AMDK8 from mainboards.  They
> should be selected in sockets, and they aren't used yet.
> 
> I added a couple of variables to src/Kconfig for lack of a better place so
> that their selects would work.
> I added select statements according to newconfig for some variables that
> were defined but never selected in mainboard configs.
> 
> Signed-off-by: Myles Watson 

Acked-by: Uwe Hermann 

But see below for material for discussions and/or a followup patch.


> Index: svn/src/mainboard/asus/m2v-mx_se/Kconfig
> ===
> --- svn.orig/src/mainboard/asus/m2v-mx_se/Kconfig
> +++ svn/src/mainboard/asus/m2v-mx_se/Kconfig
> @@ -21,7 +21,6 @@
>  config BOARD_ASUS_M2V_MX_SE
>   bool "M2V-MX SE"
>   select ARCH_X86
> - select CPU_AMD_K8
>   select CPU_AMD_SOCKET_AM2
>   select NORTHBRIDGE_AMD_AMDK8
>   select NORTHBRIDGE_AMD_AMDK8_ROOT_COMPLEX
> @@ -35,6 +34,7 @@ config BOARD_ASUS_M2V_MX_SE
>   select HAVE_OPTION_TABLE
>   select HAVE_ACPI_TABLES
>   select BOARD_ROMSIZE_KB_512
> + select VGA

All CONFIG_VGA occurences:

config/Options.lb:define CONFIG_VGA

OK


include/pc80/vga.h:#if (CONFIG_VGA == 1)
include/pc80/vga.h:#endif /* (CONFIG_VGA == 1) */

These should probably be changed to
  #if CONFIG_VGA


mainboard/intel/eagleheights/Options.lb:uses CONFIG_VGA
mainboard/intel/eagleheights/Options.lb:#default CONFIG_VGA=0

Unused here, so can be dropped from Options.lb?


mainboard/asus/m2v-mx_se/Options.lb:uses CONFIG_VGA
mainboard/asus/m2v-mx_se/Options.lb:default CONFIG_VGA = 1

OK.


pc80/vga/Makefile.inc:obj-$(CONFIG_VGA) += vga.o
pc80/vga/Config.lb:uses CONFIG_VGA
pc80/vga/Config.lb:if CONFIG_VGA

OK.


southbridge/via/k8t890/k8m890_chrome.c:#if CONFIG_VGA == 1
southbridge/via/k8t890/k8m890_chrome.c:#if CONFIG_VGA == 1

These should probably be changed to
  #if CONFIG_VGA


> Index: svn/src/mainboard/emulation/qemu-x86/Kconfig
> ===
> --- svn.orig/src/mainboard/emulation/qemu-x86/Kconfig
> +++ svn/src/mainboard/emulation/qemu-x86/Kconfig
> @@ -1,7 +1,6 @@
>  config BOARD_EMULATION_QEMU_X86
>   bool "QEMU x86"
>   select ARCH_X86
> - select CPU_I586
>   select SOUTHBRIDGE_INTEL_I82371EB
>   select CPU_EMULATION_QEMU_X86
>   select HAVE_PIRQ_TABLE

Yep, I think this is a remainder from v3.


> Index: svn/src/mainboard/intel/jarrell/Kconfig
> ===
> --- svn.orig/src/mainboard/intel/jarrell/Kconfig
> +++ svn/src/mainboard/intel/jarrell/Kconfig
> @@ -9,6 +9,7 @@ config BOARD_INTEL_JARRELL
>   select HAVE_PIRQ_TABLE
>   select HAVE_MP_TABLE
>   select UDELAY_TSC
> + select USE_WATCHDOG_ON_BOOT
>   select ATI_RAGE_XL
>   select BOARD_ROMSIZE_KB_2048

There are the following occurences:

config/Options.lb:define CONFIG_USE_WATCHDOG_ON_BOOT

  The comment says "Use the watchdog on booting" which is not really
  descriptive. What does "use" mean here? Disable it? We should fix
  the comment I guess (and the resp. Kconfig help text).

include/part/watchdog.h:#if CONFIG_USE_WATCHDOG_ON_BOOT == 1
Kconfig:config USE_WATCHDOG_ON_BOOT

mainboard/kontron/986lcd-m/Options.lb:uses CONFIG_USE_WATCHDOG_ON_BOOT
mainboard/kontron/986lcd-m/Options.lb:default CONFIG_USE_WATCHDOG_ON_BOOT=0
mainboard/intel/eagleheights/Options.lb:uses CONFIG_USE_WATCHDOG_ON_BOOT
mainboard/intel/eagleheights/Options.lb:default CONFIG_USE_WATCHDOG_ON_BOOT=0

These two can drop the variable completely I assume, as they don't use it?


mainboard/intel/jarrell/Options.lb:uses CONFIG_USE_WATCHDOG_ON_BOOT
mainboard/intel/jarrell/Options.lb:default CONFIG_USE_WATCHDOG_ON_BOOT=1

This is fixed by your change above.


mainboard/lippert/spacerunner-lx/cache_as_ram_auto.c:#if 
!CONFIG_USE_WATCHDOG_ON_BOOT
mainboard/lippert/roadrunner-lx/cache_as_ram_auto.c:#if 
!CONFIG_USE_WATCHDOG_ON_BOOT

Not sure about these two. They use the variable but never add a "uses"
line in Option.lb?


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.randomprojects.org
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig selects

[Myles Watson]

On Tue, Oct 20, 2009 at 9:42 AM, Uwe Hermann  wrote:

> On Tue, Oct 20, 2009 at 09:19:37AM -0600, Myles Watson wrote:
> > I compared the config variables that we select with the list that we
> define.
> >
> > I removed CONFIG_CPU_AMD_FAM10 & CONFIG_CPU_AMDK8 from mainboards.  They
> > should be selected in sockets, and they aren't used yet.
> >
> > I added a couple of variables to src/Kconfig for lack of a better place
> so
> > that their selects would work.
> > I added select statements according to newconfig for some variables that
> > were defined but never selected in mainboard configs.
> >
> > Signed-off-by: Myles Watson 
>
> Acked-by: Uwe Hermann 
>
Thanks for the detailed review.  Rev 4816.


> But see below for material for discussions and/or a followup patch.
>


> include/pc80/vga.h:#if (CONFIG_VGA == 1)
> include/pc80/vga.h:#endif /* (CONFIG_VGA == 1) */
>
> These should probably be changed to
>  #if CONFIG_VGA
>
Fixed

mainboard/intel/eagleheights/Options.lb:uses CONFIG_VGA
> mainboard/intel/eagleheights/Options.lb:#default CONFIG_VGA=0
>
> Unused here, so can be dropped from Options.lb?
>
Could be, but I'm trying not to touch newconfig.  It should be killed sooner
rather than later, and it won't help our comparisons if we modify it.


> These should probably be changed to
>  #if CONFIG_VGA
>
This variable should probably be something more specific, since only one
board uses it, but I don't know enough about it to rename it.


> config/Options.lb:define CONFIG_USE_WATCHDOG_ON_BOOT
>
>  The comment says "Use the watchdog on booting" which is not really
>  descriptive. What does "use" mean here? Disable it? We should fix
>  the comment I guess (and the resp. Kconfig help text).
>
I expect that it means enable it, but I don't know.  Again I was just trying
to match newconfig.

mainboard/lippert/spacerunner-lx/cache_as_ram_auto.c:#if
> !CONFIG_USE_WATCHDOG_ON_BOOT
> mainboard/lippert/roadrunner-lx/cache_as_ram_auto.c:#if
> !CONFIG_USE_WATCHDOG_ON_BOOT
>
> Not sure about these two. They use the variable but never add a "uses"
> line in Option.lb?
>
You don't have to have a uses line in Options.lb to reference a CONFIG
variable in the code.  If you try to set its value it will break, but
testing its value in C works fine (or generates a warning if it isn't always
exported.)

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Myles Watson]

Ping.  I think I've addressed all of Peter and Uwe's concerns.

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Uwe Hermann]

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.


Thanks, Uwe.
-- 
http://www.hermann-uwe.de  | http://www.randomprojects.org
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Myles Watson]

On Fri, Oct 23, 2009 at 4:56 PM, Uwe Hermann  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 

Thanks,
Myles
Index: svn/src/Kconfig
===
--- svn.orig/src/Kconfig
+++ svn/src/Kconfig
@@ -74,10 +74,6 @@ config CPU_ADDR_BITS
 	int
 	default 36
 
-config AGP_APERTURE_SIZE
-	hex
-	default 0x0
-
 config XIP_ROM_BASE
 	hex
 	default 0xfffe
@@ -216,6 +212,7 @@ config IOAPIC
 	default n
 
 config VIDEO_MB
+	default 0
 	int
 
 config USE_WATCHDOG_ON_BOOT
@@ -230,6 +227,7 @@ config VGA
 
 config GFXUMA
 	bool
+	default n
 	help
 	  Enable Unified Memory Architecture for graphics.
 
@@ -238,6 +236,26 @@ config GFXUMA
 #
 # endmenu
 
+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
+
+config HAVE_HIGH_TABLES
+	bool
+	default n
+	help
+	  This variable specifies whether a given northbridge has high table
+	  support.
+	  It is set in northbridge/*/Kconfig.
+	  Whether or not the high tables are actually written by coreboot is
+	  configurable by the user via WRITE_HIGH_TABLES.
+
 config HAVE_ACPI_TABLES
 	bool
 	help
@@ -262,14 +280,30 @@ config HAVE_PIRQ_TABLE
 	  Whether or not the PIRQ table is actually generated by coreboot
 	  is configurable by the user via GENERATE_PIRQ_TABLE.
 
-config HAVE_HIGH_TABLES
+#These Options are here to avoid "undefined" warnings.
+#The actual selection and help texts are in the following menu.
+
+config GENERATE_ACPI_TABLES
 	bool
+	default y if HAVE_ACPI_TABLES
+	default n
 
-menu "System tables"
+config GENERATE_MP_TABLE
+	bool
+	default y if HAVE_MP_TABLE
+	default n
 
-config HAVE_LOW_TABLES
+config GENERATE_PIRQ_TABLE
 	bool
-	default y
+	default y if HAVE_PIRQ_TABLE
+	default n
+
+config WRITE_HIGH_TABLES
+	bool
+	default y if HAVE_HIGH_TABLES
+	default n
+
+menu "System tables"
 
 config WRITE_HIGH_TABLES
 	bool "Write 'high' tables to avoid being overwritten in F segment"
@@ -409,3 +443,16 @@ endmenu
 config LIFT_BSP_APIC_ID
 	bool
 	default n
+
+# These probably belong somewhere else, but they are needed somewhere.
+config AP_CODE_IN_CAR
+	bool
+	default n
+
+config USE_INIT
+	bool
+	default n
+
+config ENABLE_APIC_EXT_ID
+	bool
+	default n
Index: svn/src/console/Kconfig
===
--- svn.orig/src/console/Kconfig
+++ svn/src/console/Kconfig
@@ -275,3 +275,14 @@ config DEFAULT_CONSOLE_LOGLEVEL
 
 endmenu
 
+config CONSOLE_BTEXT
+	bool
+	default n
+
+config CONSOLE_SROM
+	bool
+	default n
+
+config CONSOLE_LOGBUF
+	bool
+	default n
Index: svn/src/northbridge/amd/amdfam10/Kconfig
===
--- svn.orig/src/northbridge/amd/amdfam10/Kconfig
+++ svn/src/northbridge/amd/amdfam10/Kconfig
@@ -21,11 +21,35 @@ config NORTHBRIDGE_AMD_AMDFAM10
 	bool
 	select HAVE_HIGH_TABLES
 	select HYPERTRANSPORT_PLUGIN_SUPPORT
-	select HT3_SUPPORT
 
 config AGP_APERTURE_SIZE
 	hex
 	default 0x400
 	depends on NORTHBRIDGE_AMD_AMDFAM10
 
+config HT3_SUPPORT
+	bool
+	default y
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
+config AMDMCT
+	bool
+	default y
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
+config MEM_TRAIN_SEQ
+	int
+	default 0
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
+config HW_MEM_HOLE_SIZEK
+	hex
+	default 0x10
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
+config HW_MEM_HOLE_SIZE_AUTO_INC
+	bool
+	default n
+	depends on NORTHBRIDGE_AMD_AMDFAM10
+
 source src/northbridge/amd/amdfam10/root_complex/Kconfig
Index: svn/src/northbridge/amd/amdk8/Kconfig
===
--- svn.orig/src/northbridge/amd/amdk8/Kconfig
+++ svn/src/northbridge/amd/amdk8/Kconfig
@@ -32,4 +32,19 @@ config K8_HT_FREQ_1G_SUPPORT
 	default n
 	depends on NORTHBRIDGE_AMD_AMDK8
 
+config MEM_TRAIN_SEQ
+	int
+	default 0
+	depends on NORTHBRIDGE_AMD_AMDK8
+
+config HW_MEM_HOLE_SIZEK
+	hex
+	default 0x10
+	depends on NORTHBRIDGE_AMD_AMDK8
+
+config HW_MEM_HOLE_SIZE_AUTO_INC
+	bool
+	default n
+	depends on NORTHBRIDGE_AMD_AMDK8
+
 source src/northbridge/amd/amdk8/root_complex/Kconfig
Index: svn/src/southbridge/via/vt8237r/vt8237r.h
===
--- svn.orig/src/southbridge/via/vt8237r/vt8237r.h
+++ svn/src/southbridge/via/vt8237r/vt8237r.h
@@ -65,7 +65,7 @@
 #define I2C_TRANS_CMD			0x40
 #define CLOCK_SLAVE_ADDRESS		0x69
 
-#if DEBUG_SMBUS == 1
+#if defined(DEBUG_SMBUS) && DEBUG_SMBUS 

Re: [coreboot] [PATCH] More Kconfig changes

[Uwe Hermann]

On Fri, Oct 23, 2009 at 05:06:35PM -0600, Myles Watson wrote:
> On Fri, Oct 23, 2009 at 4:56 PM, Uwe Hermann  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 
 
Acked-by: Uwe Hermann 


> +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.


> +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?

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).

That's for another patch though.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.randomprojects.org
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Stefan Reinauer]

Uwe Hermann wrote:
> 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?
>
> Regardless of that, it seems like it should be a user-visible option in
> menuconfig
Shouldn't we clarify what it does _before_ deciding it should be user
visible? ;-)

Just an idea

Going through the AMD ram init code it loops like the code is used to
trigger certain code paths during ram init.

It does look somewhat random to me, but I guess it's due to board
requirements to choose one or the other option. In which case adding it
as a user visible option just creates another way to produce a
non-booting image without any real gain..

The Tyan s2912 uses 1, the s2912_fam10 uses 2.

Maybe we can even drop the option completely?

Going through more code I see a lot of this:

#if CONFIG_MEM_TRAIN_SEQ == 0
#define DQS_DELAY_COPY NODE_NUMS
#else
//  #define DQS_DELAY_COPY 1
#define DQS_DELAY_COPY NODE_NUMS
#endif

which is about as good as

#define DQS_DELAY_COPY NODE_NUMS

Anyone knows more details what the option is good for?

Stefan

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Myles Watson]

On Sat, Oct 24, 2009 at 5:11 PM, Uwe Hermann  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  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 
>
> Acked-by: Uwe Hermann 
>
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: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[Myles Watson]

> > 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?
> >
> > Regardless of that, it seems like it should be a user-visible option in
> > menuconfig
> Shouldn't we clarify what it does _before_ deciding it should be user
> visible? ;-)
Agreed.

> Maybe we can even drop the option completely?

> Anyone knows more details what the option is good for?

Unfortunately, I don't.  I was just trying to match newconfig.

Thanks,
Myles


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] More Kconfig changes

[ron minnich]

I don't think any of these options should be user configurable ...
it's all black magic and setting it wrong is going to ruin  your day.

ron

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Stefan Reinauer]

Myles Watson schrieb:

Fix AP_CODE_IN_CAR (only selected for two boards), STACK_SIZE, and HEAP_SIZE.

Signed-off-by: Myles Watson 

At this point my Tyan s2895 is happy with Kconfig.  Boot tested.

Thanks,
Myles
  

Looks (almost) good to me but I'd prefer someone else to check it, to.

One thing though: We're using lzma per default now if we're using 
compression. This means each board needs at _least_ a stack size of 0x8000.


Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely 
broken (and if they boot, they do by accident)


Stefan


--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Carl-Daniel Hailfinger]

On 10.10.2009 11:16, Stefan Reinauer wrote:
> Myles Watson schrieb:
>> Fix AP_CODE_IN_CAR (only selected for two boards), STACK_SIZE, and
>> HEAP_SIZE.
>>
>> Signed-off-by: Myles Watson 
>>
>> At this point my Tyan s2895 is happy with Kconfig.  Boot tested.
>>
>> Thanks,
>> Myles
>>   
> Looks (almost) good to me but I'd prefer someone else to check it, to.
>
> One thing though: We're using lzma per default now if we're using
> compression. This means each board needs at _least_ a stack size of
> 0x8000.
>
> Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely
> broken (and if they boot, they do by accident)

And the current infrastructure means we need that stack size per core if
any of the APs perform any decompression (which is a bad idea in itself).

Regards,
Carl-Daniel

-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> On 10.10.2009 11:16, Stefan Reinauer wrote:
>   
>> Myles Watson schrieb:
>> 
>>> Fix AP_CODE_IN_CAR (only selected for two boards), STACK_SIZE, and
>>> HEAP_SIZE.
>>>
>>> Signed-off-by: Myles Watson 
>>>
>>> At this point my Tyan s2895 is happy with Kconfig.  Boot tested.
>>>
>>> Thanks,
>>> Myles
>>>   
>>>   
>> Looks (almost) good to me but I'd prefer someone else to check it, to.
>>
>> One thing though: We're using lzma per default now if we're using
>> compression. This means each board needs at _least_ a stack size of
>> 0x8000.
>>
>> Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely
>> broken (and if they boot, they do by accident)
>> 
>
> And the current infrastructure means we need that stack size per core if
> any of the APs perform any decompression (which is a bad idea in itself).
>   

Which systems do that? we should fix them...


-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
  Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: i...@coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Carl-Daniel Hailfinger]

On 10.10.2009 20:08, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
>   
>> On 10.10.2009 11:16, Stefan Reinauer wrote:
>>   
>> 
>>> Myles Watson schrieb:
>>> 
>>>   
 Fix AP_CODE_IN_CAR (only selected for two boards), STACK_SIZE, and
 HEAP_SIZE.

 Signed-off-by: Myles Watson 

 At this point my Tyan s2895 is happy with Kconfig.  Boot tested.

 Thanks,
 Myles
   
   
 
>>> Looks (almost) good to me but I'd prefer someone else to check it, to.
>>>
>>> One thing though: We're using lzma per default now if we're using
>>> compression. This means each board needs at _least_ a stack size of
>>> 0x8000.
>>>
>>> Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely
>>> broken (and if they boot, they do by accident)
>>> 
>>>   
>> And the current infrastructure means we need that stack size per core if
>> any of the APs perform any decompression (which is a bad idea in itself).
>>   
>> 
>
> Which systems do that? we should fix them...
>   

AFAIK the Fam10 targets Zheng is having problems with.

Regards,
Carl-Daniel

-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

> Myles Watson schrieb:
> > Fix AP_CODE_IN_CAR (only selected for two boards), STACK_SIZE, and
> HEAP_SIZE.
> >
> > Signed-off-by: Myles Watson 
> >
> > At this point my Tyan s2895 is happy with Kconfig.  Boot tested.
> >
> > Thanks,
> > Myles
> >
> Looks (almost) good to me but I'd prefer someone else to check it, to.
I was figuring that Patrick's scripts will check this stuff and we'll get
closer and closer to newconfig.  There are a few values things that I'd like
to not match newconfig, though.  For example, fam10 and k8 had different
cache as RAM settings in newconfig, and they could share them.

> One thing though: We're using lzma per default now if we're using
> compression. This means each board needs at _least_ a stack size of
> 0x8000.
Why does LZMA use so much memory from the stack?  Couldn't we convert it to
use heap so that it is easier to tell when you run out?  I guess that would
make it dependent on a malloc call?

> Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely
> broken (and if they boot, they do by accident)

So since it's broken with Kconfig and newconfig, how can we decide what the
correct stack size should be?

What's the downside of a large stack?
What breakage should occur, heap corruption?
Should we check before LZMA how much stack is left?

Thanks,
Myles



-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Carl-Daniel Hailfinger]

On 10.10.2009 23:55, Myles Watson wrote:
>> Myles Watson schrieb:
>> 
>>> Fix AP_CODE_IN_CAR (only selected for two boards), STACK_SIZE, and
>>>   
>> HEAP_SIZE.
>> 
>>
>> One thing though: We're using lzma per default now if we're using
>> compression. This means each board needs at _least_ a stack size of
>> 0x8000.
> Why does LZMA use so much memory from the stack?  Couldn't we convert it to
> use heap so that it is easier to tell when you run out?  I guess that would
> make it dependent on a malloc call?
>   

Yes, the malloc dependency is what originally caused me to use the stack
instead.


>> Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely
>> broken (and if they boot, they do by accident)
>> 
>
> So since it's broken with Kconfig and newconfig, how can we decide what the
> correct stack size should be?
>
> What's the downside of a large stack?
>   

If you make the stack too large and you have multiple cores in CAR at
the same time, the CAR size is too small for all stacks.

> What breakage should occur, heap corruption?
> Should we check before LZMA how much stack is left?
>   

The best choice would be to make sure no AP ever uses LZMA.
Let me explain. If one AP uses LZMA, it's very likely due to
decompressing some CBFS member. If one AP does that, it is very likely
all of them are doing it, probably even at the same time (at least we
had that problem in the past). LZMA decompression uses the destination
buffer as scratch pad which means if you are decompressing the same file
to the same destination on different cores, you are likely to get
garbage there in the meantime or even at the end. Plus, decompressing
one file once per AP is totally wasteful. Nobody wants that.
Two ways to solve this:
1. Have the first AP decompress the CBFS member it wants to run and
block all other APs until decompression is complete (but you still need
a big stack for that first AP).
2. Have the BSP decompress the CBFS member the APs want to run, then
start the APs. Big benefit here is you can avoid locking and the stack
of APs can stay small.


Regards,
Carl-Daniel

-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

On Sat, Oct 10, 2009 at 4:18 PM, Carl-Daniel Hailfinger
 wrote:
> On 10.10.2009 23:55, Myles Watson wrote:
>>> One thing though: We're using lzma per default now if we're using
>>> compression. This means each board needs at _least_ a stack size of
>>> 0x8000.
>> Why does LZMA use so much memory from the stack?  Couldn't we convert it to
>> use heap so that it is easier to tell when you run out?  I guess that would
>> make it dependent on a malloc call?
>>
>
> Yes, the malloc dependency is what originally caused me to use the stack
> instead.
But we could check the position on the stack compared to the top of
the stack before running LZMA, right?

>>> Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely
>>> broken (and if they boot, they do by accident)
>>>
>>
>> So since it's broken with Kconfig and newconfig, how can we decide what the
>> correct stack size should be?
>>
>> What's the downside of a large stack?
>>
>
> If you make the stack too large and you have multiple cores in CAR at
> the same time, the CAR size is too small for all stacks.
It seems like the safest way would be to serialize AP startup and have
(at most) two stacks.

>> What breakage should occur, heap corruption?
>> Should we check before LZMA how much stack is left?
>>
>
> The best choice would be to make sure no AP ever uses LZMA.
> Let me explain. If one AP uses LZMA, it's very likely due to
> decompressing some CBFS member. If one AP does that, it is very likely
> all of them are doing it, probably even at the same time (at least we
> had that problem in the past). LZMA decompression uses the destination
> buffer as scratch pad which means if you are decompressing the same file
> to the same destination on different cores, you are likely to get
> garbage there in the meantime or even at the end. Plus, decompressing
> one file once per AP is totally wasteful. Nobody wants that.
> Two ways to solve this:
> 1. Have the first AP decompress the CBFS member it wants to run and
> block all other APs until decompression is complete (but you still need
> a big stack for that first AP).
> 2. Have the BSP decompress the CBFS member the APs want to run, then
> start the APs. Big benefit here is you can avoid locking and the stack
> of APs can stay small.
I thought the problem was that this was before RAM is available, so
the AP was decompressing into its cache.  You can't have the BSP do
that for an AP, right?

Thanks,
Myles

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Carl-Daniel Hailfinger]

On 11.10.2009 00:28, Myles Watson wrote:
> On Sat, Oct 10, 2009 at 4:18 PM, Carl-Daniel Hailfinger
>  wrote:
>   
>> On 10.10.2009 23:55, Myles Watson wrote:
>> 
 One thing though: We're using lzma per default now if we're using
 compression. This means each board needs at _least_ a stack size of
 0x8000.
 
>>> Why does LZMA use so much memory from the stack?  Couldn't we convert it to
>>> use heap so that it is easier to tell when you run out?  I guess that would
>>> make it dependent on a malloc call?
>>>
>>>   
>> Yes, the malloc dependency is what originally caused me to use the stack
>> instead.
>> 
> But we could check the position on the stack compared to the top of
> the stack before running LZMA, right?
>   

That's hideously complicated. On AMD Fam10, each AP gets its own
mini-stack at another location. The code for a stack checker is in v3
and even for the no-SMP case it is really fragile. Add multiple stack
sizes and multiple stack locations to it and the code will have to be
marked "Do not touch even if you think you understand it".
But yes, it can be done.


 Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely
 broken (and if they boot, they do by accident)

 
>>> So since it's broken with Kconfig and newconfig, how can we decide what the
>>> correct stack size should be?
>>>
>>> What's the downside of a large stack?
>>>
>>>   
>> If you make the stack too large and you have multiple cores in CAR at
>> the same time, the CAR size is too small for all stacks.
>> 
> It seems like the safest way would be to serialize AP startup and have
> (at most) two stacks.
>   

That's a good idea as well, but I'm not sure our current infrastructure
can handle that. And how would the second and subsequent APs realize
that earlier incarnations already decompressed the CBFS member? All
those ROM accesses are wasting lots of time, so we only want to do them
once.


>>> What breakage should occur, heap corruption?
>>> Should we check before LZMA how much stack is left?
>>>
>>>   
>> The best choice would be to make sure no AP ever uses LZMA.
>> Let me explain. If one AP uses LZMA, it's very likely due to
>> decompressing some CBFS member. If one AP does that, it is very likely
>> all of them are doing it, probably even at the same time (at least we
>> had that problem in the past). LZMA decompression uses the destination
>> buffer as scratch pad which means if you are decompressing the same file
>> to the same destination on different cores, you are likely to get
>> garbage there in the meantime or even at the end. Plus, decompressing
>> one file once per AP is totally wasteful. Nobody wants that.
>> Two ways to solve this:
>> 1. Have the first AP decompress the CBFS member it wants to run and
>> block all other APs until decompression is complete (but you still need
>> a big stack for that first AP).
>> 2. Have the BSP decompress the CBFS member the APs want to run, then
>> start the APs. Big benefit here is you can avoid locking and the stack
>> of APs can stay small.
>> 
> I thought the problem was that this was before RAM is available, so
> the AP was decompressing into its cache.  You can't have the BSP do
> that for an AP, right?
>   

On AMD Fam10, the BKDG says that any CAR area can be either executable
or writable (mutually exclusive). You can decide which one you want on a
4k granularity with different MTRR types.
I do not know of any place where we decompress code into the CAR area
and I'd recommend against such stuff (mainly for non-technical reasons
you don't want to know).

Regards,
Carl-Daniel

-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> 1. Have the first AP decompress the CBFS member
> 2. Have the BSP decompress the CBFS member the APs want to run

3. Parallelize decompression


//Peter hides

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

On Sat, Oct 10, 2009 at 5:02 PM, Carl-Daniel Hailfinger <
c-d.hailfinger.devel.2...@gmx.net> wrote:

> On 11.10.2009 00:28, Myles Watson wrote:
> > On Sat, Oct 10, 2009 at 4:18 PM, Carl-Daniel Hailfinger
> >  wrote:
> >
> >> On 10.10.2009 23:55, Myles Watson wrote:
> >>
>  One thing though: We're using lzma per default now if we're using
>  compression. This means each board needs at _least_ a stack size of
>  0x8000.
> 
> >> Yes, the malloc dependency is what originally caused me to use the stack
> >> instead.
>
Maybe we ought to revisit it, then, since malloc already checks if it is
running out of memory.


> > But we could check the position on the stack compared to the top of
> > the stack before running LZMA, right?
>
> That's hideously complicated. On AMD Fam10, each AP gets its own
> mini-stack at another location. The code for a stack checker is in v3
> and even for the no-SMP case it is really fragile. Add multiple stack
> sizes and multiple stack locations to it and the code will have to be
> marked "Do not touch even if you think you understand it".
> But yes, it can be done.
>
I just meant compare against the top of all stacks (or the bottom of the
heap.)  Any checking is better than none.


>  Those boards with STACK_SIZE being 0x2000 or 0x8000 are definitely
>  broken (and if they boot, they do by accident)
> >>> So since it's broken with Kconfig and newconfig, how can we decide what
> the
> >>> correct stack size should be?
>
Ping.


> > It seems like the safest way would be to serialize AP startup and have
> > (at most) two stacks.
> >
>
> That's a good idea as well, but I'm not sure our current infrastructure
> can handle that. And how would the second and subsequent APs realize
> that earlier incarnations already decompressed the CBFS member? All
> those ROM accesses are wasting lots of time, so we only want to do them
> once.

All I'm looking for is the shortest path to "not-broken".  I'm open to
suggestions.

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

On Mon, Oct 12, 2009 at 8:36 PM, Myles Watson  wrote:
>
>
> On Sat, Oct 10, 2009 at 5:02 PM, Carl-Daniel Hailfinger
>  wrote:
>>
>> On 11.10.2009 00:28, Myles Watson wrote:
>> > On Sat, Oct 10, 2009 at 4:18 PM, Carl-Daniel Hailfinger
>> >  wrote:
>> >
>> >> On 10.10.2009 23:55, Myles Watson wrote:
>> >>
>>  One thing though: We're using lzma per default now if we're using
>>  compression. This means each board needs at _least_ a stack size of
>>  0x8000.
I think I need some clarification.  When we're talking about
STACK_SIZE and HEAP_SIZE it only refers to coreboot_ram, right?  So
unless you use a compressed payload, LZMA doesn't come into play.
When you're using cache_as_ram, I don't see that we have a way of
dividing the cache between stack and heap.  Am I missing something?

Thanks,
Myles

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[ron minnich]

On Sat, Oct 10, 2009 at 3:18 PM, Carl-Daniel Hailfinger
 wrote:

> If you make the stack too large and you have multiple cores in CAR at
> the same time, the CAR size is too small for all stacks.

Let's please go over this again. Last time I checked, this was the rule:

cores 1 and up don't run in CAR.

Is that true or not. If that is true, then we don't need to worry
about this problem. Cores 1 and up don't
run in CAR. They are started up by core 0 using DRAM.


> The best choice would be to make sure no AP ever uses LZMA.

let's be clear here. When you say AP, do you mean "core > 0" or do mean an AP?

> 2. Have the BSP decompress the CBFS member the APs want to run, then
> start the APs. Big benefit here is you can avoid locking and the stack
> of APs can stay small.

Yes. And it's doable. And, I thought, at least when I worked on the v3
stuff, I did that. Did I not?

ron

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[ron minnich]

On Mon, Oct 12, 2009 at 7:36 PM, Myles Watson  wrote:

> All I'm looking for is the shortest path to "not-broken".  I'm open to
> suggestions.

So I've had this explained to me several times, let's try again.

In fact it's in the v2 code. It's just that the v2 code is so hard to read ...

1. BSP core 0 starts up in CAR. BSP core 0 sets up DRAM programming
for ALL sockets. BPS gets CBFS
   files in RAM. BSP zeros memory attached to BSP.
2. BSP uses IPIs to set up all AP stacks and EIP. AP core 0s start up.
They do what initialization they need (e.g.
   zero their own DRAM). They then stop.
2a. BSP waits for all AP core 0 to either stop or error out.
3. BPS starts up core>0 and sends IPIs to get APs to start up core>0.
4. All core 0s wait for all core>0 to stop or error out.
5. BSP waits for all AP core0 to stop again.
6. BSP continues to boot.

Is there a problem with this sequence?

thanks

ron

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

> -Original Message-
> From: ron minnich [mailto:rminn...@gmail.com]
> Sent: Monday, October 12, 2009 9:37 PM
> To: Myles Watson
> Cc: Carl-Daniel Hailfinger; Stefan Reinauer; coreboot
> Subject: Re: [coreboot] [PATCH] more Kconfig default fixes
> 
> On Mon, Oct 12, 2009 at 7:36 PM, Myles Watson  wrote:
> 
> > All I'm looking for is the shortest path to "not-broken".  I'm open to
> > suggestions.
> 
> So I've had this explained to me several times, let's try again.
> 
> In fact it's in the v2 code. It's just that the v2 code is so hard to read
> ...
> 
> 1. BSP core 0 starts up in CAR. 
All other cores start up and put themselves to sleep ASAP.

> BSP core 0 sets up DRAM programming
> for ALL sockets. BPS gets CBFS
>files in RAM. BSP zeros memory attached to BSP.
> 2. BSP uses IPIs to set up all AP stacks and EIP. AP core 0s start up.
> They do what initialization they need (e.g.
>zero their own DRAM). They then stop.
> 2a. BSP waits for all AP core 0 to either stop or error out.
> 3. BPS starts up core>0 and sends IPIs to get APs to start up core>0.
> 4. All core 0s wait for all core>0 to stop or error out.
> 5. BSP waits for all AP core0 to stop again.
> 6. BSP continues to boot.
> 
> Is there a problem with this sequence?

I don't think so. 

The reason we're back on this topic is because we're having trouble nailing
down the correct values for CONFIG_STACK and CONFIG_HEAP.  I just noticed
that they don't actually matter in CAR, so we have RAM by the time we even
check those values.

Thanks,
Myles


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[ron minnich]

On Mon, Oct 12, 2009 at 8:45 PM, Myles Watson  wrote:

> The reason we're back on this topic is because we're having trouble nailing
> down the correct values for CONFIG_STACK and CONFIG_HEAP.  I just noticed
> that they don't actually matter in CAR, so we have RAM by the time we even
> check those values.

understood. OK, let's move along :-)

ron

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

On Mon, Oct 12, 2009 at 9:47 PM, ron minnich  wrote:

> On Mon, Oct 12, 2009 at 8:45 PM, Myles Watson  wrote:
>
> > The reason we're back on this topic is because we're having trouble
> nailing
> > down the correct values for CONFIG_STACK and CONFIG_HEAP.  I just noticed
> > that they don't actually matter in CAR, so we have RAM by the time we
> even
> > check those values.
>
> understood. OK, let's move along :-)
>

So I guess the question is how should we make sure the stack and heap are
sized correctly.  Using malloc to allocate the memory for lzma makes sense,
but it is used in CAR too, so that complicates our decision.

I guess in CAR we don't have much of a heap to run into, so everything is on
the stack?

Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Stefan Reinauer]

Myles Watson schrieb:
 


> But we could check the position on the stack compared to the top of
> the stack before running LZMA, right?

That's hideously complicated. On AMD Fam10, each AP gets its own
mini-stack at another location. The code for a stack checker is in v3
and even for the no-SMP case it is really fragile. Add multiple stack
sizes and multiple stack locations to it and the code will have to be
marked "Do not touch even if you think you understand it".
But yes, it can be done.

I just meant compare against the top of all stacks (or the bottom of 
the heap.)  Any checking is better than none.
 


 Those boards with STACK_SIZE being 0x2000 or 0x8000 are
definitely
 broken (and if they boot, they do by accident)
>>> So since it's broken with Kconfig and newconfig, how can we
decide what the
>>> correct stack size should be?

Ping.

0x8000 is the minimum for all boards. I think it should be the default.



--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Stefan Reinauer]

Myles Watson schrieb:



On Mon, Oct 12, 2009 at 9:47 PM, ron minnich > wrote:


On Mon, Oct 12, 2009 at 8:45 PM, Myles Watson mailto:myle...@gmail.com>> wrote:

> The reason we're back on this topic is because we're having
trouble nailing
> down the correct values for CONFIG_STACK and CONFIG_HEAP.  I
just noticed
> that they don't actually matter in CAR, so we have RAM by the
time we even
> check those values.

understood. OK, let's move along :-)


So I guess the question is how should we make sure the stack and heap 
are sized correctly.  Using malloc to allocate the memory for lzma 
makes sense, but it is used in CAR too, so that complicates our decision.
The plan might also be to drop malloc completely and replace it with 
more distinct memory areas. malloc has a strong touch of unpredictability.


I guess in CAR we don't have much of a heap to run into, so everything 
is on the stack?

Yes. (More or less)

--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[ron minnich]

On Mon, Oct 12, 2009 at 8:51 PM, Myles Watson  wrote:

> So I guess the question is how should we make sure the stack and heap are
> sized correctly.  Using malloc to allocate the memory for lzma makes sense,
> but it is used in CAR too, so that complicates our decision.

lzma decompressor gets a void * from the caller. Caller, if CAR, uses
on-stack pointer. RAM code can, if desired,
use malloc'ed memory?

>
> I guess in CAR we don't have much of a heap to run into, so everything is on
> the stack?

yes.

ron

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

On Tue, Oct 13, 2009 at 8:59 AM, ron minnich  wrote:

> On Mon, Oct 12, 2009 at 8:51 PM, Myles Watson  wrote:
>
> > So I guess the question is how should we make sure the stack and heap are
> > sized correctly.  Using malloc to allocate the memory for lzma makes
> sense,
> > but it is used in CAR too, so that complicates our decision.
>
> lzma decompressor gets a void * from the caller. Caller, if CAR, uses
> on-stack pointer. RAM code can, if desired,
> use malloc'ed memory?
>
Not for the scratchpad.  It's allocated on the stack of the ulzma function.

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[ron minnich]

On Tue, Oct 13, 2009 at 8:40 AM, Myles Watson  wrote:
>
>
> On Tue, Oct 13, 2009 at 8:59 AM, ron minnich  wrote:
>>
>> On Mon, Oct 12, 2009 at 8:51 PM, Myles Watson  wrote:
>>
>> > So I guess the question is how should we make sure the stack and heap
>> > are
>> > sized correctly.  Using malloc to allocate the memory for lzma makes
>> > sense,
>> > but it is used in CAR too, so that complicates our decision.
>>
>> lzma decompressor gets a void * from the caller. Caller, if CAR, uses
>> on-stack pointer. RAM code can, if desired,
>> use malloc'ed memory?
>
> Not for the scratchpad.  It's allocated on the stack of the ulzma function.

yes, but I always felt that was fixable.

ron

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

> >> lzma decompressor gets a void * from the caller. Caller, if CAR, uses
> >> on-stack pointer. RAM code can, if desired,
> >> use malloc'ed memory?
> >
> > Not for the scratchpad.  It's allocated on the stack of the ulzma
> function.
> 
> yes, but I always felt that was fixable.
Sure.  We could pass it a scratchpad pointer too.

Thanks,
Myles


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Stefan Reinauer]

ron minnich wrote:
> On Tue, Oct 13, 2009 at 8:40 AM, Myles Watson  wrote:
>   
>> On Tue, Oct 13, 2009 at 8:59 AM, ron minnich  wrote:
>> 
>>> On Mon, Oct 12, 2009 at 8:51 PM, Myles Watson  wrote:
>>>
>>>   
 So I guess the question is how should we make sure the stack and heap
 are
 sized correctly.  Using malloc to allocate the memory for lzma makes
 sense,
 but it is used in CAR too, so that complicates our decision.
 
>>> lzma decompressor gets a void * from the caller. Caller, if CAR, uses
>>> on-stack pointer. RAM code can, if desired,
>>> use malloc'ed memory?
>>>   
>> Not for the scratchpad.  It's allocated on the stack of the ulzma function.
>> 
>
> yes, but I always felt that was fixable.

By making two copies of the code that behave slightly different?

There's no benefit of using heap over using stack, so why bother?


Stefan

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[ron minnich]

On Tue, Oct 13, 2009 at 9:26 AM, Stefan Reinauer  wrote:

> There's no benefit of using heap over using stack, so why bother?

beats me. I'm responding to something that people seem to feel is an issue.

ron

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

On Tue, Oct 13, 2009 at 11:08 AM, ron minnich  wrote:
> On Tue, Oct 13, 2009 at 9:26 AM, Stefan Reinauer  
> wrote:
>
>> There's no benefit of using heap over using stack, so why bother?
>
> beats me. I'm responding to something that people seem to feel is an issue.
:)

The issue is that it is forcing every platform to increase its stack
size.  The old default was 8K.  The new default is 32K.  I just wanted
to make sure that it was clear why we're choosing 32K and that it
wouldn't cause a problem for any boards to have their stack increased
by such a large amount.

This is the same patch with 32K as the default stack size.

Signed-off-by: Myles Watson 

Thanks,
Myles
Index: svn/src/mainboard/amd/dbm690t/Kconfig
===
--- svn.orig/src/mainboard/amd/dbm690t/Kconfig
+++ svn/src/mainboard/amd/dbm690t/Kconfig
@@ -13,7 +13,6 @@ config BOARD_AMD_DBM690T
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select HAVE_ACPI_TABLES
Index: svn/src/mainboard/amd/pistachio/Kconfig
===
--- svn.orig/src/mainboard/amd/pistachio/Kconfig
+++ svn/src/mainboard/amd/pistachio/Kconfig
@@ -12,7 +12,6 @@ config BOARD_AMD_PISTACHIO
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select HAVE_ACPI_TABLES
Index: svn/src/mainboard/arima/hdama/Kconfig
===
--- svn.orig/src/mainboard/arima/hdama/Kconfig
+++ svn/src/mainboard/arima/hdama/Kconfig
@@ -13,7 +13,6 @@ config BOARD_ARIMA_HDAMA
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 
Index: svn/src/mainboard/gigabyte/ga_2761gxdk/Kconfig
===
--- svn.orig/src/mainboard/gigabyte/ga_2761gxdk/Kconfig
+++ svn/src/mainboard/gigabyte/ga_2761gxdk/Kconfig
@@ -97,11 +97,6 @@ config MAX_PHYSICAL_CPUS
 	default 1
 	depends on BOARD_GIGABYTE_GA_2761GXDK
 
-config AP_CODE_IN_CAR
-	bool	
-	default n
-	depends on BOARD_GIGABYTE_GA_2761GXDK
-
 config HW_MEM_HOLE_SIZE_AUTO_INC
 	bool	
 	default n
Index: svn/src/mainboard/gigabyte/m57sli/Kconfig
===
--- svn.orig/src/mainboard/gigabyte/m57sli/Kconfig
+++ svn/src/mainboard/gigabyte/m57sli/Kconfig
@@ -99,11 +99,6 @@ config MAX_PHYSICAL_CPUS
 	default 1
 	depends on BOARD_GIGABYTE_M57SLI
 
-config AP_CODE_IN_CAR
-	bool	
-	default n
-	depends on BOARD_GIGABYTE_M57SLI
-
 config HW_MEM_HOLE_SIZE_AUTO_INC
 	bool	
 	default n
Index: svn/src/mainboard/ibm/e325/Kconfig
===
--- svn.orig/src/mainboard/ibm/e325/Kconfig
+++ svn/src/mainboard/ibm/e325/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IBM_E325
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 
Index: svn/src/mainboard/ibm/e326/Kconfig
===
--- svn.orig/src/mainboard/ibm/e326/Kconfig
+++ svn/src/mainboard/ibm/e326/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IBM_E326
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 
Index: svn/src/mainboard/iwill/dk8_htx/Kconfig
===
--- svn.orig/src/mainboard/iwill/dk8_htx/Kconfig
+++ svn/src/mainboard/iwill/dk8_htx/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IWILL_DK8_HTX
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select HAVE_ACPI_TABLES
Index: svn/src/mainboard/iwill/dk8s2/Kconfig
===
--- svn.orig/src/mainboard/iwill/dk8s2/Kconfig
+++ svn/src/mainboard/iwill/dk8s2/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IWILL_DK8S2
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 	select ATI_RAGE_XL
Index: svn/src/mainboard/iwill/dk8x/Kconfig
===
--- svn.orig/src/mainboard/iwill/dk8x/Kconfig
+++ svn/src/mainboard/iwill/dk8x/Kconfig
@@ -13,7 +13,6 @@ config BOARD_IWILL_DK8X
 	select USE_DCACHE_RAM
 	select HAVE_HARD_RESET
 	select IOAPIC
-	select AP_CODE_IN_CAR
 	select SB_HT_CHAIN_UNITID_OFFSET_ONLY
 	select WAIT_BEFORE_CPUS_INIT
 
Index: svn/src/mainboard/msi/ms7135/Kconfig
=

Re: [coreboot] [PATCH] more Kconfig default fixes

[Stefan Reinauer]

ron minnich wrote:
> On Mon, Oct 12, 2009 at 8:51 PM, Myles Watson  wrote:
>
>   
>> So I guess the question is how should we make sure the stack and heap are
>> sized correctly.  Using malloc to allocate the memory for lzma makes sense,
>> but it is used in CAR too, so that complicates our decision.
>> 
>
> lzma decompressor gets a void * from the caller. Caller, if CAR, uses
> on-stack pointer. RAM code can, if desired,
> use malloc'ed memory?
>
>   
We never call lzma while in CAR. Now that would be kind of silly, would it?


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Carl-Daniel Hailfinger]

On 15.10.2009 13:04, Stefan Reinauer wrote:
> ron minnich wrote:
>   
>> On Mon, Oct 12, 2009 at 8:51 PM, Myles Watson  wrote:
>>
>>   
>> 
>>> So I guess the question is how should we make sure the stack and heap are
>>> sized correctly.  Using malloc to allocate the memory for lzma makes sense,
>>> but it is used in CAR too, so that complicates our decision.
>>> 
>>>   
>> lzma decompressor gets a void * from the caller. Caller, if CAR, uses
>> on-stack pointer. RAM code can, if desired,
>> use malloc'ed memory?
>>
>>   
>> 
> We never call lzma while in CAR. Now that would be kind of silly, would it?
>   

Well, originally ulmza() was designed to be runnable in CAR on the OLPC.
That's why I picked a scratch pad size which would allow pretty good
compression and still fit well into the stack we had during CAR on these
boards. Part of the motivation may have been a misunderstanding, this
was one of my first coreboot patches (or even the very first one).

I don't care where ulzma places its scratch space as long as it can get
enough of it. If someone wants to use malloc() instead, check the
variable mallocneeds which has the exact allocation size needed (that
size depends on the parameters picked during compression).

Regards,
Carl-Daniel

-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> On 15.10.2009 13:04, Stefan Reinauer wrote:
>   
>> ron minnich wrote:
>>   
>> 
>>> On Mon, Oct 12, 2009 at 8:51 PM, Myles Watson  wrote:
>>>
>>>   
>>> 
>>>   
 So I guess the question is how should we make sure the stack and heap are
 sized correctly.  Using malloc to allocate the memory for lzma makes sense,
 but it is used in CAR too, so that complicates our decision.
 
   
 
>>> lzma decompressor gets a void * from the caller. Caller, if CAR, uses
>>> on-stack pointer. RAM code can, if desired,
>>> use malloc'ed memory?
>>>
>>>   
>>> 
>>>   
>> We never call lzma while in CAR. Now that would be kind of silly, would it?
>>   
>> 
>
> Well, originally ulmza() was designed to be runnable in CAR on the OLPC.
>   

What for? Decompressing to cache? This sounds a bit odd, with a 16kB
scratchpad, and only 128KB cache.

Stefan

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Carl-Daniel Hailfinger]

On 15.10.2009 14:28, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
>   
>> On 15.10.2009 13:04, Stefan Reinauer wrote:
>>   
>> 
>>> ron minnich wrote:
>>>   
>>> 
>>>   
 On Mon, Oct 12, 2009 at 8:51 PM, Myles Watson  wrote:

   
 
   
 
> So I guess the question is how should we make sure the stack and heap are
> sized correctly.  Using malloc to allocate the memory for lzma makes 
> sense,
> but it is used in CAR too, so that complicates our decision.
> 
>   
> 
>   
 lzma decompressor gets a void * from the caller. Caller, if CAR, uses
 on-stack pointer. RAM code can, if desired,
 use malloc'ed memory?

   
 
   
 
>>> We never call lzma while in CAR. Now that would be kind of silly, would it?
>>>   
>>> 
>>>   
>> Well, originally ulmza() was designed to be runnable in CAR on the OLPC.
>>   
>> 
>
> What for? Decompressing to cache? This sounds a bit odd, with a 16kB
> scratchpad, and only 128KB cache.
>   

I didn't say it was a good idea. I had not understood coreboot design
well enough to know that decompression would be run after CAR and
thought coreboot was running decompression to RAM while the stack still
lived in CAR.

Regards,
Carl-Daniel

-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

> This is the same patch with 32K as the default stack size.
> 
> Signed-off-by: Myles Watson 
Ping.

Thanks,
Myles


-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[ron minnich]

Acked-by: Ronald G. Minnich 

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] [PATCH] more Kconfig default fixes

[Myles Watson]

On Fri, Oct 16, 2009 at 12:53 PM, ron minnich  wrote:

> Acked-by: Ronald G. Minnich 
>
Rev 4793.

Thanks,
Myles
-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot