[PATCH 0/5] PCI root bridge: get bus info from _CRS

[PATCH 0/5] PCI root bridge: get bus info from _CRS

[Bjorn Helgaas]

The main patch in this series changes the way we discover the downstream bus
number of PCI root bridges.  The end result should be the same as the current
code (as far as I know, this doesn't fix any bugs).  But I believe this
follows the intent of the spec better, makes Linux work more like Windows,
and removes false accusations Linux makes against the BIOS.

---

Bjorn Helgaas (5):
      ACPI: pci_root: remove unused dev/fn information
      ACPI: pci_root: simplify list traversals
      ACPI: pci_root: use driver data rather than list lookup
      ACPI: pci_root: simplify acpi_pci_root_add() control flow
      ACPI: pci_root: check _CRS, then _BBN for downstream bus number

 drivers/acpi/pci_root.c |  185 ++++++++++++++---------------------------------
 1 files changed, 57 insertions(+), 128 deletions(-)

[PATCH 1/5] ACPI: pci_root: check _CRS, then _BBN for downstream bus number

[Bjorn Helgaas]

To find a host bridge's downstream bus number, we currently look at _BBN
first.  If _BBN returns a bus number we've already seen, we conclude that
_BBN was wrong and look for a bus number in _CRS.

However, the spec[1] (figure 5-5 and the example in sec 9.12.1) and an ACPI
FAQ[2] suggest that the OS should use _CRS to discover the bus number
range, and that _BBN is really intended to bootstrap _CRS methods that
reference PCI opregions.

This patch makes us always look at _CRS first.  If _CRS doesn't supply a
bus number, we look at _BBN.  If _BBN doesn't exist, we default to zero.
This makes the behavior consistent regardless of device discovery order.
Previously, if A and B had duplicate _BBNs and we found A first, we'd only
look at B's _CRS, whereas if we found B first, we'd only look at A's _CRS.

I'm told that Windows discovers host bridge bus numbers using _CRS, so
it should be fairly safe to rely on this BIOS functionality.

This patch also removes two misleading messages: we printed the "Wrong _BBN
value, reboot and use option 'pci=noacpi'" message before looking at _CRS,
so we would likely find the bus number in _CRS, the system would work fine,
and the user would be confused.  The "PCI _CRS %d overrides _BBN 0" message
incorrectly assumes _BBN was zero, and it's useless anyway because we
print the segment/bus number a few lines later.

References:
    [1] http://www.acpi.info/DOWNLOADS/ACPIspec30b.pdf
    [2] http://www.acpi.info/acpi_faq.htm _BBN/_CRS discussion
    http://download.microsoft.com/download/9/8/f/98f3fe47-dfc3-4e74-92a3-088782200fe7/TWAR05005_WinHEC05.ppt (slide 17)
    http://bugzilla.kernel.org/show_bug.cgi?id=1662 ASUS PR-DLS
    http://bugzilla.kernel.org/show_bug.cgi?id=1127 ASUS PR-DLSW
    http://bugzilla.kernel.org/show_bug.cgi?id=1741 ASUS PR-DLS533

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Reviewed-by: Alex Chiang <achiang@hp.com>
CC: Shaohua Li <shaohua.li@intel.com>
CC: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   54 ++++++++++++++---------------------------------
 1 files changed, 16 insertions(+), 38 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index f341b07..7847732 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -470,12 +470,12 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 {
 	int result = 0;
 	struct acpi_pci_root *root = NULL;
-	struct acpi_pci_root *tmp;
 	acpi_status status = AE_OK;
 	unsigned long long value = 0;
 	acpi_handle handle = NULL;
 	struct acpi_device *child;
 	u32 flags, base_flags;
+	int bus;
 
 
 	if (!device)
@@ -523,46 +523,24 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	/* 
 	 * Bus
 	 * ---
-	 * Obtained via _BBN, if exists, otherwise assumed to be zero (0).
+	 * Check _CRS first, then _BBN.  If no _BBN, default to zero.
 	 */
-	status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN, NULL,
-				       &value);
-	switch (status) {
-	case AE_OK:
-		root->id.bus = (u16) value;
-		break;
-	case AE_NOT_FOUND:
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Assuming bus 0 (no _BBN)\n"));
-		root->id.bus = 0;
-		break;
-	default:
-		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BBN"));
-		result = -ENODEV;
-		goto end;
-	}
-
-	/* Some systems have wrong _BBN */
-	list_for_each_entry(tmp, &acpi_pci_roots, node) {
-		if ((tmp->id.segment == root->id.segment)
-		    && (tmp->id.bus == root->id.bus)) {
-			int bus = 0;
-			acpi_status status;
-
-			printk(KERN_ERR PREFIX
-				    "Wrong _BBN value, reboot"
-				    " and use option 'pci=noacpi'\n");
-
-			status = try_get_root_bridge_busnr(device->handle, &bus);
-			if (ACPI_FAILURE(status))
-				break;
-			if (bus != root->id.bus) {
-				printk(KERN_INFO PREFIX
-				       "PCI _CRS %d overrides _BBN 0\n", bus);
-				root->id.bus = bus;
-			}
-			break;
+	status = try_get_root_bridge_busnr(device->handle, &bus);
+	if (ACPI_SUCCESS(status))
+		root->id.bus = bus;
+	else {
+		status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN,					       NULL, &value);
+		if (ACPI_SUCCESS(status))
+			root->id.bus = (u16) value;
+		else if (status == AE_NOT_FOUND)
+			root->id.bus = 0;
+		else {
+			ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BBN"));
+			result = -ENODEV;
+			goto end;
 		}
 	}
+
 	/*
 	 * Device & Function
 	 * -----------------

Re: [PATCH 1/5] ACPI: pci_root: check _CRS, then _BBN for downstream bus number

[Kenji Kaneshige]

Bjorn Helgaas wrote:
> To find a host bridge's downstream bus number, we currently look at _BBN
> first.  If _BBN returns a bus number we've already seen, we conclude that
> _BBN was wrong and look for a bus number in _CRS.
> 
> However, the spec[1] (figure 5-5 and the example in sec 9.12.1) and an ACPI
> FAQ[2] suggest that the OS should use _CRS to discover the bus number
> range, and that _BBN is really intended to bootstrap _CRS methods that
> reference PCI opregions.
> 
> This patch makes us always look at _CRS first.  If _CRS doesn't supply a
> bus number, we look at _BBN.  If _BBN doesn't exist, we default to zero.
> This makes the behavior consistent regardless of device discovery order.
> Previously, if A and B had duplicate _BBNs and we found A first, we'd only
> look at B's _CRS, whereas if we found B first, we'd only look at A's _CRS.
> 
> I'm told that Windows discovers host bridge bus numbers using _CRS, so
> it should be fairly safe to rely on this BIOS functionality.
> 
> This patch also removes two misleading messages: we printed the "Wrong _BBN
> value, reboot and use option 'pci=noacpi'" message before looking at _CRS,
> so we would likely find the bus number in _CRS, the system would work fine,
> and the user would be confused.  The "PCI _CRS %d overrides _BBN 0" message
> incorrectly assumes _BBN was zero, and it's useless anyway because we
> print the segment/bus number a few lines later.

Though it might be very stupid question, can I ask you a question?

Is minimum value of PCI bus number range reported by _CRS always
equal to the bus number of the PCI root bus? I glanced over the
specs, but I could not confirm it.

Thanks,
Kenji Kaneshige


> 
> References:
>     [1] http://www.acpi.info/DOWNLOADS/ACPIspec30b.pdf
>     [2] http://www.acpi.info/acpi_faq.htm _BBN/_CRS discussion
>     http://download.microsoft.com/download/9/8/f/98f3fe47-dfc3-4e74-92a3-088782200fe7/TWAR05005_WinHEC05.ppt (slide 17)
>     http://bugzilla.kernel.org/show_bug.cgi?id=1662 ASUS PR-DLS
>     http://bugzilla.kernel.org/show_bug.cgi?id=1127 ASUS PR-DLSW
>     http://bugzilla.kernel.org/show_bug.cgi?id=1741 ASUS PR-DLS533
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Reviewed-by: Alex Chiang <achiang@hp.com>
> CC: Shaohua Li <shaohua.li@intel.com>
> CC: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> ---
>  drivers/acpi/pci_root.c |   54 ++++++++++++++---------------------------------
>  1 files changed, 16 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index f341b07..7847732 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -470,12 +470,12 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  {
>  	int result = 0;
>  	struct acpi_pci_root *root = NULL;
> -	struct acpi_pci_root *tmp;
>  	acpi_status status = AE_OK;
>  	unsigned long long value = 0;
>  	acpi_handle handle = NULL;
>  	struct acpi_device *child;
>  	u32 flags, base_flags;
> +	int bus;
>  
>  
>  	if (!device)
> @@ -523,46 +523,24 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  	/* 
>  	 * Bus
>  	 * ---
> -	 * Obtained via _BBN, if exists, otherwise assumed to be zero (0).
> +	 * Check _CRS first, then _BBN.  If no _BBN, default to zero.
>  	 */
> -	status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN, NULL,
> -				       &value);
> -	switch (status) {
> -	case AE_OK:
> -		root->id.bus = (u16) value;
> -		break;
> -	case AE_NOT_FOUND:
> -		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Assuming bus 0 (no _BBN)\n"));
> -		root->id.bus = 0;
> -		break;
> -	default:
> -		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BBN"));
> -		result = -ENODEV;
> -		goto end;
> -	}
> -
> -	/* Some systems have wrong _BBN */
> -	list_for_each_entry(tmp, &acpi_pci_roots, node) {
> -		if ((tmp->id.segment == root->id.segment)
> -		    && (tmp->id.bus == root->id.bus)) {
> -			int bus = 0;
> -			acpi_status status;
> -
> -			printk(KERN_ERR PREFIX
> -				    "Wrong _BBN value, reboot"
> -				    " and use option 'pci=noacpi'\n");
> -
> -			status = try_get_root_bridge_busnr(device->handle, &bus);
> -			if (ACPI_FAILURE(status))
> -				break;
> -			if (bus != root->id.bus) {
> -				printk(KERN_INFO PREFIX
> -				       "PCI _CRS %d overrides _BBN 0\n", bus);
> -				root->id.bus = bus;
> -			}
> -			break;
> +	status = try_get_root_bridge_busnr(device->handle, &bus);
> +	if (ACPI_SUCCESS(status))
> +		root->id.bus = bus;
> +	else {
> +		status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN,					       NULL, &value);
> +		if (ACPI_SUCCESS(status))
> +			root->id.bus = (u16) value;
> +		else if (status == AE_NOT_FOUND)
> +			root->id.bus = 0;
> +		else {
> +			ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BBN"));
> +			result = -ENODEV;
> +			goto end;
>  		}
>  	}
> +
>  	/*
>  	 * Device & Function
>  	 * -----------------
> 
> 
> 

[PATCH 2/5] ACPI: pci_root: simplify acpi_pci_root_add() control flow

[Bjorn Helgaas]

By looking up the segment & bus number earlier, we don't have to
worry about cleaning up if it fails.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/pci_root.c |  100 ++++++++++++++++++-----------------------------
 1 files changed, 38 insertions(+), 62 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 7847732..c780008 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -185,19 +185,22 @@ get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
 	return AE_OK;
 }
 
-static acpi_status try_get_root_bridge_busnr(acpi_handle handle, int *busnum)
+static acpi_status try_get_root_bridge_busnr(acpi_handle handle,
+					     unsigned long long *bus)
 {
 	acpi_status status;
+	int busnum;
 
-	*busnum = -1;
+	busnum = -1;
 	status =
 	    acpi_walk_resources(handle, METHOD_NAME__CRS,
-				get_root_bridge_busnr_callback, busnum);
+				get_root_bridge_busnr_callback, &busnum);
 	if (ACPI_FAILURE(status))
 		return status;
 	/* Check if we really get a bus number from _CRS */
-	if (*busnum == -1)
+	if (busnum == -1)
 		return AE_ERROR;
+	*bus = busnum;
 	return AE_OK;
 }
 
@@ -468,24 +471,39 @@ EXPORT_SYMBOL(acpi_pci_osc_control_set);
 
 static int __devinit acpi_pci_root_add(struct acpi_device *device)
 {
-	int result = 0;
-	struct acpi_pci_root *root = NULL;
-	acpi_status status = AE_OK;
-	unsigned long long value = 0;
-	acpi_handle handle = NULL;
+	unsigned long long segment, bus;
+	acpi_status status;
+	int result;
+	struct acpi_pci_root *root;
+	acpi_handle handle;
 	struct acpi_device *child;
 	u32 flags, base_flags;
-	int bus;
 
+	segment = 0;
+	status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL,
+				       &segment);
+	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
+		printk(KERN_ERR PREFIX "can't evaluate _SEG\n");
+		return -ENODEV;
+	}
 
