[PATCH] Disable integrated Intel 3100 devices properly

[Ed Swierk]

Setting an integrated southbridge device (like SATA or USB2.0) to
"off" in Config.lb should cause the PCI device not to respond to
configuration requests. The code that takes care of this for the Intel
3100 is in src/southbridge/intel/i3100/i3100.c. Unfortunately I copied
the esb6300 implementation without noticing that it makes no sense at
all on the Intel 3100. The attached patch implements this properly for
the Intel 3100.

--Ed

[PATCH] Disable integrated Intel 3100 devices properly

[Uwe Hermann]

On Mon, Mar 31, 2008 at 04:38:34PM -0700, Ed Swierk wrote:
> Setting an integrated southbridge device (like SATA or USB2.0) to
> "off" in Config.lb should cause the PCI device not to respond to
> configuration requests. The code that takes care of this for the Intel
> 3100 is in src/southbridge/intel/i3100/i3100.c. Unfortunately I copied
> the esb6300 implementation without noticing that it makes no sense at
> all on the Intel 3100. The attached patch implements this properly for
> the Intel 3100.

You forgot the Signed-off-by.
> /* Temporarily enable the root complex register block at 0xa0000000 */
> 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:
> *disable |= (1 << (func == 0x0 ? 14 : func));
> break;
> case 0x1d:
> *disable |= (1 << (func + 8));
> break;
> case 0x1c:
> *disable |= (1 << (func + 16));
> break;

This part could use a small comment about what exactly is disabled in
each "case".

Rest of the patch looks good. Is this build-tested and tested on
hardware? If so, I can ack and commit if you resend with a Signed-off-by.

Uwe.

[PATCH] Disable integrated Intel 3100 devices properly

[Ed Swierk]

On Mon, Mar 31, 2008 at 5:41 PM, Uwe Hermann  wrote:
> Rest of the patch looks good. Is this build-tested and tested on
>  hardware? If so, I can ack and commit if you resend with a Signed-off-by.

Yes, I tested it on a Mt. Arvon board.

Signed-off-by: Ed Swierk

--Ed

[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.