[coreboot] r784 - in coreboot-v3: arch/x86 device mainboard/emulation/qemu-x86 mainboard/pcengines/alix1c mainboard/pcengines/alix2c3

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Aug 18 19:13:14 CEST 2008


On 18.08.2008 18:55, Stefan Reinauer wrote:
> This patch is bad. Please back it out.
>   

Can we at least establish a commit message stating the reason for the
backout?

> svn at coreboot.org wrote:
>   
>> Modified: coreboot-v3/arch/x86/Kconfig
>> ===================================================================
>> --- coreboot-v3/arch/x86/Kconfig	2008-08-18 13:03:43 UTC (rev 783)
>> +++ coreboot-v3/arch/x86/Kconfig	2008-08-18 16:48:27 UTC (rev 784)
>> @@ -93,12 +93,14 @@
>>  	
>>  config LOGICAL_CPUS
>>  	hex
>> +	depends CPU_AMD_K8
>>   
>>     
> This is a wrong dependency. It does not reflect any physical dependency,
> nor a real dependency in the code. The opposite. 

So should LOGICAL_CPUS be selectable for GeodeLX?

> It is the purpose that
> these variables are used throughout the code to verify the availability
> of certain features.
>   

I have trouble understanding that sentence.

> The correct way of describing this would be to set the above variable to
> a certain value in CPU_AMD_K8, or any related option.
>   

Ah, so you want forward dependencies with select? By the way, at least
our version of Kconfig can't set any variable except boolean, so you're
back to "depends" clauses. If you don't believe me, here is the error
message:

mainboard/amd/Kconfig:57:warning: 'APIC_ID_OFFSET' has wrong type.
'select' only accept arguments of boolean and tristate type

And all this forward-select was frowned upon by Kconfig maintainers
because it is easy to create impossible loops:
A selects B
B depends on !A
Boom.

>>  	default 1
>>  	help
>>  		How many logical CPUs there are. Fix me.
>>  
>>  config MAX_PHYSICAL_CPUS
>>  	hex
>> +	depends CPU_AMD_K8
>>   
>>     
> dito
>   

dito

>>  	default 1
>>  	help
>>  		Max number of physical CPUs (sockets)
>> @@ -138,6 +140,7 @@
>>  
>>  config SMP
>>  	boolean
>> +	depends CPU_I586 || CPU_AMD_K8
>>   
>>     
> dito
>   

No, that is a physical dependency. Show me a SMP Geode. It can't be done.

>>  	default 0
>>  	help
>>  	  This option is used to enable certain functions to make 
>> @@ -147,7 +150,7 @@
>>  
>>  config IOAPIC
>>  	boolean
>> -	depends ARCH_X86
>> +	depends ARCH_X86 && CPU_AMD_K8
>>   
>>     
> dito
>   
>>  	default 0
>>  	help
>>  	  If you want to configure an IOAPIC, set this. 
>> @@ -216,6 +219,7 @@
>>  
>>  config USBDEBUG_DIRECT
>>  	boolean
>> +	depends SOUTHBRIDGE_NVIDIA_MCP55
>>   
>>     
> dito
>   
>>  	default 0
>>  	help
>>  		Determines if we enable USB Direct debugging. If you don't have a dongle, 
>>
>> Modified: coreboot-v3/device/Kconfig
>> ===================================================================
>> --- coreboot-v3/device/Kconfig	2008-08-18 13:03:43 UTC (rev 783)
>> +++ coreboot-v3/device/Kconfig	2008-08-18 16:48:27 UTC (rev 784)
>> @@ -97,11 +97,13 @@
>>  
>>  config PCI_64BIT_PREF_MEM
>>  	bool "64 bit prefetchable memory addresses"
>> +	depends CPU_AMD_K8
>>   
>>     
> dito
>   

That one should depend on the address space of the CPU.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list