[PATCH 0/4] ACPI fixes for PCIe AER

[PATCH 0/4] ACPI fixes for PCIe AER

[Andrew Patterson]

The following patch series fixes some bugs in how Linux determines
whether PCIe Advance Error Reporting (AER) is supported on a platform.  It
is currently broken on at least HP IA-64 systems.

 - PCI: Run ACPI _OSC method on root bridges only
 - ACPI: Check for any matching CID when walking namespace.
 - PCI ACPI: AER driver should only register PCIe devices with _OSC.
 - PCI ACPI: Added a function to register _OSC with only PCIe devices.


These patches apply to gregkh's patch tree:

      git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/patches


-- 
Andrew Patterson

[PATCH 1/4] PCI ACPI: Added a function to register _OSC with only PCIe devices.

[Andrew Patterson]

From: Andrew Patterson <andrew.patterson@hp.com>

The function pci_osc_support_set() traverses every root bridge when
checking for _OSC support for a capability.  It quits as soon as it finds a
device/bridge that doesn't support the requested capability. This won't
work for systems that have mixed PCI and PCIe bridges when checking for
PCIe features.  I split this function into two -- pci_osc_support_set() and
pcie_osc_support_set(). The latter is used when only PCIe devices should be
traversed.

Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
---

 drivers/pci/pci-acpi.c   |    6 +++---
 include/linux/pci-acpi.h |   11 ++++++++++-
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 02e4876..ec61428 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -156,13 +156,13 @@ run_osc_out:
 }
 
 /**
- * pci_osc_support_set - register OS support to Firmware
+__pci_osc_support_set - register OS support to Firmware
  * @flags: OS support bits
  *
  * Update OS support fields and doing a _OSC Query to obtain an update
  * from Firmware on supported control bits.
  **/
-acpi_status pci_osc_support_set(u32 flags)
+acpi_status __pci_osc_support_set(u32 flags, const char *hid)
 {
 	u32 temp;
 	acpi_status retval;
@@ -176,7 +176,7 @@ acpi_status pci_osc_support_set(u32 flags)
 	temp = ctrlset_buf[OSC_CONTROL_TYPE];
 	ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
 	ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
-	acpi_get_devices ( PCI_ROOT_HID_STRING,
+	acpi_get_devices(hid,
 			acpi_query_osc,
 			ctrlset_buf,
 			(void **) &retval );
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 936ef82..3ba2506 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -48,7 +48,15 @@
 
 #ifdef CONFIG_ACPI
 extern acpi_status pci_osc_control_set(acpi_handle handle, u32 flags);
-extern acpi_status pci_osc_support_set(u32 flags);
+extern acpi_status __pci_osc_support_set(u32 flags, const char *hid);
+static inline acpi_status pci_osc_support_set(u32 flags)
+{
+	return __pci_osc_support_set(flags, PCI_ROOT_HID_STRING);
+}
+static inline acpi_status pcie_osc_support_set(u32 flags)
+{
+	return __pci_osc_support_set(flags, PCI_EXPRESS_ROOT_HID_STRING);
+}
 #else
 #if !defined(AE_ERROR)
 typedef u32 		acpi_status;
@@ -57,6 +65,7 @@ typedef u32 		acpi_status;
 static inline acpi_status pci_osc_control_set(acpi_handle handle, u32 flags)
 {return AE_ERROR;}
 static inline acpi_status pci_osc_support_set(u32 flags) {return AE_ERROR;} 
+static inline acpi_status pcie_osc_support_set(u32 flags) {return AE_ERROR;}
 #endif
 
 #endif	/* _PCI_ACPI_H_ */

[PATCH 2/4] PCI ACPI: AER driver should only register PCIe devices with _OSC.

[Andrew Patterson]

From: Andrew Patterson <andrew.patterson@hp.com>

AER is only used with PCIe devices so we should only check PCIe devices for
_OSC support.

Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
---

 drivers/pci/pcie/aer/aerdrv_acpi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
index 1a1eb45..f685bf5 100644
--- a/drivers/pci/pcie/aer/aerdrv_acpi.c
+++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
@@ -50,7 +50,7 @@ int aer_osc_setup(struct pcie_device *pciedev)
 	}
 
 	if (handle) {
-		pci_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);
+		pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);
 		status = pci_osc_control_set(handle,
 					OSC_PCI_EXPRESS_AER_CONTROL |
 					OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);

[PATCH 3/4] ACPI: Check for any matching CID when walking namespace.

[Andrew Patterson]

From: Andrew Patterson <andrew.patterson@hp.com>

