Patchwork Re: e1000e: why does pci_enable_pcie_error_reporting() fail on my hp2510p?

login
register
mail settings
Submitter Frans Pop
Date Aug. 21, 2009, 6:29 a.m.
Message ID <200908210829.10471.elendil@planet.nl>
Download mbox | patch
Permalink /patch/31805/
State Awaiting Upstream
Delegated to: David Miller
Headers show

Comments

Frans Pop - Aug. 21, 2009, 6:29 a.m.
On Friday 21 August 2009, Danny Feng wrote:
> On 08/20/2009 11:14 PM, Frans Pop wrote:
> > e1000e 0000:00:19.0: pci_enable_pcie_error_reporting failed
>
> I think this messages comes from hardware does not support pcie aer.
> In fact the messages is not fatal and can be ignored. Some drivers
> is silence with pci_enable_pcie_error_reporting ....

Right. The only error returned by the function is -EIO, which looks to be
"not supported". That's really not an issue to shout about in dmesg...

> My patch is just to fix the logic of pcie aer usage ...
>
> > - as the error is non-fatal, should it maybe be changed from dev_err
> >    to dev_info, so that at least it does not show up during a boot
> >    with 'quiet'?
>
> I agree with this.... Maybe some of other drivers needs this too
> (igb,ixgbe)...

Here's a patch that simply drops the message.
Note: the two scsi drivers that call the function (arcmsr and qla2xxx)
also don't check the return code.

Alternatives would be:
- change the message to something like "device does not support advanced
  error reporting (AER)" and make it dev_debug
- suppress the error in case of -EIO, but I'm not sure that makes sense
  as -EIO is the only error returned and you might not want to ignore other
  errors

If you'd prefer the alternatives, let me know and I'll modify the patch.

Cheers,
FJP


From: Frans Pop <elendil@planet.nl>
Subject: net: Don't report an error if devices don't support AER

The only error returned by pci_enable_pcie_error_reporting() is -EIO
which simply means that Advanced Error Reporting is not supported.
There is no need to report that, so remove the error check from e1001e,
igb and ixgbe.

Signed-off-by: Frans Pop <elendil@planet.nl>
    ---

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Xiaotian Feng - Aug. 21, 2009, 6:33 a.m.
On 08/21/2009 02:29 PM, Frans Pop wrote:
> On Friday 21 August 2009, Danny Feng wrote:
>> On 08/20/2009 11:14 PM, Frans Pop wrote:
>>> e1000e 0000:00:19.0: pci_enable_pcie_error_reporting failed
>>
>> I think this messages comes from hardware does not support pcie aer.
>> In fact the messages is not fatal and can be ignored. Some drivers
>> is silence with pci_enable_pcie_error_reporting ....
>
> Right. The only error returned by the function is -EIO, which looks to be
> "not supported". That's really not an issue to shout about in dmesg...
>
>> My patch is just to fix the logic of pcie aer usage ...
>>
>>> - as the error is non-fatal, should it maybe be changed from dev_err
>>>     to dev_info, so that at least it does not show up during a boot
>>>     with 'quiet'?
>>
>> I agree with this.... Maybe some of other drivers needs this too
>> (igb,ixgbe)...
>
> Here's a patch that simply drops the message.
> Note: the two scsi drivers that call the function (arcmsr and qla2xxx)
> also don't check the return code.
>
> Alternatives would be:
> - change the message to something like "device does not support advanced
>    error reporting (AER)" and make it dev_debug
> - suppress the error in case of -EIO, but I'm not sure that makes sense
>    as -EIO is the only error returned and you might not want to ignore other
>    errors
>
> If you'd prefer the alternatives, let me know and I'll modify the patch.
You may also need to silence pci_disable_pcie_error_reporting, otherwise 
rmmod/shutdown, you will get

e1000e 0000:00:19.0: pci_disable_pcie_error_reporting failed

>
> Cheers,
> FJP
>
>
> From: Frans Pop<elendil@planet.nl>
> Subject: net: Don't report an error if devices don't support AER
>
> The only error returned by pci_enable_pcie_error_reporting() is -EIO
> which simply means that Advanced Error Reporting is not supported.
> There is no need to report that, so remove the error check from e1001e,
> igb and ixgbe.
>
> Signed-off-by: Frans Pop<elendil@planet.nl>
>      ---
>
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index fa92a68..42ac8f1 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -4983,12 +4983,7 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
>   		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_enable_pcie_error_reporting(pdev);
>
>   	pci_set_master(pdev);
>   	/* PCI config space info */
> diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
> index adb09d3..8e80859 100644
> --- a/drivers/net/igb/igb_main.c
> +++ b/drivers/net/igb/igb_main.c
> @@ -1232,12 +1232,7 @@ static int __devinit igb_probe(struct pci_dev *pdev,
>   	if (err)
>   		goto err_pci_reg;
>
> -	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_enable_pcie_error_reporting(pdev);
>
>   	pci_set_master(pdev);
>   	pci_save_state(pdev);
> diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
> index 77b0381..4e0b523 100644
> --- a/drivers/net/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ixgbe/ixgbe_main.c
> @@ -5430,12 +5430,7 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
>   		goto err_pci_reg;
>   	}
>
> -	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_enable_pcie_error_reporting(pdev);
>
>   	pci_set_master(pdev);
>   	pci_save_state(pdev);
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index fa92a68..42ac8f1 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4983,12 +4983,7 @@  static int __devinit e1000_probe(struct pci_dev *pdev,
 		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_enable_pcie_error_reporting(pdev);
 
 	pci_set_master(pdev);
 	/* PCI config space info */
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index adb09d3..8e80859 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -1232,12 +1232,7 @@  static int __devinit igb_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_pci_reg;
 
-	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_enable_pcie_error_reporting(pdev);
 
 	pci_set_master(pdev);
 	pci_save_state(pdev);
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 77b0381..4e0b523 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -5430,12 +5430,7 @@  static int __devinit ixgbe_probe(struct pci_dev *pdev,
 		goto err_pci_reg;
 	}
 
-	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_enable_pcie_error_reporting(pdev);
 
 	pci_set_master(pdev);
 	pci_save_state(pdev);