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

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

[Ulf Hansson]

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

[PATCH v5 01/11] PM / Domains: Add a detach callback to the struct dev_pm_domain

[Ulf Hansson]

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 <ulf.hansson@linaro.org>
---
 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

Re: [PATCH v5 01/11] PM / Domains: Add a detach callback to the struct dev_pm_domain

[Geert Uytterhoeven]

On Fri, Sep 19, 2014 at 8:27 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> --- 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);

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

[PATCH v5 02/11] ACPI / PM: Assign the ->detach() callback when attaching the PM domain

[Ulf Hansson]

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 <ulf.hansson@linaro.org>
---
 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

[PATCH v5 03/11] PM / Domains: Add generic OF-based PM domain look-up

[Ulf Hansson]

From: Tomasz Figa <tomasz.figa@gmail.com>

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 <ulf.hansson@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Kevin Hilman <khilman@linaro.org>
---
 .../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

[PATCH v5 04/11] PM / Domains: Add APIs to attach/detach a PM domain for a device

[Ulf Hansson]

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 <ulf.hansson@linaro.org>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Kevin Hilman <khilman@linaro.org>
---
 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

Re: [PATCH v5 04/11] PM / Domains: Add APIs to attach/detach a PM domain for a device

[Geert Uytterhoeven]

On Fri, Sep 19, 2014 at 8:27 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> --- a/drivers/base/power/common.c
> +++ b/drivers/base/power/common.c

> @@ -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

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

[PATCH v5 05/11] drivercore / platform: Convert to dev_pm_domain_attach|detach()

[Ulf Hansson]

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 <ulf.hansson@linaro.org>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Kevin Hilman <khilman@linaro.org>
---
 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

[PATCH v5 08/11] spi: core: Convert to dev_pm_domain_attach|detach()

[Ulf Hansson]

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 <ulf.hansson@linaro.org>
Reviewed-by: Kevin Hilman <khilman@linaro.org>
---
 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

[PATCH v5 09/11] amba: Add support for attach/detach of PM domains

[Ulf Hansson]

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 <ulf.hansson@linaro.org>
Reviewed-by: Kevin Hilman <khilman@linaro.org>
---
 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

[PATCH v5 06/11] i2c: core: Convert to dev_pm_domain_attach|detach()

[Ulf Hansson]

Previously only the ACPI PM domain was supported by the i2c 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-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 632057a..3cd8f11 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -279,10 +279,13 @@ static int i2c_device_probe(struct device *dev)
 	if (status < 0)
 		return status;
 
-	acpi_dev_pm_attach(&client->dev, true);
-	status = driver->probe(client, i2c_match_id(driver->id_table, client));
-	if (status)
-		acpi_dev_pm_detach(&client->dev, true);
+	status = dev_pm_domain_attach(&client->dev, true);
+	if (status != -EPROBE_DEFER) {
+		status = driver->probe(client, i2c_match_id(driver->id_table,
+					client));
+		if (status)
+			dev_pm_domain_detach(&client->dev, true);
+	}
 
 	return status;
 }
@@ -302,7 +305,7 @@ static int i2c_device_remove(struct device *dev)
 		status = driver->remove(client);
 	}
 
-	acpi_dev_pm_detach(&client->dev, true);
+	dev_pm_domain_detach(&client->dev, true);
 	return status;
 }
 
-- 
1.9.1

Re: [PATCH v5 06/11] i2c: core: Convert to dev_pm_domain_attach|detach()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]

On Fri, Sep 19, 2014 at 08:27:39PM +0200, Ulf Hansson wrote:
> Previously only the ACPI PM domain was supported by the i2c 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-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Reviewed-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Looks good to me, but I'd like to give Mika a chance to look at it,
since he does ACPI with I2C. Adding to CC.

> ---
>  drivers/i2c/i2c-core.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 632057a..3cd8f11 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -279,10 +279,13 @@ static int i2c_device_probe(struct device *dev)
>  	if (status < 0)
>  		return status;
>  
> -	acpi_dev_pm_attach(&client->dev, true);
> -	status = driver->probe(client, i2c_match_id(driver->id_table, client));
> -	if (status)
> -		acpi_dev_pm_detach(&client->dev, true);
> +	status = dev_pm_domain_attach(&client->dev, true);
> +	if (status != -EPROBE_DEFER) {
> +		status = driver->probe(client, i2c_match_id(driver->id_table,
> +					client));

Very minor: I think it is more readable to keep this in one line.

> +		if (status)
> +			dev_pm_domain_detach(&client->dev, true);
> +	}
>  
>  	return status;
>  }
> @@ -302,7 +305,7 @@ static int i2c_device_remove(struct device *dev)
>  		status = driver->remove(client);
>  	}
>  
> -	acpi_dev_pm_detach(&client->dev, true);
> +	dev_pm_domain_detach(&client->dev, true);
>  	return status;
>  }
>  
> -- 
> 1.9.1
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v5 06/11] i2c: core: Convert to dev_pm_domain_attach|detach()

