[PATCH v3 0/3] Generic Device Tree based power domain look-up

[PATCH v3 0/3] Generic Device Tree based power domain look-up

[Tomasz Figa]

Up till now there was no single generic method to bind devices to their
power domains using Device Tree. Each platform has been doing this using
its own way, example of which are Exynos power domain bindings [1] and
look-up code [2].

This series is intended to change this and provide generic DT bindings for
power domain specification and generic code performing look-up of power
domains and binding them to devices.

First two patches are the most important part of this series, as they
introduce $subject. Patch 3 converts mach-exynos to use the new generic
method. Further patches are adding one more user of the new code,
mach-s3c64xx, with first 3 patches (4-6) required to clean-up its power
domain driver a bit and last 3 patches (9-11) adding display support for
Mini6410 board, including a node for display controller (FIMD) which is
a power domain consumer.

The design of DT bindings and provider code is heavily inspired by
implementation of clock providers in Common Clock Framework, while
the code binding devices to power domains by my Exynos power domain
implementation (now removed by this series ;)).

Successfully tested on Exynos4210-based Trats and Exynos4412-based Trats2
boards using MFC, 

[1] Documentation/devicetree/bindings/arm/exynos/power_domain.txt
[2] arch/arm/mach-exynos/pm_domains.c

Changes since v2:
(http://thread.gmane.org/gmane.linux.kernel/1658926)
 - rebased onto current Rafael's linux-pm bleeding-edge branch,
 - dropped patches for s3c64xx for now. I will send them in separate series,
 - do not call pm_genpd_dev_need_restore(true) in genpd_bind_domain(),
 - fixed various stylistic issues reported in review comments.

Changes since v1 (RFC):
[https://lkml.org/lkml/2014/1/11/141]
 - rebased onto current Rafael's linux-pm bleeding-edge branch,
 - reordered the patches a bit (to have the generic ones first),
 - dropped renaming of S3C64xx power domains (as suggested by Mark Brown),
 - added support for deferred probing (as suggested by Stephen Boyd),
 - fixed several minor issues pointed by Stephen Boyd,
 - replaced notifiers with direct hooks in driver core to make power domain
   support independent from specific bus type and allow error handling.

Tomasz Figa (3):
  base: power: Add generic OF-based power domain look-up
  drivercore: Bind/unbind power domain on probe/remove
  ARM: exynos: Move to generic power domain bindings

 .../bindings/arm/exynos/power_domain.txt           |  12 +-
 .../devicetree/bindings/power/power_domain.txt     |  51 ++++
 arch/arm/mach-exynos/pm_domains.c                  |  81 +-----
 drivers/base/dd.c                                  |   9 +-
 drivers/base/power/domain.c                        | 283 +++++++++++++++++++++
 include/linux/pm_domain.h                          |  46 ++++
 kernel/power/Kconfig                               |   4 +
 7 files changed, 398 insertions(+), 88 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/power_domain.txt

-- 
1.9.2

[PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Tomasz Figa]

This patch introduces generic code to perform power domain look-up using
device tree and automatically bind devices to their power domains.
Generic device tree binding is introduced to specify power domains of
devices in their device tree nodes.

Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
the generic framework in further patch.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Reviewed-by: Mark Brown <broonie@linaro.org>
Reviewed-by: Kevin Hilman <khilman@linaro.org>
Reviewed-by: Philipp Zabel <philipp.zabel@gmail.com>
[on i.MX6 GK802]
Tested-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 .../devicetree/bindings/power/power_domain.txt     |  51 ++++
 drivers/base/power/domain.c                        | 283 +++++++++++++++++++++
 include/linux/pm_domain.h                          |  46 ++++
 kernel/power/Kconfig                               |   4 +
 4 files changed, 384 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..93be5d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -0,0 +1,51 @@
+* Generic power domains
+
+System on chip designs are often divided into multiple power 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 power domain consumer devices
+with their power domains provided by power domain providers. A power domain
+provider can be represented by any node in the device tree and can provide
+one or more power domains. A consumer node can refer to the provider by
+a phandle and a set of phandle arguments (so called power domain specifier)
+of length specified by #power-domain-cells property in the power domain
+provider node.
+
+==Power domain providers==
+
+Required properties:
+ - #power-domain-cells : Number of cells in a power domain specifier;
+   Typically 0 for nodes representing a single power domain and 1 for nodes
+   providing multiple power 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 power domain provider
+and expects one cell as its phandle argument.
+
+==Power domain consumers==
+
+Required properties:
+ - power-domain : A phandle and power domain specifier as defined by bindings
+                  of power controller specified by phandle.
+
+Example:
+
+	leaky-device@12350000 {
+		compatible = "foo,i-leak-current";
+		reg = <0x12350000 0x1000>;
+		power-domain = <&power 0>;
+	};
+
+The node above defines a typical power domain consumer device, which is located
+inside power domain with index 0 of power controller represented by node with
+label "power".
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index ae098a2..7677744 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>
@@ -2189,3 +2190,285 @@ 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 power domain providers.
+ *
+ * The code below implements generic device tree based power domain providers
+ * that bind device tree nodes with generic power domains registered in the
+ * system.
+ *
+ * Any driver that registers generic power domains and need to support binding
+ * of devices to these domains is supposed to register a power domain provider,
+ * which maps a power domain specifier retrieved from device tree to a power
+ * domain.
+ *
+ * Two simple mapping functions have been provided for convenience:
+ *  - of_genpd_xlate_simple() for 1:1 device tree node to domain mapping,
+ *  - of_genpd_xlate_onecell() for mapping of multiple domains per node
+ *    by index.
+ */
+
+/**
+ * struct of_genpd_provider - Power domain provider registration structure
+ * @link: Entry in global list of domain providers
+ * @node: Pointer to device tree node of domain provider
+ * @xlate: Provider-specific xlate callback mapping a set of specifier cells
+ *         into a power 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 power 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 power domain
+ * @data: xlate function private data - pointer to struct generic_pm_domain
+ *
+ * This is a generic xlate function that can be used to model power 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 for providers using single index.
+ * @genpdspec: OF phandle args to map into a power 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 power
+ * domain controllers that have one device tree node and provide multiple
+ * power domains. A single cell is used as an index to an array of power
+ * domains specified in 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->domain_num) {
+		pr_err("%s: invalid domain index %d\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return genpd_data->domains[idx];
+}
+EXPORT_SYMBOL_GPL(of_genpd_xlate_onecell);
+
+/**
+ * of_genpd_add_provider() - Register a domain provider for a node
+ * @np: Device node pointer associated with domain provider.
+ * @xlate: Callback for decoding domain from phandle arguments.
+ * @data: Context pointer for @genpd_src_get 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 domain provider
+ * @np: Device node pointer associated with 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 power domain
+ * @genpdspec: OF phandle args to use for look-up
+ *
+ * Looks for domain provider under node specified by @genpdspec and if found
+ * uses xlate function of the provider to map phandle args to a power 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(-EPROBE_DEFER);
+	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;
+}
+
+/*
+ * Device<->domain binding using Device Tree look-up.
+ *
+ * The purpose of code below is to manage assignment of devices to their
+ * power domains in an automatic fashion, based on data read from device tree.
+ * The two functions, genpd_bind_domain() and genpd_unbind_domain() are
+ * intended to be called by higher level code that manages devices, i.e.
+ * really_probe() and __device_release_driver() to respectively bind and
+ * unbind device from its power domain.
+ *
+ * Both generic and legacy Samsung-specific DT bindings are supported to
+ * keep backwards compatibility with existing DTBs.
+ */
+
+/**
+ * genpd_bind_domain - Bind device to its power domain using Device Tree.
+ * @dev: Device to bind to its power domain.
+ *
+ * Tries to parse power domain specifier from device's OF node and if succeeds
+ * attaches the device to retrieved power domain.
+ *
+ * Returns 0 on success or negative error code otherwise.
+ */
+int genpd_bind_domain(struct device *dev)
+{
+	struct of_phandle_args pd_args;
+	struct generic_pm_domain *pd;
+	int ret;
+
+	if (!dev->of_node)
+		return 0;
+
+	ret = of_parse_phandle_with_args(dev->of_node, "power-domain",
+					"#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 0;
+	}
+
+	pd = of_genpd_get_from_provider(&pd_args);
+	if (IS_ERR(pd)) {
+		if (PTR_ERR(pd) != -EPROBE_DEFER)
+			dev_err(dev, "failed to find power domain: %ld\n",
+				PTR_ERR(pd));
+		return PTR_ERR(pd);
+	}
+
+	dev_dbg(dev, "adding to power 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 power domain %s: %d",
+			pd->name, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * genpd_unbind_domain - Unbind device from its power domain.
+ * @dev: Device to unbind from its power domain.
+ *
+ * Unbinds device from power domain previously bound to it.
+ *
+ * Returns 0 on success or negative error code otherwise.
+ */
+int genpd_unbind_domain(struct device *dev)
+{
+	struct generic_pm_domain *pd = dev_to_genpd(dev);
+	int ret;
+
+	if (!dev->of_node || IS_ERR(pd))
+		return 0;
+
+	dev_dbg(dev, "removing from power 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 power domain %s: %d",
+			pd->name, ret);
+		return ret;
+	}
+
+	/* Check if domain can be powered off after removing this device. */
+	genpd_queue_power_off_work(pd);
+
+	return 0;
+}
+#endif
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 7c1d252..04473d4 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -310,4 +310,50 @@ static inline void pm_genpd_syscore_poweron(struct device *dev)
 	pm_genpd_syscore_switch(dev, false);
 }
 
+/* OF power domain providers */
+struct of_device_id;
+
+struct genpd_onecell_data {
+	struct generic_pm_domain **domains;
+	unsigned int domain_num;
+};
+
+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_bind_domain(struct device *dev);
+int genpd_unbind_domain(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_bind_domain(struct device *dev)
+{
+	return 0;
+}
+static inline int genpd_unbind_domain(struct device *dev)
+{
+	return 0;
+}
+#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
+
 #endif /* _LINUX_PM_DOMAIN_H */
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 2fac9cc..45aa98e 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -306,6 +306,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.2

Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Ulf Hansson]

On 23 April 2014 18:46, Tomasz Figa <t.figa@samsung.com> wrote:
> This patch introduces generic code to perform power domain look-up using
> device tree and automatically bind devices to their power domains.
> Generic device tree binding is introduced to specify power domains of
> devices in their device tree nodes.
>
> Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
> the generic framework in further patch.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Reviewed-by: Mark Brown <broonie@linaro.org>
> Reviewed-by: Kevin Hilman <khilman@linaro.org>
> Reviewed-by: Philipp Zabel <philipp.zabel@gmail.com>
> [on i.MX6 GK802]
> Tested-by: Philipp Zabel <philipp.zabel@gmail.com>
> ---
>  .../devicetree/bindings/power/power_domain.txt     |  51 ++++
>  drivers/base/power/domain.c                        | 283 +++++++++++++++++++++
>  include/linux/pm_domain.h                          |  46 ++++
>  kernel/power/Kconfig                               |   4 +
>  4 files changed, 384 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..93be5d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -0,0 +1,51 @@
> +* Generic power domains
> +
> +System on chip designs are often divided into multiple power 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 power domain consumer devices
> +with their power domains provided by power domain providers. A power domain
> +provider can be represented by any node in the device tree and can provide
> +one or more power domains. A consumer node can refer to the provider by
> +a phandle and a set of phandle arguments (so called power domain specifier)
> +of length specified by #power-domain-cells property in the power domain
> +provider node.
> +
> +==Power domain providers==
> +
> +Required properties:
> + - #power-domain-cells : Number of cells in a power domain specifier;
> +   Typically 0 for nodes representing a single power domain and 1 for nodes
> +   providing multiple power 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 power domain provider
> +and expects one cell as its phandle argument.
> +
> +==Power domain consumers==
> +
> +Required properties:
> + - power-domain : A phandle and power domain specifier as defined by bindings
> +                  of power controller specified by phandle.
> +
> +Example:
> +
> +       leaky-device@12350000 {
> +               compatible = "foo,i-leak-current";
> +               reg = <0x12350000 0x1000>;
> +               power-domain = <&power 0>;
> +       };
> +
> +The node above defines a typical power domain consumer device, which is located
> +inside power domain with index 0 of power controller represented by node with
> +label "power".
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index ae098a2..7677744 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>
> @@ -2189,3 +2190,285 @@ 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

I remember we discussed this earlier, but I don't think we should add
another Kconfig for this. CONFIG_OF should be enough.

I realize that we need to handle "Samsung-specific bindings", to not
break the Exynos pm domain. Maybe adding the parsing of them into the
generic power domain in the same patch as when we switch Exynos to the
new API would do the trick?

Kind regards
Ulf Hansson

> +/*
> + * Device Tree based power domain providers.
> + *
> + * The code below implements generic device tree based power domain providers
> + * that bind device tree nodes with generic power domains registered in the
> + * system.
> + *
> + * Any driver that registers generic power domains and need to support binding
> + * of devices to these domains is supposed to register a power domain provider,
> + * which maps a power domain specifier retrieved from device tree to a power
> + * domain.
> + *
> + * Two simple mapping functions have been provided for convenience:
> + *  - of_genpd_xlate_simple() for 1:1 device tree node to domain mapping,
> + *  - of_genpd_xlate_onecell() for mapping of multiple domains per node
> + *    by index.
> + */
> +
> +/**
> + * struct of_genpd_provider - Power domain provider registration structure
> + * @link: Entry in global list of domain providers
> + * @node: Pointer to device tree node of domain provider
> + * @xlate: Provider-specific xlate callback mapping a set of specifier cells
> + *         into a power 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 power 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 power domain
> + * @data: xlate function private data - pointer to struct generic_pm_domain
> + *
> + * This is a generic xlate function that can be used to model power 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 for providers using single index.
> + * @genpdspec: OF phandle args to map into a power 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 power
> + * domain controllers that have one device tree node and provide multiple
> + * power domains. A single cell is used as an index to an array of power
> + * domains specified in 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->domain_num) {
> +               pr_err("%s: invalid domain index %d\n", __func__, idx);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       return genpd_data->domains[idx];
> +}
> +EXPORT_SYMBOL_GPL(of_genpd_xlate_onecell);
> +
> +/**
> + * of_genpd_add_provider() - Register a domain provider for a node
> + * @np: Device node pointer associated with domain provider.
> + * @xlate: Callback for decoding domain from phandle arguments.
> + * @data: Context pointer for @genpd_src_get 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 domain provider
> + * @np: Device node pointer associated with 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 power domain
> + * @genpdspec: OF phandle args to use for look-up
> + *
> + * Looks for domain provider under node specified by @genpdspec and if found
> + * uses xlate function of the provider to map phandle args to a power 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(-EPROBE_DEFER);
> +       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;
> +}
> +
> +/*
> + * Device<->domain binding using Device Tree look-up.
> + *
> + * The purpose of code below is to manage assignment of devices to their
> + * power domains in an automatic fashion, based on data read from device tree.
> + * The two functions, genpd_bind_domain() and genpd_unbind_domain() are
> + * intended to be called by higher level code that manages devices, i.e.
> + * really_probe() and __device_release_driver() to respectively bind and
> + * unbind device from its power domain.
> + *
> + * Both generic and legacy Samsung-specific DT bindings are supported to
> + * keep backwards compatibility with existing DTBs.
> + */
> +
> +/**
> + * genpd_bind_domain - Bind device to its power domain using Device Tree.
> + * @dev: Device to bind to its power domain.
> + *
> + * Tries to parse power domain specifier from device's OF node and if succeeds
> + * attaches the device to retrieved power domain.
> + *
> + * Returns 0 on success or negative error code otherwise.
> + */
> +int genpd_bind_domain(struct device *dev)
> +{
> +       struct of_phandle_args pd_args;
> +       struct generic_pm_domain *pd;
> +       int ret;
> +
> +       if (!dev->of_node)
> +               return 0;
> +
> +       ret = of_parse_phandle_with_args(dev->of_node, "power-domain",
> +                                       "#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 0;
> +       }
> +
> +       pd = of_genpd_get_from_provider(&pd_args);
> +       if (IS_ERR(pd)) {
> +               if (PTR_ERR(pd) != -EPROBE_DEFER)
> +                       dev_err(dev, "failed to find power domain: %ld\n",
> +                               PTR_ERR(pd));
> +               return PTR_ERR(pd);
> +       }
> +
> +       dev_dbg(dev, "adding to power 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 power domain %s: %d",
> +                       pd->name, ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * genpd_unbind_domain - Unbind device from its power domain.
> + * @dev: Device to unbind from its power domain.
> + *
> + * Unbinds device from power domain previously bound to it.
> + *
> + * Returns 0 on success or negative error code otherwise.
> + */
> +int genpd_unbind_domain(struct device *dev)
> +{
> +       struct generic_pm_domain *pd = dev_to_genpd(dev);
> +       int ret;
> +
> +       if (!dev->of_node || IS_ERR(pd))
> +               return 0;
> +
> +       dev_dbg(dev, "removing from power 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 power domain %s: %d",
> +                       pd->name, ret);
> +               return ret;
> +       }
> +
> +       /* Check if domain can be powered off after removing this device. */
> +       genpd_queue_power_off_work(pd);
> +
> +       return 0;
> +}
> +#endif
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 7c1d252..04473d4 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -310,4 +310,50 @@ static inline void pm_genpd_syscore_poweron(struct device *dev)
>         pm_genpd_syscore_switch(dev, false);
>  }
>
> +/* OF power domain providers */
> +struct of_device_id;
> +
> +struct genpd_onecell_data {
> +       struct generic_pm_domain **domains;
> +       unsigned int domain_num;
> +};
> +
> +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_bind_domain(struct device *dev);
> +int genpd_unbind_domain(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_bind_domain(struct device *dev)
> +{
> +       return 0;
> +}
> +static inline int genpd_unbind_domain(struct device *dev)
> +{
> +       return 0;
> +}
> +#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
> +
>  #endif /* _LINUX_PM_DOMAIN_H */
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 2fac9cc..45aa98e 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -306,6 +306,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.2
>

Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Ulf Hansson]

On 24 April 2014 11:40, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 23 April 2014 18:46, Tomasz Figa <t.figa@samsung.com> wrote:
>> This patch introduces generic code to perform power domain look-up using
>> device tree and automatically bind devices to their power domains.
>> Generic device tree binding is introduced to specify power domains of
>> devices in their device tree nodes.
>>
>> Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
>> the generic framework in further patch.
>>
>> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
>> Reviewed-by: Mark Brown <broonie@linaro.org>
>> Reviewed-by: Kevin Hilman <khilman@linaro.org>
>> Reviewed-by: Philipp Zabel <philipp.zabel@gmail.com>
>> [on i.MX6 GK802]
>> Tested-by: Philipp Zabel <philipp.zabel@gmail.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

I also implemented some test-support for genpd for a ux500 SOC as well
- tested passed successfully. :-)

[snip]

>> +
>> +#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>
> I remember we discussed this earlier, but I don't think we should add
> another Kconfig for this. CONFIG_OF should be enough.
>
> I realize that we need to handle "Samsung-specific bindings", to not
> break the Exynos pm domain. Maybe adding the parsing of them into the
> generic power domain in the same patch as when we switch Exynos to the
> new API would do the trick?

I realized that there already exists CONFIG_PM_GENERIC_DOMAINS_SLEEP
and CONFIG_PM_GENERIC_DOMAINS_RUNTIME, which serves similar purpose as
CONFIG_PM_GENERIC_DOMAINS_OF would.

So, let's ignore my previous comment and move on.

Kind regards
Ulf Hansson

Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Stephen Warren]

