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@xxxxxxxxxxxxxx>
>
> 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@xxxxxx>
> > ---
> >
> > 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@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
