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 --
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) Thanks, Rafael --
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: ...
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
--
---
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 --
... 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 --
Does the last patch work for you BTW? Rafael --
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 --
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 --
On Tue, 19 Aug 2008 20:45:53 +0200
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);
_
--
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. --
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 --
Maybe, but that's not what I'm able to do at the moment. :-) Actually, system_state == SYSTEM_POWER_OFF is a special case already, since it is the only case in which ACPI gets involved. Thanks, Rafael --
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 --
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 --
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 --
It appears to be working again as of ee096f75b69913dbad0e6f7f2572513de5c90002. -- Simon Arlott --
