[PATCH v3 0/2] pci_root: track downstream bus range

[PATCH v3 0/2] pci_root: track downstream bus range

[Bjorn Helgaas]

These patches keep track of both the beginning and end of the bus number
range under a PCI host bridge (previously we only kept the beginning) and
make that range available to the arch code, which will need it to support
the _CBA mechanism for reporting MMCONFIG regions.

These apply on the acpi-test tree.

---

Bjorn Helgaas (2):
      ACPI: pci_root: save downstream bus range
      ACPI: pci_root: pass acpi_pci_root to arch-specific scan


 arch/ia64/pci/pci.c         |    5 +++
 arch/x86/pci/acpi.c         |    5 +++
 drivers/acpi/pci_root.c     |   67 ++++++++++++++++++++++++++-----------------
 include/acpi/acpi_bus.h     |    2 +
 include/acpi/acpi_drivers.h |    3 +-
 5 files changed, 50 insertions(+), 32 deletions(-)

[PATCH v3 1/2] ACPI: pci_root: save downstream bus range

[Bjorn Helgaas]

Previously, we only saved the root bus number, i.e., the beginning of the
downstream bus range.  We now support IORESOURCE_BUS resources, so this
patch uses that to keep track of both the beginning and the end of the
downstream bus range.

It's important to know both the beginning and the end for supporting _CBA
(see PCI Firmware spec, rev 3.0, sec 4.1.3) and so we know the limits for
any possible PCI bus renumbering (we can't renumber downstream buses to be
outside the bus number range claimed by the host bridge).

It's clear from the spec that the bus range is supposed to be in _CRS, but
if we don't find it there, we'll assume [_BBN - 0xFF].

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---

 drivers/acpi/pci_root.c |   67 ++++++++++++++++++++++++++++-------------------
 include/acpi/acpi_bus.h |    2 +
 2 files changed, 41 insertions(+), 28 deletions(-)


diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index d724736..e309f56 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -119,7 +119,8 @@ 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->segment == (u16) seg) && (root->bus_nr == (u16) bus))
+		if ((root->segment == (u16) seg) &&
+		    (root->secondary.start == (u16) bus))
 			return root->device->handle;
 	return NULL;		
 }
@@ -153,7 +154,7 @@ EXPORT_SYMBOL_GPL(acpi_is_root_bridge);
 static acpi_status
 get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
 {
-	int *busnr = data;
+	struct resource *res = data;
 	struct acpi_resource_address64 address;
 
 	if (resource->type != ACPI_RESOURCE_TYPE_ADDRESS16 &&
@@ -163,28 +164,27 @@ get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
 
 	acpi_resource_to_address64(resource, &address);
 	if ((address.address_length > 0) &&
-	    (address.resource_type == ACPI_BUS_NUMBER_RANGE))
-		*busnr = address.minimum;
+	    (address.resource_type == ACPI_BUS_NUMBER_RANGE)) {
+		res->start = address.minimum;
+		res->end = address.minimum + address.address_length - 1;
+	}
 
 	return AE_OK;
 }
 
 static acpi_status try_get_root_bridge_busnr(acpi_handle handle,
-					     unsigned long long *bus)
+					     struct resource *res)
 {
 	acpi_status status;
-	int busnum;
 
-	busnum = -1;
+	res->start = -1;
 	status =
 	    acpi_walk_resources(handle, METHOD_NAME__CRS,
-				get_root_bridge_busnr_callback, &busnum);
+				get_root_bridge_busnr_callback, res);
 	if (ACPI_FAILURE(status))
 		return status;
-	/* Check if we really get a bus number from _CRS */
-	if (busnum == -1)
+	if (res->start == -1)
 		return AE_ERROR;
-	*bus = busnum;
 	return AE_OK;
 }
 
@@ -428,34 +428,47 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	struct acpi_device *child;
 	u32 flags, base_flags;
 
+	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
+	if (!root)
+		return -ENOMEM;
+
 	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;
+		result = -ENODEV;
+		goto end;
 	}
 
 	/* Check _CRS first, then _BBN.  If no _BBN, default to zero. */
