| Message ID | 20230420125941.333675-1-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v3,1/4] PCI: Keep AER status in pci_restore_state() | expand |
Hi Kai, On 4/20/23 5:59 AM, 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. > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- Any history on why it is cleared before? Is it done to hide some resume issues? > v3: > - No change. > > 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);
Hi Sathyanarayanan, On Thu, Apr 20, 2023 at 10:39 PM Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > Hi Kai, It's Kai-Heng :) > > On 4/20/23 5:59 AM, 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. > > > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > --- > > Any history on why it is cleared before? Is it done to hide some resume > issues? It was introduced by commit b07461a8e45b ("PCI/AER: Clear error status registers during enumeration and restore"). The justification is quite reasonable so I think maybe we should keep it as is. Kai-Heng > > > v3: > > - No change. > > > > 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); > > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer
On 4/20/23 6:35 PM, Kai-Heng Feng wrote: > Hi Sathyanarayanan, > > On Thu, Apr 20, 2023 at 10:39 PM Sathyanarayanan Kuppuswamy > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: >> >> Hi Kai, > > It's Kai-Heng :) > >> >> On 4/20/23 5:59 AM, 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. >>> >>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> >>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >>> --- >> >> Any history on why it is cleared before? Is it done to hide some resume >> issues? > > It was introduced by commit b07461a8e45b ("PCI/AER: Clear error status > registers during enumeration and restore"). > The justification is quite reasonable so I think maybe we should keep it as is. Yes. It looks like it is better to leave it as it is. > > Kai-Heng > >> >>> v3: >>> - No change. >>> >>> 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); >> >> -- >> Sathyanarayanan Kuppuswamy >> Linux Kernel Developer
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);