[RFC/PATCH 2/2] driver core: power management debugging

[RFC/PATCH 2/2] driver core: power management debugging

[Pekka J Enberg]

From: Nigel Cunningham <nigel@nigel.suspend2.net>

Add power management related debugging into driver core. Make the
kernel complain if a device driver lacks bus and class support for
resume or if a PCI or USB driver does not have a driver specific
resume function.

Cc: Pavel Machek <pavel@ucw.cz>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 drivers/base/core.c       |   13 +++++++++++++
 drivers/pci/pci-driver.c  |    6 ++++++
 drivers/usb/core/driver.c |    6 ++++++
 include/linux/device.h    |    1 +
 4 files changed, 26 insertions(+)

Index: 2.6/drivers/base/core.c
===================================================================
--- 2.6.orig/drivers/base/core.c	2007-04-27 14:42:13.000000000 +0300
+++ 2.6/drivers/base/core.c	2007-04-27 14:43:14.000000000 +0300
@@ -652,6 +652,18 @@ int device_add(struct device *dev)
 				class_intf->add_dev(dev, class_intf);
 		up(&dev->class->sem);
 	}
+
+#ifdef CONFIG_PM
+	if (!((dev->class && dev->class->resume) ||
+	      (dev->bus && (dev->bus->resume || dev->bus->resume_early))) &&
+	    !dev->pm_safe) {
+		printk(KERN_WARNING "Device driver %s lacks bus and class "
+				    "support for being resumed.\n",
+				    kobject_name(&dev->kobj));
+		dump_stack();
+	}
+#endif
+
  Done:
  	kfree(class_name);
 	put_device(dev);