-	bus = 0;
-	status = try_get_root_bridge_busnr(device->handle, &bus);
+	root->secondary.flags = IORESOURCE_BUS;
+	root->secondary.start = 0;
+	root->secondary.end = 0xFF;
+	status = try_get_root_bridge_busnr(device->handle, &root->secondary);
 	if (ACPI_FAILURE(status)) {
+		/*
+		 * We need both the start and end of the downstream bus range
+		 * to interpret _CBA (MMCONFIG base address), so it really is
+		 * supposed to be in _CRS.
+		 */
+		printk(KERN_WARNING FW_BUG PREFIX
+		       "no secondary bus range in _CRS\n");
 		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;
+		if (ACPI_SUCCESS(status))
+			root->secondary.start = bus;
+		else if (status == AE_NOT_FOUND)
+			root->secondary.start = 0;
+		else {
+			printk(KERN_ERR PREFIX "can't evaluate _BBN\n");
+			result = -ENODEV;
+			goto end;
 		}
 	}
 
-	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
-	if (!root)
-		return -ENOMEM;
-
 	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;
@@ -474,9 +487,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	/* TBD: Locking */
 	list_add_tail(&root->node, &acpi_pci_roots);
 
-	printk(KERN_INFO PREFIX "%s [%s] (%04x:%02x)\n",
+	printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
 	       acpi_device_name(device), acpi_device_bid(device),
-	       root->segment, root->bus_nr);
+	       root->segment, &root->secondary);
 
 	/*
 	 * Scan the Root Bridge
@@ -485,11 +498,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, segment, bus);
+	root->bus = pci_acpi_scan_root(device, segment, root->secondary.start);
 	if (!root->bus) {
 		printk(KERN_ERR PREFIX
 			    "Bus %04x:%02x not present in PCI namespace\n",
-			    root->segment, root->bus_nr);
+			    root->segment, (unsigned int)root->secondary.start);
 		result = -ENODEV;
 		goto end;
 	}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 7bf83dd..baacd98 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -373,7 +373,7 @@ struct acpi_pci_root {
 	struct acpi_pci_id id;
 	struct pci_bus *bus;
 	u16 segment;
-	u8 bus_nr;
+	struct resource secondary;	/* downstream bus range */
 
 	u32 osc_support_set;	/* _OSC state of support bits */
 	u32 osc_control_set;	/* _OSC state of control bits */

[PATCH v3 2/2] ACPI: pci_root: pass acpi_pci_root to arch-specific scan

[Bjorn Helgaas]

The acpi_pci_root structure contains all the individual items (acpi_device,
domain, bus number) we pass to pci_acpi_scan_root(), so just pass the
single acpi_pci_root pointer directly.

This will make it easier to add _CBA support later.  For _CBA, we need the
entire downstream bus range, not just the base bus number.  We have that in
the acpi_pci_root structure, so passing the pointer makes it available to
the arch-specific code.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---

 arch/ia64/pci/pci.c         |    5 ++++-
 arch/x86/pci/acpi.c         |    5 ++++-
 drivers/acpi/pci_root.c     |    2 +-
 include/acpi/acpi_drivers.h |    3 +--
 4 files changed, 10 insertions(+), 5 deletions(-)


diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 64aff52..aa2533a 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -335,8 +335,11 @@ pcibios_setup_root_windows(struct pci_bus *bus, struct pci_controller *ctrl)
 }
 
 struct pci_bus * __devinit
-pci_acpi_scan_root(struct acpi_device *device, int domain, int bus)
+pci_acpi_scan_root(struct acpi_pci_root *root)
 {
+	struct acpi_device *device = root->device;
+	int domain = root->segment;
+	int bus = root->secondary.start;
 	struct pci_controller *controller;
 	unsigned int windows = 0;
 	struct pci_bus *pbus;
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 5f11ff6..0c36ad5 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -215,8 +215,11 @@ res_alloc_fail:
 	return;
 }
 
-struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_device *device, int domain, int busnum)
+struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
 {
+	struct acpi_device *device = root->device;
+	int domain = root->segment;
+	int busnum = root->secondary.start;
 	struct pci_bus *bus;
 	struct pci_sysdata *sd;
 	int node;
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index e309f56..7f554a4 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -498,7 +498,7 @@ 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, segment, root->secondary.start);
+	root->bus = pci_acpi_scan_root(root);
 	if (!root->bus) {
 		printk(KERN_ERR PREFIX
 			    "Bus %04x:%02x not present in PCI namespace\n",
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 3a4767c..16903b3 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -102,8 +102,7 @@ int acpi_pci_bind_root(struct acpi_device *device);
 
 /* Arch-defined function to add a bus to the system */
 
-struct pci_bus *pci_acpi_scan_root(struct acpi_device *device, int domain,
-				   int bus);
+struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root);
 void pci_acpi_crs_quirks(void);
 
 /* --------------------------------------------------------------------------

Re: [PATCH v3 1/2] ACPI: pci_root: save downstream bus range

[Kenji Kaneshige]

Looks good to me. But one nitpicking.

> +	root->secondary.flags = IORESOURCE_BUS;
> +	root->secondary.start = 0;
> +	root->secondary.end = 0xFF;
> +	status = try_get_root_bridge_busnr(device->handle, &root->secondary);
>  	if (ACPI_FAILURE(status)) {

I feel a little strange about initializing 'start' and 'end' fields
here because those are changed in try_get_root_bridge_busnr().
What about the below?

	root->secondary.flags = IORESOURCE_BUS;
	status = try_get_root_bridge_busnr(device->handle, &root->secondary);
	if (ACPI_FAILURE(status)) {
		root->secondary = 0xFF;
		...

Reviewed-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

Thanks,
Kenji Kaneshige


Bjorn Helgaas wrote:
> Previously, we only saved the root bus number, i.e., the beginning of the
> downstream bus range.  We now support IORESOURCE_BUS resources, so this
> patch uses that to keep track of both the beginning and the end of the
> downstream bus range.
> 
> It's important to know both the beginning and the end for supporting _CBA
> (see PCI Firmware spec, rev 3.0, sec 4.1.3) and so we know the limits for
> any possible PCI bus renumbering (we can't renumber downstream buses to be
> outside the bus number range claimed by the host bridge).
> 
> It's clear from the spec that the bus range is supposed to be in _CRS, but
> if we don't find it there, we'll assume [_BBN - 0xFF].
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> ---
> 
>  drivers/acpi/pci_root.c |   67 ++++++++++++++++++++++++++++-------------------
>  include/acpi/acpi_bus.h |    2 +
>  2 files changed, 41 insertions(+), 28 deletions(-)
> 
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index d724736..e309f56 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -119,7 +119,8 @@ 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->segment == (u16) seg) && (root->bus_nr == (u16) bus))
> +		if ((root->segment == (u16) seg) &&
> +		    (root->secondary.start == (u16) bus))
>  			return root->device->handle;
>  	return NULL;		
>  }
> @@ -153,7 +154,7 @@ EXPORT_SYMBOL_GPL(acpi_is_root_bridge);
>  static acpi_status
>  get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
>  {
> -	int *busnr = data;
> +	struct resource *res = data;
>  	struct acpi_resource_address64 address;
>  
>  	if (resource->type != ACPI_RESOURCE_TYPE_ADDRESS16 &&
> @@ -163,28 +164,27 @@ get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
>  
>  	acpi_resource_to_address64(resource, &address);
>  	if ((address.address_length > 0) &&
> -	    (address.resource_type == ACPI_BUS_NUMBER_RANGE))
> -		*busnr = address.minimum;
> +	    (address.resource_type == ACPI_BUS_NUMBER_RANGE)) {
> +		res->start = address.minimum;
> +		res->end = address.minimum + address.address_length - 1;
> +	}
>  
>  	return AE_OK;
>  }
>  
>  static acpi_status try_get_root_bridge_busnr(acpi_handle handle,
> -					     unsigned long long *bus)
> +					     struct resource *res)
>  {
>  	acpi_status status;
> -	int busnum;
>  
> -	busnum = -1;
> +	res->start = -1;
>  	status =
>  	    acpi_walk_resources(handle, METHOD_NAME__CRS,
> -				get_root_bridge_busnr_callback, &busnum);
> +				get_root_bridge_busnr_callback, res);
>  	if (ACPI_FAILURE(status))
>  		return status;
> -	/* Check if we really get a bus number from _CRS */
> -	if (busnum == -1)
> +	if (res->start == -1)
>  		return AE_ERROR;
> -	*bus = busnum;
>  	return AE_OK;
>  }
>  
> @@ -428,34 +428,47 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  	struct acpi_device *child;
>  	u32 flags, base_flags;
>  
> +	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
> +	if (!root)
> +		return -ENOMEM;
> +
>  	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;
> +		result = -ENODEV;
> +		goto end;
>  	}
>  
>  	/* Check _CRS first, then _BBN.  If no _BBN, default to zero. */
> -	bus = 0;
> -	status = try_get_root_bridge_busnr(device->handle, &bus);
> +	root->secondary.flags = IORESOURCE_BUS;
> +	root->secondary.start = 0;
> +	root->secondary.end = 0xFF;
> +	status = try_get_root_bridge_busnr(device->handle, &root->secondary);
>  	if (ACPI_FAILURE(status)) {
> +		/*
> +		 * We need both the start and end of the downstream bus range
> +		 * to interpret _CBA (MMCONFIG base address), so it really is
> +		 * supposed to be in _CRS.
> +		 */
> +		printk(KERN_WARNING FW_BUG PREFIX
> +		       "no secondary bus range in _CRS\n");
>  		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;
> +		if (ACPI_SUCCESS(status))
> +			root->secondary.start = bus;
> +		else if (status == AE_NOT_FOUND)
> +			root->secondary.start = 0;
> +		else {
> +			printk(KERN_ERR PREFIX "can't evaluate _BBN\n");
> +			result = -ENODEV;
> +			goto end;
>  		}
>  	}
>  
> -	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
> -	if (!root)
> -		return -ENOMEM;
> -
>  	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;
> @@ -474,9 +487,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  	/* TBD: Locking */
>  	list_add_tail(&root->node, &acpi_pci_roots);
>  
> -	printk(KERN_INFO PREFIX "%s [%s] (%04x:%02x)\n",
> +	printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
>  	       acpi_device_name(device), acpi_device_bid(device),
> -	       root->segment, root->bus_nr);
> +	       root->segment, &root->secondary);
>  
>  	/*
>  	 * Scan the Root Bridge
> @@ -485,11 +498,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, segment, bus);
> +	root->bus = pci_acpi_scan_root(device, segment, root->secondary.start);
>  	if (!root->bus) {
>  		printk(KERN_ERR PREFIX
>  			    "Bus %04x:%02x not present in PCI namespace\n",
> -			    root->segment, root->bus_nr);
> +			    root->segment, (unsigned int)root->secondary.start);
>  		result = -ENODEV;
>  		goto end;
>  	}
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 7bf83dd..baacd98 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -373,7 +373,7 @@ struct acpi_pci_root {
>  	struct acpi_pci_id id;
>  	struct pci_bus *bus;
>  	u16 segment;
> -	u8 bus_nr;
> +	struct resource secondary;	/* downstream bus range */
>  
>  	u32 osc_support_set;	/* _OSC state of support bits */
>  	u32 osc_control_set;	/* _OSC state of control bits */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH v3 2/2] ACPI: pci_root: pass acpi_pci_root to arch-specific scan

