[PATCH 0/9] PCI/AER: Remove redundant Device Control Error Reporting Enable

[PATCH 0/9] PCI/AER: Remove redundant Device Control Error Reporting Enable

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Since f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native"),
ths PCI core sets the Device Control bits that enable error reporting for
PCIe devices.

This series removes redundant calls to pci_enable_pcie_error_reporting()
that do the same thing from the AER driver and several NIC drivers.

There are several more drivers where this should be removed; I started with
just the Intel drivers here.

Bjorn Helgaas (9):
  PCI/AER: Remove redundant Device Control Error Reporting Enable
  e1000e: Remove redundant pci_enable_pcie_error_reporting()
  fm10k: Remove redundant pci_enable_pcie_error_reporting()
  i40e: Remove redundant pci_enable_pcie_error_reporting()
  iavf: Remove redundant pci_enable_pcie_error_reporting()
  ice: Remove redundant pci_enable_pcie_error_reporting()
  igb: Remove redundant pci_enable_pcie_error_reporting()
  igc: Remove redundant pci_enable_pcie_error_reporting()
  ixgbe: Remove redundant pci_enable_pcie_error_reporting()

 drivers/net/ethernet/intel/e1000e/netdev.c    |  7 ---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c  |  5 --
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  4 --
 drivers/net/ethernet/intel/iavf/iavf_main.c   |  5 --
 drivers/net/ethernet/intel/ice/ice_main.c     |  3 --
 drivers/net/ethernet/intel/igb/igb_main.c     |  5 --
 drivers/net/ethernet/intel/igc/igc_main.c     |  5 --
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  5 --
 drivers/pci/pcie/aer.c                        | 48 -------------------
 9 files changed, 87 deletions(-)

-- 
2.25.1

[PATCH 1/9] PCI/AER: Remove redundant Device Control Error Reporting Enable

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

The following bits in the PCIe Device Control register enable sending of
ERR_COR, ERR_NONFATAL, or ERR_FATAL Messages (or reporting internally in
the case of Root Ports):

  Correctable Error Reporting Enable
  Non-Fatal Error Reporting Enable
  Fatal Error Reporting Enable
  Unsupported Request Reporting Enable

These enable bits are set by pci_enable_pcie_error_reporting(), and since
f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native"), we
do that in this path during enumeration:

  pci_init_capabilities
    pci_aer_init
      pci_enable_pcie_error_reporting

Previously, the AER service driver also traversed the hierarchy when
claiming a Root Port, enabling error reporting for downstream devices, but
this is redundant.

Remove the code that enables this error reporting in the AER .probe() path.
Also remove similar code that disables error reporting in the AER .remove()
path.

Note that these Device Control Reporting Enable bits do not control
interrupt generation.  That's done by the similarly-named bits in the AER
Root Error Command register, which are still set by aer_probe() and cleared
by aer_remove(), since the AER service driver handles those interrupts.
See PCIe r6.0, sec 6.2.6.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Keith Busch <kbusch@kernel.org>
---
 drivers/pci/pcie/aer.c | 48 ------------------------------------------
 1 file changed, 48 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 625f7b2cafe4..b7b69e0c778c 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1224,42 +1224,6 @@ static irqreturn_t aer_irq(int irq, void *context)
 	return IRQ_WAKE_THREAD;
 }
 
-static int set_device_error_reporting(struct pci_dev *dev, void *data)
-{
-	bool enable = *((bool *)data);
-	int type = pci_pcie_type(dev);
-
-	if ((type == PCI_EXP_TYPE_ROOT_PORT) ||
-	    (type == PCI_EXP_TYPE_RC_EC) ||
-	    (type == PCI_EXP_TYPE_UPSTREAM) ||
-	    (type == PCI_EXP_TYPE_DOWNSTREAM)) {
-		if (enable)
-			pci_enable_pcie_error_reporting(dev);
-		else
-			pci_disable_pcie_error_reporting(dev);
-	}
-
-	return 0;
-}
-
-/**
- * set_downstream_devices_error_reporting - enable/disable the error reporting  bits on the root port and its downstream ports.
- * @dev: pointer to root port's pci_dev data structure
- * @enable: true = enable error reporting, false = disable error reporting.
- */
-static void set_downstream_devices_error_reporting(struct pci_dev *dev,
-						   bool enable)
-{
-	set_device_error_reporting(dev, &enable);
-
-	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC)
-		pcie_walk_rcec(dev, set_device_error_reporting, &enable);
-	else if (dev->subordinate)
-		pci_walk_bus(dev->subordinate, set_device_error_reporting,
-			     &enable);
-
-}
-
 /**
  * aer_enable_rootport - enable Root Port's interrupts when receiving messages
  * @rpc: pointer to a Root Port data structure
@@ -1289,12 +1253,6 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
 	pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, &reg32);
 	pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32);
 
-	/*
-	 * Enable error reporting for the root port device and downstream port
-	 * devices.
-	 */
-	set_downstream_devices_error_reporting(pdev, true);
-
 	/* Enable Root Port's interrupt in response to error messages */
 	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
 	reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
@@ -1313,12 +1271,6 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
 	int aer = pdev->aer_cap;
 	u32 reg32;
 
-	/*
-	 * Disable error reporting for the root port device and downstream port
-	 * devices.
-	 */
-	set_downstream_devices_error_reporting(pdev, false);
-
 	/* Disable Root's interrupt in response to error messages */
 	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
 	reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
-- 
2.25.1

[PATCH 2/9] e1000e: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

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.

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 doesn't control interrupt generation by the Root Port; that
is controlled by the AER Root Error Command register, which is managed by
the AER service driver.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 04acd1a992fa..e1eb1de88bf9 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7418,9 +7418,6 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto err_pci_reg;
 
-	/* AER (Advanced Error Reporting) hooks */
-	pci_enable_pcie_error_reporting(pdev);
-
 	pci_set_master(pdev);
 	/* PCI config space info */
 	err = pci_save_state(pdev);
@@ -7708,7 +7705,6 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_ioremap:
 	free_netdev(netdev);
 err_alloc_etherdev:
-	pci_disable_pcie_error_reporting(pdev);
 	pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
@@ -7775,9 +7771,6 @@ static void e1000_remove(struct pci_dev *pdev)
 
 	free_netdev(netdev);
 
-	/* AER disable */
-	pci_disable_pcie_error_reporting(pdev);
-
 	pci_disable_device(pdev);
 }
 
-- 
2.25.1

[PATCH 3/9] fm10k: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

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.

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 doesn't control interrupt generation by the Root Port; that
is controlled by the AER Root Error Command register, which is managed by
the AER service driver.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index b473cb7d7c57..027d721feb18 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -2127,8 +2127,6 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_pci_reg;
 	}
 
-	pci_enable_pcie_error_reporting(pdev);
-
 	pci_set_master(pdev);
 	pci_save_state(pdev);
 
@@ -2227,7 +2225,6 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_ioremap:
 	free_netdev(netdev);
 err_alloc_netdev:
-	pci_disable_pcie_error_reporting(pdev);
 	pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
@@ -2281,8 +2278,6 @@ static void fm10k_remove(struct pci_dev *pdev)
 
 	pci_release_mem_regions(pdev);
 
-	pci_disable_pcie_error_reporting(pdev);
-
 	pci_disable_device(pdev);
 }
 
-- 
2.25.1

[PATCH 4/9] i40e: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

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.

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 doesn't control interrupt generation by the Root Port; that
is controlled by the AER Root Error Command register, which is managed by
the AER service driver.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 53d0083e35da..43693f902c27 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -15589,7 +15589,6 @@ static int i40e_init_recovery_mode(struct i40e_pf *pf, struct i40e_hw *hw)
 	timer_shutdown_sync(&pf->service_timer);
 	i40e_shutdown_adminq(hw);
 	iounmap(hw->hw_addr);
-	pci_disable_pcie_error_reporting(pf->pdev);
 	pci_release_mem_regions(pf->pdev);
 	pci_disable_device(pf->pdev);
 	kfree(pf);
@@ -15660,7 +15659,6 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_pci_reg;
 	}
 
