* RE: [linux-pm] [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq
@ 2004-11-26 2:38 Zhu, Yi
2004-11-26 11:52 ` Pavel Machek
0 siblings, 1 reply; 6+ messages in thread
From: Zhu, Yi @ 2004-11-26 2:38 UTC (permalink / raw)
To: Pavel Machek; +Cc: akpm, linux-kernel, linux-pm
Pavel Machek wrote:
> Okay, this should be better patch. It works here.
device_power_down() might fail, in this case we should
bail out, right?
Thanks,
-yi
> --- clean/kernel/power/swsusp.c 2004-10-19
> 14:16:29.000000000 +0200
> +++ linux/kernel/power/swsusp.c 2004-11-25
> 12:27:35.000000000 +0100
> @@ -854,11 +840,13 @@
> if ((error = arch_prepare_suspend()))
> return error;
> local_irq_disable();
> + device_power_down(3);
> save_processor_state();
> error = swsusp_arch_suspend();
> /* Restore control flow magically appears here */
> restore_processor_state(); restore_highmem();
> + device_power_up();
> local_irq_enable();
> return error;
> }
> @@ -878,6 +866,7 @@
> {
> int error;
> local_irq_disable();
> + device_power_down(3);
> /* We'll ignore saved state, but this gets preempt count (etc)
> right */ save_processor_state();
> error = swsusp_arch_resume();
> @@ -887,6 +876,7 @@
> BUG_ON(!error);
> restore_processor_state();
> restore_highmem();
> + device_power_up();
> local_irq_enable();
> return error;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [linux-pm] [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq
2004-11-26 2:38 [linux-pm] [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq Zhu, Yi
@ 2004-11-26 11:52 ` Pavel Machek
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2004-11-26 11:52 UTC (permalink / raw)
To: Zhu, Yi; +Cc: akpm, linux-kernel, linux-pm
Hi!
> > Okay, this should be better patch. It works here.
>
> device_power_down() might fail, in this case we should
> bail out, right?
Hmm, yes, you might want to add error=device_power_down(), if (error)
{ local_irq_enable(); return error; }. But I do not think system
devices really fail, and if they fail during resume, there's no good
way to recover the system, anyway...
I'd just apply this one, it will have to change after 2.6.10 anyway.
Pavel
> > --- clean/kernel/power/swsusp.c 2004-10-19
> > 14:16:29.000000000 +0200
> > +++ linux/kernel/power/swsusp.c 2004-11-25
> > 12:27:35.000000000 +0100
> > @@ -854,11 +840,13 @@
> > if ((error = arch_prepare_suspend()))
> > return error;
> > local_irq_disable();
> > + device_power_down(3);
> > save_processor_state();
> > error = swsusp_arch_suspend();
> > /* Restore control flow magically appears here */
> > restore_processor_state(); restore_highmem();
> > + device_power_up();
> > local_irq_enable();
> > return error;
> > }
> > @@ -878,6 +866,7 @@
> > {
> > int error;
> > local_irq_disable();
> > + device_power_down(3);
> > /* We'll ignore saved state, but this gets preempt count (etc)
> > right */ save_processor_state();
> > error = swsusp_arch_resume();
> > @@ -887,6 +876,7 @@
> > BUG_ON(!error);
> > restore_processor_state();
> > restore_highmem();
> > + device_power_up();
> > local_irq_enable();
> > return error;
> > }
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [linux-pm] [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq
@ 2004-11-25 4:59 Zhu, Yi
2004-11-25 11:28 ` Pavel Machek
2004-11-25 11:34 ` Pavel Machek
0 siblings, 2 replies; 6+ messages in thread
From: Zhu, Yi @ 2004-11-25 4:59 UTC (permalink / raw)
To: Pavel Machek; +Cc: akpm, linux-kernel, linux-pm
Pavel Machek wrote:
> Hi!
>
>> This patch makes the new swsusp code ( pm_suspend_disk since
>> 2.6.9-rc3) call suspend/resume functions for sysdev and devices in
>> dpm_off_irq list. Otherwise, PCI link device in the system won't
>> provide correct interrupt for PCI devices during resume.
>
> I do not think this is right approach; you enable interrupts
> then disable that again, potentially without interrupt controller
> being initialized.
>
> This should be better patch:
Agreed. Your patch solves the bug. But do you plan to deal with the
devices in dpm_off_irq list?
Thanks,
-yi
> --- clean/kernel/power/swsusp.c 2004-10-19
> 14:16:29.000000000 +0200
> +++ linux/kernel/power/swsusp.c 2004-11-23
> 23:11:04.000000000 +0100
> @@ -854,11 +840,13 @@
> if ((error = arch_prepare_suspend()))
> return error;
> local_irq_disable();
> + sysdev_suspend(3);
> save_processor_state();
> error = swsusp_arch_suspend();
> /* Restore control flow magically appears here */
> restore_processor_state(); restore_highmem();
> + sysdev_resume();
> local_irq_enable();
> return error;
> }
> @@ -878,6 +866,7 @@
> {
> int error;
> local_irq_disable();
> + sysdev_suspend(3);
> /* We'll ignore saved state, but this gets preempt
> count (etc) right */
> save_processor_state();
> error = swsusp_arch_resume();
> @@ -887,6 +876,7 @@
> BUG_ON(!error);
> restore_processor_state();
> restore_highmem();
> + sysdev_resume();
> local_irq_enable();
> return error;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [linux-pm] [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq
2004-11-25 4:59 Zhu, Yi
@ 2004-11-25 11:28 ` Pavel Machek
2004-11-25 11:34 ` Pavel Machek
1 sibling, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2004-11-25 11:28 UTC (permalink / raw)
To: Zhu, Yi; +Cc: akpm, linux-kernel, linux-pm
Hi!
> >> This patch makes the new swsusp code ( pm_suspend_disk since
> >> 2.6.9-rc3) call suspend/resume functions for sysdev and devices in
> >> dpm_off_irq list. Otherwise, PCI link device in the system won't
> >> provide correct interrupt for PCI devices during resume.
> >
> > I do not think this is right approach; you enable interrupts
> > then disable that again, potentially without interrupt controller
> > being initialized.
> >
> > This should be better patch:
>
> Agreed. Your patch solves the bug. But do you plan to deal with the
> devices in dpm_off_irq list?
Ouch, okay... Calling irq-off phase of device_suspend() is not
intuitive at all and I hate that -EAGAIN idea.
...ouch, wait, it is less messy than I expected. I have no business
calling sysdev_suspend directly.
I'll test the patch and post it in the next message.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-pm] [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq
2004-11-25 4:59 Zhu, Yi
2004-11-25 11:28 ` Pavel Machek
@ 2004-11-25 11:34 ` Pavel Machek
1 sibling, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2004-11-25 11:34 UTC (permalink / raw)
To: Zhu, Yi; +Cc: akpm, linux-kernel, linux-pm
Hi!
> >> This patch makes the new swsusp code ( pm_suspend_disk since
> >> 2.6.9-rc3) call suspend/resume functions for sysdev and devices in
> >> dpm_off_irq list. Otherwise, PCI link device in the system won't
> >> provide correct interrupt for PCI devices during resume.
> >
> > I do not think this is right approach; you enable interrupts
> > then disable that again, potentially without interrupt controller
> > being initialized.
> >
> > This should be better patch:
>
> Agreed. Your patch solves the bug. But do you plan to deal with the
> devices in dpm_off_irq list?
Okay, this should be better patch. It works here.
Pavel
--- clean/kernel/power/swsusp.c 2004-10-19 14:16:29.000000000 +0200
+++ linux/kernel/power/swsusp.c 2004-11-25 12:27:35.000000000 +0100
@@ -854,11 +840,13 @@
if ((error = arch_prepare_suspend()))
return error;
local_irq_disable();
+ device_power_down(3);
save_processor_state();
error = swsusp_arch_suspend();
/* Restore control flow magically appears here */
restore_processor_state();
restore_highmem();
+ device_power_up();
local_irq_enable();
return error;
}
@@ -878,6 +866,7 @@
{
int error;
local_irq_disable();
+ device_power_down(3);
/* We'll ignore saved state, but this gets preempt count (etc) right */
save_processor_state();
error = swsusp_arch_resume();
@@ -887,6 +876,7 @@
BUG_ON(!error);
restore_processor_state();
restore_highmem();
+ device_power_up();
local_irq_enable();
return error;
}
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq
@ 2004-11-24 10:18 Zhu, Yi
2004-11-24 10:25 ` [linux-pm] " Pavel Machek
0 siblings, 1 reply; 6+ messages in thread
From: Zhu, Yi @ 2004-11-24 10:18 UTC (permalink / raw)
To: akpm, linux-kernel, linux-pm
[-- Attachment #1: Type: text/plain, Size: 1817 bytes --]
Hi,
This patch makes the new swsusp code ( pm_suspend_disk since 2.6.9-rc3)
call suspend/resume functions for sysdev and devices in dpm_off_irq
list.
Otherwise, PCI link device in the system won't provide correct interrupt
for PCI
devices during resume.
See the real bug report here:
http://www.bughost.org/bugzilla/show_bug.cgi?id=363
Thanks,
-yi
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
diff -urp a/kernel/power/disk.c b/kernel/power/disk.c
--- a/kernel/power/disk.c 2004-11-12 05:45:35.000000000 +0800
+++ b/kernel/power/disk.c 2004-11-21 06:21:28.162720936 +0800
@@ -113,6 +113,11 @@ static inline void platform_finish(void)
static void finish(void)
{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ device_power_up();
+ local_irq_restore(flags);
device_resume();
platform_finish();
enable_nonboot_cpus();
@@ -124,6 +129,7 @@ static void finish(void)
static int prepare(void)
{
int error;
+ unsigned long flags;
pm_prepare_console();
@@ -147,7 +153,14 @@ static int prepare(void)
if ((error = device_suspend(PM_SUSPEND_DISK)))
goto Finish;
+ local_irq_save(flags);
+ if ((error = device_power_down(PM_SUSPEND_DISK))) {
+ local_irq_restore(flags);
+ goto Finish;
+ }
+ local_irq_restore(flags);
return 0;
+
Finish:
platform_finish();
Thaw:
diff -urp a/kernel/power/swsusp.c b/kernel/power/swsusp.c
--- a/kernel/power/swsusp.c 2004-11-12 05:45:35.000000000 +0800
+++ b/kernel/power/swsusp.c 2004-11-21 06:18:55.734893488 +0800
@@ -829,6 +829,11 @@ int suspend_prepare_image(void)
int swsusp_write(void)
{
int error;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ device_power_up();
+ local_irq_restore(flags);
device_resume();
lock_swapdevices();
error = write_suspend_image();
[-- Attachment #2: swsusp-sysdev.patch --]
[-- Type: application/octet-stream, Size: 1347 bytes --]
diff -urp a/kernel/power/disk.c b/kernel/power/disk.c
--- a/kernel/power/disk.c 2004-11-12 05:45:35.000000000 +0800
+++ b/kernel/power/disk.c 2004-11-21 06:21:28.162720936 +0800
@@ -113,6 +113,11 @@ static inline void platform_finish(void)
static void finish(void)
{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ device_power_up();
+ local_irq_restore(flags);
device_resume();
platform_finish();
enable_nonboot_cpus();
@@ -124,6 +129,7 @@ static void finish(void)
static int prepare(void)
{
int error;
+ unsigned long flags;
pm_prepare_console();
@@ -147,7 +153,14 @@ static int prepare(void)
if ((error = device_suspend(PM_SUSPEND_DISK)))
goto Finish;
+ local_irq_save(flags);
+ if ((error = device_power_down(PM_SUSPEND_DISK))) {
+ local_irq_restore(flags);
+ goto Finish;
+ }
+ local_irq_restore(flags);
return 0;
+
Finish:
platform_finish();
Thaw:
diff -urp a/kernel/power/swsusp.c b/kernel/power/swsusp.c
--- a/kernel/power/swsusp.c 2004-11-12 05:45:35.000000000 +0800
+++ b/kernel/power/swsusp.c 2004-11-21 06:18:55.734893488 +0800
@@ -829,6 +829,11 @@ int suspend_prepare_image(void)
int swsusp_write(void)
{
int error;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ device_power_up();
+ local_irq_restore(flags);
device_resume();
lock_swapdevices();
error = write_suspend_image();
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [linux-pm] [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq
2004-11-24 10:18 Zhu, Yi
@ 2004-11-24 10:25 ` Pavel Machek
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2004-11-24 10:25 UTC (permalink / raw)
To: Zhu, Yi; +Cc: akpm, linux-kernel, linux-pm
Hi!
> This patch makes the new swsusp code ( pm_suspend_disk since 2.6.9-rc3)
> call suspend/resume functions for sysdev and devices in dpm_off_irq
> list.
> Otherwise, PCI link device in the system won't provide correct interrupt
> for PCI
> devices during resume.
I do not think this is right approach; you enable interrupts then
disable that again, potentially without interrupt controller being
initialized.
This should be better patch:
Pavel
--- clean/kernel/power/swsusp.c 2004-10-19 14:16:29.000000000 +0200
+++ linux/kernel/power/swsusp.c 2004-11-23 23:11:04.000000000 +0100
@@ -854,11 +840,13 @@
if ((error = arch_prepare_suspend()))
return error;
local_irq_disable();
+ sysdev_suspend(3);
save_processor_state();
error = swsusp_arch_suspend();
/* Restore control flow magically appears here */
restore_processor_state();
restore_highmem();
+ sysdev_resume();
local_irq_enable();
return error;
}
@@ -878,6 +866,7 @@
{
int error;
local_irq_disable();
+ sysdev_suspend(3);
/* We'll ignore saved state, but this gets preempt count (etc) right */
save_processor_state();
error = swsusp_arch_resume();
@@ -887,6 +876,7 @@
BUG_ON(!error);
restore_processor_state();
restore_highmem();
+ sysdev_resume();
local_irq_enable();
return error;
}
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-11-27 5:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-11-26 2:38 [linux-pm] [PATCH] make pm_suspend_disk suspend/resume sysdev and dpm_off_irq Zhu, Yi
2004-11-26 11:52 ` Pavel Machek
-- strict thread matches above, loose matches on Subject: below --
2004-11-25 4:59 Zhu, Yi
2004-11-25 11:28 ` Pavel Machek
2004-11-25 11:34 ` Pavel Machek
2004-11-24 10:18 Zhu, Yi
2004-11-24 10:25 ` [linux-pm] " Pavel Machek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®