On Fri, Dec 04, 2020 at 08:39:09AM +0100, Lukas Wunner wrote: > On Mon, Nov 30, 2020 at 11:30:05AM -0600, Bjorn Helgaas wrote: > > On Mon, Nov 23, 2020 at 02:45:13PM +0800, Jianjun Wang wrote: > > > On Thu, 2020-11-19 at 14:28 -0600, Bjorn Helgaas wrote: > > > > > +static int mtk_pcie_setup(struct mtk_pcie_port *port) > > > > > +{ > [...] > > > > > + /* Try link up */ > > > > > + err = mtk_pcie_startup_port(port); > > > > > + if (err) { > > > > > + dev_notice(dev, "PCIe link down\n"); > > > > > + goto err_setup; > > > > > > > > Generally it should not be a fatal error if the link is not up at > > > > probe-time. You may be able to hot-add a device, or the device may > > > > have some external power control that will power it up later. > > > > > > This is for the power saving requirement. If there is no device > > > connected with the PCIe slot, the PCIe MAC and PHY should be powered > > > off. > > > > > > Is there any standard flow to support power down the hardware at > > > probe-time if no device is connected and power it up when hot-add a > > > device? > > > > That's a good question. I assume this looks like a standard PCIe > > hot-add event? > > > > When you hot-add a device, does the Root Port generate a Presence > > Detect Changed interrupt? The pciehp driver should field that > > interrupt and turn on power to the slot via the Power Controller > > Control bit in the Slot Control register. > > > > Does your hardware require something more than that to control the MAC > > and PHY power? > > Power saving of unused PCIe ports is generally achieved through the > runtime PM framework. When a PCIe port runtime suspends, the PCIe > core will transition it to D3hot. On top of that, the platform > may be able to transition the port to D3cold. Currently only the > ACPI platform supports that. Conceivably, devicetree-based systems > may want to disable certain clocks or regulators when a PCIe port > runtime suspends. I think we do not support that yet but it could > be added to drivers/pci/pcie/portdrv*. > > A hotplug port is expected to signal PDC and DLLSC interrupts even > when in D3hot. At least that's our experience with Thunderbolt. > To support hotplug interrupts in D3cold, some external mechanism > (such as a PME) is necessary to wake up the port on hotplug. > This is also supported with recent Thunderbolt systems. > > Because we've seen various incompatibilities when runtime suspending > PCIe ports, certain conditions must be satisfied for runtime PM > to be enabled. They're encoded in pci_bridge_d3_possible(). > Generally, hotplug ports only runtime suspend if they belong to > a Thunderbolt controller or if the ACPI platform explicitly allows > runtime PM (through presence of a _PR3 method or a device property). > Non-hotplug ports runtime suspend if the BIOS is newer than 2015 > (as specified by DMI). > > Obviously, this policy is very x86-focussed because both Thunderbolt > and DMI are only really a thing on x86. That's about to change though > because Apple's new arm64-based Macs have Thunderbolt integrated into > the SoC and arm64 SoCs are making inroads in the datacenter, which is > an important use case for PCIe hotplug (hot-swappable NVMe drives). > So we may have to amend pci_bridge_d3_possible() to whitelist > PCIe ports for runtime PM on specific arches or systems. Thanks for all this very useful information! My interpretation for the mediatek situation: - I assume this patch leaves or puts the Root Port in D3cold if no downstream devices are present. - I don't see any support for PME or similar mechanisms to signal a hot-add while the RP is in D3cold. - So I assume you don't support hot-add if the slot was empty at boot and that's acceptable for your platform.