[Rafael J. Wysocki]

On Saturday, September 20, 2014 02:23:19 PM Wolfram Sang wrote:
> 
> --bAmEntskrkuBymla
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
> 
> On Fri, Sep 19, 2014 at 08:27:39PM +0200, Ulf Hansson wrote:
> > Previously only the ACPI PM domain was supported by the i2c bus.
> >=20
> > 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.
> >=20
> > Cc: linux-i2c@vger.kernel.org
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Reviewed-by: Kevin Hilman <khilman@linaro.org>
> 
> Looks good to me, but I'd like to give Mika a chance to look at it,
> since he does ACPI with I2C. Adding to CC.

It is not applicable without the [4/9] in this series anyway.

> > ---
> >  drivers/i2c/i2c-core.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >=20
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 632057a..3cd8f11 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -279,10 +279,13 @@ static int i2c_device_probe(struct device *dev)
> >  	if (status < 0)
> >  		return status;
> > =20
> > -	acpi_dev_pm_attach(&client->dev, true);
> > -	status =3D driver->probe(client, i2c_match_id(driver->id_table, client)=
> );
> > -	if (status)
> > -		acpi_dev_pm_detach(&client->dev, true);
> > +	status =3D dev_pm_domain_attach(&client->dev, true);
> > +	if (status !=3D -EPROBE_DEFER) {
> > +		status =3D driver->probe(client, i2c_match_id(driver->id_table,
> > +					client));
> 
> Very minor: I think it is more readable to keep this in one line.

Agreed.

> > +		if (status)
> > +			dev_pm_domain_detach(&client->dev, true);
> > +	}
> > =20
> >  	return status;
> >  }
> > @@ -302,7 +305,7 @@ static int i2c_device_remove(struct device *dev)
> >  		status =3D driver->remove(client);
> >  	}
> > =20
> > -	acpi_dev_pm_detach(&client->dev, true);
> > +	dev_pm_domain_detach(&client->dev, true);
> >  	return status;
> >  }
> > =20
> > --=20
> > 1.9.1
> >=20
> 
> --bAmEntskrkuBymla
> Content-Type: application/pgp-signature; name="signature.asc"
> Content-Description: Digital signature
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
> 
> iQIcBAEBAgAGBQJUHXG2AAoJEBQN5MwUoCm2E7EP/1ykOCTRgrozlYwdbXxO9X8Z
> b6ad23VambrSiQJTZcYheydiVW9qrXIsn8TQf1flWChXtkD06odQRy8tGN8Vw3MQ
> MTdhyB19x1Z0TMkdmcrbhohsRchjsaeMgcpCNklW8ul8nASuDjY9KUdhVfUaY07q
> O1zyZxrA5ykPCq/xP2gakHTTOrB6pj0zwOlDe4yQOd1hJcWrhX0/EBxY4DgvvQIr
> o05w2rdr+s6qCCgV3WSY9EJ0JBOqNhvvGpGnpZe6tms8e1RmrEnSil3piBC1nhHP
> LPeZr/ZHdoRbEgUPNnr/otLhufVeE5uMLu7FFhmZi3DTVXXVb+kMjE17C6XYc3mM
> DIumY2ubMBInCIZDjFZa0BvwuodKqkWrG5cmgimDw8VywhDfmCXm7Q1immbKZ0rw
> p9R4YHkBl5ii0KCyTSrf5M4+ksV80iplb4jmIwfmU0kKnOIan1fgkmyHXR/WN67M
> d6sZUpMsuCUNTRj7Axe6LdKuuQe/EbO9GC66umjxtckJIesM/BSHHgGAGqnxQolQ
> q/NnP2JBTVz6sSjoBRb+EMw5cri2Yb2M1Ak24nw+uoveiqBGGMuFFQGoHyV9UXVP
> zA0MaUKKESiwO1RIaqhYe5lRwIwFSUktD5s/yYQqKbSF0zGAeRn79O+U2ei7xI9a
> gv2ABkxPmYxBPoSf9y1B
> =izqF
> -----END PGP SIGNATURE-----
> 
> --bAmEntskrkuBymla--
> 

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

Re: [PATCH v5 06/11] i2c: core: Convert to dev_pm_domain_attach|detach()

[Mika Westerberg]

On Sat, Sep 20, 2014 at 02:23:19PM +0200, Wolfram Sang wrote:
> On Fri, Sep 19, 2014 at 08:27:39PM +0200, Ulf Hansson wrote:
> > Previously only the ACPI PM domain was supported by the i2c 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-i2c@vger.kernel.org
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Reviewed-by: Kevin Hilman <khilman@linaro.org>
> 
> Looks good to me, but I'd like to give Mika a chance to look at it,
> since he does ACPI with I2C. Adding to CC.

Looks fine to me.

