diff mbox series

[2/3] ata: ahci_platform: allow disabling of hotplug to save power

Message ID 20180407204738.28308-2-0v3rdr0n3@gmail.com
State Not Applicable
Delegated to: David Miller
Headers show
Series [1/3] ata: ahci: rpm_put port on port_stop to match rpm_get in port_start | expand

Commit Message

Sam Morris April 7, 2018, 8:47 p.m. UTC 
From: Samuel Morris <samorris@lexmark.com>

A number of resources remain powered to support hotplug. On platforms
I've worked with, allowing the ahci_platform to suspend saves about
150mW. This patch allows the device to be auto suspended if the
config parameter is set.

Signed-off-by: Samuel Morris <samorris@lexmark.com>
---
 drivers/ata/Kconfig            | 10 +++++
 drivers/ata/ahci_platform.c    |  9 ++++-
 drivers/ata/libahci_platform.c | 86 ++++++++++++++++++++++++++++++++++--------
 include/linux/ahci_platform.h  |  2 +
 4 files changed, 90 insertions(+), 17 deletions(-)

Comments

kernel test robot April 8, 2018, 9:55 a.m. UTC | #1 
Hi Samuel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tj-libata/for-next]
[also build test ERROR on v4.16 next-20180406]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/0v3rdr0n3-gmail-com/ata-ahci-rpm_put-port-on-port_stop-to-match-rpm_get-in-port_start/20180408-085104
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-next
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

>> drivers/ata/ahci_platform.o:(.rodata+0x6a0): undefined reference to `ahci_platform_runtime_suspend'
>> drivers/ata/ahci_platform.o:(.rodata+0x6a4): undefined reference to `ahci_platform_runtime_resume'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 2b16e7c8fff3..866663dd11ab 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -119,6 +119,16 @@  config SATA_AHCI_PLATFORM
 
 	  If unsure, say N.
 
+config AHCI_HOTPLUG_DISABLED
+	tristate "Platform AHCI SATA hotplug disable"
+	default n
+	help
+	  This option allows the AHCI Platform resources usually kept on
+		to support hotplug to be runtime suspended, effectively disabling
+		hotplug, but saving power.
+
+	  If unsure, say N.
+
 config AHCI_BRCM
 	tristate "Broadcom AHCI SATA support"
 	depends on ARCH_BRCMSTB || BMIPS_GENERIC || ARCH_BCM_NSP
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 99f9a895a459..e58d971df5d1 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -68,8 +68,13 @@  static int ahci_probe(struct platform_device *pdev)
 	return rc;
 }
 
-static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_platform_suspend,
-			 ahci_platform_resume);
+static const struct dev_pm_ops ahci_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(ahci_platform_suspend, ahci_platform_resume)
+#ifdef CONFIG_AHCI_HOTPLUG_DISABLED
+	SET_RUNTIME_PM_OPS(ahci_platform_runtime_suspend,
+			ahci_platform_runtime_resume, NULL)
+#endif
+};
 
 static const struct of_device_id ahci_of_match[] = {
 	{ .compatible = "generic-ahci", },
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 46a762442dc5..17fc4f19a53d 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -268,10 +268,14 @@  static void ahci_platform_put_resources(struct device *dev, void *res)
 	struct ahci_host_priv *hpriv = res;
 	int c;
 
+#ifdef CONFIG_AHCI_HOTPLUG_DISABLED
+	pm_runtime_disable(dev);
+#else
 	if (hpriv->got_runtime_pm) {
 		pm_runtime_put_sync(dev);
 		pm_runtime_disable(dev);
 	}
+#endif
 
 	for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++)
 		clk_put(hpriv->clks[c]);
@@ -493,9 +497,15 @@  struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 		if (rc == -EPROBE_DEFER)
 			goto err_out;
 	}
+
+#ifdef CONFIG_AHCI_HOTPLUG_DISABLED
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+#else
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 	hpriv->got_runtime_pm = true;
+#endif
 
 	devres_remove_group(dev, NULL);
 	return hpriv;