-	if (!device)
-		return -EINVAL;
+	/* Check _CRS first, then _BBN.  If no _BBN, default to zero. */
+	bus = 0;
+	status = try_get_root_bridge_busnr(device->handle, &bus);
+	if (ACPI_FAILURE(status)) {
+		status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN,					       NULL, &bus);
+		if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
+			printk(KERN_ERR PREFIX
+			     "no bus number in _CRS and can't evaluate _BBN\n");
+			return -ENODEV;
+		}
+	}
 
 	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
 	if (!root)
 		return -ENOMEM;
-	INIT_LIST_HEAD(&root->node);
 
+	INIT_LIST_HEAD(&root->node);
 	root->device = device;
 	strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
@@ -498,54 +516,13 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	flags = base_flags = OSC_PCI_SEGMENT_GROUPS_SUPPORT;
 	acpi_pci_osc_support(root, flags);
 
-	/* 
-	 * Segment
-	 * -------
-	 * Obtained via _SEG, if exists, otherwise assumed to be zero (0).
-	 */
-	status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL,
-				       &value);
-	switch (status) {
-	case AE_OK:
-		root->id.segment = (u16) value;
-		break;
-	case AE_NOT_FOUND:
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-				  "Assuming segment 0 (no _SEG)\n"));
-		root->id.segment = 0;
-		break;
-	default:
-		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _SEG"));
-		result = -ENODEV;
-		goto end;
-	}
-
-	/* 
-	 * Bus
-	 * ---
-	 * Check _CRS first, then _BBN.  If no _BBN, default to zero.
-	 */
-	status = try_get_root_bridge_busnr(device->handle, &bus);
-	if (ACPI_SUCCESS(status))
-		root->id.bus = bus;
-	else {
-		status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN,					       NULL, &value);
-		if (ACPI_SUCCESS(status))
-			root->id.bus = (u16) value;
-		else if (status == AE_NOT_FOUND)
-			root->id.bus = 0;
-		else {
-			ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BBN"));
-			result = -ENODEV;
-			goto end;
-		}
-	}
-
 	/*
 	 * Device & Function
 	 * -----------------
 	 * Obtained from _ADR (which has already been evaluated for us).
 	 */
