[PATCH v5 00/11] PM / Domains: Generic OF-based support

Changes in v5:
	- Converted dev_pm_domain_detach() to a void function
	- Added a ->detach() callback to the PM domain struct, invoked from the
	  dev_pm_domain_detach().
	- Make ACPI and genpd both assign the ->detach() callback at successfull
	  attachment.
	- The above changes made it possible to make acpi_pm_domain_detach() to
	  be static, added a separate patch for that.

Changes in v4:
	- Rebased patch "PM / Domains: Add generic OF-based PM domain look-up" -
	  and updated the author and the commit message.
	- Adopted review comments for "PM / Domains: Add APIs to attach/detach
	  a PM domain for a device".
	- Updated author and commit message for "ARM: exynos: Move to generic
	  PM domain DT bindings".
	- Added some acks and reviewed by tags.
	- Started to use the "--in-reply-to" option to git-send-email. It should
	  provide the option to show a better diffstat per patch.

Changes in v3:
	- Aligned on terminology, now using "PM domain" in comments and commit
	- messages/headers.
	- Improved English and grammar in comments and commit messages/headers.
	- Adopted proposal from Geert, to have compile-time-check wrapper
	  functions for the API that adds xlate_simple and xlate_onecell
	  providers.
 	- Renamed "domain_num" to "num_domains", in genpd_onecell_data struct.
	- Handle non-contiguous arrays for onecell PM domain providers.
	- Rebased the Exynos patch to follow the new genpd API changes.


Changes in v2:
	- Fix the ACPI patch, it didn't even compile for CONFIG_ACPI.
	- Updated some comments in code and in commit messages.
	- Fixed the dev_pm_domain_attach API to handle EPROBE_DEFER properly.
	- Rebased the ARM Exynos patch.
	- Added some Tested-by tags.


This patchset has a bit of a history and some parts of it has been posted
earlier.

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/262725.html

In the first revision I intentially didn't increase version number of the
patches, since I think it would have cause more confusion than clarity.

A summary of changes in V1 and since the last patchset, from the link above:
	- Instead of letting driver core handling the device to power domain
	binding/unbinding, follow the behavior of how the ACPI power domain
	is handled.


This is a summary of what these patches are intended to do:

1)
Add generic power domain OF-based support which also includes APIs to handle
attach/detach of generic power domains to devices.

2)
Adding a common API to attach/detach power domains and include support for the
ACPI and the generic power domain in there.

3)

From subsystem level code, at probe/remove, convert from invoking the ACPI

specific power domain attach/detach functions to the new common attach/detach
APIs.

4)
Add support for the AMBA bus to attach/detach power domains, using the new
common APIs.

5)
Convert Exynos to use the new generic power domain OF support.

Obviously, there are dependencies througout this patchset, which means if they
get accepted the all need to go together. It might also be convenient to share
them through an immutable branch.


Tomasz Figa (2):
  PM / Domains: Add generic OF-based PM domain look-up
  ARM: exynos: Move to generic PM domain DT bindings

Ulf Hansson (9):
  PM / Domains: Add a detach callback to the struct dev_pm_domain
  ACPI / PM: Assign the ->detach() callback when attaching the PM domain
  PM / Domains: Add APIs to attach/detach a PM domain for a device
  drivercore / platform: Convert to dev_pm_domain_attach|detach()
  i2c: core: Convert to dev_pm_domain_attach|detach()
  mmc: sdio: Convert to dev_pm_domain_attach|detach()
  spi: core: Convert to dev_pm_domain_attach|detach()
  amba: Add support for attach/detach of PM domains
  ACPI / PM: Convert acpi_dev_pm_detach() into a static function

 .../bindings/arm/exynos/power_domain.txt           |  13 +-
 .../devicetree/bindings/power/power_domain.txt     |  49 ++++
 arch/arm/mach-exynos/pm_domains.c                  |  78 +-----
 drivers/acpi/device_pm.c                           |  71 ++---
 drivers/amba/bus.c                                 |  10 +-
 drivers/base/platform.c                            |  15 +-
 drivers/base/power/common.c                        |  52 ++++
 drivers/base/power/domain.c                        | 289 +++++++++++++++++++++
 drivers/i2c/i2c-core.c                             |  13 +-
 drivers/mmc/core/sdio_bus.c                        |   4 +-
 drivers/spi/spi.c                                  |  12 +-
 include/linux/acpi.h                               |   2 -
 include/linux/pm.h                                 |  12 +
 include/linux/pm_domain.h                          |  52 ++++
 kernel/power/Kconfig                               |   4 +
 15 files changed, 535 insertions(+), 141 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/power_domain.txt

-- 
1.9.1

The intent of this callback is to simplify detachment of devices from
their PM domains. Further patches will show the benefit.

Signed-off-by: Ulf Hansson <redacted>
---
 include/linux/pm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 72c0fe0..ef4e16f 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h

