Re: [PATCH 1/2] Fix /sys/device/.../power/state

[Matthew Garrett <mjg59@srcf.ucam.org>]

On Wed, Dec 20, 2006 at 01:18:06PM -0800, David Brownell wrote:
> >  	/* disallow incomplete suspend sequences */
> > -	if (dev->bus && (dev->bus->suspend_late || dev->bus->resume_early))
> > +	if (dev->bus && dev->bus->pm_has_noirq_stage 
> > +	    && dev->bus->pm_has_noirq_stage(dev))
> >  		return error;
> >  
> 
> I'm suspecting these two patches won't be merged, but this fragment has
> two bugs.  One is the whitespace bug already mentioned.

I'm a bit curious about the whitespace issue - CodingStyle doesn't seem 
to discuss what to do with if statements that end up longer than 80 
characters, which is (I think) what you're talking about?

> The other is that
> the original test must still be used if that bus primitve doesn't exist.

I dislike that. We're asking to suspend an individual device - whether 
the bus supports devices that need to suspend with interrupts disabled 
is irrelevent, it's the device that we care about. We should just make 
it necessary for every bus to support this method until the interface is 
removed.

> And in a different vein, I'm a bit surprised that the update to the
> feature-removal-schedule.txt file is a separate patch, but:

It seemed like a logically distinct change, but I'm happy to merge them.

> > +       bus->pm_has_noirq_stage()
> > -When:  July 2007
> > +When:  Once alternative functionality has been implemented
> 
> The "When" shouldn't change.

We shouldn't remove interfaces that userland uses until there's been a 
replacement for long enough that userland can switch over. Setting a 
date for removing this interface when most drivers don't implement the 
replacement isn't reasonable.

-- 
Matthew Garrett | mjg59@srcf.ucam.org