Re: [coreboot] [PATCH] don't print too early on mcp55-based boards
On Mon, Nov 01, 2010 at 11:01:41PM +0100, Peter Stuge wrote: Ward Vandewege wrote: See attached. Perhaps we should also print a post code if the SMBus controller can't be found - suggestions for a value? 0x5B ? Let's do that as part of the die modification. We can't print this early. This patch fixes a hang on supermicro/h8dme supermicro/h8dmr supermicro/h8dmr_fam10 and possibly on other mcp55-based boards. Signed-off-by: Ward Vandewege w...@gnu.org Acked-by: Peter Stuge pe...@stuge.se r6048. Thanks, Ward. -- Ward Vandewege w...@fsf.org Free Software Foundation - Senior Systems Administrator -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] don't print too early on mcp55-based boards
Ward Vandewege wrote:
I'm not sure adding a post code to 'die' is a good idea. The
problem with doing that is that it would clobber any previous post
codes, which might be a better indicator for what's going wrong.
I think it is important to have a special POST code for use in die()
to positively identify that that is really where the code has hung.
Stefan Reinauer wrote:
We could add a post code to the parameters of the die() function.
I think this is a good idea, and I would like to suggest that die()
does the following:
void die(u8 post) {
while(1) {
post_code(0xdd);
mdelay(700);
post_code(post);
mdelay(700);
}
}
There is no printk() which is sad. I think it's better to only use
POST codes if that is the only reliable output method, but I think
it's a lot better if the print functions can just be made to not
hang if called before they can actually output something. That would
need a bit more work though, so for now I'd like to suggest the
above.
//Peter
--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] don't print too early on mcp55-based boards
On Tue, Nov 02, 2010 at 11:20:43PM +0100, Uwe Hermann wrote: I don't object to the patch, and we should probably fix this in all other southbridges, I think the same problem applies there. But: the die() call itself also does a printk(), so that'll still hang if the error path is chosen (at that point it probably doesn't matter much, though, as we die anyway). Right, I think it does not matter. If the die happens when printk is already functional, great, if not it will hang there which is fine. I also agree that die() should have a POST code, preferrably something easy to remember. It already has a commented-out //post_code(0xff);. Not sure why it's disabled, but I think it should be something other than 0xff, that's a bit too special for my taste. We have 0xee: Not supposed to get here as per documentation/POSTCODES, so maybe we can use 0xdd (d as in die), if that's not already used elsewhere. So, thinking about this a little more, I'm not sure adding a post code to 'die' is a good idea. The problem with doing that is that it would clobber any previous post codes, which might be a better indicator for what's going wrong. Perhaps a good way to deal with fatal runtime error conditions would be a) set a unique post code b) call die in the assumption that die does not clobber the post code. What do you think? Thanks, Ward. -- Ward Vandewege w...@fsf.org Free Software Foundation - Senior Systems Administrator -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] don't print too early on mcp55-based boards
Sent from my mobile phone On 03.11.2010, at 09:56, Ward Vandewege w...@gnu.org wrote: On Tue, Nov 02, 2010 at 11:20:43PM +0100, Uwe Hermann wrote: I don't object to the patch, and we should probably fix this in all other southbridges, I think the same problem applies there. But: the die() call itself also does a printk(), so that'll still hang if the error path is chosen (at that point it probably doesn't matter much, though, as we die anyway). Right, I think it does not matter. If the die happens when printk is already functional, great, if not it will hang there which is fine. I also agree that die() should have a POST code, preferrably something easy to remember. It already has a commented-out //post_code(0xff);. Not sure why it's disabled, but I think it should be something other than 0xff, that's a bit too special for my taste. We have 0xee: Not supposed to get here as per documentation/POSTCODES, so maybe we can use 0xdd (d as in die), if that's not already used elsewhere. So, thinking about this a little more, I'm not sure adding a post code to 'die' is a good idea. The problem with doing that is that it would clobber any previous post codes, which might be a better indicator for what's going wrong. Perhaps a good way to deal with fatal runtime error conditions would be a) set a unique post code b) call die in the assumption that die does not clobber the post code. We could add a post code to the parameters of the die() function. What do you think? Thanks, Ward. -- Ward Vandewege w...@fsf.org Free Software Foundation - Senior Systems Administrator -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] don't print too early on mcp55-based boards
On Mon, Nov 01, 2010 at 11:01:41PM +0100, Peter Stuge wrote: Ward Vandewege wrote: See attached. Perhaps we should also print a post code if the SMBus controller can't be found - suggestions for a value? 0x5B ? We can't print this early. This patch fixes a hang on supermicro/h8dme supermicro/h8dmr supermicro/h8dmr_fam10 and possibly on other mcp55-based boards. Signed-off-by: Ward Vandewege w...@gnu.org Acked-by: Peter Stuge pe...@stuge.se I don't object to the patch, and we should probably fix this in all other southbridges, I think the same problem applies there. But: the die() call itself also does a printk(), so that'll still hang if the error path is chosen (at that point it probably doesn't matter much, though, as we die anyway). I also agree that die() should have a POST code, preferrably something easy to remember. It already has a commented-out //post_code(0xff);. Not sure why it's disabled, but I think it should be something other than 0xff, that's a bit too special for my taste. We have 0xee: Not supposed to get here as per documentation/POSTCODES, so maybe we can use 0xdd (d as in die), if that's not already used elsewhere. Uwe. -- http://hermann-uwe.de | http://sigrok.org http://randomprojects.org | http://unmaintained-free-software.org -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
[coreboot] [PATCH] don't print too early on mcp55-based boards
See attached. Perhaps we should also print a post code if the SMBus
controller can't be found - suggestions for a value?
Thanks,
Ward.
--
Ward Vandewege w...@fsf.org
Free Software Foundation - Senior Systems Administrator
We can't print this early.
This patch fixes a hang on
supermicro/h8dme
supermicro/h8dmr
supermicro/h8dmr_fam10
and possibly on other mcp55-based boards.
Signed-off-by: Ward Vandewege w...@gnu.org
Index: src/southbridge/nvidia/mcp55/mcp55_early_smbus.c
===
--- src/southbridge/nvidia/mcp55/mcp55_early_smbus.c (revision 6011)
+++ src/southbridge/nvidia/mcp55/mcp55_early_smbus.c (working copy)
@@ -32,11 +32,8 @@
device_t dev;
dev = pci_locate_device(PCI_ID(0x10de, 0x0368), 0);
- if (dev == PCI_DEV_INVALID) {
- printk(BIOS_WARNING, SMBUS controller not found\n);
- } else {
- printk(BIOS_DEBUG, SMBus controller enabled\n);
- }
+ if (dev == PCI_DEV_INVALID)
+ die(SMBus controller not found\n);
/* set smbus iobase */
pci_write_config32(dev, 0x20, SMBUS0_IO_BASE | 1);
--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] don't print too early on mcp55-based boards
Ward Vandewege wrote: See attached. Perhaps we should also print a post code if the SMBus controller can't be found - suggestions for a value? 0x5B ? We can't print this early. This patch fixes a hang on supermicro/h8dme supermicro/h8dmr supermicro/h8dmr_fam10 and possibly on other mcp55-based boards. Signed-off-by: Ward Vandewege w...@gnu.org Acked-by: Peter Stuge pe...@stuge.se -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
