Skip to contentSkip to conversation

Complain if driver reenables interrupts during drivers_[suspend|resume] & re-disable

PATCH In review Nigel Cunningham 5 participants 6 messages Feb 7, 2006 – Feb 8, 2006

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Complain if driver reenables interrupts during drivers_[suspend|resume] & re-disable

Hi all.

This patch is designed to help with diagnosing and fixing the cause of problems in suspending/resuming, due to drivers wrongly re-enabling interrupts in their .suspend or .resume methods.

I nearly forgot about it in sending patches in suspend2 that might help where swsusp fails.

 power/resume.c  |    5 +++++
 power/suspend.c |    5 +++++
 sys.c           |   37 +++++++++++++++++++++++++++++++++++--
 3 files changed, 45 insertions(+), 2 deletions(-)
diff -ruNp 8010-driver-model-debug.patch-old/drivers/base/power/resume.c 
8010-driver-model-debug.patch-new/drivers/base/power/resume.c
--- 8010-driver-model-debug.patch-old/drivers/base/power/resume.c	
2006-01-19 21:27:39.000000000 +1000
+++ 8010-driver-model-debug.patch-new/drivers/base/power/resume.c	
2006-01-31 19:54:52.000000000 +1000

@@ -35,6 +35,11 @@ int resume_device(struct device * dev)
if (dev->bus && dev->bus->resume) {
dev_dbg(dev,"resuming\n");
error = dev->bus->resume(dev);
+ if (!irqs_disabled()) {
+ printk(KERN_EMERG "WARNING: Interrupts reenabled while resuming device
%s.\n",
+ kobject_name(&dev->kobj));
+ local_irq_disable();
+ }
}
up(&dev->sem);
return error;
diff -ruNp 8010-driver-model-debug.patch-old/drivers/base/power/suspend.c 8010-driver-model-debug.patch-new/drivers/base/power/suspend.c --- 8010-driver-model-debug.patch-old/drivers/base/power/suspend.c

2006-01-19 21:27:39.000000000 +1000
+++ 8010-driver-model-debug.patch-new/drivers/base/power/suspend.c	
2006-01-31 19:54:44.000000000 +1000

