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