First off, thanks for the extra review.
On Fri, Aug 28, 2009 at 10:00 AM, Uwe Hermann<[email protected]> 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 -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