On 04/23/2014 10:46 AM, Tomasz Figa wrote:
> This patch introduces generic code to perform power domain look-up using
> device tree and automatically bind devices to their power domains.
> Generic device tree binding is introduced to specify power domains of
> devices in their device tree nodes.
> 
> Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
> the generic framework in further patch.

> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt

> +==Power domain consumers==
> +
> +Required properties:
> + - power-domain : A phandle and power domain specifier as defined by bindings
> +                  of power controller specified by phandle.

It seems quite likely that a single logical device could have components
in multiple power domains. Consider an HDMI controller with different
power domains for the HDMI core, CEC communication, DDC/I2C
communication, and the I/O pads, with no clear separation between those
two components of the module (no separate register spaces, but the
bits/registers are interleaved all together).

As such, I think that rather than a "power-domain" property, we need a
pair of "power-domains", and "power-domain-names" properties, and
preferably with mandatory usage of name-based lookups, rather than
allowing a random mix of name-based and index-based lookups like we have
with some existing resource bindings.

Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Ulf Hansson]

On 28 April 2014 19:35, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/23/2014 10:46 AM, Tomasz Figa wrote:
>> This patch introduces generic code to perform power domain look-up using
>> device tree and automatically bind devices to their power domains.
>> Generic device tree binding is introduced to specify power domains of
>> devices in their device tree nodes.
>>
>> Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
>> the generic framework in further patch.
>
>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>
>> +==Power domain consumers==
>> +
>> +Required properties:
>> + - power-domain : A phandle and power domain specifier as defined by bindings
>> +                  of power controller specified by phandle.
>
> It seems quite likely that a single logical device could have components
> in multiple power domains. Consider an HDMI controller with different
> power domains for the HDMI core, CEC communication, DDC/I2C
> communication, and the I/O pads, with no clear separation between those
> two components of the module (no separate register spaces, but the
> bits/registers are interleaved all together).
>
> As such, I think that rather than a "power-domain" property, we need a
> pair of "power-domains", and "power-domain-names" properties, and
> preferably with mandatory usage of name-based lookups, rather than
> allowing a random mix of name-based and index-based lookups like we have
> with some existing resource bindings.