-	pci_enable_pcie_error_reporting(pdev);
 	pci_set_master(pdev);
 
 	/* Now that we have a PCI connection, we need to do the
@@ -16218,7 +16216,6 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_ioremap:
 	kfree(pf);
 err_pf_alloc:
-	pci_disable_pcie_error_reporting(pdev);
 	pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
@@ -16366,7 +16363,6 @@ static void i40e_remove(struct pci_dev *pdev)
 	kfree(pf);
 	pci_release_mem_regions(pdev);
 
-	pci_disable_pcie_error_reporting(pdev);
 	pci_disable_device(pdev);
 }
 
-- 
2.25.1

[PATCH 5/9] iavf: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

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.

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 doesn't control interrupt generation by the Root Port; that
is controlled by the AER Root Error Command register, which is managed by
the AER service driver.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/iavf/iavf_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index c4e451ef7942..2835af20ec19 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -4876,8 +4876,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_pci_reg;
 	}
 
-	pci_enable_pcie_error_reporting(pdev);
-
 	pci_set_master(pdev);
 
 	netdev = alloc_etherdev_mq(sizeof(struct iavf_adapter),
@@ -4956,7 +4954,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_ioremap:
 	free_netdev(netdev);
 err_alloc_etherdev:
-	pci_disable_pcie_error_reporting(pdev);
 	pci_release_regions(pdev);
 err_pci_reg:
 err_dma:
@@ -5172,8 +5169,6 @@ static void iavf_remove(struct pci_dev *pdev)
 
 	free_netdev(netdev);
 
-	pci_disable_pcie_error_reporting(pdev);
-
 	pci_disable_device(pdev);
 }
 
-- 
2.25.1

[PATCH 6/9] ice: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

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.

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 doesn't control interrupt generation by the Root Port; that
is controlled by the AER Root Error Command register, which is managed by
the AER service driver.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/ice/ice_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index a9a7f8b52140..9fb68919df02 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4672,7 +4672,6 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 		return err;
 	}
 
-	pci_enable_pcie_error_reporting(pdev);
 	pci_set_master(pdev);
 
 	pf->pdev = pdev;
@@ -4975,7 +4974,6 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 	ice_devlink_destroy_regions(pf);
 	ice_deinit_hw(hw);
 err_exit_unroll:
-	pci_disable_pcie_error_reporting(pdev);
 	pci_disable_device(pdev);
 	return err;
 }
@@ -5103,7 +5101,6 @@ static void ice_remove(struct pci_dev *pdev)
 	ice_reset(&pf->hw, ICE_RESET_PFR);
 	pci_wait_for_pending_transaction(pdev);
 	ice_clear_interrupt_scheme(pf);
-	pci_disable_pcie_error_reporting(pdev);
 	pci_disable_device(pdev);
 }
 
-- 
2.25.1

[PATCH 7/9] igb: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

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.

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 doesn't control interrupt generation by the Root Port; that
is controlled by the AER Root Error Command register, which is managed by
the AER service driver.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/igb/igb_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 3c0c35ecea10..c56b991fa610 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3194,8 +3194,6 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto err_pci_reg;
 
-	pci_enable_pcie_error_reporting(pdev);
-
 	pci_set_master(pdev);
 	pci_save_state(pdev);
 
@@ -3626,7 +3624,6 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_ioremap:
 	free_netdev(netdev);
 err_alloc_etherdev:
-	pci_disable_pcie_error_reporting(pdev);
 	pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
@@ -3837,8 +3834,6 @@ static void igb_remove(struct pci_dev *pdev)
 	kfree(adapter->shadow_vfta);
 	free_netdev(netdev);
 
-	pci_disable_pcie_error_reporting(pdev);
-
 	pci_disable_device(pdev);
 }
 
-- 
2.25.1

[PATCH 8/9] igc: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

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.

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 doesn't control interrupt generation by the Root Port; that
is controlled by the AER Root Error Command register, which is managed by
the AER service driver.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/igc/igc_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 44b1740dc098..a4df4ef088a9 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6430,8 +6430,6 @@ static int igc_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_pci_reg;
 
-	pci_enable_pcie_error_reporting(pdev);
-
 	err = pci_enable_ptm(pdev, NULL);
 	if (err < 0)
 		dev_info(&pdev->dev, "PCIe PTM not supported by PCIe bus/controller\n");
@@ -6636,7 +6634,6 @@ static int igc_probe(struct pci_dev *pdev,
 err_ioremap:
 	free_netdev(netdev);
 err_alloc_etherdev:
-	pci_disable_pcie_error_reporting(pdev);
 	pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
@@ -6684,8 +6681,6 @@ static void igc_remove(struct pci_dev *pdev)
 
 	free_netdev(netdev);
 
-	pci_disable_pcie_error_reporting(pdev);
-
 	pci_disable_device(pdev);
 }
 
-- 
2.25.1

[PATCH 9/9] ixgbe: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

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.

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 doesn't control interrupt generation by the Root Port; that
is controlled by the AER Root Error Command register, which is managed by
the AER service driver.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ab8370c413f3..cb718c6216fc 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10808,8 +10808,6 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_pci_reg;
 	}
 
-	pci_enable_pcie_error_reporting(pdev);
-
 	pci_set_master(pdev);
 	pci_save_state(pdev);
 
@@ -11237,7 +11235,6 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	disable_dev = !test_and_set_bit(__IXGBE_DISABLED, &adapter->state);
 	free_netdev(netdev);
 err_alloc_etherdev:
-	pci_disable_pcie_error_reporting(pdev);
 	pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
@@ -11326,8 +11323,6 @@ static void ixgbe_remove(struct pci_dev *pdev)
 	disable_dev = !test_and_set_bit(__IXGBE_DISABLED, &adapter->state);
 	free_netdev(netdev);
 
-	pci_disable_pcie_error_reporting(pdev);
-
 	if (disable_dev)
 		pci_disable_device(pdev);
 }
-- 
2.25.1

Re: [PATCH 1/9] PCI/AER: Remove redundant Device Control Error Reporting Enable

[Sathyanarayanan Kuppuswamy]

On 1/18/23 3:46 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> The following bits in the PCIe Device Control register enable sending of
> ERR_COR, ERR_NONFATAL, or ERR_FATAL Messages (or reporting internally in
> the case of Root Ports):
> 
>   Correctable Error Reporting Enable
>   Non-Fatal Error Reporting Enable
>   Fatal Error Reporting Enable
>   Unsupported Request Reporting Enable
> 
> These enable bits are set by pci_enable_pcie_error_reporting(), and since
> f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native"), we
> do that in this path during enumeration:
> 
>   pci_init_capabilities
>     pci_aer_init
>       pci_enable_pcie_error_reporting
> 
> Previously, the AER service driver also traversed the hierarchy when
> claiming a Root Port, enabling error reporting for downstream devices, but
> this is redundant.
> 
> Remove the code that enables this error reporting in the AER .probe() path.
> Also remove similar code that disables error reporting in the AER .remove()
> path.
> 
> Note that these Device Control Reporting Enable bits do not control
> interrupt generation.  That's done by the similarly-named bits in the AER
> Root Error Command register, which are still set by aer_probe() and cleared
> by aer_remove(), since the AER service driver handles those interrupts.
> See PCIe r6.0, sec 6.2.6.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Cc: Keith Busch <kbusch@kernel.org>
> ---

Looks fine to me.

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>  drivers/pci/pcie/aer.c | 48 ------------------------------------------
>  1 file changed, 48 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 625f7b2cafe4..b7b69e0c778c 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1224,42 +1224,6 @@ static irqreturn_t aer_irq(int irq, void *context)
>  	return IRQ_WAKE_THREAD;
>  }
>  
> -static int set_device_error_reporting(struct pci_dev *dev, void *data)
> -{
> -	bool enable = *((bool *)data);
> -	int type = pci_pcie_type(dev);
> -
> -	if ((type == PCI_EXP_TYPE_ROOT_PORT) ||
> -	    (type == PCI_EXP_TYPE_RC_EC) ||
> -	    (type == PCI_EXP_TYPE_UPSTREAM) ||
> -	    (type == PCI_EXP_TYPE_DOWNSTREAM)) {
> -		if (enable)
> -			pci_enable_pcie_error_reporting(dev);
> -		else
> -			pci_disable_pcie_error_reporting(dev);
> -	}
> -
> -	return 0;
> -}
> -
> -/**
> - * set_downstream_devices_error_reporting - enable/disable the error reporting  bits on the root port and its downstream ports.
> - * @dev: pointer to root port's pci_dev data structure
> - * @enable: true = enable error reporting, false = disable error reporting.
> - */
> -static void set_downstream_devices_error_reporting(struct pci_dev *dev,
> -						   bool enable)
> -{
> -	set_device_error_reporting(dev, &enable);
> -
> -	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC)
> -		pcie_walk_rcec(dev, set_device_error_reporting, &enable);
> -	else if (dev->subordinate)
> -		pci_walk_bus(dev->subordinate, set_device_error_reporting,
> -			     &enable);
> -
> -}
> -
>  /**
>   * aer_enable_rootport - enable Root Port's interrupts when receiving messages
>   * @rpc: pointer to a Root Port data structure
> @@ -1289,12 +1253,6 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
>  	pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, &reg32);
>  	pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32);
>  
> -	/*
> -	 * Enable error reporting for the root port device and downstream port
> -	 * devices.
> -	 */
> -	set_downstream_devices_error_reporting(pdev, true);
> -
>  	/* Enable Root Port's interrupt in response to error messages */
>  	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
>  	reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> @@ -1313,12 +1271,6 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
>  	int aer = pdev->aer_cap;
>  	u32 reg32;
>  
> -	/*
> -	 * Disable error reporting for the root port device and downstream port
> -	 * devices.
> -	 */
> -	set_downstream_devices_error_reporting(pdev, false);
> -
>  	/* Disable Root's interrupt in response to error messages */
>  	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
>  	reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH 0/9] PCI/AER: Remove redundant Device Control Error Reporting Enable

