Re: [RFC Add no_suspend attribute V2] Let the driver know if it's in use

[Greg KH <greg@kroah.com>]

On Fri, Apr 24, 2009 at 07:55:55PM +0200, Michael Trimarchi wrote:
> >  /**
> > + *	device_set_no_suspend_enable - Mark the device as used by userspace
> > + *	application
> > + */

This is not proper kernel-doc, please fix this up.

And "no_suspend_enable" is ackward, drop the "enable" part?


> > +void device_set_no_suspend_enable(struct device *dev, bool enable)
> > +{
> > +	struct device *next;
> > +
> > +	mutex_lock(&dpm_list_mtx);
> > +
> > +	/* the new status is equal the old one */
> > +	if (dev->power.no_suspend == !!enable)
> > +		goto out;
> > +
> > +	/* change the device status */
> > +	dev->power.no_suspend = !!enable;
> > +	if (dev->power.no_suspend)
> > +		dev->power.subtree_no_suspend = 0;
> >   
> I find a bug here, i will fix.
> It can be ok the rest of the code?
> > +
> > +	list_for_each_entry_reverse(next, &dev->power.entry, power.entry) {
> > +		/* 
> > +		 * exit if we find a node with the same parent of the start
> > +		 * device
> > +		 */
> > +		if (dev->parent && next->parent == dev->parent)
> > +			break;
> > +
> > +		if (next->parent) {
> > +			/* Propagate the status */
> > +			next->power.subtree_no_suspend =
> > +				device_no_suspend_enable(next->parent);
> > +		}
> > +	}
> > +out:
> > +	mutex_unlock(&dpm_list_mtx);
> > +	return;
> > +}
> > +EXPORT_SYMBOL_GPL(device_set_no_suspend_enable);
> > +
> > +/**
> >   *	device_pm_add - add a device to the list of active devices
> >   *	@dev:	Device to be added to the list
> >   */
> > @@ -78,6 +117,11 @@ void device_pm_add(struct device *dev)
> >  		if (dev->parent->power.status >= DPM_SUSPENDING)
> >  			dev_warn(dev, "parent %s should not be sleeping\n",
> >  				 dev_name(dev->parent));
> > +		if (device_no_suspend_enable(dev->parent)) {
> > +			/* if the parent has suspend disable, propagate it
> > +			 * to the new child */
> > +			dev->power.subtree_no_suspend = 1;
> > +		}
> >  	} else if (transition_started) {
> >  		/*
> >  		 * We refuse to register parentless devices while a PM
> > @@ -87,7 +131,15 @@ void device_pm_add(struct device *dev)
> >  		dev_WARN(dev, "Parentless device registered during a PM transaction\n");
> >  	}
> >  
> > -	list_add_tail(&dev->power.entry, &dpm_list);
> > +	if (dev->parent) {
> > +		/*
> > +		 * if the device has a parent insert just before it.
> > +		 */
> > +		list_add_tail(&dev->power.entry, &(dev->parent)->power.entry);
> > +	}
> > +	else
> > +		list_add_tail(&dev->power.entry, &dpm_list);
> > +

Why are you changing the ordering for when we add devices to the list?
This seems like you are adding stuff now in backwards order, why make
this change?

confused,

greg k-h