Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S265805AbUATV6Q (ORCPT ); Tue, 20 Jan 2004 16:58:16 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S265808AbUATV6Q (ORCPT ); Tue, 20 Jan 2004 16:58:16 -0500 Received: from gate.crashing.org ([63.228.1.57]:40917 "EHLO gate.crashing.org") by vger.kernel.org with ESMTP id S265805AbUATV5a (ORCPT ); Tue, 20 Jan 2004 16:57:30 -0500 Subject: Re: Help port swsusp to ppc. From: Benjamin Herrenschmidt To: Pavel Machek Cc: ncunningham@users.sourceforge.net, Hugang , ncunningham@clear.net.nz, Linux Kernel Mailing List , debian-powerpc@lists.debian.org In-Reply-To: <20040120204407.GA9749@elf.ucw.cz> References: <20040119105237.62a43f65@localhost> <1074483354.10595.5.camel@gaston> <1074489645.2111.8.camel@laptop-linux> <1074490463.10595.16.camel@gaston> <1074534964.2505.6.camel@laptop-linux> <1074549790.10595.55.camel@gaston> <20040120204407.GA9749@elf.ucw.cz> Content-Type: text/plain Message-Id: <1074635683.795.43.camel@gaston> Mime-Version: 1.0 X-Mailer: Ximian Evolution 1.4.5 Date: Wed, 21 Jan 2004 08:54:44 +1100 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2654 Lines: 56 > FYI, this is that "ugly C-generated assembly" we are talking about. I > do not think it is so bad. The x86 version has been cleaned up and isn't _that_ bad, though it could definitely use some comments and I don't like the "Lxxx" labels, I'd rather either use number with the "nb" or "nf" GAS constructs or use real words labels. Looking at it though, I fail to see the need to get it generated by gcc in the first place :) The PPC version that was proposed is horrible. > FYI, there are exactly 6 variables in "nosave" section. Two loop > variables you can see in above code, one spinlock, number of pages to > save, pointer to directory of pages to be copied, and its length. > > I could probably move spinlock and length of pgdir out of there... That's not too much, you could probably afford having a one page header to the suspend image with those informations and the page copy loop (provided by the suspended kernel so you don't have _any_ compatibility issue and can even do it from the bootloader one day...) > > device_suspend is, imho, hairy too. We have some semantics that need > > cleanup here, I'll have to talk to Patrick about them. Putting devices > > to an idle state is what you need and what kexec need, and doesn't mean > > putting them to _sleep_. Or maybe we could pass a specific state to > > Okay, I can agree that putting them into sleep is not ideal [it > puzzles users, at least]. But it is quite simple and should work > okay. I do not want another round of device_suspend changes in > 2.6.X... [Well, perhaps if it was done in right&compatible way (tm), > it would be acceptable...] We already have the shutdown() callback or we can simply use device_suspend() with a D1 parameter instead of D3 or D4. That's what I'd do... Looking at swsusp code in current 2.6, when do you do that pass ? On the shutdown pass, you call devices_suspend(4); which is fine. But I don't see where you call devices_suspend(X) on the resume path. IMHO, that should be done from the boot kernel after loading the suspend image and before starting the resume process. Your mdelay(1000) for waiting for DMA to settle down is PLAIN WRONG imho, even dangerous. (And typically, an USB controller will still be hapilly be DMA'ing all over your memory). That's what I call idling devices at this point, and that's where I'd call devices_suspend(1), and we should do that for kexec too. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/