[Sathyanarayanan Kuppuswamy]

On 1/18/23 3:46 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Since f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native"),
> ths PCI core sets the Device Control bits that enable error reporting for
> PCIe devices.
> 
> This series removes redundant calls to pci_enable_pcie_error_reporting()
> that do the same thing from the AER driver and several NIC drivers.
> 
> There are several more drivers where this should be removed; I started with
> just the Intel drivers here.
> 
> Bjorn Helgaas (9):
>   PCI/AER: Remove redundant Device Control Error Reporting Enable
>   e1000e: Remove redundant pci_enable_pcie_error_reporting()
>   fm10k: Remove redundant pci_enable_pcie_error_reporting()
>   i40e: Remove redundant pci_enable_pcie_error_reporting()
>   iavf: Remove redundant pci_enable_pcie_error_reporting()
>   ice: Remove redundant pci_enable_pcie_error_reporting()
>   igb: Remove redundant pci_enable_pcie_error_reporting()
>   igc: Remove redundant pci_enable_pcie_error_reporting()
>   ixgbe: Remove redundant pci_enable_pcie_error_reporting()

It should be simpler to do in one patch. Any reason to split
it into multiple patches?

> 
>  drivers/net/ethernet/intel/e1000e/netdev.c    |  7 ---
>  drivers/net/ethernet/intel/fm10k/fm10k_pci.c  |  5 --
>  drivers/net/ethernet/intel/i40e/i40e_main.c   |  4 --
>  drivers/net/ethernet/intel/iavf/iavf_main.c   |  5 --
>  drivers/net/ethernet/intel/ice/ice_main.c     |  3 --
>  drivers/net/ethernet/intel/igb/igb_main.c     |  5 --
>  drivers/net/ethernet/intel/igc/igc_main.c     |  5 --
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  5 --
>  drivers/pci/pcie/aer.c                        | 48 -------------------
>  9 files changed, 87 deletions(-)
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH 0/9] PCI/AER: Remove redundant Device Control Error Reporting Enable

[Bjorn Helgaas]

On Wed, Jan 18, 2023 at 07:55:33PM -0800, Sathyanarayanan Kuppuswamy wrote:
> On 1/18/23 3:46 PM, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > Since f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native"),
> > ths PCI core sets the Device Control bits that enable error reporting for
> > PCIe devices.
> > 
> > This series removes redundant calls to pci_enable_pcie_error_reporting()
> > that do the same thing from the AER driver and several NIC drivers.
> > 
> > There are several more drivers where this should be removed; I started with
> > just the Intel drivers here.
> > 
> > Bjorn Helgaas (9):
> >   PCI/AER: Remove redundant Device Control Error Reporting Enable
> >   e1000e: Remove redundant pci_enable_pcie_error_reporting()
> >   fm10k: Remove redundant pci_enable_pcie_error_reporting()
> >   i40e: Remove redundant pci_enable_pcie_error_reporting()
> >   iavf: Remove redundant pci_enable_pcie_error_reporting()
> >   ice: Remove redundant pci_enable_pcie_error_reporting()
> >   igb: Remove redundant pci_enable_pcie_error_reporting()
> >   igc: Remove redundant pci_enable_pcie_error_reporting()
> >   ixgbe: Remove redundant pci_enable_pcie_error_reporting()
> 
> It should be simpler to do in one patch. Any reason to split
> it into multiple patches?

Sure, the driver patches could easily be squashed, either by me or be
the netdev folks if they prefer it that way.  There are close to 50
callers, and I hesitate to do them all in a single patch because it
becomes unwieldy to backport (probably pointless for this situation)
or to revert if there's any issue.

These are all trivial removals, but there are a few that are more
complicated and will require closer review, so I didn't include those
here.

> >  drivers/net/ethernet/intel/e1000e/netdev.c    |  7 ---
> >  drivers/net/ethernet/intel/fm10k/fm10k_pci.c  |  5 --
> >  drivers/net/ethernet/intel/i40e/i40e_main.c   |  4 --
> >  drivers/net/ethernet/intel/iavf/iavf_main.c   |  5 --
> >  drivers/net/ethernet/intel/ice/ice_main.c     |  3 --
> >  drivers/net/ethernet/intel/igb/igb_main.c     |  5 --
> >  drivers/net/ethernet/intel/igc/igc_main.c     |  5 --
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  5 --
> >  drivers/pci/pcie/aer.c                        | 48 -------------------
> >  9 files changed, 87 deletions(-)
> > 
> 
> -- 
> Sathyanarayanan Kuppuswamy
> Linux Kernel Developer