@@ -989,6 +1001,7 @@ struct device *device_create(struct clas
 	dev->class = class;
 	dev->parent = parent;
 	dev->release = device_create_release;
+	dev->pm_safe = 1;
 
 	va_start(args, fmt);
 	vsnprintf(dev->bus_id, BUS_ID_SIZE, fmt, args);
Index: 2.6/drivers/pci/pci-driver.c
===================================================================
--- 2.6.orig/drivers/pci/pci-driver.c	2007-04-27 14:42:13.000000000 +0300
+++ 2.6/drivers/pci/pci-driver.c	2007-04-27 14:43:14.000000000 +0300
@@ -451,6 +451,12 @@ int __pci_register_driver(struct pci_dri
 	if (error)
 		driver_unregister(&drv->driver);
 
+#ifdef CONFIG_PM
+	if (!drv->resume)
+		printk(KERN_WARNING "PCI driver %s lacks driver specific "
+				    "resume support.\n", drv->name);
+#endif
+
 	return error;
 }
 
Index: 2.6/drivers/usb/core/driver.c
===================================================================
--- 2.6.orig/drivers/usb/core/driver.c	2007-04-27 14:42:13.000000000 +0300
+++ 2.6/drivers/usb/core/driver.c	2007-04-27 14:43:14.000000000 +0300
@@ -721,6 +721,12 @@ 	int retval = 0;
 		pr_info("%s: registered new device driver %s\n",
 			usbcore_name, new_udriver->name);
 		usbfs_update_special();
+#ifdef CONFIG_PM
+		if (!new_udriver->resume)
+			printk(KERN_WARNING "USB driver %s lacks driver "
+					    "specific resume support.\n",
+					    new_udriver->name);
+#endif
 	} else {
 		printk(KERN_ERR "%s: error %d registering device "
 			"	driver %s\n",
Index: 2.6/include/linux/device.h
===================================================================
--- 2.6.orig/include/linux/device.h	2007-04-27 14:42:13.000000000 +0300
+++ 2.6/include/linux/device.h	2007-04-27 14:43:14.000000000 +0300
@@ -401,6 +401,7 @@ struct device {
 	char	bus_id[BUS_ID_SIZE];	/* position on parent bus */
 	struct device_type	*type;
 	unsigned		is_registered:1;
+	unsigned		pm_safe:1; /* no resume function is safe */
 	struct device_attribute uevent_attr;
 	struct device_attribute *devt_attr;
 

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Pavel Machek]

Hi!

> Add power management related debugging into driver core. Make the
> kernel complain if a device driver lacks bus and class support for
> resume or if a PCI or USB driver does not have a driver specific
> resume function.
> 
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

Looks ok to me. Probably should go to Greg.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Alan Stern]

On Fri, 27 Apr 2007, Pekka J Enberg wrote:

> From: Nigel Cunningham <nigel@nigel.suspend2.net>
> 
> Add power management related debugging into driver core. Make the
> kernel complain if a device driver lacks bus and class support for
> resume or if a PCI or USB driver does not have a driver specific
> resume function.

> Index: 2.6/drivers/usb/core/driver.c
> ===================================================================
> --- 2.6.orig/drivers/usb/core/driver.c	2007-04-27 14:42:13.000000000 +0300
> +++ 2.6/drivers/usb/core/driver.c	2007-04-27 14:43:14.000000000 +0300
> @@ -721,6 +721,12 @@ 	int retval = 0;
>  		pr_info("%s: registered new device driver %s\n",
>  			usbcore_name, new_udriver->name);
>  		usbfs_update_special();
> +#ifdef CONFIG_PM
> +		if (!new_udriver->resume)
> +			printk(KERN_WARNING "USB driver %s lacks driver "
> +					    "specific resume support.\n",
> +					    new_udriver->name);
> +#endif
>  	} else {
>  		printk(KERN_ERR "%s: error %d registering device "
>  			"	driver %s\n",

This part seems unnecessary.  There is only one USB device driver, it is
built into the USB core, and it does have the appropriate methods.  
Checking isn't needed.

Now perhaps you would prefer to check the USB interface drivers -- there 
are many of them, and quite a few don't have suspend or resume methods.  
You would need to modify usb_register_driver() instead of 
usb_register_device_driver().

On the other hand, the drivers' maintainers are probably quite aware of 
the missing PM support, so it's not clear that printing out warning 
messages will actually help anybody.

Alan Stern

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Pekka Enberg]

On 4/27/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> Now perhaps you would prefer to check the USB interface drivers -- there
> are many of them, and quite a few don't have suspend or resume methods.
> You would need to modify usb_register_driver() instead of
> usb_register_device_driver().

I'll change that. Thanks.

On 4/27/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> On the other hand, the drivers' maintainers are probably quite aware of
> the missing PM support, so it's not clear that printing out warning
> messages will actually help anybody.

It will help to debug a swsusp problem that is reported by a non-developer, no?

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Greg KH]

On Fri, Apr 27, 2007 at 03:25:46PM +0300, Pekka J Enberg wrote:
> From: Nigel Cunningham <nigel@nigel.suspend2.net>
> 
> Add power management related debugging into driver core. Make the
> kernel complain if a device driver lacks bus and class support for
> resume or if a PCI or USB driver does not have a driver specific
> resume function.
> 
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
>  drivers/base/core.c       |   13 +++++++++++++
>  drivers/pci/pci-driver.c  |    6 ++++++
>  drivers/usb/core/driver.c |    6 ++++++
>  include/linux/device.h    |    1 +
>  4 files changed, 26 insertions(+)
> 
> Index: 2.6/drivers/base/core.c
> ===================================================================
> --- 2.6.orig/drivers/base/core.c	2007-04-27 14:42:13.000000000 +0300
> +++ 2.6/drivers/base/core.c	2007-04-27 14:43:14.000000000 +0300
> @@ -652,6 +652,18 @@ int device_add(struct device *dev)
>  				class_intf->add_dev(dev, class_intf);
>  		up(&dev->class->sem);
>  	}
> +
> +#ifdef CONFIG_PM
> +	if (!((dev->class && dev->class->resume) ||
> +	      (dev->bus && (dev->bus->resume || dev->bus->resume_early))) &&
> +	    !dev->pm_safe) {
> +		printk(KERN_WARNING "Device driver %s lacks bus and class "
> +				    "support for being resumed.\n",
> +				    kobject_name(&dev->kobj));
> +		dump_stack();
> +	}
> +#endif


