Patchwork Fix compile failure when VGA_ROM_RUN disabled on epia-cn.

The cn700.c code references mainboard_interrupt_handlers() which isn't
defined if VGA_ROM_RUN is off.  Define a dummy implementation of that
function for this case.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 src/arch/x86/include/arch/interrupt.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

* Kevin O'Connor <kevin@koconnor.net> [110116 18:59]:
> The cn700.c code references mainboard_interrupt_handlers() which isn't
> defined if VGA_ROM_RUN is off.  Define a dummy implementation of that
> function for this case.
> 
> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Acked-by: Stefan Reinauer <stepan@coreboot.org>

And I think the extern should also be removed. It's not needed for
functions.

> ---
>  src/arch/x86/include/arch/interrupt.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/src/arch/x86/include/arch/interrupt.h b/src/arch/x86/include/arch/interrupt.h
> index 2d2330b..c3cda30 100644
> --- a/src/arch/x86/include/arch/interrupt.h
> +++ b/src/arch/x86/include/arch/interrupt.h
> @@ -22,4 +22,8 @@
>  #include "registers.h"
>  
>  /* setup interrupt handlers for mainboard */
> +#if defined(CONFIG_PCI_OPTION_ROM_RUN_REALMODE) && CONFIG_PCI_OPTION_ROM_RUN_REALMODE
>  extern void mainboard_interrupt_handlers(int intXX, void *intXX_func);
> +#else
> +static inline void mainboard_interrupt_handlers(int intXX, void *intXX_func) { }
> +#endif
> -- 
> 1.7.3.4
> 
> 
> -- 
> coreboot mailing list: coreboot@coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
>

Kevin O'Connor wrote:
> The cn700.c code references mainboard_interrupt_handlers() which isn't
> defined if VGA_ROM_RUN is off.  Define a dummy implementation of that
> function for this case.

Shouldn't cn700.c code be changed instead, so that it doesn't
reference the function when VGA_ROM_RUN is off?


//Peter

* Peter Stuge <peter@stuge.se> [110117 02:32]:
> Kevin O'Connor wrote:
> > The cn700.c code references mainboard_interrupt_handlers() which isn't
> > defined if VGA_ROM_RUN is off.  Define a dummy implementation of that
> > function for this case.
> 
> Shouldn't cn700.c code be changed instead, so that it doesn't
> reference the function when VGA_ROM_RUN is off?

Possibly. In this case it would require an ifdef in each of these files:
./src/mainboard/technexion/tim5690/vgabios.c
./src/northbridge/via/cn400/vga.c
./src/northbridge/via/cn700/vga.c
./src/northbridge/via/cx700/vga.c
./src/northbridge/via/vt8623/vga.c
./src/northbridge/via/vx800/vga.c
because they all expose the same problem.

instead of the one in ./src/arch/x86/include/arch/interrupt.h

Stefan

Stefan Reinauer wrote:
> > > The cn700.c code references mainboard_interrupt_handlers() which isn't
> > > defined if VGA_ROM_RUN is off.  Define a dummy implementation of that
> > > function for this case.
> > 
> > Shouldn't cn700.c code be changed instead, so that it doesn't
> > reference the function when VGA_ROM_RUN is off?
> 
> Possibly. In this case it would require an ifdef in each of these files:
> ./src/mainboard/technexion/tim5690/vgabios.c
> ./src/northbridge/via/cn400/vga.c
> ./src/northbridge/via/cn700/vga.c
> ./src/northbridge/via/cx700/vga.c
> ./src/northbridge/via/vt8623/vga.c
> ./src/northbridge/via/vx800/vga.c
> because they all expose the same problem.
> 
> instead of the one in ./src/arch/x86/include/arch/interrupt.h

It still sounds like the right solution. We know that coreboot
support for via hardware suffers from copypastitis. Would be good to
unify that at some point, but for this patch I think the fix should
go into the components that are affected, so that they don't use what
isn't available.


//Peter

On Mon, Jan 17, 2011 at 05:37:45AM +0100, Peter Stuge wrote:
> Stefan Reinauer wrote:
> > > > The cn700.c code references mainboard_interrupt_handlers() which isn't
> > > > defined if VGA_ROM_RUN is off.  Define a dummy implementation of that
> > > > function for this case.
> > > 
> > > Shouldn't cn700.c code be changed instead, so that it doesn't
> > > reference the function when VGA_ROM_RUN is off?
> > 
> > Possibly. In this case it would require an ifdef in each of these files:
> > ./src/mainboard/technexion/tim5690/vgabios.c
> > ./src/northbridge/via/cn400/vga.c
> > ./src/northbridge/via/cn700/vga.c
> > ./src/northbridge/via/cx700/vga.c
> > ./src/northbridge/via/vt8623/vga.c
> > ./src/northbridge/via/vx800/vga.c
> > because they all expose the same problem.
> > 
> > instead of the one in ./src/arch/x86/include/arch/interrupt.h
> 
> It still sounds like the right solution. We know that coreboot
> support for via hardware suffers from copypastitis. Would be good to
> unify that at some point, but for this patch I think the fix should
> go into the components that are affected, so that they don't use what
> isn't available.

In this particular case, I think the code is cleaner to ignore calls
to mainboard_interrupt_handlers() when option rom support is not
desired.  The northbridge code is telling the option rom code it has a
helper function - the northbridge code doesn't care that the option
rom code isn't going to use it, and shouldn't have to care.

-Kevin

Kevin O'Connor wrote:
> > We know that coreboot support for via hardware suffers from
> > copypastitis. Would be good to unify that at some point, but for
> > this patch I think the fix should go into the components that are
> > affected, so that they don't use what isn't available.
> 
> In this particular case, I think the code is cleaner to ignore calls
> to mainboard_interrupt_handlers() when option rom support is not
> desired.  The northbridge code is telling the option rom code

Neither of which encompass the header file.

I think it would be an equally good solution to add the empty
function into the code for each northbridge.


//Peter