On Mon, Sep 26, 2022 at 10:16:23PM +0800, Zhuo Chen wrote: > On 9/23/22 5:50 AM, Bjorn Helgaas wrote: > > On Fri, Sep 02, 2022 at 02:16:34AM +0800, Zhuo Chen wrote: > > > Statements clearing AER error status in aer_enable_rootport() has the > > > same function as pci_aer_raw_clear_status(). So we replace them, which > > > has no functional changes.
> > > - pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, ®32); > > > - pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, reg32); > > > - pci_read_config_dword(pdev, aer + PCI_ERR_COR_STATUS, ®32); > > > - pci_write_config_dword(pdev, aer + PCI_ERR_COR_STATUS, reg32); > > > - pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, ®32); > > > - pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32); > > > + pci_aer_raw_clear_status(pdev); > > > > It's true that this is functionally equivalent. > > > > But 20e15e673b05 ("PCI/AER: Add pci_aer_raw_clear_status() to > > unconditionally clear Error Status") says pci_aer_raw_clear_status() > > is only for use in the EDR path (this should have been included in the > > function comment), so I think we should preserve that property and use > > pci_aer_clear_status() here. > > > > pci_aer_raw_clear_status() is the same as pci_aer_clear_status() > > except it doesn't check pcie_aer_is_native(). And I'm pretty sure we > > can't get to aer_enable_rootport() *unless* pcie_aer_is_native(), > > because get_port_device_capability() checks the same thing, so they > > should be equivalent here. > > > Thanks Bjorn, this very detailed correction is helpful. By the way, 'only > for use in the EDR path' obviously written in the function comments may be > better. So far only commit log has included these. Yes, definitely! I goofed when I applied that patch without making sure there was something in the function comment. Bjorn