On Wednesday 20 December 2006 5:29 pm, Matthew Garrett wrote:
> 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,
Make that "strongly suspecting" given what Greg said ... he normally
gets the final say over drivers/core/* things, and you seem alone in
wanting to help those sysfs files extend their withered existence.
> > 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?
It does say that indents must use only tabs, which that clearly doesn't.
I think you'll find that
if (some_very_long_condition
&& probably_not_quite_as_long
&& or_too_long_for_one_line) {
do_this;
and_this;
}
is widely accepted. (The conditions get an extra indent so they don't
look like they're part of the block executing if the test is true.)
>
> > The other is that
> > the original test must still be used if that bus primitve doesn't exist.
>
> I dislike that.
Tough noogies, as they say. In a tradeoff between correctness and your
personal taste (or even mine, sigh!), the normal tradeoff is in favor
of correctness.
> 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.
But you _didn't_ do anything to "make it necessary". Which means that
your patch *WILL* cause bugs whenever a driver uses those calls, and
courtesy of your patch userspace tries to suspend that device ...
> > > + 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.
Userland can stop using this **TODAY** and just "ifdown", so that
argument seems weak. For all your examples, the userland interface
is already available.
- Dave
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
