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

Subsystems: intel ethernet drivers, networking drivers, the rest

STALE1322d

Revision v1 of 5 in this series.

7 messages, 3 authors, 2023-01-20 · open the first message on its own page

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

From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2023-01-18 23:47:01

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 <redacted>
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

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

From: Tony Nguyen <anthony.l.nguyen@intel.com>
Date: 2023-01-19 18:28:35

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 <redacted>
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>
quoted hunk
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: [Intel-wired-lan] [PATCH 2/9] e1000e: Remove redundant pci_enable_pcie_error_reporting()

From: Tony Nguyen <anthony.l.nguyen@intel.com>
Date: 2023-01-19 21:45:16

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:
quoted
On 1/18/2023 3:46 PM, Bjorn Helgaas wrote:
quoted
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 <redacted>
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()

From: Jakub Kicinski <kuba@kernel.org>
Date: 2023-01-20 03:17:50

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()

From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2023-01-20 05:26:32

[+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:
quoted
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 <redacted>
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()

From: Jakub Kicinski <kuba@kernel.org>
Date: 2023-01-20 05:39:04

On Thu, 19 Jan 2023 13:31:39 -0800 Tony Nguyen wrote:
quoted
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()

From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2023-01-20 13:14:31

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:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help