| Message ID | 20230307203021.881866-1-helgaas@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | ntb: idt: drop redundant pci_enable_pcie_error_reporting() | expand |
Hi Bjorn, On Tue, Mar 07, 2023 at 02:30:21PM -0600, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > pci_enable_pcie_error_reporting() enables the device to send ERR_* > Messages. Since f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is > native"), the PCI core does this for all devices during enumeration, so the > driver doesn't need to do it itself. > > Remove the redundant pci_enable_pcie_error_reporting() call from the > driver. Also remove the corresponding pci_disable_pcie_error_reporting() > from the driver .remove() path. > > Note that this only controls ERR_* Messages from the device. An ERR_* > Message may cause the Root Port to generate an interrupt, depending on the > AER Root Error Command register managed by the AER service driver. Thanks for the patch. Acked-by: Serge Semin <fancer.lancer@gmail.com> BTW seeing the pci_aer_init() method fully clears the AER status don't you think the pci_aer_clear_nonfatal_status() function invocation should be dropped as well? Note the patchset doing that was already submitted a while ago: https://lore.kernel.org/linux-pci/20220928105946.12469-1-chenzhuo.1@bytedance.com * particular patch: https://lore.kernel.org/linux-pci/20220928105946.12469-4-chenzhuo.1@bytedance.com -Serge(y) > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/ntb/hw/idt/ntb_hw_idt.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c > index 0ed6f809ff2e..7192ff59dd91 100644 > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c > @@ -2651,20 +2651,18 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) > } > > /* > - * Enable the device advanced error reporting. It's not critical to > + * The PCI core enables device error reporting. It's not critical to > * have AER disabled in the kernel. > + * > + * Cleanup nonfatal error status before getting to init. > */ > - 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); > + pci_aer_clear_nonfatal_status(pdev); > > /* First enable the PCI device */ > ret = pcim_enable_device(pdev); > if (ret != 0) { > dev_err(&pdev->dev, "Failed to enable PCIe device\n"); > - goto err_disable_aer; > + return ret; > } > > /* > @@ -2692,8 +2690,6 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) > > err_clear_master: > pci_clear_master(pdev); > -err_disable_aer: > - (void)pci_disable_pcie_error_reporting(pdev); > > return ret; > } > @@ -2714,9 +2710,6 @@ static void idt_deinit_pci(struct idt_ntb_dev *ndev) > /* Clear the bus master disabling the Request TLPs translation */ > pci_clear_master(pdev); > > - /* Disable the AER capability */ > - (void)pci_disable_pcie_error_reporting(pdev); > - > dev_dbg(&pdev->dev, "NT-function PCIe interface cleared"); > } > > -- > 2.25.1 >
diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c index 0ed6f809ff2e..7192ff59dd91 100644 --- a/drivers/ntb/hw/idt/ntb_hw_idt.c +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c @@ -2651,20 +2651,18 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) } /* - * Enable the device advanced error reporting. It's not critical to + * The PCI core enables device error reporting. It's not critical to * have AER disabled in the kernel. + * + * Cleanup nonfatal error status before getting to init. */ - 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); + pci_aer_clear_nonfatal_status(pdev); /* First enable the PCI device */ ret = pcim_enable_device(pdev); if (ret != 0) { dev_err(&pdev->dev, "Failed to enable PCIe device\n"); - goto err_disable_aer; + return ret; } /* @@ -2692,8 +2690,6 @@ static int idt_init_pci(struct idt_ntb_dev *ndev) err_clear_master: pci_clear_master(pdev); -err_disable_aer: - (void)pci_disable_pcie_error_reporting(pdev); return ret; } @@ -2714,9 +2710,6 @@ static void idt_deinit_pci(struct idt_ntb_dev *ndev) /* Clear the bus master disabling the Request TLPs translation */ pci_clear_master(pdev); - /* Disable the AER capability */ - (void)pci_disable_pcie_error_reporting(pdev); - dev_dbg(&pdev->dev, "NT-function PCIe interface cleared"); }