[Kenji Kaneshige]

Looks good to me.

Reviewed-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

Thanks,
Kenji Kaneshige



Bjorn Helgaas wrote:
> The acpi_pci_root structure contains all the individual items (acpi_device,
> domain, bus number) we pass to pci_acpi_scan_root(), so just pass the
> single acpi_pci_root pointer directly.
> 
> This will make it easier to add _CBA support later.  For _CBA, we need the
> entire downstream bus range, not just the base bus number.  We have that in
> the acpi_pci_root structure, so passing the pointer makes it available to
> the arch-specific code.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> ---
> 
>  arch/ia64/pci/pci.c         |    5 ++++-
>  arch/x86/pci/acpi.c         |    5 ++++-
>  drivers/acpi/pci_root.c     |    2 +-
>  include/acpi/acpi_drivers.h |    3 +--
>  4 files changed, 10 insertions(+), 5 deletions(-)
> 
> 
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index 64aff52..aa2533a 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -335,8 +335,11 @@ pcibios_setup_root_windows(struct pci_bus *bus, struct pci_controller *ctrl)
>  }
>  
>  struct pci_bus * __devinit
> -pci_acpi_scan_root(struct acpi_device *device, int domain, int bus)
> +pci_acpi_scan_root(struct acpi_pci_root *root)
>  {
> +	struct acpi_device *device = root->device;
> +	int domain = root->segment;
> +	int bus = root->secondary.start;
>  	struct pci_controller *controller;
>  	unsigned int windows = 0;
>  	struct pci_bus *pbus;
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index 5f11ff6..0c36ad5 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -215,8 +215,11 @@ res_alloc_fail:
>  	return;
>  }
>  
> -struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_device *device, int domain, int busnum)
> +struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
>  {
> +	struct acpi_device *device = root->device;
> +	int domain = root->segment;
> +	int busnum = root->secondary.start;
>  	struct pci_bus *bus;
>  	struct pci_sysdata *sd;
>  	int node;
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index e309f56..7f554a4 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -498,7 +498,7 @@ 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, segment, root->secondary.start);
> +	root->bus = pci_acpi_scan_root(root);
>  	if (!root->bus) {
>  		printk(KERN_ERR PREFIX
>  			    "Bus %04x:%02x not present in PCI namespace\n",
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index 3a4767c..16903b3 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -102,8 +102,7 @@ int acpi_pci_bind_root(struct acpi_device *device);
>  
>  /* Arch-defined function to add a bus to the system */
>  
> -struct pci_bus *pci_acpi_scan_root(struct acpi_device *device, int domain,
> -				   int bus);
> +struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root);
>  void pci_acpi_crs_quirks(void);
>  
>  /* --------------------------------------------------------------------------
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH v3 1/2] ACPI: pci_root: save downstream bus range

[Bjorn Helgaas]

On Wednesday 10 March 2010 02:50:01 am Kenji Kaneshige wrote:
> Looks good to me. But one nitpicking.
> 
> > +	root->secondary.flags = IORESOURCE_BUS;
> > +	root->secondary.start = 0;
> > +	root->secondary.end = 0xFF;
> > +	status = try_get_root_bridge_busnr(device->handle, &root->secondary);
> >  	if (ACPI_FAILURE(status)) {
> 
> I feel a little strange about initializing 'start' and 'end' fields
> here because those are changed in try_get_root_bridge_busnr().
> What about the below?
> 
> 	root->secondary.flags = IORESOURCE_BUS;
> 	status = try_get_root_bridge_busnr(device->handle, &root->secondary);
> 	if (ACPI_FAILURE(status)) {
> 		root->secondary = 0xFF;
> 		...

Oooh, that's much better.  Wish I'd thought of that to begin with!
I'll fix that and repost these.  Thanks!

Bjorn

> Reviewed-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> Thanks,
> Kenji Kaneshige
> 
> 
> Bjorn Helgaas wrote:
> > Previously, we only saved the root bus number, i.e., the beginning of the
> > downstream bus range.  We now support IORESOURCE_BUS resources, so this
> > patch uses that to keep track of both the beginning and the end of the
> > downstream bus range.
> > 
> > It's important to know both the beginning and the end for supporting _CBA
> > (see PCI Firmware spec, rev 3.0, sec 4.1.3) and so we know the limits for
> > any possible PCI bus renumbering (we can't renumber downstream buses to be
> > outside the bus number range claimed by the host bridge).
> > 
> > It's clear from the spec that the bus range is supposed to be in _CRS, but
> > if we don't find it there, we'll assume [_BBN - 0xFF].
> > 
> > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > ---
> > 
> >  drivers/acpi/pci_root.c |   67 ++++++++++++++++++++++++++++-------------------
> >  include/acpi/acpi_bus.h |    2 +
> >  2 files changed, 41 insertions(+), 28 deletions(-)
> > 
> > 
> > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> > index d724736..e309f56 100644
> > --- a/drivers/acpi/pci_root.c
> > +++ b/drivers/acpi/pci_root.c
> > @@ -119,7 +119,8 @@ 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->segment == (u16) seg) && (root->bus_nr == (u16) bus))
> > +		if ((root->segment == (u16) seg) &&
> > +		    (root->secondary.start == (u16) bus))
> >  			return root->device->handle;
> >  	return NULL;		
> >  }
> > @@ -153,7 +154,7 @@ EXPORT_SYMBOL_GPL(acpi_is_root_bridge);
> >  static acpi_status
> >  get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
> >  {
> > -	int *busnr = data;
> > +	struct resource *res = data;
> >  	struct acpi_resource_address64 address;
> >  
> >  	if (resource->type != ACPI_RESOURCE_TYPE_ADDRESS16 &&
> > @@ -163,28 +164,27 @@ get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
> >  
> >  	acpi_resource_to_address64(resource, &address);
> >  	if ((address.address_length > 0) &&
> > -	    (address.resource_type == ACPI_BUS_NUMBER_RANGE))
> > -		*busnr = address.minimum;
> > +	    (address.resource_type == ACPI_BUS_NUMBER_RANGE)) {
> > +		res->start = address.minimum;
> > +		res->end = address.minimum + address.address_length - 1;
> > +	}
> >  
> >  	return AE_OK;
> >  }
> >  
> >  static acpi_status try_get_root_bridge_busnr(acpi_handle handle,
> > -					     unsigned long long *bus)
> > +					     struct resource *res)
> >  {
> >  	acpi_status status;
> > -	int busnum;
> >  
> > -	busnum = -1;
> > +	res->start = -1;
> >  	status =
> >  	    acpi_walk_resources(handle, METHOD_NAME__CRS,
> > -				get_root_bridge_busnr_callback, &busnum);
> > +				get_root_bridge_busnr_callback, res);
> >  	if (ACPI_FAILURE(status))
> >  		return status;
> > -	/* Check if we really get a bus number from _CRS */
> > -	if (busnum == -1)
> > +	if (res->start == -1)
> >  		return AE_ERROR;
> > -	*bus = busnum;
> >  	return AE_OK;
> >  }
> >  
> > @@ -428,34 +428,47 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
> >  	struct acpi_device *child;
> >  	u32 flags, base_flags;
> >  
> > +	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
> > +	if (!root)
> > +		return -ENOMEM;
> > +
> >  	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;
> > +		result = -ENODEV;
> > +		goto end;
> >  	}
> >  
> >  	/* Check _CRS first, then _BBN.  If no _BBN, default to zero. */
> > -	bus = 0;
> > -	status = try_get_root_bridge_busnr(device->handle, &bus);
> > +	root->secondary.flags = IORESOURCE_BUS;
> > +	root->secondary.start = 0;
> > +	root->secondary.end = 0xFF;
> > +	status = try_get_root_bridge_busnr(device->handle, &root->secondary);
> >  	if (ACPI_FAILURE(status)) {
> > +		/*
> > +		 * We need both the start and end of the downstream bus range
> > +		 * to interpret _CBA (MMCONFIG base address), so it really is
> > +		 * supposed to be in _CRS.
> > +		 */
> > +		printk(KERN_WARNING FW_BUG PREFIX
> > +		       "no secondary bus range in _CRS\n");
> >  		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;
> > +		if (ACPI_SUCCESS(status))
> > +			root->secondary.start = bus;
> > +		else if (status == AE_NOT_FOUND)
> > +			root->secondary.start = 0;
> > +		else {
> > +			printk(KERN_ERR PREFIX "can't evaluate _BBN\n");
> > +			result = -ENODEV;
> > +			goto end;
> >  		}
> >  	}
> >  
> > -	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
> > -	if (!root)
> > -		return -ENOMEM;
> > -
> >  	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;
> > @@ -474,9 +487,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
> >  	/* TBD: Locking */
> >  	list_add_tail(&root->node, &acpi_pci_roots);
> >  
> > -	printk(KERN_INFO PREFIX "%s [%s] (%04x:%02x)\n",
> > +	printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
> >  	       acpi_device_name(device), acpi_device_bid(device),
> > -	       root->segment, root->bus_nr);
> > +	       root->segment, &root->secondary);
> >  
> >  	/*
> >  	 * Scan the Root Bridge
> > @@ -485,11 +498,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, segment, bus);
> > +	root->bus = pci_acpi_scan_root(device, segment, root->secondary.start);
> >  	if (!root->bus) {
> >  		printk(KERN_ERR PREFIX
> >  			    "Bus %04x:%02x not present in PCI namespace\n",
> > -			    root->segment, root->bus_nr);
> > +			    root->segment, (unsigned int)root->secondary.start);
> >  		result = -ENODEV;
> >  		goto end;
> >  	}
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index 7bf83dd..baacd98 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -373,7 +373,7 @@ struct acpi_pci_root {
> >  	struct acpi_pci_id id;
> >  	struct pci_bus *bus;
> >  	u16 segment;
> > -	u8 bus_nr;
> > +	struct resource secondary;	/* downstream bus range */
> >  
> >  	u32 osc_support_set;	/* _OSC state of support bits */
> >  	u32 osc_control_set;	/* _OSC state of control bits */
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> 
> 
>