Re: [coreboot] [PATCH] Disable integrated Intel 3100 devices properly

[Uwe Hermann]

On Mon, Mar 31, 2008 at 07:01:52PM -0700, Ed Swierk wrote:
> /* To disable an integrated southbridge device, set the corresponding
>   flag in the Function Disable register */

Please use this format for multi-line comments as per coding guidelines:

/*
 * To disable an integrated southbridge device, set the corresponding
 * flag in the Function Disable register.
 */

Also, please end all sentences in code comments with a full stop, here...
> /* Temporarily enable the root complex register block at 0xa0000000 */

here,
> lpc_dev = dev_find_slot(0x0, PCI_DEVFN(0x1f, 0x0));
> pci_write_config32(lpc_dev, 0xf0, 0xa0000000 | (1 << 0));
> disable = (volatile u32 *) 0xa0003418;
> func = PCI_FUNC(dev->path.u.pci.devfn);
> switch (PCI_SLOT(dev->path.u.pci.devfn)) {
> case 0x1f: /* LPC (fn0), SATA (fn2), SMBus (fn3) */
> *disable |= (1 << (func == 0x0 ? 14 : func));
> break;
> case 0x1d: /* UHCI (fn0, fn1), EHCI (fn7) */
> *disable |= (1 << (func + 8));
> break;
> case 0x1c: /* PCIe ports B0-B3 (fn0-fn3) */
> *disable |= (1 << (func + 16));
> break;
> }
> /* Disable the root complex register block */

and here.

Rest looks good.

Acked-by: Uwe Hermann

Uwe.