diff mbox

[E1000-devel,BUG,2.6.30+] e100 sometimes causes oops during resume

Message ID 20090916014448.GA1070@bizet.domek.prywatny
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Karol Lewandowski Sept. 16, 2009, 1:44 a.m. UTC 
On Tue, Sep 15, 2009 at 03:54:20PM -0700, Graham, David wrote:

> A v2.6.30..v2.6.31 diff shows that this is probably exposed by
> Rafael Wysocki's commit 6905b1f1, which now allows systems with e100
> to sleep. If I understand correctly, it looks like these systems
> simply couldn't sleep before. Is that right Rafael?.

Probably true, but that wasn't the case for my (I guess
ACPI-controlled) system.


> I don't think its likely that the commit is a direct cause of the
> problem, but that the suspend/resume cycle now allows us to see
> another issue.

From my (very limited) understanding commit message is at least in
conflict with patch body.

Precisely patch was supposed to "Fix this problem by ignoring the
return value of pci_set_power_state() in __e100_power_off()."

That patch is doing something rather different -- it returns 0, yes,
but it also ignores 'wake' bool as set by __e100_shutdown().  That
seems wrong to me.


Correct patch would be that (hand-made), right?


+++ b/drivers/net/e100.c
@@ -2895,12 +2895,13 @@ static void __e100_shutdown(struct pci_dev *pdev, bool *enable_wake)
 
 static int __e100_power_off(struct pci_dev *pdev, bool wake)
 {
        if (wake) {
                return pci_prepare_to_sleep(pdev);
        } else {
                pci_wake_from_d3(pdev, false);
-               return pci_set_power_state(pdev, PCI_D3hot);
+               pci_set_power_state(pdev, PCI_D3hot);
        }
+
+       return 0;
 }

I can test, or rather -- start testing this tommorow, if this makes
sense to you.

Thanks.
--
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

Comments

Karol Lewandowski Sept. 16, 2009, 9:19 a.m. UTC | #1 
On Wed, Sep 16, 2009 at 03:44:48AM +0200, Karol Lewandowski wrote:
> On Tue, Sep 15, 2009 at 03:54:20PM -0700, Graham, David wrote:
> 
> > A v2.6.30..v2.6.31 diff shows that this is probably exposed by
> > Rafael Wysocki's commit 6905b1f1, which now allows systems with e100
> > to sleep. If I understand correctly, it looks like these systems
> > simply couldn't sleep before. Is that right Rafael?.
> 
> Probably true, but that wasn't the case for my (I guess
> ACPI-controlled) system.
> 
> 
> > I don't think its likely that the commit is a direct cause of the
> > problem, but that the suspend/resume cycle now allows us to see
> > another issue.
> 
> From my (very limited) understanding commit message is at least in
> conflict with patch body.

No, it isn't, I must just go to bed earlier than 4AM. :/

Sorry for noise.
--
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
Graham, David Sept. 16, 2009, 9:06 p.m. UTC | #2 
Karol Lewandowski wrote:
> 
> I can test, or rather -- start testing this tommorow, if this makes
> sense to you.
> 
Yes please Karol, test with 6905b1f1...removed. I will continue testing 
here too on 2.6.31, though I have not had a repro yet. And are you 
performing a standby or hibernate, I want to be sure that I'm resuming 
from the same state.
Thanks.
> Thanks.
> 

--
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
Karol Lewandowski Sept. 16, 2009, 9:17 p.m. UTC | #3 
On Wed, Sep 16, 2009 at 02:06:55PM -0700, Graham, David wrote:
> Karol Lewandowski wrote:
>>
>> I can test, or rather -- start testing this tommorow, if this makes
>> sense to you.
>>
> Yes please Karol, test with 6905b1f1...removed.

Ok.


> I will continue testing  
> here too on 2.6.31, though I have not had a repro yet. And are you  
> performing a standby or hibernate, I want to be sure that I'm resuming  
> from the same state.

I don't use hibernate, I always suspend to ram.

Thanks.
--
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
diff mbox

Patch

--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -2895,12 +2895,13 @@  static void __e100_shutdown(struct pci_dev *pdev, bool *enable_wake)
 
 static int __e100_power_off(struct pci_dev *pdev, bool wake)
 {
-       if (wake) {
+       if (wake)
                return pci_prepare_to_sleep(pdev);
-       } else {
-               pci_wake_from_d3(pdev, false);
-               return pci_set_power_state(pdev, PCI_D3hot);
-       }
+
+       pci_wake_from_d3(pdev, false);
+       pci_set_power_state(pdev, PCI_D3hot);
+
+       return 0;
 }