Re: [PATCH 2/9] e1000e: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/18/2023 3:46 PM, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 7 -------
>   1 file changed, 7 deletions(-)

Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 04acd1a992fa..e1eb1de88bf9 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -7418,9 +7418,6 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	if (err)
>   		goto err_pci_reg;
>   
> -	/* AER (Advanced Error Reporting) hooks */
> -	pci_enable_pcie_error_reporting(pdev);
> -
>   	pci_set_master(pdev);
>   	/* PCI config space info */
>   	err = pci_save_state(pdev);
> @@ -7708,7 +7705,6 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   err_ioremap:
>   	free_netdev(netdev);
>   err_alloc_etherdev:
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_release_mem_regions(pdev);
>   err_pci_reg:
>   err_dma:
> @@ -7775,9 +7771,6 @@ static void e1000_remove(struct pci_dev *pdev)
>   
>   	free_netdev(netdev);
>   
> -	/* AER disable */
> -	pci_disable_pcie_error_reporting(pdev);
> -
>   	pci_disable_device(pdev);
>   }
>   

Re: [PATCH 3/9] fm10k: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/18/2023 3:46 PM, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 5 -----
>   1 file changed, 5 deletions(-)

Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> index b473cb7d7c57..027d721feb18 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> @@ -2127,8 +2127,6 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		goto err_pci_reg;
>   	}
>   
> -	pci_enable_pcie_error_reporting(pdev);
> -
>   	pci_set_master(pdev);
>   	pci_save_state(pdev);
>   
> @@ -2227,7 +2225,6 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   err_ioremap:
>   	free_netdev(netdev);
>   err_alloc_netdev:
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_release_mem_regions(pdev);
>   err_pci_reg:
>   err_dma:
> @@ -2281,8 +2278,6 @@ static void fm10k_remove(struct pci_dev *pdev)
>   
>   	pci_release_mem_regions(pdev);
>   
> -	pci_disable_pcie_error_reporting(pdev);
> -
>   	pci_disable_device(pdev);
>   }
>   

