From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH v4] pm_ops: add system quiesce/activate hooks Date: Sat, 14 Apr 2007 00:09:32 +0200 Message-ID: <200704140009.33298.rjw@sisk.pl> References: <1175810054.3489.34.camel@johannes.berg> <1176499086.7052.124.camel@johannes.berg> <20070413213349.GL28264@elf.ucw.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20070413213349.GL28264@elf.ucw.cz> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-pm-bounces@lists.linux-foundation.org Errors-To: linux-pm-bounces@lists.linux-foundation.org To: Pavel Machek , Johannes Berg Cc: linux-pm@lists.linux-foundation.org List-Id: linux-pm@vger.kernel.org On Friday, 13 April 2007 23:33, Pavel Machek wrote: > On Fri 2007-04-13 23:18:05, Johannes Berg wrote: > > On Fri, 2007-04-13 at 23:12 +0200, Pavel Machek wrote: > > = > > = > > > > Whoops, I had that in an earlier version, it's not supposed to be c= alled > > > > for (u)swsusp because pm_ops involvement there is a hack anyway. I'= ll > > > > fix the description. > > > = > > > But (u)swsusp needs to shut down the devices, and it does both suspend > > > and powerdown, too...? > > = > > Yes, but the special S4 state is just hacked onto pm_ops, nothing but > > ACPI S4 will ever sanely use pm_ops. If I'd invented pm_ops I'd made > > pm_ops solely for suspend to ram/standby/... and added a *separate* hook > > for pm_disk_mode and S4 because logically that really doesn't belong to > > pm_ops which does almost exclusively things for suspend to ram/standby. > = > Still, swsusp shuts down the devices: > = > if ((error =3D arch_prepare_suspend())) > return error; > = > local_irq_disable(); > /* At this point, device_suspend() has been called, but *not* > * device_power_down(). We *must* device_power_down() now. > * Otherwise, drivers for some devices (e.g. interrupt > controllers) > * become desynchronized with the actual state of the hardware > * at resume time, and evil weirdness ensues. > */ > if ((error =3D device_power_down(PMSG_FREEZE))) { > printk(KERN_ERR "Some devices failed to power down, abort= ing suspend\n"); > goto Enable_irqs; > } > = > This local_irq_disable() is very similar to the one you are doing in > suspend-to-RAM. According to your description, it needs to play with > the decrementor, too. But I'd really prefer not to have pm_ops call > here -- exactly because swsusp has nothing to do with pm_ops in > shutdown mode. Hmm, I missed that. :-( I think we need to make things clear: Either we add the additional hooks to pm_ops in which case they should be taken into account in the (u)swsusp code too, or we don't add them at all. Well, there also is one more solution. Namely, we can add the hooks to pm_= ops and make (u)swsusp use something else instead of pm_ops, but that would req= uire some more consideration. Greetings, Rafael