@@ -619,6 +619,7 @@ extern int dev_pm_put_subsys_data(struct device *dev);
  */
 struct dev_pm_domain {
 	struct dev_pm_ops	ops;
+	void (*detach)(struct device *, bool);
 };
 
 /*

-- 
1.9.1

As as preparation to simplify the detachment of devices from their PM
domains, we assign the ->detach() callback to genpd_dev_pm_detach().

Signed-off-by: Ulf Hansson <redacted>
---
 drivers/acpi/device_pm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 67075f8..eec5ed5 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c

@@ -1072,6 +1072,8 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)
 		acpi_dev_pm_full_power(adev);
 		acpi_device_wakeup(adev, ACPI_STATE_S0, false);
 	}
+
+	dev->pm_domain->detach = acpi_dev_pm_detach;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);

-- 
1.9.1

From: Tomasz Figa <redacted>

This patch introduces generic code to perform PM domain look-up using
device tree and automatically bind devices to their PM domains.

Generic device tree bindings are introduced to specify PM domains of
devices in their device tree nodes.

Backwards compatibility with legacy Samsung-specific PM domain bindings
is provided, but for now the new code is not compiled when
CONFIG_ARCH_EXYNOS is selected to avoid collision with legacy code.
This will change as soon as the Exynos PM domain code gets converted to
use the generic framework in further patch.

This patch was originally submitted by Tomasz Figa when he was employed
by Samsung.
http://marc.info/?l=linux-pm&m=139955349702152&w=2

Signed-off-by: Ulf Hansson <redacted>
Acked-by: Rob Herring <robh@kernel.org>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Kevin Hilman <redacted>
---
 .../devicetree/bindings/power/power_domain.txt     |  49 ++++
 drivers/base/power/domain.c                        | 289 +++++++++++++++++++++
 include/linux/pm_domain.h                          |  52 ++++
 kernel/power/Kconfig                               |   4 +
 4 files changed, 394 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/power_domain.txt

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
new file mode 100644
index 0000000..98c1667
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/power_domain.txt

@@ -0,0 +1,49 @@
+* Generic PM domains
+
+System on chip designs are often divided into multiple PM domains that can be
+used for power gating of selected IP blocks for power saving by reduced leakage
+current.
+
+This device tree binding can be used to bind PM domain consumer devices with
+their PM domains provided by PM domain providers. A PM domain provider can be
+represented by any node in the device tree and can provide one or more PM
+domains. A consumer node can refer to the provider by a phandle and a set of
+phandle arguments (so called PM domain specifiers) of length specified by the
+#power-domain-cells property in the PM domain provider node.
+
+==PM domain providers==
+
+Required properties:
+ - #power-domain-cells : Number of cells in a PM domain specifier;
+   Typically 0 for nodes representing a single PM domain and 1 for nodes
+   providing multiple PM domains (e.g. power controllers), but can be any value
+   as specified by device tree binding documentation of particular provider.
+
+Example:
+
+	power: power-controller@12340000 {
+		compatible = "foo,power-controller";
+		reg = <0x12340000 0x1000>;
+		#power-domain-cells = <1>;
+	};
+
+The node above defines a power controller that is a PM domain provider and
+expects one cell as its phandle argument.
+
+==PM domain consumers==
+
+Required properties:
+ - power-domains : A phandle and PM domain specifier as defined by bindings of
+                   the power controller specified by phandle.
+
+Example:
+
+	leaky-device@12350000 {
+		compatible = "foo,i-leak-current";
+		reg = <0x12350000 0x1000>;
+		power-domains = <&power 0>;
+	};
+
+The node above defines a typical PM domain consumer device, which is located
+inside a PM domain with index 0 of a power controller represented by a node
+with the label "power".

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index cf4651a..cd93e75 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c

@@ -8,6 +8,7 @@
 
 #include <linux/kernel.h>
 #include <linux/io.h>
+#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_qos.h>

@@ -1933,3 +1934,291 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);
 }
+
+#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
+/*
+ * Device Tree based PM domain providers.
+ *
+ * The code below implements generic device tree based PM domain providers that
+ * bind device tree nodes with generic PM domains registered in the system.
+ *
+ * Any driver that registers generic PM domains and needs to support binding of
+ * devices to these domains is supposed to register a PM domain provider, which
+ * maps a PM domain specifier retrieved from the device tree to a PM domain.
+ *
+ * Two simple mapping functions have been provided for convenience:
+ *  - __of_genpd_xlate_simple() for 1:1 device tree node to PM domain mapping.
+ *  - __of_genpd_xlate_onecell() for mapping of multiple PM domains per node by
+ *    index.
+ */
+
+/**
+ * struct of_genpd_provider - PM domain provider registration structure
+ * @link: Entry in global list of PM domain providers
+ * @node: Pointer to device tree node of PM domain provider
+ * @xlate: Provider-specific xlate callback mapping a set of specifier cells
+ *         into a PM domain.
+ * @data: context pointer to be passed into @xlate callback
+ */
+struct of_genpd_provider {
+	struct list_head link;
+	struct device_node *node;
+	genpd_xlate_t xlate;
+	void *data;
+};
+
+/* List of registered PM domain providers. */
+static LIST_HEAD(of_genpd_providers);
+/* Mutex to protect the list above. */
+static DEFINE_MUTEX(of_genpd_mutex);
+
+/**
+ * __of_genpd_xlate_simple() - Xlate function for direct node-domain mapping
+ * @genpdspec: OF phandle args to map into a PM domain
+ * @data: xlate function private data - pointer to struct generic_pm_domain
+ *
+ * This is a generic xlate function that can be used to model PM domains that
+ * have their own device tree nodes. The private data of xlate function needs
+ * to be a valid pointer to struct generic_pm_domain.
+ */
+struct generic_pm_domain *__of_genpd_xlate_simple(
+					struct of_phandle_args *genpdspec,
+					void *data)
+{
+	if (genpdspec->args_count != 0)
+		return ERR_PTR(-EINVAL);
+	return data;
+}
+EXPORT_SYMBOL_GPL(__of_genpd_xlate_simple);
+
+/**
+ * __of_genpd_xlate_onecell() - Xlate function using a single index.
+ * @genpdspec: OF phandle args to map into a PM domain
+ * @data: xlate function private data - pointer to struct genpd_onecell_data
+ *
+ * This is a generic xlate function that can be used to model simple PM domain
+ * controllers that have one device tree node and provide multiple PM domains.
+ * A single cell is used as an index into an array of PM domains specified in
+ * the genpd_onecell_data struct when registering the provider.
+ */
+struct generic_pm_domain *__of_genpd_xlate_onecell(
+					struct of_phandle_args *genpdspec,
+					void *data)
+{
+	struct genpd_onecell_data *genpd_data = data;
+	unsigned int idx = genpdspec->args[0];
+
+	if (genpdspec->args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	if (idx >= genpd_data->num_domains) {
+		pr_err("%s: invalid domain index %u\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!genpd_data->domains[idx])
+		return ERR_PTR(-ENOENT);
+
+	return genpd_data->domains[idx];
+}
+EXPORT_SYMBOL_GPL(__of_genpd_xlate_onecell);
+
+/**
+ * __of_genpd_add_provider() - Register a PM domain provider for a node
+ * @np: Device node pointer associated with the PM domain provider.
+ * @xlate: Callback for decoding PM domain from phandle arguments.
+ * @data: Context pointer for @xlate callback.
+ */
+int __of_genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
+			void *data)
+{
+	struct of_genpd_provider *cp;
+
+	cp = kzalloc(sizeof(*cp), GFP_KERNEL);
+	if (!cp)
+		return -ENOMEM;
+
+	cp->node = of_node_get(np);
+	cp->data = data;
+	cp->xlate = xlate;
+
+	mutex_lock(&of_genpd_mutex);
+	list_add(&cp->link, &of_genpd_providers);
+	mutex_unlock(&of_genpd_mutex);
+	pr_debug("Added domain provider from %s\n", np->full_name);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__of_genpd_add_provider);
+
+/**
+ * of_genpd_del_provider() - Remove a previously registered PM domain provider
+ * @np: Device node pointer associated with the PM domain provider
+ */
+void of_genpd_del_provider(struct device_node *np)
+{
+	struct of_genpd_provider *cp;
+
+	mutex_lock(&of_genpd_mutex);
+	list_for_each_entry(cp, &of_genpd_providers, link) {
+		if (cp->node == np) {
+			list_del(&cp->link);
+			of_node_put(cp->node);
+			kfree(cp);
+			break;
+		}
+	}
+	mutex_unlock(&of_genpd_mutex);
+}
+EXPORT_SYMBOL_GPL(of_genpd_del_provider);
+
+/**
+ * of_genpd_get_from_provider() - Look-up PM domain
+ * @genpdspec: OF phandle args to use for look-up
+ *
+ * Looks for a PM domain provider under the node specified by @genpdspec and if
+ * found, uses xlate function of the provider to map phandle args to a PM
+ * domain.
+ *
+ * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
+ * on failure.
+ */
+static struct generic_pm_domain *of_genpd_get_from_provider(
+					struct of_phandle_args *genpdspec)
+{
+	struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
+	struct of_genpd_provider *provider;
+
+	mutex_lock(&of_genpd_mutex);
+
+	/* Check if we have such a provider in our array */
+	list_for_each_entry(provider, &of_genpd_providers, link) {
+		if (provider->node == genpdspec->np)
+			genpd = provider->xlate(genpdspec, provider->data);
+		if (!IS_ERR(genpd))
+			break;
+	}
+
+	mutex_unlock(&of_genpd_mutex);
+
+	return genpd;
+}
+
+/**
+ * genpd_dev_pm_detach - Detach a device from its PM domain.
+ * @dev: Device to attach.
+ * @power_off: Currently not used
+ *
+ * Try to locate a corresponding generic PM domain, which the device was
+ * attached to previously. If such is found, the device is detached from it.
+ */
+static void genpd_dev_pm_detach(struct device *dev, bool power_off)
+{
+	struct generic_pm_domain *pd = NULL, *gpd;
+	int ret = 0;
+
+	if (!dev->pm_domain)
+		return;
+
+	mutex_lock(&gpd_list_lock);
+	list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
+		if (&gpd->domain == dev->pm_domain) {
+			pd = gpd;
+			break;
+		}
+	}
+	mutex_unlock(&gpd_list_lock);
+
+	if (!pd)
+		return;
+
+	dev_dbg(dev, "removing from PM domain %s\n", pd->name);
+
+	while (1) {
+		ret = pm_genpd_remove_device(pd, dev);
+		if (ret != -EAGAIN)
+			break;
+		cond_resched();
+	}
+
+	if (ret < 0) {
+		dev_err(dev, "failed to remove from PM domain %s: %d",
+			pd->name, ret);
+		return;
+	}
+
+	/* Check if PM domain can be powered off after removing this device. */
+	genpd_queue_power_off_work(pd);
+}
+
+/**
+ * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
+ * @dev: Device to attach.
+ *
+ * Parse device's OF node to find a PM domain specifier. If such is found,
+ * attaches the device to retrieved pm_domain ops.
+ *
+ * Both generic and legacy Samsung-specific DT bindings are supported to keep
+ * backwards compatibility with existing DTBs.
+ *
+ * Returns 0 on successfully attached PM domain or negative error code.
+ */
+int genpd_dev_pm_attach(struct device *dev)
+{
+	struct of_phandle_args pd_args;
+	struct generic_pm_domain *pd;
+	int ret;
+
+	if (!dev->of_node)
+		return -ENODEV;
+
+	if (dev->pm_domain)
+		return -EEXIST;
+
+	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
+					"#power-domain-cells", 0, &pd_args);
+	if (ret < 0) {
+		if (ret != -ENOENT)
+			return ret;
+
+		/*
+		 * Try legacy Samsung-specific bindings
+		 * (for backwards compatibility of DT ABI)
+		 */
+		pd_args.args_count = 0;
+		pd_args.np = of_parse_phandle(dev->of_node,
+						"samsung,power-domain", 0);
+		if (!pd_args.np)
+			return -ENOENT;
+	}
+
+	pd = of_genpd_get_from_provider(&pd_args);
+	if (IS_ERR(pd)) {
+		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
+			__func__, PTR_ERR(pd));
+		of_node_put(dev->of_node);
+		return PTR_ERR(pd);
+	}
+
+	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
+
+	while (1) {
+		ret = pm_genpd_add_device(pd, dev);
+		if (ret != -EAGAIN)
+			break;
+		cond_resched();
+	}
+
+	if (ret < 0) {
+		dev_err(dev, "failed to add to PM domain %s: %d",
+			pd->name, ret);
+		of_node_put(dev->of_node);
+		return ret;
+	}
+
+	dev->pm_domain->detach = genpd_dev_pm_detach;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
+#endif

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index aa03586..292079d 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h

