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