I think you are reporting the wrong thing here, we want to know about
the busses and classes without suspend support, not the individual
devices, right?

And dumping stuff like this to the log for every bus/class isn't the
nicest :(


> +
>   Done:
>   	kfree(class_name);
>  	put_device(dev);
> @@ -989,6 +1001,7 @@ struct device *device_create(struct clas
>  	dev->class = class;
>  	dev->parent = parent;
>  	dev->release = device_create_release;
> +	dev->pm_safe = 1;

I don't understand the use of this flag, it looks like it is only being
set, which doesn't really make it very useful.

So NAK on this patch.

Oh, and please CC: me on driver core and USB and PCI core changes
please...

thanks,

greg k-h

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Nigel Cunningham]

[-- Attachment #1.1: Type: text/plain, Size: 2659 bytes --]

Hi Alan.

On Fri, 2007-04-27 at 10:08 -0400, Alan Stern wrote:
> On Fri, 27 Apr 2007, Pekka J Enberg wrote:
> 
> > From: Nigel Cunningham <nigel@nigel.suspend2.net>
> > 
> > Add power management related debugging into driver core. Make the
> > kernel complain if a device driver lacks bus and class support for
> > resume or if a PCI or USB driver does not have a driver specific
> > resume function.
> 
> > Index: 2.6/drivers/usb/core/driver.c
> > ===================================================================
> > --- 2.6.orig/drivers/usb/core/driver.c	2007-04-27 14:42:13.000000000 +0300
> > +++ 2.6/drivers/usb/core/driver.c	2007-04-27 14:43:14.000000000 +0300
> > @@ -721,6 +721,12 @@ 	int retval = 0;
> >  		pr_info("%s: registered new device driver %s\n",
> >  			usbcore_name, new_udriver->name);
> >  		usbfs_update_special();
> > +#ifdef CONFIG_PM
> > +		if (!new_udriver->resume)
> > +			printk(KERN_WARNING "USB driver %s lacks driver "
> > +					    "specific resume support.\n",
> > +					    new_udriver->name);
> > +#endif
> >  	} else {
> >  		printk(KERN_ERR "%s: error %d registering device "
> >  			"	driver %s\n",
> 
> This part seems unnecessary.  There is only one USB device driver, it is
> built into the USB core, and it does have the appropriate methods.  
> Checking isn't needed.

Sorry. I thought you were wrong for a minute, but then I looked again at
the messages in my dmesg...

[   33.944214] Device driver usbdev1.1_ep00 lacks bus and class support for being resumed.
[   34.051765] Device driver usbdev1.1_ep81 lacks bus and class support for being resumed.
[   34.113740] Device driver usbdev2.1_ep00 lacks bus and class support for being resumed.
[   34.221541] Device driver usbdev2.1_ep81 lacks bus and class support for being resumed.
[   34.251562] Device driver usbdev3.1_ep00 lacks bus and class support for being resumed.
[   34.361345] Device driver usbdev3.1_ep81 lacks bus and class support for being resumed.

They're coming from the other printk, of course.

> Now perhaps you would prefer to check the USB interface drivers -- there 
> are many of them, and quite a few don't have suspend or resume methods.  
> You would need to modify usb_register_driver() instead of 
> usb_register_device_driver().

Would they be the ones covered above?

> On the other hand, the drivers' maintainers are probably quite aware of 
> the missing PM support, so it's not clear that printing out warning 
> messages will actually help anybody.

It can help the user, when they're looking for possibilities as to why things aren't working.

Regards,

Nigel

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Nigel Cunningham]

[-- Attachment #1.1: Type: text/plain, Size: 2475 bytes --]

Hi.

On Fri, 2007-04-27 at 08:40 -0700, Greg KH wrote:
> On Fri, Apr 27, 2007 at 03:25:46PM +0300, Pekka J Enberg wrote:
> > From: Nigel Cunningham <nigel@nigel.suspend2.net>
> > 
> > Add power management related debugging into driver core. Make the
> > kernel complain if a device driver lacks bus and class support for
> > resume or if a PCI or USB driver does not have a driver specific
> > resume function.
> > 
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Rafael J. Wysocki <rjw@sisk.pl>
> > Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> > ---
> >  drivers/base/core.c       |   13 +++++++++++++
> >  drivers/pci/pci-driver.c  |    6 ++++++
> >  drivers/usb/core/driver.c |    6 ++++++
> >  include/linux/device.h    |    1 +
> >  4 files changed, 26 insertions(+)
> > 
> > Index: 2.6/drivers/base/core.c
> > ===================================================================
> > --- 2.6.orig/drivers/base/core.c	2007-04-27 14:42:13.000000000 +0300
> > +++ 2.6/drivers/base/core.c	2007-04-27 14:43:14.000000000 +0300
> > @@ -652,6 +652,18 @@ int device_add(struct device *dev)
> >  				class_intf->add_dev(dev, class_intf);
> >  		up(&dev->class->sem);
> >  	}
> > +
> > +#ifdef CONFIG_PM
> > +	if (!((dev->class && dev->class->resume) ||
> > +	      (dev->bus && (dev->bus->resume || dev->bus->resume_early))) &&
> > +	    !dev->pm_safe) {
> > +		printk(KERN_WARNING "Device driver %s lacks bus and class "
> > +				    "support for being resumed.\n",
> > +				    kobject_name(&dev->kobj));
> > +		dump_stack();
> > +	}
> > +#endif
> 
> 
> I think you are reporting the wrong thing here, we want to know about
> the busses and classes without suspend support, not the individual
> devices, right?

Yeah, I suppose you're right. I started off seeking to make a message
for each device lacking anything that would save its state. Guess I got
muddled somewhere :)

> And dumping stuff like this to the log for every bus/class isn't the
> nicest :(
> 
> 
> > +
> >   Done:
> >   	kfree(class_name);
> >  	put_device(dev);
> > @@ -989,6 +1001,7 @@ struct device *device_create(struct clas
> >  	dev->class = class;
> >  	dev->parent = parent;
> >  	dev->release = device_create_release;
> > +	dev->pm_safe = 1;
> 
> I don't understand the use of this flag, it looks like it is only being
> set, which doesn't really make it very useful.

The use was above (!dev->pm_safe).

Regards,

Nigel


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Greg KH]

On Sat, Apr 28, 2007 at 07:38:17AM +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Fri, 2007-04-27 at 08:40 -0700, Greg KH wrote:
> > On Fri, Apr 27, 2007 at 03:25:46PM +0300, Pekka J Enberg wrote:
> > > From: Nigel Cunningham <nigel@nigel.suspend2.net>
> > > 
> > > Add power management related debugging into driver core. Make the
> > > kernel complain if a device driver lacks bus and class support for
> > > resume or if a PCI or USB driver does not have a driver specific
> > > resume function.
> > > 
> > > Cc: Pavel Machek <pavel@ucw.cz>
> > > Cc: Rafael J. Wysocki <rjw@sisk.pl>
> > > Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> > > ---
> > >  drivers/base/core.c       |   13 +++++++++++++
> > >  drivers/pci/pci-driver.c  |    6 ++++++
> > >  drivers/usb/core/driver.c |    6 ++++++
> > >  include/linux/device.h    |    1 +
> > >  4 files changed, 26 insertions(+)
> > > 
> > > Index: 2.6/drivers/base/core.c
> > > ===================================================================
> > > --- 2.6.orig/drivers/base/core.c	2007-04-27 14:42:13.000000000 +0300
> > > +++ 2.6/drivers/base/core.c	2007-04-27 14:43:14.000000000 +0300
> > > @@ -652,6 +652,18 @@ int device_add(struct device *dev)
> > >  				class_intf->add_dev(dev, class_intf);
> > >  		up(&dev->class->sem);
> > >  	}
> > > +
> > > +#ifdef CONFIG_PM
> > > +	if (!((dev->class && dev->class->resume) ||
> > > +	      (dev->bus && (dev->bus->resume || dev->bus->resume_early))) &&
> > > +	    !dev->pm_safe) {
> > > +		printk(KERN_WARNING "Device driver %s lacks bus and class "
> > > +				    "support for being resumed.\n",
> > > +				    kobject_name(&dev->kobj));
> > > +		dump_stack();
> > > +	}
> > > +#endif
> > 
> > 
> > I think you are reporting the wrong thing here, we want to know about
> > the busses and classes without suspend support, not the individual
> > devices, right?
> 
> Yeah, I suppose you're right. I started off seeking to make a message
> for each device lacking anything that would save its state. Guess I got
> muddled somewhere :)
> 
> > And dumping stuff like this to the log for every bus/class isn't the
> > nicest :(
> > 
> > 
> > > +
> > >   Done:
> > >   	kfree(class_name);
> > >  	put_device(dev);
> > > @@ -989,6 +1001,7 @@ struct device *device_create(struct clas
> > >  	dev->class = class;
> > >  	dev->parent = parent;
> > >  	dev->release = device_create_release;
> > > +	dev->pm_safe = 1;
> > 
> > I don't understand the use of this flag, it looks like it is only being
> > set, which doesn't really make it very useful.
> 
> The use was above (!dev->pm_safe).

Yes, but nothing was ever turnning that flag off, so how could that
check ever fail?

Oh wait, it's only set in device_create()...

Ugh, device_create is _not_ the proper place for that, lots of kernel
code adds struct device to the tree with a class assigned to it without
using device_create(), that's just there to be a helper for people who
don't need the additional flexibility.

So that code isn't even doing what you think it is doing :)

This patch is pretty much completely wrong :(

thanks,

greg k-h

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Greg KH]

On Sat, Apr 28, 2007 at 07:25:59AM +1000, Nigel Cunningham wrote:
> Hi Alan.
> 
> On Fri, 2007-04-27 at 10:08 -0400, Alan Stern wrote:
> > On Fri, 27 Apr 2007, Pekka J Enberg wrote:
> > 
> > > From: Nigel Cunningham <nigel@nigel.suspend2.net>
> > > 
> > > Add power management related debugging into driver core. Make the
> > > kernel complain if a device driver lacks bus and class support for
> > > resume or if a PCI or USB driver does not have a driver specific
> > > resume function.
> > 
> > > Index: 2.6/drivers/usb/core/driver.c
> > > ===================================================================
> > > --- 2.6.orig/drivers/usb/core/driver.c	2007-04-27 14:42:13.000000000 +0300
> > > +++ 2.6/drivers/usb/core/driver.c	2007-04-27 14:43:14.000000000 +0300
> > > @@ -721,6 +721,12 @@ 	int retval = 0;
> > >  		pr_info("%s: registered new device driver %s\n",
> > >  			usbcore_name, new_udriver->name);
> > >  		usbfs_update_special();
> > > +#ifdef CONFIG_PM
> > > +		if (!new_udriver->resume)
> > > +			printk(KERN_WARNING "USB driver %s lacks driver "
> > > +					    "specific resume support.\n",
> > > +					    new_udriver->name);
> > > +#endif
> > >  	} else {
> > >  		printk(KERN_ERR "%s: error %d registering device "
> > >  			"	driver %s\n",
> > 
> > This part seems unnecessary.  There is only one USB device driver, it is
> > built into the USB core, and it does have the appropriate methods.  
> > Checking isn't needed.
> 
> Sorry. I thought you were wrong for a minute, but then I looked again at
> the messages in my dmesg...
> 
> [   33.944214] Device driver usbdev1.1_ep00 lacks bus and class support for being resumed.
> [   34.051765] Device driver usbdev1.1_ep81 lacks bus and class support for being resumed.
> [   34.113740] Device driver usbdev2.1_ep00 lacks bus and class support for being resumed.
> [   34.221541] Device driver usbdev2.1_ep81 lacks bus and class support for being resumed.
> [   34.251562] Device driver usbdev3.1_ep00 lacks bus and class support for being resumed.
> [   34.361345] Device driver usbdev3.1_ep81 lacks bus and class support for being resumed.

Those are usb endpoints and have NOTHING to do with suspend right now,
so the check is bogus.

> > On the other hand, the drivers' maintainers are probably quite aware of 
> > the missing PM support, so it's not clear that printing out warning 
> > messages will actually help anybody.
> 
> It can help the user, when they're looking for possibilities as to why things aren't working.

You will drown out the valid issues with invalid ones, like the above...

greg k-h

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Alan Stern]

On Sat, 28 Apr 2007, Nigel Cunningham wrote:

> Hi Alan.

> Sorry. I thought you were wrong for a minute, but then I looked again at
> the messages in my dmesg...
> 
> [   33.944214] Device driver usbdev1.1_ep00 lacks bus and class support for being resumed.
> [   34.051765] Device driver usbdev1.1_ep81 lacks bus and class support for being resumed.
> [   34.113740] Device driver usbdev2.1_ep00 lacks bus and class support for being resumed.
> [   34.221541] Device driver usbdev2.1_ep81 lacks bus and class support for being resumed.
> [   34.251562] Device driver usbdev3.1_ep00 lacks bus and class support for being resumed.
> [   34.361345] Device driver usbdev3.1_ep81 lacks bus and class support for being resumed.
> 
> They're coming from the other printk, of course.
> 
> > Now perhaps you would prefer to check the USB interface drivers -- there 
> > are many of them, and quite a few don't have suspend or resume methods.  
> > You would need to modify usb_register_driver() instead of 
> > usb_register_device_driver().
> 
> Would they be the ones covered above?

No.  As Greg pointed out, these usbdevXX_epYY "devices" are nothing but 
placeholders at the moment.  They don't actually do anything and they have 
no need for power management.  (But they do manage to clutter up the 
system log with lots of extraneous warnings from the PM core...)

> > On the other hand, the drivers' maintainers are probably quite aware of 
> > the missing PM support, so it's not clear that printing out warning 
> > messages will actually help anybody.
> 
> It can help the user, when they're looking for possibilities as to why things aren't working.

Maybe.  But the warnings will occur when the driver is registered, which 
is often long before the problem shows up.  The user may not make the 
connection.


On a completely different topic: Nigel, now's your big chance!  If you 
hurry, you can rename Suspend2 to Hibernate -- beating out Pavel, who will 
then be forced to rename swsusp to Hibernate2!  :-)

Alan Stern

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Nigel Cunningham]

