Re: APM driver patch summary

From: Russell King (rmk@arm.linux.org.uk)
Date: Mon Dec 17 2001 - 17:22:24 EST


On Mon, Dec 17, 2001 at 10:04:02PM +0000, Russell King wrote:
> On Mon, Dec 17, 2001 at 01:28:13PM -0500, Thomas Hood wrote:
> > Each of these changes is required, IMHO. However, the Russell King
> > patch probably won't apply without modifications. Also, it needs to
> > be modified so that it will send a resume event to listeners in case
> > a driver rejects a suspend event that listeners have already
> > processed.
>
> It does do that - check the suspend() function.

Actually you're right, it doesn't send a resume event. However, It's
worse than that - it will leave all sleeping listeners still sleeping.

Try this patch instead - this should cause all listeners to get a -EIO
to indicate that the suspend failed, and also get the resume event.

Thanks for finding that. This (untested) patch should fix the problem.
(difference is in suspend()):

--- orig/arch/i386/kernel/apm.c Fri Nov 23 10:12:07 2001
+++ linux/arch/i386/kernel/apm.c Mon Dec 17 22:20:03 2001
@@ -1133,40 +1133,22 @@
 #endif
 }
 
-static int send_event(apm_event_t event)
-{
- switch (event) {
- case APM_SYS_SUSPEND:
- case APM_CRITICAL_SUSPEND:
- case APM_USER_SUSPEND:
- /* map all suspends to ACPI D3 */
- if (pm_send_all(PM_SUSPEND, (void *)3)) {
- if (event == APM_CRITICAL_SUSPEND) {
- printk(KERN_CRIT
- "apm: Critical suspend was vetoed, "
- "expect armageddon\n" );
- return 0;
- }
- if (apm_info.connection_version > 0x100)
- apm_set_power_state(APM_STATE_REJECT);
- return 0;
- }
- break;
- case APM_NORMAL_RESUME:
- case APM_CRITICAL_RESUME:
- /* map all resumes to ACPI D0 */
- (void) pm_send_all(PM_RESUME, (void *)0);
- break;
- }
-
- return 1;
-}
-
-static int suspend(void)
+static int suspend(int vetoable)
 {
         int err;
         struct apm_user *as;
 
+ if (pm_send_all(PM_SUSPEND, (void *)3)) {
+ if (!vetoable) {
+ printk(KERN_CRIT
+ "apm: suspend was vetoed, expect armageddon\n");
+ } else if (apm_info.connection_version > 0x100) {
+ apm_set_power_state(APM_STATE_REJECT);
+ err = -EIO;
+ goto out;
+ }
+ }
+
         get_time_diff();
         cli();
         err = apm_set_power_state(APM_STATE_SUSPEND);
@@ -1176,12 +1158,17 @@
                 err = APM_SUCCESS;
         if (err != APM_SUCCESS)
                 apm_error("suspend", err);
- send_event(APM_NORMAL_RESUME);
         sti();
+
+ pm_send_all(PM_RESUME, (void *)0);
+
+ err = (err == APM_SUCCESS) ? 0 : -EIO;
+
+ out:
         queue_event(APM_NORMAL_RESUME, NULL);
         for (as = user_list; as != NULL; as = as->next) {
                 as->suspend_wait = 0;
- as->suspend_result = ((err == APM_SUCCESS) ? 0 : -EIO);
+ as->suspend_result = err;
         }
         ignore_normal_resume = 1;
         wake_up_interruptible(&apm_suspend_waitqueue);
@@ -1241,11 +1228,9 @@
                 switch (event) {
                 case APM_SYS_STANDBY:
                 case APM_USER_STANDBY:
- if (send_event(event)) {
- queue_event(event, NULL);
- if (standbys_pending <= 0)
- standby();
- }
+ queue_event(event, NULL);
+ if (standbys_pending <= 0)
+ standby();
                         break;
 
                 case APM_USER_SUSPEND:
@@ -1270,12 +1255,10 @@
                          */
                         if (waiting_for_resume)
                                 return;
- if (send_event(event)) {
- queue_event(event, NULL);
- waiting_for_resume = 1;
- if (suspends_pending <= 0)
- (void) suspend();
- }
+ queue_event(event, NULL);
+ waiting_for_resume = 1;
+ if (suspends_pending <= 0)
+ (void) suspend(1);
                         break;
 
                 case APM_NORMAL_RESUME:
@@ -1287,7 +1270,7 @@
                         if ((event != APM_NORMAL_RESUME)
                             || (ignore_normal_resume == 0)) {
                                 set_time();
- send_event(event);
+ pm_send_all(PM_RESUME, (void *)0);
                                 queue_event(event, NULL);
                         }
                         break;
@@ -1295,7 +1278,6 @@
                 case APM_CAPABILITY_CHANGE:
                 case APM_LOW_BATTERY:
                 case APM_POWER_STATUS_CHANGE:
- send_event(event);
                         queue_event(event, NULL);
                         break;
 
@@ -1304,12 +1286,11 @@
                         break;
 
                 case APM_CRITICAL_SUSPEND:
- send_event(event);
                         /*
                          * We can only hope it worked - we are not allowed
                          * to reject a critical suspend.
                          */
- (void) suspend();
+ (void) suspend(0);
                         break;
                 }
         }
@@ -1479,9 +1460,7 @@
                         as->standbys_read--;
                         as->standbys_pending--;
                         standbys_pending--;
- } else if (!send_event(APM_USER_STANDBY))
- return -EAGAIN;
- else
+ } else
                         queue_event(APM_USER_STANDBY, as);
                 if (standbys_pending <= 0)
                         standby();
@@ -1491,13 +1470,10 @@
                         as->suspends_read--;
                         as->suspends_pending--;
                         suspends_pending--;
- } else if (!send_event(APM_USER_SUSPEND))
- return -EAGAIN;
- else
+ } else
                         queue_event(APM_USER_SUSPEND, as);
                 if (suspends_pending <= 0) {
- if (suspend() != APM_SUCCESS)
- return -EIO;
+ return suspend(1);
                 } else {
                         as->suspend_wait = 1;
                         wait_event_interruptible(apm_suspend_waitqueue,
@@ -1528,7 +1504,7 @@
         if (as->suspends_pending > 0) {
                 suspends_pending -= as->suspends_pending;
                 if (suspends_pending <= 0)
- (void) suspend();
+ (void) suspend(1);
         }
         if (user_list == as)
                 user_list = as->next;

--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

- 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/



This archive was generated by hypermail 2b29 : Sun Dec 23 2001 - 21:00:14 EST