[coreboot] [PATCH] Add CPP logic to VIA CAR init code.

Warren Turkal wt at penguintechs.org
Sun Oct 3 18:54:37 CEST 2010


I see that is the change that fixed it. However, I'm still not sure I
understand the fix completely. When CONFIG_TINY_BOOTBLOCK is defined
and AUTO_XIP_ROM_BASE is not defined, we get the following line (line
number 159 or so) in build/mainboard/via/vt8454c/crt0.s:
 movl $AUTO_XIP_ROM_BASE, %eax

How does that assemble without an error?

Is the AUTO_XIP_ROM_BASE set to some value (presumably 0) by the
assembler for some reason that I don't see? For reference, here is the
equivalent line from crt0.disasm:
100 0115 B8000000 >---->-------movl>---$REAL_XIP_ROM_BASE, %eax

Thanks,
wt

On Sun, Oct 3, 2010 at 9:31 AM, Warren Turkal <wt at penguintechs.org> wrote:
> Is this what your orl change fixed?
>
> wt
>
> On Sun, Oct 3, 2010 at 6:34 AM, Stefan Reinauer
> <stefan.reinauer at coresystems.de> wrote:
>>  On 10/3/10 11:24 AM, Warren Turkal wrote:
>>> *ping* I really need an ack or nack on this.
>>
>> NACK.. Still the wrong way, it just blindly comments out an arbitrary
>> piece of code.
>>
>> However, the code has been fixed already in the repo.
>>
>> Stefan
>>> Thanks,
>>> wt
>>>
>>> On Sat, Oct 2, 2010 at 1:59 AM, Warren Turkal <wt at penguintechs.org> wrote:
>>>> VIA/AMD experts,
>>>>
>>>> This patch get's the via/vt8454c back to building. However, I am not
>>>> sure if the code that is being #ifdef'ed out will actually ever be used
>>>> on a via platform. The code comes straight from the amd CAR
>>>> implementation. A couple of questions are raised by this:
>>>> 1) Should we just delete the code from the via file instead of this
>>>>   patch?
>>>> 2) Should the amd and via CAR code be integrated into one file? Maybe
>>>>   just portions of the files if not the whole files?
>>>>
>>>> Also, another happy side effect of this change is that all the c7 boards
>>>> seem to build with tiny bootblocks. Would everyone be ok with my making
>>>> that change?
>>>>
>>>> Thanks,
>>>> wt
>>>> 8<----------------------------------------------------------------------
>>>> The execute-in-place (XIP) config options need to be set in order to get
>>>> XIP functionality, so it needs to be excluded when those settings are
>>>> not set.
>>>>
>>>> Signed-off-by: Warren Turkal <wt at penguintechs.org>
>>>> ---
>>>>  src/cpu/via/car/cache_as_ram.inc |    4 ++++
>>>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/src/cpu/via/car/cache_as_ram.inc b/src/cpu/via/car/cache_as_ram.inc
>>>> index be00fe3..d18ac3a 100644
>>>> --- a/src/cpu/via/car/cache_as_ram.inc
>>>> +++ b/src/cpu/via/car/cache_as_ram.inc
>>>> @@ -85,6 +85,8 @@ clear_fixed_var_mtrr_out:
>>>>        movl    $(~(CacheSize - 1) | 0x800), %eax
>>>>        wrmsr
>>>>
>>>> +#if defined(CONFIG_XIP_ROM_SIZE) && defined(CONFIG_XIP_ROM_BASE)
>>>> +
>>>>  #if defined(CONFIG_TINY_BOOTBLOCK) && CONFIG_TINY_BOOTBLOCK
>>>>  #define REAL_XIP_ROM_BASE AUTO_XIP_ROM_BASE
>>>>  #else
>>>> @@ -106,6 +108,8 @@ clear_fixed_var_mtrr_out:
>>>>        movl    $(~(CONFIG_XIP_ROM_SIZE - 1) | 0x800), %eax
>>>>        wrmsr
>>>>
>>>> +#endif /* CONFIG_XIP_ROM_SIZE && CONFIG_XIP_ROM_BASE */
>>>> +
>>>>        /* Set the default memory type and enable fixed and variable MTRRs. */
>>>>        /* TODO: Or also enable fixed MTRRs? Bug in the code? */
>>>>        movl    $MTRRdefType_MSR, %ecx
>>>> --
>>>> 1.7.1
>>>>
>>>>
>>
>>
>> --
>> coreboot mailing list: coreboot at coreboot.org
>> http://www.coreboot.org/mailman/listinfo/coreboot
>>
>




More information about the coreboot mailing list