Each struct device have only one dev_pm_domain pointer, thus a device
are not able to reside in more than one power domain.

Therefore I doubt this will be very useful, unless I missed your point. :-)

Kind regards
Ulf Hansson

Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Stephen Warren]

On 05/06/2014 02:22 AM, Ulf Hansson wrote:
> On 28 April 2014 19:35, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/23/2014 10:46 AM, Tomasz Figa wrote:
>>> This patch introduces generic code to perform power domain look-up using
>>> device tree and automatically bind devices to their power domains.
>>> Generic device tree binding is introduced to specify power domains of
>>> devices in their device tree nodes.
>>>
>>> Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
>>> the generic framework in further patch.
>>
>>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>
>>> +==Power domain consumers==
>>> +
>>> +Required properties:
>>> + - power-domain : A phandle and power domain specifier as defined by bindings
>>> +                  of power controller specified by phandle.
>>
>> It seems quite likely that a single logical device could have components
>> in multiple power domains. Consider an HDMI controller with different
>> power domains for the HDMI core, CEC communication, DDC/I2C
>> communication, and the I/O pads, with no clear separation between those
>> two components of the module (no separate register spaces, but the
>> bits/registers are interleaved all together).
>>
>> As such, I think that rather than a "power-domain" property, we need a
>> pair of "power-domains", and "power-domain-names" properties, and
>> preferably with mandatory usage of name-based lookups, rather than
>> allowing a random mix of name-based and index-based lookups like we have
>> with some existing resource bindings.
> 
> Each struct device have only one dev_pm_domain pointer, thus a device
> are not able to reside in more than one power domain.
> 
> Therefore I doubt this will be very useful, unless I missed your point. :-)