[-- Attachment #1.1: Type: text/plain, Size: 2687 bytes --]

Hi.

On Sat, 2007-04-28 at 10:42 -0400, Alan Stern wrote:
> On Sat, 28 Apr 2007, Nigel Cunningham wrote:
> 
> > Hi Alan.
> 
> > Sorry. I thought you were wrong for a minute, but then I looked again at
> > the messages in my dmesg...
> > 
> > [   33.944214] Device driver usbdev1.1_ep00 lacks bus and class support for being resumed.
> > [   34.051765] Device driver usbdev1.1_ep81 lacks bus and class support for being resumed.
> > [   34.113740] Device driver usbdev2.1_ep00 lacks bus and class support for being resumed.
> > [   34.221541] Device driver usbdev2.1_ep81 lacks bus and class support for being resumed.
> > [   34.251562] Device driver usbdev3.1_ep00 lacks bus and class support for being resumed.
> > [   34.361345] Device driver usbdev3.1_ep81 lacks bus and class support for being resumed.
> > 
> > They're coming from the other printk, of course.
> > 
> > > Now perhaps you would prefer to check the USB interface drivers -- there 
> > > are many of them, and quite a few don't have suspend or resume methods.  
> > > You would need to modify usb_register_driver() instead of 
> > > usb_register_device_driver().
> > 
> > Would they be the ones covered above?
> 
> No.  As Greg pointed out, these usbdevXX_epYY "devices" are nothing but 
> placeholders at the moment.  They don't actually do anything and they have 
> no need for power management.  (But they do manage to clutter up the 
> system log with lots of extraneous warnings from the PM core...)

Ok, so they could have the pm_safe flag set to suppress the message.

> > > On the other hand, the drivers' maintainers are probably quite aware of 
> > > the missing PM support, so it's not clear that printing out warning 
> > > messages will actually help anybody.
> > 
> > It can help the user, when they're looking for possibilities as to why things aren't working.
> 
> Maybe.  But the warnings will occur when the driver is registered, which 
> is often long before the problem shows up.  The user may not make the 
> connection.

True, but if we make them show up earlier, they at least have a chance
to see what might cause problems before the problems occur. If we do it
at the time, they might have zero chance to see messages like this.

> On a completely different topic: Nigel, now's your big chance!  If you 
> hurry, you can rename Suspend2 to Hibernate -- beating out Pavel, who will 
> then be forced to rename swsusp to Hibernate2!  :-)

