apm.c: ignore_normal_resume is set to 1 a bit too late

FIX New Thomas Hood 2 participants 3 messages Jun 13, 2005 1 message not in the archive

Share & tools

Thomas Hood

author

2005-06-13 09:32 UTC #0 lore raw

This message contains a fix for a bug in the apm driver.

A bug report was submitted to the Debian BTS saying that on the submitter's system the apmd proxy script was being run twice on resume.

Having seen exactly the same problem some years ago and knowing that the solution then was to ensure that the ignore_normal_resume flag got set before there was any chance of an APM RESUME event being processed, I checked the current apm.c and I found that ignore_normal_resume was once again being set too late. I asked the submitter to move the line where the flag was set and he reported that this change solved the problem. I append the message in question. The line numbers I mention there are for Linux 2.6.11.

Please make the indicated change to the apm driver.

-------- Forwarded Message -------- [email protected] wrote to the submitter of Debian bug #310865:

Quoted 9 lines

In arch/i386/kernel/apm.c there is at approximately line 1229:
        ignore_normal_resume = 1;
Move this up so that it occurs right after line 1222:
        err = set_system_power_state(APM_STATE_SUSPEND);
Let us know if that helps.

It does. Very nice.
I don't understand what I did and how it works. Will you try to push that into kernel sources or is this no permanent fix?

Signature · 1 line

Thomas Hood <[email protected]>

A message that is not in the archive (sent off-list or not yet received): [email protected]

Thomas Hood

author

2005-06-13 13:39 UTC #2 lore raw

PATCH 1/1 apm.c: ignore_normal_resume is set a bit too late

Quoted 4 lines · Stephen Rothwell

I am not using APM any more and have no way to test such a change. So, can you please do a proper patch with comment and Signed-off-by line and send it to Andrew Morton ([email protected]) (and cc lkml, I guess). When I see it, I will Ack it to Andrew.

Summary: apm: Prevent double APM resume on Thinkpad X31

This patch causes the ignore_normal_resume flag to be set slightly earlier, before there is a chance that the apm driver will receive the normal resume event from the BIOS. (Addresses Debian bug #310865)

Signed-off-by Thomas Hood <[email protected]>

Signature · 1 line

Thomas Hood <[email protected]>

arch/i386/kernel/apm.c renamed from arch/i386/kernel/apm.c_ORIG +1 -1

@@ -1220,13 +1220,13 @@ static int suspend(int vetoable)
 
 	save_processor_state();
 	err = set_system_power_state(APM_STATE_SUSPEND);
+	ignore_normal_resume = 1;
 	restore_processor_state();
 
 	write_seqlock_irq(&xtime_lock);
 	spin_lock(&i8253_lock);
 	reinit_timer();
 	set_time();
-	ignore_normal_resume = 1;
 
 	spin_unlock(&i8253_lock);
 	write_sequnlock_irq(&xtime_lock);

Jesper Juhl

2005-06-13 21:41 UTC #3 lore raw

In reply to Thomas Hood

Quoted 26 lines · Thomas Hood

This message contains a fix for a bug in the apm driver.
A bug report was submitted to the Debian BTS saying that on the submitter's system the apmd proxy script was being run twice on resume.
Having seen exactly the same problem some years ago and knowing that the solution then was to ensure that the ignore_normal_resume flag got set before there was any chance of an APM RESUME event being processed, I checked the current apm.c and I found that ignore_normal_resume was once again being set too late. I asked the submitter to move the line where the flag was set and he reported that this change solved the problem. I append the message in question. The line numbers I mention there are for Linux 2.6.11.
Please make the indicated change to the apm driver.
-------- Forwarded Message -------- [email protected] wrote to the submitter of Debian bug #310865:
Quoted 8 lines
In arch/i386/kernel/apm.c there is at approximately line 1229:
        ignore_normal_resume = 1;
Move this up so that it occurs right after line 1222:
        err = set_system_power_state(APM_STATE_SUSPEND);

Quoted 9 lines

Quoted 1 line
Let us know if that helps.
It does. Very nice.
I don't understand what I did and how it works. Will you try to push that into kernel sources or is this no permanent fix? --
Thomas Hood <[email protected]>

Here it is in the form of a patch against 2.6.12-rc6-mm1 for easier merging - Stephen, Andrew, is this a valid fix? I don't know the apm code well enough to properly judge.
(also attached since gmail tends to mangle inline patches)

Prevent double resume in apm.

Signed-off-by Jesper juhl <[email protected]>

---

 arch/i386/kernel/apm.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.12-rc6-mm1-orig/arch/i386/kernel/apm.c 2005-06-12
15:58:34.000000000 +0200
+++ linux-2.6.12-rc6-mm1/arch/i386/kernel/apm.c 2005-06-13
23:39:57.000000000 +0200
@@ -1222,6 +1222,7 @@ static int suspend(int vetoable)

save_processor_state();
err = set_system_power_state(APM_STATE_SUSPEND);
+ ignore_normal_resume = 1;
restore_processor_state();

local_irq_disable();
@@ -1229,7 +1230,6 @@ static int suspend(int vetoable)
spin_lock(&i8253_lock); reinit_timer();
set_time();
- ignore_normal_resume = 1;

spin_unlock(&i8253_lock);
write_sequnlock(&xtime_lock);

Signature · 3 lines

Jesper Juhl <[email protected]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

arch/i386/kernel/apm.c +1 -1

@@ -1222,6 +1222,7 @@ static int suspend(int vetoable)
 
 	save_processor_state();
 	err = set_system_power_state(APM_STATE_SUSPEND);
+	ignore_normal_resume = 1;
 	restore_processor_state();
 
 	local_irq_disable();

@@ -1229,7 +1230,6 @@ static int suspend(int vetoable)
 	spin_lock(&i8253_lock);
 	reinit_timer();
 	set_time();
-	ignore_normal_resume = 1;
 
 	spin_unlock(&i8253_lock);
 	write_sequnlock(&xtime_lock);