On Thursday, September 6, 2018 5:50:13 PM CEST Mika Westerberg wrote:
> When PCIe hotplug port is transitioned into D3hot link to the downstream
> component will go down. If hotplug interrupt generation is enabled when
> that happens it will trigger immediately waking up the system and
> bringing the link back up.
>
> To prevent this disable hotplug interrupt generation when system suspend
> is entered. This does not prevent wakeup from low power states according
> to PCIe 4.0 spec section 6.7.3.4:
>
> Software enables a hot-plug event to generate a wakeup event by
> enabling software notification of the event as described in Section
> 6.7.3.1. Note that in order for software to disable interrupt generation
> while keeping wakeup generation enabled, the Hot-Plug Interrupt Enable
> bit must be cleared.
>
> So as long as we have set the slot event mask accordingly wakeup should
> work even if slot interrupt is disabled. The port should trigger wake
> and then send PME to the root port when the PCIe hierarchy is brought
> back up.
>
> Limit this to systems using native PME mechanism to make sure older
> Apple systems depending on commit e3354628c376 ("PCI: pciehp: Support
> interrupts sent from D3hot") still continue working.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> ---
> drivers/pci/hotplug/pciehp.h | 2 ++
> drivers/pci/hotplug/pciehp_core.c | 18 ++++++++++++++++++
> drivers/pci/hotplug/pciehp_hpc.c | 10 ++++++++++
> 3 files changed, 30 insertions(+)
>
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index 811cf83f956d..58bddeb3c5a9 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -187,6 +187,8 @@ struct controller *pcie_init(struct pcie_device *dev);
> int pcie_init_notification(struct controller *ctrl);
> void pcie_shutdown_notification(struct controller *ctrl);
> void pcie_clear_hotplug_events(struct controller *ctrl);
> +void pcie_enable_interrupt(struct controller *ctrl);
> +void pcie_disable_interrupt(struct controller *ctrl);
> int pciehp_power_on_slot(struct slot *slot);
> void pciehp_power_off_slot(struct slot *slot);
> void pciehp_get_power_status(struct slot *slot, u8 *status);
> diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
> index ec48c9433ae5..c9cf9c3b5f7f 100644
> --- a/drivers/pci/hotplug/pciehp_core.c
> +++ b/drivers/pci/hotplug/pciehp_core.c
> @@ -302,8 +302,23 @@ static void pciehp_remove(struct pcie_device *dev)
> }
>
> #ifdef CONFIG_PM
> +static bool pme_is_native(struct pcie_device *dev)
> +{
> + const struct pci_host_bridge *host;
> +
> + host = pci_find_host_bridge(dev->port->bus);
> + return pcie_ports_native || host->native_pme;
> +}
> +
> static int pciehp_suspend(struct pcie_device *dev)
> {
> + /*
> + * Disable hotplug interrupt so that it does not trigger
> + * immediately when the downstream link goes down.
> + */
> + if (pme_is_native(dev))
> + pcie_disable_interrupt(get_service_data(dev));
> +
> return 0;
> }
>
> @@ -327,6 +342,9 @@ static int pciehp_resume(struct pcie_device *dev)
> {
> struct controller *ctrl = get_service_data(dev);
>
> + if (pme_is_native(dev))
> + pcie_enable_interrupt(ctrl);
> +
> pciehp_check_presence(ctrl);
>
> return 0;
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 7136e3430925..2249c4d06efd 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -748,6 +748,16 @@ void pcie_clear_hotplug_events(struct controller *ctrl)
> PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC);
> }
>
> +void pcie_enable_interrupt(struct controller *ctrl)
> +{
> + pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_HPIE, PCI_EXP_SLTCTL_HPIE);
> +}
> +
> +void pcie_disable_interrupt(struct controller *ctrl)
> +{
> + pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_HPIE);
> +}
> +
> /*
> * pciehp has a 1:1 bus:slot relationship so we ultimately want a secondary
> * bus reset of the bridge, but at the same time we want to ensure that it is
>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
