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 <[email protected]> wrote: > Is this what your orl change fixed? > > wt > > On Sun, Oct 3, 2010 at 6:34 AM, Stefan Reinauer > <[email protected]> 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 <[email protected]> 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 <[email protected]> >>>> --- >>>> 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: [email protected] >> http://www.coreboot.org/mailman/listinfo/coreboot >> > -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
