diff mbox series

[03/10] PCI: pciehp: Disable hotplug interrupt during suspend

Message ID 20180906155020.51700-4-mika.westerberg@linux.intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show
Series PCI: Allow D3cold for PCIe hierarchies | expand

Commit Message

Mika Westerberg Sept. 6, 2018, 3:50 p.m. UTC
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@linux.intel.com>
---
 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(+)

Comments

Rafael J. Wysocki Sept. 11, 2018, 8:55 a.m. UTC | #1
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@linux.intel.com>
> ---
>  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@intel.com>
diff mbox series

Patch

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