A struct device is a Linux-internal concept. DT is supposed to represent
the HW in an OS-agnostic fashion, not according to the limitations of
any one OS's driver model.

It's certainly true that a single logical HW module (represented by a
single DT node) can have parts in multiple power domains.

Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Tomasz Figa]

On 06.05.2014 18:26, Stephen Warren wrote:
> On 05/06/2014 02:22 AM, Ulf Hansson wrote:
>> On 28 April 2014 19:35, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/23/2014 10:46 AM, Tomasz Figa wrote:
>>>> This patch introduces generic code to perform power domain look-up using
>>>> device tree and automatically bind devices to their power domains.
>>>> Generic device tree binding is introduced to specify power domains of
>>>> devices in their device tree nodes.
>>>>
>>>> Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
>>>> the generic framework in further patch.
>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>>
>>>> +==Power domain consumers==
>>>> +
>>>> +Required properties:
>>>> + - power-domain : A phandle and power domain specifier as defined by bindings
>>>> +                  of power controller specified by phandle.
>>>
>>> It seems quite likely that a single logical device could have components
>>> in multiple power domains. Consider an HDMI controller with different
>>> power domains for the HDMI core, CEC communication, DDC/I2C
>>> communication, and the I/O pads, with no clear separation between those
>>> two components of the module (no separate register spaces, but the
>>> bits/registers are interleaved all together).
>>>
>>> As such, I think that rather than a "power-domain" property, we need a
>>> pair of "power-domains", and "power-domain-names" properties, and
>>> preferably with mandatory usage of name-based lookups, rather than
>>> allowing a random mix of name-based and index-based lookups like we have
>>> with some existing resource bindings.
>>
>> Each struct device have only one dev_pm_domain pointer, thus a device
>> are not able to reside in more than one power domain.
>>
>> Therefore I doubt this will be very useful, unless I missed your point. :-)
>
> A struct device is a Linux-internal concept. DT is supposed to represent
> the HW in an OS-agnostic fashion, not according to the limitations of
> any one OS's driver model.
>
> It's certainly true that a single logical HW module (represented by a
> single DT node) can have parts in multiple power domains.
>

AFAIK a single DT node is supposed to represent a single _physical_ HW 
module. It all depends on description granularity, though. A physical 
chip (or IP block) might be further divided into multiple blocks, so I 
can imagine some of them being in different power domains than another. 
Still, based on the fact that in Linux we never needed to specify 
multiple power domains for a device, is this really a real use case?

Anyway, the binding is defined in a way that lets you simply turn the 
single phandle+specifier into a list of them, if such need ever shows up.

Best regards,
Tomasz

Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Stephen Warren]

On 05/06/2014 11:15 AM, Tomasz Figa wrote:
> 
> 
> On 06.05.2014 18:26, Stephen Warren wrote:
>> On 05/06/2014 02:22 AM, Ulf Hansson wrote:
>>> On 28 April 2014 19:35, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 04/23/2014 10:46 AM, Tomasz Figa wrote:
>>>>> This patch introduces generic code to perform power domain look-up
>>>>> using
>>>>> device tree and automatically bind devices to their power domains.
>>>>> Generic device tree binding is introduced to specify power domains of
>>>>> devices in their device tree nodes.
>>>>>
>>>>> Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
>>>>> the generic framework in further patch.
>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/power/power_domain.txt
>>>>> b/Documentation/devicetree/bindings/power/power_domain.txt
>>>>
>>>>> +==Power domain consumers==
>>>>> +
>>>>> +Required properties:
>>>>> + - power-domain : A phandle and power domain specifier as defined
>>>>> by bindings
>>>>> +                  of power controller specified by phandle.
>>>>
>>>> It seems quite likely that a single logical device could have
>>>> components
>>>> in multiple power domains. Consider an HDMI controller with different
>>>> power domains for the HDMI core, CEC communication, DDC/I2C
>>>> communication, and the I/O pads, with no clear separation between those
>>>> two components of the module (no separate register spaces, but the
>>>> bits/registers are interleaved all together).
>>>>
>>>> As such, I think that rather than a "power-domain" property, we need a
>>>> pair of "power-domains", and "power-domain-names" properties, and
>>>> preferably with mandatory usage of name-based lookups, rather than
>>>> allowing a random mix of name-based and index-based lookups like we
>>>> have
>>>> with some existing resource bindings.
>>>
>>> Each struct device have only one dev_pm_domain pointer, thus a device
>>> are not able to reside in more than one power domain.
>>>
>>> Therefore I doubt this will be very useful, unless I missed your
>>> point. :-)
>>
>> A struct device is a Linux-internal concept. DT is supposed to represent
>> the HW in an OS-agnostic fashion, not according to the limitations of
>> any one OS's driver model.
>>
>> It's certainly true that a single logical HW module (represented by a
>> single DT node) can have parts in multiple power domains.
> 
> AFAIK a single DT node is supposed to represent a single _physical_ HW
> module. It all depends on description granularity, though. A physical
> chip (or IP block) might be further divided into multiple blocks, so I
> can imagine some of them being in different power domains than another.

Physical-vs-logical probably means different things to different people.
What I mean is that DT should represent the HW modules at the
granularity that the HW documentation or designers describe the HW.

> Still, based on the fact that in Linux we never needed to specify
> multiple power domains for a device, is this really a real use case?

a) Linux has absolutely nothing to do with this. Again, DT is purely a
HW description that should not be influenced by OS limitations.

b) Linux does have to deal with this. HW exists which uses multiple
power domains per DT device node.

The fact that Linux forces the driver internals to create a separate
struct device for each part of the actual device that's in a different
power domain should not force us into representing the HW incorrectly in DT.

> Anyway, the binding is defined in a way that lets you simply turn the
> single phandle+specifier into a list of them, if such need ever shows up.

In my opinion, for all new resource bindings, we need to *exclusively*
support named-based lookup. That's the only way to ensure a simple
forward-compatible path to extensible DT bindings in the face of
multiple instances of a certain type of resource, where some are
optional. If resource lookups start out by index, and then later
(missing information when the DT binding was written, extending the DT
binding to a new HW revision/model) we find we need to add more
instances of that resource, some of which are optional, then anything
but purely name-based lookup makes the situation very complex.

We should solve this by simply using named-based lookup from the start
in all cases. That's my point here. Hence, we need to addm and enforce
usage of, a power-domain-names property.

Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up

[Tomasz Figa]

On 06.05.2014 19:24, Stephen Warren wrote:
> On 05/06/2014 11:15 AM, Tomasz Figa wrote:
>>
>>
>> On 06.05.2014 18:26, Stephen Warren wrote:
>>> On 05/06/2014 02:22 AM, Ulf Hansson wrote:
>>>> On 28 April 2014 19:35, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 04/23/2014 10:46 AM, Tomasz Figa wrote:
>>>>>> This patch introduces generic code to perform power domain look-up
>>>>>> using
>>>>>> device tree and automatically bind devices to their power domains.
>>>>>> Generic device tree binding is introduced to specify power domains of
>>>>>> devices in their device tree nodes.
>>>>>>
>>>>>> Backwards compatibility with legacy Samsung-specific power 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 Exynos power domain code gets converted to use
>>>>>> the generic framework in further patch.
>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/power/power_domain.txt
>>>>>> b/Documentation/devicetree/bindings/power/power_domain.txt
>>>>>
>>>>>> +==Power domain consumers==
>>>>>> +
>>>>>> +Required properties:
>>>>>> + - power-domain : A phandle and power domain specifier as defined
>>>>>> by bindings
>>>>>> +                  of power controller specified by phandle.
>>>>>
>>>>> It seems quite likely that a single logical device could have
>>>>> components
>>>>> in multiple power domains. Consider an HDMI controller with different
>>>>> power domains for the HDMI core, CEC communication, DDC/I2C
>>>>> communication, and the I/O pads, with no clear separation between those
>>>>> two components of the module (no separate register spaces, but the
>>>>> bits/registers are interleaved all together).
>>>>>
>>>>> As such, I think that rather than a "power-domain" property, we need a
>>>>> pair of "power-domains", and "power-domain-names" properties, and
>>>>> preferably with mandatory usage of name-based lookups, rather than
>>>>> allowing a random mix of name-based and index-based lookups like we
>>>>> have
>>>>> with some existing resource bindings.
>>>>
>>>> Each struct device have only one dev_pm_domain pointer, thus a device
>>>> are not able to reside in more than one power domain.
>>>>
>>>> Therefore I doubt this will be very useful, unless I missed your
>>>> point. :-)
>>>
>>> A struct device is a Linux-internal concept. DT is supposed to represent
>>> the HW in an OS-agnostic fashion, not according to the limitations of
>>> any one OS's driver model.
>>>
>>> It's certainly true that a single logical HW module (represented by a
>>> single DT node) can have parts in multiple power domains.
>>
>> AFAIK a single DT node is supposed to represent a single _physical_ HW
>> module. It all depends on description granularity, though. A physical
>> chip (or IP block) might be further divided into multiple blocks, so I
>> can imagine some of them being in different power domains than another.
>
> Physical-vs-logical probably means different things to different people.
> What I mean is that DT should represent the HW modules at the
> granularity that the HW documentation or designers describe the HW.
>
>> Still, based on the fact that in Linux we never needed to specify
>> multiple power domains for a device, is this really a real use case?
>
> a) Linux has absolutely nothing to do with this. Again, DT is purely a
> HW description that should not be influenced by OS limitations.
>
> b) Linux does have to deal with this. HW exists which uses multiple
> power domains per DT device node.
>
> The fact that Linux forces the driver internals to create a separate
> struct device for each part of the actual device that's in a different
> power domain should not force us into representing the HW incorrectly in DT.
>
>> Anyway, the binding is defined in a way that lets you simply turn the
>> single phandle+specifier into a list of them, if such need ever shows up.
>
> In my opinion, for all new resource bindings, we need to *exclusively*
> support named-based lookup. That's the only way to ensure a simple
> forward-compatible path to extensible DT bindings in the face of
> multiple instances of a certain type of resource, where some are
> optional. If resource lookups start out by index, and then later
> (missing information when the DT binding was written, extending the DT
> binding to a new HW revision/model) we find we need to add more
> instances of that resource, some of which are optional, then anything
> but purely name-based lookup makes the situation very complex.
>
> We should solve this by simply using named-based lookup from the start
> in all cases. That's my point here. Hence, we need to addm and enforce
> usage of, a power-domain-names property.

Majority of use cases will need just a single power domain. Until a real 
user of multiple power domains shows up along with a developer willing 
to extend Linux PM code to account for this, I'd keep this as is.

The one thing to make the binding more future-proof would be probably 
changing name of the property to "power-domains" for consistency with 
other bindings using *-names.

For now, the Linux power domain implementation would simply use the 
first available entry in "power-domains" property, but it could be 
extended in future with code to perform name-based look-up.

Best regards,
Tomasz

[PATCH v3 2/3] drivercore: Bind/unbind power domain on probe/remove

[Tomasz Figa]

On a number of platforms, devices are part of controllable power
domains, which need to be enabled before such devices can be accessed
and may be powered down when the device is idle to save some power.
This means that on systems that support power domain control using
generic power domains subsystem, it is necessary to add device to its
power domain before binding a driver to it and remove it from its power
domain after its driver is unbound to make sure that an unused device
does not affect power domain state.

Since this is not limited to particular busses and specific
archs/platforms, it is more convenient to do the above directly in
driver core, just as done with pinctrl default configuration. This patch
adds necessary code to really_probe() and __device_release_driver() to
achieve this and maintain consistent stack-like ordering of operations
happening when binding and unbinding a driver.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Reviewed-by: Philipp Zabel <philipp.zabel@gmail.com>
[on i.MX6 GK802]
Tested-by: Philipp Zabel <philipp.zabel@gmail.com>
Reviewed-by: Mark Brown <broonie@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/dd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 8986b9f..7501c42 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -23,6 +23,7 @@
 #include <linux/kthread.h>
 #include <linux/wait.h>
 #include <linux/async.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/pinctrl/devinfo.h>
 
@@ -273,6 +274,11 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 
 	dev->driver = drv;
 
+	/* If using genpd, bind power domain now before probing */
+	ret = genpd_bind_domain(dev);
+	if (ret)
+		goto probe_failed;
+
 	/* If using pinctrl, bind pins now before probing */
 	ret = pinctrl_bind_pins(dev);
 	if (ret)
@@ -303,6 +309,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 probe_failed:
 	devres_release_all(dev);
 	driver_sysfs_remove(dev);
+	genpd_unbind_domain(dev);
 	dev->driver = NULL;
 	dev_set_drvdata(dev, NULL);
 
@@ -513,7 +520,7 @@ static void __device_release_driver(struct device *dev)
 			blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 						     BUS_NOTIFY_UNBOUND_DRIVER,
 						     dev);
-
+		genpd_unbind_domain(dev);
 	}
 }
 
-- 
1.9.2

[PATCH v3 3/3] ARM: exynos: Move to generic power domain bindings

[Tomasz Figa]

This patch moves Exynos power domain code to use the new generic power
domain look-up framework introduced by previous patch, allowing the new
code to be compiled with CONFIG_ARCH_EXYNOS selected as well.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 .../bindings/arm/exynos/power_domain.txt           | 12 ++--
 arch/arm/mach-exynos/pm_domains.c                  | 81 +---------------------
 kernel/power/Kconfig                               |  2 +-
 3 files changed, 7 insertions(+), 88 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index 5216b41..60f26a8 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.
 
 Node of a device using power domains must have a samsung,power-domain property
 defined with a phandle to respective power domain.
@@ -17,12 +19,8 @@ Example:
 	lcd0: power-domain-lcd0 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023C00 0x10>;
+		#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 fe6570e..9cad3c6 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -73,89 +73,14 @@ 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;
 	struct device_node *np;
 
 	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
 		struct exynos_pm_domain *pd;
 		int on;
 
-		pdev = of_find_device_by_node(np);
-
 		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
 		if (!pd) {
 			pr_err("%s: failed to allocate memory for domain\n",
@@ -168,17 +93,13 @@ 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;
-
-		platform_set_drvdata(pdev, pd);
 
 		on = __raw_readl(pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN;
 
 		pm_genpd_init(&pd->pd, NULL, !on);
+		of_genpd_add_provider(np, of_genpd_xlate_simple, &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 45aa98e..b17588c 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -308,7 +308,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.2

Re: [PATCH v3 3/3] ARM: exynos: Move to generic power domain bindings

[Ulf Hansson]

On 23 April 2014 18:46, Tomasz Figa <t.figa@samsung.com> wrote:
> This patch moves Exynos power domain code to use the new generic power
> domain look-up framework introduced by previous patch, allowing the new
> code to be compiled with CONFIG_ARCH_EXYNOS selected as well.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>

Besides that I don't like new Kconfig CONFIG_PM_GENERIC_DOMAINS_OF,
this looks good to me.

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  .../bindings/arm/exynos/power_domain.txt           | 12 ++--
>  arch/arm/mach-exynos/pm_domains.c                  | 81 +---------------------
>  kernel/power/Kconfig                               |  2 +-
>  3 files changed, 7 insertions(+), 88 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> index 5216b41..60f26a8 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.
>
>  Node of a device using power domains must have a samsung,power-domain property
>  defined with a phandle to respective power domain.
> @@ -17,12 +19,8 @@ Example:
>         lcd0: power-domain-lcd0 {
>                 compatible = "samsung,exynos4210-pd";
>                 reg = <0x10023C00 0x10>;
> +               #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 fe6570e..9cad3c6 100644
> --- a/arch/arm/mach-exynos/pm_domains.c
> +++ b/arch/arm/mach-exynos/pm_domains.c
> @@ -73,89 +73,14 @@ 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;
>         struct device_node *np;
>
>         for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
>                 struct exynos_pm_domain *pd;
>                 int on;
>
> -               pdev = of_find_device_by_node(np);
> -
>                 pd = kzalloc(sizeof(*pd), GFP_KERNEL);
>                 if (!pd) {
>                         pr_err("%s: failed to allocate memory for domain\n",
> @@ -168,17 +93,13 @@ 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;
> -
> -               platform_set_drvdata(pdev, pd);
>
>                 on = __raw_readl(pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN;
>
>                 pm_genpd_init(&pd->pd, NULL, !on);
> +               of_genpd_add_provider(np, of_genpd_xlate_simple, &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 45aa98e..b17588c 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -308,7 +308,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.2
>