[coreboot] patch: beginnings of support for geode gx2, v2, and Kconfig

ron minnich rminnich at gmail.com
Sat Aug 29 04:45:24 CEST 2009


First off, thanks for the extra review.


On Fri, Aug 28, 2009 at 10:00 AM, Uwe Hermann<uwe at hermann-uwe.de> wrote:
> On Thu, Aug 27, 2009 at 10:40:04PM -0700, ron minnich wrote:
>> Index: src/northbridge/amd/gx2/Kconfig
>> ===================================================================
>> --- src/northbridge/amd/gx2/Kconfig   (revision 0)
>> +++ src/northbridge/amd/gx2/Kconfig   (revision 0)
>> @@ -0,0 +1,27 @@
> [...]
>> +config NORTHBRIDGE_AMD_GX2
>> +     bool
>> +     default n
>> +
>> +config HAVE_HIGH_TABLES
>> +     bool
>> +     default y
>
> What's the plan for HAVE_HIGH_TABLES? Some NB/SB files define it to y,
> but the default is already y. Aren't all chipsets/boards supposed to
> use HAVE_HIGH_TABLES=y already? Or are some not fully supported, yet?

This was crud that slipped in. I removed it in 4616.
>
>
>> Index: src/northbridge/amd/gx2/northbridge.c
>> ===================================================================
>> --- src/northbridge/amd/gx2/northbridge.c     (revision 4596)
>> +++ src/northbridge/amd/gx2/northbridge.c     (working copy)
>> @@ -486,7 +486,6 @@
>>       printk_debug("gx2 north: enable_dev\n");
>>       void northbridgeinit(void);
>>       void chipsetinit(struct northbridge_amd_gx2_config *nb);
>> -     void setup_realmode_idt(void);
>>       void do_vsmbios(void);
>>          /* Set the operations if it is a special bus type */
>>          if (dev->path.type == DEVICE_PATH_PCI_DOMAIN) {
>> @@ -499,8 +498,6 @@
>>               cpubug();
>>               chipsetinit(nb);
>>               setup_gx2();
>> -             /* do this here for now -- this chip really breaks our device model */
>> -             setup_realmode_idt();
>>               do_vsmbios();
>>               graphics_init();
>>               dev->ops = &pci_domain_ops;
>
> Why this? Intentional?

This has been in there forever, and I'm not yet willing to remove it.

>
>
>> Index: src/cpu/amd/model_gx2/Kconfig
>> ===================================================================
>> --- src/cpu/amd/model_gx2/Kconfig     (revision 0)
>> +++ src/cpu/amd/model_gx2/Kconfig     (revision 0)
>> @@ -0,0 +1,3 @@
>> +config CPU_AMD_GX2
>> +     bool
>> +     default false
>
> Please use n (not false) for consistency, I think most of the kconfigs
> use that, and also the Linux kernel IIRC.

Thanks for catching this, fixed in 4616.

ron




More information about the coreboot mailing list