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

Uwe Hermann uwe at hermann-uwe.de
Tue Apr 1 17:10:55 CEST 2008


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 at hermann-uwe.de>


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org




More information about the coreboot mailing list