On Tue, Apr 2, 2013 at 8:04 PM, huang ying <huang.ying.caritas@xxxxxxxxx> wrote: > On Wed, Apr 3, 2013 at 12:30 AM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: >> 2) acpiphp currently uses the presence of _ADR/_EJ0/_RMV to detect >> hotplug slots. I don't think this is sufficient (see >> https://bugzilla.kernel.org/show_bug.cgi?id=54981 for details). >> Therefore, I don't think pci_bus_has_hotplug_slots() in port_dbg.patch >> can be accurate. I think it returns "false" for some buses where it >> should return "true," such as the ExpressCard slot on Chris Clayton's >> system (see bug 54981). > > Yes. pci_bus_has_hotplug_slots() is not accurate. But I still think > it can be used in port runtime PM. Because if there is no hotplug > slots registered, the hotplug itself can not work properly, with or > without port runtime PM enabled. I'm not sure this is true. For concreteness, let's talk about Chris Clayton's machine. He has a root port 00:1c.3, that leads to an ExpressCard slot. We request to use PCIe native hotplug, but BIOS declines, so we have to use acpiphp. The SCI that signals a hotplug event is generated by the 00:1c.3 root port. Therefore, 00:1c.3 must remain powered up even when the slot is empty. So the question really is, "Can we tell that there's a hotplug slot below 00:1c.3?" acpiphp currently does not detect this slot because 00:1c.3 does have an _ADR method, but there's no _EJ0 or _RMV method. If you are suggesting that "acpiphp hotplug doesn't work correctly for this slot even with 00:1c.3 powered up, so we might as well turn 00:1c.3 off," I completely disagree. That would mean that even after we fix acpiphp so it re-enumerates when we receive a Bus Check, hotplug would still be broken because the powered-off port will not generate the SCI that triggers the Bus Check. > And we should add necessary > pm_runtime_get_sync/put_sync into pci_scan_bus to deal with "rescan". > What do you think about that? > > pci_dev->is_hotplug_bridge is not accurate too. It reports a internal > port of my X220 as a hotplug-able port. But it appears that it will > report more instead of less. It can report correctly for port in bug > 54981. Do you think that is a good choice for port runtime PM > filtering? pci_dev->is_hotplug_bridge is currently set in these cases: 1) A quirk for a PLX bridge 2) The PCIe Slot Capability "Hot-Plug Capable" bit is set 3) The acpiphp driver thinks there's an ejectable slot below the device This doesn't look reliable to me at all. I don't think think it's even possible for acpiphp to deduce from AML whether there are ejectable slots. If we're using acpiphp, I feel queasy about looking at the PCIe Capability to figure out whether slots are present. I don't think it's a good idea to mix the PCIe and ACPI worlds that way. And ACPI hotplug could be used for other hotplug schemes besides PCIe, so we can't even count on the PCIe capability existing. So I disagree that pci_dev->is_hotplug_bridge is set for every device that may have a hotplug slot below it. I think it's mainly the acpiphp case where it's hard to tell when runtime PM is safe; it might be possible to do runtime PM on a port where we are using pciehp. Bjorn -- 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