The callback function acpi_ns_get_device_callback called from
acpi_get_devices() will check CID's if the HID does not match.  This code
has a bug where it requires that all CIDs match the HID. Changed the code
so that any CID match will do.

Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
---

 drivers/acpi/namespace/nsxfeval.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/namespace/nsxfeval.c b/drivers/acpi/namespace/nsxfeval.c
index f39fbc6..e562b24 100644
--- a/drivers/acpi/namespace/nsxfeval.c
+++ b/drivers/acpi/namespace/nsxfeval.c
@@ -443,6 +443,7 @@ acpi_ns_get_device_callback(acpi_handle obj_handle,
 	struct acpica_device_id hid;
 	struct acpi_compatible_id_list *cid;
 	acpi_native_uint i;
+	int found;
 
 	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
 	if (ACPI_FAILURE(status)) {
@@ -496,16 +497,20 @@ acpi_ns_get_device_callback(acpi_handle obj_handle,
 
 			/* Walk the CID list */
 
+			found = 0;
 			for (i = 0; i < cid->count; i++) {
 				if (ACPI_STRNCMP(cid->id[i].value, info->hid,
 						 sizeof(struct
-							acpi_compatible_id)) !=
+							acpi_compatible_id)) ==
 				    0) {
-					ACPI_FREE(cid);
-					return (AE_OK);
+					found = 1;
+					break;
 				}
 			}
 			ACPI_FREE(cid);
+			if (!found) {
+				return (AE_OK);
+			}
 		}
 	}
 

[PATCH 4/4] PCI: Run ACPI _OSC method on root bridges only

[Andrew Patterson]

From: Andrew Patterson <andrew.patterson@hp.com>

According to the PCI Firmware Specification Revision 3.0 section 4.5, _OSC
should only be called on a root brdige.  Here is the relevant passage: "The
_OSC interface defined in this section applies only to Host Bridge ACPI
devices that originate PCI, PCI-X, or PCI Express hierarchies". Changed the
code to find the parent root bridge of the device and call _OSC on that.

Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
---

 drivers/pci/pcie/aer/aerdrv_acpi.c |   22 ++++++----------------
 1 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
index f685bf5..8c199ae 100644
--- a/drivers/pci/pcie/aer/aerdrv_acpi.c
+++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
@@ -31,23 +31,13 @@ int aer_osc_setup(struct pcie_device *pciedev)
 {
 	acpi_status status = AE_NOT_FOUND;
 	struct pci_dev *pdev = pciedev->port;
-	acpi_handle handle = DEVICE_ACPI_HANDLE(&pdev->dev);
-	struct pci_bus *parent;
+	acpi_handle handle = 0;
 
-	while (!handle) {
-		if (!pdev || !pdev->bus->parent)
-			break;
-		parent = pdev->bus->parent;
-		if (!parent->self)
-			/* Parent must be a host bridge */
-			handle = acpi_get_pci_rootbridge_handle(
-					pci_domain_nr(parent),
-					parent->number);
-		else
-			handle = DEVICE_ACPI_HANDLE(
-					&(parent->self->dev));
-		pdev = parent->self;
-	}
+	/* Find root host bridge */
+	while (pdev->bus && pdev->bus->self)
+		pdev = pdev->bus->self;
+	handle = acpi_get_pci_rootbridge_handle(
+		pci_domain_nr(pdev->bus), pdev->bus->number);
 
 	if (handle) {
 		pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);

Re: [PATCH 3/4] ACPI: Check for any matching CID when walking namespace.

[Greg KH]

On Tue, Jan 22, 2008 at 05:18:22PM -0700, Andrew Patterson wrote:
> From: Andrew Patterson <andrew.patterson@hp.com>
> 
> The callback function acpi_ns_get_device_callback called from
> acpi_get_devices() will check CID's if the HID does not match.  This code
> has a bug where it requires that all CIDs match the HID. Changed the code
> so that any CID match will do.
> 
> Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
> ---
> 
>  drivers/acpi/namespace/nsxfeval.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)

This should probably go through the ACPI tree, not the PCI tree, right?

thanks,

greg k-h

Re: [PATCH 3/4] ACPI: Check for any matching CID when walking namespace.

[Andrew Patterson]

On Wed, 2008-01-23 at 09:42 -0800, Greg KH wrote:
> On Tue, Jan 22, 2008 at 05:18:22PM -0700, Andrew Patterson wrote:
> > From: Andrew Patterson <andrew.patterson@hp.com>
> > 
> > The callback function acpi_ns_get_device_callback called from
> > acpi_get_devices() will check CID's if the HID does not match.  This code
> > has a bug where it requires that all CIDs match the HID. Changed the code
> > so that any CID match will do.
> > 
> > Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
> > ---
> > 
> >  drivers/acpi/namespace/nsxfeval.c |   11 ++++++++---
> >  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> This should probably go through the ACPI tree, not the PCI tree, right?
> 

