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

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Friday, 21 of March 2008, David Brownell wrote:
> On Thursday 20 March 2008, Rafael J. Wysocki wrote:
> > Is there any reason why pci_choose_state() should try to figure out what kind
> > of operation is being performed by the driver and tailor its output to that?
> 
> It's always been specified to do that ... but it's always done
> that in a buggy way.  (Which is why USB never used it.)

I hope you realize that your change will affect all drivers calling
pci_choose_state() on ACPI systems on FREEZE/PRETHAW in a hard to predict
way?  Perhaps some of them actually rely on getting D3hot on FREEZE, for
example?

> > I don't think so.  Rather, the driver should know what it's doing and either
> > call pci_choose_state() or not.  If the state of the device is not to be
> > changed, there's no reason to call pci_choose_state() at all.
> 
> 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.

> What's the dividing line between being OK to offload vs not eing OK?  Why not
> let the drivers make that choice?
> 
> 
> > 	[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()?

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html