2006-11-03 19:43:48

by Kok, Auke

[permalink] [raw]
Subject: [PATCH] e1000: Fix regression: garbled stats and irq allocation during swsusp


e1000: Fix regression: garbled stats and irq allocation during swsusp

After 7.0.33/2.6.16, e1000 suspend/resume left the user with an enabled
device showing garbled statistics and undetermined irq allocation state,
where `ifconfig eth0 down` would display `trying to free already freed irq`.

Explicitly free and allocate irq as well as powerup the PHY during resume
fixes.

Signed-off-by: Auke Kok <[email protected]>

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index c0269b5..32cdccb 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -5081,6 +5081,8 @@ #endif
if (adapter->hw.phy_type == e1000_phy_igp_3)
e1000_phy_powerdown_workaround(&adapter->hw);

+ e1000_free_irq(adapter);
+
/* Release control of h/w to f/w. If f/w is AMT enabled, this
* would have already happened in close and is redundant. */
e1000_release_hw_control(adapter);
@@ -5111,6 +5113,10 @@ e1000_resume(struct pci_dev *pdev)
pci_enable_wake(pdev, PCI_D3hot, 0);
pci_enable_wake(pdev, PCI_D3cold, 0);

+ if ((err = e1000_request_irq(adapter)))
+ return err;
+
+ e1000_power_up_phy(adapter);
e1000_reset(adapter);
E1000_WRITE_REG(&adapter->hw, WUS, ~0);


2006-11-04 15:20:48

by Toralf Förster

[permalink] [raw]
Subject: Re: [PATCH] e1000: Fix regression: garbled stats and irq allocation during swsusp

Hi Auke,

good news. I incorporated the changes manually b/cI couldn't apply your patch
against the latest git tree from Linux with the program patch.
The patch would solve http://bugzilla.kernel.org/show_bug.cgi?id=7207 :-)
Here's the diff after my edit of the file e1000_main.c:

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 8d04752..400bdee 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -4799,7 +4799,8 @@ e1000_suspend(struct pci_dev *pdev, pm_m

if (adapter->hw.phy_type == e1000_phy_igp_3)
e1000_phy_powerdown_workaround(&adapter->hw);
-
+ e1000_free_irq(adapter);
+
/* Release control of h/w to f/w. If f/w is AMT enabled, this
* would have already happened in close and is redundant. */
e1000_release_hw_control(adapter);
@@ -4830,6 +4831,10 @@ e1000_resume(struct pci_dev *pdev)
pci_enable_wake(pdev, PCI_D3hot, 0);
pci_enable_wake(pdev, PCI_D3cold, 0);

+ if ((err = e1000_request_irq(adapter)))
+ return err;
+
+ e1000_power_up_phy(adapter);
e1000_reset(adapter);
E1000_WRITE_REG(&adapter->hw, WUS, ~0);



Am Friday 03 November 2006 20:43 schrieb Auke Kok:
>
> e1000: Fix regression: garbled stats and irq allocation during swsusp
>
> After 7.0.33/2.6.16, e1000 suspend/resume left the user with an enabled
> device showing garbled statistics and undetermined irq allocation state,
> where `ifconfig eth0 down` would display `trying to free already freed irq`.
>
> Explicitly free and allocate irq as well as powerup the PHY during resume
> fixes.
>
> Signed-off-by: Auke Kok <[email protected]>
>
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index c0269b5..32cdccb 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -5081,6 +5081,8 @@ #endif
> if (adapter->hw.phy_type == e1000_phy_igp_3)
> e1000_phy_powerdown_workaround(&adapter->hw);
>
> + e1000_free_irq(adapter);
> +
> /* Release control of h/w to f/w. If f/w is AMT enabled, this
> * would have already happened in close and is redundant. */
> e1000_release_hw_control(adapter);
> @@ -5111,6 +5113,10 @@ e1000_resume(struct pci_dev *pdev)
> pci_enable_wake(pdev, PCI_D3hot, 0);
> pci_enable_wake(pdev, PCI_D3cold, 0);
>
> + if ((err = e1000_request_irq(adapter)))
> + return err;
> +
> + e1000_power_up_phy(adapter);
> e1000_reset(adapter);
> E1000_WRITE_REG(&adapter->hw, WUS, ~0);
>
>

--
MfG/Sincerely

Toralf F?rster
+++ I'm not subscribed to the email list, please Cc: me +++


Attachments:
(No filename) (2.60 kB)
(No filename) (189.00 B)
Download all attachments

2006-11-06 07:56:06

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] e1000: Fix regression: garbled stats and irq allocation during swsusp

Auke Kok wrote:
>
> e1000: Fix regression: garbled stats and irq allocation during swsusp
>
> After 7.0.33/2.6.16, e1000 suspend/resume left the user with an enabled
> device showing garbled statistics and undetermined irq allocation state,
> where `ifconfig eth0 down` would display `trying to free already freed
> irq`.
>
> Explicitly free and allocate irq as well as powerup the PHY during resume
> fixes.
>
> Signed-off-by: Auke Kok <[email protected]>

ACK, but:

Applying 'e1000: Fix regression: garbled stats and irq allocation during
swsusp'

fatal: corrupt patch at line 8

2006-11-06 09:39:16

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] e1000: Fix regression: garbled stats and irq allocation during swsusp

Hi!

> >e1000: Fix regression: garbled stats and irq allocation during swsusp
> >
> >After 7.0.33/2.6.16, e1000 suspend/resume left the user with an enabled
> >device showing garbled statistics and undetermined irq allocation state,
> >where `ifconfig eth0 down` would display `trying to free already freed
> >irq`.
> >
> >Explicitly free and allocate irq as well as powerup the PHY during resume
> >fixes.
> >
> >Signed-off-by: Auke Kok <[email protected]>
>
> ACK, but:
>
> Applying 'e1000: Fix regression: garbled stats and irq allocation during
> swsusp'
>
> fatal: corrupt patch at line 8

Toralf posted rediffed (manually applied) version... should I forward
it to you?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-11-06 15:38:47

by Kok, Auke

[permalink] [raw]
Subject: Re: [PATCH] e1000: Fix regression: garbled stats and irq allocation during swsusp

Jeff Garzik wrote:
> Auke Kok wrote:
>>
>> e1000: Fix regression: garbled stats and irq allocation during swsusp
>>
>> After 7.0.33/2.6.16, e1000 suspend/resume left the user with an enabled
>> device showing garbled statistics and undetermined irq allocation state,
>> where `ifconfig eth0 down` would display `trying to free already freed
>> irq`.
>>
>> Explicitly free and allocate irq as well as powerup the PHY during resume
>> fixes.
>>
>> Signed-off-by: Auke Kok <[email protected]>
>
> ACK, but:
>
> Applying 'e1000: Fix regression: garbled stats and irq allocation during
> swsusp'
>
> fatal: corrupt patch at line 8


There's another problem with it, that needs attention as well. I will reformat and
re-post the patch to everyone.

Cheers,

Auke


From: <[email protected]>
Also, would this cause a problem if I ifdown the interface to call
->close then suspend? Looks like it'd free_irq twice, maybe need a
netif_running() check?