e1000e: fix use of pci_enable_pcie_error_reporting

PATCH In review Xiaotian Feng 3 participants 5 messages Aug 7, 2009 – Aug 13, 2009

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Xiaotian Feng

author

2009-08-07 09:36 UTC #0 lore raw

PATCH 1/1 e1000e: fix use of pci_enable_pcie_error_reporting

commit 111b9dc5 introduces pcie aer support for e1000e, but it is not reasonable to disable it in e1000_remove but enable it in e1000_resume. This patch enables aer support in e1000_probe.

Signed-off-by Xiaotian Feng <[email protected]>

---
 drivers/net/e1000e/netdev.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

drivers/net/e1000e/netdev.c +8 -8

index 63415bb..e2f0304 100644
@@ -4670,14 +4670,6 @@ static int e1000_resume(struct pci_dev *pdev)
 		return err;
 	}
 
-	/* AER (Advanced Error Reporting) hooks */
-	err = pci_enable_pcie_error_reporting(pdev);
-	if (err) {
-		dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
-		                    "0x%x\n", err);
-		/* non-fatal, continue */
-	}
-
 	pci_set_master(pdev);
 
 	pci_enable_wake(pdev, PCI_D3hot, 0);

@@ -4990,6 +4982,14 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_pci_reg;
 
+        /* AER (Advanced Error Reporting) hooks */
+        err = pci_enable_pcie_error_reporting(pdev);
+        if (err) {
+                dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
+                                    "0x%x\n", err);
+                /* non-fatal, continue */
+        }
+
 	pci_set_master(pdev);
 	/* PCI config space info */
 	err = pci_save_state(pdev);

Signature · 8 lines

1.6.2.5


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

David Miller

maintainer

2009-08-13 03:46 UTC #1 lore raw

From: Xiaotian Feng <[email protected]>
Date: Fri, 7 Aug 2009 17:36:14 +0800

Quoted 5 lines

commit 111b9dc5 introduces pcie aer support for e1000e, but it is not reasonable to disable it in e1000_remove but enable it in e1000_resume. This patch enables aer support in e1000_probe.
Signed-off-by Xiaotian Feng <[email protected]>

In moving this block of code, you've corrupted the indentation, making it more indented than it should be.

In any event, I expect the Intel folks to pick this up.

Quoted 37 lines

drivers/net/e1000e/netdev.c +8 -8

index 63415bb..e2f0304 100644
@@ -4670,14 +4670,6 @@ static int e1000_resume(struct pci_dev *pdev)
 		return err;
 	}
 
-	/* AER (Advanced Error Reporting) hooks */
-	err = pci_enable_pcie_error_reporting(pdev);
-	if (err) {
-		dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
-		                    "0x%x\n", err);
-		/* non-fatal, continue */
-	}
-
 	pci_set_master(pdev);
 
 	pci_enable_wake(pdev, PCI_D3hot, 0);

@@ -4990,6 +4982,14 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_pci_reg;
 
+        /* AER (Advanced Error Reporting) hooks */
+        err = pci_enable_pcie_error_reporting(pdev);
+        if (err) {
+                dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
+                                    "0x%x\n", err);
+                /* non-fatal, continue */
+        }
+
 	pci_set_master(pdev);
 	/* PCI config space info */
 	err = pci_save_state(pdev);

Signature · 1 line

1.6.2.5

Danny Feng

author

2009-08-13 04:29 UTC #2 lore raw

Quoted 7 lines · David Miller

From: Xiaotian Feng<[email protected]>
Date: Fri, 7 Aug 2009 17:36:14 +0800
Quoted 4 lines
commit 111b9dc5 introduces pcie aer support for e1000e, but it is not reasonable to disable it in e1000_remove but enable it in e1000_resume. This patch enables aer support in e1000_probe.

Quoted 6 lines

Quoted 1 line
Signed-off-by Xiaotian Feng<[email protected]>
In moving this block of code, you've corrupted the indentation, making it more indented than it should be.
In any event, I expect the Intel folks to pick this up.

Yes, I agree.... but each time I resume from suspend or rmmod e1000e, there's a warning message like "pci_enable_pcie_error_reporting failed 0xfffffffb". Since some devices may not support aer, why not silence this kind of warning?

modprobe e1000e everything is as usual, but then rmmod e1000e, we'll see "pci_disable_pcie_error_reporting failed 0xfffffffb", it is so weird...

Quoted 24 lines

drivers/net/e1000e/netdev.c +0 -8

index 63415bb..e2f0304 100644
@@ -4670,14 +4670,6 @@ static int e1000_resume(struct pci_dev *pdev)
  		return err;
  	}
 