Re: [PATCH v5 06/11] i2c: core: Convert to dev_pm_domain_attach|detach()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Mon, Sep 22, 2014 at 12:52:53PM +0300, Mika Westerberg wrote:
> On Sat, Sep 20, 2014 at 02:23:19PM +0200, Wolfram Sang wrote:
> > On Fri, Sep 19, 2014 at 08:27:39PM +0200, Ulf Hansson wrote:
> > > Previously only the ACPI PM domain was supported by the i2c 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-i2c@vger.kernel.org
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > Reviewed-by: Kevin Hilman <khilman@linaro.org>
> > 
> > Looks good to me, but I'd like to give Mika a chance to look at it,
> > since he does ACPI with I2C. Adding to CC.
> 
> Looks fine to me.

Thanks! So:

Acked-by: Wolfram Sang <wsa@the-dreams.de>


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v5 07/11] mmc: sdio: Convert to dev_pm_domain_attach|detach()

[Ulf Hansson]

Previously only the ACPI PM domain was supported by the sdio 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-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mmc/core/sdio_bus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 4fa8fef9..1df0fc6 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -315,7 +315,7 @@ int sdio_add_func(struct sdio_func *func)
 	ret = device_add(&func->dev);
 	if (ret == 0) {
 		sdio_func_set_present(func);
-		acpi_dev_pm_attach(&func->dev, false);
+		dev_pm_domain_attach(&func->dev, false);
 	}
 
 	return ret;
@@ -332,7 +332,7 @@ void sdio_remove_func(struct sdio_func *func)
 	if (!sdio_func_present(func))
 		return;
 
-	acpi_dev_pm_detach(&func->dev, false);
+	dev_pm_domain_detach(&func->dev, false);
 	device_del(&func->dev);
 	put_device(&func->dev);
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 07/11] mmc: sdio: Convert to dev_pm_domain_attach|detach()

[Dmitry Torokhov]

Hi Ulf,