+	root->id.segment = segment & 0xFFFF;
+	root->id.bus = bus & 0xFF;
 	root->id.device = device->pnp.bus_address >> 16;
 	root->id.function = device->pnp.bus_address & 0xFFFF;
 
@@ -611,13 +588,12 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	if (flags != base_flags)
 		acpi_pci_osc_support(root, flags);
 
-      end:
-	if (result) {
-		if (!list_empty(&root->node))
-			list_del(&root->node);
-		kfree(root);
-	}
+	return 0;
 
+end:
+	if (!list_empty(&root->node))
+		list_del(&root->node);
+	kfree(root);
 	return result;
 }
 

[PATCH 3/5] ACPI: pci_root: use driver data rather than list lookup

[Bjorn Helgaas]

There's no need to search the list to find the acpi_pci_root
structure.  We saved it as device->driver_data when we added
the device.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/pci_root.c |   21 ++++-----------------
 1 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index c780008..e490a2e 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -599,30 +599,17 @@ end:
 
 static int acpi_pci_root_start(struct acpi_device *device)
 {
-	struct acpi_pci_root *root;
-
+	struct acpi_pci_root *root = acpi_driver_data(device);
 
-	list_for_each_entry(root, &acpi_pci_roots, node) {
-		if (root->device == device) {
-			pci_bus_add_devices(root->bus);
-			return 0;
-		}
-	}
-	return -ENODEV;
+	pci_bus_add_devices(root->bus);
+	return 0;
 }
 
 static int acpi_pci_root_remove(struct acpi_device *device, int type)
 {
-	struct acpi_pci_root *root = NULL;
-
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	root = acpi_driver_data(device);
+	struct acpi_pci_root *root = acpi_driver_data(device);
 
 	kfree(root);
-
 	return 0;
 }
 

