[PATCH] e1000e: Simplify MSI interrupt testing

[PATCH] e1000e: Simplify MSI interrupt testing

[Jean Delvare]

The code is quite convoluted, simplify it. This also avoids calling
e1000_request_irq() without testing the value it returned, which was
bad.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
I understand that we need to request the IRQ again after testing, but
why doing it twice?

I sent this patch to the e1000-devel list on August 26th, 2010, but
didn't receive any answer:
http://sourceforge.net/mailarchive/forum.php?thread_name=201008261445.44334.jdelvare%40suse.de&forum_name=e1000-devel

 drivers/net/e1000e/netdev.c |   29 ++++-------------------------
 1 file changed, 4 insertions(+), 25 deletions(-)

--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -3385,22 +3385,16 @@ static int e1000_test_msi_interrupt(stru
 
 	if (adapter->flags & FLAG_MSI_TEST_FAILED) {
 		adapter->int_mode = E1000E_INT_MODE_LEGACY;
-		err = -EIO;
-		e_info("MSI interrupt test failed!\n");
-	}
+		e_info("MSI interrupt test failed, using legacy interrupt.\n");
+	} else
+		e_dbg("MSI interrupt test succeeded!\n");
 
 	free_irq(adapter->pdev->irq, netdev);
 	pci_disable_msi(adapter->pdev);
 
-	if (err == -EIO)
-		goto msi_test_failed;
-
-	/* okay so the test worked, restore settings */
-	e_dbg("MSI interrupt test succeeded!\n");
 msi_test_failed:
 	e1000e_set_interrupt_capability(adapter);
-	e1000_request_irq(adapter);
-	return err;
+	return e1000_request_irq(adapter);
 }
 
 /**
@@ -3432,21 +3426,6 @@ static int e1000_test_msi(struct e1000_a
 		pci_write_config_word(adapter->pdev, PCI_COMMAND, pci_cmd);
 	}
 
-	/* success ! */
-	if (!err)
-		return 0;
-
-	/* EIO means MSI test failed */
-	if (err != -EIO)
-		return err;
-
-	/* back to INTx mode */
-	e_warn("MSI interrupt test failed, using legacy interrupt.\n");
-
-	e1000_free_irq(adapter);
-
-	err = e1000_request_irq(adapter);
-
 	return err;
 }
 

-- 
Jean Delvare
Suse L3

Re: [PATCH] e1000e: Simplify MSI interrupt testing

[David Miller]

From: Jean Delvare <jdelvare@suse.de>
Date: Mon, 13 Sep 2010 10:45:39 +0200

> The code is quite convoluted, simplify it. This also avoids calling
> e1000_request_irq() without testing the value it returned, which was
> bad.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> I understand that we need to request the IRQ again after testing, but
> why doing it twice?
> 
> I sent this patch to the e1000-devel list on August 26th, 2010, but
> didn't receive any answer:
> http://sourceforge.net/mailarchive/forum.php?thread_name=201008261445.44334.jdelvare%40suse.de&forum_name=e1000-devel

Intel folks, please provide feedback or else I'll just blindly apply Jean's
patch to the net-next-2.6 tree as long as it compiles ;-)

Re: [PATCH] e1000e: Simplify MSI interrupt testing

[Jeff Kirsher]

On Mon, Sep 13, 2010 at 20:13, David Miller <davem@davemloft.net> wrote:
> From: Jean Delvare <jdelvare@suse.de>
> Date: Mon, 13 Sep 2010 10:45:39 +0200
>
>> The code is quite convoluted, simplify it. This also avoids calling
>> e1000_request_irq() without testing the value it returned, which was
>> bad.
>>
>> Signed-off-by: Jean Delvare <jdelvare@suse.de>
>> Cc: Bruce Allan <bruce.w.allan@intel.com>
>> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>> I understand that we need to request the IRQ again after testing, but
>> why doing it twice?
>>
>> I sent this patch to the e1000-devel list on August 26th, 2010, but
>> didn't receive any answer:
>> http://sourceforge.net/mailarchive/forum.php?thread_name=201008261445.44334.jdelvare%40suse.de&forum_name=e1000-devel
>
> Intel folks, please provide feedback or else I'll just blindly apply Jean's
> patch to the net-next-2.6 tree as long as it compiles ;-)
> --

Sorry, I believe this patch is good to go.  I am still trying to catch
up on email after being out on vacation.

Let me verify that we tested this patch and get back to you (later
tonight) Dave.

-- 
Cheers,
Jeff

Re: [PATCH] e1000e: Simplify MSI interrupt testing

[Jeff Kirsher]

On Mon, Sep 13, 2010 at 01:45, Jean Delvare <jdelvare@suse.de> wrote:
> The code is quite convoluted, simplify it. This also avoids calling
> e1000_request_irq() without testing the value it returned, which was
> bad.
>
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Cc: Bruce Allan <bruce.w.allan@intel.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

FYI - Emil tested this while I was out, and all was good.  Sorry for
the delayed response Jean.

Re: [PATCH] e1000e: Simplify MSI interrupt testing

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 13 Sep 2010 23:34:54 -0700

> On Mon, Sep 13, 2010 at 01:45, Jean Delvare <jdelvare@suse.de> wrote:
>> The code is quite convoluted, simplify it. This also avoids calling
>> e1000_request_irq() without testing the value it returned, which was
>> bad.
>>
>> Signed-off-by: Jean Delvare <jdelvare@suse.de>
>> Cc: Bruce Allan <bruce.w.allan@intel.com>
> 
> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> FYI - Emil tested this while I was out, and all was good.  Sorry for
> the delayed response Jean.

Applied, thanks a lot.