[linux-pm] [patch 2.6.25-rc6 3/7] pci_choose_state() cleanup and fixes

[Rafael J. Wysocki]

On Saturday, 22 of March 2008, David Brownell wrote:
> On Friday 21 March 2008, Rafael J. Wysocki wrote:
> > > 
> > > You seem to object to letting drivers offload this particular
> > > bit of work to infrastructure.
> > 
> > No, I don't.  I just don't think it's a good idea to change the existing and
> > widely used function for this purpose.  If I needed some specific functionality
> > at the infrastructure level, I'd add a new function for that with a new
> > changelog etc.  Then, made drivers switch to that and remove the old one.
> 
> I see that a lot of drivers have at some point, not long ago,
> been converted to use this routine.  They previously just
> used PCI_D3 in all cases.
> 
> It seems to me that your objection boils down to the concern
> that those drivers may just have pushed their bug out a level,
> rather than actually fixing their bugs.

Or, the authors of the drivers looked at what the code in
pci_choose_state() actually did and used it on that basis.

> Which I can sympathize with ... but that doesn't change the
> fact that any driver in that position *still* has a bug that
> needs to be fixed.  And if that bug is highlighted by this
> patch ... well, there's still a driver bug to be fixed.

Well, IMHO, the safe way of doing thing would be to audit and possibly fix the
drivers first.  Otherwise, we may get regression reports and we'll have to
revert the patch anyway if we're unable to fix them in a timely manner.

> > > >     [Note that with the new suspend/hibernation callbacks there
> > > > won't be the pm_message_t argument to pass to pci_choose_state().]
> > >
> > > The pm_message_t will necessarily linger until all drivers have
> > > been converted and re-tested.  Which can't be an overnight thing.
> >
> > No, it can't.  Still, suppose a driver is using the new callbacks.  How is
> > it supposed to use pci_choose_state()?
> 
> Hey, you're the one providing those callbacks.  How were you
> going to answer that question *before* I posted this overdue
> bugfix patch for pci_choose_state()?

Then it was simple, because pci_choose_state() returned D0 only for PMSG_ON.

Also, I was going to provide a simplified version of it that wouldn't take
arguments and would return D3hot in case when platform_pci_choose_state()
was not defined.

Thanks,
Rafael