[coreboot] [PATCH] Add support for high tables to via vt8454c

[Patrick Georgi]

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