* [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 @ 2008-08-17 6:25 Yinghai Lu 2008-08-17 13:02 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2008-08-17 6:25 UTC (permalink / raw) To: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton Cc: linux-kernel, netdev, Yinghai Lu after | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> | Date: Sun May 18 15:02:37 2008 +0200 | | [netdrvr] forcedeth: setup wake-on-lan before shutting down | | When hibernating in 'shutdown' mode, after saving the image the suspend hook | is not called again. | However, if the device is in promiscous mode, wake-on-lan will not work. | This adds a shutdown hook to setup wake-on-lan before the final shutdown. | | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. check with e1000 is using pci_choose_state in _shutdown. So change that pci_choose_state(pdev, ...), and it works. Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com> --- drivers/net/forcedeth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6/drivers/net/forcedeth.c =================================================================== --- linux-2.6.orig/drivers/net/forcedeth.c +++ linux-2.6/drivers/net/forcedeth.c @@ -5988,7 +5988,7 @@ static void nv_shutdown(struct pci_dev * pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); pci_disable_device(pdev); - pci_set_power_state(pdev, PCI_D3hot); + pci_set_power_state(pdev, pci_choose_state(pdev, PMSG_SUSPEND)); } #else #define nv_suspend NULL ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-17 6:25 [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 Yinghai Lu @ 2008-08-17 13:02 ` Rafael J. Wysocki 2008-08-17 16:55 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-17 13:02 UTC (permalink / raw) To: Yinghai Lu Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev On Sunday, 17 of August 2008, Yinghai Lu wrote: > after > > | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> > | Date: Sun May 18 15:02:37 2008 +0200 > | > | [netdrvr] forcedeth: setup wake-on-lan before shutting down > | > | When hibernating in 'shutdown' mode, after saving the image the suspend hook > | is not called again. > | However, if the device is in promiscous mode, wake-on-lan will not work. > | This adds a shutdown hook to setup wake-on-lan before the final shutdown. > | > | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> > | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> > > my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec > > after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. > > check with e1000 is using pci_choose_state in _shutdown. > > So change that pci_choose_state(pdev, ...), and it works. Well, this doesn't look like a good solution to me, because you're putting PMSG_SUSPEND in there, which is not correct for shutdown. The right thing to do would be to avoid changing the device power state if nv_shutdown() is used for kexec or to rework the initialization of the adapter to handle the case when it's initially in D3. Does it help if you just remove the pci_set_power_state(pdev, PCI_D3hot) altogether? > Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com> > > --- > drivers/net/forcedeth.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-2.6/drivers/net/forcedeth.c > =================================================================== > --- linux-2.6.orig/drivers/net/forcedeth.c > +++ linux-2.6/drivers/net/forcedeth.c > @@ -5988,7 +5988,7 @@ static void nv_shutdown(struct pci_dev * > pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > pci_disable_device(pdev); > - pci_set_power_state(pdev, PCI_D3hot); > + pci_set_power_state(pdev, pci_choose_state(pdev, PMSG_SUSPEND)); > } > #else > #define nv_suspend NULL Thanks, Rafael ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-17 13:02 ` Rafael J. Wysocki @ 2008-08-17 16:55 ` Rafael J. Wysocki 2008-08-17 19:16 ` Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-17 16:55 UTC (permalink / raw) To: Yinghai Lu Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > On Sunday, 17 of August 2008, Yinghai Lu wrote: > > after > > > > | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > > | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> > > | Date: Sun May 18 15:02:37 2008 +0200 > > | > > | [netdrvr] forcedeth: setup wake-on-lan before shutting down > > | > > | When hibernating in 'shutdown' mode, after saving the image the suspend hook > > | is not called again. > > | However, if the device is in promiscous mode, wake-on-lan will not work. > > | This adds a shutdown hook to setup wake-on-lan before the final shutdown. > > | > > | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> > > | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> > > > > my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec > > > > after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. > > > > check with e1000 is using pci_choose_state in _shutdown. This is wrong. > > So change that pci_choose_state(pdev, ...), and it works. > > Well, this doesn't look like a good solution to me, because you're putting > PMSG_SUSPEND in there, which is not correct for shutdown. The right thing to > do would be to avoid changing the device power state if nv_shutdown() is > used for kexec or to rework the initialization of the adapter to handle the > case when it's initially in D3. > > Does it help if you just remove the pci_set_power_state(pdev, PCI_D3hot) > altogether? Ah, sorry. I see it does. Actually, I think you can use pci_prepare_to_sleep() instead of pci_enable_wake() / pci_set_power_state() combo. It wasn't designed for this purpose, but should work nevertheless. Can you please check if the appended patch works instead of your one? Rafael --- Fix the problem that boxes with NVidia MCP55 don't work with MSI in a kexeced kernel. Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/net/forcedeth.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Index: linux-2.6/drivers/net/forcedeth.c =================================================================== --- linux-2.6.orig/drivers/net/forcedeth.c +++ linux-2.6/drivers/net/forcedeth.c @@ -5975,10 +5975,8 @@ static void nv_shutdown(struct pci_dev * if (netif_running(dev)) nv_close(dev); - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); pci_disable_device(pdev); - pci_set_power_state(pdev, PCI_D3hot); + pci_prepare_to_sleep(pdev); } #else #define nv_suspend NULL ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-17 16:55 ` Rafael J. Wysocki @ 2008-08-17 19:16 ` Yinghai Lu 2008-08-17 19:29 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2008-08-17 19:16 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev On Sun, Aug 17, 2008 at 9:55 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: >> On Sunday, 17 of August 2008, Yinghai Lu wrote: >> > after >> > >> > | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 >> > | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> >> > | Date: Sun May 18 15:02:37 2008 +0200 >> > | >> > | [netdrvr] forcedeth: setup wake-on-lan before shutting down >> > | >> > | When hibernating in 'shutdown' mode, after saving the image the suspend hook >> > | is not called again. >> > | However, if the device is in promiscous mode, wake-on-lan will not work. >> > | This adds a shutdown hook to setup wake-on-lan before the final shutdown. >> > | >> > | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> >> > | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> >> > >> > my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec >> > >> > after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. >> > >> > check with e1000 is using pci_choose_state in _shutdown. > > This is wrong. > >> > So change that pci_choose_state(pdev, ...), and it works. >> >> Well, this doesn't look like a good solution to me, because you're putting >> PMSG_SUSPEND in there, which is not correct for shutdown. The right thing to >> do would be to avoid changing the device power state if nv_shutdown() is >> used for kexec or to rework the initialization of the adapter to handle the >> case when it's initially in D3. >> >> Does it help if you just remove the pci_set_power_state(pdev, PCI_D3hot) >> altogether? > > Ah, sorry. I see it does. > > Actually, I think you can use pci_prepare_to_sleep() instead of > pci_enable_wake() / pci_set_power_state() combo. It wasn't designed for this > purpose, but should work nevertheless. > > Can you please check if the appended patch works instead of your one? > > Rafael > > --- > Fix the problem that boxes with NVidia MCP55 don't work with MSI > in a kexeced kernel. > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > --- > drivers/net/forcedeth.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > Index: linux-2.6/drivers/net/forcedeth.c > =================================================================== > --- linux-2.6.orig/drivers/net/forcedeth.c > +++ linux-2.6/drivers/net/forcedeth.c > @@ -5975,10 +5975,8 @@ static void nv_shutdown(struct pci_dev * > if (netif_running(dev)) > nv_close(dev); > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > pci_disable_device(pdev); > - pci_set_power_state(pdev, PCI_D3hot); > + pci_prepare_to_sleep(pdev); > } > #else > #define nv_suspend NULL > > your patch doesn't work... it seems that silicon has problem with D3Hot calling init_nic+0x0/0x1b forcedeth: Reverse Engineered nForce ethernet driver. Version 0.61. .. forcedeth 0000:00:09.0: BAR 0: got res [0xdefb9000-0xdefb9fff] bus [0xdefb9000-0xdefb9fff] flags 0x20200 forcedeth 0000:00:09.0: BAR 0: error updating (0xdefb9000 != 0x000000) forcedeth 0000:00:09.0: BAR 0: moved to bus [0xdefb9000-0xdefb9fff] flags 0x20200 forcedeth 0000:00:09.0: BAR 1: got res [0x8080-0x8087] bus [0x8080-0x8087] flags 0x20101 forcedeth 0000:00:09.0: BAR 1: moved to bus [0x8080-0x8087] flags 0x20101 forcedeth 0000:00:09.0: BAR 2: got res [0xdefbe000-0xdefbe0ff] bus [0xdefbe000-0xdefbe0ff] flags 0x20200 forcedeth 0000:00:09.0: BAR 2: moved to bus [0xdefbe000-0xdefbe0ff] flags 0x20200 forcedeth 0000:00:09.0: BAR 3: got res [0xdefb8c00-0xdefb8c0f] bus [0xdefb8c00-0xdefb8c0f] flags 0x20200 forcedeth 0000:00:09.0: BAR 3: moved to bus [0xdefb8c00-0xdefb8c0f] flags 0x20200 forcedeth 0000:00:09.0: enabling device (0000 -> 0003) forcedeth 0000:00:09.0: PCI INT A -> Link[LMAD] -> GSI 20 (level, low) -> IRQ 20 forcedeth 0000:00:09.0: enabling bus mastering forcedeth 0000:00:09.0: setting latency timer to 64 forcedeth 0000:00:09.0: Invalid Mac address detected: ff:ff:ff:ff:ff:ff forcedeth 0000:00:09.0: Please complain to your hardware vendor. Switching to a random MAC. forcedeth 0000:00:09.0: open: Could not find a valid PHY. forcedeth 0000:00:09.0: PCI INT A disabled forcedeth: probe of 0000:00:09.0 failed with error -12 forcedeth 0000:80:08.0: BAR 0: got res [0xddefb000-0xddefbfff] bus [0xddefb000-0xddefbfff] flags 0x20200 forcedeth 0000:80:08.0: BAR 0: moved to bus [0xddefb000-0xddefbfff] flags 0x20200 forcedeth 0000:80:08.0: BAR 1: got res [0x4080-0x4087] bus [0x4080-0x4087] flags 0x20101 forcedeth 0000:80:08.0: BAR 1: moved to bus [0x4080-0x4087] flags 0x20101 forcedeth 0000:80:08.0: BAR 2: got res [0xddefac00-0xddefacff] bus [0xddefac00-0xddefacff] flags 0x20200 forcedeth 0000:80:08.0: BAR 2: moved to bus [0xddefac00-0xddefacff] flags 0x20200 forcedeth 0000:80:08.0: BAR 3: got res [0xddefa800-0xddefa80f] bus [0xddefa800-0xddefa80f] flags 0x20200 forcedeth 0000:80:08.0: BAR 3: moved to bus [0xddefa800-0xddefa80f] flags 0x20200 forcedeth 0000:80:08.0: enabling device (0000 -> 0003) forcedeth 0000:80:08.0: PCI INT A -> Link[IIM0] -> GSI 47 (level, low) -> IRQ 47 forcedeth 0000:80:08.0: enabling bus mastering forcedeth 0000:80:08.0: setting latency timer to 64 nv_probe: set workarund bit for reversed mac addr forcedeth 0000:80:08.0: ifname eth1, PHY OUI 0x5043 @ 2, addr 00:14:4f:8d:5d:b0 forcedeth 0000:80:08.0: highdma csum vlan pwrctl mgmt timirq gbit lnktim msi desc-v3 ... initcall init_nic+0x0/0x1b returned 0 after 5351 msecs YH ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-17 19:16 ` Yinghai Lu @ 2008-08-17 19:29 ` Rafael J. Wysocki 2008-08-17 19:34 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-17 19:29 UTC (permalink / raw) To: Yinghai Lu Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev On Sunday, 17 of August 2008, Yinghai Lu wrote: > On Sun, Aug 17, 2008 at 9:55 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > >> On Sunday, 17 of August 2008, Yinghai Lu wrote: > >> > after > >> > > >> > | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > >> > | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> > >> > | Date: Sun May 18 15:02:37 2008 +0200 > >> > | > >> > | [netdrvr] forcedeth: setup wake-on-lan before shutting down > >> > | > >> > | When hibernating in 'shutdown' mode, after saving the image the suspend hook > >> > | is not called again. > >> > | However, if the device is in promiscous mode, wake-on-lan will not work. > >> > | This adds a shutdown hook to setup wake-on-lan before the final shutdown. > >> > | > >> > | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> > >> > | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> > >> > > >> > my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec > >> > > >> > after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. > >> > > >> > check with e1000 is using pci_choose_state in _shutdown. > > > > This is wrong. > > > >> > So change that pci_choose_state(pdev, ...), and it works. > >> > >> Well, this doesn't look like a good solution to me, because you're putting > >> PMSG_SUSPEND in there, which is not correct for shutdown. The right thing to > >> do would be to avoid changing the device power state if nv_shutdown() is > >> used for kexec or to rework the initialization of the adapter to handle the > >> case when it's initially in D3. > >> > >> Does it help if you just remove the pci_set_power_state(pdev, PCI_D3hot) > >> altogether? > > > > Ah, sorry. I see it does. > > > > Actually, I think you can use pci_prepare_to_sleep() instead of > > pci_enable_wake() / pci_set_power_state() combo. It wasn't designed for this > > purpose, but should work nevertheless. > > > > Can you please check if the appended patch works instead of your one? > > > > Rafael > > > > --- > > Fix the problem that boxes with NVidia MCP55 don't work with MSI > > in a kexeced kernel. > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > --- > > drivers/net/forcedeth.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > Index: linux-2.6/drivers/net/forcedeth.c > > =================================================================== > > --- linux-2.6.orig/drivers/net/forcedeth.c > > +++ linux-2.6/drivers/net/forcedeth.c > > @@ -5975,10 +5975,8 @@ static void nv_shutdown(struct pci_dev * > > if (netif_running(dev)) > > nv_close(dev); > > > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > > pci_disable_device(pdev); > > - pci_set_power_state(pdev, PCI_D3hot); > > + pci_prepare_to_sleep(pdev); > > } > > #else > > #define nv_suspend NULL > > > > > > your patch doesn't work... it seems that silicon has problem with D3Hot Okay, so perhaps it's better to do something like this: --- drivers/net/forcedeth.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) Index: linux-2.6/drivers/net/forcedeth.c =================================================================== --- linux-2.6.orig/drivers/net/forcedeth.c +++ linux-2.6/drivers/net/forcedeth.c @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * if (netif_running(dev)) nv_close(dev); - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); pci_disable_device(pdev); - pci_set_power_state(pdev, PCI_D3hot); + if (system_state == SYS_POWER_OFF) { + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); + pci_set_power_state(pdev, PCI_D3hot); + } } #else #define nv_suspend NULL ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-17 19:29 ` Rafael J. Wysocki @ 2008-08-17 19:34 ` Rafael J. Wysocki 2008-08-17 20:58 ` Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-17 19:34 UTC (permalink / raw) To: Yinghai Lu Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > On Sunday, 17 of August 2008, Yinghai Lu wrote: > > On Sun, Aug 17, 2008 at 9:55 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > > >> On Sunday, 17 of August 2008, Yinghai Lu wrote: > > >> > after > > >> > > > >> > | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > > >> > | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> > > >> > | Date: Sun May 18 15:02:37 2008 +0200 > > >> > | > > >> > | [netdrvr] forcedeth: setup wake-on-lan before shutting down > > >> > | > > >> > | When hibernating in 'shutdown' mode, after saving the image the suspend hook > > >> > | is not called again. > > >> > | However, if the device is in promiscous mode, wake-on-lan will not work. > > >> > | This adds a shutdown hook to setup wake-on-lan before the final shutdown. > > >> > | > > >> > | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> > > >> > | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> > > >> > > > >> > my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec > > >> > > > >> > after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. > > >> > > > >> > check with e1000 is using pci_choose_state in _shutdown. > > > > > > This is wrong. > > > > > >> > So change that pci_choose_state(pdev, ...), and it works. > > >> > > >> Well, this doesn't look like a good solution to me, because you're putting > > >> PMSG_SUSPEND in there, which is not correct for shutdown. The right thing to > > >> do would be to avoid changing the device power state if nv_shutdown() is > > >> used for kexec or to rework the initialization of the adapter to handle the > > >> case when it's initially in D3. > > >> > > >> Does it help if you just remove the pci_set_power_state(pdev, PCI_D3hot) > > >> altogether? > > > > > > Ah, sorry. I see it does. > > > > > > Actually, I think you can use pci_prepare_to_sleep() instead of > > > pci_enable_wake() / pci_set_power_state() combo. It wasn't designed for this > > > purpose, but should work nevertheless. > > > > > > Can you please check if the appended patch works instead of your one? > > > > > > Rafael > > > > > > --- > > > Fix the problem that boxes with NVidia MCP55 don't work with MSI > > > in a kexeced kernel. > > > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > > --- > > > drivers/net/forcedeth.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > Index: linux-2.6/drivers/net/forcedeth.c > > > =================================================================== > > > --- linux-2.6.orig/drivers/net/forcedeth.c > > > +++ linux-2.6/drivers/net/forcedeth.c > > > @@ -5975,10 +5975,8 @@ static void nv_shutdown(struct pci_dev * > > > if (netif_running(dev)) > > > nv_close(dev); > > > > > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > > > pci_disable_device(pdev); > > > - pci_set_power_state(pdev, PCI_D3hot); > > > + pci_prepare_to_sleep(pdev); > > > } > > > #else > > > #define nv_suspend NULL > > > > > > > > > > your patch doesn't work... it seems that silicon has problem with D3Hot > > Okay, so perhaps it's better to do something like this: > > --- > drivers/net/forcedeth.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > Index: linux-2.6/drivers/net/forcedeth.c > =================================================================== > --- linux-2.6.orig/drivers/net/forcedeth.c > +++ linux-2.6/drivers/net/forcedeth.c > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > if (netif_running(dev)) > nv_close(dev); > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > pci_disable_device(pdev); > - pci_set_power_state(pdev, PCI_D3hot); > + if (system_state == SYS_POWER_OFF) { Sorry, it should be SYSTEM_POWER_OFF here. Corrected patch is appended. > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > + pci_set_power_state(pdev, PCI_D3hot); > + } > } > #else > #define nv_suspend NULL > -- --- drivers/net/forcedeth.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) Index: linux-2.6/drivers/net/forcedeth.c =================================================================== --- linux-2.6.orig/drivers/net/forcedeth.c +++ linux-2.6/drivers/net/forcedeth.c @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * if (netif_running(dev)) nv_close(dev); - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); pci_disable_device(pdev); - pci_set_power_state(pdev, PCI_D3hot); + if (system_state == SYSTEM_POWER_OFF) { + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); + pci_set_power_state(pdev, PCI_D3hot); + } } #else #define nv_suspend NULL ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-17 19:34 ` Rafael J. Wysocki @ 2008-08-17 20:58 ` Yinghai Lu 2008-08-17 21:47 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2008-08-17 20:58 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev On Sun, Aug 17, 2008 at 12:34 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: >> On Sunday, 17 of August 2008, Yinghai Lu wrote: >> > On Sun, Aug 17, 2008 at 9:55 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> > > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: >> > >> On Sunday, 17 of August 2008, Yinghai Lu wrote: >> > >> > after >> > >> > >> > >> > | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 >> > >> > | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> >> > >> > | Date: Sun May 18 15:02:37 2008 +0200 >> > >> > | >> > >> > | [netdrvr] forcedeth: setup wake-on-lan before shutting down >> > >> > | >> > >> > | When hibernating in 'shutdown' mode, after saving the image the suspend hook >> > >> > | is not called again. >> > >> > | However, if the device is in promiscous mode, wake-on-lan will not work. >> > >> > | This adds a shutdown hook to setup wake-on-lan before the final shutdown. >> > >> > | >> > >> > | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> >> > >> > | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> >> > >> > >> > >> > my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec >> > >> > >> > >> > after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. >> > >> > >> > >> > check with e1000 is using pci_choose_state in _shutdown. >> > > >> > > This is wrong. >> > > >> > >> > So change that pci_choose_state(pdev, ...), and it works. >> > >> >> > >> Well, this doesn't look like a good solution to me, because you're putting >> > >> PMSG_SUSPEND in there, which is not correct for shutdown. The right thing to >> > >> do would be to avoid changing the device power state if nv_shutdown() is >> > >> used for kexec or to rework the initialization of the adapter to handle the >> > >> case when it's initially in D3. >> > >> >> > >> Does it help if you just remove the pci_set_power_state(pdev, PCI_D3hot) >> > >> altogether? >> > > >> > > Ah, sorry. I see it does. >> > > >> > > Actually, I think you can use pci_prepare_to_sleep() instead of >> > > pci_enable_wake() / pci_set_power_state() combo. It wasn't designed for this >> > > purpose, but should work nevertheless. >> > > >> > > Can you please check if the appended patch works instead of your one? >> > > >> > > Rafael >> > > >> > > --- >> > > Fix the problem that boxes with NVidia MCP55 don't work with MSI >> > > in a kexeced kernel. >> > > >> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> >> > > --- >> > > drivers/net/forcedeth.c | 4 +--- >> > > 1 file changed, 1 insertion(+), 3 deletions(-) >> > > >> > > Index: linux-2.6/drivers/net/forcedeth.c >> > > =================================================================== >> > > --- linux-2.6.orig/drivers/net/forcedeth.c >> > > +++ linux-2.6/drivers/net/forcedeth.c >> > > @@ -5975,10 +5975,8 @@ static void nv_shutdown(struct pci_dev * >> > > if (netif_running(dev)) >> > > nv_close(dev); >> > > >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); >> > > pci_disable_device(pdev); >> > > - pci_set_power_state(pdev, PCI_D3hot); >> > > + pci_prepare_to_sleep(pdev); >> > > } >> > > #else >> > > #define nv_suspend NULL >> > > >> > > >> > >> > your patch doesn't work... it seems that silicon has problem with D3Hot >> >> Okay, so perhaps it's better to do something like this: >> >> --- >> drivers/net/forcedeth.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> Index: linux-2.6/drivers/net/forcedeth.c >> =================================================================== >> --- linux-2.6.orig/drivers/net/forcedeth.c >> +++ linux-2.6/drivers/net/forcedeth.c >> @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * >> if (netif_running(dev)) >> nv_close(dev); >> >> - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); >> pci_disable_device(pdev); >> - pci_set_power_state(pdev, PCI_D3hot); >> + if (system_state == SYS_POWER_OFF) { > > Sorry, it should be SYSTEM_POWER_OFF here. Corrected patch is appended. > >> + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) >> + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> + pci_set_power_state(pdev, PCI_D3hot); >> + } >> } >> #else >> #define nv_suspend NULL >> -- > > --- > drivers/net/forcedeth.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > Index: linux-2.6/drivers/net/forcedeth.c > =================================================================== > --- linux-2.6.orig/drivers/net/forcedeth.c > +++ linux-2.6/drivers/net/forcedeth.c > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > if (netif_running(dev)) > nv_close(dev); > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > pci_disable_device(pdev); > - pci_set_power_state(pdev, PCI_D3hot); > + if (system_state == SYSTEM_POWER_OFF) { > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > + pci_set_power_state(pdev, PCI_D3hot); > + } > } > #else > #define nv_suspend NULL > why not like e1000 to pci_set_power_state(pdev, pci_choose_state(pdev, PMSG_SUSPEND)); pci_choose_state would check if platform support those state... YH ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-17 20:58 ` Yinghai Lu @ 2008-08-17 21:47 ` Rafael J. Wysocki 2008-08-18 10:22 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-17 21:47 UTC (permalink / raw) To: Yinghai Lu Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes On Sunday, 17 of August 2008, Yinghai Lu wrote: > On Sun, Aug 17, 2008 at 12:34 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > >> On Sunday, 17 of August 2008, Yinghai Lu wrote: > >> > On Sun, Aug 17, 2008 at 9:55 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > >> > > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > >> > >> On Sunday, 17 of August 2008, Yinghai Lu wrote: > >> > >> > after > >> > >> > > >> > >> > | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > >> > >> > | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> > >> > >> > | Date: Sun May 18 15:02:37 2008 +0200 > >> > >> > | > >> > >> > | [netdrvr] forcedeth: setup wake-on-lan before shutting down > >> > >> > | > >> > >> > | When hibernating in 'shutdown' mode, after saving the image the suspend hook > >> > >> > | is not called again. > >> > >> > | However, if the device is in promiscous mode, wake-on-lan will not work. > >> > >> > | This adds a shutdown hook to setup wake-on-lan before the final shutdown. > >> > >> > | > >> > >> > | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> > >> > >> > | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> > >> > >> > > >> > >> > my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec > >> > >> > > >> > >> > after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. > >> > >> > > >> > >> > check with e1000 is using pci_choose_state in _shutdown. > >> > > > >> > > This is wrong. > >> > > > >> > >> > So change that pci_choose_state(pdev, ...), and it works. > >> > >> > >> > >> Well, this doesn't look like a good solution to me, because you're putting > >> > >> PMSG_SUSPEND in there, which is not correct for shutdown. The right thing to > >> > >> do would be to avoid changing the device power state if nv_shutdown() is > >> > >> used for kexec or to rework the initialization of the adapter to handle the > >> > >> case when it's initially in D3. > >> > >> > >> > >> Does it help if you just remove the pci_set_power_state(pdev, PCI_D3hot) > >> > >> altogether? > >> > > > >> > > Ah, sorry. I see it does. > >> > > > >> > > Actually, I think you can use pci_prepare_to_sleep() instead of > >> > > pci_enable_wake() / pci_set_power_state() combo. It wasn't designed for this > >> > > purpose, but should work nevertheless. > >> > > > >> > > Can you please check if the appended patch works instead of your one? > >> > > > >> > > Rafael > >> > > > >> > > --- > >> > > Fix the problem that boxes with NVidia MCP55 don't work with MSI > >> > > in a kexeced kernel. > >> > > > >> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > >> > > --- > >> > > drivers/net/forcedeth.c | 4 +--- > >> > > 1 file changed, 1 insertion(+), 3 deletions(-) > >> > > > >> > > Index: linux-2.6/drivers/net/forcedeth.c > >> > > =================================================================== > >> > > --- linux-2.6.orig/drivers/net/forcedeth.c > >> > > +++ linux-2.6/drivers/net/forcedeth.c > >> > > @@ -5975,10 +5975,8 @@ static void nv_shutdown(struct pci_dev * > >> > > if (netif_running(dev)) > >> > > nv_close(dev); > >> > > > >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > >> > > pci_disable_device(pdev); > >> > > - pci_set_power_state(pdev, PCI_D3hot); > >> > > + pci_prepare_to_sleep(pdev); > >> > > } > >> > > #else > >> > > #define nv_suspend NULL > >> > > > >> > > > >> > > >> > your patch doesn't work... it seems that silicon has problem with D3Hot > >> > >> Okay, so perhaps it's better to do something like this: > >> > >> --- > >> drivers/net/forcedeth.c | 8 +++++--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >> Index: linux-2.6/drivers/net/forcedeth.c > >> =================================================================== > >> --- linux-2.6.orig/drivers/net/forcedeth.c > >> +++ linux-2.6/drivers/net/forcedeth.c > >> @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > >> if (netif_running(dev)) > >> nv_close(dev); > >> > >> - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > >> pci_disable_device(pdev); > >> - pci_set_power_state(pdev, PCI_D3hot); > >> + if (system_state == SYS_POWER_OFF) { > > > > Sorry, it should be SYSTEM_POWER_OFF here. Corrected patch is appended. > > > >> + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > >> + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> + pci_set_power_state(pdev, PCI_D3hot); > >> + } > >> } > >> #else > >> #define nv_suspend NULL > >> -- > > > > --- > > drivers/net/forcedeth.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > Index: linux-2.6/drivers/net/forcedeth.c > > =================================================================== > > --- linux-2.6.orig/drivers/net/forcedeth.c > > +++ linux-2.6/drivers/net/forcedeth.c > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > > if (netif_running(dev)) > > nv_close(dev); > > > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > > pci_disable_device(pdev); > > - pci_set_power_state(pdev, PCI_D3hot); > > + if (system_state == SYSTEM_POWER_OFF) { > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > + pci_set_power_state(pdev, PCI_D3hot); > > + } > > } > > #else > > #define nv_suspend NULL > > > > why not like e1000 to > pci_set_power_state(pdev, pci_choose_state(pdev, PMSG_SUSPEND)); > > pci_choose_state would check if platform support those state... ... but ->shutdown() only involves the platform in the 'system_state == SYSTEM_POWER_OFF' case! In fact, using pci_choose_state() in ->shutdown() is a very convoluted way of checking the 'system_state' value, and not a 100% reliable one. :-) Namely, the fact that pci_choose_state() returns you D0 instead of D3hot for 'system_state != SYSTEM_POWER_OFF' is a pure coincidence (accidentally, there is an ACPI handle for the device, but there need not be one) and you should not rely on that in general. Generally speaking, pci_choose_state() is not to be used outside of the drivers' ->suspend() routines. [Yes, this means e1000 will have to be fixed, as I said before.] Apart from this, pci_choose_state() is broken and will shortly be deprecated. Moreover, in future all direct references to PMSG_SUSPEND from the drivers will be removed. Thanks, Rafael ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-17 21:47 ` Rafael J. Wysocki @ 2008-08-18 10:22 ` Rafael J. Wysocki 2008-08-18 21:50 ` Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-18 10:22 UTC (permalink / raw) To: Yinghai Lu Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > On Sunday, 17 of August 2008, Yinghai Lu wrote: > > On Sun, Aug 17, 2008 at 12:34 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > > >> On Sunday, 17 of August 2008, Yinghai Lu wrote: > > >> > On Sun, Aug 17, 2008 at 9:55 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > >> > > On Sunday, 17 of August 2008, Rafael J. Wysocki wrote: > > >> > >> On Sunday, 17 of August 2008, Yinghai Lu wrote: > > >> > >> > after > > >> > >> > > > >> > >> > | commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > > >> > >> > | Author: Tobias Diedrich <ranma+kernel@tdiedrich.de> > > >> > >> > | Date: Sun May 18 15:02:37 2008 +0200 > > >> > >> > | > > >> > >> > | [netdrvr] forcedeth: setup wake-on-lan before shutting down > > >> > >> > | > > >> > >> > | When hibernating in 'shutdown' mode, after saving the image the suspend hook > > >> > >> > | is not called again. > > >> > >> > | However, if the device is in promiscous mode, wake-on-lan will not work. > > >> > >> > | This adds a shutdown hook to setup wake-on-lan before the final shutdown. > > >> > >> > | > > >> > >> > | Signed-off-by: Tobias Diedrich <ranma+kernel@tdiedrich.de> > > >> > >> > | Signed-off-by: Jeff Garzik <jgarzik@redhat.com> > > >> > >> > > > >> > >> > my servers with nvidia mcp55 nic doesn't work with msi in second kernel by kexec > > >> > >> > > > >> > >> > after remove pci_set_power_state(, PCI_D3hot) that nic/msi will work again. > > >> > >> > > > >> > >> > check with e1000 is using pci_choose_state in _shutdown. > > >> > > > > >> > > This is wrong. > > >> > > > > >> > >> > So change that pci_choose_state(pdev, ...), and it works. > > >> > >> > > >> > >> Well, this doesn't look like a good solution to me, because you're putting > > >> > >> PMSG_SUSPEND in there, which is not correct for shutdown. The right thing to > > >> > >> do would be to avoid changing the device power state if nv_shutdown() is > > >> > >> used for kexec or to rework the initialization of the adapter to handle the > > >> > >> case when it's initially in D3. > > >> > >> > > >> > >> Does it help if you just remove the pci_set_power_state(pdev, PCI_D3hot) > > >> > >> altogether? > > >> > > > > >> > > Ah, sorry. I see it does. > > >> > > > > >> > > Actually, I think you can use pci_prepare_to_sleep() instead of > > >> > > pci_enable_wake() / pci_set_power_state() combo. It wasn't designed for this > > >> > > purpose, but should work nevertheless. > > >> > > > > >> > > Can you please check if the appended patch works instead of your one? > > >> > > > > >> > > Rafael > > >> > > > > >> > > --- > > >> > > Fix the problem that boxes with NVidia MCP55 don't work with MSI > > >> > > in a kexeced kernel. > > >> > > > > >> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > >> > > --- > > >> > > drivers/net/forcedeth.c | 4 +--- > > >> > > 1 file changed, 1 insertion(+), 3 deletions(-) > > >> > > > > >> > > Index: linux-2.6/drivers/net/forcedeth.c > > >> > > =================================================================== > > >> > > --- linux-2.6.orig/drivers/net/forcedeth.c > > >> > > +++ linux-2.6/drivers/net/forcedeth.c > > >> > > @@ -5975,10 +5975,8 @@ static void nv_shutdown(struct pci_dev * > > >> > > if (netif_running(dev)) > > >> > > nv_close(dev); > > >> > > > > >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > > >> > > pci_disable_device(pdev); > > >> > > - pci_set_power_state(pdev, PCI_D3hot); > > >> > > + pci_prepare_to_sleep(pdev); > > >> > > } > > >> > > #else > > >> > > #define nv_suspend NULL > > >> > > > > >> > > > > >> > > > >> > your patch doesn't work... it seems that silicon has problem with D3Hot > > >> > > >> Okay, so perhaps it's better to do something like this: > > >> > > >> --- > > >> drivers/net/forcedeth.c | 8 +++++--- > > >> 1 file changed, 5 insertions(+), 3 deletions(-) > > >> > > >> Index: linux-2.6/drivers/net/forcedeth.c > > >> =================================================================== > > >> --- linux-2.6.orig/drivers/net/forcedeth.c > > >> +++ linux-2.6/drivers/net/forcedeth.c > > >> @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > > >> if (netif_running(dev)) > > >> nv_close(dev); > > >> > > >> - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > >> - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > > >> pci_disable_device(pdev); > > >> - pci_set_power_state(pdev, PCI_D3hot); > > >> + if (system_state == SYS_POWER_OFF) { > > > > > > Sorry, it should be SYSTEM_POWER_OFF here. Corrected patch is appended. > > > > > >> + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > > >> + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > >> + pci_set_power_state(pdev, PCI_D3hot); > > >> + } > > >> } > > >> #else > > >> #define nv_suspend NULL > > >> -- > > > > > > --- > > > drivers/net/forcedeth.c | 8 +++++--- > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > Index: linux-2.6/drivers/net/forcedeth.c > > > =================================================================== > > > --- linux-2.6.orig/drivers/net/forcedeth.c > > > +++ linux-2.6/drivers/net/forcedeth.c > > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > > > if (netif_running(dev)) > > > nv_close(dev); > > > > > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > > > pci_disable_device(pdev); > > > - pci_set_power_state(pdev, PCI_D3hot); > > > + if (system_state == SYSTEM_POWER_OFF) { > > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > > + pci_set_power_state(pdev, PCI_D3hot); > > > + } > > > } > > > #else > > > #define nv_suspend NULL > > > > > > > why not like e1000 to > > pci_set_power_state(pdev, pci_choose_state(pdev, PMSG_SUSPEND)); > > > > pci_choose_state would check if platform support those state... > > ... but ->shutdown() only involves the platform in the > 'system_state == SYSTEM_POWER_OFF' case! > > In fact, using pci_choose_state() in ->shutdown() is a very convoluted way of > checking the 'system_state' value, and not a 100% reliable one. :-) > > Namely, the fact that pci_choose_state() returns you D0 instead of D3hot for > 'system_state != SYSTEM_POWER_OFF' is a pure coincidence (accidentally, there > is an ACPI handle for the device, but there need not be one) and you should > not rely on that in general. Generally speaking, pci_choose_state() is not to > be used outside of the drivers' ->suspend() routines. > > [Yes, this means e1000 will have to be fixed, as I said before.] > > Apart from this, pci_choose_state() is broken and will shortly be deprecated. > Moreover, in future all direct references to PMSG_SUSPEND from the drivers > will be removed. Does the last patch work for you BTW? Rafael ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-18 10:22 ` Rafael J. Wysocki @ 2008-08-18 21:50 ` Yinghai Lu 2008-08-18 22:08 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2008-08-18 21:50 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> > > drivers/net/forcedeth.c | 8 +++++--- >> > > 1 file changed, 5 insertions(+), 3 deletions(-) >> > > >> > > Index: linux-2.6/drivers/net/forcedeth.c >> > > =================================================================== >> > > --- linux-2.6.orig/drivers/net/forcedeth.c >> > > +++ linux-2.6/drivers/net/forcedeth.c >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * >> > > if (netif_running(dev)) >> > > nv_close(dev); >> > > >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); >> > > pci_disable_device(pdev); >> > > - pci_set_power_state(pdev, PCI_D3hot); >> > > + if (system_state == SYSTEM_POWER_OFF) { >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> > > + pci_set_power_state(pdev, PCI_D3hot); >> > > + } >> > > } >> > > #else >> > > #define nv_suspend NULL >> > > >> > > Does the last patch work for you BTW? > it works. YH ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-18 21:50 ` Yinghai Lu @ 2008-08-18 22:08 ` Rafael J. Wysocki 2008-08-18 22:36 ` Yinghai Lu ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-18 22:08 UTC (permalink / raw) To: Yinghai Lu Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes, Simon Arlott On Monday, 18 of August 2008, Yinghai Lu wrote: > On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > >> > > drivers/net/forcedeth.c | 8 +++++--- > >> > > 1 file changed, 5 insertions(+), 3 deletions(-) > >> > > > >> > > Index: linux-2.6/drivers/net/forcedeth.c > >> > > =================================================================== > >> > > --- linux-2.6.orig/drivers/net/forcedeth.c > >> > > +++ linux-2.6/drivers/net/forcedeth.c > >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > >> > > if (netif_running(dev)) > >> > > nv_close(dev); > >> > > > >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > >> > > pci_disable_device(pdev); > >> > > - pci_set_power_state(pdev, PCI_D3hot); > >> > > + if (system_state == SYSTEM_POWER_OFF) { > >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> > > + pci_set_power_state(pdev, PCI_D3hot); > >> > > + } > >> > > } > >> > > #else > >> > > #define nv_suspend NULL > >> > > > >> > > > > Does the last patch work for you BTW? > > > > it works. OK, thanks for testing. I think we can use it as a quick fix for 2.6.27. Do you agree? Still, it would be helpful to verify if this is the same MSI issue reported by Simon. Thanks, Rafael ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-18 22:08 ` Rafael J. Wysocki @ 2008-08-18 22:36 ` Yinghai Lu 2008-08-19 18:45 ` [PATCH] forcedeth: Fix kexec regression Rafael J. Wysocki 2008-08-18 22:37 ` [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 Yinghai Lu 2008-08-18 22:42 ` Simon Arlott 2 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2008-08-18 22:36 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes, Simon Arlott On Mon, Aug 18, 2008 at 3:08 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Monday, 18 of August 2008, Yinghai Lu wrote: >> On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> >> >> > > drivers/net/forcedeth.c | 8 +++++--- >> >> > > 1 file changed, 5 insertions(+), 3 deletions(-) >> >> > > >> >> > > Index: linux-2.6/drivers/net/forcedeth.c >> >> > > =================================================================== >> >> > > --- linux-2.6.orig/drivers/net/forcedeth.c >> >> > > +++ linux-2.6/drivers/net/forcedeth.c >> >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * >> >> > > if (netif_running(dev)) >> >> > > nv_close(dev); >> >> > > >> >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); >> >> > > pci_disable_device(pdev); >> >> > > - pci_set_power_state(pdev, PCI_D3hot); >> >> > > + if (system_state == SYSTEM_POWER_OFF) { >> >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) >> >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> > > + pci_set_power_state(pdev, PCI_D3hot); >> >> > > + } >> >> > > } >> >> > > #else >> >> > > #define nv_suspend NULL >> >> > > >> >> > >> >> > Does the last patch work for you BTW? >> > >> >> it works. > > OK, thanks for testing. > > I think we can use it as a quick fix for 2.6.27. Do you agree? > yes YH ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] forcedeth: Fix kexec regression 2008-08-18 22:36 ` Yinghai Lu @ 2008-08-19 18:45 ` Rafael J. Wysocki 2008-08-19 20:37 ` Andrew Morton 2008-08-20 7:01 ` Eric W. Biederman 0 siblings, 2 replies; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-19 18:45 UTC (permalink / raw) To: Jeff Garzik, David Miller Cc: Yinghai Lu, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes, Simon Arlott From: Rafael J. Wysocki <rjw@sisk.pl> forcedeth: Fix kexec regression Fix regression tracked as http://bugzilla.kernel.org/show_bug.cgi?id=11361 and caused by commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 ("[netdrvr] forcedeth: setup wake-on-lan before shutting down") that makes network adapters integrated into the NVidia MCP55 chipsets fail to work in kexeced kernels. The problem appears to be that if the adapter is put into D3_hot during ->shutdown(), it cannot be brought back into D0 after kexec (ref. http://marc.info/?l=linux-kernel&m=121900062814967&w=4). Therefore, only put forcedeth into D3 during ->shutdown() if the system is to be powered off. Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Tested-by: Yinghai Lu <yhlu.kernel@gmail.com> --- drivers/net/forcedeth.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) Index: linux-2.6/drivers/net/forcedeth.c =================================================================== --- linux-2.6.orig/drivers/net/forcedeth.c +++ linux-2.6/drivers/net/forcedeth.c @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * if (netif_running(dev)) nv_close(dev); - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); pci_disable_device(pdev); - pci_set_power_state(pdev, PCI_D3hot); + if (system_state == SYSTEM_POWER_OFF) { + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); + pci_set_power_state(pdev, PCI_D3hot); + } } #else #define nv_suspend NULL ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] forcedeth: Fix kexec regression 2008-08-19 18:45 ` [PATCH] forcedeth: Fix kexec regression Rafael J. Wysocki @ 2008-08-19 20:37 ` Andrew Morton 2008-08-19 20:49 ` Rafael J. Wysocki 2008-08-20 7:01 ` Eric W. Biederman 1 sibling, 1 reply; 23+ messages in thread From: Andrew Morton @ 2008-08-19 20:37 UTC (permalink / raw) To: Rafael J. Wysocki Cc: jgarzik, davem, yhlu.kernel, mingo, ebiederm, linux-kernel, netdev, jbarnes, simon On Tue, 19 Aug 2008 20:45:53 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > forcedeth: Fix kexec regression > > Fix regression tracked as > http://bugzilla.kernel.org/show_bug.cgi?id=11361 and > caused by commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > ("[netdrvr] forcedeth: setup wake-on-lan before shutting down") > that makes network adapters integrated into the NVidia > MCP55 chipsets fail to work in kexeced kernels. The problem appears > to be that if the adapter is put into D3_hot during ->shutdown(), > it cannot be brought back into D0 after kexec (ref. > http://marc.info/?l=linux-kernel&m=121900062814967&w=4). Therefore, > only put forcedeth into D3 during ->shutdown() if the system is to be > powered off. > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > Tested-by: Yinghai Lu <yhlu.kernel@gmail.com> > --- > drivers/net/forcedeth.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > Index: linux-2.6/drivers/net/forcedeth.c > =================================================================== > --- linux-2.6.orig/drivers/net/forcedeth.c > +++ linux-2.6/drivers/net/forcedeth.c > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > if (netif_running(dev)) > nv_close(dev); > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > pci_disable_device(pdev); > - pci_set_power_state(pdev, PCI_D3hot); > + if (system_state == SYSTEM_POWER_OFF) { > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > + pci_set_power_state(pdev, PCI_D3hot); > + } > } > #else > #define nv_suspend NULL hm, I wonder if this has any relation to forcedeth-add-pci_enable_device-to-nv_resume.patch: From: Simon Arlott <simon@fire.lp0.eu> My NIC stops working after resuming from standby, it's not receiving any interrupts. Commit 25d90810ff49d2a63475776f24c74c6bb49b045f ([netdrvr] forcedeth: reorder suspend/resume code) introduces pci_disable_device to nv_suspend, but there's no corresponding pci_enable_device in nv_resume - so I added one (copied from e1000). This results in interrupts being re-enabled after suspend. However, the NIC (10de:0373) still doesn't work after resume. Cc: Tobias Diedrich <ranma+kernel@tdiedrich.de> Cc: Jeff Garzik <jgarzik@redhat.com> Cc: Ayaz Abdulla <aabdulla@nvidia.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- drivers/net/forcedeth.c | 7 +++++++ 1 file changed, 7 insertions(+) diff -puN drivers/net/forcedeth.c~forcedeth-add-pci_enable_device-to-nv_resume drivers/net/forcedeth.c --- a/drivers/net/forcedeth.c~forcedeth-add-pci_enable_device-to-nv_resume +++ a/drivers/net/forcedeth.c @@ -5960,6 +5960,13 @@ static int nv_resume(struct pci_dev *pde pci_set_power_state(pdev, PCI_D0); pci_restore_state(pdev); + rc = pci_enable_device(pdev); + if (rc) { + printk(KERN_ERR "forcedeth: Cannot enable PCI device from suspend\n"); + return rc; + } + pci_set_master(pdev); + /* ack any pending wake events, disable PME */ pci_enable_wake(pdev, PCI_D0, 0); _ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] forcedeth: Fix kexec regression 2008-08-19 20:37 ` Andrew Morton @ 2008-08-19 20:49 ` Rafael J. Wysocki 0 siblings, 0 replies; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-19 20:49 UTC (permalink / raw) To: Andrew Morton Cc: jgarzik, davem, yhlu.kernel, mingo, ebiederm, linux-kernel, netdev, jbarnes, simon On Tuesday, 19 of August 2008, Andrew Morton wrote: > On Tue, 19 Aug 2008 20:45:53 +0200 > "Rafael J. Wysocki" <rjw@sisk.pl> wrote: > > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > forcedeth: Fix kexec regression > > > > Fix regression tracked as > > http://bugzilla.kernel.org/show_bug.cgi?id=11361 and > > caused by commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > > ("[netdrvr] forcedeth: setup wake-on-lan before shutting down") > > that makes network adapters integrated into the NVidia > > MCP55 chipsets fail to work in kexeced kernels. The problem appears > > to be that if the adapter is put into D3_hot during ->shutdown(), > > it cannot be brought back into D0 after kexec (ref. > > http://marc.info/?l=linux-kernel&m=121900062814967&w=4). Therefore, > > only put forcedeth into D3 during ->shutdown() if the system is to be > > powered off. > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > Tested-by: Yinghai Lu <yhlu.kernel@gmail.com> > > --- > > drivers/net/forcedeth.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > Index: linux-2.6/drivers/net/forcedeth.c > > =================================================================== > > --- linux-2.6.orig/drivers/net/forcedeth.c > > +++ linux-2.6/drivers/net/forcedeth.c > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > > if (netif_running(dev)) > > nv_close(dev); > > > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > > pci_disable_device(pdev); > > - pci_set_power_state(pdev, PCI_D3hot); > > + if (system_state == SYSTEM_POWER_OFF) { > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > > + pci_set_power_state(pdev, PCI_D3hot); > > + } > > } > > #else > > #define nv_suspend NULL > > hm, I wonder if this has any relation to > forcedeth-add-pci_enable_device-to-nv_resume.patch: There may be a connection, but we need a confirmation that kexec works on the affected boxes with MSI disabled. However, the $subject patch makes sense even if that's the case IMO. Anyway, the patch below fixes things for Simon only if MSI are disabled. > From: Simon Arlott <simon@fire.lp0.eu> > > My NIC stops working after resuming from standby, it's not receiving any > interrupts. > > Commit 25d90810ff49d2a63475776f24c74c6bb49b045f ([netdrvr] forcedeth: > reorder suspend/resume code) introduces pci_disable_device to nv_suspend, > but there's no corresponding pci_enable_device in nv_resume - so I added > one (copied from e1000). This results in interrupts being re-enabled > after suspend. > > However, the NIC (10de:0373) still doesn't work after resume. > > Cc: Tobias Diedrich <ranma+kernel@tdiedrich.de> > Cc: Jeff Garzik <jgarzik@redhat.com> > Cc: Ayaz Abdulla <aabdulla@nvidia.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > drivers/net/forcedeth.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff -puN drivers/net/forcedeth.c~forcedeth-add-pci_enable_device-to-nv_resume drivers/net/forcedeth.c > --- a/drivers/net/forcedeth.c~forcedeth-add-pci_enable_device-to-nv_resume > +++ a/drivers/net/forcedeth.c > @@ -5960,6 +5960,13 @@ static int nv_resume(struct pci_dev *pde > > pci_set_power_state(pdev, PCI_D0); > pci_restore_state(pdev); > + rc = pci_enable_device(pdev); > + if (rc) { > + printk(KERN_ERR "forcedeth: Cannot enable PCI device from suspend\n"); > + return rc; > + } > + pci_set_master(pdev); > + > /* ack any pending wake events, disable PME */ > pci_enable_wake(pdev, PCI_D0, 0); > > _ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] forcedeth: Fix kexec regression 2008-08-19 18:45 ` [PATCH] forcedeth: Fix kexec regression Rafael J. Wysocki 2008-08-19 20:37 ` Andrew Morton @ 2008-08-20 7:01 ` Eric W. Biederman 2008-08-20 13:12 ` Rafael J. Wysocki 1 sibling, 1 reply; 23+ messages in thread From: Eric W. Biederman @ 2008-08-20 7:01 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Jeff Garzik, David Miller, Yinghai Lu, Ingo Molnar, Andrew Morton, linux-kernel, netdev, Jesse Barnes, Simon Arlott "Rafael J. Wysocki" <rjw@sisk.pl> writes: > From: Rafael J. Wysocki <rjw@sisk.pl> > > forcedeth: Fix kexec regression > > Fix regression tracked as > http://bugzilla.kernel.org/show_bug.cgi?id=11361 and > caused by commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > ("[netdrvr] forcedeth: setup wake-on-lan before shutting down") > that makes network adapters integrated into the NVidia > MCP55 chipsets fail to work in kexeced kernels. The problem appears > to be that if the adapter is put into D3_hot during ->shutdown(), > it cannot be brought back into D0 after kexec (ref. > http://marc.info/?l=linux-kernel&m=121900062814967&w=4). Therefore, > only put forcedeth into D3 during ->shutdown() if the system is to be > powered off. Any chance we can fix this by teaching the forcedeth driver to bring a card out of PCI_D3hot? The kexec on panic guys are going to need that and it is just generally more robust all of the way around than a special case in shutdown based on system_state. Eric ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] forcedeth: Fix kexec regression 2008-08-20 7:01 ` Eric W. Biederman @ 2008-08-20 13:12 ` Rafael J. Wysocki 0 siblings, 0 replies; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-20 13:12 UTC (permalink / raw) To: Eric W. Biederman Cc: Jeff Garzik, David Miller, Yinghai Lu, Ingo Molnar, Andrew Morton, linux-kernel, netdev, Jesse Barnes, Simon Arlott On Wednesday, 20 of August 2008, Eric W. Biederman wrote: > "Rafael J. Wysocki" <rjw@sisk.pl> writes: > > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > forcedeth: Fix kexec regression > > > > Fix regression tracked as > > http://bugzilla.kernel.org/show_bug.cgi?id=11361 and > > caused by commit f735a2a1a4f2a0f5cd823ce323e82675990469e2 > > ("[netdrvr] forcedeth: setup wake-on-lan before shutting down") > > that makes network adapters integrated into the NVidia > > MCP55 chipsets fail to work in kexeced kernels. The problem appears > > to be that if the adapter is put into D3_hot during ->shutdown(), > > it cannot be brought back into D0 after kexec (ref. > > http://marc.info/?l=linux-kernel&m=121900062814967&w=4). Therefore, > > only put forcedeth into D3 during ->shutdown() if the system is to be > > powered off. > > Any chance we can fix this by teaching the forcedeth driver to > bring a card out of PCI_D3hot? Maybe, but that's not what I'm able to do at the moment. :-) Yinghai Lu thinks that the hardware has a problem with that, though. > The kexec on panic guys are going to need that and it is just generally > more robust all of the way around than a special case in shutdown > based on system_state. Actually, system_state == SYSTEM_POWER_OFF is a special case already, since it is the only case in which ACPI gets involved. Thanks, Rafael ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-18 22:08 ` Rafael J. Wysocki 2008-08-18 22:36 ` Yinghai Lu @ 2008-08-18 22:37 ` Yinghai Lu 2008-08-18 22:42 ` Simon Arlott 2 siblings, 0 replies; 23+ messages in thread From: Yinghai Lu @ 2008-08-18 22:37 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes, Simon Arlott On Mon, Aug 18, 2008 at 3:08 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Monday, 18 of August 2008, Yinghai Lu wrote: >> On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> >> >> > > drivers/net/forcedeth.c | 8 +++++--- >> >> > > 1 file changed, 5 insertions(+), 3 deletions(-) >> >> > > >> >> > > Index: linux-2.6/drivers/net/forcedeth.c >> >> > > =================================================================== >> >> > > --- linux-2.6.orig/drivers/net/forcedeth.c >> >> > > +++ linux-2.6/drivers/net/forcedeth.c >> >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * >> >> > > if (netif_running(dev)) >> >> > > nv_close(dev); >> >> > > >> >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); >> >> > > pci_disable_device(pdev); >> >> > > - pci_set_power_state(pdev, PCI_D3hot); >> >> > > + if (system_state == SYSTEM_POWER_OFF) { >> >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) >> >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> > > + pci_set_power_state(pdev, PCI_D3hot); >> >> > > + } >> >> > > } >> >> > > #else >> >> > > #define nv_suspend NULL >> >> > > >> >> > >> >> > Does the last patch work for you BTW? >> > >> >> it works. > > OK, thanks for testing. > > I think we can use it as a quick fix for 2.6.27. Do you agree? > yes YH ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-18 22:08 ` Rafael J. Wysocki 2008-08-18 22:36 ` Yinghai Lu 2008-08-18 22:37 ` [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 Yinghai Lu @ 2008-08-18 22:42 ` Simon Arlott 2008-08-19 17:58 ` Rafael J. Wysocki 2 siblings, 1 reply; 23+ messages in thread From: Simon Arlott @ 2008-08-18 22:42 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Yinghai Lu, Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes On 18/08/08 23:08, Rafael J. Wysocki wrote: > On Monday, 18 of August 2008, Yinghai Lu wrote: >> On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> >> >> > > drivers/net/forcedeth.c | 8 +++++--- >> >> > > 1 file changed, 5 insertions(+), 3 deletions(-) >> >> > > >> >> > > Index: linux-2.6/drivers/net/forcedeth.c >> >> > > =================================================================== >> >> > > --- linux-2.6.orig/drivers/net/forcedeth.c >> >> > > +++ linux-2.6/drivers/net/forcedeth.c >> >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * >> >> > > if (netif_running(dev)) >> >> > > nv_close(dev); >> >> > > >> >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); >> >> > > pci_disable_device(pdev); >> >> > > - pci_set_power_state(pdev, PCI_D3hot); >> >> > > + if (system_state == SYSTEM_POWER_OFF) { >> >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) >> >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> > > + pci_set_power_state(pdev, PCI_D3hot); >> >> > > + } >> >> > > } >> >> > > #else >> >> > > #define nv_suspend NULL >> >> > > >> >> > >> >> > Does the last patch work for you BTW? >> > >> >> it works. > > OK, thanks for testing. > > I think we can use it as a quick fix for 2.6.27. Do you agree? > > Still, it would be helpful to verify if this is the same MSI issue reported by > Simon. I tried to test that patch but even without it standby/resume has stopped working: http://img89.imageshack.us/img89/1861/1005552wb5.jpg (standby, tainted) http://img89.imageshack.us/img89/5409/1005561ys2.jpg (resume) http://img503.imageshack.us/img503/8720/1005571bc0.jpg (resume) http://img135.imageshack.us/img135/5048/1005572be3.jpg (standby) -- Simon Arlott ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-18 22:42 ` Simon Arlott @ 2008-08-19 17:58 ` Rafael J. Wysocki 2008-08-19 18:33 ` Simon Arlott 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-19 17:58 UTC (permalink / raw) To: Simon Arlott Cc: Yinghai Lu, Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes On Tuesday, 19 of August 2008, Simon Arlott wrote: > On 18/08/08 23:08, Rafael J. Wysocki wrote: > > On Monday, 18 of August 2008, Yinghai Lu wrote: > >> On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > >> > >> >> > > drivers/net/forcedeth.c | 8 +++++--- > >> >> > > 1 file changed, 5 insertions(+), 3 deletions(-) > >> >> > > > >> >> > > Index: linux-2.6/drivers/net/forcedeth.c > >> >> > > =================================================================== > >> >> > > --- linux-2.6.orig/drivers/net/forcedeth.c > >> >> > > +++ linux-2.6/drivers/net/forcedeth.c > >> >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > >> >> > > if (netif_running(dev)) > >> >> > > nv_close(dev); > >> >> > > > >> >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > >> >> > > pci_disable_device(pdev); > >> >> > > - pci_set_power_state(pdev, PCI_D3hot); > >> >> > > + if (system_state == SYSTEM_POWER_OFF) { > >> >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > >> >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> >> > > + pci_set_power_state(pdev, PCI_D3hot); > >> >> > > + } > >> >> > > } > >> >> > > #else > >> >> > > #define nv_suspend NULL > >> >> > > > >> >> > > >> > >> > Does the last patch work for you BTW? > >> > > >> > >> it works. > > > > OK, thanks for testing. > > > > I think we can use it as a quick fix for 2.6.27. Do you agree? > > > > Still, it would be helpful to verify if this is the same MSI issue reported by > > Simon. > > I tried to test that patch but even without it standby/resume has stopped working: Which kernel is this? > http://img89.imageshack.us/img89/1861/1005552wb5.jpg (standby, tainted) > http://img89.imageshack.us/img89/5409/1005561ys2.jpg (resume) > http://img503.imageshack.us/img503/8720/1005571bc0.jpg (resume) > http://img135.imageshack.us/img135/5048/1005572be3.jpg (standby) Rafael ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-19 17:58 ` Rafael J. Wysocki @ 2008-08-19 18:33 ` Simon Arlott 2008-08-19 21:09 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Simon Arlott @ 2008-08-19 18:33 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Yinghai Lu, Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes On 19/08/08 18:58, Rafael J. Wysocki wrote: > On Tuesday, 19 of August 2008, Simon Arlott wrote: >> On 18/08/08 23:08, Rafael J. Wysocki wrote: >> > On Monday, 18 of August 2008, Yinghai Lu wrote: >> >> On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> >> >> >> >> > > drivers/net/forcedeth.c | 8 +++++--- >> >> >> > > 1 file changed, 5 insertions(+), 3 deletions(-) >> >> >> > > >> >> >> > > Index: linux-2.6/drivers/net/forcedeth.c >> >> >> > > =================================================================== >> >> >> > > --- linux-2.6.orig/drivers/net/forcedeth.c >> >> >> > > +++ linux-2.6/drivers/net/forcedeth.c >> >> >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * >> >> >> > > if (netif_running(dev)) >> >> >> > > nv_close(dev); >> >> >> > > >> >> >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); >> >> >> > > pci_disable_device(pdev); >> >> >> > > - pci_set_power_state(pdev, PCI_D3hot); >> >> >> > > + if (system_state == SYSTEM_POWER_OFF) { >> >> >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) >> >> >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> >> > > + pci_set_power_state(pdev, PCI_D3hot); >> >> >> > > + } >> >> >> > > } >> >> >> > > #else >> >> >> > > #define nv_suspend NULL >> >> >> > > >> >> >> > >> >> >> >> > Does the last patch work for you BTW? >> >> > >> >> >> >> it works. >> > >> > OK, thanks for testing. >> > >> > I think we can use it as a quick fix for 2.6.27. Do you agree? >> > >> > Still, it would be helpful to verify if this is the same MSI issue reported by >> > Simon. >> >> I tried to test that patch but even without it standby/resume has stopped working: > > Which kernel is this? linus-2.6 a7f5aaf36ded825477c4d7167cc6eb1bcdc63191 I've reverted to 2b12a4c524812fb3f6ee590a02e65b95c8c32229 where standby/resume still works and applied the above patch plus the pci_enable_device patch. It still doesn't work after resume if MSI is enabled. -- Simon Arlott ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-19 18:33 ` Simon Arlott @ 2008-08-19 21:09 ` Rafael J. Wysocki 2008-08-30 19:39 ` Simon Arlott 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2008-08-19 21:09 UTC (permalink / raw) To: Simon Arlott Cc: Yinghai Lu, Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes On Tuesday, 19 of August 2008, Simon Arlott wrote: > On 19/08/08 18:58, Rafael J. Wysocki wrote: > > On Tuesday, 19 of August 2008, Simon Arlott wrote: > >> On 18/08/08 23:08, Rafael J. Wysocki wrote: > >> > On Monday, 18 of August 2008, Yinghai Lu wrote: > >> >> On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > >> >> > >> >> >> > > drivers/net/forcedeth.c | 8 +++++--- > >> >> >> > > 1 file changed, 5 insertions(+), 3 deletions(-) > >> >> >> > > > >> >> >> > > Index: linux-2.6/drivers/net/forcedeth.c > >> >> >> > > =================================================================== > >> >> >> > > --- linux-2.6.orig/drivers/net/forcedeth.c > >> >> >> > > +++ linux-2.6/drivers/net/forcedeth.c > >> >> >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * > >> >> >> > > if (netif_running(dev)) > >> >> >> > > nv_close(dev); > >> >> >> > > > >> >> >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> >> >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); > >> >> >> > > pci_disable_device(pdev); > >> >> >> > > - pci_set_power_state(pdev, PCI_D3hot); > >> >> >> > > + if (system_state == SYSTEM_POWER_OFF) { > >> >> >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) > >> >> >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); > >> >> >> > > + pci_set_power_state(pdev, PCI_D3hot); > >> >> >> > > + } > >> >> >> > > } > >> >> >> > > #else > >> >> >> > > #define nv_suspend NULL > >> >> >> > > > >> >> >> > > >> >> > >> >> > Does the last patch work for you BTW? > >> >> > > >> >> > >> >> it works. > >> > > >> > OK, thanks for testing. > >> > > >> > I think we can use it as a quick fix for 2.6.27. Do you agree? > >> > > >> > Still, it would be helpful to verify if this is the same MSI issue reported by > >> > Simon. > >> > >> I tried to test that patch but even without it standby/resume has stopped working: > > > > Which kernel is this? > > linus-2.6 a7f5aaf36ded825477c4d7167cc6eb1bcdc63191 > > I've reverted to 2b12a4c524812fb3f6ee590a02e65b95c8c32229 where standby/resume still > works and applied the above patch plus the pci_enable_device patch. It still doesn't > work after resume if MSI is enabled. My patch only affects the shutdown code path and is not related to suspend/standby etc. Could you identify the commit that broke standby/resume for you between 2b12a4c524812fb3f6ee590a02e65b95c8c32229 and a7f5aaf36ded825477c4d7167cc6eb1bcdc63191 ? It looks like that should be bisectable. Thanks, Rafael ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 2008-08-19 21:09 ` Rafael J. Wysocki @ 2008-08-30 19:39 ` Simon Arlott 0 siblings, 0 replies; 23+ messages in thread From: Simon Arlott @ 2008-08-30 19:39 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Yinghai Lu, Jeff Garzik, Ingo Molnar, Eric W. Biederman, Andrew Morton, linux-kernel, netdev, Jesse Barnes On 19/08/08 22:09, Rafael J. Wysocki wrote: > On Tuesday, 19 of August 2008, Simon Arlott wrote: >> On 19/08/08 18:58, Rafael J. Wysocki wrote: >> > On Tuesday, 19 of August 2008, Simon Arlott wrote: >> >> On 18/08/08 23:08, Rafael J. Wysocki wrote: >> >> > On Monday, 18 of August 2008, Yinghai Lu wrote: >> >> >> On Mon, Aug 18, 2008 at 3:22 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> >> >> >> >> >> >> > > drivers/net/forcedeth.c | 8 +++++--- >> >> >> >> > > 1 file changed, 5 insertions(+), 3 deletions(-) >> >> >> >> > > >> >> >> >> > > Index: linux-2.6/drivers/net/forcedeth.c >> >> >> >> > > =================================================================== >> >> >> >> > > --- linux-2.6.orig/drivers/net/forcedeth.c >> >> >> >> > > +++ linux-2.6/drivers/net/forcedeth.c >> >> >> >> > > @@ -5975,10 +5975,12 @@ static void nv_shutdown(struct pci_dev * >> >> >> >> > > if (netif_running(dev)) >> >> >> >> > > nv_close(dev); >> >> >> >> > > >> >> >> >> > > - pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> >> >> > > - pci_enable_wake(pdev, PCI_D3cold, np->wolenabled); >> >> >> >> > > pci_disable_device(pdev); >> >> >> >> > > - pci_set_power_state(pdev, PCI_D3hot); >> >> >> >> > > + if (system_state == SYSTEM_POWER_OFF) { >> >> >> >> > > + if (pci_enable_wake(pdev, PCI_D3cold, np->wolenabled)) >> >> >> >> > > + pci_enable_wake(pdev, PCI_D3hot, np->wolenabled); >> >> >> >> > > + pci_set_power_state(pdev, PCI_D3hot); >> >> >> >> > > + } >> >> >> >> > > } >> >> >> >> > > #else >> >> >> >> > > #define nv_suspend NULL >> >> >> >> > > >> >> >> >> > >> >> >> >> >> >> > Does the last patch work for you BTW? >> >> >> > >> >> >> >> >> >> it works. >> >> > >> >> > OK, thanks for testing. >> >> > >> >> > I think we can use it as a quick fix for 2.6.27. Do you agree? >> >> > >> >> > Still, it would be helpful to verify if this is the same MSI issue reported by >> >> > Simon. >> >> >> >> I tried to test that patch but even without it standby/resume has stopped working: >> > >> > Which kernel is this? >> >> linus-2.6 a7f5aaf36ded825477c4d7167cc6eb1bcdc63191 >> >> I've reverted to 2b12a4c524812fb3f6ee590a02e65b95c8c32229 where standby/resume still >> works and applied the above patch plus the pci_enable_device patch. It still doesn't >> work after resume if MSI is enabled. > > My patch only affects the shutdown code path and is not related to > suspend/standby etc. > > Could you identify the commit that broke standby/resume for you between > 2b12a4c524812fb3f6ee590a02e65b95c8c32229 and > a7f5aaf36ded825477c4d7167cc6eb1bcdc63191 ? It looks like that should be > bisectable. It appears to be working again as of ee096f75b69913dbad0e6f7f2572513de5c90002. -- Simon Arlott ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2008-08-30 19:40 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-08-17 6:25 [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 Yinghai Lu 2008-08-17 13:02 ` Rafael J. Wysocki 2008-08-17 16:55 ` Rafael J. Wysocki 2008-08-17 19:16 ` Yinghai Lu 2008-08-17 19:29 ` Rafael J. Wysocki 2008-08-17 19:34 ` Rafael J. Wysocki 2008-08-17 20:58 ` Yinghai Lu 2008-08-17 21:47 ` Rafael J. Wysocki 2008-08-18 10:22 ` Rafael J. Wysocki 2008-08-18 21:50 ` Yinghai Lu 2008-08-18 22:08 ` Rafael J. Wysocki 2008-08-18 22:36 ` Yinghai Lu 2008-08-19 18:45 ` [PATCH] forcedeth: Fix kexec regression Rafael J. Wysocki 2008-08-19 20:37 ` Andrew Morton 2008-08-19 20:49 ` Rafael J. Wysocki 2008-08-20 7:01 ` Eric W. Biederman 2008-08-20 13:12 ` Rafael J. Wysocki 2008-08-18 22:37 ` [PATCH] net: forcedeth use pci_choose_state instead of PCI_D3hot - v2 Yinghai Lu 2008-08-18 22:42 ` Simon Arlott 2008-08-19 17:58 ` Rafael J. Wysocki 2008-08-19 18:33 ` Simon Arlott 2008-08-19 21:09 ` Rafael J. Wysocki 2008-08-30 19:39 ` Simon Arlott
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).
