Re: [PATCH] 6700/6702PXH quirk

[Bjorn Helgaas <bjorn.helgaas@hp.com>]

On Friday 05 August 2005 5:51 pm, Kristen Accardi wrote:
> On the 6700/6702 PXH part, a MSI may get corrupted if an ACPI hotplug
> driver and SHPC driver in MSI mode are used together.  This patch will
> prevent MSI from being enabled for the SHPC as part of an early pci
> quirk, as well as on any pci device which sets the no_msi bit.  

For mailing list archaeology, I assume this is erratum 15 in the
6700/6702 PXH spec update:
	http://download.intel.com/design/chipsets/specupdt/30270609.pdf

which says

	An MSI is generated by the standard hot-plug controller may
	get corrupted in presence of another ACPI hot-plug driver.
	The ACPI driver performs configuration reads of DWSEL/DWORD
	register in order to determine the hot-plug capability of all
	the ACPI devices.  If the MSI is generated by the Standard
	Hot-Plug Controller (SHPC) in this time period, there is a
	possibility of the MSI getting corrupted.  As a result the
	MSI may not get issued upstream to the MCH.  The above is a
	result of interaction of separate events that are unpredict-
	able.

So what still bugs me about this (and I'm probably just showing my
ignorance here), is that we seem to have two drivers (SHPC and ACPI)
poking at the same hardware.  Why is this?

And where exactly is the ACPI code that is involved?  I see shpc_init()
doing config reads of DWORD_DATA, but I don't see how ACPI is involved.
Is there some AML that's doing the config accesses?  Why would there
be AML if we're using SHPC?

> @@ -699,6 +699,9 @@ int pci_enable_msi(struct pci_dev* dev)
>  	if (!pci_msi_enable || !dev)
>   		return status;
>  
> +	if (dev->no_msi)
> +		return status;
> +

Is there any reason not to fold this into the test above it?

> +static void __devinit quirk_pcie_pxh(struct pci_dev *dev)
> +{
> +	disable_msi_mode(dev, pci_find_capability(dev, PCI_CAP_ID_MSI),
> +					PCI_CAP_ID_MSI);

Is this even needed?  You're doing early fixups, which happen before
any drivers touch the device, so you should only need to disable MSI
if the BIOS can leave it enabled.  But I would have thought MSI would
be disabled until a driver explicitly enables it.