LOL. I don't care about names, or about beating Pavel at anything. I
just want to get better hibernation support in the kernel. It would be
nice to get rid of that awful swsusp name though! :)

Nigel

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Greg KH]

On Sun, Apr 29, 2007 at 08:55:52AM +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Sat, 2007-04-28 at 10:42 -0400, Alan Stern wrote:
> > On Sat, 28 Apr 2007, Nigel Cunningham wrote:
> > 
> > > Hi Alan.
> > 
> > > Sorry. I thought you were wrong for a minute, but then I looked again at
> > > the messages in my dmesg...
> > > 
> > > [   33.944214] Device driver usbdev1.1_ep00 lacks bus and class support for being resumed.
> > > [   34.051765] Device driver usbdev1.1_ep81 lacks bus and class support for being resumed.
> > > [   34.113740] Device driver usbdev2.1_ep00 lacks bus and class support for being resumed.
> > > [   34.221541] Device driver usbdev2.1_ep81 lacks bus and class support for being resumed.
> > > [   34.251562] Device driver usbdev3.1_ep00 lacks bus and class support for being resumed.
> > > [   34.361345] Device driver usbdev3.1_ep81 lacks bus and class support for being resumed.
> > > 
> > > They're coming from the other printk, of course.
> > > 
> > > > Now perhaps you would prefer to check the USB interface drivers -- there 
> > > > are many of them, and quite a few don't have suspend or resume methods.  
> > > > You would need to modify usb_register_driver() instead of 
> > > > usb_register_device_driver().
> > > 
> > > Would they be the ones covered above?
> > 
> > No.  As Greg pointed out, these usbdevXX_epYY "devices" are nothing but 
> > placeholders at the moment.  They don't actually do anything and they have 
> > no need for power management.  (But they do manage to clutter up the 
> > system log with lots of extraneous warnings from the PM core...)
> 
> Ok, so they could have the pm_safe flag set to suppress the message.

