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

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?

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

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);
>   }
>

[+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

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

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.