On Mon, Mar 17, 2008 at 6:45 AM, Uwe Hermann uwe@hermann-uwe.de wrote:
Do we actually need this kind of check? If the LPC device is not there or is the wrong one, that's clearly a programming bug, no? This function will only be called from i3100 boards, so the LPC device is surely available.
Similar example: when you write code for an Intel board and include an AMD CPU init file or something -- clearly a bug, no need for bloating the code to check for that kind of programmer bug. Opinions?
I'd say drop this check (here and in many other files/chipsets).
Agreed. I think it is reasonable for code in src/southbridge/intel/i3100 to assume it is actually being run on an Intel 3100 southbridge.
Added: trunk/coreboot-v2/src/southbridge/intel/i3100/i3100_reset.c
--- trunk/coreboot-v2/src/southbridge/intel/i3100/i3100_reset.c (rev 0) +++ trunk/coreboot-v2/src/southbridge/intel/i3100/i3100_reset.c 2008-03-16 23:34:10 UTC (rev 3157) @@ -0,0 +1,26 @@
[...]
+#include <arch/io.h>
+void hard_reset(void) +{
outb(0x06, 0xcf9);+}
Is this tested? When and where is it used/invoked? coreboot-internally as part of bringup or when you say 'reboot' in Linux?
It is invoked by coreboot itself (in hardwaremain(), I believe) but only after a reboot, not at initial powerup. Without it, coreboot hangs during the root PCI bus scan.
Does it have to be in an extra file? That's a bit of an overkill for one line of code. Can we merge it into some other i3100 file?
It would be nice if it could go into i3100.c, but I'm not sure I understand how drivers get linked in.
--Ed