@@ -264,4 +264,56 @@ static inline void pm_genpd_syscore_poweroff(struct device *dev) {}
 static inline void pm_genpd_syscore_poweron(struct device *dev) {}
 #endif
 
+/* OF PM domain providers */
+struct of_device_id;
+
+struct genpd_onecell_data {
+	struct generic_pm_domain **domains;
+	unsigned int num_domains;
+};
+
+typedef struct generic_pm_domain *(*genpd_xlate_t)(struct of_phandle_args *args,
+						void *data);
+
+#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
+int __of_genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
+			void *data);
+void of_genpd_del_provider(struct device_node *np);
+
+struct generic_pm_domain *__of_genpd_xlate_simple(
+					struct of_phandle_args *genpdspec,
+					void *data);
+struct generic_pm_domain *__of_genpd_xlate_onecell(
+					struct of_phandle_args *genpdspec,
+					void *data);
+
+int genpd_dev_pm_attach(struct device *dev);
+#else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
+static inline int __of_genpd_add_provider(struct device_node *np,
+					genpd_xlate_t xlate, void *data)
+{
+	return 0;
+}
+static inline void of_genpd_del_provider(struct device_node *np) {}
+
+#define __of_genpd_xlate_simple		NULL
+#define __of_genpd_xlate_onecell	NULL
+
+static inline int genpd_dev_pm_attach(struct device *dev)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
+
+static inline int of_genpd_add_provider_simple(struct device_node *np,
+					struct generic_pm_domain *genpd)
+{
+	return __of_genpd_add_provider(np, __of_genpd_xlate_simple, genpd);
+}
+static inline int of_genpd_add_provider_onecell(struct device_node *np,
+					struct genpd_onecell_data *data)
+{
+	return __of_genpd_add_provider(np, __of_genpd_xlate_onecell, data);
+}
+
 #endif /* _LINUX_PM_DOMAIN_H */

diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index e4e4121..897619b 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig

@@ -302,6 +302,10 @@ config PM_GENERIC_DOMAINS_RUNTIME
 	def_bool y
 	depends on PM_RUNTIME && PM_GENERIC_DOMAINS
 
+config PM_GENERIC_DOMAINS_OF
+	def_bool y
+	depends on PM_GENERIC_DOMAINS && OF && !ARCH_EXYNOS
+
 config CPU_PM
 	bool
 	depends on SUSPEND || CPU_IDLE

-- 
1.9.1

