On Tue, May 10, 2016 at 12:18:59AM +0200, Rafael J. Wysocki wrote: [...] > > >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > > >> index ae3fe4e..4581e0e 100644 > > >> --- a/drivers/acpi/pci_root.c > > >> +++ b/drivers/acpi/pci_root.c > > >> @@ -564,6 +564,11 @@ static int acpi_pci_root_add(struct acpi_device *device, > > >> } > > >> } > > >> > > >> + /* > > >> + * pci_create_root_bus() needs to detect the parent device type, > > >> + * so initialize its companion data accordingly. > > >> + */ > > >> + ACPI_COMPANION_SET(&device->dev, device); > > >> root->device = device; > > >> root->segment = segment & 0xFFFF; > > >> strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME); > > >> @@ -846,7 +851,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > > >> > > >> pci_acpi_root_add_resources(info); > > >> pci_add_resource(&info->resources, &root->secondary); > > >> - bus = pci_create_root_bus(NULL, busnum, ops->pci_ops, > > >> + bus = pci_create_root_bus(&device->dev, busnum, ops->pci_ops, > > >> sysdata, &info->resources); > > > > > > "device" here is a struct acpi_device *. Rafael, is that the right > > > thing to do? I dimly recall proposing something similar long ago and > > > that it turned out to be a bad idea. > > > > > > > Joining Bjorn's question. Rafael, do you recall what was the issue here > > from the past? > > Yes, I do. > > Anything struct acpi_device doesn't represent a real device. It > represents a firmware object that can be associated with a device. > IOW, nothing under LNXSYSTM\:00/ should ever be used as the parent or > sibling etc with respect to anything outside of that directory. In > fact, the entire LNXSYSTM\:00/ should be located under > /sys/firmware/acpi/ and it was a mistake to put it under > /sys/devices/. > > One immediate consequence of the above change, AFAICS, would be that > the whole PCI hierarchy would now hang under > /sys/devices/LNXSYSTM\:00/LNXSYBUS\:00/PNP0A08\:00/ which would not > make any sense whatever, because > /sys/devices/LNXSYSTM\:00/LNXSYBUS\:00/PNP0A08\:00/physical_node > already points to /sys/devices/pci0000\:00/. > > IOW, both PNP0A08\:00/ and pci0000\:00/ already represent the same > thing, ie. the host bridge. > > If you want to have a parent for pci0000\:00/, you need a > physical_node for LNXSYBUS\:00 and point to that as the parent from > pci0000\:00/. That at least will lead to a sysfs hierarchy that makes > sense, although it may break user space tools I suppose (which may be > assuming that pci0000\:00/ will always be present directly under > /sys/devices/). Ok, I have a question though. As an example, DT based host controllers (that pass the parent pointer to pci_create_root_bus()), are already rooted at the respective host controller platform device sysfs path, so if user space can't cope with that, that is a problem *now* on some systems unless I am missing something. Anyway, thanks for clarifying the companion handling mechanism, we decidedly have to find a proper way to handle it instead of working around it. Lorenzo -- 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
