[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