[coreboot] [commit] r5359 - in trunk/src: . cpu/amd/model_10xxxcpu/amd/model_fxx cpu/amd/model_lx cpu/x86/mtrrinclude/cpu/x86 mainboard/amd/mahogany_fam10mainboard/amd/serengeti_cheetah mainboard/asus/a8n_...

[Stefan Reinauer]

wasn't meant to be private only.

On 4/7/10 12:24 AM, Stefan Reinauer wrote:
> On 4/7/10 12:06 AM, Myles Watson wrote:
>   
>>   
>>     
>>> -----Original Message-----
>>> From: coreboot-bounces at coreboot.org [mailto:coreboot-bounces at coreboot.org]
>>> On Behalf Of repository service
>>> Sent: Tuesday, April 06, 2010 3:50 PM
>>> To: coreboot at coreboot.org
>>> Subject: [coreboot] [commit] r5359 - in trunk/src: .
>>> cpu/amd/model_10xxxcpu/amd/model_fxx cpu/amd/model_lx
>>> cpu/x86/mtrrinclude/cpu/x86
>>> mainboard/amd/mahogany_fam10mainboard/amd/serengeti_cheetah
>>> mainboard/asus/a8n_...
>>>
>>> Author: stepan
>>> Date: Tue Apr  6 21:50:21 2010
>>> New Revision: 5359
>>> URL: https://tracker.coreboot.org/trac/coreboot/changeset/5359
>>>
>>> Log:
>>> No warnings day, next round.
>>>     
>>>       
>> Thanks.
>>
>>   
>>     
>>>    trunk/src/cpu/x86/mtrr/earlymtrr.c
>>>     
>>>       
>>   
>>     
>>>    trunk/src/include/cpu/x86/mtrr.h
>>>     
>>>       
>>   
>>     
>>> Modified: trunk/src/cpu/x86/mtrr/earlymtrr.c
>>> ==========================================================================
>>> ====
>>> --- trunk/src/cpu/x86/mtrr/earlymtrr.c	Tue Apr  6 21:49:31 2010
>>> 	(r5358)
>>> +++ trunk/src/cpu/x86/mtrr/earlymtrr.c	Tue Apr  6 21:50:21 2010
>>> 	(r5359)
>>> @@ -4,37 +4,7 @@
>>>  #include <cpu/x86/mtrr.h>
>>>  #include <cpu/x86/msr.h>
>>>
>>> -/* Validate CONFIG_XIP_ROM_SIZE and CONFIG_XIP_ROM_BASE */
>>> -#if defined(CONFIG_XIP_ROM_SIZE) && !defined(CONFIG_XIP_ROM_BASE)
>>> -# error "CONFIG_XIP_ROM_SIZE without CONFIG_XIP_ROM_BASE"
>>>     
>>>       
>> ...
>> I'm surprised you wanted to move this.  It made sense to me to check it when
>> compiling CAR.
>>   
>>     
> The idea was to be able to have the check, no matter if only
> cpu/x86/earlymtrr.c or only cpu/amd/amd_earlymtrr.c is included.
>
> It seems that in the long term most of the earlymtrr code can completely
> go away though, so I was not sure where to put it.
>
>   
>>> Modified: trunk/src/include/cpu/x86/mtrr.h
>>> ==========================================================================
>>> ====
>>> --- trunk/src/include/cpu/x86/mtrr.h	Tue Apr  6 21:49:31 2010
>>> 	(r5358)
>>> +++ trunk/src/include/cpu/x86/mtrr.h	Tue Apr  6 21:50:21 2010
>>> 	(r5359)
>>>     
>>>       
>> ...
>>   
>>     
>>> +#if !defined(CONFIG_RAMTOP)
>>> +# error "CONFIG_RAMTOP not defined"
>>> +#endif
>>>     
>>>       
>> Now that we have Kconfig, I think this check can disappear.  Is there a way
>> to un-define CONFIG_RAMTOP?
>>     
> In theory, yes. If you say this in Kconfig:
>
> config FOO
> bool
> depends on SOME_FEATURE
>
> config BAR
> bool
> depends on FOO
>
> You might end up with BAR not being there even though we modified
> Kconfig to always set 0 bools to 0 instead of omitting them.
> That's because Kconfig completely skips BAR if FOO is not set.
>
> With RAMTOP being
>
> config RAMTOP
> hex
> default 0x200000
>
> it can't happen.
>
> But I didn't want to change the rough logical flow with self-acked code,
> so I thought I better leave an error check too many in there ;-)
>
> If you feel it should go, you have my Acked-by: Stefan Reinauer
> <stepan at coresystems.de>
>
>   


-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866