Re: [PATCH] PCI / ACPI: Always resume devices on ACPI wakeup notifications

[Bjorn Helgaas <bhelgaas@google.com>]

On Tue, Apr 2, 2013 at 8:04 PM, huang ying <huang.ying.caritas@gmail.com> wrote:
> On Wed, Apr 3, 2013 at 12:30 AM, Bjorn Helgaas <bhelgaas@google.com> 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