@@ -723,6 +733,21 @@  int ahci_platform_resume_host(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(ahci_platform_resume_host);
 
+static int _ahci_platform_suspend(struct device *dev)
+{
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	int rc;
+
+	rc = ahci_platform_suspend_host(dev);
+	if (rc)
+		return rc;
+
+	ahci_platform_disable_resources(hpriv);
+
+	return 0;
+}
+
 /**
  * ahci_platform_suspend - Suspend an ahci-platform device
  * @dev: the platform device to suspend
@@ -734,20 +759,45 @@  EXPORT_SYMBOL_GPL(ahci_platform_resume_host);
  * 0 on success otherwise a negative error code
  */
 int ahci_platform_suspend(struct device *dev)
+{
+	return _ahci_platform_suspend(dev);
+}
+EXPORT_SYMBOL_GPL(ahci_platform_suspend);
+
+/**
+ * ahci_platform_runtime_suspend - Runtime suspend an ahci-platform device
+ * @dev: the platform device to suspend
+ *
+ * This function suspends the host associated with the device, followed by
+ * disabling all the resources of the device.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_runtime_suspend(struct device *dev)
+{
+	return _ahci_platform_suspend(dev);
+}
+EXPORT_SYMBOL_GPL(ahci_platform_runtime_suspend);
+
+static int _ahci_platform_resume(struct device *dev)
 {
 	struct ata_host *host = dev_get_drvdata(dev);
 	struct ahci_host_priv *hpriv = host->private_data;
 	int rc;
 
-	rc = ahci_platform_suspend_host(dev);
+	rc = ahci_platform_enable_resources(hpriv);
 	if (rc)
 		return rc;
 
-	ahci_platform_disable_resources(hpriv);
+	rc = ahci_platform_resume_host(dev);
+	if (rc) {
+		ahci_platform_disable_resources(hpriv);
+		return rc;
+	}
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(ahci_platform_suspend);
 
 /**
  * ahci_platform_resume - Resume an ahci-platform device
@@ -761,31 +811,37 @@  EXPORT_SYMBOL_GPL(ahci_platform_suspend);
  */
 int ahci_platform_resume(struct device *dev)
 {
-	struct ata_host *host = dev_get_drvdata(dev);
-	struct ahci_host_priv *hpriv = host->private_data;
 	int rc;
 
-	rc = ahci_platform_enable_resources(hpriv);
+	rc = _ahci_platform_resume(dev);
 	if (rc)
 		return rc;
 
-	rc = ahci_platform_resume_host(dev);
-	if (rc)
-		goto disable_resources;
-
 	/* We resumed so update PM runtime state */
 	pm_runtime_disable(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
 	return 0;
-
-disable_resources:
-	ahci_platform_disable_resources(hpriv);
-
-	return rc;
 }
 EXPORT_SYMBOL_GPL(ahci_platform_resume);
+
+/**
+ * ahci_platform_runtime_resume - Runtime resume an ahci-platform device
+ * @dev: the platform device to resume
+ *
+ * This function enables all the resources of the device followed by
+ * resuming the host associated with the device.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_runtime_resume(struct device *dev)
+{
+	return _ahci_platform_resume(dev);
+}
+EXPORT_SYMBOL_GPL(ahci_platform_runtime_resume);
+
 #endif
 
 MODULE_DESCRIPTION("AHCI SATA platform library");
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index 1b0a17b22cd3..6396e6982103 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -42,5 +42,7 @@  int ahci_platform_suspend_host(struct device *dev);
 int ahci_platform_resume_host(struct device *dev);
 int ahci_platform_suspend(struct device *dev);
 int ahci_platform_resume(struct device *dev);
+int ahci_platform_runtime_suspend(struct device *dev);
+int ahci_platform_runtime_resume(struct device *dev);
 
 #endif /* _AHCI_PLATFORM_H */