Re: USB not processing APM suspend event properly?

Russell King (rmk@arm.linux.org.uk)
Wed, 12 Dec 2001 22:49:57 +0000


On Wed, Dec 12, 2001 at 10:41:33PM +0000, Alan Cox wrote:
> > my USB mouse, but then tried (twice) to reconnect it again
> > while the apm driver was still processing the suspend. The
> > attempts to reconnect failed. I presume there is something
> > wrong with this picture. Does this indicate a bug in
> > USB power management?
>
> More it indicates a bug in the APM code. Have a look at Russell King's
> patches which actually issue the APM and user mode events in the right
> order. There is a USB problem there too, but the USB problem can't be fixed
> sanely until the APM order is fixed

And for those who have an interest, here's the patch... It might be
outdated a little now though. It came out of a problem I had on my
laptop, where the machine drove itself into total battery exhaustion
because Linux wouldn't let the APM bios suspend/hibernate the machine.

This was caused by the PCMCIA network interface being off, and the apmd
scripts trying to fuser -k and unmount an in-use NFS partition, which
in turn wanted to generate NFS traffic to the server over the shut down
PCMCIA network interface...

--------------------- original mail follows --------------------

Basically, I've changed the order that things happen on suspend, such that
we always pass the event to apmd, and any other listeners on /dev/apm_bios.
Once everyone has replied (subject to the rules of course), it then sends
the PM_SUSPEND to the devices.

The original code sent PM_SUSPEND on receiving the original request, and
then multiple times each time a listener on /dev/apm_bios replied. Not
good if apmd wants to unmount NFS, and your NFS mounts require traffic
over your PCMCIA ether card!

--- orig/arch/i386/kernel/apm.c Wed Nov 7 14:46:35 2001
+++ linux/arch/i386/kernel/apm.c Wed Nov 7 15:01:22 2001
@@ -1133,40 +1133,21 @@
#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);
+ return -EIO;
+ }
+ }
+
get_time_diff();
cli();
err = apm_set_power_state(APM_STATE_SUSPEND);
@@ -1176,12 +1157,15 @@
err = APM_SUCCESS;
if (err != APM_SUCCESS)
apm_error("suspend", err);
- send_event(APM_NORMAL_RESUME);
sti();
+
+ pm_send_all(PM_RESUME, (void *)0);
+
queue_event(APM_NORMAL_RESUME, NULL);
+ err = (err == APM_SUCCESS) ? 0 : -EIO;
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 +1225,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 +1252,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 +1267,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 +1275,6 @@
case APM_CAPABILITY_CHANGE:
case APM_LOW_BATTERY:
case APM_POWER_STATUS_CHANGE:
- send_event(event);
queue_event(event, NULL);
break;

@@ -1304,12 +1283,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 +1457,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 +1467,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 +1501,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/