Re: [PATCH 4/9] i40e: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/18/2023 3:46 PM, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ----
>   1 file changed, 4 deletions(-)

Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 53d0083e35da..43693f902c27 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -15589,7 +15589,6 @@ static int i40e_init_recovery_mode(struct i40e_pf *pf, struct i40e_hw *hw)
>   	timer_shutdown_sync(&pf->service_timer);
>   	i40e_shutdown_adminq(hw);
>   	iounmap(hw->hw_addr);
> -	pci_disable_pcie_error_reporting(pf->pdev);
>   	pci_release_mem_regions(pf->pdev);
>   	pci_disable_device(pf->pdev);
>   	kfree(pf);
> @@ -15660,7 +15659,6 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		goto err_pci_reg;
>   	}
>   
> -	pci_enable_pcie_error_reporting(pdev);
>   	pci_set_master(pdev);
>   
>   	/* Now that we have a PCI connection, we need to do the
> @@ -16218,7 +16216,6 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   err_ioremap:
>   	kfree(pf);
>   err_pf_alloc:
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_release_mem_regions(pdev);
>   err_pci_reg:
>   err_dma:
> @@ -16366,7 +16363,6 @@ static void i40e_remove(struct pci_dev *pdev)
>   	kfree(pf);
>   	pci_release_mem_regions(pdev);
>   
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_disable_device(pdev);
>   }
>   

Re: [PATCH 5/9] iavf: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/18/2023 3:46 PM, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/iavf/iavf_main.c | 5 -----
>   1 file changed, 5 deletions(-)

Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index c4e451ef7942..2835af20ec19 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -4876,8 +4876,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		goto err_pci_reg;
>   	}
>   
> -	pci_enable_pcie_error_reporting(pdev);
> -
>   	pci_set_master(pdev);
>   
>   	netdev = alloc_etherdev_mq(sizeof(struct iavf_adapter),
> @@ -4956,7 +4954,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   err_ioremap:
>   	free_netdev(netdev);
>   err_alloc_etherdev:
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_release_regions(pdev);
>   err_pci_reg:
>   err_dma:
> @@ -5172,8 +5169,6 @@ static void iavf_remove(struct pci_dev *pdev)
>   
>   	free_netdev(netdev);
>   
> -	pci_disable_pcie_error_reporting(pdev);
> -
>   	pci_disable_device(pdev);
>   }
>   

Re: [PATCH 6/9] ice: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/18/2023 3:46 PM, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/ice/ice_main.c | 3 ---
>   1 file changed, 3 deletions(-)

Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index a9a7f8b52140..9fb68919df02 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -4672,7 +4672,6 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
>   		return err;
>   	}
>   
> -	pci_enable_pcie_error_reporting(pdev);
>   	pci_set_master(pdev);
>   
>   	pf->pdev = pdev;
> @@ -4975,7 +4974,6 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
>   	ice_devlink_destroy_regions(pf);
>   	ice_deinit_hw(hw);
>   err_exit_unroll:
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_disable_device(pdev);
>   	return err;
>   }
> @@ -5103,7 +5101,6 @@ static void ice_remove(struct pci_dev *pdev)
>   	ice_reset(&pf->hw, ICE_RESET_PFR);
>   	pci_wait_for_pending_transaction(pdev);
>   	ice_clear_interrupt_scheme(pf);
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_disable_device(pdev);
>   }
>   

Re: [PATCH 7/9] igb: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/18/2023 3:46 PM, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 5 -----
>   1 file changed, 5 deletions(-)

Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 3c0c35ecea10..c56b991fa610 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3194,8 +3194,6 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	if (err)
>   		goto err_pci_reg;
>   
> -	pci_enable_pcie_error_reporting(pdev);
> -
>   	pci_set_master(pdev);
>   	pci_save_state(pdev);
>   
> @@ -3626,7 +3624,6 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   err_ioremap:
>   	free_netdev(netdev);
>   err_alloc_etherdev:
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_release_mem_regions(pdev);
>   err_pci_reg:
>   err_dma:
> @@ -3837,8 +3834,6 @@ static void igb_remove(struct pci_dev *pdev)
>   	kfree(adapter->shadow_vfta);
>   	free_netdev(netdev);
>   
> -	pci_disable_pcie_error_reporting(pdev);
> -
>   	pci_disable_device(pdev);
>   }
>   

Re: [PATCH 8/9] igc: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/18/2023 3:46 PM, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 5 -----
>   1 file changed, 5 deletions(-)

Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 44b1740dc098..a4df4ef088a9 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6430,8 +6430,6 @@ static int igc_probe(struct pci_dev *pdev,
>   	if (err)
>   		goto err_pci_reg;
>   
> -	pci_enable_pcie_error_reporting(pdev);
> -
>   	err = pci_enable_ptm(pdev, NULL);
>   	if (err < 0)
>   		dev_info(&pdev->dev, "PCIe PTM not supported by PCIe bus/controller\n");
> @@ -6636,7 +6634,6 @@ static int igc_probe(struct pci_dev *pdev,
>   err_ioremap:
>   	free_netdev(netdev);
>   err_alloc_etherdev:
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_release_mem_regions(pdev);
>   err_pci_reg:
>   err_dma:
> @@ -6684,8 +6681,6 @@ static void igc_remove(struct pci_dev *pdev)
>   
>   	free_netdev(netdev);
>   
> -	pci_disable_pcie_error_reporting(pdev);
> -
>   	pci_disable_device(pdev);
>   }
>   

Re: [PATCH 9/9] ixgbe: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/18/2023 3:46 PM, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 -----
>   1 file changed, 5 deletions(-)

Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index ab8370c413f3..cb718c6216fc 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -10808,8 +10808,6 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		goto err_pci_reg;
>   	}
>   
> -	pci_enable_pcie_error_reporting(pdev);
> -
>   	pci_set_master(pdev);
>   	pci_save_state(pdev);
>   
> @@ -11237,7 +11235,6 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	disable_dev = !test_and_set_bit(__IXGBE_DISABLED, &adapter->state);
>   	free_netdev(netdev);
>   err_alloc_etherdev:
> -	pci_disable_pcie_error_reporting(pdev);
>   	pci_release_mem_regions(pdev);
>   err_pci_reg:
>   err_dma:
> @@ -11326,8 +11323,6 @@ static void ixgbe_remove(struct pci_dev *pdev)
>   	disable_dev = !test_and_set_bit(__IXGBE_DISABLED, &adapter->state);
>   	free_netdev(netdev);
>   
> -	pci_disable_pcie_error_reporting(pdev);
> -
>   	if (disable_dev)
>   		pci_disable_device(pdev);
>   }

Re: [Intel-wired-lan] [PATCH 2/9] e1000e: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

[+cc Sathy]

On Thu, Jan 19, 2023 at 10:28:16AM -0800, Tony Nguyen wrote:
> On 1/18/2023 3:46 PM, 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.
> > 
> > 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 doesn't control interrupt generation by the Root Port; that
> > is controlled by the AER Root Error Command register, which is managed by
> > the AER service driver.
> > 
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> > Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> > Cc: intel-wired-lan@lists.osuosl.org
> > Cc: netdev@vger.kernel.org
> > ---
> >   drivers/net/ethernet/intel/e1000e/netdev.c | 7 -------
> >   1 file changed, 7 deletions(-)
> 
> Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>

Thanks a million for taking a look at these, Tony!

These driver patches are all independent and have no dependency on the
1/9 PCI/AER patch.  What's your opinion on merging these?  Should they
go via netdev?  Should they be squashed into a single patch that does
all the Intel drivers at once?

I'm happy to squash them and/or merge them via the PCI tree, whatever
is easiest.

Bjorn

Re: [Intel-wired-lan] [PATCH 2/9] e1000e: Remove redundant pci_enable_pcie_error_reporting()

[Tony Nguyen]

On 1/19/2023 10:40 AM, Bjorn Helgaas wrote:
> [+cc Sathy]
> 
> On Thu, Jan 19, 2023 at 10:28:16AM -0800, Tony Nguyen wrote:
>> On 1/18/2023 3:46 PM, 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.
>>>
>>> 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 doesn't control interrupt generation by the Root Port; that
>>> is controlled by the AER Root Error Command register, which is managed by
>>> the AER service driver.
>>>
>>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
>>> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
>>> Cc: intel-wired-lan@lists.osuosl.org
>>> Cc: netdev@vger.kernel.org
>>> ---
>>>    drivers/net/ethernet/intel/e1000e/netdev.c | 7 -------
>>>    1 file changed, 7 deletions(-)
>>
>> Reviewed-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> 
> Thanks a million for taking a look at these, Tony!
> 
> These driver patches are all independent and have no dependency on the
> 1/9 PCI/AER patch.  What's your opinion on merging these?  Should they
> go via netdev?  Should they be squashed into a single patch that does
> all the Intel drivers at once?
> 
> I'm happy to squash them and/or merge them via the PCI tree, whatever
> is easiest.

Since there's no dependency, IMO, it'd make sense to go through 
Intel-wired-lan/netdev. Keeping them per driver is fine.

Thanks,
Tony

Re: [PATCH 2/9] e1000e: Remove redundant pci_enable_pcie_error_reporting()

[Jakub Kicinski]

On Wed, 18 Jan 2023 17:46:05 -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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

How would you like to route these? Looks like there's no dependency 
so we can pick them up?

Re: [Intel-wired-lan] [PATCH 2/9] e1000e: Remove redundant pci_enable_pcie_error_reporting()

[Jakub Kicinski]

On Thu, 19 Jan 2023 13:31:39 -0800 Tony Nguyen wrote:
> > Thanks a million for taking a look at these, Tony!
> > 
> > These driver patches are all independent and have no dependency on the
> > 1/9 PCI/AER patch.  What's your opinion on merging these?  Should they
> > go via netdev?  Should they be squashed into a single patch that does
> > all the Intel drivers at once?
> > 
> > I'm happy to squash them and/or merge them via the PCI tree, whatever
> > is easiest.  
> 
> Since there's no dependency, IMO, it'd make sense to go through 
> Intel-wired-lan/netdev. Keeping them per driver is fine.

Ah, damn, I spammed Bjorn with the same question because email was
pooped most of the day :/ Reportedly not vger, email in general but 
fool me once...

Tony, if you could take these via your tree that'd be best.

Re: [Intel-wired-lan] [PATCH 2/9] e1000e: Remove redundant pci_enable_pcie_error_reporting()

[Bjorn Helgaas]

On Thu, Jan 19, 2023 at 07:17:35PM -0800, Jakub Kicinski wrote:
> On Wed, 18 Jan 2023 17:46:05 -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.
> > 
> > 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 doesn't control interrupt generation by the Root Port; that
> > is controlled by the AER Root Error Command register, which is managed by
> > the AER service driver.
> > 
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> How would you like to route these? Looks like there's no dependency 
> so we can pick them up?

Right, no dependencies, so you can pick them up.  Sounds like you and
Tony have it worked out.  Thanks!

Bjorn

Re: [PATCH 1/9] PCI/AER: Remove redundant Device Control Error Reporting Enable

[Stefan Roese]

On 1/19/23 00:46, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> The following bits in the PCIe Device Control register enable sending of
> ERR_COR, ERR_NONFATAL, or ERR_FATAL Messages (or reporting internally in
> the case of Root Ports):
> 
>    Correctable Error Reporting Enable
>    Non-Fatal Error Reporting Enable
>    Fatal Error Reporting Enable
>    Unsupported Request Reporting Enable
> 
> These enable bits are set by pci_enable_pcie_error_reporting(), and since
> f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native"), we
> do that in this path during enumeration:
> 
>    pci_init_capabilities
>      pci_aer_init
>        pci_enable_pcie_error_reporting
> 
> Previously, the AER service driver also traversed the hierarchy when
> claiming a Root Port, enabling error reporting for downstream devices, but
> this is redundant.
> 
> Remove the code that enables this error reporting in the AER .probe() path.
> Also remove similar code that disables error reporting in the AER .remove()
> path.
> 
> Note that these Device Control Reporting Enable bits do not control
> interrupt generation.  That's done by the similarly-named bits in the AER
> Root Error Command register, which are still set by aer_probe() and cleared
> by aer_remove(), since the AER service driver handles those interrupts.
> See PCIe r6.0, sec 6.2.6.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Cc: Keith Busch <kbusch@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pcie/aer.c | 48 ------------------------------------------
>   1 file changed, 48 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 625f7b2cafe4..b7b69e0c778c 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1224,42 +1224,6 @@ static irqreturn_t aer_irq(int irq, void *context)
>   	return IRQ_WAKE_THREAD;
>   }
>   
> -static int set_device_error_reporting(struct pci_dev *dev, void *data)
> -{
> -	bool enable = *((bool *)data);
> -	int type = pci_pcie_type(dev);
> -
> -	if ((type == PCI_EXP_TYPE_ROOT_PORT) ||
> -	    (type == PCI_EXP_TYPE_RC_EC) ||
> -	    (type == PCI_EXP_TYPE_UPSTREAM) ||
> -	    (type == PCI_EXP_TYPE_DOWNSTREAM)) {
> -		if (enable)
> -			pci_enable_pcie_error_reporting(dev);
> -		else
> -			pci_disable_pcie_error_reporting(dev);
> -	}
> -
> -	return 0;
> -}
> -
> -/**
> - * set_downstream_devices_error_reporting - enable/disable the error reporting  bits on the root port and its downstream ports.
> - * @dev: pointer to root port's pci_dev data structure
> - * @enable: true = enable error reporting, false = disable error reporting.
> - */
> -static void set_downstream_devices_error_reporting(struct pci_dev *dev,
> -						   bool enable)
> -{
> -	set_device_error_reporting(dev, &enable);
> -
> -	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC)
> -		pcie_walk_rcec(dev, set_device_error_reporting, &enable);
> -	else if (dev->subordinate)
> -		pci_walk_bus(dev->subordinate, set_device_error_reporting,
> -			     &enable);
> -
> -}
> -
>   /**
>    * aer_enable_rootport - enable Root Port's interrupts when receiving messages
>    * @rpc: pointer to a Root Port data structure
> @@ -1289,12 +1253,6 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
>   	pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, &reg32);
>   	pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32);
>   
> -	/*
> -	 * Enable error reporting for the root port device and downstream port
> -	 * devices.
> -	 */
> -	set_downstream_devices_error_reporting(pdev, true);
> -
>   	/* Enable Root Port's interrupt in response to error messages */
>   	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
>   	reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> @@ -1313,12 +1271,6 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
>   	int aer = pdev->aer_cap;
>   	u32 reg32;
>   
> -	/*
> -	 * Disable error reporting for the root port device and downstream port
> -	 * devices.
> -	 */
> -	set_downstream_devices_error_reporting(pdev, false);
> -
>   	/* Disable Root's interrupt in response to error messages */
>   	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
>   	reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [Intel-wired-lan] [PATCH 8/9] igc: Remove redundant pci_enable_pcie_error_reporting()