To maintain scalability let's add common methods to attach and detach
a PM domain for a device, dev_pm_domain_attach|detach().

Typically dev_pm_domain_attach() shall be invoked from subsystem level
code at the probe phase to try to attach a device to its PM domain.
The reversed actions may be done a the remove phase and then by
invoking dev_pm_domain_detach().

When attachment succeeds, the attach function should assign its
corresponding detach function to a new ->detach() callback added in the
struct dev_pm_domain.

Signed-off-by: Ulf Hansson <redacted>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Kevin Hilman <redacted>
---
 drivers/base/power/common.c | 52 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h          | 11 ++++++++++
 2 files changed, 63 insertions(+)

diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index df2e5ee..a0a415d 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c

@@ -11,6 +11,8 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/pm_clock.h>
+#include <linux/acpi.h>
+#include <linux/pm_domain.h>
 
 /**
  * dev_pm_get_subsys_data - Create or refcount power.subsys_data for device.

@@ -82,3 +84,53 @@ int dev_pm_put_subsys_data(struct device *dev)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dev_pm_put_subsys_data);
+
+/**
+ * dev_pm_domain_attach - Attach a device to its PM domain.
+ * @dev: Device to attach.
+ * @power_on: Used to indicate whether we should power on the device.
+ *
+ * The @dev may only be attached to a single PM domain. By iterating through
+ * the available alternatives we try to find a valid PM domain for the device.
+ * As attachement succeeds, the ->detach() callback in the struct dev_pm_domain
+ * should be assigned by the corresponding attach function.
+ *
+ * This function should typically be invoked from subsystem level code during
+ * the probe phase. Especially for those that holds devices which requires
+ * power management through PM domains.
+ *
+ * Callers must ensure proper synchronization of this function with power
+ * management callbacks.
+ *
+ * Returns 0 on successfully attached PM domain or negative error code.
+ */
+int dev_pm_domain_attach(struct device *dev, bool power_on)
+{
+	int ret;
+
+	ret = acpi_dev_pm_attach(dev, power_on);
+	if (ret)
+		ret = genpd_dev_pm_attach(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
+
+/**
+ * dev_pm_domain_detach - Detach a device from its PM domain.
+ * @dev: Device to attach.
+ * @power_off: Used to indicate whether we should power off the device.
+ *
+ * This functions will reverse the actions from dev_pm_domain_attach() and thus
+ * try to detach the @dev from its PM domain. Typically it should be invoked
+ * from subsystem level code during the remove phase.
+ *
+ * Callers must ensure proper synchronization of this function with power
+ * management callbacks.
+ */
+void dev_pm_domain_detach(struct device *dev, bool power_off)
+{
+	if (dev->pm_domain && dev->pm_domain->detach)
+		dev->pm_domain->detach(dev, power_off);
+}
+EXPORT_SYMBOL_GPL(dev_pm_domain_detach);

diff --git a/include/linux/pm.h b/include/linux/pm.h
index ef4e16f..ff6400d 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h

@@ -622,6 +622,17 @@ struct dev_pm_domain {
 	void (*detach)(struct device *, bool);
 };
 
+#ifdef CONFIG_PM
+extern int dev_pm_domain_attach(struct device *dev, bool power_on);
+extern void dev_pm_domain_detach(struct device *dev, bool power_off);
+#else
+static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
+{
+	return -ENODEV;
+}
+static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
+#endif
+
 /*
  * The PM_EVENT_ messages are also used by drivers implementing the legacy
  * suspend framework, based on the ->suspend() and ->resume() callbacks common

-- 
1.9.1

Previously only the ACPI PM domain was supported by the platform bus.

Let's convert to the common attach/detach functions for PM domains,
which currently means we are extending the support to include the
generic PM domain as well.

Signed-off-by: Ulf Hansson <redacted>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Kevin Hilman <redacted>
---
 drivers/base/platform.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ab4f4ce..904be3d 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c

@@ -506,11 +506,12 @@ static int platform_drv_probe(struct device *_dev)
 	if (ret < 0)
 		return ret;
 
-	acpi_dev_pm_attach(_dev, true);
-
-	ret = drv->probe(dev);
-	if (ret)
-		acpi_dev_pm_detach(_dev, true);
+	ret = dev_pm_domain_attach(_dev, true);
+	if (ret != -EPROBE_DEFER) {
+		ret = drv->probe(dev);
+		if (ret)
+			dev_pm_domain_detach(_dev, true);
+	}
 
 	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
 		dev_warn(_dev, "probe deferral not supported\n");

@@ -532,7 +533,7 @@ static int platform_drv_remove(struct device *_dev)
 	int ret;
 
 	ret = drv->remove(dev);
-	acpi_dev_pm_detach(_dev, true);
+	dev_pm_domain_detach(_dev, true);
 
 	return ret;
 }

@@ -543,7 +544,7 @@ static void platform_drv_shutdown(struct device *_dev)
 	struct platform_device *dev = to_platform_device(_dev);
 
 	drv->shutdown(dev);
-	acpi_dev_pm_detach(_dev, true);
+	dev_pm_domain_detach(_dev, true);
 }
 
 /**

-- 
1.9.1

Previously only the ACPI PM domain was supported by the spi bus.

Let's convert to the common attach/detach functions for PM domains,
which currently means we are extending the support to include the
generic PM domain as well.

Cc: linux-spi@vger.kernel.org
Signed-off-by: Ulf Hansson <redacted>
Reviewed-by: Kevin Hilman <redacted>
---
 drivers/spi/spi.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ca935df..72a0beb 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c

@@ -264,10 +264,12 @@ static int spi_drv_probe(struct device *dev)
 	if (ret)
 		return ret;
 
-	acpi_dev_pm_attach(dev, true);
-	ret = sdrv->probe(to_spi_device(dev));
-	if (ret)
-		acpi_dev_pm_detach(dev, true);
+	ret = dev_pm_domain_attach(dev, true);
+	if (ret != -EPROBE_DEFER) {
+		ret = sdrv->probe(to_spi_device(dev));
+		if (ret)
+			dev_pm_domain_detach(dev, true);
+	}
 
 	return ret;
 }

@@ -278,7 +280,7 @@ static int spi_drv_remove(struct device *dev)
 	int ret;
 
 	ret = sdrv->remove(to_spi_device(dev));
-	acpi_dev_pm_detach(dev, true);
+	dev_pm_domain_detach(dev, true);
 
 	return ret;
 }

-- 
1.9.1

AMBA devices may on some SoCs resides in PM domains. To be able to
manage these devices from there, let's try to attach devices to their
corresponding PM domain during the probe phase.

To reverse these actions at the remove phase, we try to detach the
device from its PM domain.

Signed-off-by: Ulf Hansson <redacted>
Reviewed-by: Kevin Hilman <redacted>
---
 drivers/amba/bus.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 3cf61a1..8f52393 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c

@@ -182,9 +182,15 @@ static int amba_probe(struct device *dev)
 	int ret;
 
 	do {
+		ret = dev_pm_domain_attach(dev, true);
+		if (ret == -EPROBE_DEFER)
+			break;
+
 		ret = amba_get_enable_pclk(pcdev);
-		if (ret)
+		if (ret) {
+			dev_pm_domain_detach(dev, true);
 			break;
+		}
 
 		pm_runtime_get_noresume(dev);
 		pm_runtime_set_active(dev);

@@ -199,6 +205,7 @@ static int amba_probe(struct device *dev)
 		pm_runtime_put_noidle(dev);
 
 		amba_put_disable_pclk(pcdev);
+		dev_pm_domain_detach(dev, true);
 	} while (0);
 
 	return ret;

@@ -220,6 +227,7 @@ static int amba_remove(struct device *dev)
 	pm_runtime_put_noidle(dev);
 
 	amba_put_disable_pclk(pcdev);
+	dev_pm_domain_detach(dev, true);
 
 	return ret;
 }

-- 
1.9.1

The ->detach() callback for the PM domain has now been fully adopted,
thus there no users left of the acpi_dev_pm_detach() API. This allow us
to convert it into a static function.

Signed-off-by: Ulf Hansson <redacted>
---
 drivers/acpi/device_pm.c | 69 ++++++++++++++++++++++++------------------------
 include/linux/acpi.h     |  2 --
 2 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index eec5ed5..bea6896 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c

@@ -1041,6 +1041,40 @@ static struct dev_pm_domain acpi_general_pm_domain = {
 };
 
 /**
+ * acpi_dev_pm_detach - Remove ACPI power management from the device.
+ * @dev: Device to take care of.
+ * @power_off: Whether or not to try to remove power from the device.
+ *
+ * Remove the device from the general ACPI PM domain and remove its wakeup
+ * notifier.  If @power_off is set, additionally remove power from the device if
+ * possible.
+ *
+ * Callers must ensure proper synchronization of this function with power
+ * management callbacks.
+ */
+static void acpi_dev_pm_detach(struct device *dev, bool power_off)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+
+	if (adev && dev->pm_domain == &acpi_general_pm_domain) {
+		dev->pm_domain = NULL;
+		acpi_remove_pm_notifier(adev);
+		if (power_off) {
+			/*
+			 * If the device's PM QoS resume latency limit or flags
+			 * have been exposed to user space, they have to be
+			 * hidden at this point, so that they don't affect the
+			 * choice of the low-power state to put the device into.
+			 */
+			dev_pm_qos_hide_latency_limit(dev);
+			dev_pm_qos_hide_flags(dev);
+			acpi_device_wakeup(adev, ACPI_STATE_S0, false);
+			acpi_dev_pm_low_power(dev, adev, ACPI_STATE_S0);
+		}
+	}
+}
+
+/**
  * acpi_dev_pm_attach - Prepare device for ACPI power management.
  * @dev: Device to prepare.
  * @power_on: Whether or not to power on the device.

@@ -1077,39 +1111,4 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
-
-/**
- * acpi_dev_pm_detach - Remove ACPI power management from the device.
- * @dev: Device to take care of.
- * @power_off: Whether or not to try to remove power from the device.
- *
- * Remove the device from the general ACPI PM domain and remove its wakeup
- * notifier.  If @power_off is set, additionally remove power from the device if
- * possible.
- *
- * Callers must ensure proper synchronization of this function with power
- * management callbacks.
- */
-void acpi_dev_pm_detach(struct device *dev, bool power_off)
-{
-	struct acpi_device *adev = ACPI_COMPANION(dev);
-
-	if (adev && dev->pm_domain == &acpi_general_pm_domain) {
-		dev->pm_domain = NULL;
-		acpi_remove_pm_notifier(adev);
-		if (power_off) {
-			/*
-			 * If the device's PM QoS resume latency limit or flags
-			 * have been exposed to user space, they have to be
-			 * hidden at this point, so that they don't affect the
-			 * choice of the low-power state to put the device into.
-			 */
-			dev_pm_qos_hide_latency_limit(dev);
-			dev_pm_qos_hide_flags(dev);
-			acpi_device_wakeup(adev, ACPI_STATE_S0, false);
-			acpi_dev_pm_low_power(dev, adev, ACPI_STATE_S0);
-		}
-	}
-}
-EXPORT_SYMBOL_GPL(acpi_dev_pm_detach);
 #endif /* CONFIG_PM */

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 807cbc4..b7926bb 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h

@@ -587,7 +587,6 @@ static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM)
 struct acpi_device *acpi_dev_pm_get_node(struct device *dev);
 int acpi_dev_pm_attach(struct device *dev, bool power_on);