On Fri, Sep 19, 2014 at 08:27:40PM +0200, Ulf Hansson wrote:
> Previously only the ACPI PM domain was supported by the sdio 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-mmc@vger.kernel.org
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Reviewed-by: Kevin Hilman <khilman@linaro.org>
> ---
>  drivers/mmc/core/sdio_bus.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> index 4fa8fef9..1df0fc6 100644
> --- a/drivers/mmc/core/sdio_bus.c
> +++ b/drivers/mmc/core/sdio_bus.c
> @@ -315,7 +315,7 @@ int sdio_add_func(struct sdio_func *func)
>  	ret = device_add(&func->dev);
>  	if (ret == 0) {
>  		sdio_func_set_present(func);
> -		acpi_dev_pm_attach(&func->dev, false);
> +		dev_pm_domain_attach(&func->dev, false);

Admittedly it is not brought in by your change, but I am a bit worried
about this code. In all other busses we attach power domain to a device
before probing it (and detach if probe fails). Why here we only attach
it to power domain after adding the device to the bus? If driver for the
function has already been loaded the probe() would run without device
being attached to a power domain...

Adding Aaron as he's the author of the original change.

Thanks.

-- 
Dmitry

Re: [PATCH v5 07/11] mmc: sdio: Convert to dev_pm_domain_attach|detach()

[Aaron Lu]

On 10/02/2014 08:27 AM, Dmitry Torokhov wrote:
> Hi Ulf,
> 
> On Fri, Sep 19, 2014 at 08:27:40PM +0200, Ulf Hansson wrote:
>> Previously only the ACPI PM domain was supported by the sdio 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-mmc@vger.kernel.org
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Reviewed-by: Kevin Hilman <khilman@linaro.org>
>> ---
>>  drivers/mmc/core/sdio_bus.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
>> index 4fa8fef9..1df0fc6 100644
>> --- a/drivers/mmc/core/sdio_bus.c
>> +++ b/drivers/mmc/core/sdio_bus.c
>> @@ -315,7 +315,7 @@ int sdio_add_func(struct sdio_func *func)
>>  	ret = device_add(&func->dev);
>>  	if (ret == 0) {
>>  		sdio_func_set_present(func);
>> -		acpi_dev_pm_attach(&func->dev, false);
>> +		dev_pm_domain_attach(&func->dev, false);
> 
> Admittedly it is not brought in by your change, but I am a bit worried
> about this code. In all other busses we attach power domain to a device
> before probing it (and detach if probe fails). Why here we only attach
> it to power domain after adding the device to the bus? If driver for the
> function has already been loaded the probe() would run without device
> being attached to a power domain...

Sorry for replying late...

I see your concern, but it should be safe here. The dev_pm_domain_attach
does primarily two things(for ACPI based system): assign the pm_domian
field so that later system and runtime PM transitions we have proper
callbacks for this device; power on the device if needed.

I was using Broadcom BCM43241 SDIO wifi card when developing the code,
it doesn't need the pm_domain set in its probe function and since the
SDIO wifi card is powered on after system boot, no power on is needed
either. That's why I added the attach function after device_add to save
a little code(detach if probe fails).

Feel free to change the sequence if the current one is a bad one, I'm OK
with either case.

> 
> Adding Aaron as he's the author of the original change.

Thanks,
Aaron

Re: [PATCH v5 07/11] mmc: sdio: Convert to dev_pm_domain_attach|detach()

[Ulf Hansson]

On 13 October 2014 04:48, Aaron Lu <aaron.lu@intel.com> wrote:
> On 10/02/2014 08:27 AM, Dmitry Torokhov wrote:
>> Hi Ulf,
>>
>> On Fri, Sep 19, 2014 at 08:27:40PM +0200, Ulf Hansson wrote:
>>> Previously only the ACPI PM domain was supported by the sdio 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-mmc@vger.kernel.org
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> Reviewed-by: Kevin Hilman <khilman@linaro.org>
>>> ---
>>>  drivers/mmc/core/sdio_bus.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
>>> index 4fa8fef9..1df0fc6 100644
>>> --- a/drivers/mmc/core/sdio_bus.c
>>> +++ b/drivers/mmc/core/sdio_bus.c
>>> @@ -315,7 +315,7 @@ int sdio_add_func(struct sdio_func *func)
>>>      ret = device_add(&func->dev);
>>>      if (ret == 0) {
>>>              sdio_func_set_present(func);
>>> -            acpi_dev_pm_attach(&func->dev, false);
>>> +            dev_pm_domain_attach(&func->dev, false);
>>
>> Admittedly it is not brought in by your change, but I am a bit worried
>> about this code. In all other busses we attach power domain to a device
>> before probing it (and detach if probe fails). Why here we only attach
>> it to power domain after adding the device to the bus? If driver for the
>> function has already been loaded the probe() would run without device
>> being attached to a power domain...
>
> Sorry for replying late...
>
> I see your concern, but it should be safe here. The dev_pm_domain_attach
> does primarily two things(for ACPI based system): assign the pm_domian
> field so that later system and runtime PM transitions we have proper
> callbacks for this device; power on the device if needed.
>
> I was using Broadcom BCM43241 SDIO wifi card when developing the code,
> it doesn't need the pm_domain set in its probe function and since the
> SDIO wifi card is powered on after system boot, no power on is needed
> either. That's why I added the attach function after device_add to save
> a little code(detach if probe fails).
>
> Feel free to change the sequence if the current one is a bad one, I'm OK
> with either case.

Thanks for clarifying this. I plan to change the behaviour according
to how other buses handles it.

Though, the patch will be a part another patchset that handles a
related initialization problem for the generic PM domain. I will keep
you guys on cc when I post it.

Kind regards
Uffe

[PATCH v5 10/11] ARM: exynos: Move to generic PM domain DT bindings

[Ulf Hansson]

From: Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch moves Exynos PM domain code to use the new generic PM domain
look-up framework introduced in previous patches, thus also allowing
the new code to be compiled with CONFIG_ARCH_EXYNOS.

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

Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 .../bindings/arm/exynos/power_domain.txt           | 13 ++--
 arch/arm/mach-exynos/pm_domains.c                  | 78 +---------------------
 kernel/power/Kconfig                               |  2 +-
 3 files changed, 8 insertions(+), 85 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index 8b4f7b7f..abde1ea 100644
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
@@ -8,6 +8,8 @@ Required Properties:
     * samsung,exynos4210-pd - for exynos4210 type power domain.
 - reg: physical base address of the controller and length of memory mapped
     region.
+- #power-domain-cells: number of cells in power domain specifier;
+    must be 0.
 
 Optional Properties:
 - clocks: List of clock handles. The parent clocks of the input clocks to the
@@ -29,6 +31,7 @@ Example:
 	lcd0: power-domain-lcd0 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023C00 0x10>;
+		#power-domain-cells = <0>;
 	};
 
 	mfc_pd: power-domain@10044060 {
@@ -37,12 +40,8 @@ Example:
 		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_SW_ACLK333>,
 			<&clock CLK_MOUT_USER_ACLK333>;
 		clock-names = "oscclk", "pclk0", "clk0";
+		#power-domain-cells = <0>;
 	};
 
-Example of the node using power domain:
-
-	node {
-		/* ... */
-		samsung,power-domain = <&lcd0>;
-		/* ... */
-	};
+See Documentation/devicetree/bindings/power/power_domain.txt for description
+of consumer-side bindings.
diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
index fd76e1b..20f2671 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -105,78 +105,6 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
 	return exynos_pd_power(domain, false);
 }
 
-static void exynos_add_device_to_domain(struct exynos_pm_domain *pd,
-					 struct device *dev)
-{
-	int ret;
-
-	dev_dbg(dev, "adding to power domain %s\n", pd->pd.name);
-
-	while (1) {
-		ret = pm_genpd_add_device(&pd->pd, dev);
-		if (ret != -EAGAIN)
-			break;
-		cond_resched();
-	}
-
-	pm_genpd_dev_need_restore(dev, true);
-}
-
-static void exynos_remove_device_from_domain(struct device *dev)
-{
-	struct generic_pm_domain *genpd = dev_to_genpd(dev);
-	int ret;
-
-	dev_dbg(dev, "removing from power domain %s\n", genpd->name);
-
-	while (1) {
-		ret = pm_genpd_remove_device(genpd, dev);
-		if (ret != -EAGAIN)
-			break;
-		cond_resched();
-	}
-}
-
-static void exynos_read_domain_from_dt(struct device *dev)
-{
-	struct platform_device *pd_pdev;
-	struct exynos_pm_domain *pd;
-	struct device_node *node;
-
-	node = of_parse_phandle(dev->of_node, "samsung,power-domain", 0);
-	if (!node)
-		return;
-	pd_pdev = of_find_device_by_node(node);
-	if (!pd_pdev)
-		return;
-	pd = platform_get_drvdata(pd_pdev);
-	exynos_add_device_to_domain(pd, dev);
-}
-
-static int exynos_pm_notifier_call(struct notifier_block *nb,
-				    unsigned long event, void *data)
-{
-	struct device *dev = data;
-
-	switch (event) {
-	case BUS_NOTIFY_BIND_DRIVER:
-		if (dev->of_node)
-			exynos_read_domain_from_dt(dev);
-
-		break;
-
-	case BUS_NOTIFY_UNBOUND_DRIVER:
-		exynos_remove_device_from_domain(dev);
-
-		break;
-	}
-	return NOTIFY_DONE;
-}
-
-static struct notifier_block platform_nb = {
-	.notifier_call = exynos_pm_notifier_call,
-};
-
 static __init int exynos4_pm_init_power_domain(void)
 {
 	struct platform_device *pdev;
@@ -202,7 +130,6 @@ static __init int exynos4_pm_init_power_domain(void)
 		pd->base = of_iomap(np, 0);
 		pd->pd.power_off = exynos_pd_power_off;
 		pd->pd.power_on = exynos_pd_power_on;
-		pd->pd.of_node = np;
 
 		pd->oscclk = clk_get(dev, "oscclk");
 		if (IS_ERR(pd->oscclk))
@@ -228,15 +155,12 @@ static __init int exynos4_pm_init_power_domain(void)
 			clk_put(pd->oscclk);
 
 no_clk:
-		platform_set_drvdata(pdev, pd);
-
 		on = __raw_readl(pd->base + 0x4) & INT_LOCAL_PWR_EN;
 
 		pm_genpd_init(&pd->pd, NULL, !on);
+		of_genpd_add_provider_simple(np, &pd->pd);
 	}
 
-	bus_register_notifier(&platform_bus_type, &platform_nb);
-
 	return 0;
 }
 arch_initcall(exynos4_pm_init_power_domain);
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 897619b..bbef57f 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -304,7 +304,7 @@ config PM_GENERIC_DOMAINS_RUNTIME
 
 config PM_GENERIC_DOMAINS_OF
 	def_bool y
-	depends on PM_GENERIC_DOMAINS && OF && !ARCH_EXYNOS
+	depends on PM_GENERIC_DOMAINS && OF
 
 config CPU_PM
 	bool
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 11/11] ACPI / PM: Convert acpi_dev_pm_detach() into a static function

[Ulf Hansson]

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 <ulf.hansson@linaro.org>
---
 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

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

[Dmitry Torokhov]

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

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

[Rafael J. Wysocki]

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:
> > 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.

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

[Ulf Hansson]

On 22 September 2014 16:19, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> 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:
>> > 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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

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>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Grygorii Strashko]

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:
>> 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>
> 
> 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

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

[Rafael J. Wysocki]

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:
> > 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:
> >>> 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

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

[Grygorii Strashko]

On 09/24/2014 04:51 PM, Rafael J. Wysocki wrote:
> 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:
>>> 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:
>>>>> 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

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

[Ulf Hansson]

On 25 September 2014 13:21, Thierry Reding <thierry.reding@gmail.com> 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

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4193 bytes --]

On Thu, Sep 25, 2014 at 05:29:10PM +0200, Ulf Hansson wrote:
> On 25 September 2014 13:21, Thierry Reding <thierry.reding@gmail.com> 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?

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().

> > 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.

> > 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.

> > 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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

[Stephen Boyd]

On 09/25, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 05:29:10PM +0200, Ulf Hansson wrote:
> > On 25 September 2014 13:21, Thierry Reding <thierry.reding@gmail.com> 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?
> 
> 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().
> 
> > > 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

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

[Kevin Hilman]

Stephen Boyd <sboyd@codeaurora.org> writes:

> On 09/25, Thierry Reding wrote:

[...]

>> 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.

Excellent!  I'm glad you're heading in that direction.  One other
important point here is that keeping drivers "simple" like this makes it
much easier for them to stay portable across SoCs.

Also, neither tegra or qcom are unique here.

FWIW, OMAP has similar ordering requirements, and quite a bit of "stuff"
to properly get a device and powerdomain to actually power down (and
power up) properly.  The OMAP implementation pre-dates runtime PM, power
domains etc. so we hid all of this behind the omap_hwmod abstraction and
the omap_device API where all clocks, resets, and various other magic is
handled, and so that device drivers didn't have to care about the details.

These days, those APIs are now used by the OMAP pm_domain implementation
(which pre-dates genpd), so drivers (still) don't have to care about the
details only have to care about runtime PM and QoS.  After genpd came
along, the goal was to convert OMAP to it, but well, other circumstances
have changed things such that those working in that area are, um...,
somewhat less focused on OMAP. ;)

Kevin

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 5702 bytes --]

On Thu, Sep 25, 2014 at 05:27:57PM -0700, Stephen Boyd wrote:
> On 09/25, Thierry Reding wrote:
> > On Thu, Sep 25, 2014 at 05:29:10PM +0200, Ulf Hansson wrote:
> > > On 25 September 2014 13:21, Thierry Reding <thierry.reding@gmail.com> 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?
> > 
> > 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().
> > 
> > > > 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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

[Kevin Hilman]

Thierry Reding <thierry.reding@gmail.com> writes:

> 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. 

I think you're on the right path here.  You can get rid of this conflict
by removing the direct/manual clock management from the drivers, and
using runtime PM instead: s/clk_enable/pm_runtime_get_sync/,
s/clk_disable/pm_runtime_put_sync/.  When using runtime PM, those calls
trickle down through the power domain driver, which can manage the
device clocks, as well as any additional clocks and resets needed for
power gating.

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

In these cases, you should use the per-device PM QoS framework to set
per-device latencies.  Then your power-domain can look up these
latencies and decide whether or not to actually power gate or not.

Kevin

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3530 bytes --]

On Thu, Sep 25, 2014 at 03:45:11PM -0700, Kevin Hilman wrote:
> Thierry Reding <thierry.reding@gmail.com> writes:
> 
> > 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. 
> 
> I think you're on the right path here.  You can get rid of this conflict
> by removing the direct/manual clock management from the drivers, and
> using runtime PM instead: s/clk_enable/pm_runtime_get_sync/,
> s/clk_disable/pm_runtime_put_sync/.  When using runtime PM, those calls
> trickle down through the power domain driver, which can manage the
> device clocks, as well as any additional clocks and resets needed for
> power gating.

Okay. The DT part of it is going to be pretty nasty (as usual) because
we currently have the clocks and resets within the device's device tree
node (which I think is where they really belong).

So one possibility would be to move the clocks and resets to the power
domain controller's node, like so:

	pmc@... {
		power-domains {
			...

			sata@... {
				clocks = <&tegra_car 124>;
				resets = <&tegra_car 124>;
			};
		};
	};

An alternative would be to make the power-domain controller look up the
clock within the user's device tree node. That could be problematic,
because while the module clock is always the first clock in current
device trees, there aren't ordering guarantees, so we'd have to rely on
the clock name.

> > Furthermore for some devices it may turn out that turning the domain
> > off and on introduces too much latency to be useful.
> 
> In these cases, you should use the per-device PM QoS framework to set
> per-device latencies.  Then your power-domain can look up these
> latencies and decide whether or not to actually power gate or not.

Does this allow fine-grained control over what parts of the "power
domain" get disabled? For example some drivers may want to only turn off
the clock under some circumstances, which would keep the hardware state,
whereas in other situations or drivers it might be fine to completely
turn off the power partition and reset the hardware block (loosing all
hardware state).

Keeping a reference to the clock in the power domain will prevent that
from working since the driver itself won't be able to disable the
hardware clock.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

[Geert Uytterhoeven]

Hi Thierry,

On Fri, Sep 26, 2014 at 9:31 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Thu, Sep 25, 2014 at 03:45:11PM -0700, Kevin Hilman wrote:
>> Thierry Reding <thierry.reding@gmail.com> writes:
>> > 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.
>>
>> I think you're on the right path here.  You can get rid of this conflict
>> by removing the direct/manual clock management from the drivers, and
>> using runtime PM instead: s/clk_enable/pm_runtime_get_sync/,
>> s/clk_disable/pm_runtime_put_sync/.  When using runtime PM, those calls
>> trickle down through the power domain driver, which can manage the
>> device clocks, as well as any additional clocks and resets needed for
>> power gating.
>
> Okay. The DT part of it is going to be pretty nasty (as usual) because
> we currently have the clocks and resets within the device's device tree
> node (which I think is where they really belong).
>
> So one possibility would be to move the clocks and resets to the power
> domain controller's node, like so:
>
>         pmc@... {
>                 power-domains {
>                         ...
>
>                         sata@... {
>                                 clocks = <&tegra_car 124>;
>                                 resets = <&tegra_car 124>;
>                         };
>                 };
>         };
>
> An alternative would be to make the power-domain controller look up the
> clock within the user's device tree node. That could be problematic,
> because while the module clock is always the first clock in current
> device trees, there aren't ordering guarantees, so we'd have to rely on
> the clock name.

Or on some other way.
Do you have a separate hardware block that controls all (and only) the
module clocks?

On shmobile SoCs, all module clocks are controlled using the MSTP
(Module SToP) clocks.

In my old RFC series "[PATCH/RFC 0/4] of: Register clocks for Runtime PM
with PM core" (https://lkml.org/lkml/2014/4/24/1118) the MSTP clock driver
advertised using a new CLK_RUNTIME_PM flag that its clocks are module
clocks and thus suitable for runtime PM.

There were some issues with that series, but the general idea of letting a
clock driver advertise that all its clocks are module clocks could still be
useful. Then the power domain driver knows which clocks to manage.

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

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4257 bytes --]

On Fri, Sep 26, 2014 at 10:06:24AM +0200, Geert Uytterhoeven wrote:
> Hi Thierry,
> 
> On Fri, Sep 26, 2014 at 9:31 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Thu, Sep 25, 2014 at 03:45:11PM -0700, Kevin Hilman wrote:
> >> Thierry Reding <thierry.reding@gmail.com> writes:
> >> > 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.
> >>
> >> I think you're on the right path here.  You can get rid of this conflict
> >> by removing the direct/manual clock management from the drivers, and
> >> using runtime PM instead: s/clk_enable/pm_runtime_get_sync/,
> >> s/clk_disable/pm_runtime_put_sync/.  When using runtime PM, those calls
> >> trickle down through the power domain driver, which can manage the
> >> device clocks, as well as any additional clocks and resets needed for
> >> power gating.
> >
> > Okay. The DT part of it is going to be pretty nasty (as usual) because
> > we currently have the clocks and resets within the device's device tree
> > node (which I think is where they really belong).
> >
> > So one possibility would be to move the clocks and resets to the power
> > domain controller's node, like so:
> >
> >         pmc@... {
> >                 power-domains {
> >                         ...
> >
> >                         sata@... {
> >                                 clocks = <&tegra_car 124>;
> >                                 resets = <&tegra_car 124>;
> >                         };
> >                 };
> >         };
> >
> > An alternative would be to make the power-domain controller look up the
> > clock within the user's device tree node. That could be problematic,
> > because while the module clock is always the first clock in current
> > device trees, there aren't ordering guarantees, so we'd have to rely on
> > the clock name.
> 
> Or on some other way.
> Do you have a separate hardware block that controls all (and only) the
> module clocks?

No, the "clock and reset controller" controls all clocks (and resets) on
the SoC.

> On shmobile SoCs, all module clocks are controlled using the MSTP
> (Module SToP) clocks.
> 
> In my old RFC series "[PATCH/RFC 0/4] of: Register clocks for Runtime PM
> with PM core" (https://lkml.org/lkml/2014/4/24/1118) the MSTP clock driver
> advertised using a new CLK_RUNTIME_PM flag that its clocks are module
> clocks and thus suitable for runtime PM.
> 
> There were some issues with that series, but the general idea of letting a
> clock driver advertise that all its clocks are module clocks could still be
> useful. Then the power domain driver knows which clocks to manage.

That sounds interesting. Although it would still mean that we need a way
to associate a clock with the correct power domain. I guess the driver
could do that by iterating over all available clocks in the device's
clocks property and grab only those that are marked CLK_RUNTIME_PM.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

[Geert Uytterhoeven]

Hi Thierry,

On Fri, Sep 26, 2014 at 11:56 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
>> > An alternative would be to make the power-domain controller look up the
>> > clock within the user's device tree node. That could be problematic,
>> > because while the module clock is always the first clock in current
>> > device trees, there aren't ordering guarantees, so we'd have to rely on
>> > the clock name.
>>
>> Or on some other way.
>> Do you have a separate hardware block that controls all (and only) the
>> module clocks?
>
> No, the "clock and reset controller" controls all clocks (and resets) on
> the SoC.

Sorry, with "only the module clocks" I meant "not clocks that are not module
clocks". Having a reset controller function in there is fine.

So it seems you do have a clock provider that provides module clocks,
and can mark them CLK_RUNTIME_PM.

>> On shmobile SoCs, all module clocks are controlled using the MSTP
>> (Module SToP) clocks.
>>
>> In my old RFC series "[PATCH/RFC 0/4] of: Register clocks for Runtime PM
>> with PM core" (https://lkml.org/lkml/2014/4/24/1118) the MSTP clock driver
>> advertised using a new CLK_RUNTIME_PM flag that its clocks are module
>> clocks and thus suitable for runtime PM.
>>
>> There were some issues with that series, but the general idea of letting a
>> clock driver advertise that all its clocks are module clocks could still be
>> useful. Then the power domain driver knows which clocks to manage.
>
> That sounds interesting. Although it would still mean that we need a way
> to associate a clock with the correct power domain. I guess the driver
> could do that by iterating over all available clocks in the device's
> clocks property and grab only those that are marked CLK_RUNTIME_PM.

Yes, that's the idea.

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

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2407 bytes --]

On Fri, Sep 26, 2014 at 12:01:13PM +0200, Geert Uytterhoeven wrote:
> Hi Thierry,
> 
> On Fri, Sep 26, 2014 at 11:56 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> >> > An alternative would be to make the power-domain controller look up the
> >> > clock within the user's device tree node. That could be problematic,
> >> > because while the module clock is always the first clock in current
> >> > device trees, there aren't ordering guarantees, so we'd have to rely on
> >> > the clock name.
> >>
> >> Or on some other way.
> >> Do you have a separate hardware block that controls all (and only) the
> >> module clocks?
> >
> > No, the "clock and reset controller" controls all clocks (and resets) on
> > the SoC.
> 
> Sorry, with "only the module clocks" I meant "not clocks that are not module
> clocks". Having a reset controller function in there is fine.
> 
> So it seems you do have a clock provider that provides module clocks,
> and can mark them CLK_RUNTIME_PM.
> 
> >> On shmobile SoCs, all module clocks are controlled using the MSTP
> >> (Module SToP) clocks.
> >>
> >> In my old RFC series "[PATCH/RFC 0/4] of: Register clocks for Runtime PM
> >> with PM core" (https://lkml.org/lkml/2014/4/24/1118) the MSTP clock driver
> >> advertised using a new CLK_RUNTIME_PM flag that its clocks are module
> >> clocks and thus suitable for runtime PM.
> >>
> >> There were some issues with that series, but the general idea of letting a
> >> clock driver advertise that all its clocks are module clocks could still be
> >> useful. Then the power domain driver knows which clocks to manage.
> >
> > That sounds interesting. Although it would still mean that we need a way
> > to associate a clock with the correct power domain. I guess the driver
> > could do that by iterating over all available clocks in the device's
> > clocks property and grab only those that are marked CLK_RUNTIME_PM.
> 
> Yes, that's the idea.

It sounds like this could work. Reading up on the thread you linked to
there's some resistance to merging what you propose, though. Given that
we may want to control the clocks and resets in the controller as well,
maybe duplicating the clock references would actually be the proper way
to do it.

I'll have to think a little bit more about it, or implement it to see
how it all fits. Thanks for the pointers.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

[Kevin Hilman]

Thierry Reding <thierry.reding@gmail.com> writes:

> On Thu, Sep 25, 2014 at 03:45:11PM -0700, Kevin Hilman wrote:
>> Thierry Reding <thierry.reding@gmail.com> writes:
>> 
>> > 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. 
>> 
>> I think you're on the right path here.  You can get rid of this conflict
>> by removing the direct/manual clock management from the drivers, and
>> using runtime PM instead: s/clk_enable/pm_runtime_get_sync/,
>> s/clk_disable/pm_runtime_put_sync/.  When using runtime PM, those calls
>> trickle down through the power domain driver, which can manage the
>> device clocks, as well as any additional clocks and resets needed for
>> power gating.
>
> Okay. The DT part of it is going to be pretty nasty (as usual) because
> we currently have the clocks and resets within the device's device tree
> node (which I think is where they really belong).
>
> So one possibility would be to move the clocks and resets to the power
> domain controller's node, like so:
>
> 	pmc@... {
> 		power-domains {
> 			...
>
> 			sata@... {
> 				clocks = <&tegra_car 124>;
> 				resets = <&tegra_car 124>;
> 			};
> 		};
> 	};
>
> An alternative would be to make the power-domain controller look up the
> clock within the user's device tree node. That could be problematic,
> because while the module clock is always the first clock in current
> device trees, there aren't ordering guarantees, so we'd have to rely on
> the clock name.
>
>> > Furthermore for some devices it may turn out that turning the domain
>> > off and on introduces too much latency to be useful.
>> 
>> In these cases, you should use the per-device PM QoS framework to set
>> per-device latencies.  Then your power-domain can look up these
>> latencies and decide whether or not to actually power gate or not.
>
> Does this allow fine-grained control over what parts of the "power
> domain" get disabled? 

Sure.  Your power-domain can have control over this, and can make
decisions based on QoS constraints or other considerations.

> For example some drivers may want to only turn off
> the clock under some circumstances, which would keep the hardware state,
> whereas in other situations or drivers it might be fine to completely
> turn off the power partition and reset the hardware block (loosing all
> hardware state).

Well, IMO, it shouldn't be the drivers making these decisions, because
the "circumstances" around these decisions typically are related to QoS
constraints that affect the whole power domain, not just that device.

Part of the benefit of moving direct clock management out of the
drivers, and into some pm_domain/genpd logic is that you put them in the
place that is has enough information to make decision based on the whole
power domain.  e.g. a specific device driver doesn't know (and shouldn't
know, IMO) about the what other devices might be in the same power
domain.  Only the power-domain logic has enough of a "big picture" to
make the right decisions.  The wakeup latency and the save/restore
context needs of a device depend on other factors in the power domain,
like whether that device is the last one to be idle, or whether the
power domain is going to be power gated due to constraints set by some
other device. 

> Keeping a reference to the clock in the power domain will prevent that
> from working since the driver itself won't be able to disable the
> hardware clock.

IMO, the driver itself shouldn't be disabling the hardware clock in the
first place.  That should be left up to the domain logic.

Kevin