Patchwork fix hd8me early-boot 50 sec stallout

login
register
about
Submitter Joe Korty
Date 2010-05-14 16:45:27
Message ID <20100514164527.GA24346@tsunami.ccur.com>
Download mbox | patch
Permalink /patch/1329/
State New
Headers show

Comments

Joe Korty - 2010-05-14 16:45:27
Fix the SuperMicro H8DME-2 early-boot 50 sec stall.

Somewhere between r5491 and r5496, a change occured which
added some 30-50 seconds to the boot time of a Supermicro
H8DME-2.  This rises by an additional 30-50 seconds if
the boot sequence decides it needs to reset-restart.

The problem was traced to a printk which was enabled
somewhere between those two svn revisions.  Apparently this
printk appears too early in the boot sequence to function
correctly.

This patch again comments out the printk, but this
time around, adds an explanation, to encourage future
maintainers to keep it commented out.

Please apply.

Joe

Signed-off-by: Joe Korty <joe.korty@ccur.com>
Myles Watson - 2010-05-14 19:13:49
On Fri, May 14, 2010 at 10:45 AM, Joe Korty <joe.korty@ccur.com> wrote:
> Fix the SuperMicro H8DME-2 early-boot 50 sec stall.
>
> Somewhere between r5491 and r5496, a change occured which
> added some 30-50 seconds to the boot time of a Supermicro
> H8DME-2.  This rises by an additional 30-50 seconds if
> the boot sequence decides it needs to reset-restart.
>
> The problem was traced to a printk which was enabled
> somewhere between those two svn revisions.  Apparently this
> printk appears too early in the boot sequence to function
> correctly.
It's possible that someone was confused because there are two
invocations of enable_smbus in romstage.c

Do you need both?
>
> This patch again comments out the printk, but this
> time around, adds an explanation, to encourage future
> maintainers to keep it commented out.
>
> Please apply.
I think we could commit this, but while you're thinking about it maybe
we should fix it a little more.

I think it would make sense to return from the function and set a post
code when the SMBus controller is not found, instead of calling the
write functions with an invalid device.

Thanks,
Myles

> Index: trunk/src/southbridge/nvidia/mcp55/mcp55_early_smbus.c
> ===================================================================
> --- trunk.orig/src/southbridge/nvidia/mcp55/mcp55_early_smbus.c 2010-05-14 08:37:35.000000000 -0400
> +++ trunk/src/southbridge/nvidia/mcp55/mcp55_early_smbus.c      2010-05-14 08:39:06.000000000 -0400
> @@ -32,11 +32,14 @@
>        device_t dev;
>        dev = pci_locate_device(PCI_ID(0x10de, 0x0368), 0);
>
> +#if 0
> +       /* for the h8dme, at least, this is too early for a printk */
>        if (dev == PCI_DEV_INVALID) {

                  post code & return here, with no else.

>        } else {
>                printk(BIOS_DEBUG, "SMBus controller enabled\n");
>        }
> +#endif
>
>        /* 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
>

Patch

Index: trunk/src/southbridge/nvidia/mcp55/mcp55_early_smbus.c
===================================================================
--- trunk.orig/src/southbridge/nvidia/mcp55/mcp55_early_smbus.c	2010-05-14 08:37:35.000000000 -0400
+++ trunk/src/southbridge/nvidia/mcp55/mcp55_early_smbus.c	2010-05-14 08:39:06.000000000 -0400
@@ -32,11 +32,14 @@ 
 	device_t dev;
 	dev = pci_locate_device(PCI_ID(0x10de, 0x0368), 0);
 
+#if 0
+	/* for the h8dme, at least, this is too early for a printk */
 	if (dev == PCI_DEV_INVALID) {
 		printk(BIOS_WARNING, "SMBUS controller not found\n");
 	} else {
 		printk(BIOS_DEBUG, "SMBus controller enabled\n");
 	}
+#endif
 
 	/* set smbus iobase */
 	pci_write_config32(dev, 0x20, SMBUS0_IO_BASE | 1);