diff mbox series

[v3,2/2] acpi: Move check for _DSD StorageD3Enable property to acpi

Message ID 20210528160253.9492-1-mario.limonciello@amd.com
State Superseded
Headers show
Series None | expand

Commit Message

Mario Limonciello May 28, 2021, 4:02 p.m. UTC
Although first implemented for NVME, this check may be usable by
other drivers as well.

Suggested-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/device_pm.c | 24 ++++++++++++++++++++++++
 drivers/nvme/host/pci.c  | 28 +---------------------------
 include/linux/acpi.h     |  5 +++++
 3 files changed, 30 insertions(+), 27 deletions(-)

Changes from v2->v3:
 * New patch suggested by Keith on the mailing list.
 * Rebase on variable initialization optimization in first patch
 * Add kernel doc to his patch

Comments

Christoph Hellwig May 31, 2021, 6:53 a.m. UTC | #1
On Fri, May 28, 2021 at 11:02:53AM -0500, Mario Limonciello wrote:
> Although first implemented for NVME, this check may be usable by

> other drivers as well.


I think we want to expand this a little more, mentioning that the
Microsoft spec for it also explicitly mentions SATA/AHCI, that Google
has a local tree where this applied to SDHCI and that even if a slot
is intended for storage a user can plug any card into it.

> +/**

> + * acpi_storage_d3 - Check if a storage device should use D3.

> + * @dev: Device to check

> + *

> + * Look for a _DSD property specifying that the storage device

> + * should use D3 to support deep platform power savings during

> + * suspend-to-idle.

> + *

> + */


Maybe something like:

/**
 * acpi_storage_d3 - check if a device must use D3 for suspend
 * @dev: Device to check
 *
 * Returns %true if @pdev should be put into D3 when the ->suspend method is
 * called, else %false.  The name of this function is somewhat misleading
 * as it has nothing to do with storage except for the name of the ACPI
 * property.  On some platforms resume will not work if this hint is ignored.
 */
diff mbox series

Patch

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index d260bc1f3e6e..b959fa54f478 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1340,4 +1340,28 @@  int acpi_dev_pm_attach(struct device *dev, bool power_on)
 	return 1;
 }
 EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
+
+/**
+ * acpi_storage_d3 - Check if a storage device should use D3.
+ * @dev: Device to check
+ *
+ * Look for a _DSD property specifying that the storage device
+ * should use D3 to support deep platform power savings during
+ * suspend-to-idle.
+ *
+ */
+bool acpi_storage_d3(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	u8 val;
+
+	if (!adev)
+		return false;
+	if (fwnode_property_read_u8(acpi_fwnode_handle(adev), "StorageD3Enable",
+			&val))
+		return false;
+	return val == 1;
+}
+EXPORT_SYMBOL_GPL(acpi_storage_d3);
+
 #endif /* CONFIG_PM */
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 3aa7245a505f..8fbc4c87a0d8 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2828,32 +2828,6 @@  static unsigned long check_vendor_combination_bug(struct pci_dev *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_ACPI
-static bool nvme_acpi_storage_d3(struct pci_dev *dev)
-{
-	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
-	u8 val;
-
-	/*
-	 * Look for _DSD property specifying that the storage device on the port
-	 * must use D3 to support deep platform power savings during
-	 * suspend-to-idle.
-	 */
-
-	if (!adev)
-		return false;
-	if (fwnode_property_read_u8(acpi_fwnode_handle(adev), "StorageD3Enable",
-			&val))
-		return false;
-	return val == 1;
-}
-#else
-static inline bool nvme_acpi_storage_d3(struct pci_dev *dev)
-{
-	return false;
-}
-#endif /* CONFIG_ACPI */
-
 static void nvme_async_probe(void *data, async_cookie_t cookie)
 {
 	struct nvme_dev *dev = data;
@@ -2903,7 +2877,7 @@  static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	quirks |= check_vendor_combination_bug(pdev);
 
-	if (!noacpi && nvme_acpi_storage_d3(pdev)) {
+	if (!noacpi && acpi_storage_d3(&pdev->dev)) {
 		/*
 		 * Some systems use a bios work around to ask for D3 on
 		 * platforms that support kernel managed suspend.
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c60745f657e9..dd0dafd21e33 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1004,6 +1004,7 @@  int acpi_dev_resume(struct device *dev);
 int acpi_subsys_runtime_suspend(struct device *dev);
 int acpi_subsys_runtime_resume(struct device *dev);
 int acpi_dev_pm_attach(struct device *dev, bool power_on);
+bool acpi_storage_d3(struct device *dev);
 #else
 static inline int acpi_subsys_runtime_suspend(struct device *dev) { return 0; }
 static inline int acpi_subsys_runtime_resume(struct device *dev) { return 0; }
@@ -1011,6 +1012,10 @@  static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
 {
 	return 0;
 }
+static inline bool acpi_storage_d3(struct device *dev)
+{
+	return false;
+}
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_SLEEP)