[coreboot] final set of changes to allow rumba to build with Kconfig
ron minnich
rminnich at gmail.com
Sat Aug 29 04:53:02 CEST 2009
On Fri, Aug 28, 2009 at 10:09 AM, Uwe Hermann<uwe at hermann-uwe.de> 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
More information about the coreboot
mailing list