On Fri, Aug 28, 2009 at 10:09 AM, Uwe Hermann<[email protected]> wrote: > On Fri, Aug 28, 2009 at 07:51:42AM -0700, ron minnich wrote: >> Index: src/cpu/x86/tsc/Makefile.inc >> =================================================================== >> --- src/cpu/x86/tsc/Makefile.inc (revision 4607) >> +++ src/cpu/x86/tsc/Makefile.inc (working copy) >> @@ -1,7 +1,2 @@ >> obj-y += delay_tsc.o >> >> -# default CONFIG_TSC_X86RDTSC_CALIBRATE_WITH_TIMER2=0 >> -# if CONFIG_UDELAY_TSC >> -# default CONFIG_HAVE_INIT_TIMER=1 >> -# object delay_tsc.o >> -# end > > This looks strange. You use "select UDELAY_TSC" below, but that is never > defined anywhere, and also not used here?
We should define this in either src/cpu/x86/Kconfig or src/cpu/x86/tsc/Kconfig. I vote for src/cpu/x86/Kconfig because the src/cpu/x86 directory is really way too fragmented already. > > >> Index: src/mainboard/amd/rumba/Kconfig >> =================================================================== >> --- src/mainboard/amd/rumba/Kconfig (revision 0) >> +++ src/mainboard/amd/rumba/Kconfig (revision 0) >> @@ -0,0 +1,62 @@ > [...] >> +config BOARD_AMD_RUMBA >> + bool "Rumba" >> + select ARCH_X86 >> + select CPU_AMD_GX2 >> + select NORTHBRIDGE_AMD_GX2 >> + select SOUTHBRIDGE_AMD_CS5536 >> + select UDELAY_TSC >> + select HAVE_PIRQ_TABLE >> + help >> + AMD Rumba mainboard. > [...] >> +config TSC_X86RDTSC_CALIBRATE_WITH_TIMER2 >> + int >> + default 0 >> + depends on BOARD_AMD_RUMBA > > Same as UDELAY_TSC, never defined? Defined here, right? But it really ought to be in the src/cpu/x86 tree somewhere. > > >> Index: src/mainboard/amd/rumba/Makefile.inc >> =================================================================== >> --- src/mainboard/amd/rumba/Makefile.inc (revision 0) >> +++ src/mainboard/amd/rumba/Makefile.inc (revision 0) >> @@ -0,0 +1 @@ >> +include $(src)/mainboard/Makefile.romccboard.inc > > We'll have to check if this works. From a quick glance > the Rumba does not have the mmx related lines (which _are_ in > Makefile.romccboard.inc, though): I speculate that rumba ought to just have its own? > > crt0-y += ../../../../src/cpu/x86/fpu/enable_fpu.inc > crt0-y += ../../../../src/cpu/x86/mmx/enable_mmx.inc Or we need to make these conditional includes. Your call, I think you know better than I do. > crt0-y += auto.inc > crt0-y += ../../../../src/cpu/x86/mmx/disable_mmx.inc > > Rumba only has: > > mainboardinit cpu/x86/fpu/enable_fpu.inc > mainboardinit ./auto.inc > > It's not a big deal to adapt Makefile.romccboard.inc to handle both > cases, but it would be good to know if those mmx lines are actually > needed in general (and on GX2 specifically). I will hold off on committing until we decide these things, or I get back to this and this patch has hung ... it's already acked :-) ron -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
