Am 27.04.2009 23:37, schrieb Peter Stuge:
Patrick Georgi wrote:
attached patch adds high table support to via vt8454c and might be
a good template for other boards and chipsets.
I don't think it is.
You are right.
I've figured as much yesterday (after this mail, obviously), after
talking with Stefan about unifying the part in mainboard.c
+++ src/mainboard/via/vt8454c/mainboard.c (Arbeitskopie)
@@ -20,8 +20,23 @@
*/
#include<device/device.h>
+#include<boot/tables.h>
+#include<console/console.h>
#include "chip.h"
+/* in arch/i386/boot/tables.c */
+extern uint64_t high_tables_base, high_tables_size;
+
+int add_mainboard_resources(struct lb_memory *mem)
+{
+#if HAVE_HIGH_TABLES == 1
+ printk_debug("Adding high table area\n");
+ lb_add_memory_range(mem, LB_MEM_TABLE,
+ high_tables_base, high_tables_size);
+#endif
+ return 0;
+}
+
This code doesn't seem mainboard specific so I think it must not be
in mainboard/.
add_mainboard_resources is necessary for some boards (eg. kontron), but
this generic code could be added to the caller of
add_mainboard_resources (wrapped in HAVE_HIGH_TABLES, of course).
That way, boards that really need it (for other things) can use this
function, while others don't have to do anything.
I'll prepare a patch for that.
+++ src/northbridge/via/cx700/northbridge.c (Arbeitskopie)
@@ -87,6 +87,12 @@
return tolm;
}
+#if HAVE_HIGH_TABLES==1
+/* maximum size of high tables in KB */
+#define HIGH_TABLES_SIZE 64
+extern uint64_t high_tables_base, high_tables_size;
+#endif
+
static void pci_domain_set_resources(device_t dev)>> {
device_t mc_dev;
@@ -117,6 +123,12 @@
else
tomk = (((rambits<< 6) - (4<< reg) - 1) * 1024);
+#if HAVE_HIGH_TABLES == 1
+ high_tables_base = (tomk - HIGH_TABLES_SIZE) * 1024;
+ high_tables_size = HIGH_TABLES_SIZE* 1024;
+ printk_debug("tom: %lx, high_tables_base: %llx, high_tables_size:
%llx\n", tomk*1024, high_tables_base, high_tables_size);
+#endif
+
Likewise, this does not seem northbridge specific.
It needs knowledge about the top of memory. I don't think we can push
this elsewhere without merely selecting a different global variable (eg.
one that is exported by the northbridge).
And in this case, we're better off with the current solution, as it
doesn't force changes for all northbridges at once.
What is not northbridge specific is the HIGH_TABLES_SIZE. It also
depends on the amount of ACPI code etc, which is mainboard specific.
I'll work on the other part first, to have more time to think over this
and discuss it.
I'm open to better solutions. What I'd like to see eventually is that we
can drop HAVE_HIGH_TABLES (and HAVE_LOW_TABLES, really) because both are
set for all boards. Same for CONFIG_CBFS, btw. Having too many options
makes our support matrix explode.
Patrick
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