Yes, but it is needed to get PCIe AER working (at least on the systems
that I have seen).  I suspect we will have the same issue with PCIe
hotplug. Len Brown is looking at it.

> thanks,
> 
> greg k-h

Re: [PATCH 3/4] ACPI: Check for any matching CID when walking namespace.

[Len Brown]

Right Greg, this is not only an ACPI patch, 
but a patch that touches the ACPICA core.
Lets get Bob to see if it applies to the latest upstream ACPICA code.

Andrew,
Do you give Intel permission to accept this patch under
both licenses on the top of the file, such that we can
re-distribute it in ACPICA to all the OSs that use ACPICA?

(hey, and check out the spiffy new acpica home page: http://acpica.org/ )

thanks,
-Len

On Tuesday 22 January 2008 19:18, Andrew Patterson wrote:
> From: Andrew Patterson <andrew.patterson@hp.com>
> 
> The callback function acpi_ns_get_device_callback called from
> acpi_get_devices() will check CID's if the HID does not match.  This code
> has a bug where it requires that all CIDs match the HID. Changed the code
> so that any CID match will do.
> 
> Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
> ---
> 
>  drivers/acpi/namespace/nsxfeval.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/namespace/nsxfeval.c b/drivers/acpi/namespace/nsxfeval.c
> index f39fbc6..e562b24 100644
> --- a/drivers/acpi/namespace/nsxfeval.c
> +++ b/drivers/acpi/namespace/nsxfeval.c
> @@ -443,6 +443,7 @@ acpi_ns_get_device_callback(acpi_handle obj_handle,
>  	struct acpica_device_id hid;
>  	struct acpi_compatible_id_list *cid;
>  	acpi_native_uint i;
> +	int found;
>  
>  	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
>  	if (ACPI_FAILURE(status)) {
> @@ -496,16 +497,20 @@ acpi_ns_get_device_callback(acpi_handle obj_handle,
>  
>  			/* Walk the CID list */
>  
> +			found = 0;
>  			for (i = 0; i < cid->count; i++) {
>  				if (ACPI_STRNCMP(cid->id[i].value, info->hid,
>  						 sizeof(struct
> -							acpi_compatible_id)) !=
> +							acpi_compatible_id)) ==
>  				    0) {
> -					ACPI_FREE(cid);
> -					return (AE_OK);
> +					found = 1;
> +					break;
>  				}
>  			}
>  			ACPI_FREE(cid);
> +			if (!found) {
> +				return (AE_OK);
> +			}
>  		}
>  	}
>  
> 

Re: [PATCH 3/4] ACPI: Check for any matching CID when walking namespace.

[Andrew Patterson]

On Wed, 2008-01-23 at 14:39 -0500, Len Brown wrote:
> Right Greg, this is not only an ACPI patch, 
> but a patch that touches the ACPICA core.
> Lets get Bob to see if it applies to the latest upstream ACPICA code.
> 
> Andrew,
> Do you give Intel permission to accept this patch under
> both licenses on the top of the file, such that we can
> re-distribute it in ACPICA to all the OSs that use ACPICA?
> 

Yes, by all means.

Andrew Patterson
Hewlett-Packard

> (hey, and check out the spiffy new acpica home page: http://acpica.org/ )
> 
> thanks,
> -Len
> 
> On Tuesday 22 January 2008 19:18, Andrew Patterson wrote:
> > From: Andrew Patterson <andrew.patterson@hp.com>
> > 
> > The callback function acpi_ns_get_device_callback called from
> > acpi_get_devices() will check CID's if the HID does not match.  This code
> > has a bug where it requires that all CIDs match the HID. Changed the code
> > so that any CID match will do.
> > 
> > Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
> > ---
> > 
> >  drivers/acpi/namespace/nsxfeval.c |   11 ++++++++---
> >  1 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/acpi/namespace/nsxfeval.c b/drivers/acpi/namespace/nsxfeval.c
> > index f39fbc6..e562b24 100644
> > --- a/drivers/acpi/namespace/nsxfeval.c
> > +++ b/drivers/acpi/namespace/nsxfeval.c
> > @@ -443,6 +443,7 @@ acpi_ns_get_device_callback(acpi_handle obj_handle,
> >  	struct acpica_device_id hid;
> >  	struct acpi_compatible_id_list *cid;
> >  	acpi_native_uint i;
> > +	int found;
> >  
> >  	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
> >  	if (ACPI_FAILURE(status)) {
> > @@ -496,16 +497,20 @@ acpi_ns_get_device_callback(acpi_handle obj_handle,
> >  
> >  			/* Walk the CID list */
> >  
> > +			found = 0;
> >  			for (i = 0; i < cid->count; i++) {
> >  				if (ACPI_STRNCMP(cid->id[i].value, info->hid,
> >  						 sizeof(struct
> > -							acpi_compatible_id)) !=
> > +							acpi_compatible_id)) ==
> >  				    0) {
> > -					ACPI_FREE(cid);
> > -					return (AE_OK);
> > +					found = 1;
> > +					break;
> >  				}
> >  			}
> >  			ACPI_FREE(cid);
> > +			if (!found) {
> > +				return (AE_OK);
> > +			}
> >  		}
> >  	}
> >  
> > 