-void acpi_dev_pm_detach(struct device *dev, bool power_off);
 #else
 static inline struct acpi_device *acpi_dev_pm_get_node(struct device *dev)
 {

@@ -597,7 +596,6 @@ static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
 {
 	return -ENODEV;
 }
-static inline void acpi_dev_pm_detach(struct device *dev, bool power_off) {}
 #endif
 
 #ifdef CONFIG_ACPI

-- 
1.9.1

Hi Ulf,

On Fri, Sep 19, 2014 at 08:27:33PM +0200, Ulf Hansson wrote:

Changes in v5:
	- Converted dev_pm_domain_detach() to a void function
	- Added a ->detach() callback to the PM domain struct, invoked from the
	  dev_pm_domain_detach().
	- Make ACPI and genpd both assign the ->detach() callback at successfull
	  attachment.
	- The above changes made it possible to make acpi_pm_domain_detach() to
	  be static, added a separate patch for that.

Thank you for making these changes. For the series:

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

-- 
Dmitry

On Fri, Sep 19, 2014 at 8:27 PM, Ulf Hansson [off-list ref] wrote:

attachment

+ * should be assigned by the corresponding attach function.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

On Fri, Sep 19, 2014 at 8:27 PM, Ulf Hansson [off-list ref] wrote:

I think it would help to add the parameter names, especially for
the "bool" parameter:

void (*detach)(struct device *dev, bool power_off);

 };

 /*

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

On Friday, September 19, 2014 11:48:49 AM Dmitry Torokhov wrote:

Hi Ulf,

On Fri, Sep 19, 2014 at 08:27:33PM +0200, Ulf Hansson wrote:

quoted

Changes in v5:
	- Converted dev_pm_domain_detach() to a void function
	- Added a ->detach() callback to the PM domain struct, invoked from the
	  dev_pm_domain_detach().
	- Make ACPI and genpd both assign the ->detach() callback at successfull
	  attachment.
	- The above changes made it possible to make acpi_pm_domain_detach() to
	  be static, added a separate patch for that.

Thank you for making these changes. For the series:

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I've queued up this patchset for 3.18 (I fixed up the two minor issues pointed
to by Geert in the process).

Thanks everyone!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

On 22 September 2014 16:19, Rafael J. Wysocki [off-list ref] wrote:

On Friday, September 19, 2014 11:48:49 AM Dmitry Torokhov wrote:

quoted

Hi Ulf,

On Fri, Sep 19, 2014 at 08:27:33PM +0200, Ulf Hansson wrote:

quoted

Changes in v5:
    - Converted dev_pm_domain_detach() to a void function
    - Added a ->detach() callback to the PM domain struct, invoked from the
      dev_pm_domain_detach().
    - Make ACPI and genpd both assign the ->detach() callback at successfull
      attachment.
    - The above changes made it possible to make acpi_pm_domain_detach() to
      be static, added a separate patch for that.

Thank you for making these changes. For the series:

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I've queued up this patchset for 3.18 (I fixed up the two minor issues pointed
to by Geert in the process).

Thanks Rafael!

Feel free to apply Wolfram's comment on patch 6 as well.

Kind regards
Uffe

On Mon, Sep 22, 2014 at 04:19:54PM +0200, Rafael J. Wysocki wrote:

I've queued up this patchset for 3.18 (I fixed up the two minor issues pointed
to by Geert in the process).

Oh, dear.  I'd been hoping to be able to test the series on my s3c4xx
system but I don't get home until tomorrow :/

Acked-by: Mark Brown <broonie@kernel.org>

Hi Rafael,

On 09/22/2014 05:19 PM, Rafael J. Wysocki wrote:

On Friday, September 19, 2014 11:48:49 AM Dmitry Torokhov wrote:

quoted

Hi Ulf,

On Fri, Sep 19, 2014 at 08:27:33PM +0200, Ulf Hansson wrote:

quoted

Changes in v5:
	- Converted dev_pm_domain_detach() to a void function
	- Added a ->detach() callback to the PM domain struct, invoked from the
	  dev_pm_domain_detach().
	- Make ACPI and genpd both assign the ->detach() callback at successfull
	  attachment.
	- The above changes made it possible to make acpi_pm_domain_detach() to
	  be static, added a separate patch for that.

Thank you for making these changes. For the series:

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I've queued up this patchset for 3.18 (I fixed up the two minor issues pointed
to by Geert in the process).

Could you point me on branch where I can find these patches?
Also, are there any chances to have these patches  in next/linux-next.git,
so they will become available for testing and re-using?

Best regards,
-grygorii

On Wednesday, September 24, 2014 03:44:07 PM Grygorii Strashko wrote:

Hi Rafael,

On 09/22/2014 05:19 PM, Rafael J. Wysocki wrote:

quoted

On Friday, September 19, 2014 11:48:49 AM Dmitry Torokhov wrote:

quoted

Hi Ulf,

On Fri, Sep 19, 2014 at 08:27:33PM +0200, Ulf Hansson wrote:

quoted

Changes in v5:
	- Converted dev_pm_domain_detach() to a void function
	- Added a ->detach() callback to the PM domain struct, invoked from the
	  dev_pm_domain_detach().
	- Make ACPI and genpd both assign the ->detach() callback at successfull
	  attachment.
	- The above changes made it possible to make acpi_pm_domain_detach() to
	  be static, added a separate patch for that.

Thank you for making these changes. For the series:

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I've queued up this patchset for 3.18 (I fixed up the two minor issues pointed
to by Geert in the process).

Could you point me on branch where I can find these patches?
Also, are there any chances to have these patches  in next/linux-next.git,
so they will become available for testing and re-using?

It is in the bleeding-edge branch of the linux-pm.git tree at the moment,
which is rebased quite often, but you can use it for testing.

It should appear in linux-next tomorrow.

Rafael

On 09/24/2014 04:51 PM, Rafael J. Wysocki wrote:

On Wednesday, September 24, 2014 03:44:07 PM Grygorii Strashko wrote:

quoted

Hi Rafael,

On 09/22/2014 05:19 PM, Rafael J. Wysocki wrote:

quoted

On Friday, September 19, 2014 11:48:49 AM Dmitry Torokhov wrote:

quoted

Hi Ulf,

On Fri, Sep 19, 2014 at 08:27:33PM +0200, Ulf Hansson wrote:

quoted

Changes in v5:
	- Converted dev_pm_domain_detach() to a void function
	- Added a ->detach() callback to the PM domain struct, invoked from the
	  dev_pm_domain_detach().
	- Make ACPI and genpd both assign the ->detach() callback at successfull
	  attachment.
	- The above changes made it possible to make acpi_pm_domain_detach() to
	  be static, added a separate patch for that.

Thank you for making these changes. For the series:

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I've queued up this patchset for 3.18 (I fixed up the two minor issues pointed
to by Geert in the process).

Could you point me on branch where I can find these patches?
Also, are there any chances to have these patches  in next/linux-next.git,
so they will become available for testing and re-using?

It is in the bleeding-edge branch of the linux-pm.git tree at the moment,
which is rebased quite often, but you can use it for testing.

It should appear in linux-next tomorrow.

Thank you.

Regards,
- grygorii

I just noticed these patches because they conflicted with some of the
local patches I had to add a very similar framework. One of the reasons
why I hadn't posted these publicly yet is because the platform where I
want to use this (Tegra) is somewhat quirky when it comes to power
domains.

On Tegra these domains are called power gates and they currently have
their own API. We've been looking at migrating things over to some
generic framework for some time and PM domains do seem like a good fit.
However one of the quirks regarding these domains on Tegra is that a
fixed sequence exists that needs to be respected when enabling or
disabling a power partition. The exact sequence can be found in the
drivers/soc/tegra/pmc.c driver's tegra_powergate_sequence_power_up()
function. Essentially we need to call into the clock and reset drivers
at very specific moments during the operations that the PMC does.

One solution to this would be to make the needed clocks and resets
available to the power domain driver via DT, but then we have the
problem that two drivers would be controlling the same resources. For
example drivers could still want to disable the clock for more fine-
grained power management. Furthermore for some devices it may turn out
that turning the domain off and on introduces too much latency to be
useful.

Does anyone have any better ideas on how to make that work with this
generic PM domain framework? Or is Tegra just too special to be a good
fit?

Thierry

On 25 September 2014 13:21, Thierry Reding [off-list ref] wrote:

I just noticed these patches because they conflicted with some of the
local patches I had to add a very similar framework. One of the reasons
why I hadn't posted these publicly yet is because the platform where I
want to use this (Tegra) is somewhat quirky when it comes to power
domains.

It's great that more things goes on in this area. :-)

On Tegra these domains are called power gates and they currently have
their own API. We've been looking at migrating things over to some
generic framework for some time and PM domains do seem like a good fit.
However one of the quirks regarding these domains on Tegra is that a
fixed sequence exists that needs to be respected when enabling or
disabling a power partition. The exact sequence can be found in the
drivers/soc/tegra/pmc.c driver's tegra_powergate_sequence_power_up()
function. Essentially we need to call into the clock and reset drivers
at very specific moments during the operations that the PMC does.

I am not sure I fully understand how the power gating actually
happens. How is it triggered?

One solution to this would be to make the needed clocks and resets
available to the power domain driver via DT, but then we have the
problem that two drivers would be controlling the same resources. For
example drivers could still want to disable the clock for more fine-
grained power management.

Sorry, but I think I need a better understanding to be able to comment.

But maybe, drivers could implement runtime PM support and define
runtime PM callbacks. From the callbacks those will handle clocks and
resets, is not that enough? What more is needed from a PM domain point
of view?

Furthermore for some devices it may turn out
that turning the domain off and on introduces too much latency to be
useful.

This should be handled by the generic PM domain governor. Through the
per device QOS, you are able to set latencies constraints which could
prevent a PM domain from being gated.

Does anyone have any better ideas on how to make that work with this
generic PM domain framework? Or is Tegra just too special to be a good
fit?

I certainly think it's worth a try, I would be surprised if we
shouldn't be able to address requirements from Tegra.

As you might have figured out, I am dedicated to improve the generic
power domain such it could fit more SOCs than today, thus I am also
hoping for more SOC to start to convert to it.

Kind regards
Uffe

On Thu, Sep 25, 2014 at 05:29:10PM +0200, Ulf Hansson wrote:

On 25 September 2014 13:21, Thierry Reding [off-list ref] wrote:

quoted

I just noticed these patches because they conflicted with some of the
local patches I had to add a very similar framework. One of the reasons
why I hadn't posted these publicly yet is because the platform where I
want to use this (Tegra) is somewhat quirky when it comes to power
domains.

It's great that more things goes on in this area. :-)

quoted

On Tegra these domains are called power gates and they currently have
their own API. We've been looking at migrating things over to some
generic framework for some time and PM domains do seem like a good fit.
However one of the quirks regarding these domains on Tegra is that a
fixed sequence exists that needs to be respected when enabling or
disabling a power partition. The exact sequence can be found in the
drivers/soc/tegra/pmc.c driver's tegra_powergate_sequence_power_up()
function. Essentially we need to call into the clock and reset drivers
at very specific moments during the operations that the PMC does.

I am not sure I fully understand how the power gating actually
happens. How is it triggered?

Drivers explicitly call the custom API. So all drivers that need to turn
on power partitions have a call to tegra_powergate_sequence_power_up()
in .probe() and tegra_powergate_power_off() in .remove().

quoted

One solution to this would be to make the needed clocks and resets
available to the power domain driver via DT, but then we have the
problem that two drivers would be controlling the same resources. For
example drivers could still want to disable the clock for more fine-
grained power management.

Sorry, but I think I need a better understanding to be able to comment.

But maybe, drivers could implement runtime PM support and define
runtime PM callbacks. From the callbacks those will handle clocks and
resets, is not that enough? What more is needed from a PM domain point
of view?

Let me quote the actual power up sequence code:

	int tegra_powergate_sequence_power_up(int id, struct clk *clk,
					      struct reset_control *rst)
	{
		int ret;

		reset_control_assert(rst);

		ret = tegra_powergate_power_on(id);
		if (ret)
			goto err_power;

		ret = clk_prepare_enable(clk);
		if (ret)
			goto err_clk;

		usleep_range(10, 20);

		ret = tegra_powergate_remove_clamping(id);
		if (ret)
			goto err_clamp;

		usleep_range(10, 20);
		reset_control_deassert(rst);

		return 0;

	err_clamp:
		clk_disable_unprepare(clk);
	err_clk:
		tegra_powergate_power_off(id);
	err_power:
		return ret;
	}
	EXPORT_SYMBOL(tegra_powergate_sequence_power_up);

The critical part is that we need to enable the clock after the
partition has been powered, but before the clamps are removed.
Implementing this with runtime PM support in drivers won't work
because the power domain driver has to do both the powering up
and removing the clamps, so there's no place to inject the call
to enable the clock.

quoted

Furthermore for some devices it may turn out
that turning the domain off and on introduces too much latency to be
useful.

This should be handled by the generic PM domain governor. Through the
per device QOS, you are able to set latencies constraints which could
prevent a PM domain from being gated.

Okay, that sounds good.

quoted

Does anyone have any better ideas on how to make that work with this
generic PM domain framework? Or is Tegra just too special to be a good
fit?

I certainly think it's worth a try, I would be surprised if we
shouldn't be able to address requirements from Tegra.

As you might have figured out, I am dedicated to improve the generic
power domain such it could fit more SOCs than today, thus I am also
hoping for more SOC to start to convert to it.

I do have code that seems to work in most cases without following the
above sequence, but there are no guarantees, so I'm reluctant to make
that change.

Thierry

On 09/25, Thierry Reding wrote:

On Thu, Sep 25, 2014 at 05:29:10PM +0200, Ulf Hansson wrote:

quoted

On 25 September 2014 13:21, Thierry Reding [off-list ref] wrote:

quoted

I just noticed these patches because they conflicted with some of the
local patches I had to add a very similar framework. One of the reasons
why I hadn't posted these publicly yet is because the platform where I
want to use this (Tegra) is somewhat quirky when it comes to power
domains.

It's great that more things goes on in this area. :-)

quoted

On Tegra these domains are called power gates and they currently have
their own API. We've been looking at migrating things over to some
generic framework for some time and PM domains do seem like a good fit.
However one of the quirks regarding these domains on Tegra is that a
fixed sequence exists that needs to be respected when enabling or
disabling a power partition. The exact sequence can be found in the
drivers/soc/tegra/pmc.c driver's tegra_powergate_sequence_power_up()
function. Essentially we need to call into the clock and reset drivers
at very specific moments during the operations that the PMC does.

I am not sure I fully understand how the power gating actually
happens. How is it triggered?

Drivers explicitly call the custom API. So all drivers that need to turn
on power partitions have a call to tegra_powergate_sequence_power_up()
in .probe() and tegra_powergate_power_off() in .remove().

quoted

quoted

One solution to this would be to make the needed clocks and resets
available to the power domain driver via DT, but then we have the
problem that two drivers would be controlling the same resources. For
example drivers could still want to disable the clock for more fine-
grained power management.

Sorry, but I think I need a better understanding to be able to comment.

But maybe, drivers could implement runtime PM support and define
runtime PM callbacks. From the callbacks those will handle clocks and
resets, is not that enough? What more is needed from a PM domain point
of view?

Let me quote the actual power up sequence code:

	int tegra_powergate_sequence_power_up(int id, struct clk *clk,
					      struct reset_control *rst)
	{
		int ret;

		reset_control_assert(rst);

		ret = tegra_powergate_power_on(id);
		if (ret)
			goto err_power;

		ret = clk_prepare_enable(clk);
		if (ret)
			goto err_clk;

		usleep_range(10, 20);

		ret = tegra_powergate_remove_clamping(id);
		if (ret)
			goto err_clamp;

		usleep_range(10, 20);
		reset_control_deassert(rst);

		return 0;

	err_clamp:
		clk_disable_unprepare(clk);
	err_clk:
		tegra_powergate_power_off(id);
	err_power:
		return ret;
	}
	EXPORT_SYMBOL(tegra_powergate_sequence_power_up);

The critical part is that we need to enable the clock after the
partition has been powered, but before the clamps are removed.
Implementing this with runtime PM support in drivers won't work
because the power domain driver has to do both the powering up
and removing the clamps, so there's no place to inject the call
to enable the clock.

FWIW, Qualcomm platforms have pretty much the same design. Our
power domain controls live in the same register space as the
clocks and resets. Is Tegra the same way? To power on/off a
domain we need to go and forcefully turn a clock on and assert a
reset or perhaps we need the clock to be off and assert a reset.
It depends on the domain.

Historically we've supported this by requiring all drivers to
disable their clocks and deassert any resets before calling into
this code (we put this behind the regulator API). Then we're free
to do whatever is necessary to power on/off, eventally leaving
the clocks off if they were forced on and finally giving control
to the drivers so they can manage their own clocks and resets. It
would be better for us to take ownership of the clocks and resets
in the domain so we don't have this prerequisite of clocks being
off before calling into the domain code. I plan to do pretty much
Kevin outlined and use runtime PM, power domains, and per-device
pm QoS to figure out if we should gate clocks (clk_disable), or
if we go to a lower power mode where we unprepare clocks
(clk_unprepare), or if we go to the lowest power mode where we
apply clamps, etc. (called power collapse for us). Then the
drivers just interact with runtime PM and QoS and they aren't
aware of all this SoC glue.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

On Thu, Sep 25, 2014 at 05:27:57PM -0700, Stephen Boyd wrote:

On 09/25, Thierry Reding wrote:

quoted

On Thu, Sep 25, 2014 at 05:29:10PM +0200, Ulf Hansson wrote:

quoted

On 25 September 2014 13:21, Thierry Reding [off-list ref] wrote:

quoted

I just noticed these patches because they conflicted with some of the
local patches I had to add a very similar framework. One of the reasons
why I hadn't posted these publicly yet is because the platform where I
want to use this (Tegra) is somewhat quirky when it comes to power
domains.

It's great that more things goes on in this area. :-)

quoted

On Tegra these domains are called power gates and they currently have
their own API. We've been looking at migrating things over to some
generic framework for some time and PM domains do seem like a good fit.
However one of the quirks regarding these domains on Tegra is that a
fixed sequence exists that needs to be respected when enabling or
disabling a power partition. The exact sequence can be found in the
drivers/soc/tegra/pmc.c driver's tegra_powergate_sequence_power_up()
function. Essentially we need to call into the clock and reset drivers
at very specific moments during the operations that the PMC does.

I am not sure I fully understand how the power gating actually
happens. How is it triggered?

Drivers explicitly call the custom API. So all drivers that need to turn
on power partitions have a call to tegra_powergate_sequence_power_up()
in .probe() and tegra_powergate_power_off() in .remove().

quoted

quoted

One solution to this would be to make the needed clocks and resets
available to the power domain driver via DT, but then we have the
problem that two drivers would be controlling the same resources. For
example drivers could still want to disable the clock for more fine-
grained power management.

Sorry, but I think I need a better understanding to be able to comment.

But maybe, drivers could implement runtime PM support and define
runtime PM callbacks. From the callbacks those will handle clocks and
resets, is not that enough? What more is needed from a PM domain point
of view?

Let me quote the actual power up sequence code:

	int tegra_powergate_sequence_power_up(int id, struct clk *clk,
					      struct reset_control *rst)
	{
		int ret;

		reset_control_assert(rst);

		ret = tegra_powergate_power_on(id);
		if (ret)
			goto err_power;

		ret = clk_prepare_enable(clk);
		if (ret)
			goto err_clk;

		usleep_range(10, 20);

		ret = tegra_powergate_remove_clamping(id);
		if (ret)
			goto err_clamp;

		usleep_range(10, 20);
		reset_control_deassert(rst);

		return 0;

	err_clamp:
		clk_disable_unprepare(clk);
	err_clk:
		tegra_powergate_power_off(id);
	err_power:
		return ret;
	}
	EXPORT_SYMBOL(tegra_powergate_sequence_power_up);

The critical part is that we need to enable the clock after the
partition has been powered, but before the clamps are removed.
Implementing this with runtime PM support in drivers won't work
because the power domain driver has to do both the powering up
and removing the clamps, so there's no place to inject the call
to enable the clock.

FWIW, Qualcomm platforms have pretty much the same design. Our
power domain controls live in the same register space as the
clocks and resets. Is Tegra the same way?

Unfortunately the powergates are in a completely different block.

                                          To power on/off a
domain we need to go and forcefully turn a clock on and assert a
reset or perhaps we need the clock to be off and assert a reset.
It depends on the domain.

It seems like we may even need to interact with the memory controller to
make sure there are no outstanding requests before gating a partition,
and similarly interact with the memory controller after ungating to make
sure the memory clients can resume transactions. We currently don't do
that and it seems to work, but most likely only because we only ungate
on driver probe and gate on driver removal.

Historically we've supported this by requiring all drivers to
disable their clocks and deassert any resets before calling into
this code (we put this behind the regulator API). Then we're free
to do whatever is necessary to power on/off, eventally leaving
the clocks off if they were forced on and finally giving control
to the drivers so they can manage their own clocks and resets. It
would be better for us to take ownership of the clocks and resets
in the domain so we don't have this prerequisite of clocks being
off before calling into the domain code. I plan to do pretty much
Kevin outlined and use runtime PM, power domains, and per-device
pm QoS to figure out if we should gate clocks (clk_disable), or
if we go to a lower power mode where we unprepare clocks
(clk_unprepare), or if we go to the lowest power mode where we
apply clamps, etc. (called power collapse for us). Then the
drivers just interact with runtime PM and QoS and they aren't
aware of all this SoC glue.

That sounds like a good plan and very similar to what I had in mind for
Tegra. I have a feeling that the road leading there could be rather
bumpy...

Thierry