No, we don't want a flag just to shut up a message, that's the first
thing a developer will do when they see that message, without realizing
what exactly they should be doing instead.

Trust me, I know the lengths kernel developers go to to try to work
around "helpful hints" that the kernel can spit out at you :(

thanks,

greg k-h

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Nigel Cunningham]

[-- Attachment #1.1: Type: text/plain, Size: 2246 bytes --]

Hi.

On Sat, 2007-04-28 at 23:50 -0700, Greg KH wrote:
> On Sun, Apr 29, 2007 at 08:55:52AM +1000, Nigel Cunningham wrote:
> > Hi.
> > 
> > On Sat, 2007-04-28 at 10:42 -0400, Alan Stern wrote:
> > > On Sat, 28 Apr 2007, Nigel Cunningham wrote:
> > > 
> > > > Hi Alan.
> > > 
> > > > Sorry. I thought you were wrong for a minute, but then I looked again at
> > > > the messages in my dmesg...
> > > > 
> > > > [   33.944214] Device driver usbdev1.1_ep00 lacks bus and class support for being resumed.
> > > > [   34.051765] Device driver usbdev1.1_ep81 lacks bus and class support for being resumed.
> > > > [   34.113740] Device driver usbdev2.1_ep00 lacks bus and class support for being resumed.
> > > > [   34.221541] Device driver usbdev2.1_ep81 lacks bus and class support for being resumed.
> > > > [   34.251562] Device driver usbdev3.1_ep00 lacks bus and class support for being resumed.
> > > > [   34.361345] Device driver usbdev3.1_ep81 lacks bus and class support for being resumed.
> > > > 
> > > > They're coming from the other printk, of course.
> > > > 
> > > > > Now perhaps you would prefer to check the USB interface drivers -- there 
> > > > > are many of them, and quite a few don't have suspend or resume methods.  
> > > > > You would need to modify usb_register_driver() instead of 
> > > > > usb_register_device_driver().
> > > > 
> > > > Would they be the ones covered above?
> > > 
> > > No.  As Greg pointed out, these usbdevXX_epYY "devices" are nothing but 
> > > placeholders at the moment.  They don't actually do anything and they have 
> > > no need for power management.  (But they do manage to clutter up the 
> > > system log with lots of extraneous warnings from the PM core...)
> > 
> > Ok, so they could have the pm_safe flag set to suppress the message.
> 
> No, we don't want a flag just to shut up a message, that's the first
> thing a developer will do when they see that message, without realizing
> what exactly they should be doing instead.
> 
> Trust me, I know the lengths kernel developers go to to try to work
> around "helpful hints" that the kernel can spit out at you :(

Ok, then. So... what would you suggest (if anything)?

Regards,

Nigel

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC/PATCH 2/2] driver core: power management debugging

[Greg KH]

On Sun, Apr 29, 2007 at 05:47:11PM +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Sat, 2007-04-28 at 23:50 -0700, Greg KH wrote:
> > On Sun, Apr 29, 2007 at 08:55:52AM +1000, Nigel Cunningham wrote:
> > > Hi.
> > > 
> > > On Sat, 2007-04-28 at 10:42 -0400, Alan Stern wrote:
> > > > On Sat, 28 Apr 2007, Nigel Cunningham wrote:
> > > > 
> > > > > Hi Alan.
> > > > 
> > > > > Sorry. I thought you were wrong for a minute, but then I looked again at
> > > > > the messages in my dmesg...
> > > > > 
> > > > > [   33.944214] Device driver usbdev1.1_ep00 lacks bus and class support for being resumed.
> > > > > [   34.051765] Device driver usbdev1.1_ep81 lacks bus and class support for being resumed.
> > > > > [   34.113740] Device driver usbdev2.1_ep00 lacks bus and class support for being resumed.
> > > > > [   34.221541] Device driver usbdev2.1_ep81 lacks bus and class support for being resumed.
> > > > > [   34.251562] Device driver usbdev3.1_ep00 lacks bus and class support for being resumed.
> > > > > [   34.361345] Device driver usbdev3.1_ep81 lacks bus and class support for being resumed.
> > > > > 
> > > > > They're coming from the other printk, of course.
> > > > > 
> > > > > > Now perhaps you would prefer to check the USB interface drivers -- there 
> > > > > > are many of them, and quite a few don't have suspend or resume methods.  
> > > > > > You would need to modify usb_register_driver() instead of 
> > > > > > usb_register_device_driver().
> > > > > 
> > > > > Would they be the ones covered above?
> > > > 
> > > > No.  As Greg pointed out, these usbdevXX_epYY "devices" are nothing but 
> > > > placeholders at the moment.  They don't actually do anything and they have 
> > > > no need for power management.  (But they do manage to clutter up the 
> > > > system log with lots of extraneous warnings from the PM core...)
> > > 
> > > Ok, so they could have the pm_safe flag set to suppress the message.
> > 
> > No, we don't want a flag just to shut up a message, that's the first
> > thing a developer will do when they see that message, without realizing
> > what exactly they should be doing instead.
> > 
> > Trust me, I know the lengths kernel developers go to to try to work
> > around "helpful hints" that the kernel can spit out at you :(
> 
> Ok, then. So... what would you suggest (if anything)?

As your check was for something that wasn't really correct at all, I
suggest just dropping the patch :)

thanks,

greg k-h