Re: [PATCH v3 1/4] PCI: Keep AER status in pci_restore_state()

[Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>]

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

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer