From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [linux-pm] [PATCH 2/2] PCI PM: Introduce pci_preferred_state Date: Fri, 9 May 2008 19:13:39 +0200 Message-ID: <200805091913.40197.rjw@sisk.pl> References: <200805091744.04749.rjw@sisk.pl> <200805090947.08620.jbarnes@virtuousgeek.org> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Return-path: Received: from ogre.sisk.pl ([217.79.144.158]:51110 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756127AbYEIROR (ORCPT ); Fri, 9 May 2008 13:14:17 -0400 In-Reply-To: <200805090947.08620.jbarnes@virtuousgeek.org> Content-Disposition: inline Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Jesse Barnes Cc: Alan Stern , Pavel Machek , ACPI Devel Maling List , pm list On Friday, 9 of May 2008, Jesse Barnes wrote: > > > > > I thought about that too. I'd like to know what the other people > > > > > think, though. > > > > > > > > The point of this isn't at all clear. > > > > > > > > Is this routine meant to be called during a hibernation > > > > transition? > > > > > > Yes, it is. > > > > > > > Or is it just for suspend? > > > > > > > > And why would the return value ever be anything other than D3_hot? (Or > > > > why would the driver ever want to put a device in a different state?) > > > > > > In principle, the driver may want to put the device into a state having > > > shorter wake up latency than D3_hot. > > > > > > > AFAICS, the only reason would be because platform_pci_choose_state() > > > > suggested something else. In which case there's no need for the > > > > "policy" argument. > > > > > > There is a need in two cases: > > > - if platform_pci_choose_state() is not defined (it only is defined for > > > ACPI systems at the moment), > > > - if platform_pci_choose_state() returns PCI_POWER_ERROR meaning that it > > > cannot handle the device. > > > > > > I agree with Pavel that the driver could pass a "fallback state" as a > > > second argument to be used in case the platform cannot provide it with > > > one. > > > > Modified patch follows. > > So why not make platform_pci_choose_state do: > + pci_power_t noacpi_pci_choose_state(struct pci_dev *dev, pci_message_t > state) > + { > + if (!pci_find_capability(dev, PCI_CAP_ID_PM)) > + return state; > + } > > instead? Then in the PCI core we would assign either > platform_pci_choose_state to acpi_pci_choose_state or noacpi_pci_choose_state Good idea. > (though that's a bad name). Does generic_pci_choose_state() sound better? > But really, since drivers should probably know what power state to put their > devices in for suspend & hibernate, maybe on non-ACPI systems the function > should just return an error and the driver can choose... That's one possibility too, but in that case many drivers will do state = pci_preferred_state(dev); if (state == PCI_POWER_ERROR) state = something; It's just shorter to write state = pci_preferred_state(dev, something); Thanks, Rafael