Re: [PATCH 00/19] What's in the suspend tree for 2.6.31

Re: [PATCH 00/19] What's in the suspend tree for 2.6.31

[Pavel Machek]

On Thu 2009-06-11 21:41:10, Rafael J. Wysocki wrote:
> Hi,
> 
> The following patches are in the linux-next branch of the
> suspend-2.6 tree and they are candidates for merging into
> 2.6.31.
> 
> Please let me know of any urgent PM patches that need to go into
> 2.6.31 and are not present in this series.

Something is very wrong with the series. 06/19 is missing subject line
(it is not in mail header) and all message-ids are same or something.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 00/19] What's in the suspend tree for 2.6.31

[Rafael J. Wysocki]

On Friday 12 June 2009, Pavel Machek wrote:
> On Thu 2009-06-11 21:41:10, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following patches are in the linux-next branch of the
> > suspend-2.6 tree and they are candidates for merging into
> > 2.6.31.
> > 
> > Please let me know of any urgent PM patches that need to go into
> > 2.6.31 and are not present in this series.
> 
> Something is very wrong with the series. 06/19 is missing subject line
> (it is not in mail header) and all message-ids are same or something.

Ah, sorry, my bad.  Fix in the works.

Thanks,
Rafael

Re: [PATCH 01/19] PM: Warn if interrupts are enabled during suspend-resume of sysdevs

[Pavel Machek]

On Thu 2009-06-11 21:41:15, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Sysdevs have to be suspended and resumed with interrupts disabled and
> things usually break in a way that's difficult to debug if one of
> sysdev drivers enables interrupts by mistake during suspend or
> resume.  Add extra checks that will generate warnings in such cases.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 02/19] x86: unify power/cpu_(32|64) headers

[Pavel Machek]

On Thu 2009-06-11 22:26:52, Rafael J. Wysocki wrote:
> From: Sergio Luis <sergio@larces.uece.br>
> 
> First step towards the unification of cpu_32.c and cpu_64.c.
> This commit unifies the headers of such files, making both
> of them use the same header files. It also remove the uneeded
> <module.h>.
> 
> Signed-off-by: Sergio Luis <sergio@larces.uece.br>
> Signed-off-by: Lauro Salmito <laurosalmito@gmail.com>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 03/19] x86: unify power/cpu_(32|64) global variables

[Pavel Machek]

On Thu 2009-06-11 22:33:13, Rafael J. Wysocki wrote:
> From: Sergio Luis <sergio@larces.uece.br>
> 
> Aiming total unification of cpu_32.c and cpu_64.c, in this step
> we do unify the global variables and existing forward declarations
> for such files.
> 
> Signed-off-by: Sergio Luis <sergio@larces.uece.br>
> Signed-off-by: Lauro Salmito <laurosalmito@gmail.com>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 07/19] x86: unify power/cpu_(32|64).c

[Pavel Machek]

On Thu 2009-06-11 22:35:11, Rafael J. Wysocki wrote:
> From: Sergio Luis <sergio@larces.uece.br>
> 
> This is the last unification step. Here we do remove one of the files
> and rename the left one as cpu.c, as both are now the same.
> Also update power/Makefile, telling it to build cpu.o, instead of
> cpu_(32|64).o
> 
> Signed-off-by: Sergio Luis <sergio@larces.uece.br>
> Signed-off-by: Lauro Salmito <laurosalmito@gmail.com>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>


> +#ifdef CONFIG_X86_32
> +	store_gdt(&ctxt->gdt);
> +	store_idt(&ctxt->idt);
> +#else
> +/* CONFIG_X86_64 */
> +	store_gdt((struct desc_ptr *)&ctxt->gdt_limit);
> +	store_idt((struct desc_ptr *)&ctxt->idt_limit);
> +#endif

I'd slightly prefer /* CONFIG_X86_64 */ to be moved to line above, or
dropped at all. it seems unneccessarily verbose.


> +	store_tr(ctxt->tr);
> +
> +	/* XMM0..XMM15 should be handled by kernel_fpu_begin(). */
> +	/*
> +	 * segment registers
> +	 */
> +#ifdef CONFIG_X86_32
> +	savesegment(es, ctxt->es);
> +	savesegment(fs, ctxt->fs);
> +	savesegment(gs, ctxt->gs);
> +	savesegment(ss, ctxt->ss);
> +#else
> +/* CONFIG_X86_64 */
> +	asm volatile ("movw %%ds, %0" : "=m" (ctxt->ds));
> +	asm volatile ("movw %%es, %0" : "=m" (ctxt->es));
> +	asm volatile ("movw %%fs, %0" : "=m" (ctxt->fs));
> +	asm volatile ("movw %%gs, %0" : "=m" (ctxt->gs));
> +	asm volatile ("movw %%ss, %0" : "=m" (ctxt->ss));
> +
> +	rdmsrl(MSR_FS_BASE, ctxt->fs_base);
> +	rdmsrl(MSR_GS_BASE, ctxt->gs_base);
> +	rdmsrl(MSR_KERNEL_GS_BASE, ctxt->gs_kernel_base);
> +	mtrr_save_fixed_ranges(NULL);
> +
> +	rdmsrl(MSR_EFER, ctxt->efer);
> +#endif

Can i386's mtrr_save_fixed_ranges() be moved to similar place?

> +	/*
> +	 * control registers
> +	 */
> +	ctxt->cr0 = read_cr0();
> +	ctxt->cr2 = read_cr2();
> +	ctxt->cr3 = read_cr3();
> +#ifdef CONFIG_X86_32
> +	ctxt->cr4 = read_cr4_safe();
> +#else
> +/* CONFIG_X86_64 */
> +	ctxt->cr4 = read_cr4();
> +	ctxt->cr8 = read_cr8();
> +#endif

Could we use read_cr4_safe on x86-64, too? Why the difference?

Should we be saving cr8 on 32-bit machines that have it? (That was
interrupt priority, IIRC?)

> +	/* cr4 was introduced in the Pentium CPU */
> +#ifdef CONFIG_X86_32
> +	if (ctxt->cr4)
> +		write_cr4(ctxt->cr4);
> +#else

Aha, is read_cr4_safe() needed because i486 does not have cr4?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 09/19] PM: Rename device_power_down/up()

[Pavel Machek]

On Thu 2009-06-11 22:37:28, Rafael J. Wysocki wrote:
> From: Magnus Damm <damm@igel.co.jp>
> 
> Rename the functions performing "_noirq" dev_pm_ops
> operations from device_power_down() and device_power_up()
> to device_suspend_noirq() and device_resume_noirq().
> 
> The new function names are chosen to show that the functions
> are responsible for calling the _noirq() versions to finalize
> the suspend/resume operation. The current function names do
> not perform power down/up anymore so the names may be misleading.
> 
> Global function renames:
> - device_power_down() -> device_suspend_noirq()
> - device_power_up() -> device_resume_noirq()
> 
> Static function renames:
> - suspend_device_noirq() -> __device_suspend_noirq()
> - resume_device_noirq() -> __device_resume_noirq()
> 
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> Acked-by: Len Brown <lenb@kernel.org>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 10/19] PM core: rename suspend and resume functions

[Pavel Machek]

On Thu 2009-06-11 22:38:06, Rafael J. Wysocki wrote:
> From: Alan Stern <stern@rowland.harvard.edu>
> 
> This patch (as1241) renames a bunch of functions in the PM core.
> Rather than go through a boring list of name changes, suffice it to
> say that in the end we have a bunch of pairs of functions:
> 
> 	device_resume_noirq	dpm_resume_noirq
> 	device_resume		dpm_resume
> 	device_complete		dpm_complete
> 	device_suspend_noirq	dpm_suspend_noirq
> 	device_suspend		dpm_suspend
> 	device_prepare		dpm_prepare
> 
> in which device_X does the X operation on a single device and dpm_X
> invokes device_X for all devices in the dpm_list.
> 
> In addition, the old dpm_power_up and device_resume_noirq have been
> combined into a single function (dpm_resume_noirq).
> 
> Lastly, dpm_suspend_start and dpm_resume_end are the renamed versions
> of the former top-level device_suspend and device_resume routines.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Acked-by: Magnus Damm <damm@igel.co.jp>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 12/19] PM/Suspend: Do not shrink memory before suspend

[Pavel Machek]

On Thu 2009-06-11 22:39:03, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Remove the shrinking of memory from the suspend-to-RAM code, where
> it is not really necessary.

I still have not seen any explanation why it is not neccessary.

It is true that I did not have time to stress my system to the point
when it has 0 pages free, but we certainly know that it is possible,
and we know that at least one driver allocates memory and will be
broken if that happens.

NAK.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 14/19] PM: Remove device_type suspend()/resume()

[Pavel Machek]

On Thu 2009-06-11 22:40:00, Rafael J. Wysocki wrote:
> From: Magnus Damm <damm@igel.co.jp>
> 
> This patch removes the legacy callbacks ->suspend() and
> ->resume() from struct device_type. These callbacks seem
> unused, and new code should instead make use of struct
> dev_pm_ops.
> 
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 15/19] Driver Core: Rework platform suspend/resume, print warning

[Pavel Machek]

On Thu 2009-06-11 22:40:30, Rafael J. Wysocki wrote:
> From: Magnus Damm <damm@igel.co.jp>
> 
> This patch reworks the platform driver code for legacy
> suspend and resume to avoid installing callbacks in
> struct device_driver. A warning is also added telling
> users to update the platform driver to use dev_pm_ops.
> 
> The functions platform_legacy_suspend()/resume() directly
> call suspend and resume callbacks in struct platform_driver
> instead of wrapping things in platform_drv_suspend()/resume().
> 
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 19/19] PM: Add empty suspend/resume device irq functions

[Pavel Machek]

On Thu 2009-06-11 22:42:09, Rafael J. Wysocki wrote:
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
> device interrupts" introduced some helper functions. However these
> functions are only available for architectures which support
> GENERIC_HARDIRQS.

I believe architectures that do not use GENERIC_HARDIRQs should just
provide the empty functions themselves.

If we want to help them, perhaps few attribute((weak)) functions in
the core are the way to go...
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 19/19] PM: Add empty suspend/resume device irq functions

[Heiko Carstens]

On Fri, Jun 12, 2009 at 01:13:17PM +0200, Pavel Machek wrote:
> On Thu 2009-06-11 22:42:09, Rafael J. Wysocki wrote:
> > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> > 
> > git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
> > device interrupts" introduced some helper functions. However these
> > functions are only available for architectures which support
> > GENERIC_HARDIRQS.
> 
> I believe architectures that do not use GENERIC_HARDIRQs should just
> provide the empty functions themselves.
> 
> If we want to help them, perhaps few attribute((weak)) functions in
> the core are the way to go...

We can still do that when there actually is an architecture that doesn't
support GENERIC_HARDIRQs _and_ needs non-empty functions.

Re: [PATCH 19/19] PM: Add empty suspend/resume device irq functions

[Pavel Machek]

On Fri 2009-06-12 13:32:18, Heiko Carstens wrote:
> On Fri, Jun 12, 2009 at 01:13:17PM +0200, Pavel Machek wrote:
> > On Thu 2009-06-11 22:42:09, Rafael J. Wysocki wrote:
> > > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> > > 
> > > git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
> > > device interrupts" introduced some helper functions. However these
> > > functions are only available for architectures which support
> > > GENERIC_HARDIRQS.
> > 
> > I believe architectures that do not use GENERIC_HARDIRQs should just
> > provide the empty functions themselves.
> > 
> > If we want to help them, perhaps few attribute((weak)) functions in
> > the core are the way to go...
> 
> We can still do that when there actually is an architecture that doesn't
> support GENERIC_HARDIRQs _and_ needs non-empty functions.

Ok, I guess that makes sense.

Acked-by: Pavel Machek <pavel@ucw.cz>
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 12/19] PM/Suspend: Do not shrink memory before suspend

[Rafael J. Wysocki]

On Friday 12 June 2009, Pavel Machek wrote:
> On Thu 2009-06-11 22:39:03, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Remove the shrinking of memory from the suspend-to-RAM code, where
> > it is not really necessary.
> 
> I still have not seen any explanation why it is not neccessary.
> 
> It is true that I did not have time to stress my system to the point
> when it has 0 pages free, but we certainly know that it is possible,
> and we know that at least one driver allocates memory and will be
> broken if that happens.
> 
> NAK.

Sorry, I'm going to let Linus decide about this one.

Best,
Rafael