* Re: [PATCH] e1000e: fix use of pci_enable_pcie_error_reporting
2009-08-07 9:36 [PATCH] e1000e: fix use of pci_enable_pcie_error_reporting Xiaotian Feng
@ 2009-08-13 3:46 ` David Miller
2009-08-13 4:29 ` Danny Feng
2009-08-13 4:54 ` Jeff Kirsher
2009-08-13 23:00 ` Jeff Kirsher
2 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2009-08-13 3:46 UTC (permalink / raw)
To: dfeng
Cc: john.ronciak, peter.p.waskiewicz.jr, bruce.w.allan,
jesse.brandeburg, jeffrey.t.kirsher, e1000-devel, netdev,
linux-kernel
From: Xiaotian Feng <dfeng@redhat.com>
Date: Fri, 7 Aug 2009 17:36:14 +0800
> 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 <dfeng@redhat.com>
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.
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 63415bb..e2f0304 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] e1000e: fix use of pci_enable_pcie_error_reporting
2009-08-13 3:46 ` David Miller
@ 2009-08-13 4:29 ` Danny Feng
0 siblings, 0 replies; 5+ messages in thread
From: Danny Feng @ 2009-08-13 4:29 UTC (permalink / raw)
To: David Miller
Cc: e1000-devel, netdev, bruce.w.allan, jesse.brandeburg,
linux-kernel, john.ronciak, jeffrey.t.kirsher
On 08/13/2009 11:46 AM, David Miller wrote:
> From: Xiaotian Feng<dfeng@redhat.com>
> Date: Fri, 7 Aug 2009 17:36:14 +0800
>
>> 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<dfeng@redhat.com>
>
> 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...
>
>> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
>> index 63415bb..e2f0304 100644
>> --- a/drivers/net/e1000e/netdev.c
>> +++ b/drivers/net/e1000e/netdev.c
>> @@ -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
>>
------------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] e1000e: fix use of pci_enable_pcie_error_reporting
2009-08-07 9:36 [PATCH] e1000e: fix use of pci_enable_pcie_error_reporting Xiaotian Feng
2009-08-13 3:46 ` David Miller
@ 2009-08-13 4:54 ` Jeff Kirsher
2009-08-13 23:00 ` Jeff Kirsher
2 siblings, 0 replies; 5+ messages in thread
From: Jeff Kirsher @ 2009-08-13 4:54 UTC (permalink / raw)
To: Xiaotian Feng
Cc: john.ronciak, peter.p.waskiewicz.jr, bruce.w.allan,
jesse.brandeburg, davem, e1000-devel, netdev, linux-kernel
On Fri, Aug 7, 2009 at 02:36, Xiaotian Feng<dfeng@redhat.com> wrote:
> 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 <dfeng@redhat.com>
> ---
> drivers/net/e1000e/netdev.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 63415bb..e2f0304 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -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
>
> --
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.
--
Cheers,
Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] e1000e: fix use of pci_enable_pcie_error_reporting
2009-08-07 9:36 [PATCH] e1000e: fix use of pci_enable_pcie_error_reporting Xiaotian Feng
2009-08-13 3:46 ` David Miller
2009-08-13 4:54 ` Jeff Kirsher
@ 2009-08-13 23:00 ` Jeff Kirsher
2 siblings, 0 replies; 5+ messages in thread
From: Jeff Kirsher @ 2009-08-13 23:00 UTC (permalink / raw)
To: Xiaotian Feng
Cc: john.ronciak, peter.p.waskiewicz.jr, bruce.w.allan,
jesse.brandeburg, davem, e1000-devel, netdev, linux-kernel
On Fri, Aug 7, 2009 at 02:36, Xiaotian Feng<dfeng@redhat.com> wrote:
> 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 <dfeng@redhat.com>
> ---
> drivers/net/e1000e/netdev.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 63415bb..e2f0304 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -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
>
> --
This patch passed testing, I have added it to my queue of patches and
will push this out to Dave (with the whitespace corrected).
--
Cheers,
Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread