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(-)
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
* 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
-
Kevin O'Connor -
Peter Stuge -
Stefan Reinauer