| Message ID | 20220927153524.49172-4-chenzhuo.1@bytedance.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/AER: Fix and optimize usage of status clearing api | expand |
On 9/27/22 8:35 AM, Zhuo Chen wrote: > Status bits for ERR_NONFATAL errors only are cleared in > pci_aer_clear_nonfatal_status(), but we want clear uncorrectable > error status in idt_init_pci(), so we change to use > pci_aer_clear_uncorrect_error_status(). You mean currently driver does not clear fatal errors now, and it is a problem? Any error reported? Also, I am wondering why is it required to clear errors during init code. Is it a norm? > > Signed-off-by: Zhuo Chen <chenzhuo.1@bytedance.com> > --- > drivers/ntb/hw/idt/ntb_hw_idt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c > index 0ed6f809ff2e..d5f0aa87f817 100644 > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c > @@ -2657,8 +2657,8 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) > ret = pci_enable_pcie_error_reporting(pdev); > if (ret != 0) > dev_warn(&pdev->dev, "PCIe AER capability disabled\n"); > - else /* Cleanup nonfatal error status before getting to init */ > - pci_aer_clear_nonfatal_status(pdev); > + else /* Cleanup uncorrectable error status before getting to init */ > + pci_aer_clear_uncorrect_error_status(pdev); > > /* First enable the PCI device */ > ret = pcim_enable_device(pdev);
On 9/28/22 3:39 AM, Sathyanarayanan Kuppuswamy wrote: > > > On 9/27/22 8:35 AM, Zhuo Chen wrote: >> Status bits for ERR_NONFATAL errors only are cleared in >> pci_aer_clear_nonfatal_status(), but we want clear uncorrectable >> error status in idt_init_pci(), so we change to use >> pci_aer_clear_uncorrect_error_status(). > > You mean currently driver does not clear fatal errors now, and it is > a problem? Any error reported? > Hi Sathyanarayanan, No error reports yet, I just changes the behavior back to what it was before commit e7b0b847de6d ("PCI/AER: Clear only ERR_NONFATAL bits during non-fatal recovery"), because this commit change the original function in commit bf2a952d31d2 ("NTB: Add IDT 89HPESxNTx PCIe-switches support"). > Also, I am wondering why is it required to clear errors during init > code. Is it a norm? > I think there is no need to clear errors during init code. >> >> Signed-off-by: Zhuo Chen <chenzhuo.1@bytedance.com> >> --- >> drivers/ntb/hw/idt/ntb_hw_idt.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c >> index 0ed6f809ff2e..d5f0aa87f817 100644 >> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c >> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c >> @@ -2657,8 +2657,8 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) >> ret = pci_enable_pcie_error_reporting(pdev); >> if (ret != 0) >> dev_warn(&pdev->dev, "PCIe AER capability disabled\n"); >> - else /* Cleanup nonfatal error status before getting to init */ >> - pci_aer_clear_nonfatal_status(pdev); >> + else /* Cleanup uncorrectable error status before getting to init */ >> + pci_aer_clear_uncorrect_error_status(pdev); >> >> /* First enable the PCI device */ >> ret = pcim_enable_device(pdev); >
On 9/27/22 9:20 PM, Zhuo Chen wrote: > > > On 9/28/22 3:39 AM, Sathyanarayanan Kuppuswamy wrote: >> >> >> On 9/27/22 8:35 AM, Zhuo Chen wrote: >>> Status bits for ERR_NONFATAL errors only are cleared in >>> pci_aer_clear_nonfatal_status(), but we want clear uncorrectable >>> error status in idt_init_pci(), so we change to use >>> pci_aer_clear_uncorrect_error_status(). >> >> You mean currently driver does not clear fatal errors now, and it is >> a problem? Any error reported? >> > Hi Sathyanarayanan, > > No error reports yet, I just changes the behavior back to what it was before commit e7b0b847de6d ("PCI/AER: Clear only ERR_NONFATAL bits during non-fatal recovery"), because this commit change the original function in commit bf2a952d31d2 ("NTB: Add IDT 89HPESxNTx PCIe-switches support"). > Ok. Thanks for clarifying. >> Also, I am wondering why is it required to clear errors during init >> code. Is it a norm? >> > I think there is no need to clear errors during init code. >>> >>> Signed-off-by: Zhuo Chen <chenzhuo.1@bytedance.com> >>> --- >>> drivers/ntb/hw/idt/ntb_hw_idt.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c >>> index 0ed6f809ff2e..d5f0aa87f817 100644 >>> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c >>> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c >>> @@ -2657,8 +2657,8 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) >>> ret = pci_enable_pcie_error_reporting(pdev); >>> if (ret != 0) >>> dev_warn(&pdev->dev, "PCIe AER capability disabled\n"); >>> - else /* Cleanup nonfatal error status before getting to init */ >>> - pci_aer_clear_nonfatal_status(pdev); >>> + else /* Cleanup uncorrectable error status before getting to init */ >>> + pci_aer_clear_uncorrect_error_status(pdev); >>> /* First enable the PCI device */ >>> ret = pcim_enable_device(pdev); >> >
On Tue, Sep 27, 2022 at 11:35:18PM +0800, Zhuo Chen wrote: > Status bits for ERR_NONFATAL errors only are cleared in > pci_aer_clear_nonfatal_status(), but we want clear uncorrectable > error status in idt_init_pci(), so we change to use > pci_aer_clear_uncorrect_error_status(). Have the modification changed since: https://lore.kernel.org/linux-pci/20220901181634.99591-2-chenzhuo.1@bytedance.com/ ? AFAICS it hasn't. Then my ab-tag should have been preserved on v2. One more time: Acked-by: Serge Semin <fancer.lancer@gmail.com> Don't forget to add it should you have some more patchset re-spins. -Sergey > > Signed-off-by: Zhuo Chen <chenzhuo.1@bytedance.com> > --- > drivers/ntb/hw/idt/ntb_hw_idt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c > index 0ed6f809ff2e..d5f0aa87f817 100644 > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c > @@ -2657,8 +2657,8 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) > ret = pci_enable_pcie_error_reporting(pdev); > if (ret != 0) > dev_warn(&pdev->dev, "PCIe AER capability disabled\n"); > - else /* Cleanup nonfatal error status before getting to init */ > - pci_aer_clear_nonfatal_status(pdev); > + else /* Cleanup uncorrectable error status before getting to init */ > + pci_aer_clear_uncorrect_error_status(pdev); > > /* First enable the PCI device */ > ret = pcim_enable_device(pdev); > -- > 2.30.1 (Apple Git-130) >
diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c index 0ed6f809ff2e..d5f0aa87f817 100644 --- a/drivers/ntb/hw/idt/ntb_hw_idt.c +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c @@ -2657,8 +2657,8 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) ret = pci_enable_pcie_error_reporting(pdev); if (ret != 0) dev_warn(&pdev->dev, "PCIe AER capability disabled\n"); - else /* Cleanup nonfatal error status before getting to init */ - pci_aer_clear_nonfatal_status(pdev); + else /* Cleanup uncorrectable error status before getting to init */ + pci_aer_clear_uncorrect_error_status(pdev); /* First enable the PCI device */ ret = pcim_enable_device(pdev);
Status bits for ERR_NONFATAL errors only are cleared in pci_aer_clear_nonfatal_status(), but we want clear uncorrectable error status in idt_init_pci(), so we change to use pci_aer_clear_uncorrect_error_status(). Signed-off-by: Zhuo Chen <chenzhuo.1@bytedance.com> --- drivers/ntb/hw/idt/ntb_hw_idt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)