[naamax.meir]

On 1/19/2023 01:46, 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that
> is controlled by the AER Root Error Command register, which is managed by
> the AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 5 -----
>   1 file changed, 5 deletions(-)
Tested-by: Naama Meir <naamax.meir@linux.intel.com>

RE: [Intel-wired-lan] [PATCH 6/9] ice: Remove redundant pci_enable_pcie_error_reporting()

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Bjorn Helgaas
> Sent: Thursday, January 19, 2023 5:16 AM
> To: linux-pci@vger.kernel.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; intel-wired-lan@lists.osuosl.org; Bjorn
> Helgaas <bhelgaas@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH 6/9] ice: Remove redundant
> pci_enable_pcie_error_reporting()
> 
> 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that is
> controlled by the AER Root Error Command register, which is managed by the
> AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 3 ---
>  1 file changed, 3 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)

RE: [Intel-wired-lan] [PATCH 7/9] igb: Remove redundant pci_enable_pcie_error_reporting()

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Bjorn Helgaas
> Sent: Thursday, January 19, 2023 5:16 AM
> To: linux-pci@vger.kernel.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; intel-wired-lan@lists.osuosl.org; Bjorn
> Helgaas <bhelgaas@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH 7/9] igb: Remove redundant
> pci_enable_pcie_error_reporting()
> 
> 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that is
> controlled by the AER Root Error Command register, which is managed by the
> AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 5 -----
>  1 file changed, 5 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)