-	/* AER (Advanced Error Reporting) hooks */
-	err = pci_enable_pcie_error_reporting(pdev);
-	if (err) {
-		dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
-		                    "0x%x\n", err);
-		/* non-fatal, continue */
-	}
-
  	pci_set_master(pdev);
 
  	pci_enable_wake(pdev, PCI_D3hot, 0);

@@ -4990,6 +4982,14 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
  	if (err)
  		goto err_pci_reg;

Quoted 14 lines

drivers/net/e1000e/netdev.c +8 -0

+        /* AER (Advanced Error Reporting) hooks */
+        err = pci_enable_pcie_error_reporting(pdev);
+        if (err) {
+                dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
+                                    "0x%x\n", err);
+                /* non-fatal, continue */
+        }
+
  	pci_set_master(pdev);
  	/* PCI config space info */
  	err = pci_save_state(pdev);

Signature · 1 line

1.6.2.5

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july

Jeff Kirsher

2009-08-13 04:54 UTC #3 lore raw

In reply to Xiaotian Feng

Quoted 46 lines · Xiaotian Feng

commit 111b9dc5 introduces pcie aer support for e1000e, but it is not reasonable to disable it in e1000_remove but enable it in e1000_resume. This patch enables aer support in e1000_probe.

Signed-off-by Xiaotian Feng <[email protected]>

---
drivers/net/e1000e/netdev.c | 16 ++++++++--------

 1 files changed, 8 insertions(+), 8 deletions(-)

drivers/net/e1000e/netdev.c +8 -9

index 63415bb..e2f0304 100644
@@ -4670,14 +4670,6 @@ static int e1000_resume(struct pci_dev *pdev)
                return err;
        }
 
-       /* AER (Advanced Error Reporting) hooks */
-       err = pci_enable_pcie_error_reporting(pdev);
-       if (err) {
-               dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
-                                   "0x%x\n", err);
-               /* non-fatal, continue */
-       }
-
        pci_set_master(pdev);
 
        pci_enable_wake(pdev, PCI_D3hot, 0);

@@ -4990,6 +4982,14 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
        if (err)
                goto err_pci_reg;
 
+        /* AER (Advanced Error Reporting) hooks */
+        err = pci_enable_pcie_error_reporting(pdev);
+        if (err) {
+                dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
+                                    "0x%x\n", err);
+                /* non-fatal, continue */
+        }
+
        pci_set_master(pdev);
        /* PCI config space info */
        err = pci_save_state(pdev);
--

1.6.2.5

Quoted 1 line

--

I have added this to my tree for testing. Sorry I did not respond earlier.

Dave - I will make sure that the tabs/whitespace is correct.

Signature · 2 lines

Cheers,
Jeff

Jeff Kirsher

2009-08-13 23:00 UTC #4 lore raw

In reply to Xiaotian Feng

Quoted 46 lines · Xiaotian Feng

commit 111b9dc5 introduces pcie aer support for e1000e, but it is not reasonable to disable it in e1000_remove but enable it in e1000_resume. This patch enables aer support in e1000_probe.

Signed-off-by Xiaotian Feng <[email protected]>

---
drivers/net/e1000e/netdev.c | 16 ++++++++--------

 1 files changed, 8 insertions(+), 8 deletions(-)

drivers/net/e1000e/netdev.c +8 -9

index 63415bb..e2f0304 100644
@@ -4670,14 +4670,6 @@ static int e1000_resume(struct pci_dev *pdev)
                return err;
        }
 
-       /* AER (Advanced Error Reporting) hooks */
-       err = pci_enable_pcie_error_reporting(pdev);
-       if (err) {
-               dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
-                                   "0x%x\n", err);
-               /* non-fatal, continue */
-       }
-
        pci_set_master(pdev);
 
        pci_enable_wake(pdev, PCI_D3hot, 0);

@@ -4990,6 +4982,14 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
        if (err)
                goto err_pci_reg;
 
+        /* AER (Advanced Error Reporting) hooks */
+        err = pci_enable_pcie_error_reporting(pdev);
+        if (err) {
+                dev_err(&pdev->dev, "pci_enable_pcie_error_reporting failed "
+                                    "0x%x\n", err);
+                /* non-fatal, continue */
+        }
+
        pci_set_master(pdev);
        /* PCI config space info */
        err = pci_save_state(pdev);
--

1.6.2.5

Quoted 1 line

--

This patch passed testing, I have added it to my queue of patches and will push this out to Dave (with the whitespace corrected).

Signature · 2 lines

Cheers,
Jeff