RE: [PATCH 3/4] ACPI: Check for any matching CID when walking namespace.

[Moore, Robert]

Ok, I've rolled this in:

			/* Walk the CID list */

			found = FALSE;
			for (i = 0; i < cid->count; i++) {
				if (ACPI_STRNCMP (cid->id[i].value,
info->hid,
						sizeof (struct
acpi_compatible_id)) == 0) {

					/* Found a matching CID */

					found = TRUE;
					break;
				}
			}

			ACPI_FREE (cid);
			if (!found) {
				return (AE_OK);
			}
		}
	}

	/* We have a valid device, invoke the user function */

	status = info->user_function (obj_handle, nesting_level,
info->context,
			 return_value);
	return (status);
}


Thanks,
Bob


>-----Original Message-----
>From: Len Brown [mailto:lenb@kernel.org]
>Sent: Wednesday, January 23, 2008 11:40 AM
>To: Andrew Patterson; Moore, Robert
>Cc: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org;
>gregkh@suse.de; matthew@wil.cx; Nguyen, Tom L;
linux-acpi@vger.kernel.org
>Subject: Re: [PATCH 3/4] ACPI: Check for any matching CID when walking
>namespace.
>
>Right Greg, this is not only an ACPI patch,
>but a patch that touches the ACPICA core.
>Lets get Bob to see if it applies to the latest upstream ACPICA code.
>
>Andrew,
>Do you give Intel permission to accept this patch under
>both licenses on the top of the file, such that we can
>re-distribute it in ACPICA to all the OSs that use ACPICA?
>
>(hey, and check out the spiffy new acpica home page: http://acpica.org/
)
>
>thanks,
>-Len
>
>On Tuesday 22 January 2008 19:18, Andrew Patterson wrote:
>> From: Andrew Patterson <andrew.patterson@hp.com>
>>
>> The callback function acpi_ns_get_device_callback called from
>> acpi_get_devices() will check CID's if the HID does not match.  This
code
>> has a bug where it requires that all CIDs match the HID. Changed the
code
>> so that any CID match will do.
>>
>> Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
>> ---
>>
>>  drivers/acpi/namespace/nsxfeval.c |   11 ++++++++---
>>  1 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/namespace/nsxfeval.c
>b/drivers/acpi/namespace/nsxfeval.c
>> index f39fbc6..e562b24 100644
>> --- a/drivers/acpi/namespace/nsxfeval.c
>> +++ b/drivers/acpi/namespace/nsxfeval.c
>> @@ -443,6 +443,7 @@ acpi_ns_get_device_callback(acpi_handle
obj_handle,
>>  	struct acpica_device_id hid;
>>  	struct acpi_compatible_id_list *cid;
>>  	acpi_native_uint i;
>> +	int found;
>>
>>  	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
>>  	if (ACPI_FAILURE(status)) {
>> @@ -496,16 +497,20 @@ acpi_ns_get_device_callback(acpi_handle
obj_handle,
>>
>>  			/* Walk the CID list */
>>
>> +			found = 0;
>>  			for (i = 0; i < cid->count; i++) {
>>  				if (ACPI_STRNCMP(cid->id[i].value,
info->hid,
>>  						 sizeof(struct
>> -
acpi_compatible_id)) !=
>> +
acpi_compatible_id)) ==
>>  				    0) {
>> -					ACPI_FREE(cid);
>> -					return (AE_OK);
>> +					found = 1;
>> +					break;
>>  				}
>>  			}
>>  			ACPI_FREE(cid);
>> +			if (!found) {
>> +				return (AE_OK);
>> +			}
>>  		}
>>  	}
>>
>>