RE: [Intel-wired-lan] [PATCH 9/9] ixgbe: Remove redundant pci_enable_pcie_error_reporting()

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Bjorn Helgaas
> Sent: Thursday, January 19, 2023 5:16 AM
> To: linux-pci@vger.kernel.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; intel-wired-lan@lists.osuosl.org; Bjorn
> Helgaas <bhelgaas@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH 9/9] ixgbe: Remove redundant
> pci_enable_pcie_error_reporting()
> 
> 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that is
> controlled by the AER Root Error Command register, which is managed by the
> AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 -----
>  1 file changed, 5 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)

RE: [Intel-wired-lan] [PATCH 4/9] i40e: Remove redundant pci_enable_pcie_error_reporting()

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Bjorn Helgaas
> Sent: Thursday, January 19, 2023 5:16 AM
> To: linux-pci@vger.kernel.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; intel-wired-lan@lists.osuosl.org; Bjorn
> Helgaas <bhelgaas@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH 4/9] i40e: Remove redundant
> pci_enable_pcie_error_reporting()
> 
> 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.
> 
> 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 doesn't control interrupt generation by the Root Port; that is
> controlled by the AER Root Error Command register, which is managed by the
> AER service driver.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ----
>  1 file changed, 4 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)

Re: [PATCH 0/9] PCI/AER: Remove redundant Device Control Error Reporting Enable

[Bjorn Helgaas]

[+cc all the folks I forgot to cc when sending the original cover
letter, sorry about that, plus the folks who very generously tested
the driver patches]

On Wed, Jan 18, 2023 at 05:46:03PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Since f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native"),
> ths PCI core sets the Device Control bits that enable error reporting for
> PCIe devices.
> 
> This series removes redundant calls to pci_enable_pcie_error_reporting()
> that do the same thing from the AER driver and several NIC drivers.
> 
> There are several more drivers where this should be removed; I started with
> just the Intel drivers here.
> 
> Bjorn Helgaas (9):
>   PCI/AER: Remove redundant Device Control Error Reporting Enable
>   e1000e: Remove redundant pci_enable_pcie_error_reporting()
>   fm10k: Remove redundant pci_enable_pcie_error_reporting()
>   i40e: Remove redundant pci_enable_pcie_error_reporting()
>   iavf: Remove redundant pci_enable_pcie_error_reporting()
>   ice: Remove redundant pci_enable_pcie_error_reporting()
>   igb: Remove redundant pci_enable_pcie_error_reporting()
>   igc: Remove redundant pci_enable_pcie_error_reporting()
>   ixgbe: Remove redundant pci_enable_pcie_error_reporting()

Thank you very much for reviewing and testing these.  I applied the
first (PCI/AER) patch to the PCI tree.

I think Jakub is planning to merge the rest via the netdev tree.
There are no dependencies, so they can be squashed if desired.  Let me
know if you'd prefer me to merge them or squash them.

>  drivers/net/ethernet/intel/e1000e/netdev.c    |  7 ---
>  drivers/net/ethernet/intel/fm10k/fm10k_pci.c  |  5 --
>  drivers/net/ethernet/intel/i40e/i40e_main.c   |  4 --
>  drivers/net/ethernet/intel/iavf/iavf_main.c   |  5 --
>  drivers/net/ethernet/intel/ice/ice_main.c     |  3 --
>  drivers/net/ethernet/intel/igb/igb_main.c     |  5 --
>  drivers/net/ethernet/intel/igc/igc_main.c     |  5 --
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  5 --
>  drivers/pci/pcie/aer.c                        | 48 -------------------
>  9 files changed, 87 deletions(-)
> 
> -- 
> 2.25.1
>