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 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot