[PATCH v3 0/8] PM / Domains: DT support for domain idle states & atomic PM domains

[PATCH v3 0/8] PM / Domains: DT support for domain idle states & atomic PM domains

[Lina Iyer]

Hi all,

Changes since v2 [3] -
- Addressed review comments from v2.
	- domain-idle-states documentation updated
	- fixed compiler issues with imx driver
	- minor code change in pm_domains.c
- The series is available at [4].

Changes since v1 [2] -
- Addressed review comments from v1.
        - Fixes around dynamic allocation of genpd states
        - Used OF method for iterating phandles
        - Updated documentation, examples
        - Rename state variable (provider -> fwnode)
- The series is available at [3].

The changes from [1] are -
- Allocating memory for domain idle states dynamically
- Conform to naming conventions for internal and exported genpd functions
- DT binding example for domain-idle-state
- Use fwnode instead of of_node
- Handle atomic case for removal of PM Domain
- Rebase on top of Rafael's pm/genpd tree

Thanks,
Lina

Lina Iyer (8):
  PM / Domains: Make genpd state allocation dynamic
  PM / Domain: Add residency property to genpd states
  PM / Domains: Allow domain power states to be read from DT
  PM / Domains: Save the fwnode in genpd_power_state
  dt/bindings: Update binding for PM domain idle states
  PM / Domains: Abstract genpd locking
  PM / Domains: Support IRQ safe PM domains
  PM / doc: Update device documentation for devices in IRQ safe PM
    domains

 .../devicetree/bindings/power/power_domain.txt     |  43 +++
 Documentation/power/devices.txt                    |   9 +-
 arch/arm/mach-imx/gpc.c                            |  17 +-
 drivers/base/power/domain.c                        | 358 +++++++++++++++++----
 include/linux/pm_domain.h                          |  28 +-
 5 files changed, 383 insertions(+), 72 deletions(-)

-- 
2.7.4

[1]. https://www.spinics.net/lists/arm-kernel/msg526814.html
[2]. http://www.spinics.net/lists/arm-kernel/msg535106.html
[3]. https://git.linaro.org/people/lina.iyer/linux-next.git/shortlog/refs/heads/genpd-v2
[4]. https://git.linaro.org/people/lina.iyer/linux-next.git/shortlog/refs/heads/genpd-v3

[PATCH v3 1/8] PM / Domains: Make genpd state allocation dynamic

[Lina Iyer]

Allow PM Domain states to be defined dynamically by the drivers. This
removes the limitation on the maximum number of states possible for a
domain.

Cc: Axel Haslam <ahaslam+renesas@baylibre.com>
Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 arch/arm/mach-imx/gpc.c     | 17 ++++++++++-------
 drivers/base/power/domain.c | 35 +++++++++++++++++++++++------------
 include/linux/pm_domain.h   |  5 ++---
 3 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 0df062d..57a410b 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -380,13 +380,6 @@ static struct pu_domain imx6q_pu_domain = {
 		.name = "PU",
 		.power_off = imx6q_pm_pu_power_off,
 		.power_on = imx6q_pm_pu_power_on,
-		.states = {
-			[0] = {
-				.power_off_latency_ns = 25000,
-				.power_on_latency_ns = 2000000,
-			},
-		},
-		.state_count = 1,
 	},
 };
 
@@ -430,6 +423,16 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
 	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
 		return 0;
 
+	imx6q_pu_domain.base.states = devm_kzalloc(dev,
+					sizeof(*imx6q_pu_domain.base.states),
+					GFP_KERNEL);
+	if (!imx6q_pu_domain.base.states)
+		return -ENOMEM;
+
+	imx6q_pu_domain.base.states[0].power_off_latency_ns = 25000;
+	imx6q_pu_domain.base.states[0].power_on_latency_ns = 2000000;
+	imx6q_pu_domain.base.state_count = 1;
+
 	pm_genpd_init(&imx6q_pu_domain.base, NULL, false);
 	return of_genpd_add_provider_onecell(dev->of_node,
 					     &imx_gpc_onecell_data);
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e023066..37ab7f1 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1282,6 +1282,21 @@ out:
 }
 EXPORT_SYMBOL_GPL(pm_genpd_remove_subdomain);
 
+static int genpd_set_default_power_state(struct generic_pm_domain *genpd)
+{
+	struct genpd_power_state *state;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	genpd->states = state;
+	genpd->state_count = 1;
+	genpd->free = state;
+
+	return 0;
+}
+
 /**
  * pm_genpd_init - Initialize a generic I/O PM domain object.
  * @genpd: PM domain object to initialize.
@@ -1293,6 +1308,8 @@ EXPORT_SYMBOL_GPL(pm_genpd_remove_subdomain);
 int pm_genpd_init(struct generic_pm_domain *genpd,
 		  struct dev_power_governor *gov, bool is_off)
 {
+	int ret;
+
 	if (IS_ERR_OR_NULL(genpd))
 		return -EINVAL;
 
@@ -1325,19 +1342,12 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
 		genpd->dev_ops.start = pm_clk_resume;
 	}
 
-	if (genpd->state_idx >= GENPD_MAX_NUM_STATES) {
-		pr_warn("Initial state index out of bounds.\n");
-		genpd->state_idx = GENPD_MAX_NUM_STATES - 1;
-	}
-
-	if (genpd->state_count > GENPD_MAX_NUM_STATES) {
-		pr_warn("Limiting states to  %d\n", GENPD_MAX_NUM_STATES);
-		genpd->state_count = GENPD_MAX_NUM_STATES;
-	}
-
 	/* Use only one "off" state if there were no states declared */
-	if (genpd->state_count == 0)
-		genpd->state_count = 1;
+	if (genpd->state_count == 0) {
+		ret = genpd_set_default_power_state(genpd);
+		if (ret)
+			return ret;
+	}
 
 	mutex_lock(&gpd_list_lock);
 	list_add(&genpd->gpd_list_node, &gpd_list);
@@ -1377,6 +1387,7 @@ static int genpd_remove(struct generic_pm_domain *genpd)
 	list_del(&genpd->gpd_list_node);
 	mutex_unlock(&genpd->lock);
 	cancel_work_sync(&genpd->power_off_work);
+	kfree(genpd->free);
 	pr_debug("%s: removed %s\n", __func__, genpd->name);
 
 	return 0;
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index a09fe5c..de1d8f3 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -19,8 +19,6 @@
 /* Defines used for the flags field in the struct generic_pm_domain */
 #define GENPD_FLAG_PM_CLK	(1U << 0) /* PM domain uses PM clk */
 
-#define GENPD_MAX_NUM_STATES	8 /* Number of possible low power states */
-
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
 	GPD_STATE_POWER_OFF,	/* PM domain is off */
@@ -70,9 +68,10 @@ struct generic_pm_domain {
 	void (*detach_dev)(struct generic_pm_domain *domain,
 			   struct device *dev);
 	unsigned int flags;		/* Bit field of configs for genpd */
-	struct genpd_power_state states[GENPD_MAX_NUM_STATES];
+	struct genpd_power_state *states;
 	unsigned int state_count; /* number of states */
 	unsigned int state_idx; /* state that genpd will go to when off */
+	void *free; /* Free the state that was allocated for default */
 
 };
 
-- 
2.7.4

[PATCH v3 2/8] PM / Domain: Add residency property to genpd states

[Lina Iyer]

Residency of a domain's idle state indicates that the minimum idle time
for the domain's idle state to be beneficial for power. Add the
parameter to the state node. Future patches, will use the residency
value in the genpd governor to determine if it is worth while to enter
an idle state.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 include/linux/pm_domain.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index de1d8f3..f4492eb 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -38,6 +38,7 @@ struct gpd_dev_ops {
 struct genpd_power_state {
 	s64 power_off_latency_ns;
 	s64 power_on_latency_ns;
+	s64 residency_ns;
 };
 
 struct generic_pm_domain {
-- 
2.7.4

[PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT

[Lina Iyer]

This patch allows domains to define idle states in the DT. SoC's can
define domain idle states in DT using the "domain-idle-states" property
of the domain provider. Add API to read the idle states from DT that can
be set in the genpd object.

This patch is based on the original patch by Marc Titinger.

Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/base/power/domain.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   |  8 ++++
 2 files changed, 102 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 37ab7f1..9af75ba 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1916,6 +1916,100 @@ out:
 	return ret ? -EPROBE_DEFER : 0;
 }
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
+
+static const struct of_device_id idle_state_match[] = {
+	{ .compatible = "arm,idle-state", },
+	{ }
+};
+
+static int genpd_parse_state(struct genpd_power_state *genpd_state,
+				    struct device_node *state_node)
+{
+	int err;
+	u32 residency;
+	u32 entry_latency, exit_latency;
+	const struct of_device_id *match_id;
+
+	match_id = of_match_node(idle_state_match, state_node);
+	if (!match_id)
+		return -EINVAL;
+
+	err = of_property_read_u32(state_node, "entry-latency-us",
+						&entry_latency);
+	if (err) {
+		pr_debug(" * %s missing entry-latency-us property\n",
+						state_node->full_name);
+		return -EINVAL;
+	}
+
+	err = of_property_read_u32(state_node, "exit-latency-us",
+						&exit_latency);
+	if (err) {
+		pr_debug(" * %s missing exit-latency-us property\n",
+						state_node->full_name);
+		return -EINVAL;
+	}
+
+	err = of_property_read_u32(state_node, "min-residency-us", &residency);
+	if (!err)
+		genpd_state->residency_ns = 1000 * residency;
+
+	genpd_state->power_on_latency_ns = 1000 * exit_latency;
+	genpd_state->power_off_latency_ns = 1000 * entry_latency;
+
+	return 0;
+}
+
+/**
+ * of_genpd_parse_idle_states: Return array of idle states for the genpd.
+ *
+ * @dn: The genpd device node
+ * @states: The pointer to which the state array will be saved.
+ * @n: The count of elements in the array returned from this function.
+ *
+ * Returns the device states parsed from the OF node. The memory for the states
+ * is allocated by this function and is the responsibility of the caller to
+ * free the memory after use.
+ */
+int of_genpd_parse_idle_states(struct device_node *dn,
+			struct genpd_power_state **states, int *n)
+{
+	struct genpd_power_state *st;
+	struct device_node *np;
+	int i = 0;
+	int err, ret;
+	int count;
+	struct of_phandle_iterator it;
+
+	count = of_count_phandle_with_args(dn, "domain-idle-states", NULL);
+	if (!count)
+		return -EINVAL;
+
+	st = kcalloc(count, sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return -ENOMEM;
+
+	/* Loop over the phandles until all the requested entry is found */
+	of_for_each_phandle(&it, err, dn, "domain-idle-states", NULL, 0) {
+		np = it.node;
+		ret = genpd_parse_state(&st[i++], np);
+		if (ret) {
+			pr_err
+			("Parsing idle state node %s failed with err %d\n",
+							np->full_name, ret);
+			of_node_put(np);
+			kfree(st);
+			return ret;
+		}
+	}
+
+	*n = count;
+	*states = st;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
+
 #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f4492eb..b489496 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -205,6 +205,8 @@ extern int of_genpd_add_device(struct of_phandle_args *args,
 extern int of_genpd_add_subdomain(struct of_phandle_args *parent,
 				  struct of_phandle_args *new_subdomain);
 extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
+extern int of_genpd_parse_idle_states(struct device_node *dn,
+			struct genpd_power_state **states, int *n);
 
 int genpd_dev_pm_attach(struct device *dev);
 #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
@@ -234,6 +236,12 @@ static inline int of_genpd_add_subdomain(struct of_phandle_args *parent,
 	return -ENODEV;
 }
 
+static inline int of_genpd_parse_idle_states(struct device_node *dn,
+			struct genpd_power_state **states, int *n)
+{
+	return -ENODEV;
+}
+
 static inline int genpd_dev_pm_attach(struct device *dev)
 {
 	return -ENODEV;
-- 
2.7.4

[PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT

[Sudeep Holla]

On 14/10/16 18:47, Lina Iyer wrote:
> This patch allows domains to define idle states in the DT. SoC's can
> define domain idle states in DT using the "domain-idle-states" property
> of the domain provider. Add API to read the idle states from DT that can
> be set in the genpd object.
>
> This patch is based on the original patch by Marc Titinger.
>
> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  drivers/base/power/domain.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pm_domain.h   |  8 ++++
>  2 files changed, 102 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 37ab7f1..9af75ba 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1916,6 +1916,100 @@ out:
>  	return ret ? -EPROBE_DEFER : 0;
>  }
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
> +
> +static const struct of_device_id idle_state_match[] = {
> +	{ .compatible = "arm,idle-state", },
> +	{ }
> +};
> +

I still think it's better to have another compatible to serve this
purpose. We don't want to end up creating genpd domains just because
they are "arm,idle-state" compatible IMO ?

I agree you can prevent it checking for OSC mode support in the
firmware. But I want to understand if you have any strong reasons for
avoiding that approach.

-- 
Regards,
Sudeep

[PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT

[Lina Iyer]

Hi Sudeep,

On Mon, Oct 24 2016 at 07:39 -0600, Sudeep Holla wrote:
>
>
>On 14/10/16 18:47, Lina Iyer wrote:
>>This patch allows domains to define idle states in the DT. SoC's can
>>define domain idle states in DT using the "domain-idle-states" property
>>of the domain provider. Add API to read the idle states from DT that can
>>be set in the genpd object.
>>
>>This patch is based on the original patch by Marc Titinger.
>>
>>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>---
>> drivers/base/power/domain.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/pm_domain.h   |  8 ++++
>> 2 files changed, 102 insertions(+)
>>
>>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>index 37ab7f1..9af75ba 100644
>>--- a/drivers/base/power/domain.c
>>+++ b/drivers/base/power/domain.c
>>@@ -1916,6 +1916,100 @@ out:
>> 	return ret ? -EPROBE_DEFER : 0;
>> }
>> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>+
>>+static const struct of_device_id idle_state_match[] = {
>>+	{ .compatible = "arm,idle-state", },
>>+	{ }
>>+};
>>+
>
>I still think it's better to have another compatible to serve this
>purpose. We don't want to end up creating genpd domains just because
>they are "arm,idle-state" compatible IMO ?
>
>I agree you can prevent it checking for OSC mode support in the
>firmware. But I want to understand if you have any strong reasons for
>avoiding that approach.
>
Why are you still held up with OSI/PC PSCI modes? I repeat again this
series is not about any of that, it is just about PM domains. PM domains
have idle states and the idle-state description is similar in definition
to arm,idle-state and therefore uses the same compatible. There is no
point re-defining something that already exists in the kernel.

I was able to find the original thread, where we discussed this [1].

I suggest, you read about PM domains and its idle states and understand
this series in the context of PM domains.

Thanks,
Lina

[1]. http://www.serverphorums.com/read.php?12,1303996

[PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT

[Sudeep Holla]

On 24/10/16 17:48, Lina Iyer wrote:
> Hi Sudeep,
>
> On Mon, Oct 24 2016 at 07:39 -0600, Sudeep Holla wrote:
>>
>>
>> On 14/10/16 18:47, Lina Iyer wrote:
>>> This patch allows domains to define idle states in the DT. SoC's can
>>> define domain idle states in DT using the "domain-idle-states" property
>>> of the domain provider. Add API to read the idle states from DT that can
>>> be set in the genpd object.
>>>
>>> This patch is based on the original patch by Marc Titinger.
>>>
>>> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>> ---
>>> drivers/base/power/domain.c | 94
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/pm_domain.h   |  8 ++++
>>> 2 files changed, 102 insertions(+)
>>>
>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>> index 37ab7f1..9af75ba 100644
>>> --- a/drivers/base/power/domain.c
>>> +++ b/drivers/base/power/domain.c
>>> @@ -1916,6 +1916,100 @@ out:
>>>     return ret ? -EPROBE_DEFER : 0;
>>> }
>>> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>> +
>>> +static const struct of_device_id idle_state_match[] = {
>>> +    { .compatible = "arm,idle-state", },
>>> +    { }
>>> +};
>>> +
>>
>> I still think it's better to have another compatible to serve this
>> purpose. We don't want to end up creating genpd domains just because
>> they are "arm,idle-state" compatible IMO ?
>>
>> I agree you can prevent it checking for OSC mode support in the
>> firmware. But I want to understand if you have any strong reasons for
>> avoiding that approach.
>>
> Why are you still held up with OSI/PC PSCI modes?

I am just pointing that out to make sure you are not defining these
DT bindings with just QCOM platform and OSC in consideration.

I am thinking about how can it be used/extended in other use-cases.

> I repeat again this
> series is not about any of that, it is just about PM domains.

I completely understand that, no argument on that. What I worry is that
if a system(in future) represents this power domains and domain idles
states and doesn't want to create a genpd, do we have to handle it
differently or even the way your CPUidle series would do or can we say
those states need to specify that they are compatible with the new
feature(the one being added in this series) with a new compatible.
I prefer the latter than the former to avoid all possible workarounds
in future.

> PM domains
> have idle states and the idle-state description is similar in definition
> to arm,idle-state and therefore uses the same compatible. There is no
> point re-defining something that already exists in the kernel.
>

Yes I understand that but "arm,idle-states" were not defined with this
feature in mind and hence I am bit worried if it could cause any issue
especially if deprecate cpu-idle-states and move to this model
completely. I really don't like this mix and hence I am raising the
concern here. I am trying to ease that migration.

> I was able to find the original thread, where we discussed this [1].
>
> I suggest, you read about PM domains and its idle states and understand
> this series in the context of PM domains.
>

Sure, will do that. Thanks for pointing that out. But the concern I am
raising is entirely different. I am asking if this re-use will cause any
issue in future as pointed out above.

I re-iterate that I understand this series is independent of the CPUIdle
and hence asking why not make it completely independent by just adding
the new compatible.

I am *not asking to redefine something completely*. What I am saying is
*just* to add new compatible that may(for cpu devices)/may not(for
other/non-CPU devices) be used along with "arm,idle-state".

I may be too paranoid here but I think that's safer. It helps to skip
creating of genpd if required for some domains as I had explained before.

-- 
Regards,
Sudeep

[PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT

[Lina Iyer]

On Mon, Oct 24 2016 at 11:27 -0600, Sudeep Holla wrote:
>
>
>On 24/10/16 17:48, Lina Iyer wrote:
>>Hi Sudeep,
>>
>>On Mon, Oct 24 2016 at 07:39 -0600, Sudeep Holla wrote:
>>>
>>>
>>>On 14/10/16 18:47, Lina Iyer wrote:
>>>>This patch allows domains to define idle states in the DT. SoC's can
>>>>define domain idle states in DT using the "domain-idle-states" property
>>>>of the domain provider. Add API to read the idle states from DT that can
>>>>be set in the genpd object.
>>>>
>>>>This patch is based on the original patch by Marc Titinger.
>>>>
>>>>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>>>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>>>---
>>>>drivers/base/power/domain.c | 94
>>>>+++++++++++++++++++++++++++++++++++++++++++++
>>>>include/linux/pm_domain.h   |  8 ++++
>>>>2 files changed, 102 insertions(+)
>>>>
>>>>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>>>index 37ab7f1..9af75ba 100644
>>>>--- a/drivers/base/power/domain.c
>>>>+++ b/drivers/base/power/domain.c
>>>>@@ -1916,6 +1916,100 @@ out:
>>>>    return ret ? -EPROBE_DEFER : 0;
>>>>}
>>>>EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>>>+
>>>>+static const struct of_device_id idle_state_match[] = {
>>>>+    { .compatible = "arm,idle-state", },
>>>>+    { }
>>>>+};
>>>>+
>>>
>>>I still think it's better to have another compatible to serve this
>>>purpose. We don't want to end up creating genpd domains just because
>>>they are "arm,idle-state" compatible IMO ?
>>>
>>>I agree you can prevent it checking for OSC mode support in the
>>>firmware. But I want to understand if you have any strong reasons for
>>>avoiding that approach.
>>>
>>Why are you still held up with OSI/PC PSCI modes?
>
>I am just pointing that out to make sure you are not defining these
>DT bindings with just QCOM platform and OSC in consideration.
>
>I am thinking about how can it be used/extended in other use-cases.
>
>>I repeat again this
>>series is not about any of that, it is just about PM domains.
>
>I completely understand that, no argument on that. What I worry is that
>if a system(in future) represents this power domains and domain idles
>states and doesn't want to create a genpd, do we have to handle it
>differently or even the way your CPUidle series would do or can we say
>those states need to specify that they are compatible with the new
>feature(the one being added in this series) with a new compatible.
>I prefer the latter than the former to avoid all possible workarounds
>in future.
>
>>PM domains
>>have idle states and the idle-state description is similar in definition
>>to arm,idle-state and therefore uses the same compatible. There is no
>>point re-defining something that already exists in the kernel.
>>
>
>Yes I understand that but "arm,idle-states" were not defined with this
>feature in mind and hence I am bit worried if it could cause any issue
>especially if deprecate cpu-idle-states and move to this model
>completely. I really don't like this mix and hence I am raising the
>concern here. I am trying to ease that migration.
>
>>I was able to find the original thread, where we discussed this [1].
>>
>>I suggest, you read about PM domains and its idle states and understand
>>this series in the context of PM domains.
>>
>
>Sure, will do that. Thanks for pointing that out. But the concern I am
>raising is entirely different. I am asking if this re-use will cause any
>issue in future as pointed out above.
>
>I re-iterate that I understand this series is independent of the CPUIdle
>and hence asking why not make it completely independent by just adding
>the new compatible.
>
>I am *not asking to redefine something completely*. What I am saying is
>*just* to add new compatible that may(for cpu devices)/may not(for
>other/non-CPU devices) be used along with "arm,idle-state".
>
I am fine with that, as long as the compatible string is an alternate
for "arm,idle-state" it shouldn't be a problem.

Any recommendations?

Thanks,
Lina

>I may be too paranoid here but I think that's safer. It helps to skip
>creating of genpd if required for some domains as I had explained before.
>
>-- 
>Regards,
>Sudeep

[PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT

[Sudeep Holla]

On 24/10/16 17:48, Lina Iyer wrote:
> Hi Sudeep,

[..]

>
> I was able to find the original thread, where we discussed this [1].
>
> [1]. http://www.serverphorums.com/read.php?12,1303996
>

I went through the discussion and that's valid discussion and I agree.
But what am looking for is backward compatibility and not exactly same
as the discussion and I agree that can be solved in different way.

I just feel that's one easily way to solve such issues. I am open to
suggestions.

-- 
Regards,
Sudeep

[PATCH v3 4/8] PM / Domains: Save the fwnode in genpd_power_state

[Lina Iyer]

Save the fwnode for the genpd state in the state node. PM Domain clients
may use the fwnode to read in the platform specific domain state
properties and associate them with the state.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 1 +
 include/linux/pm_domain.h   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 9af75ba..1a6073aa 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1956,6 +1956,7 @@ static int genpd_parse_state(struct genpd_power_state *genpd_state,
 
 	genpd_state->power_on_latency_ns = 1000 * exit_latency;
 	genpd_state->power_off_latency_ns = 1000 * entry_latency;
+	genpd_state->fwnode = &state_node->fwnode;
 
 	return 0;
 }
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index b489496..6a89881 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -39,6 +39,7 @@ struct genpd_power_state {
 	s64 power_off_latency_ns;
 	s64 power_on_latency_ns;
 	s64 residency_ns;
+	struct fwnode_handle *fwnode;
 };
 
 struct generic_pm_domain {
-- 
2.7.4

[PATCH v3 5/8] dt/bindings: Update binding for PM domain idle states

[Lina Iyer]

Update DT bindings to describe idle states of PM domains.

This patch is based on the original patch by Marc Titinger.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/power/power_domain.txt     | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 025b5e7..e165036 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -29,6 +29,15 @@ Optional properties:
    specified by this binding. More details about power domain specifier are
    available in the next section.
 
+- domain-idle-states : A phandle of an idle-state that shall be soaked into a
+                generic domain power state. The idle state definitions are
+                compatible with arm,idle-state specified in [1].
+  The domain-idle-state property reflects the idle state of this PM domain and
+  not the idle states of the devices or sub-domains in the PM domain. Devices
+  and sub-domains have their own idle-states independent of the parent
+  domain's idle states. In the absence of this property, the domain would be
+  considered as capable of being powered-on or powered-off.
+
 Example:
 
 	power: power-controller at 12340000 {
@@ -59,6 +68,38 @@ The nodes above define two power controllers: 'parent' and 'child'.
 Domains created by the 'child' power controller are subdomains of '0' power
 domain provided by the 'parent' power controller.
 
+Example 3:
+	parent: power-controller at 12340000 {
+		compatible = "foo,power-controller";
+		reg = <0x12340000 0x1000>;
+		#power-domain-cells = <0>;
+		domain-idle-states = <&DOMAIN_RET>, <&DOMAIN_PWR_DN>;
+	};
+
+	child: power-controller at 12341000 {
+		compatible = "foo,power-controller";
+		reg = <0x12341000 0x1000>;
+		power-domains = <&parent 0>;
+		#power-domain-cells = <0>;
+		domain-idle-states = <&DOMAIN_PWR_DN>;
+	};
+
+	DOMAIN_RET: state at 0 {
+		compatible = "arm,idle-state";
+		reg = <0x0>;
+		entry-latency-us = <1000>;
+		exit-latency-us = <2000>;
+		min-residency-us = <10000>;
+	};
+
+	DOMAIN_PWR_DN: state at 1 {
+		compatible = "arm,idle-state";
+		reg = <0x1>;
+		entry-latency-us = <5000>;
+		exit-latency-us = <8000>;
+		min-residency-us = <7000>;
+	};
+
 ==PM domain consumers==
 
 Required properties:
@@ -76,3 +117,5 @@ Example:
 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".
+
+[1]. Documentation/devicetree/bindings/arm/idle-states.txt
-- 
2.7.4

[PATCH v3 6/8] PM / Domains: Abstract genpd locking

[Lina Iyer]

Abstract genpd lock/unlock calls, in preparation for domain specific
locks added in the following patches.

Cc: Kevin Hilman <khilman@kernel.org>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 121 +++++++++++++++++++++++++++++---------------
 include/linux/pm_domain.h   |   5 +-
 2 files changed, 85 insertions(+), 41 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 1a6073aa..4194012 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -39,6 +39,46 @@
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
+struct genpd_lock_ops {
+	void (*lock)(struct generic_pm_domain *genpd);
+	void (*lock_nested)(struct generic_pm_domain *genpd, int depth);
+	int (*lock_interruptible)(struct generic_pm_domain *genpd);
+	void (*unlock)(struct generic_pm_domain *genpd);
+};
+
+static void genpd_lock_mtx(struct generic_pm_domain *genpd)
+{
+	mutex_lock(&genpd->mlock);
+}
+
+static void genpd_lock_nested_mtx(struct generic_pm_domain *genpd,
+					int depth)
+{
+	mutex_lock_nested(&genpd->mlock, depth);
+}
+
+static int genpd_lock_interruptible_mtx(struct generic_pm_domain *genpd)
+{
+	return mutex_lock_interruptible(&genpd->mlock);
+}
+
+static void genpd_unlock_mtx(struct generic_pm_domain *genpd)
+{
+	return mutex_unlock(&genpd->mlock);
+}
+
+static const struct genpd_lock_ops genpd_mtx_ops = {
+	.lock = genpd_lock_mtx,
+	.lock_nested = genpd_lock_nested_mtx,
+	.lock_interruptible = genpd_lock_interruptible_mtx,
+	.unlock = genpd_unlock_mtx,
+};
+
+#define genpd_lock(p)			p->lock_ops->lock(p)
+#define genpd_lock_nested(p, d)		p->lock_ops->lock_nested(p, d)
+#define genpd_lock_interruptible(p)	p->lock_ops->lock_interruptible(p)
+#define genpd_unlock(p)			p->lock_ops->unlock(p)
+
 /*
  * Get the generic PM domain for a particular struct device.
  * This validates the struct device pointer, the PM domain pointer,
@@ -200,9 +240,9 @@ static int genpd_poweron(struct generic_pm_domain *genpd, unsigned int depth)
 
 		genpd_sd_counter_inc(master);
 
-		mutex_lock_nested(&master->lock, depth + 1);
+		genpd_lock_nested(master, depth + 1);
 		ret = genpd_poweron(master, depth + 1);
-		mutex_unlock(&master->lock);
+		genpd_unlock(master);
 
 		if (ret) {
 			genpd_sd_counter_dec(master);
@@ -255,9 +295,9 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
 		spin_unlock_irq(&dev->power.lock);
 
 		if (!IS_ERR(genpd)) {
-			mutex_lock(&genpd->lock);
+			genpd_lock(genpd);
 			genpd->max_off_time_changed = true;
-			mutex_unlock(&genpd->lock);
+			genpd_unlock(genpd);
 		}
 
 		dev = dev->parent;
@@ -354,9 +394,9 @@ static void genpd_power_off_work_fn(struct work_struct *work)
 
 	genpd = container_of(work, struct generic_pm_domain, power_off_work);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	genpd_poweroff(genpd, true);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 }
 
 /**
@@ -472,9 +512,9 @@ static int genpd_runtime_suspend(struct device *dev)
 	if (dev->power.irq_safe)
 		return 0;
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	genpd_poweroff(genpd, false);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	return 0;
 }
@@ -509,9 +549,9 @@ static int genpd_runtime_resume(struct device *dev)
 		goto out;
 	}
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	ret = genpd_poweron(genpd, 0);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (ret)
 		return ret;
@@ -547,9 +587,9 @@ err_stop:
 	genpd_stop_dev(genpd, dev);
 err_poweroff:
 	if (!dev->power.irq_safe) {
-		mutex_lock(&genpd->lock);
+		genpd_lock(genpd);
 		genpd_poweroff(genpd, 0);
-		mutex_unlock(&genpd->lock);
+		genpd_unlock(genpd);
 	}
 
 	return ret;
@@ -732,20 +772,20 @@ static int pm_genpd_prepare(struct device *dev)
 	if (resume_needed(dev, genpd))
 		pm_runtime_resume(dev);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count++ == 0)
 		genpd->suspended_count = 0;
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	ret = pm_generic_prepare(dev);
 	if (ret) {
-		mutex_lock(&genpd->lock);
+		genpd_lock(genpd);
 
 		genpd->prepared_count--;
 
-		mutex_unlock(&genpd->lock);
+		genpd_unlock(genpd);
 	}
 
 	return ret;
@@ -936,13 +976,13 @@ static void pm_genpd_complete(struct device *dev)
 
 	pm_generic_complete(dev);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	genpd->prepared_count--;
 	if (!genpd->prepared_count)
 		genpd_queue_power_off_work(genpd);
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 }
 
 /**
@@ -1071,7 +1111,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;
@@ -1088,7 +1128,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (ret)
 		genpd_free_dev_data(dev, gpd_data);
@@ -1130,7 +1170,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 	gpd_data = to_gpd_data(pdd);
 	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;
@@ -1145,14 +1185,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 
 	list_del_init(&pdd->list_node);
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	genpd_free_dev_data(dev, gpd_data);
 
 	return 0;
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 	dev_pm_qos_add_notifier(dev, &gpd_data->nb);
 
 	return ret;
@@ -1187,8 +1227,8 @@ static int genpd_add_subdomain(struct generic_pm_domain *genpd,
 	if (!link)
 		return -ENOMEM;
 
-	mutex_lock(&subdomain->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	genpd_lock(subdomain);
+	genpd_lock_nested(genpd, SINGLE_DEPTH_NESTING);
 
 	if (genpd->status == GPD_STATE_POWER_OFF
 	    &&  subdomain->status != GPD_STATE_POWER_OFF) {
@@ -1211,8 +1251,8 @@ static int genpd_add_subdomain(struct generic_pm_domain *genpd,
 		genpd_sd_counter_inc(genpd);
 
  out:
-	mutex_unlock(&genpd->lock);
-	mutex_unlock(&subdomain->lock);
+	genpd_unlock(genpd);
+	genpd_unlock(subdomain);
 	if (ret)
 		kfree(link);
 	return ret;
@@ -1250,8 +1290,8 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(subdomain))
 		return -EINVAL;
 
-	mutex_lock(&subdomain->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	genpd_lock(subdomain);
+	genpd_lock_nested(genpd, SINGLE_DEPTH_NESTING);
 
 	if (!list_empty(&subdomain->master_links) || subdomain->device_count) {
 		pr_warn("%s: unable to remove subdomain %s\n", genpd->name,
@@ -1275,8 +1315,8 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 	}
 
 out:
-	mutex_unlock(&genpd->lock);
-	mutex_unlock(&subdomain->lock);
+	genpd_unlock(genpd);
+	genpd_unlock(subdomain);
 
 	return ret;
 }
@@ -1316,7 +1356,8 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
 	INIT_LIST_HEAD(&genpd->master_links);
 	INIT_LIST_HEAD(&genpd->slave_links);
 	INIT_LIST_HEAD(&genpd->dev_list);
-	mutex_init(&genpd->lock);
+	mutex_init(&genpd->mlock);
+	genpd->lock_ops = &genpd_mtx_ops;
 	genpd->gov = gov;
 	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
 	atomic_set(&genpd->sd_count, 0);
@@ -1364,16 +1405,16 @@ static int genpd_remove(struct generic_pm_domain *genpd)
 	if (IS_ERR_OR_NULL(genpd))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->has_provider) {
-		mutex_unlock(&genpd->lock);
+		genpd_unlock(genpd);
 		pr_err("Provider present, unable to remove %s\n", genpd->name);
 		return -EBUSY;
 	}
 
 	if (!list_empty(&genpd->master_links) || genpd->device_count) {
-		mutex_unlock(&genpd->lock);
+		genpd_unlock(genpd);
 		pr_err("%s: unable to remove %s\n", __func__, genpd->name);
 		return -EBUSY;
 	}
@@ -1385,7 +1426,7 @@ static int genpd_remove(struct generic_pm_domain *genpd)
 	}
 
 	list_del(&genpd->gpd_list_node);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 	cancel_work_sync(&genpd->power_off_work);
 	kfree(genpd->free);
 	pr_debug("%s: removed %s\n", __func__, genpd->name);
@@ -1909,9 +1950,9 @@ int genpd_dev_pm_attach(struct device *dev)
 	dev->pm_domain->detach = genpd_dev_pm_detach;
 	dev->pm_domain->sync = genpd_dev_pm_sync;
 
-	mutex_lock(&pd->lock);
+	genpd_lock(pd);
 	ret = genpd_poweron(pd, 0);
-	mutex_unlock(&pd->lock);
+	genpd_unlock(pd);
 out:
 	return ret ? -EPROBE_DEFER : 0;
 }
@@ -2064,7 +2105,7 @@ static int pm_genpd_summary_one(struct seq_file *s,
 	char state[16];
 	int ret;
 
-	ret = mutex_lock_interruptible(&genpd->lock);
+	ret = genpd_lock_interruptible(genpd);
 	if (ret)
 		return -ERESTARTSYS;
 
@@ -2101,7 +2142,7 @@ static int pm_genpd_summary_one(struct seq_file *s,
 
 	seq_puts(s, "\n");
 exit:
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	return 0;
 }
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 6a89881..811b968 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -42,13 +42,14 @@ struct genpd_power_state {
 	struct fwnode_handle *fwnode;
 };
 
+struct genpd_lock_ops;
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */
 	struct list_head master_links;	/* Links with PM domain as a master */
 	struct list_head slave_links;	/* Links with PM domain as a slave */
 	struct list_head dev_list;	/* List of devices */
-	struct mutex lock;
 	struct dev_power_governor *gov;
 	struct work_struct power_off_work;
 	struct fwnode_handle *provider;	/* Identity of the domain provider */
@@ -74,6 +75,8 @@ struct generic_pm_domain {
 	unsigned int state_count; /* number of states */
 	unsigned int state_idx; /* state that genpd will go to when off */
 	void *free; /* Free the state that was allocated for default */
+	const struct genpd_lock_ops *lock_ops;
+	struct mutex mlock;
 
 };
 
-- 
2.7.4

[PATCH v3 7/8] PM / Domains: Support IRQ safe PM domains

[Lina Iyer]

Generic Power Domains currently support turning on/off only in process
context. This prevents the usage of PM domains for domains that could be
powered on/off in a context where IRQs are disabled. Many such domains
exist today and do not get powered off, when the IRQ safe devices in
that domain are powered off, because of this limitation.

However, not all domains can operate in IRQ safe contexts. Genpd
therefore, has to support both cases where the domain may or may not
operate in IRQ safe contexts. Configuring genpd to use an appropriate
lock for that domain, would allow domains that have IRQ safe devices to
runtime suspend and resume, in atomic context.

To achieve domain specific locking, set the domain's ->flag to
GENPD_FLAG_IRQ_SAFE while defining the domain. This indicates that genpd
should use a spinlock instead of a mutex for locking the domain. Locking
is abstracted through genpd_lock() and genpd_unlock() functions that use
the flag to determine the appropriate lock to be used for that domain.

Domains that have lower latency to suspend and resume and can operate
with IRQs disabled may now be able to save power, when the component
devices and sub-domains are idle at runtime.

The restriction this imposes on the domain hierarchy is that non-IRQ
safe domains may not have IRQ-safe subdomains, but IRQ safe domains may
have IRQ safe and non-IRQ safe subdomains and devices.

Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 111 ++++++++++++++++++++++++++++++++++++++++----
 include/linux/pm_domain.h   |  10 +++-
 2 files changed, 110 insertions(+), 11 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 4194012..aac656a 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -74,11 +74,70 @@ static const struct genpd_lock_ops genpd_mtx_ops = {
 	.unlock = genpd_unlock_mtx,
 };
 
+static void genpd_lock_spin(struct generic_pm_domain *genpd)
+	__acquires(&genpd->slock)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&genpd->slock, flags);
+	genpd->lock_flags = flags;
+}
+
+static void genpd_lock_nested_spin(struct generic_pm_domain *genpd,
+					int depth)
+	__acquires(&genpd->slock)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave_nested(&genpd->slock, flags, depth);
+	genpd->lock_flags = flags;
+}
+
+static int genpd_lock_interruptible_spin(struct generic_pm_domain *genpd)
+	__acquires(&genpd->slock)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&genpd->slock, flags);
+	genpd->lock_flags = flags;
+	return 0;
+}
+
+static void genpd_unlock_spin(struct generic_pm_domain *genpd)
+	__releases(&genpd->slock)
+{
+	spin_unlock_irqrestore(&genpd->slock, genpd->lock_flags);
+}
+
+static const struct genpd_lock_ops genpd_spin_ops = {
+	.lock = genpd_lock_spin,
+	.lock_nested = genpd_lock_nested_spin,
+	.lock_interruptible = genpd_lock_interruptible_spin,
+	.unlock = genpd_unlock_spin,
+};
+
 #define genpd_lock(p)			p->lock_ops->lock(p)
 #define genpd_lock_nested(p, d)		p->lock_ops->lock_nested(p, d)
 #define genpd_lock_interruptible(p)	p->lock_ops->lock_interruptible(p)
 #define genpd_unlock(p)			p->lock_ops->unlock(p)
 
+#define genpd_is_irq_safe(genpd)	(genpd->flags & GENPD_FLAG_IRQ_SAFE)
+
+static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
+		struct generic_pm_domain *genpd)
+{
+	bool ret;
+
+	ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd);
+
+	/* Warn once for each IRQ safe dev in no sleep domain */
+	if (ret)
+		dev_warn_once(dev, "PM domain %s will not be powered off\n",
+				genpd->name);
+
+	return ret;
+}
+
 /*
  * Get the generic PM domain for a particular struct device.
  * This validates the struct device pointer, the PM domain pointer,
@@ -343,7 +402,12 @@ static int genpd_poweroff(struct generic_pm_domain *genpd, bool is_async)
 		if (stat > PM_QOS_FLAGS_NONE)
 			return -EBUSY;
 
-		if (!pm_runtime_suspended(pdd->dev) || pdd->dev->power.irq_safe)
+		/*
+		 * Do not allow PM domain to be powered off, when an IRQ safe
+		 * device is part of a non-IRQ safe domain.
+		 */
+		if (!pm_runtime_suspended(pdd->dev) ||
+			irq_safe_dev_in_no_sleep_domain(pdd->dev, genpd))
 			not_suspended++;
 	}
 
@@ -506,10 +570,10 @@ static int genpd_runtime_suspend(struct device *dev)
 	}
 
 	/*
-	 * If power.irq_safe is set, this routine will be run with interrupts
-	 * off, so it can't use mutexes.
+	 * If power.irq_safe is set, this routine may be run with
+	 * IRQs disabled, so suspend only if the PM domain also is irq_safe.
 	 */
-	if (dev->power.irq_safe)
+	if (irq_safe_dev_in_no_sleep_domain(dev, genpd))
 		return 0;
 
 	genpd_lock(genpd);
@@ -543,8 +607,11 @@ static int genpd_runtime_resume(struct device *dev)
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	/* If power.irq_safe, the PM domain is never powered off. */
-	if (dev->power.irq_safe) {
+	/*
+	 * As we don't power off a non IRQ safe domain, which holds
+	 * an IRQ safe device, we don't need to restore power to it.
+	 */
+	if (irq_safe_dev_in_no_sleep_domain(dev, genpd)) {
 		timed = false;
 		goto out;
 	}
@@ -586,7 +653,8 @@ static int genpd_runtime_resume(struct device *dev)
 err_stop:
 	genpd_stop_dev(genpd, dev);
 err_poweroff:
-	if (!dev->power.irq_safe) {
+	if (!pm_runtime_is_irq_safe(dev) ||
+		(pm_runtime_is_irq_safe(dev) && genpd_is_irq_safe(genpd))) {
 		genpd_lock(genpd);
 		genpd_poweroff(genpd, 0);
 		genpd_unlock(genpd);
@@ -1223,6 +1291,17 @@ static int genpd_add_subdomain(struct generic_pm_domain *genpd,
 	    || genpd == subdomain)
 		return -EINVAL;
 
+	/*
+	 * If the domain can be powered on/off in an IRQ safe
+	 * context, ensure that the subdomain can also be
+	 * powered on/off in that context.
+	 */
+	if (!genpd_is_irq_safe(genpd) && genpd_is_irq_safe(subdomain)) {
+		WARN("Parent %s of subdomain %s must be IRQ safe\n",
+				genpd->name, subdomain->name);
+		return -EINVAL;
+	}
+
 	link = kzalloc(sizeof(*link), GFP_KERNEL);
 	if (!link)
 		return -ENOMEM;
@@ -1337,6 +1416,17 @@ static int genpd_set_default_power_state(struct generic_pm_domain *genpd)
 	return 0;
 }
 
+static void genpd_lock_init(struct generic_pm_domain *genpd)
+{
+	if (genpd->flags & GENPD_FLAG_IRQ_SAFE) {
+		spin_lock_init(&genpd->slock);
+		genpd->lock_ops = &genpd_spin_ops;
+	} else {
+		mutex_init(&genpd->mlock);
+		genpd->lock_ops = &genpd_mtx_ops;
+	}
+}
+
 /**
  * pm_genpd_init - Initialize a generic I/O PM domain object.
  * @genpd: PM domain object to initialize.
@@ -1356,8 +1446,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
 	INIT_LIST_HEAD(&genpd->master_links);
 	INIT_LIST_HEAD(&genpd->slave_links);
 	INIT_LIST_HEAD(&genpd->dev_list);
-	mutex_init(&genpd->mlock);
-	genpd->lock_ops = &genpd_mtx_ops;
+	genpd_lock_init(genpd);
 	genpd->gov = gov;
 	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
 	atomic_set(&genpd->sd_count, 0);
@@ -2131,7 +2220,9 @@ static int pm_genpd_summary_one(struct seq_file *s,
 	}
 
 	list_for_each_entry(pm_data, &genpd->dev_list, list_node) {
-		kobj_path = kobject_get_path(&pm_data->dev->kobj, GFP_KERNEL);
+		kobj_path = kobject_get_path(&pm_data->dev->kobj,
+				genpd_is_irq_safe(genpd) ?
+				GFP_ATOMIC : GFP_KERNEL);
 		if (kobj_path == NULL)
 			continue;
 
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 811b968..81ece61 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -15,9 +15,11 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/notifier.h>
+#include <linux/spinlock.h>
 
 /* Defines used for the flags field in the struct generic_pm_domain */
 #define GENPD_FLAG_PM_CLK	(1U << 0) /* PM domain uses PM clk */
+#define GENPD_FLAG_IRQ_SAFE	(1U << 1) /* PM domain operates in atomic */
 
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
@@ -76,7 +78,13 @@ struct generic_pm_domain {
 	unsigned int state_idx; /* state that genpd will go to when off */
 	void *free; /* Free the state that was allocated for default */
 	const struct genpd_lock_ops *lock_ops;
-	struct mutex mlock;
+	union {
+		struct mutex mlock;
+		struct {
+			spinlock_t slock;
+			unsigned long lock_flags;
+		};
+	};
 
 };
 
-- 
2.7.4

[PATCH v3 8/8] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Lina Iyer]

Update documentation to reflect the changes made to support IRQ safe PM
domains.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 Documentation/power/devices.txt | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
index 8ba6625..0401b53 100644
--- a/Documentation/power/devices.txt
+++ b/Documentation/power/devices.txt
@@ -607,7 +607,14 @@ individually.  Instead, a set of devices sharing a power resource can be put
 into a low-power state together at the same time by turning off the shared
 power resource.  Of course, they also need to be put into the full-power state
 together, by turning the shared power resource on.  A set of devices with this
-property is often referred to as a power domain.
+property is often referred to as a power domain. A power domain may also be
+nested inside another power domain.
+
+Devices and PM domains may be defined as IRQ-safe, if they can be powered
+on/off even when the IRQs are disabled. An IRQ-safe device in a domain will
+disallow power management on the domain, unless the domain is also defined as
+IRQ-safe. The restriction this framework imposes on the parent domain of an
+IRQ-safe domain is that it must also be defined as IRQ-safe.
 
 Support for power domains is provided through the pm_domain field of struct
 device.  This field is a pointer to an object of type struct dev_pm_domain,
-- 
2.7.4

[PATCH v3 8/8] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Rafael J. Wysocki]

On Fri, Oct 14, 2016 at 7:47 PM, Lina Iyer <lina.iyer@linaro.org> wrote:
> Update documentation to reflect the changes made to support IRQ safe PM
> domains.
>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  Documentation/power/devices.txt | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
> index 8ba6625..0401b53 100644
> --- a/Documentation/power/devices.txt
> +++ b/Documentation/power/devices.txt
> @@ -607,7 +607,14 @@ individually.  Instead, a set of devices sharing a power resource can be put
>  into a low-power state together at the same time by turning off the shared
>  power resource.  Of course, they also need to be put into the full-power state
>  together, by turning the shared power resource on.  A set of devices with this
> -property is often referred to as a power domain.
> +property is often referred to as a power domain. A power domain may also be
> +nested inside another power domain.
> +
> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
> +on/off even when the IRQs are disabled. An IRQ-safe device in a domain will
> +disallow power management on the domain, unless the domain is also defined as
> +IRQ-safe. The restriction this framework imposes on the parent domain of an
> +IRQ-safe domain is that it must also be defined as IRQ-safe.

I would put this paragraph below, before the last paragraph in the section.

Also I suppose that a domain should only be defined as "IRQ-safe" if
all of the devices in it are "IRQ-safe" (or there will be problems at
least in principle).  If that is the case, it should be stated clearly
in the paragraph you are adding as well.

Thanks,
Rafael

[PATCH v3 8/8] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Lina Iyer]

On Fri, Oct 21 2016 at 07:07 -0600, Rafael J. Wysocki wrote:
>On Fri, Oct 14, 2016 at 7:47 PM, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Update documentation to reflect the changes made to support IRQ safe PM
>> domains.
>>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  Documentation/power/devices.txt | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
>> index 8ba6625..0401b53 100644
>> --- a/Documentation/power/devices.txt
>> +++ b/Documentation/power/devices.txt
>> @@ -607,7 +607,14 @@ individually.  Instead, a set of devices sharing a power resource can be put
>>  into a low-power state together at the same time by turning off the shared
>>  power resource.  Of course, they also need to be put into the full-power state
>>  together, by turning the shared power resource on.  A set of devices with this
>> -property is often referred to as a power domain.
>> +property is often referred to as a power domain. A power domain may also be
>> +nested inside another power domain.
>> +
>> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
>> +on/off even when the IRQs are disabled. An IRQ-safe device in a domain will
>> +disallow power management on the domain, unless the domain is also defined as
>> +IRQ-safe. The restriction this framework imposes on the parent domain of an
>> +IRQ-safe domain is that it must also be defined as IRQ-safe.
>
>I would put this paragraph below, before the last paragraph in the section.
>
OK.

>Also I suppose that a domain should only be defined as "IRQ-safe" if
>all of the devices in it are "IRQ-safe" (or there will be problems at
>least in principle).  If that is the case, it should be stated clearly
>in the paragraph you are adding as well.
>
Will add.

Thanks,
Lina

[PATCH v3 [fix]] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Lina Iyer]

Update documentation to reflect the changes made to support IRQ safe PM
domains.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
Changes since v3:
- Moved para to the end of the section
- Added clause for all IRQ safe devices in a domain
- Cleanup explanation of nested domains
---
 Documentation/power/devices.txt | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
index 8ba6625..9218ce6 100644
--- a/Documentation/power/devices.txt
+++ b/Documentation/power/devices.txt
@@ -607,7 +607,9 @@ individually.  Instead, a set of devices sharing a power resource can be put
 into a low-power state together at the same time by turning off the shared
 power resource.  Of course, they also need to be put into the full-power state
 together, by turning the shared power resource on.  A set of devices with this
-property is often referred to as a power domain.
+property is often referred to as a power domain. A power domain may also be
+nested inside another power domain. The nested domain is referred to as the
+sub-domain of the parent domain.
 
 Support for power domains is provided through the pm_domain field of struct
 device.  This field is a pointer to an object of type struct dev_pm_domain,
@@ -629,6 +631,13 @@ support for power domains into subsystem-level callbacks, for example by
 modifying the platform bus type.  Other platforms need not implement it or take
 it into account in any way.
 
+Devices and PM domains may be defined as IRQ-safe, if they can be powered
+on/off even when the IRQs are disabled. An IRQ-safe device in a domain will
+disallow power management on the domain, unless the domain is also defined as
+IRQ-safe. In other words, a domain containing all IRQ-safe devices must also
+be defined as IRQ-safe. Another restriction this framework imposes on the
+parent domain of an IRQ-safe domain is that the parent domain must also be
+defined as IRQ-safe.
 
 Device Low Power (suspend) States
 ---------------------------------
-- 
2.7.4

[PATCH v3 [fix]] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Rafael J. Wysocki]

On Friday, October 21, 2016 03:52:55 PM Lina Iyer wrote:
> Update documentation to reflect the changes made to support IRQ safe PM
> domains.
> 
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> Changes since v3:
> - Moved para to the end of the section
> - Added clause for all IRQ safe devices in a domain
> - Cleanup explanation of nested domains
> ---
>  Documentation/power/devices.txt | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
> index 8ba6625..9218ce6 100644
> --- a/Documentation/power/devices.txt
> +++ b/Documentation/power/devices.txt
> @@ -607,7 +607,9 @@ individually.  Instead, a set of devices sharing a power resource can be put
>  into a low-power state together at the same time by turning off the shared
>  power resource.  Of course, they also need to be put into the full-power state
>  together, by turning the shared power resource on.  A set of devices with this
> -property is often referred to as a power domain.
> +property is often referred to as a power domain. A power domain may also be
> +nested inside another power domain. The nested domain is referred to as the
> +sub-domain of the parent domain.
>  
>  Support for power domains is provided through the pm_domain field of struct
>  device.  This field is a pointer to an object of type struct dev_pm_domain,
> @@ -629,6 +631,13 @@ support for power domains into subsystem-level callbacks, for example by
>  modifying the platform bus type.  Other platforms need not implement it or take
>  it into account in any way.
>  
> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
> +on/off even when the IRQs are disabled.

What IRQ-safe means for devices is that their runtime PM callbacks may be
invoked with interrupts disabled on the local CPU.  I guess the meaning of
IRQ-safe for PM domains is analogous, but the above isn't precise enough to me.

> An IRQ-safe device in a domain will
> +disallow power management on the domain, unless the domain is also defined as
> +IRQ-safe. In other words, a domain containing all IRQ-safe devices must also
> +be defined as IRQ-safe. Another restriction this framework imposes on the
> +parent domain of an IRQ-safe domain is that the parent domain must also be
> +defined as IRQ-safe.

What about this:

"Devices may be defined as IRQ-safe which indicates to the PM core that their
runtime PM callbacks may be invoked with disabled interrupts (see
Documentation/power/runtime_pm.txt for more information).  If an IRQ-safe
device belongs to a PM domain, the runtime PM of the domain will be disallowed,
unless the domain itself is defined as IRQ-safe.  However, a PM domain can only
be defined as IRQ-safe if all of the devices in it are IRQ-safe.  Moreover, if
an IRQ-safe domain has a parent domain, the runtime PM of the parent is only
allowed if the parent itself is IRQ-safe too with the additional restriction
that all child domains of an IRQ-safe parent must also be IRQ-safe."

Does it actually reflect what the code does?

Thanks,
Rafael

[PATCH v3 [fix]] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Lina Iyer]

On Sat, Oct 22 2016 at 18:19 -0600, Rafael J. Wysocki wrote:
>On Friday, October 21, 2016 03:52:55 PM Lina Iyer wrote:
>> Update documentation to reflect the changes made to support IRQ safe PM
>> domains.
>>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>> Changes since v3:
>> - Moved para to the end of the section
>> - Added clause for all IRQ safe devices in a domain
>> - Cleanup explanation of nested domains
>> ---
>>  Documentation/power/devices.txt | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
>> index 8ba6625..9218ce6 100644
>> --- a/Documentation/power/devices.txt
>> +++ b/Documentation/power/devices.txt
>> @@ -607,7 +607,9 @@ individually.  Instead, a set of devices sharing a power resource can be put
>>  into a low-power state together at the same time by turning off the shared
>>  power resource.  Of course, they also need to be put into the full-power state
>>  together, by turning the shared power resource on.  A set of devices with this
>> -property is often referred to as a power domain.
>> +property is often referred to as a power domain. A power domain may also be
>> +nested inside another power domain. The nested domain is referred to as the
>> +sub-domain of the parent domain.
>>
>>  Support for power domains is provided through the pm_domain field of struct
>>  device.  This field is a pointer to an object of type struct dev_pm_domain,
>> @@ -629,6 +631,13 @@ support for power domains into subsystem-level callbacks, for example by
>>  modifying the platform bus type.  Other platforms need not implement it or take
>>  it into account in any way.
>>
>> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
>> +on/off even when the IRQs are disabled.
>
>What IRQ-safe means for devices is that their runtime PM callbacks may be
>invoked with interrupts disabled on the local CPU.  I guess the meaning of
>IRQ-safe for PM domains is analogous, but the above isn't precise enough to me.
>
>> An IRQ-safe device in a domain will
>> +disallow power management on the domain, unless the domain is also defined as
>> +IRQ-safe. In other words, a domain containing all IRQ-safe devices must also
>> +be defined as IRQ-safe. Another restriction this framework imposes on the
>> +parent domain of an IRQ-safe domain is that the parent domain must also be
>> +defined as IRQ-safe.
>
>What about this:
>
>"Devices may be defined as IRQ-safe which indicates to the PM core that their
>runtime PM callbacks may be invoked with disabled interrupts (see
>Documentation/power/runtime_pm.txt for more information).  If an IRQ-safe
>device belongs to a PM domain, the runtime PM of the domain will be disallowed,
>unless the domain itself is defined as IRQ-safe.  However, a PM domain can only
>be defined as IRQ-safe if all of the devices in it are IRQ-safe. 
>
This is correct. But the last line may need a bit of modification. If
all devices in a PM domain are IRQ-safe and the domain is NOT, then it
it is a valid combination just that the domain would never do runtime
PM.

> Moreover, if
>an IRQ-safe domain has a parent domain, the runtime PM of the parent is only
>allowed if the parent itself is IRQ-safe too with the additional restriction
>that all child domains of an IRQ-safe parent must also be IRQ-safe."
>
>Does it actually reflect what the code does?
>
Yes, this precisely reflects the code.

Thanks,
Lina

[PATCH v3 [fix]] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Rafael J. Wysocki]

On Monday, October 24, 2016 10:16:05 AM Lina Iyer wrote:
> On Sat, Oct 22 2016 at 18:19 -0600, Rafael J. Wysocki wrote:
> >On Friday, October 21, 2016 03:52:55 PM Lina Iyer wrote:
> >> Update documentation to reflect the changes made to support IRQ safe PM
> >> domains.
> >>
> >> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> >> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> >> ---
> >> Changes since v3:
> >> - Moved para to the end of the section
> >> - Added clause for all IRQ safe devices in a domain
> >> - Cleanup explanation of nested domains
> >> ---
> >>  Documentation/power/devices.txt | 11 ++++++++++-
> >>  1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
> >> index 8ba6625..9218ce6 100644
> >> --- a/Documentation/power/devices.txt
> >> +++ b/Documentation/power/devices.txt
> >> @@ -607,7 +607,9 @@ individually.  Instead, a set of devices sharing a power resource can be put
> >>  into a low-power state together at the same time by turning off the shared
> >>  power resource.  Of course, they also need to be put into the full-power state
> >>  together, by turning the shared power resource on.  A set of devices with this
> >> -property is often referred to as a power domain.
> >> +property is often referred to as a power domain. A power domain may also be
> >> +nested inside another power domain. The nested domain is referred to as the
> >> +sub-domain of the parent domain.
> >>
> >>  Support for power domains is provided through the pm_domain field of struct
> >>  device.  This field is a pointer to an object of type struct dev_pm_domain,
> >> @@ -629,6 +631,13 @@ support for power domains into subsystem-level callbacks, for example by
> >>  modifying the platform bus type.  Other platforms need not implement it or take
> >>  it into account in any way.
> >>
> >> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
> >> +on/off even when the IRQs are disabled.
> >
> >What IRQ-safe means for devices is that their runtime PM callbacks may be
> >invoked with interrupts disabled on the local CPU.  I guess the meaning of
> >IRQ-safe for PM domains is analogous, but the above isn't precise enough to me.
> >
> >> An IRQ-safe device in a domain will
> >> +disallow power management on the domain, unless the domain is also defined as
> >> +IRQ-safe. In other words, a domain containing all IRQ-safe devices must also
> >> +be defined as IRQ-safe. Another restriction this framework imposes on the
> >> +parent domain of an IRQ-safe domain is that the parent domain must also be
> >> +defined as IRQ-safe.
> >
> >What about this:
> >
> >"Devices may be defined as IRQ-safe which indicates to the PM core that their
> >runtime PM callbacks may be invoked with disabled interrupts (see
> >Documentation/power/runtime_pm.txt for more information).  If an IRQ-safe
> >device belongs to a PM domain, the runtime PM of the domain will be disallowed,
> >unless the domain itself is defined as IRQ-safe.  However, a PM domain can only
> >be defined as IRQ-safe if all of the devices in it are IRQ-safe. 
> >
> This is correct. But the last line may need a bit of modification. If
> all devices in a PM domain are IRQ-safe and the domain is NOT, then it
> it is a valid combination just that the domain would never do runtime
> PM.

That doesn't contradict the last sentence of mine above.  I guess what you mean
is that having a non-IRQ-safe device in an IRQ-safe domain is a valid
configuration.  I wonder how it works then. :-)

In any case, what about changing that sentence to something like:

"However, it only makes sense to define a PM domain as IRQ-safe if all devices
in it are IRQ-safe."

Thanks,
Rafael

[PATCH v3 [fix]] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Lina Iyer]

On Mon, Oct 24 2016 at 15:15 -0600, Rafael J. Wysocki wrote:
>On Monday, October 24, 2016 10:16:05 AM Lina Iyer wrote:
>> On Sat, Oct 22 2016 at 18:19 -0600, Rafael J. Wysocki wrote:
>> >On Friday, October 21, 2016 03:52:55 PM Lina Iyer wrote:
>> >> Update documentation to reflect the changes made to support IRQ safe PM
>> >> domains.
>> >>
>> >> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> >> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
>> >> ---
>> >> Changes since v3:
>> >> - Moved para to the end of the section
>> >> - Added clause for all IRQ safe devices in a domain
>> >> - Cleanup explanation of nested domains
>> >> ---
>> >>  Documentation/power/devices.txt | 11 ++++++++++-
>> >>  1 file changed, 10 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
>> >> index 8ba6625..9218ce6 100644
>> >> --- a/Documentation/power/devices.txt
>> >> +++ b/Documentation/power/devices.txt
>> >> @@ -607,7 +607,9 @@ individually.  Instead, a set of devices sharing a power resource can be put
>> >>  into a low-power state together at the same time by turning off the shared
>> >>  power resource.  Of course, they also need to be put into the full-power state
>> >>  together, by turning the shared power resource on.  A set of devices with this
>> >> -property is often referred to as a power domain.
>> >> +property is often referred to as a power domain. A power domain may also be
>> >> +nested inside another power domain. The nested domain is referred to as the
>> >> +sub-domain of the parent domain.
>> >>
>> >>  Support for power domains is provided through the pm_domain field of struct
>> >>  device.  This field is a pointer to an object of type struct dev_pm_domain,
>> >> @@ -629,6 +631,13 @@ support for power domains into subsystem-level callbacks, for example by
>> >>  modifying the platform bus type.  Other platforms need not implement it or take
>> >>  it into account in any way.
>> >>
>> >> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
>> >> +on/off even when the IRQs are disabled.
>> >
>> >What IRQ-safe means for devices is that their runtime PM callbacks may be
>> >invoked with interrupts disabled on the local CPU.  I guess the meaning of
>> >IRQ-safe for PM domains is analogous, but the above isn't precise enough to me.
>> >
>> >> An IRQ-safe device in a domain will
>> >> +disallow power management on the domain, unless the domain is also defined as
>> >> +IRQ-safe. In other words, a domain containing all IRQ-safe devices must also
>> >> +be defined as IRQ-safe. Another restriction this framework imposes on the
>> >> +parent domain of an IRQ-safe domain is that the parent domain must also be
>> >> +defined as IRQ-safe.
>> >
>> >What about this:
>> >
>> >"Devices may be defined as IRQ-safe which indicates to the PM core that their
>> >runtime PM callbacks may be invoked with disabled interrupts (see
>> >Documentation/power/runtime_pm.txt for more information).  If an IRQ-safe
>> >device belongs to a PM domain, the runtime PM of the domain will be disallowed,
>> >unless the domain itself is defined as IRQ-safe.  However, a PM domain can only
>> >be defined as IRQ-safe if all of the devices in it are IRQ-safe.
>> >
>> This is correct. But the last line may need a bit of modification. If
>> all devices in a PM domain are IRQ-safe and the domain is NOT, then it
>> it is a valid combination just that the domain would never do runtime
>> PM.
>
>That doesn't contradict the last sentence of mine above.  I guess what you mean
>is that having a non-IRQ-safe device in an IRQ-safe domain is a valid
>configuration.  I wonder how it works then. :-)
>
>In any case, what about changing that sentence to something like:
>
>"However, it only makes sense to define a PM domain as IRQ-safe if all devices
>in it are IRQ-safe."
>
That's precise. I will add your para instead of mine to the
documentation.

Thanks,
Lina
>

[PATCH v3 [fix-2]] PM / doc: Update device documentation for devices in IRQ safe PM domains

[Lina Iyer]

Update documentation to reflect the changes made to support IRQ safe PM
domains.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Signed-off-by: Rafael J. Wysocki <rjw@rjwysocki.net>
---
Changes since:
- Updated the text per Rafael's suggestion.
- Updated commit text with signed-off-by
---
 Documentation/power/devices.txt | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
index 8ba6625..73ddea3 100644
--- a/Documentation/power/devices.txt
+++ b/Documentation/power/devices.txt
@@ -607,7 +607,9 @@ individually.  Instead, a set of devices sharing a power resource can be put
 into a low-power state together at the same time by turning off the shared
 power resource.  Of course, they also need to be put into the full-power state
 together, by turning the shared power resource on.  A set of devices with this
-property is often referred to as a power domain.
+property is often referred to as a power domain. A power domain may also be
+nested inside another power domain. The nested domain is referred to as the
+sub-domain of the parent domain.
 
 Support for power domains is provided through the pm_domain field of struct
 device.  This field is a pointer to an object of type struct dev_pm_domain,
@@ -629,6 +631,16 @@ support for power domains into subsystem-level callbacks, for example by
 modifying the platform bus type.  Other platforms need not implement it or take
 it into account in any way.
 
+Devices may be defined as IRQ-safe which indicates to the PM core that their
+runtime PM callbacks may be invoked with disabled interrupts (see
+Documentation/power/runtime_pm.txt for more information).  If an IRQ-safe
+device belongs to a PM domain, the runtime PM of the domain will be
+disallowed, unless the domain itself is defined as IRQ-safe. However, it
+makes sense to define a PM domain as IRQ-safe only if all the devices in it
+are IRQ-safe. Moreover, if an IRQ-safe domain has a parent domain, the runtime
+PM of the parent is only allowed if the parent itself is IRQ-safe too with the
+additional restriction that all child domains of an IRQ-safe parent must also
+be IRQ-safe.
 
 Device Low Power (suspend) States
 ---------------------------------
-- 
2.7.4

[PATCH v3 0/8] PM / Domains: DT support for domain idle states & atomic PM domains

[Ulf Hansson]

On 14 October 2016 at 19:47, Lina Iyer <lina.iyer@linaro.org> wrote:
> Hi all,
>
> Changes since v2 [3] -
> - Addressed review comments from v2.
>         - domain-idle-states documentation updated
>         - fixed compiler issues with imx driver
>         - minor code change in pm_domains.c
> - The series is available at [4].
>
> Changes since v1 [2] -
> - Addressed review comments from v1.
>         - Fixes around dynamic allocation of genpd states
>         - Used OF method for iterating phandles
>         - Updated documentation, examples
>         - Rename state variable (provider -> fwnode)
> - The series is available at [3].
>
> The changes from [1] are -
> - Allocating memory for domain idle states dynamically
> - Conform to naming conventions for internal and exported genpd functions
> - DT binding example for domain-idle-state
> - Use fwnode instead of of_node
> - Handle atomic case for removal of PM Domain
> - Rebase on top of Rafael's pm/genpd tree
>
> Thanks,
> Lina
>
> Lina Iyer (8):
>   PM / Domains: Make genpd state allocation dynamic
>   PM / Domain: Add residency property to genpd states
>   PM / Domains: Allow domain power states to be read from DT
>   PM / Domains: Save the fwnode in genpd_power_state
>   dt/bindings: Update binding for PM domain idle states
>   PM / Domains: Abstract genpd locking
>   PM / Domains: Support IRQ safe PM domains
>   PM / doc: Update device documentation for devices in IRQ safe PM
>     domains
>
>  .../devicetree/bindings/power/power_domain.txt     |  43 +++
>  Documentation/power/devices.txt                    |   9 +-
>  arch/arm/mach-imx/gpc.c                            |  17 +-
>  drivers/base/power/domain.c                        | 358 +++++++++++++++++----
>  include/linux/pm_domain.h                          |  28 +-
>  5 files changed, 383 insertions(+), 72 deletions(-)
>

Rafael, Lina,

This looks good to me! Unless any other objections, I suggest to apply
this to get it tested in linux-next.

Kind regards
Uffe

> --
> 2.7.4
>
> [1]. https://www.spinics.net/lists/arm-kernel/msg526814.html
> [2]. http://www.spinics.net/lists/arm-kernel/msg535106.html
> [3]. https://git.linaro.org/people/lina.iyer/linux-next.git/shortlog/refs/heads/genpd-v2
> [4]. https://git.linaro.org/people/lina.iyer/linux-next.git/shortlog/refs/heads/genpd-v3

[PATCH v3 0/8] PM / Domains: DT support for domain idle states & atomic PM domains

[Lina Iyer]

On Mon, Oct 17 2016 at 01:30 -0600, Ulf Hansson wrote:
>On 14 October 2016 at 19:47, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Hi all,
>>
>> Changes since v2 [3] -
>> - Addressed review comments from v2.
>>         - domain-idle-states documentation updated
>>         - fixed compiler issues with imx driver
>>         - minor code change in pm_domains.c
>> - The series is available at [4].
>>
>> Changes since v1 [2] -
>> - Addressed review comments from v1.
>>         - Fixes around dynamic allocation of genpd states
>>         - Used OF method for iterating phandles
>>         - Updated documentation, examples
>>         - Rename state variable (provider -> fwnode)
>> - The series is available at [3].
>>
>> The changes from [1] are -
>> - Allocating memory for domain idle states dynamically
>> - Conform to naming conventions for internal and exported genpd functions
>> - DT binding example for domain-idle-state
>> - Use fwnode instead of of_node
>> - Handle atomic case for removal of PM Domain
>> - Rebase on top of Rafael's pm/genpd tree
>>
>> Thanks,
>> Lina
>>
>> Lina Iyer (8):
>>   PM / Domains: Make genpd state allocation dynamic
>>   PM / Domain: Add residency property to genpd states
>>   PM / Domains: Allow domain power states to be read from DT
>>   PM / Domains: Save the fwnode in genpd_power_state
>>   dt/bindings: Update binding for PM domain idle states
>>   PM / Domains: Abstract genpd locking
>>   PM / Domains: Support IRQ safe PM domains
>>   PM / doc: Update device documentation for devices in IRQ safe PM
>>     domains
>>
>>  .../devicetree/bindings/power/power_domain.txt     |  43 +++
>>  Documentation/power/devices.txt                    |   9 +-
>>  arch/arm/mach-imx/gpc.c                            |  17 +-
>>  drivers/base/power/domain.c                        | 358 +++++++++++++++++----
>>  include/linux/pm_domain.h                          |  28 +-
>>  5 files changed, 383 insertions(+), 72 deletions(-)
>>
>
>Rafael, Lina,
>
>This looks good to me! Unless any other objections, I suggest to apply
>this to get it tested in linux-next.
>
>Kind regards
>Uffe
>
Rafael,

If there are no objections, could you pick this patch for linux-next?

Thanks,
Lina
>> --
>> 2.7.4
>>
>> [1]. https://www.spinics.net/lists/arm-kernel/msg526814.html
>> [2]. http://www.spinics.net/lists/arm-kernel/msg535106.html
>> [3]. https://git.linaro.org/people/lina.iyer/linux-next.git/shortlog/refs/heads/genpd-v2
>> [4]. https://git.linaro.org/people/lina.iyer/linux-next.git/shortlog/refs/heads/genpd-v3

[PATCH v3 0/8] PM / Domains: DT support for domain idle states & atomic PM domains

[Rafael J. Wysocki]

On Fri, Oct 21, 2016 at 12:44 AM, Lina Iyer <lina.iyer@linaro.org> wrote:
> On Mon, Oct 17 2016 at 01:30 -0600, Ulf Hansson wrote:
>>
>> On 14 October 2016 at 19:47, Lina Iyer <lina.iyer@linaro.org> wrote:
>>>
>>> Hi all,
>>>
>>> Changes since v2 [3] -
>>> - Addressed review comments from v2.
>>>         - domain-idle-states documentation updated
>>>         - fixed compiler issues with imx driver
>>>         - minor code change in pm_domains.c
>>> - The series is available at [4].
>>>
>>> Changes since v1 [2] -
>>> - Addressed review comments from v1.
>>>         - Fixes around dynamic allocation of genpd states
>>>         - Used OF method for iterating phandles
>>>         - Updated documentation, examples
>>>         - Rename state variable (provider -> fwnode)
>>> - The series is available at [3].
>>>
>>> The changes from [1] are -
>>> - Allocating memory for domain idle states dynamically
>>> - Conform to naming conventions for internal and exported genpd functions
>>> - DT binding example for domain-idle-state
>>> - Use fwnode instead of of_node
>>> - Handle atomic case for removal of PM Domain
>>> - Rebase on top of Rafael's pm/genpd tree
>>>
>>> Thanks,
>>> Lina
>>>
>>> Lina Iyer (8):
>>>   PM / Domains: Make genpd state allocation dynamic
>>>   PM / Domain: Add residency property to genpd states
>>>   PM / Domains: Allow domain power states to be read from DT
>>>   PM / Domains: Save the fwnode in genpd_power_state
>>>   dt/bindings: Update binding for PM domain idle states
>>>   PM / Domains: Abstract genpd locking
>>>   PM / Domains: Support IRQ safe PM domains
>>>   PM / doc: Update device documentation for devices in IRQ safe PM
>>>     domains
>>>
>>>  .../devicetree/bindings/power/power_domain.txt     |  43 +++
>>>  Documentation/power/devices.txt                    |   9 +-
>>>  arch/arm/mach-imx/gpc.c                            |  17 +-
>>>  drivers/base/power/domain.c                        | 358
>>> +++++++++++++++++----
>>>  include/linux/pm_domain.h                          |  28 +-
>>>  5 files changed, 383 insertions(+), 72 deletions(-)
>>>
>>
>> Rafael, Lina,
>>
>> This looks good to me! Unless any other objections, I suggest to apply
>> this to get it tested in linux-next.
>>
>> Kind regards
>> Uffe
>>
> Rafael,
>
> If there are no objections, could you pick this patch for linux-next?

It is in my queue, but not at the top yet.

Thanks,
Rafael

[PATCH v3 0/8] PM / Domains: DT support for domain idle states & atomic PM domains

[Lina Iyer]

On Fri, Oct 21 2016 at 16:48 -0600, Rafael J. Wysocki wrote:
>On Fri, Oct 21, 2016 at 12:44 AM, Lina Iyer <lina.iyer@linaro.org> wrote:
>> On Mon, Oct 17 2016 at 01:30 -0600, Ulf Hansson wrote:
>>>
>>> On 14 October 2016 at 19:47, Lina Iyer <lina.iyer@linaro.org> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Changes since v2 [3] -
>>>> - Addressed review comments from v2.
>>>>         - domain-idle-states documentation updated
>>>>         - fixed compiler issues with imx driver
>>>>         - minor code change in pm_domains.c
>>>> - The series is available at [4].
>>>>
>>>> Changes since v1 [2] -
>>>> - Addressed review comments from v1.
>>>>         - Fixes around dynamic allocation of genpd states
>>>>         - Used OF method for iterating phandles
>>>>         - Updated documentation, examples
>>>>         - Rename state variable (provider -> fwnode)
>>>> - The series is available at [3].
>>>>
>>>> The changes from [1] are -
>>>> - Allocating memory for domain idle states dynamically
>>>> - Conform to naming conventions for internal and exported genpd functions
>>>> - DT binding example for domain-idle-state
>>>> - Use fwnode instead of of_node
>>>> - Handle atomic case for removal of PM Domain
>>>> - Rebase on top of Rafael's pm/genpd tree
>>>>
>>>> Thanks,
>>>> Lina
>>>>
>>>> Lina Iyer (8):
>>>>   PM / Domains: Make genpd state allocation dynamic
>>>>   PM / Domain: Add residency property to genpd states
>>>>   PM / Domains: Allow domain power states to be read from DT
>>>>   PM / Domains: Save the fwnode in genpd_power_state
>>>>   dt/bindings: Update binding for PM domain idle states
>>>>   PM / Domains: Abstract genpd locking
>>>>   PM / Domains: Support IRQ safe PM domains
>>>>   PM / doc: Update device documentation for devices in IRQ safe PM
>>>>     domains
>>>>
>>>>  .../devicetree/bindings/power/power_domain.txt     |  43 +++
>>>>  Documentation/power/devices.txt                    |   9 +-
>>>>  arch/arm/mach-imx/gpc.c                            |  17 +-
>>>>  drivers/base/power/domain.c                        | 358
>>>> +++++++++++++++++----
>>>>  include/linux/pm_domain.h                          |  28 +-
>>>>  5 files changed, 383 insertions(+), 72 deletions(-)
>>>>
>>>
>>> Rafael, Lina,
>>>
>>> This looks good to me! Unless any other objections, I suggest to apply
>>> this to get it tested in linux-next.
>>>
>>> Kind regards
>>> Uffe
>>>
>> Rafael,
>>
>> If there are no objections, could you pick this patch for linux-next?
>
>It is in my queue, but not at the top yet.
>
Thank you Rafael.

-- Lina

>Thanks,
>Rafael

[PATCH v3 0/8] PM / Domains: DT support for domain idle states & atomic PM domains

[Rafael J. Wysocki]

On Fri, Oct 21, 2016 at 3:33 AM, Lina Iyer <lina.iyer@linaro.org> wrote:
> On Fri, Oct 21 2016 at 16:48 -0600, Rafael J. Wysocki wrote:
>>
>> On Fri, Oct 21, 2016 at 12:44 AM, Lina Iyer <lina.iyer@linaro.org> wrote:
>>>
>>> On Mon, Oct 17 2016 at 01:30 -0600, Ulf Hansson wrote:
>>>>
>>>>
>>>> On 14 October 2016 at 19:47, Lina Iyer <lina.iyer@linaro.org> wrote:
>>>>>
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Changes since v2 [3] -
>>>>> - Addressed review comments from v2.
>>>>>         - domain-idle-states documentation updated
>>>>>         - fixed compiler issues with imx driver
>>>>>         - minor code change in pm_domains.c
>>>>> - The series is available at [4].
>>>>>
>>>>> Changes since v1 [2] -
>>>>> - Addressed review comments from v1.
>>>>>         - Fixes around dynamic allocation of genpd states
>>>>>         - Used OF method for iterating phandles
>>>>>         - Updated documentation, examples
>>>>>         - Rename state variable (provider -> fwnode)
>>>>> - The series is available at [3].
>>>>>
>>>>> The changes from [1] are -
>>>>> - Allocating memory for domain idle states dynamically
>>>>> - Conform to naming conventions for internal and exported genpd
>>>>> functions
>>>>> - DT binding example for domain-idle-state
>>>>> - Use fwnode instead of of_node
>>>>> - Handle atomic case for removal of PM Domain
>>>>> - Rebase on top of Rafael's pm/genpd tree
>>>>>
>>>>> Thanks,
>>>>> Lina
>>>>>
>>>>> Lina Iyer (8):
>>>>>   PM / Domains: Make genpd state allocation dynamic
>>>>>   PM / Domain: Add residency property to genpd states
>>>>>   PM / Domains: Allow domain power states to be read from DT
>>>>>   PM / Domains: Save the fwnode in genpd_power_state
>>>>>   dt/bindings: Update binding for PM domain idle states
>>>>>   PM / Domains: Abstract genpd locking
>>>>>   PM / Domains: Support IRQ safe PM domains
>>>>>   PM / doc: Update device documentation for devices in IRQ safe PM
>>>>>     domains
>>>>>
>>>>>  .../devicetree/bindings/power/power_domain.txt     |  43 +++
>>>>>  Documentation/power/devices.txt                    |   9 +-
>>>>>  arch/arm/mach-imx/gpc.c                            |  17 +-
>>>>>  drivers/base/power/domain.c                        | 358
>>>>> +++++++++++++++++----
>>>>>  include/linux/pm_domain.h                          |  28 +-
>>>>>  5 files changed, 383 insertions(+), 72 deletions(-)
>>>>>
>>>>
>>>> Rafael, Lina,
>>>>
>>>> This looks good to me! Unless any other objections, I suggest to apply
>>>> this to get it tested in linux-next.
>>>>
>>>> Kind regards
>>>> Uffe
>>>>
>>> Rafael,
>>>
>>> If there are no objections, could you pick this patch for linux-next?
>>
>>
>> It is in my queue, but not at the top yet.
>>
> Thank you Rafael.

OK, I'm queuing up patches [1-7/8] as I don't see anything
particularly objectionable in them.

The documentation one needs some more work, however, and I've sent
comments on that one already.

Thanks,
Rafael

[PATCH v3 0/8] PM / Domains: DT support for domain idle states & atomic PM domains

[Kevin Hilman]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> On 14 October 2016 at 19:47, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Hi all,
>>
>> Changes since v2 [3] -
>> - Addressed review comments from v2.
>>         - domain-idle-states documentation updated
>>         - fixed compiler issues with imx driver
>>         - minor code change in pm_domains.c
>> - The series is available at [4].
>>
>> Changes since v1 [2] -
>> - Addressed review comments from v1.
>>         - Fixes around dynamic allocation of genpd states
>>         - Used OF method for iterating phandles
>>         - Updated documentation, examples
>>         - Rename state variable (provider -> fwnode)
>> - The series is available at [3].
>>
>> The changes from [1] are -
>> - Allocating memory for domain idle states dynamically
>> - Conform to naming conventions for internal and exported genpd functions
>> - DT binding example for domain-idle-state
>> - Use fwnode instead of of_node
>> - Handle atomic case for removal of PM Domain
>> - Rebase on top of Rafael's pm/genpd tree
>>
>> Thanks,
>> Lina
>>
>> Lina Iyer (8):
>>   PM / Domains: Make genpd state allocation dynamic
>>   PM / Domain: Add residency property to genpd states
>>   PM / Domains: Allow domain power states to be read from DT
>>   PM / Domains: Save the fwnode in genpd_power_state
>>   dt/bindings: Update binding for PM domain idle states
>>   PM / Domains: Abstract genpd locking
>>   PM / Domains: Support IRQ safe PM domains
>>   PM / doc: Update device documentation for devices in IRQ safe PM
>>     domains
>>
>>  .../devicetree/bindings/power/power_domain.txt     |  43 +++
>>  Documentation/power/devices.txt                    |   9 +-
>>  arch/arm/mach-imx/gpc.c                            |  17 +-
>>  drivers/base/power/domain.c                        | 358 +++++++++++++++++----
>>  include/linux/pm_domain.h                          |  28 +-
>>  5 files changed, 383 insertions(+), 72 deletions(-)
>>
>
> Rafael, Lina,
>
> This looks good to me! Unless any other objections, I suggest to apply
> this to get it tested in linux-next.

Looks like it might be a little late now, but anyways

Reviewed-by: Kevin Hilman <khilman@baylibre.com>