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@xxxxxxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxx>
> > > Cc: Rafael J. Wysocki <rjw@xxxxxxx>
> > > Signed-off-by: Pekka Enberg <penberg@xxxxxxxxxxxxxx>
> > > ---
> > > 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
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