[PATCH 4/5] ACPI: pci_root: simplify list traversals

[Bjorn Helgaas]

Using list_for_each_entry() makes traversing the root list easier.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Reviewed-by: Alex Chiang <achiang@hp.com>
---
 drivers/acpi/pci_root.c |   23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index e490a2e..d9d34e2 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -82,7 +82,7 @@ static DEFINE_MUTEX(osc_lock);
 int acpi_pci_register_driver(struct acpi_pci_driver *driver)
 {
 	int n = 0;
-	struct list_head *entry;
+	struct acpi_pci_root *root;
 
 	struct acpi_pci_driver **pptr = &sub_driver;
 	while (*pptr)
@@ -92,9 +92,7 @@ int acpi_pci_register_driver(struct acpi_pci_driver *driver)
 	if (!driver->add)
 		return 0;
 
-	list_for_each(entry, &acpi_pci_roots) {
-		struct acpi_pci_root *root;
-		root = list_entry(entry, struct acpi_pci_root, node);
+	list_for_each_entry(root, &acpi_pci_roots, node) {
 		driver->add(root->device->handle);
 		n++;
 	}
@@ -106,7 +104,7 @@ EXPORT_SYMBOL(acpi_pci_register_driver);
 
 void acpi_pci_unregister_driver(struct acpi_pci_driver *driver)
 {
-	struct list_head *entry;
+	struct acpi_pci_root *root;
 
 	struct acpi_pci_driver **pptr = &sub_driver;
 	while (*pptr) {
@@ -120,23 +118,19 @@ void acpi_pci_unregister_driver(struct acpi_pci_driver *driver)
 	if (!driver->remove)
 		return;
 
-	list_for_each(entry, &acpi_pci_roots) {
-		struct acpi_pci_root *root;
-		root = list_entry(entry, struct acpi_pci_root, node);
+	list_for_each_entry(root, &acpi_pci_roots, node)
 		driver->remove(root->device->handle);
-	}
 }
 
 EXPORT_SYMBOL(acpi_pci_unregister_driver);
 
 acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
 {
-	struct acpi_pci_root *tmp;
+	struct acpi_pci_root *root;
 	
-	list_for_each_entry(tmp, &acpi_pci_roots, node) {
-		if ((tmp->id.segment == (u16) seg) && (tmp->id.bus == (u16) bus))
-			return tmp->device->handle;
-	}
+	list_for_each_entry(root, &acpi_pci_roots, node)
+		if ((root->id.segment == (u16) seg) && (root->id.bus == (u16) bus))
+			return root->device->handle;
 	return NULL;		
 }
 
@@ -325,6 +319,7 @@ static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
 static struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
 {
 	struct acpi_pci_root *root;
+
 	list_for_each_entry(root, &acpi_pci_roots, node) {
 		if (root->device->handle == handle)
 			return root;

[PATCH 5/5] ACPI: pci_root: remove unused dev/fn information

[Bjorn Helgaas]

We never use the PCI device & function number, so remove it to make
it clear that it's not needed.  Many PCI host bridges don't even
appear in config space, so it's meaningless to look at stuff from
_ADR, which doesn't exist in that case.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Reviewed-by: Alex Chiang <achiang@hp.com>
---
 drivers/acpi/pci_root.c |   25 +++++++++----------------
 1 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index d9d34e2..8a5bf3b 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -63,9 +63,10 @@ static struct acpi_driver acpi_pci_root_driver = {
 
 struct acpi_pci_root {
 	struct list_head node;
-	struct acpi_device * device;
-	struct acpi_pci_id id;
+	struct acpi_device *device;
 	struct pci_bus *bus;
+	u16 segment;
+	u8 bus_nr;
 
 	u32 osc_support_set;	/* _OSC state of support bits */
 	u32 osc_control_set;	/* _OSC state of control bits */
@@ -129,7 +130,7 @@ acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
 	struct acpi_pci_root *root;
 	
 	list_for_each_entry(root, &acpi_pci_roots, node)
-		if ((root->id.segment == (u16) seg) && (root->id.bus == (u16) bus))
+		if ((root->segment == (u16) seg) && (root->bus_nr == (u16) bus))
 			return root->device->handle;
 	return NULL;		
 }
@@ -500,6 +501,8 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 
 	INIT_LIST_HEAD(&root->node);
 	root->device = device;
+	root->segment = segment & 0xFFFF;
+	root->bus_nr = bus & 0xFF;
 	strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
 	device->driver_data = root;
@@ -512,16 +515,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	acpi_pci_osc_support(root, flags);
 
 	/*
-	 * Device & Function
-	 * -----------------
-	 * Obtained from _ADR (which has already been evaluated for us).
-	 */
-	root->id.segment = segment & 0xFFFF;
-	root->id.bus = bus & 0xFF;
-	root->id.device = device->pnp.bus_address >> 16;
-	root->id.function = device->pnp.bus_address & 0xFFFF;
-
-	/*
 	 * TBD: Need PCI interface for enumeration/configuration of roots.
 	 */
 
@@ -530,7 +523,7 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 
 	printk(KERN_INFO PREFIX "%s [%s] (%04x:%02x)\n",
 	       acpi_device_name(device), acpi_device_bid(device),
-	       root->id.segment, root->id.bus);
+	       root->segment, root->bus_nr);
 
 	/*
 	 * Scan the Root Bridge
@@ -539,11 +532,11 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	 * PCI namespace does not get created until this call is made (and 
 	 * thus the root bridge's pci_dev does not exist).
 	 */
-	root->bus = pci_acpi_scan_root(device, root->id.segment, root->id.bus);
+	root->bus = pci_acpi_scan_root(device, segment, bus);
 	if (!root->bus) {
 		printk(KERN_ERR PREFIX
 			    "Bus %04x:%02x not present in PCI namespace\n",
-			    root->id.segment, root->id.bus);
+			    root->segment, root->bus_nr);
 		result = -ENODEV;
 		goto end;
 	}

Re: [PATCH 0/5] PCI root bridge: get bus info from _CRS

[Len Brown]

series applied to acpi-test

thanks,
Len Brown, Intel Open Source Technology Center