From patchwork Sat Jan 31 01:41:13 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 21305 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 2A7E1DDF6E for ; Sat, 31 Jan 2009 12:41:36 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754340AbZAaBlb (ORCPT ); Fri, 30 Jan 2009 20:41:31 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753385AbZAaBla (ORCPT ); Fri, 30 Jan 2009 20:41:30 -0500 Received: from smtp1.linux-foundation.org ([140.211.169.13]:60162 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752241AbZAaBl3 (ORCPT ); Fri, 30 Jan 2009 20:41:29 -0500 Received: from imap1.linux-foundation.org (imap1.linux-foundation.org [140.211.169.55]) by smtp1.linux-foundation.org (8.14.2/8.13.5/Debian-3ubuntu1.1) with ESMTP id n0V1fEWl013028 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 30 Jan 2009 17:41:15 -0800 Received: from localhost (localhost [127.0.0.1]) by imap1.linux-foundation.org (8.13.5.20060308/8.13.5/Debian-3ubuntu1.1) with ESMTP id n0V1fEPt029026; Fri, 30 Jan 2009 17:41:14 -0800 Date: Fri, 30 Jan 2009 17:41:13 -0800 (PST) From: Linus Torvalds X-X-Sender: torvalds@localhost.localdomain To: "Rafael J. Wysocki" cc: Parag Warudkar , Matt Carlson , "netdev@vger.kernel.org" , Linux Kernel Mailing List , "David S. Miller" , Andrew Morton Subject: Re: 2.6.29-rc3: tg3 dead after resume In-Reply-To: <200901310138.51214.rjw@sisk.pl> Message-ID: References: <200901310059.17746.rjw@sisk.pl> <200901310138.51214.rjw@sisk.pl> User-Agent: Alpine 2.00 (LFD 1167 2008-08-23) MIME-Version: 1.0 X-Spam-Status: No, hits=-3.466 required=5 tests=AWL,BAYES_00 X-Spam-Checker-Version: SpamAssassin 3.2.4-osdl_revision__1.47__ X-MIMEDefang-Filter: lf$Revision: 1.188 $ X-Scanned-By: MIMEDefang 2.63 on 140.211.169.13 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sat, 31 Jan 2009, Rafael J. Wysocki wrote: > > Can you test the patch below, please? Rafael, you're making this _way_ too difficult. Don't make it use the new PM infrastructure, because that one is certainly broken: pci_pm_default_suspend_generic() is total crap. It's saving the disabled state. No WAY is that correct. That "pci_disable_enabled_device()" should be removed, but even then that's wrong, because if the driver suspend disabled it, you're now (again) saving the disabled state. But all of that is only called if you use the new PM infrastructure. So the thing is, when you're trying to move the PCI-E drive to the new pm infrastructure, you're making things _worse_. The legacy PM infrastructure at least does the whole pci_dev->state_saved = false; i = drv->suspend(pci_dev, state); .. if (pci_dev->state_saved) goto Fixup; thing, which will avoid overwriting the state if it was already saved. HOWEVER. The problem here (I think) is that PCI-E actually does the state save late, so it won't ever see the "state_saved" in the early ->suspend. I think a patch like the one below at least simplifies this all, and lets the PCI layer itself do all the core restore stuff. The new PM infrastructure gets this totally wrong, and (a) disables the device before saving state and (b) overwrites the (now corrupted) saved state that the driver perhaps already saved, after the driver may even have put it to sleep. So let's not use the new PM infrastructure - I don't think it's ready yet. Let's start simplifying first. Start off by getting rid of the suspend_early/resume_late, since the PCI layer now does it for us. I don't see why we don't resume with IO/MEM on, though. The legacy suspend sequence shouldn't disable them, afaik. Linus --- drivers/pci/pcie/portdrv_pci.c | 14 -------------- 1 files changed, 0 insertions(+), 14 deletions(-) -- 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 --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c index 99a914a..08a8e3c 100644 --- a/drivers/pci/pcie/portdrv_pci.c +++ b/drivers/pci/pcie/portdrv_pci.c @@ -55,16 +55,6 @@ static int pcie_portdrv_suspend(struct pci_dev *dev, pm_message_t state) } -static int pcie_portdrv_suspend_late(struct pci_dev *dev, pm_message_t state) -{ - return pci_save_state(dev); -} - -static int pcie_portdrv_resume_early(struct pci_dev *dev) -{ - return pci_restore_state(dev); -} - static int pcie_portdrv_resume(struct pci_dev *dev) { pcie_portdrv_restore_config(dev); @@ -72,8 +62,6 @@ static int pcie_portdrv_resume(struct pci_dev *dev) } #else #define pcie_portdrv_suspend NULL -#define pcie_portdrv_suspend_late NULL -#define pcie_portdrv_resume_early NULL #define pcie_portdrv_resume NULL #endif @@ -292,8 +280,6 @@ static struct pci_driver pcie_portdriver = { .remove = pcie_portdrv_remove, .suspend = pcie_portdrv_suspend, - .suspend_late = pcie_portdrv_suspend_late, - .resume_early = pcie_portdrv_resume_early, .resume = pcie_portdrv_resume, .err_handler = &pcie_portdrv_err_handler,