@@ -58,6 +58,11 @@ int suspend_device(struct device * dev,
if (dev->bus && dev->bus->suspend && !dev->power.power_state.event) {
dev_dbg(dev, "suspending\n");
error = dev->bus->suspend(dev, state);
+ if (!irqs_disabled()) {
+ printk(KERN_EMERG "WARNING: Interrupts reenabled while suspending
device %s.\n",
+ kobject_name(&dev->kobj));
+ local_irq_disable();
+ }
}
up(&dev->sem);
return error;
diff -ruNp 8010-driver-model-debug.patch-old/drivers/base/sys.c 8010-driver-model-debug.patch-new/drivers/base/sys.c
--- 8010-driver-model-debug.patch-old/drivers/base/sys.c 2006-01-19
21:27:39.000000000 +1000
+++ 8010-driver-model-debug.patch-new/drivers/base/sys.c 2006-01-31
19:54:09.000000000 +1000
@@ -298,16 +298,34 @@ static void __sysdev_resume(struct sys_d
if (cls->resume)
cls->resume(dev);

+ if (!irqs_disabled()) {
+ printk(KERN_EMERG "WARNING: Interrupts reenabled while resuming sysdev
class specific driver %s.\n",
+ kobject_name(&dev->kobj));
+ local_irq_disable();
+ }
+
/* Call auxillary drivers next. */ list_for_each_entry(drv, &cls->drivers, entry) {
- if (drv->resume)
+ if (drv->resume) {
drv->resume(dev);
+ if (!irqs_disabled()) {
+ printk(KERN_EMERG "WARNING: Interrupts reenabled while resuming sysdev
class driver %s.\n",
+ kobject_name(&dev->kobj));
+ local_irq_disable();
+ }
+ }
}

/* Call global drivers. */ list_for_each_entry(drv, &sysdev_drivers, entry) {
- if (drv->resume)
+ if (drv->resume) {
drv->resume(dev);
+ if (!irqs_disabled()) {
+ printk(KERN_EMERG "WARNING: Interrupts reenabled while resuming sysdev
driver %s.\n",
+ kobject_name(&dev->kobj));
+ local_irq_disable();
+ }
+ }
}
}

@@ -346,6 +364,11 @@ int sysdev_suspend(pm_message_t state)
list_for_each_entry(drv, &sysdev_drivers, entry) {
if (drv->suspend) {
ret = drv->suspend(sysdev, state);
+ if (!irqs_disabled()) {
+ printk(KERN_EMERG "WARNING: Interrupts reenabled while suspending
sysdev driver %s.\n",
+ kobject_name(&sysdev->kobj));
+ local_irq_disable();
+ }
if (ret)
goto gbl_driver;
}
@@ -355,6 +378,11 @@ int sysdev_suspend(pm_message_t state)
list_for_each_entry(drv, &cls->drivers, entry) {
if (drv->suspend) {
ret = drv->suspend(sysdev, state);
+ if (!irqs_disabled()) {
+ printk(KERN_EMERG "WARNING: Interrupts reenabled while suspending
sysdev class driver %s.\n",
+ kobject_name(&sysdev->kobj));
+ local_irq_disable();
+ }
if (ret)
goto aux_driver;
}
@@ -363,6 +391,11 @@ int sysdev_suspend(pm_message_t state)
/* Now call the generic one */
if (cls->suspend) {
ret = cls->suspend(sysdev, state);
+ if (!irqs_disabled()) {
+ printk(KERN_EMERG "WARNING: Interrupts reenabled while suspending
class driver %s.\n",
+ kobject_name(&sysdev->kobj));
+ local_irq_disable();
+ }
if (ret)
goto cls_driver;
}

#1 lore raw
Quoted 3 lines · Nigel Cunningham

This patch is designed to help with diagnosing and fixing the cause of problems in suspending/resuming, due to drivers wrongly re-enabling interrupts in their .suspend or .resume methods.

Sorry, aren't interrupts enabled at this point anyway? They are if you issue a suspend to RAM via sysfs, eg:

state_store->enter_state->suspend_prepare->device_suspend->suspend_device

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core
#2 lore raw
In reply to Nigel Cunningham
Quoted 8 lines · Nigel Cunningham

Hi all.

This patch is designed to help with diagnosing and fixing the cause of problems in suspending/resuming, due to drivers wrongly re-enabling interrupts in their .suspend or .resume methods.

I nearly forgot about it in sending patches in suspend2 that might help where swsusp fails.

Ugh ? Interrupts aren't supposed to be off during resume()...

Ben.

Quoted 129 lines
 power/resume.c  |    5 +++++
 power/suspend.c |    5 +++++
 sys.c           |   37 +++++++++++++++++++++++++++++++++++--
 3 files changed, 45 insertions(+), 2 deletions(-)
diff -ruNp 8010-driver-model-debug.patch-old/drivers/base/power/resume.c 
8010-driver-model-debug.patch-new/drivers/base/power/resume.c
--- 8010-driver-model-debug.patch-old/drivers/base/power/resume.c	
2006-01-19 21:27:39.000000000 +1000
+++ 8010-driver-model-debug.patch-new/drivers/base/power/resume.c	
2006-01-31 19:54:52.000000000 +1000
(quoted diff) +5 -0
@@ -35,6 +35,11 @@ int resume_device(struct device * dev)
 	if (dev->bus && dev->bus->resume) {
 		dev_dbg(dev,"resuming\n");
 		error = dev->bus->resume(dev);
+		if (!irqs_disabled()) {
+			printk(KERN_EMERG "WARNING: Interrupts reenabled while resuming device 
 %s.\n",
+				kobject_name(&dev->kobj));
+			local_irq_disable();
+		}
 	}
 	up(&dev->sem);

return error;
diff -ruNp 8010-driver-model-debug.patch-old/drivers/base/power/suspend.c 8010-driver-model-debug.patch-new/drivers/base/power/suspend.c --- 8010-driver-model-debug.patch-old/drivers/base/power/suspend.c

2006-01-19 21:27:39.000000000 +1000
+++ 8010-driver-model-debug.patch-new/drivers/base/power/suspend.c	
2006-01-31 19:54:44.000000000 +1000
(quoted diff) +5 -0
@@ -58,6 +58,11 @@ int suspend_device(struct device * dev, 
 	if (dev->bus && dev->bus->suspend && !dev->power.power_state.event) {
 		dev_dbg(dev, "suspending\n");
 		error = dev->bus->suspend(dev, state);
+		if (!irqs_disabled()) {
+			printk(KERN_EMERG "WARNING: Interrupts reenabled while suspending 
 device %s.\n",
+				kobject_name(&dev->kobj));
+			local_irq_disable();
+		}
 	}
 	up(&dev->sem);

return error;
diff -ruNp 8010-driver-model-debug.patch-old/drivers/base/sys.c 8010-driver-model-debug.patch-new/drivers/base/sys.c
--- 8010-driver-model-debug.patch-old/drivers/base/sys.c 2006-01-19
21:27:39.000000000 +1000
+++ 8010-driver-model-debug.patch-new/drivers/base/sys.c 2006-01-31
19:54:09.000000000 +1000

(quoted diff) +20 -2
@@ -298,16 +298,34 @@ static void __sysdev_resume(struct sys_d
 	if (cls->resume)
 		cls->resume(dev);
 
+	if (!irqs_disabled()) {
+		printk(KERN_EMERG "WARNING: Interrupts reenabled while resuming sysdev 
 class specific driver %s.\n",
+				kobject_name(&dev->kobj));
+		local_irq_disable();
+	}
+
 	/* Call auxillary drivers next. */
 	list_for_each_entry(drv, &cls->drivers, entry) {
-		if (drv->resume)
+		if (drv->resume) {
 			drv->resume(dev);
+			if (!irqs_disabled()) {
+				printk(KERN_EMERG "WARNING: Interrupts reenabled while resuming sysdev 
 class driver %s.\n",
+				kobject_name(&dev->kobj));
+				local_irq_disable();
+			}
+		}
 	}
 
 	/* Call global drivers. */
 	list_for_each_entry(drv, &sysdev_drivers, entry) {
-		if (drv->resume)
+		if (drv->resume) {
 			drv->resume(dev);
+			if (!irqs_disabled()) {
+				printk(KERN_EMERG "WARNING: Interrupts reenabled while resuming sysdev 
 driver %s.\n",
+				kobject_name(&dev->kobj));
+				local_irq_disable();
+			}
+		}
(quoted diff) +5 -0
 	}
 }
@@ -346,6 +364,11 @@ int sysdev_suspend(pm_message_t state)
 			list_for_each_entry(drv, &sysdev_drivers, entry) {
 				if (drv->suspend) {
 					ret = drv->suspend(sysdev, state);
+					if (!irqs_disabled()) {
+						printk(KERN_EMERG "WARNING: Interrupts reenabled while suspending 
 sysdev driver %s.\n",
+							kobject_name(&sysdev->kobj));
+						local_irq_disable();
+					}
 					if (ret)
 						goto gbl_driver;
(quoted diff) +5 -0
 				}
@@ -355,6 +378,11 @@ int sysdev_suspend(pm_message_t state)
 			list_for_each_entry(drv, &cls->drivers, entry) {
 				if (drv->suspend) {
 					ret = drv->suspend(sysdev, state);
+					if (!irqs_disabled()) {
+						printk(KERN_EMERG "WARNING: Interrupts reenabled while suspending 
 sysdev class driver %s.\n",
+						kobject_name(&sysdev->kobj));
+						local_irq_disable();
+					}
 					if (ret)
 						goto aux_driver;
(quoted diff) +5 -0
 				}
@@ -363,6 +391,11 @@ int sysdev_suspend(pm_message_t state)
 			/* Now call the generic one */
 			if (cls->suspend) {
 				ret = cls->suspend(sysdev, state);
+				if (!irqs_disabled()) {
+					printk(KERN_EMERG "WARNING: Interrupts reenabled while suspending 
 class driver %s.\n",
+					kobject_name(&sysdev->kobj));
+					local_irq_disable();
+				}
 				if (ret)
 					goto cls_driver;

}

#3 lore raw
In reply to Nigel Cunningham
Quoted 9 lines · Nigel Cunningham

Hi all.

This patch is designed to help with diagnosing and fixing the cause of problems in suspending/resuming, due to drivers wrongly re-enabling interrupts in their .suspend or .resume methods.

I nearly forgot about it in sending patches in suspend2 that might help where swsusp fails.

Only sysdevs are guaranteed to be suspebded/resumed with interrupts off, other devices are suspended with interrupts on (at least on first pass over device list).

Signature · 1 line
Dmitry
#4 lore raw
Quoted 7 lines · Dmitry Torokhov
Quoted 6 lines · Nigel Cunningham

Hi all.

This patch is designed to help with diagnosing and fixing the cause of problems in suspending/resuming, due to drivers wrongly re-enabling interrupts in their .suspend or .resume methods.

Quoted 7 lines
Quoted 3 lines

I nearly forgot about it in sending patches in suspend2 that might help where swsusp fails.

Only sysdevs are guaranteed to be suspebded/resumed with interrupts off, other devices are suspended with interrupts on (at least on first pass over device list).

Yes, and AFAICT this is how it's supposed to be. [There was an LKML thread last year that finished with this conclusion. I think I can find it for reference, if necessary.]

Greetings,
Rafael

#5 lore raw
In reply to Dmitry Torokhov

Hi Dmitry et al.

Quoted 7 lines · Dmitry Torokhov
Quoted 6 lines · Nigel Cunningham

Hi all.

This patch is designed to help with diagnosing and fixing the cause of problems in suspending/resuming, due to drivers wrongly re-enabling interrupts in their .suspend or .resume methods.

Quoted 7 lines
Quoted 3 lines

I nearly forgot about it in sending patches in suspend2 that might help where swsusp fails.

Only sysdevs are guaranteed to be suspebded/resumed with interrupts off, other devices are suspended with interrupts on (at least on first pass over device list).

Ok. I guess I missed that outcome of that discussion. Sorry for the bogusness :(. Is the sysdev bit useful at all?

Regards,

Nigel