When AER is using the same IRQ as PME, AER interrupt is treated as a
wakeup event and it can disrupt system suspend process.
If that happens, the system will report it's woken up by PME IRQ without
indicating any AER error since AER status is cleared on resume.
So keep the AER status so users can know the system is woken up by AER
instead of PME.
Signed-off-by: Kai-Heng Feng <[email protected]>
---
v2:
- New patch.
drivers/pci/pci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7a67611dc5f4..71aead00fc20 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1778,7 +1778,6 @@ void pci_restore_state(struct pci_dev *dev)
pci_restore_dpc_state(dev);
pci_restore_ptm_state(dev);
- pci_aer_clear_status(dev);
pci_restore_aer_state(dev);
pci_restore_config_space(dev);
--
2.34.1
PCIe service that shares IRQ with PME may cause spurious wakeup on
system suspend.
Since AER is conditionally disabled in previous patch, also apply the
same logic to disable DPC which depends on AER to work.
PCIe Base Spec 5.0, section 5.2 "Link State Power Management" states
that TLP and DLLP transmission is disabled for a Link in L2/L3 Ready
(D3hot), L2 (D3cold with aux power) and L3 (D3cold), so we don't lose
much here to disable DPC during system suspend.
This is very similar to previous attempts to suspend AER and DPC [1],
but with a different reason.
[1] https://lore.kernel.org/linux-pci/[email protected]/
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216295
Signed-off-by: Kai-Heng Feng <[email protected]>
---
v2:
- Only disable DPC IRQ.
- No more check on PME IRQ#.
drivers/pci/pcie/dpc.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index a5d7c69b764e..98bdefde6df1 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -385,6 +385,30 @@ static int dpc_probe(struct pcie_device *dev)
return status;
}
+static int dpc_suspend(struct pcie_device *dev)
+{
+ struct pci_dev *pdev = dev->port;
+ u16 ctl;
+
+ pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl);
+ ctl &= ~PCI_EXP_DPC_CTL_INT_EN;
+ pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl);
+
+ return 0;
+}
+
+static int dpc_resume(struct pcie_device *dev)
+{
+ struct pci_dev *pdev = dev->port;
+ u16 ctl;
+
+ pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl);
+ ctl |= PCI_EXP_DPC_CTL_INT_EN;
+ pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl);
+
+ return 0;
+}
+
static void dpc_remove(struct pcie_device *dev)
{
struct pci_dev *pdev = dev->port;
@@ -400,6 +424,8 @@ static struct pcie_port_service_driver dpcdriver = {
.port_type = PCIE_ANY_PORT,
.service = PCIE_PORT_SERVICE_DPC,
.probe = dpc_probe,
+ .suspend = dpc_suspend,
+ .resume = dpc_resume,
.remove = dpc_remove,
};
--
2.34.1
PCIe service that shares IRQ with PME may cause spurious wakeup on
system suspend.
PCIe Base Spec 5.0, section 5.2 "Link State Power Management" states
that TLP and DLLP transmission is disabled for a Link in L2/L3 Ready
(D3hot), L2 (D3cold with aux power) and L3 (D3cold), so we don't lose
much here to disable AER during system suspend.
This is very similar to previous attempts to suspend AER and DPC [1],
but with a different reason.
[1] https://lore.kernel.org/linux-pci/[email protected]/
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216295
Signed-off-by: Kai-Heng Feng <[email protected]>
---
v2:
- Only disable AER IRQ.
- No more check on PME IRQ#.
- Use helper.
drivers/pci/pcie/aer.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 1420e1f27105..9c07fdbeb52d 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1356,6 +1356,26 @@ static int aer_probe(struct pcie_device *dev)
return 0;
}
+static int aer_suspend(struct pcie_device *dev)
+{
+ struct aer_rpc *rpc = get_service_data(dev);
+ struct pci_dev *pdev = rpc->rpd;
+
+ aer_disable_irq(pdev);
+
+ return 0;
+}
+
+static int aer_resume(struct pcie_device *dev)
+{
+ struct aer_rpc *rpc = get_service_data(dev);
+ struct pci_dev *pdev = rpc->rpd;
+
+ aer_enable_irq(pdev);
+
+ return 0;
+}
+
/**
* aer_root_reset - reset Root Port hierarchy, RCEC, or RCiEP
* @dev: pointer to Root Port, RCEC, or RCiEP
@@ -1420,6 +1440,8 @@ static struct pcie_port_service_driver aerdriver = {
.service = PCIE_PORT_SERVICE_AER,
.probe = aer_probe,
+ .suspend = aer_suspend,
+ .resume = aer_resume,
.remove = aer_remove,
};
--
2.34.1
There are many places that enable and disable AER interrput, so move
them into helpers.
Signed-off-by: Kai-Heng Feng <[email protected]>
---
v2:
- New patch.
drivers/pci/pcie/aer.c | 45 +++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index f6c24ded134c..1420e1f27105 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1227,6 +1227,28 @@ static irqreturn_t aer_irq(int irq, void *context)
return IRQ_WAKE_THREAD;
}
+static void aer_enable_irq(struct pci_dev *pdev)
+{
+ int aer = pdev->aer_cap;
+ u32 reg32;
+
+ /* Enable Root Port's interrupt in response to error messages */
+ pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, ®32);
+ reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
+ pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
+}
+
+static void aer_disable_irq(struct pci_dev *pdev)
+{
+ int aer = pdev->aer_cap;
+ u32 reg32;
+
+ /* Disable Root's interrupt in response to error messages */
+ pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, ®32);
+ reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
+ pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
+}
+
/**
* aer_enable_rootport - enable Root Port's interrupts when receiving messages
* @rpc: pointer to a Root Port data structure
@@ -1256,10 +1278,7 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, ®32);
pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32);
- /* Enable Root Port's interrupt in response to error messages */
- pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, ®32);
- reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
- pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
+ aer_enable_irq(pdev);
}
/**
@@ -1274,10 +1293,7 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
int aer = pdev->aer_cap;
u32 reg32;
- /* Disable Root's interrupt in response to error messages */
- pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, ®32);
- reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
- pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
+ aer_disable_irq(pdev);
/* Clear Root's error status reg */
pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, ®32);
@@ -1372,12 +1388,8 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
*/
aer = root ? root->aer_cap : 0;
- if ((host->native_aer || pcie_ports_native) && aer) {
- /* Disable Root's interrupt in response to error messages */
- pci_read_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, ®32);
- reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
- pci_write_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, reg32);
- }
+ if ((host->native_aer || pcie_ports_native) && aer)
+ aer_disable_irq(root);
if (type == PCI_EXP_TYPE_RC_EC || type == PCI_EXP_TYPE_RC_END) {
rc = pcie_reset_flr(dev, PCI_RESET_DO_RESET);
@@ -1396,10 +1408,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
pci_read_config_dword(root, aer + PCI_ERR_ROOT_STATUS, ®32);
pci_write_config_dword(root, aer + PCI_ERR_ROOT_STATUS, reg32);
- /* Enable Root Port's interrupt in response to error messages */
- pci_read_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, ®32);
- reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
- pci_write_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, reg32);
+ aer_enable_irq(root);
}
return rc ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED;
--
2.34.1
Typo in $subject:
interrput -> interrupt
On Thu, Apr 20, 2023 at 09:58:28AM +0800, Kai-Heng Feng wrote:
> There are many places that enable and disable AER interrput, so move
> them into helpers.
>
> Signed-off-by: Kai-Heng Feng <[email protected]>
> ---
> v2:
> - New patch.
>
> drivers/pci/pcie/aer.c | 45 +++++++++++++++++++++++++-----------------
> 1 file changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index f6c24ded134c..1420e1f27105 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1227,6 +1227,28 @@ static irqreturn_t aer_irq(int irq, void *context)
> return IRQ_WAKE_THREAD;
> }
>
> +static void aer_enable_irq(struct pci_dev *pdev)
> +{
> + int aer = pdev->aer_cap;
> + u32 reg32;
> +
> + /* Enable Root Port's interrupt in response to error messages */
> + pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, ®32);
> + reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> + pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> +}
> +
> +static void aer_disable_irq(struct pci_dev *pdev)
> +{
> + int aer = pdev->aer_cap;
> + u32 reg32;
> +
> + /* Disable Root's interrupt in response to error messages */
> + pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, ®32);
> + reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> + pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> +}
> +
> /**
> * aer_enable_rootport - enable Root Port's interrupts when receiving messages
> * @rpc: pointer to a Root Port data structure
> @@ -1256,10 +1278,7 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
> pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, ®32);
> pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32);
>
> - /* Enable Root Port's interrupt in response to error messages */
> - pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, ®32);
> - reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> + aer_enable_irq(pdev);
> }
>
> /**
> @@ -1274,10 +1293,7 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
> int aer = pdev->aer_cap;
> u32 reg32;
>
> - /* Disable Root's interrupt in response to error messages */
> - pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, ®32);
> - reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> + aer_disable_irq(pdev);
>
> /* Clear Root's error status reg */
> pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, ®32);
> @@ -1372,12 +1388,8 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
> */
> aer = root ? root->aer_cap : 0;
>
> - if ((host->native_aer || pcie_ports_native) && aer) {
> - /* Disable Root's interrupt in response to error messages */
> - pci_read_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, ®32);
> - reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, reg32);
> - }
> + if ((host->native_aer || pcie_ports_native) && aer)
> + aer_disable_irq(root);
>
> if (type == PCI_EXP_TYPE_RC_EC || type == PCI_EXP_TYPE_RC_END) {
> rc = pcie_reset_flr(dev, PCI_RESET_DO_RESET);
> @@ -1396,10 +1408,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
> pci_read_config_dword(root, aer + PCI_ERR_ROOT_STATUS, ®32);
> pci_write_config_dword(root, aer + PCI_ERR_ROOT_STATUS, reg32);
>
> - /* Enable Root Port's interrupt in response to error messages */
> - pci_read_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, ®32);
> - reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, reg32);
> + aer_enable_irq(root);
> }
>
> return rc ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED;
> --
> 2.34.1
On Thu, Apr 20, 2023 at 09:58:27AM +0800, Kai-Heng Feng wrote:
> When AER is using the same IRQ as PME, AER interrupt is treated as a
> wakeup event and it can disrupt system suspend process.
>
> If that happens, the system will report it's woken up by PME IRQ without
> indicating any AER error since AER status is cleared on resume.
>
> So keep the AER status so users can know the system is woken up by AER
> instead of PME.
>
> Signed-off-by: Kai-Heng Feng <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
On Thu, Apr 20, 2023 at 09:58:29AM +0800, Kai-Heng Feng wrote:
> PCIe service that shares IRQ with PME may cause spurious wakeup on
> system suspend.
>
> PCIe Base Spec 5.0, section 5.2 "Link State Power Management" states
> that TLP and DLLP transmission is disabled for a Link in L2/L3 Ready
> (D3hot), L2 (D3cold with aux power) and L3 (D3cold), so we don't lose
> much here to disable AER during system suspend.
>
> This is very similar to previous attempts to suspend AER and DPC [1],
> but with a different reason.
>
> [1] https://lore.kernel.org/linux-pci/[email protected]/
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216295
>
> Signed-off-by: Kai-Heng Feng <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
On Thu, Apr 20, 2023 at 09:58:30AM +0800, Kai-Heng Feng wrote:
> PCIe service that shares IRQ with PME may cause spurious wakeup on
> system suspend.
>
> Since AER is conditionally disabled in previous patch, also apply the
> same logic to disable DPC which depends on AER to work.
>
> PCIe Base Spec 5.0, section 5.2 "Link State Power Management" states
> that TLP and DLLP transmission is disabled for a Link in L2/L3 Ready
> (D3hot), L2 (D3cold with aux power) and L3 (D3cold), so we don't lose
> much here to disable DPC during system suspend.
>
> This is very similar to previous attempts to suspend AER and DPC [1],
> but with a different reason.
>
> [1] https://lore.kernel.org/linux-pci/[email protected]/
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216295
>
> Signed-off-by: Kai-Heng Feng <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>