| Message ID | 201009131045.40018.jdelvare@suse.de |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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 ;-) -- 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
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.
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. -- 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
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. -- 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
--- 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; }
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(-)