On Saturday, 10 of May 2008, Rafael J. Wysocki wrote:
> On Saturday, 10 of May 2008, Jesse Barnes wrote:
> > On Friday, May 09, 2008 2:44 pm Rafael J. Wysocki wrote:
> > > Okay, what about this:
> > >
> > > ---
> > > From: Rafael J. Wysocki <rjw@xxxxxxx>
> > >
> > > The new suspend and hibernation callbacks introduced with
> > > 'struct pm_ops' and 'struct pm_ext_ops' do not take a
> > > pm_message_t argument, so the drivers using them will not be able
> > > to use pci_choose_state() in its present form. For this reason,
> > > introduce a new function, pci_choose_and_set_state(), playing the
> > > role of pci_choose_state() combined with pci_set_power_state() and
> > > allowing the driver to put the device into a power state chosen by
> > > the platform.
> >
> > Yeah, that looks pretty good. The name is long but I can't think of a better
> > one offhand. Can you also update Documentation/power/pci.txt with the latest
> > best practices?
>
> I'm going to do that, eventually, but rather in a separate patch, when
> everything is ready for the new framework, while at the moment we still have
> some design work to do.
>
> For example, some drivers may want to call pci_enable_wake() for the
> target state and that must be done before pci_set_power_state() in case the
> target state is D3cold. To allow them to do that, we'll need a variant of
> pci_choose_and_set_state() with a 'wake_enabled' argument.
Below is how I think that may look like. Please tell me what you think.
Thanks,
Rafael
---
From: Rafael J. Wysocki <rjw@xxxxxxx>
The new suspend and hibernation callbacks introduced with
'struct pm_ops' and 'struct pm_ext_ops' do not take a
pm_message_t argument, so the drivers using them will not be able
to use pci_choose_state() in its present form. For this reason,
introduce a new function, raw_pci_change_state(), playing the
role of pci_choose_state() combined with pci_enable_wake() and
pci_set_power_state() and allowing the driver to put the device
into a power state chosen by the platform, optionally configuring
the device as a source of wakeup events.
Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
---
drivers/pci/pci.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/pci.h | 11 +++++++++
2 files changed, 72 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -509,7 +509,67 @@ pci_set_power_state(struct pci_dev *dev,
}
pci_power_t (*platform_pci_choose_state)(struct pci_dev *dev);
-
+
+/**
+ * raw_pci_change_state - Choose the power state of a PCI device and put the
+ * device into that state.
+ * @dev: PCI device to be put into a low power state
+ * @state: State to put the device into by default
+ * @enable_wake: If set, attempt to enable device to generate wakeup events
+ * @fail_no_wake: If set, return error code if the attempt to enable device
+ * to generate wakeup events fails
+ *
+ * Use the platform driver to choose the preferred PCI power state of given
+ * device and put the device into that state. If the target power state of
+ * the device cannot be chosen using the platform driver, the driver-provided
+ * @state is used. If @enable_wake is set, try to enable the device to
+ * generate wakeup events.
+ *
+ * RETURN VALUE:
+ * -EINVAL if trying to enter a lower state than we're already in.
+ * 0 if we're already in the requested state.
+ * -EIO if device does not support PCI PM.
+ * 0 if we can successfully change the power state.
+ *
+ * If both @enable_wake and @fail_no_wake are set, additionally:
+ * -EIO if the device can't ever be a wakeup event source.
+ * -EINVAL if the device can't generate wakeup events from given state.
+ */
+
+int raw_pci_change_state(struct pci_dev *dev, pci_power_t state,
+ bool enable_wake, bool fail_no_wake)
+{
+ if (!pci_find_capability(dev, PCI_CAP_ID_PM))
+ return -EIO;
+
+ if (platform_pci_choose_state) {
+ pci_power_t ret = platform_pci_choose_state(dev);
+
+ switch (ret) {
+ case PCI_POWER_ERROR:
+ case PCI_UNKNOWN:
+ break;
+ case PCI_D1:
+ case PCI_D2:
+ if (pci_no_d1d2(dev))
+ break;
+ default:
+ state = ret;
+ }
+ }
+
+ if (enable_wake) {
+ int error = pci_enable_wake(dev, state, true);
+
+ if (error && fail_no_wake)
+ return error;
+ }
+
+ return pci_set_power_state(dev, state);
+}
+
+EXPORT_SYMBOL(raw_pci_change_state);
+
/**
* pci_choose_state - Choose the power state of a PCI device
* @dev: PCI device to be suspended
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -615,6 +615,17 @@ size_t pci_get_rom_size(void __iomem *ro
int pci_save_state(struct pci_dev *dev);
int pci_restore_state(struct pci_dev *dev);
int pci_set_power_state(struct pci_dev *dev, pci_power_t state);
+int raw_pci_change_state(struct pci_dev *dev, pci_power_t state,
+ bool enable_wake, bool fail_no_wake);
+static inline int pci_change_state(struct pci_dev *dev, pci_power_t state)
+{
+ return raw_pci_change_state(dev, state, false, false);
+}
+static inline int pci_change_state_wake(struct pci_dev *dev,
+ pci_power_t state)
+{
+ return raw_pci_change_state(dev, state, true, false);
+}
pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable);
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
