* [PATCH 0/2] PM / Domains: Updates to IRQ safe PM domains
@ 2017-02-08 16:34 ` Lina Iyer
0 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:34 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This follows the discussions at Linux Plumbers Conference on PM domains and
idle states. In the context of CPU hierarchy, the CPU's idle states may be
presented using the property domain-idle-states if the domain is considered as
the boundary that powers on/off an individual CPU.
To that effect, domain idle states may not be specified but not compatible with
what PM domains expects. Hence ignore the idle states that are not
"domain-idle-state" compatible.
The second patch covers the case, where the hierarchy of PM domains is not
powered off in the same context. Currently, a workqueue is scheduled to power
off the parent domains in the hierarchy. In case of CPU PM domains, this will
not work, since we are running with interrupts disabled. Hence for IRQ safe PM
domains, power off the parent domains in the same context as the caller.
I understand Ulf is working on some argument changes in the same area of
functions. Will work with him to resolve conflicts before the merge. This is
based on the tip of Rafel's next.
Thanks,
Lina
Lina Iyer (2):
PM / Domains: Ignore domain-idle-states that are not compatible
PM / Domains: Support in context powering off parent domain
.../devicetree/bindings/power/power_domain.txt | 4 ++-
drivers/base/power/domain.c | 37 +++++++++++++++-------
2 files changed, 28 insertions(+), 13 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
2017-02-08 16:34 ` Lina Iyer
@ 2017-02-08 16:34 ` Lina Iyer
-1 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:34 UTC (permalink / raw)
To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
Cc: devicetree, lorenzo.pieralisi, Juri.Lelli, Rob Herring,
linux-arm-msm, sboyd, brendan.jackman, sudeep.holla, andy.gross,
Lina Iyer
Re-using idle state definition provided by arm,idle-state for domain
idle states creates a lot of confusion and limits further evolution of
the domain idle definition. To keep things clear and simple, define a
idle states for domain using a new compatible "domain-idle-state".
Fix existing PM domains code to look for the newly defined compatible.
Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
.../devicetree/bindings/power/power_domain.txt | 8 +++---
drivers/base/power/domain.c | 2 +-
3 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
new file mode 100644
index 0000000..eefc7ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
@@ -0,0 +1,33 @@
+PM Domain Idle State Node:
+
+A domain idle state node represents the state parameters that will be used to
+select the state when there are no active components in the domain.
+
+The state node has the following parameters -
+
+- compatible:
+ Usage: Required
+ Value type: <string>
+ Definition: Must be "domain-idle-state".
+
+- entry-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency in
+ microseconds required to enter the idle state.
+ The exit-latency-us duration may be guaranteed
+ only after entry-latency-us has passed.
+
+- exit-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency
+ in microseconds required to exit the idle state.
+
+- min-residency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing minimum residency duration
+ in microseconds after which the idle state will yield
+ power benefits after overcoming the overhead in entering
+i the idle state.
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e165036..723e1ad 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -31,7 +31,7 @@ Optional properties:
- 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].
+ compatible with domain-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
@@ -85,7 +85,7 @@ Example 3:
};
DOMAIN_RET: state@0 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x0>;
entry-latency-us = <1000>;
exit-latency-us = <2000>;
@@ -93,7 +93,7 @@ Example 3:
};
DOMAIN_PWR_DN: state@1 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x1>;
entry-latency-us = <5000>;
exit-latency-us = <8000>;
@@ -118,4 +118,4 @@ 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
+[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 661737c..f0bc672 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
static const struct of_device_id idle_state_match[] = {
- { .compatible = "arm,idle-state", },
+ { .compatible = "domain-idle-state", },
{ }
};
--
2.7.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2017-02-08 16:34 ` Lina Iyer
0 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:34 UTC (permalink / raw)
To: linux-arm-kernel
Re-using idle state definition provided by arm,idle-state for domain
idle states creates a lot of confusion and limits further evolution of
the domain idle definition. To keep things clear and simple, define a
idle states for domain using a new compatible "domain-idle-state".
Fix existing PM domains code to look for the newly defined compatible.
Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
.../devicetree/bindings/power/power_domain.txt | 8 +++---
drivers/base/power/domain.c | 2 +-
3 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
new file mode 100644
index 0000000..eefc7ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
@@ -0,0 +1,33 @@
+PM Domain Idle State Node:
+
+A domain idle state node represents the state parameters that will be used to
+select the state when there are no active components in the domain.
+
+The state node has the following parameters -
+
+- compatible:
+ Usage: Required
+ Value type: <string>
+ Definition: Must be "domain-idle-state".
+
+- entry-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency in
+ microseconds required to enter the idle state.
+ The exit-latency-us duration may be guaranteed
+ only after entry-latency-us has passed.
+
+- exit-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency
+ in microseconds required to exit the idle state.
+
+- min-residency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing minimum residency duration
+ in microseconds after which the idle state will yield
+ power benefits after overcoming the overhead in entering
+i the idle state.
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e165036..723e1ad 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -31,7 +31,7 @@ Optional properties:
- 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].
+ compatible with domain-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
@@ -85,7 +85,7 @@ Example 3:
};
DOMAIN_RET: state at 0 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x0>;
entry-latency-us = <1000>;
exit-latency-us = <2000>;
@@ -93,7 +93,7 @@ Example 3:
};
DOMAIN_PWR_DN: state at 1 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x1>;
entry-latency-us = <5000>;
exit-latency-us = <8000>;
@@ -118,4 +118,4 @@ 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
+[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 661737c..f0bc672 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
static const struct of_device_id idle_state_match[] = {
- { .compatible = "arm,idle-state", },
+ { .compatible = "domain-idle-state", },
{ }
};
--
2.7.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2017-02-08 16:34 ` Lina Iyer
@ 2017-02-08 16:42 ` Lina Iyer
-1 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:42 UTC (permalink / raw)
To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
Cc: andy.gross, sboyd, linux-arm-msm, brendan.jackman,
lorenzo.pieralisi, sudeep.holla, Juri.Lelli, devicetree,
Rob Herring
On Wed, Feb 08 2017 at 09:35 -0700, Lina Iyer wrote:
>Re-using idle state definition provided by arm,idle-state for domain
>idle states creates a lot of confusion and limits further evolution of
>the domain idle definition. To keep things clear and simple, define a
>idle states for domain using a new compatible "domain-idle-state".
>
>Fix existing PM domains code to look for the newly defined compatible.
>
>Cc: <devicetree@vger.kernel.org>
>Cc: Rob Herring <robh@kernel.org>
>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Sorry, this has already been applied. Got tagged along with the email.
Kindly ignore.
Thanks,
Lina
>---
> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> .../devicetree/bindings/power/power_domain.txt | 8 +++---
> drivers/base/power/domain.c | 2 +-
> 3 files changed, 38 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
>diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>new file mode 100644
>index 0000000..eefc7ed
>--- /dev/null
>+++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>@@ -0,0 +1,33 @@
>+PM Domain Idle State Node:
>+
>+A domain idle state node represents the state parameters that will be used to
>+select the state when there are no active components in the domain.
>+
>+The state node has the following parameters -
>+
>+- compatible:
>+ Usage: Required
>+ Value type: <string>
>+ Definition: Must be "domain-idle-state".
>+
>+- entry-latency-us
>+ Usage: Required
>+ Value type: <prop-encoded-array>
>+ Definition: u32 value representing worst case latency in
>+ microseconds required to enter the idle state.
>+ The exit-latency-us duration may be guaranteed
>+ only after entry-latency-us has passed.
>+
>+- exit-latency-us
>+ Usage: Required
>+ Value type: <prop-encoded-array>
>+ Definition: u32 value representing worst case latency
>+ in microseconds required to exit the idle state.
>+
>+- min-residency-us
>+ Usage: Required
>+ Value type: <prop-encoded-array>
>+ Definition: u32 value representing minimum residency duration
>+ in microseconds after which the idle state will yield
>+ power benefits after overcoming the overhead in entering
>+i the idle state.
>diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>index e165036..723e1ad 100644
>--- a/Documentation/devicetree/bindings/power/power_domain.txt
>+++ b/Documentation/devicetree/bindings/power/power_domain.txt
>@@ -31,7 +31,7 @@ Optional properties:
>
> - 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].
>+ compatible with domain-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
>@@ -85,7 +85,7 @@ Example 3:
> };
>
> DOMAIN_RET: state@0 {
>- compatible = "arm,idle-state";
>+ compatible = "domain-idle-state";
> reg = <0x0>;
> entry-latency-us = <1000>;
> exit-latency-us = <2000>;
>@@ -93,7 +93,7 @@ Example 3:
> };
>
> DOMAIN_PWR_DN: state@1 {
>- compatible = "arm,idle-state";
>+ compatible = "domain-idle-state";
> reg = <0x1>;
> entry-latency-us = <5000>;
> exit-latency-us = <8000>;
>@@ -118,4 +118,4 @@ 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
>+[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>index 661737c..f0bc672 100644
>--- a/drivers/base/power/domain.c
>+++ b/drivers/base/power/domain.c
>@@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
>- { .compatible = "arm,idle-state", },
>+ { .compatible = "domain-idle-state", },
> { }
> };
>
>--
>2.7.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2017-02-08 16:42 ` Lina Iyer
0 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:42 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 08 2017 at 09:35 -0700, Lina Iyer wrote:
>Re-using idle state definition provided by arm,idle-state for domain
>idle states creates a lot of confusion and limits further evolution of
>the domain idle definition. To keep things clear and simple, define a
>idle states for domain using a new compatible "domain-idle-state".
>
>Fix existing PM domains code to look for the newly defined compatible.
>
>Cc: <devicetree@vger.kernel.org>
>Cc: Rob Herring <robh@kernel.org>
>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Sorry, this has already been applied. Got tagged along with the email.
Kindly ignore.
Thanks,
Lina
>---
> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> .../devicetree/bindings/power/power_domain.txt | 8 +++---
> drivers/base/power/domain.c | 2 +-
> 3 files changed, 38 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
>diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>new file mode 100644
>index 0000000..eefc7ed
>--- /dev/null
>+++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>@@ -0,0 +1,33 @@
>+PM Domain Idle State Node:
>+
>+A domain idle state node represents the state parameters that will be used to
>+select the state when there are no active components in the domain.
>+
>+The state node has the following parameters -
>+
>+- compatible:
>+ Usage: Required
>+ Value type: <string>
>+ Definition: Must be "domain-idle-state".
>+
>+- entry-latency-us
>+ Usage: Required
>+ Value type: <prop-encoded-array>
>+ Definition: u32 value representing worst case latency in
>+ microseconds required to enter the idle state.
>+ The exit-latency-us duration may be guaranteed
>+ only after entry-latency-us has passed.
>+
>+- exit-latency-us
>+ Usage: Required
>+ Value type: <prop-encoded-array>
>+ Definition: u32 value representing worst case latency
>+ in microseconds required to exit the idle state.
>+
>+- min-residency-us
>+ Usage: Required
>+ Value type: <prop-encoded-array>
>+ Definition: u32 value representing minimum residency duration
>+ in microseconds after which the idle state will yield
>+ power benefits after overcoming the overhead in entering
>+i the idle state.
>diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>index e165036..723e1ad 100644
>--- a/Documentation/devicetree/bindings/power/power_domain.txt
>+++ b/Documentation/devicetree/bindings/power/power_domain.txt
>@@ -31,7 +31,7 @@ Optional properties:
>
> - 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].
>+ compatible with domain-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
>@@ -85,7 +85,7 @@ Example 3:
> };
>
> DOMAIN_RET: state at 0 {
>- compatible = "arm,idle-state";
>+ compatible = "domain-idle-state";
> reg = <0x0>;
> entry-latency-us = <1000>;
> exit-latency-us = <2000>;
>@@ -93,7 +93,7 @@ Example 3:
> };
>
> DOMAIN_PWR_DN: state at 1 {
>- compatible = "arm,idle-state";
>+ compatible = "domain-idle-state";
> reg = <0x1>;
> entry-latency-us = <5000>;
> exit-latency-us = <8000>;
>@@ -118,4 +118,4 @@ 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
>+[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>index 661737c..f0bc672 100644
>--- a/drivers/base/power/domain.c
>+++ b/drivers/base/power/domain.c
>@@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
>- { .compatible = "arm,idle-state", },
>+ { .compatible = "domain-idle-state", },
> { }
> };
>
>--
>2.7.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 1/2] PM / Domains: Ignore domain-idle-states that are not compatible
2017-02-08 16:34 ` Lina Iyer
@ 2017-02-08 16:34 ` Lina Iyer
-1 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:34 UTC (permalink / raw)
To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
Cc: andy.gross, sboyd, linux-arm-msm, brendan.jackman,
lorenzo.pieralisi, sudeep.holla, Juri.Lelli, Lina Iyer,
devicetree, Rob Herring
domain-idle-states property may have phandles to idle state bindings
that may not be compatible with idle state definition defined in [1].
Such phandles would just be ignored and not throw and error when read by
the domain core.
Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
Documentation/devicetree/bindings/power/power_domain.txt | 4 +++-
drivers/base/power/domain.c | 16 +++++++++-------
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 723e1ad..940707d 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -31,7 +31,9 @@ Optional properties:
- 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 domain-idle-state specified in [1].
+ compatible with domain-idle-state specified in [1]. phandles
+ that are not compatible with domain-idle-state will be
+ ignored.
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
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 6b23d82..3825bb9 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2065,11 +2065,6 @@ static int genpd_parse_state(struct genpd_power_state *genpd_state,
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);
@@ -2118,6 +2113,7 @@ int of_genpd_parse_idle_states(struct device_node *dn,
int err, ret;
int count;
struct of_phandle_iterator it;
+ const struct of_device_id *match_id;
count = of_count_phandle_with_args(dn, "domain-idle-states", NULL);
if (count <= 0)
@@ -2130,6 +2126,9 @@ int of_genpd_parse_idle_states(struct device_node *dn,
/* 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;
+ match_id = of_match_node(idle_state_match, np);
+ if (!match_id)
+ continue;
ret = genpd_parse_state(&st[i++], np);
if (ret) {
pr_err
@@ -2141,8 +2140,11 @@ int of_genpd_parse_idle_states(struct device_node *dn,
}
}
- *n = count;
- *states = st;
+ *n = i;
+ if (!i)
+ kfree(st);
+ else
+ *states = st;
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 1/2] PM / Domains: Ignore domain-idle-states that are not compatible
@ 2017-02-08 16:34 ` Lina Iyer
0 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:34 UTC (permalink / raw)
To: linux-arm-kernel
domain-idle-states property may have phandles to idle state bindings
that may not be compatible with idle state definition defined in [1].
Such phandles would just be ignored and not throw and error when read by
the domain core.
Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
Documentation/devicetree/bindings/power/power_domain.txt | 4 +++-
drivers/base/power/domain.c | 16 +++++++++-------
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 723e1ad..940707d 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -31,7 +31,9 @@ Optional properties:
- 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 domain-idle-state specified in [1].
+ compatible with domain-idle-state specified in [1]. phandles
+ that are not compatible with domain-idle-state will be
+ ignored.
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
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 6b23d82..3825bb9 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2065,11 +2065,6 @@ static int genpd_parse_state(struct genpd_power_state *genpd_state,
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);
@@ -2118,6 +2113,7 @@ int of_genpd_parse_idle_states(struct device_node *dn,
int err, ret;
int count;
struct of_phandle_iterator it;
+ const struct of_device_id *match_id;
count = of_count_phandle_with_args(dn, "domain-idle-states", NULL);
if (count <= 0)
@@ -2130,6 +2126,9 @@ int of_genpd_parse_idle_states(struct device_node *dn,
/* 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;
+ match_id = of_match_node(idle_state_match, np);
+ if (!match_id)
+ continue;
ret = genpd_parse_state(&st[i++], np);
if (ret) {
pr_err
@@ -2141,8 +2140,11 @@ int of_genpd_parse_idle_states(struct device_node *dn,
}
}
- *n = count;
- *states = st;
+ *n = i;
+ if (!i)
+ kfree(st);
+ else
+ *states = st;
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 1/2] PM / Domains: Ignore domain-idle-states that are not compatible
2017-02-08 16:34 ` Lina Iyer
@ 2017-02-09 8:40 ` Ulf Hansson
-1 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2017-02-09 8:40 UTC (permalink / raw)
To: Lina Iyer
Cc: devicetree, Lorenzo Pieralisi, Juri Lelli, linux-pm,
Rafael J. Wysocki, Kevin Hilman, Stephen Boyd, Sudeep Holla,
Brendan Jackman, linux-arm-msm, Andy Gross, Rob Herring,
linux-arm-kernel
On 8 February 2017 at 17:34, Lina Iyer <lina.iyer@linaro.org> wrote:
> domain-idle-states property may have phandles to idle state bindings
> that may not be compatible with idle state definition defined in [1].
I don't find the reference to [1] in the change-log, could you please add it.
> Such phandles would just be ignored and not throw and error when read by
> the domain core.
Perhaps, you could also share a minimal snipped from a DTS as it helps
to describe why and what this change is needed.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> Documentation/devicetree/bindings/power/power_domain.txt | 4 +++-
> drivers/base/power/domain.c | 16 +++++++++-------
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index 723e1ad..940707d 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,9 @@ Optional properties:
>
> - 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 domain-idle-state specified in [1].
> + compatible with domain-idle-state specified in [1]. phandles
> + that are not compatible with domain-idle-state will be
> + ignored.
> 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
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 6b23d82..3825bb9 100644
> --- a/drivers/base/power/domain.c
Always split DT documentation changes from the code changes and make
the DT doc changes precede the code changes in the series of patches.
> +++ b/drivers/base/power/domain.c
> @@ -2065,11 +2065,6 @@ static int genpd_parse_state(struct genpd_power_state *genpd_state,
> 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);
> @@ -2118,6 +2113,7 @@ int of_genpd_parse_idle_states(struct device_node *dn,
> int err, ret;
> int count;
> struct of_phandle_iterator it;
> + const struct of_device_id *match_id;
>
> count = of_count_phandle_with_args(dn, "domain-idle-states", NULL);
> if (count <= 0)
> @@ -2130,6 +2126,9 @@ int of_genpd_parse_idle_states(struct device_node *dn,
> /* 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;
> + match_id = of_match_node(idle_state_match, np);
> + if (!match_id)
> + continue;
Earlier we have allocated "count" numbers of struct genpd_power_state,
by using a kcalloc().
This change may lead to that we could have allocated more memory than
actually needed - because there may be some nodes that doesn't match.
Perhaps it's better to do a pre-iteration to find the real numbers of
how many struct genpd_power_state we actually need to allocate!?
> ret = genpd_parse_state(&st[i++], np);
> if (ret) {
> pr_err
> @@ -2141,8 +2140,11 @@ int of_genpd_parse_idle_states(struct device_node *dn,
> }
> }
>
> - *n = count;
> - *states = st;
> + *n = i;
> + if (!i)
> + kfree(st);
> + else
> + *states = st;
>
> return 0;
> }
> --
> 2.7.4
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 1/2] PM / Domains: Ignore domain-idle-states that are not compatible
@ 2017-02-09 8:40 ` Ulf Hansson
0 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2017-02-09 8:40 UTC (permalink / raw)
To: linux-arm-kernel
On 8 February 2017 at 17:34, Lina Iyer <lina.iyer@linaro.org> wrote:
> domain-idle-states property may have phandles to idle state bindings
> that may not be compatible with idle state definition defined in [1].
I don't find the reference to [1] in the change-log, could you please add it.
> Such phandles would just be ignored and not throw and error when read by
> the domain core.
Perhaps, you could also share a minimal snipped from a DTS as it helps
to describe why and what this change is needed.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> Documentation/devicetree/bindings/power/power_domain.txt | 4 +++-
> drivers/base/power/domain.c | 16 +++++++++-------
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index 723e1ad..940707d 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,9 @@ Optional properties:
>
> - 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 domain-idle-state specified in [1].
> + compatible with domain-idle-state specified in [1]. phandles
> + that are not compatible with domain-idle-state will be
> + ignored.
> 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
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 6b23d82..3825bb9 100644
> --- a/drivers/base/power/domain.c
Always split DT documentation changes from the code changes and make
the DT doc changes precede the code changes in the series of patches.
> +++ b/drivers/base/power/domain.c
> @@ -2065,11 +2065,6 @@ static int genpd_parse_state(struct genpd_power_state *genpd_state,
> 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);
> @@ -2118,6 +2113,7 @@ int of_genpd_parse_idle_states(struct device_node *dn,
> int err, ret;
> int count;
> struct of_phandle_iterator it;
> + const struct of_device_id *match_id;
>
> count = of_count_phandle_with_args(dn, "domain-idle-states", NULL);
> if (count <= 0)
> @@ -2130,6 +2126,9 @@ int of_genpd_parse_idle_states(struct device_node *dn,
> /* 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;
> + match_id = of_match_node(idle_state_match, np);
> + if (!match_id)
> + continue;
Earlier we have allocated "count" numbers of struct genpd_power_state,
by using a kcalloc().
This change may lead to that we could have allocated more memory than
actually needed - because there may be some nodes that doesn't match.
Perhaps it's better to do a pre-iteration to find the real numbers of
how many struct genpd_power_state we actually need to allocate!?
> ret = genpd_parse_state(&st[i++], np);
> if (ret) {
> pr_err
> @@ -2141,8 +2140,11 @@ int of_genpd_parse_idle_states(struct device_node *dn,
> }
> }
>
> - *n = count;
> - *states = st;
> + *n = i;
> + if (!i)
> + kfree(st);
> + else
> + *states = st;
>
> return 0;
> }
> --
> 2.7.4
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/2] PM / Domains: Support in context powering off parent domain
2017-02-08 16:34 ` Lina Iyer
@ 2017-02-08 16:34 ` Lina Iyer
-1 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:34 UTC (permalink / raw)
To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
Cc: andy.gross, sboyd, linux-arm-msm, brendan.jackman,
lorenzo.pieralisi, sudeep.holla, Juri.Lelli, Lina Iyer
Powering off a domain schedules a work to opportunistically power off
the parent domains. Domains that are IRQ safe may have parents that are
also IRQ safe. It would be beneficial to power off such IRQ safe parents
in the same context as well.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
drivers/base/power/domain.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 3825bb9..51e2254 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -375,7 +375,8 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
* If all of the @genpd's devices have been suspended and all of its subdomains
* have been powered down, remove power from @genpd.
*/
-static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
+static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async,
+ unsigned int depth)
{
struct pm_domain_data *pdd;
struct gpd_link *link;
@@ -442,7 +443,17 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
list_for_each_entry(link, &genpd->slave_links, slave_node) {
genpd_sd_counter_dec(link->master);
- genpd_queue_power_off_work(link->master);
+ /*
+ * Power off the parent in the same context if the parent
+ * domain is also IRQ safe.
+ */
+ if (genpd_is_irq_safe(genpd) &&
+ genpd_is_irq_safe(link->master)) {
+ genpd_lock_nested(link->master, depth + 1);
+ genpd_power_off(link->master, false, depth + 1);
+ genpd_unlock(link->master);
+ } else
+ genpd_queue_power_off_work(link->master);
}
return 0;
@@ -459,7 +470,7 @@ static void genpd_power_off_work_fn(struct work_struct *work)
genpd = container_of(work, struct generic_pm_domain, power_off_work);
genpd_lock(genpd);
- genpd_power_off(genpd, true);
+ genpd_power_off(genpd, true, 0);
genpd_unlock(genpd);
}
@@ -578,7 +589,7 @@ static int genpd_runtime_suspend(struct device *dev)
return 0;
genpd_lock(genpd);
- genpd_power_off(genpd, false);
+ genpd_power_off(genpd, false, 0);
genpd_unlock(genpd);
return 0;
@@ -658,7 +669,7 @@ static int genpd_runtime_resume(struct device *dev)
if (!pm_runtime_is_irq_safe(dev) ||
(pm_runtime_is_irq_safe(dev) && genpd_is_irq_safe(genpd))) {
genpd_lock(genpd);
- genpd_power_off(genpd, 0);
+ genpd_power_off(genpd, 0, 0);
genpd_unlock(genpd);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 2/2] PM / Domains: Support in context powering off parent domain
@ 2017-02-08 16:34 ` Lina Iyer
0 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2017-02-08 16:34 UTC (permalink / raw)
To: linux-arm-kernel
Powering off a domain schedules a work to opportunistically power off
the parent domains. Domains that are IRQ safe may have parents that are
also IRQ safe. It would be beneficial to power off such IRQ safe parents
in the same context as well.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
drivers/base/power/domain.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 3825bb9..51e2254 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -375,7 +375,8 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
* If all of the @genpd's devices have been suspended and all of its subdomains
* have been powered down, remove power from @genpd.
*/
-static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
+static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async,
+ unsigned int depth)
{
struct pm_domain_data *pdd;
struct gpd_link *link;
@@ -442,7 +443,17 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
list_for_each_entry(link, &genpd->slave_links, slave_node) {
genpd_sd_counter_dec(link->master);
- genpd_queue_power_off_work(link->master);
+ /*
+ * Power off the parent in the same context if the parent
+ * domain is also IRQ safe.
+ */
+ if (genpd_is_irq_safe(genpd) &&
+ genpd_is_irq_safe(link->master)) {
+ genpd_lock_nested(link->master, depth + 1);
+ genpd_power_off(link->master, false, depth + 1);
+ genpd_unlock(link->master);
+ } else
+ genpd_queue_power_off_work(link->master);
}
return 0;
@@ -459,7 +470,7 @@ static void genpd_power_off_work_fn(struct work_struct *work)
genpd = container_of(work, struct generic_pm_domain, power_off_work);
genpd_lock(genpd);
- genpd_power_off(genpd, true);
+ genpd_power_off(genpd, true, 0);
genpd_unlock(genpd);
}
@@ -578,7 +589,7 @@ static int genpd_runtime_suspend(struct device *dev)
return 0;
genpd_lock(genpd);
- genpd_power_off(genpd, false);
+ genpd_power_off(genpd, false, 0);
genpd_unlock(genpd);
return 0;
@@ -658,7 +669,7 @@ static int genpd_runtime_resume(struct device *dev)
if (!pm_runtime_is_irq_safe(dev) ||
(pm_runtime_is_irq_safe(dev) && genpd_is_irq_safe(genpd))) {
genpd_lock(genpd);
- genpd_power_off(genpd, 0);
+ genpd_power_off(genpd, 0, 0);
genpd_unlock(genpd);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 2/2] PM / Domains: Support in context powering off parent domain
2017-02-08 16:34 ` Lina Iyer
@ 2017-02-09 9:02 ` Ulf Hansson
-1 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2017-02-09 9:02 UTC (permalink / raw)
To: Lina Iyer
Cc: Kevin Hilman, Rafael J. Wysocki, linux-pm, linux-arm-kernel,
Andy Gross, Stephen Boyd, linux-arm-msm, Brendan Jackman,
Lorenzo Pieralisi, Sudeep Holla, Juri Lelli
On 8 February 2017 at 17:34, Lina Iyer <lina.iyer@linaro.org> wrote:
> Powering off a domain schedules a work to opportunistically power off
> the parent domains. Domains that are IRQ safe may have parents that are
> also IRQ safe. It would be beneficial to power off such IRQ safe parents
> in the same context as well.
>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> drivers/base/power/domain.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 3825bb9..51e2254 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -375,7 +375,8 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
> * If all of the @genpd's devices have been suspended and all of its subdomains
> * have been powered down, remove power from @genpd.
> */
> -static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
> +static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async,
> + unsigned int depth)
> {
> struct pm_domain_data *pdd;
> struct gpd_link *link;
> @@ -442,7 +443,17 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
>
> list_for_each_entry(link, &genpd->slave_links, slave_node) {
> genpd_sd_counter_dec(link->master);
> - genpd_queue_power_off_work(link->master);
> + /*
> + * Power off the parent in the same context if the parent
> + * domain is also IRQ safe.
> + */
> + if (genpd_is_irq_safe(genpd) &&
> + genpd_is_irq_safe(link->master)) {
> + genpd_lock_nested(link->master, depth + 1);
> + genpd_power_off(link->master, false, depth + 1);
This doesn't work. You must not call genpd_power_off() using "false" here.
That's because "true" in the recursive call, for the master domain
tells genpd_power_off() that is has been called from genpd's
->runtime_suspend() callback. That means genpd_power_off() thinks it's
okay to allow *one* device in the domain to not be runtime suspended
when allowing a power off to be done. This assumption is not correct
for the master domain.
> + genpd_unlock(link->master);
> + } else
> + genpd_queue_power_off_work(link->master);
> }
>
> return 0;
> @@ -459,7 +470,7 @@ static void genpd_power_off_work_fn(struct work_struct *work)
> genpd = container_of(work, struct generic_pm_domain, power_off_work);
>
> genpd_lock(genpd);
> - genpd_power_off(genpd, true);
> + genpd_power_off(genpd, true, 0);
> genpd_unlock(genpd);
> }
>
> @@ -578,7 +589,7 @@ static int genpd_runtime_suspend(struct device *dev)
> return 0;
>
> genpd_lock(genpd);
> - genpd_power_off(genpd, false);
> + genpd_power_off(genpd, false, 0);
> genpd_unlock(genpd);
>
> return 0;
> @@ -658,7 +669,7 @@ static int genpd_runtime_resume(struct device *dev)
> if (!pm_runtime_is_irq_safe(dev) ||
> (pm_runtime_is_irq_safe(dev) && genpd_is_irq_safe(genpd))) {
> genpd_lock(genpd);
> - genpd_power_off(genpd, 0);
> + genpd_power_off(genpd, 0, 0);
> genpd_unlock(genpd);
> }
>
> --
> 2.7.4
>
Some more thoughts..
Actually, I have been thinking of changing genpd to avoid queuing
power off works, no matter if the PM domain are IRQ safe or not. There
are several reasons, but primarily it helps to avoid wasting power.
Currently I don't see any reasons to why such change shouldn't be
feasible. As a matter of fact, changing this became possible while we
removed the intermediate states in genpd in commit ba2bbfbf6307 ("PM /
Domains: Remove intermediate states from the power off sequence").
Allow me to help out and cook a patch for this, it's already in the pipe. :-)
Kind regards
Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/2] PM / Domains: Support in context powering off parent domain
@ 2017-02-09 9:02 ` Ulf Hansson
0 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2017-02-09 9:02 UTC (permalink / raw)
To: linux-arm-kernel
On 8 February 2017 at 17:34, Lina Iyer <lina.iyer@linaro.org> wrote:
> Powering off a domain schedules a work to opportunistically power off
> the parent domains. Domains that are IRQ safe may have parents that are
> also IRQ safe. It would be beneficial to power off such IRQ safe parents
> in the same context as well.
>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> drivers/base/power/domain.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 3825bb9..51e2254 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -375,7 +375,8 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
> * If all of the @genpd's devices have been suspended and all of its subdomains
> * have been powered down, remove power from @genpd.
> */
> -static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
> +static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async,
> + unsigned int depth)
> {
> struct pm_domain_data *pdd;
> struct gpd_link *link;
> @@ -442,7 +443,17 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
>
> list_for_each_entry(link, &genpd->slave_links, slave_node) {
> genpd_sd_counter_dec(link->master);
> - genpd_queue_power_off_work(link->master);
> + /*
> + * Power off the parent in the same context if the parent
> + * domain is also IRQ safe.
> + */
> + if (genpd_is_irq_safe(genpd) &&
> + genpd_is_irq_safe(link->master)) {
> + genpd_lock_nested(link->master, depth + 1);
> + genpd_power_off(link->master, false, depth + 1);
This doesn't work. You must not call genpd_power_off() using "false" here.
That's because "true" in the recursive call, for the master domain
tells genpd_power_off() that is has been called from genpd's
->runtime_suspend() callback. That means genpd_power_off() thinks it's
okay to allow *one* device in the domain to not be runtime suspended
when allowing a power off to be done. This assumption is not correct
for the master domain.
> + genpd_unlock(link->master);
> + } else
> + genpd_queue_power_off_work(link->master);
> }
>
> return 0;
> @@ -459,7 +470,7 @@ static void genpd_power_off_work_fn(struct work_struct *work)
> genpd = container_of(work, struct generic_pm_domain, power_off_work);
>
> genpd_lock(genpd);
> - genpd_power_off(genpd, true);
> + genpd_power_off(genpd, true, 0);
> genpd_unlock(genpd);
> }
>
> @@ -578,7 +589,7 @@ static int genpd_runtime_suspend(struct device *dev)
> return 0;
>
> genpd_lock(genpd);
> - genpd_power_off(genpd, false);
> + genpd_power_off(genpd, false, 0);
> genpd_unlock(genpd);
>
> return 0;
> @@ -658,7 +669,7 @@ static int genpd_runtime_resume(struct device *dev)
> if (!pm_runtime_is_irq_safe(dev) ||
> (pm_runtime_is_irq_safe(dev) && genpd_is_irq_safe(genpd))) {
> genpd_lock(genpd);
> - genpd_power_off(genpd, 0);
> + genpd_power_off(genpd, 0, 0);
> genpd_unlock(genpd);
> }
>
> --
> 2.7.4
>
Some more thoughts..
Actually, I have been thinking of changing genpd to avoid queuing
power off works, no matter if the PM domain are IRQ safe or not. There
are several reasons, but primarily it helps to avoid wasting power.
Currently I don't see any reasons to why such change shouldn't be
feasible. As a matter of fact, changing this became possible while we
removed the intermediate states in genpd in commit ba2bbfbf6307 ("PM /
Domains: Remove intermediate states from the power off sequence").
Allow me to help out and cook a patch for this, it's already in the pipe. :-)
Kind regards
Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/2] PM / Domains: Support in context powering off parent domain
2017-02-09 9:02 ` Ulf Hansson
@ 2017-02-09 9:03 ` Ulf Hansson
-1 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2017-02-09 9:03 UTC (permalink / raw)
To: Lina Iyer
Cc: Lorenzo Pieralisi, Juri Lelli, linux-pm, Rafael J. Wysocki,
Kevin Hilman, Stephen Boyd, Sudeep Holla, Brendan Jackman,
linux-arm-msm, Andy Gross, linux-arm-kernel
On 9 February 2017 at 10:02, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 8 February 2017 at 17:34, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Powering off a domain schedules a work to opportunistically power off
>> the parent domains. Domains that are IRQ safe may have parents that are
>> also IRQ safe. It would be beneficial to power off such IRQ safe parents
>> in the same context as well.
>>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>> drivers/base/power/domain.c | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 3825bb9..51e2254 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -375,7 +375,8 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
>> * If all of the @genpd's devices have been suspended and all of its subdomains
>> * have been powered down, remove power from @genpd.
>> */
>> -static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
>> +static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async,
>> + unsigned int depth)
>> {
>> struct pm_domain_data *pdd;
>> struct gpd_link *link;
>> @@ -442,7 +443,17 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
>>
>> list_for_each_entry(link, &genpd->slave_links, slave_node) {
>> genpd_sd_counter_dec(link->master);
>> - genpd_queue_power_off_work(link->master);
>> + /*
>> + * Power off the parent in the same context if the parent
>> + * domain is also IRQ safe.
>> + */
>> + if (genpd_is_irq_safe(genpd) &&
>> + genpd_is_irq_safe(link->master)) {
>> + genpd_lock_nested(link->master, depth + 1);
>> + genpd_power_off(link->master, false, depth + 1);
>
> This doesn't work. You must not call genpd_power_off() using "false" here.
>
> That's because "true" in the recursive call, for the master domain
/s/true/false
> tells genpd_power_off() that is has been called from genpd's
> ->runtime_suspend() callback. That means genpd_power_off() thinks it's
> okay to allow *one* device in the domain to not be runtime suspended
> when allowing a power off to be done. This assumption is not correct
> for the master domain.
>
>> + genpd_unlock(link->master);
>> + } else
>> + genpd_queue_power_off_work(link->master);
>> }
>>
>> return 0;
>> @@ -459,7 +470,7 @@ static void genpd_power_off_work_fn(struct work_struct *work)
>> genpd = container_of(work, struct generic_pm_domain, power_off_work);
>>
>> genpd_lock(genpd);
>> - genpd_power_off(genpd, true);
>> + genpd_power_off(genpd, true, 0);
>> genpd_unlock(genpd);
>> }
>>
>> @@ -578,7 +589,7 @@ static int genpd_runtime_suspend(struct device *dev)
>> return 0;
>>
>> genpd_lock(genpd);
>> - genpd_power_off(genpd, false);
>> + genpd_power_off(genpd, false, 0);
>> genpd_unlock(genpd);
>>
>> return 0;
>> @@ -658,7 +669,7 @@ static int genpd_runtime_resume(struct device *dev)
>> if (!pm_runtime_is_irq_safe(dev) ||
>> (pm_runtime_is_irq_safe(dev) && genpd_is_irq_safe(genpd))) {
>> genpd_lock(genpd);
>> - genpd_power_off(genpd, 0);
>> + genpd_power_off(genpd, 0, 0);
>> genpd_unlock(genpd);
>> }
>>
>> --
>> 2.7.4
>>
>
> Some more thoughts..
>
> Actually, I have been thinking of changing genpd to avoid queuing
> power off works, no matter if the PM domain are IRQ safe or not. There
> are several reasons, but primarily it helps to avoid wasting power.
>
> Currently I don't see any reasons to why such change shouldn't be
> feasible. As a matter of fact, changing this became possible while we
> removed the intermediate states in genpd in commit ba2bbfbf6307 ("PM /
> Domains: Remove intermediate states from the power off sequence").
>
> Allow me to help out and cook a patch for this, it's already in the pipe. :-)
>
> Kind regards
> Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/2] PM / Domains: Support in context powering off parent domain
@ 2017-02-09 9:03 ` Ulf Hansson
0 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2017-02-09 9:03 UTC (permalink / raw)
To: linux-arm-kernel
On 9 February 2017 at 10:02, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 8 February 2017 at 17:34, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Powering off a domain schedules a work to opportunistically power off
>> the parent domains. Domains that are IRQ safe may have parents that are
>> also IRQ safe. It would be beneficial to power off such IRQ safe parents
>> in the same context as well.
>>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>> drivers/base/power/domain.c | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 3825bb9..51e2254 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -375,7 +375,8 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
>> * If all of the @genpd's devices have been suspended and all of its subdomains
>> * have been powered down, remove power from @genpd.
>> */
>> -static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
>> +static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async,
>> + unsigned int depth)
>> {
>> struct pm_domain_data *pdd;
>> struct gpd_link *link;
>> @@ -442,7 +443,17 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool is_async)
>>
>> list_for_each_entry(link, &genpd->slave_links, slave_node) {
>> genpd_sd_counter_dec(link->master);
>> - genpd_queue_power_off_work(link->master);
>> + /*
>> + * Power off the parent in the same context if the parent
>> + * domain is also IRQ safe.
>> + */
>> + if (genpd_is_irq_safe(genpd) &&
>> + genpd_is_irq_safe(link->master)) {
>> + genpd_lock_nested(link->master, depth + 1);
>> + genpd_power_off(link->master, false, depth + 1);
>
> This doesn't work. You must not call genpd_power_off() using "false" here.
>
> That's because "true" in the recursive call, for the master domain
/s/true/false
> tells genpd_power_off() that is has been called from genpd's
> ->runtime_suspend() callback. That means genpd_power_off() thinks it's
> okay to allow *one* device in the domain to not be runtime suspended
> when allowing a power off to be done. This assumption is not correct
> for the master domain.
>
>> + genpd_unlock(link->master);
>> + } else
>> + genpd_queue_power_off_work(link->master);
>> }
>>
>> return 0;
>> @@ -459,7 +470,7 @@ static void genpd_power_off_work_fn(struct work_struct *work)
>> genpd = container_of(work, struct generic_pm_domain, power_off_work);
>>
>> genpd_lock(genpd);
>> - genpd_power_off(genpd, true);
>> + genpd_power_off(genpd, true, 0);
>> genpd_unlock(genpd);
>> }
>>
>> @@ -578,7 +589,7 @@ static int genpd_runtime_suspend(struct device *dev)
>> return 0;
>>
>> genpd_lock(genpd);
>> - genpd_power_off(genpd, false);
>> + genpd_power_off(genpd, false, 0);
>> genpd_unlock(genpd);
>>
>> return 0;
>> @@ -658,7 +669,7 @@ static int genpd_runtime_resume(struct device *dev)
>> if (!pm_runtime_is_irq_safe(dev) ||
>> (pm_runtime_is_irq_safe(dev) && genpd_is_irq_safe(genpd))) {
>> genpd_lock(genpd);
>> - genpd_power_off(genpd, 0);
>> + genpd_power_off(genpd, 0, 0);
>> genpd_unlock(genpd);
>> }
>>
>> --
>> 2.7.4
>>
>
> Some more thoughts..
>
> Actually, I have been thinking of changing genpd to avoid queuing
> power off works, no matter if the PM domain are IRQ safe or not. There
> are several reasons, but primarily it helps to avoid wasting power.
>
> Currently I don't see any reasons to why such change shouldn't be
> feasible. As a matter of fact, changing this became possible while we
> removed the intermediate states in genpd in commit ba2bbfbf6307 ("PM /
> Domains: Remove intermediate states from the power off sequence").
>
> Allow me to help out and cook a patch for this, it's already in the pipe. :-)
>
> Kind regards
> Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-03 21:54 [PATCH] PM / Domains: Fix for domain idle state Lina Iyer
@ 2016-11-03 21:54 ` Lina Iyer
0 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2016-11-03 21:54 UTC (permalink / raw)
To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
Cc: andy.gross, sboyd, linux-arm-msm, brendan.jackman,
lorenzo.pieralisi, sudeep.holla, Juri.Lelli, Lina Iyer,
devicetree, Rob Herring
Re-using idle state definition provided by arm,idle-state for domain
idle states creates a lot of confusion and limits further evolution of
the domain idle definition. To keep things clear and simple, define a
idle states for domain using a new compatible "domain-idle-state".
Fix existing PM domains code to look for the newly defined compatible.
Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
.../devicetree/bindings/power/power_domain.txt | 8 +++---
drivers/base/power/domain.c | 2 +-
3 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
new file mode 100644
index 0000000..eefc7ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
@@ -0,0 +1,33 @@
+PM Domain Idle State Node:
+
+A domain idle state node represents the state parameters that will be used to
+select the state when there are no active components in the domain.
+
+The state node has the following parameters -
+
+- compatible:
+ Usage: Required
+ Value type: <string>
+ Definition: Must be "domain-idle-state".
+
+- entry-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency in
+ microseconds required to enter the idle state.
+ The exit-latency-us duration may be guaranteed
+ only after entry-latency-us has passed.
+
+- exit-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency
+ in microseconds required to exit the idle state.
+
+- min-residency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing minimum residency duration
+ in microseconds after which the idle state will yield
+ power benefits after overcoming the overhead in entering
+i the idle state.
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e165036..723e1ad 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -31,7 +31,7 @@ Optional properties:
- 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].
+ compatible with domain-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
@@ -85,7 +85,7 @@ Example 3:
};
DOMAIN_RET: state@0 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x0>;
entry-latency-us = <1000>;
exit-latency-us = <2000>;
@@ -93,7 +93,7 @@ Example 3:
};
DOMAIN_PWR_DN: state@1 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x1>;
entry-latency-us = <5000>;
exit-latency-us = <8000>;
@@ -118,4 +118,4 @@ 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
+[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 661737c..f0bc672 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
static const struct of_device_id idle_state_match[] = {
- { .compatible = "arm,idle-state", },
+ { .compatible = "domain-idle-state", },
{ }
};
--
2.7.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-03 21:54 ` Lina Iyer
0 siblings, 0 replies; 40+ messages in thread
From: Lina Iyer @ 2016-11-03 21:54 UTC (permalink / raw)
To: linux-arm-kernel
Re-using idle state definition provided by arm,idle-state for domain
idle states creates a lot of confusion and limits further evolution of
the domain idle definition. To keep things clear and simple, define a
idle states for domain using a new compatible "domain-idle-state".
Fix existing PM domains code to look for the newly defined compatible.
Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
.../devicetree/bindings/power/power_domain.txt | 8 +++---
drivers/base/power/domain.c | 2 +-
3 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
new file mode 100644
index 0000000..eefc7ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
@@ -0,0 +1,33 @@
+PM Domain Idle State Node:
+
+A domain idle state node represents the state parameters that will be used to
+select the state when there are no active components in the domain.
+
+The state node has the following parameters -
+
+- compatible:
+ Usage: Required
+ Value type: <string>
+ Definition: Must be "domain-idle-state".
+
+- entry-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency in
+ microseconds required to enter the idle state.
+ The exit-latency-us duration may be guaranteed
+ only after entry-latency-us has passed.
+
+- exit-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency
+ in microseconds required to exit the idle state.
+
+- min-residency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing minimum residency duration
+ in microseconds after which the idle state will yield
+ power benefits after overcoming the overhead in entering
+i the idle state.
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e165036..723e1ad 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -31,7 +31,7 @@ Optional properties:
- 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].
+ compatible with domain-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
@@ -85,7 +85,7 @@ Example 3:
};
DOMAIN_RET: state at 0 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x0>;
entry-latency-us = <1000>;
exit-latency-us = <2000>;
@@ -93,7 +93,7 @@ Example 3:
};
DOMAIN_PWR_DN: state at 1 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x1>;
entry-latency-us = <5000>;
exit-latency-us = <8000>;
@@ -118,4 +118,4 @@ 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
+[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 661737c..f0bc672 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
static const struct of_device_id idle_state_match[] = {
- { .compatible = "arm,idle-state", },
+ { .compatible = "domain-idle-state", },
{ }
};
--
2.7.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-03 21:54 ` Lina Iyer
@ 2016-11-07 11:14 ` Ulf Hansson
-1 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2016-11-07 11:14 UTC (permalink / raw)
To: Lina Iyer
Cc: Kevin Hilman, Rafael J. Wysocki, linux-pm, linux-arm-kernel,
Andy Gross, Stephen Boyd, linux-arm-msm, Brendan Jackman,
Lorenzo Pieralisi, Sudeep Holla, Juri Lelli, devicetree,
Rob Herring
On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> .../devicetree/bindings/power/power_domain.txt | 8 +++---
> drivers/base/power/domain.c | 2 +-
> 3 files changed, 38 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> + Usage: Required
> + Value type: <string>
> + Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency in
> + microseconds required to enter the idle state.
> + The exit-latency-us duration may be guaranteed
> + only after entry-latency-us has passed.
As we anyway are going to change this, why not use an u64 and have the
value in ns instead of us?
That should give us better flexibility and I think this would also be
what Rob would recommend, if I remember earlier similar comments from
him.
> +
> +- exit-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency
> + in microseconds required to exit the idle state.
Ditto.
> +
> +- min-residency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing minimum residency duration
> + in microseconds after which the idle state will yield
> + power benefits after overcoming the overhead in entering
> +i the idle state.
Ditto.
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
> - 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].
> + compatible with domain-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
> @@ -85,7 +85,7 @@ Example 3:
> };
>
> DOMAIN_RET: state@0 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x0>;
> entry-latency-us = <1000>;
> exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
> };
>
> DOMAIN_PWR_DN: state@1 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x1>;
> entry-latency-us = <5000>;
> exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ 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
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
> - { .compatible = "arm,idle-state", },
> + { .compatible = "domain-idle-state", },
> { }
> };
>
> --
> 2.7.4
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-07 11:14 ` Ulf Hansson
0 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2016-11-07 11:14 UTC (permalink / raw)
To: linux-arm-kernel
On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> .../devicetree/bindings/power/power_domain.txt | 8 +++---
> drivers/base/power/domain.c | 2 +-
> 3 files changed, 38 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> + Usage: Required
> + Value type: <string>
> + Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency in
> + microseconds required to enter the idle state.
> + The exit-latency-us duration may be guaranteed
> + only after entry-latency-us has passed.
As we anyway are going to change this, why not use an u64 and have the
value in ns instead of us?
That should give us better flexibility and I think this would also be
what Rob would recommend, if I remember earlier similar comments from
him.
> +
> +- exit-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency
> + in microseconds required to exit the idle state.
Ditto.
> +
> +- min-residency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing minimum residency duration
> + in microseconds after which the idle state will yield
> + power benefits after overcoming the overhead in entering
> +i the idle state.
Ditto.
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
> - 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].
> + compatible with domain-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
> @@ -85,7 +85,7 @@ Example 3:
> };
>
> DOMAIN_RET: state at 0 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x0>;
> entry-latency-us = <1000>;
> exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
> };
>
> DOMAIN_PWR_DN: state at 1 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x1>;
> entry-latency-us = <5000>;
> exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ 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
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
> - { .compatible = "arm,idle-state", },
> + { .compatible = "domain-idle-state", },
> { }
> };
>
> --
> 2.7.4
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <CAPDyKFoCjf1qSBDWUY_wNx21_78fRrFqcqrFbsSmabzAZJxQAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-07 11:14 ` Ulf Hansson
@ 2016-11-10 19:58 ` Rob Herring
-1 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2016-11-10 19:58 UTC (permalink / raw)
To: Ulf Hansson
Cc: Lina Iyer, Kevin Hilman, Rafael J. Wysocki,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Andy Gross,
Stephen Boyd, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
Brendan Jackman, Lorenzo Pieralisi, Sudeep Holla, Juri Lelli,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > Re-using idle state definition provided by arm,idle-state for domain
> > idle states creates a lot of confusion and limits further evolution of
> > the domain idle definition. To keep things clear and simple, define a
> > idle states for domain using a new compatible "domain-idle-state".
> >
> > Fix existing PM domains code to look for the newly defined compatible.
> >
> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> > drivers/base/power/domain.c | 2 +-
> > 3 files changed, 38 insertions(+), 5 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > new file mode 100644
> > index 0000000..eefc7ed
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > @@ -0,0 +1,33 @@
> > +PM Domain Idle State Node:
> > +
> > +A domain idle state node represents the state parameters that will be used to
> > +select the state when there are no active components in the domain.
> > +
> > +The state node has the following parameters -
> > +
> > +- compatible:
> > + Usage: Required
> > + Value type: <string>
> > + Definition: Must be "domain-idle-state".
> > +
> > +- entry-latency-us
> > + Usage: Required
> > + Value type: <prop-encoded-array>
> > + Definition: u32 value representing worst case latency in
> > + microseconds required to enter the idle state.
> > + The exit-latency-us duration may be guaranteed
> > + only after entry-latency-us has passed.
>
> As we anyway are going to change this, why not use an u64 and have the
> value in ns instead of us?
I can't imagine that you would need more resolution or range. For times
less than 1us, s/w and register access times are going to dominate the
time.
Unless there is a real need, I'd keep alignment with the existing
binding.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-10 19:58 ` Rob Herring
0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2016-11-10 19:58 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> > Re-using idle state definition provided by arm,idle-state for domain
> > idle states creates a lot of confusion and limits further evolution of
> > the domain idle definition. To keep things clear and simple, define a
> > idle states for domain using a new compatible "domain-idle-state".
> >
> > Fix existing PM domains code to look for the newly defined compatible.
> >
> > Cc: <devicetree@vger.kernel.org>
> > Cc: Rob Herring <robh@kernel.org>
> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> > ---
> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> > drivers/base/power/domain.c | 2 +-
> > 3 files changed, 38 insertions(+), 5 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > new file mode 100644
> > index 0000000..eefc7ed
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > @@ -0,0 +1,33 @@
> > +PM Domain Idle State Node:
> > +
> > +A domain idle state node represents the state parameters that will be used to
> > +select the state when there are no active components in the domain.
> > +
> > +The state node has the following parameters -
> > +
> > +- compatible:
> > + Usage: Required
> > + Value type: <string>
> > + Definition: Must be "domain-idle-state".
> > +
> > +- entry-latency-us
> > + Usage: Required
> > + Value type: <prop-encoded-array>
> > + Definition: u32 value representing worst case latency in
> > + microseconds required to enter the idle state.
> > + The exit-latency-us duration may be guaranteed
> > + only after entry-latency-us has passed.
>
> As we anyway are going to change this, why not use an u64 and have the
> value in ns instead of us?
I can't imagine that you would need more resolution or range. For times
less than 1us, s/w and register access times are going to dominate the
time.
Unless there is a real need, I'd keep alignment with the existing
binding.
Rob
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-10 19:58 ` Rob Herring
@ 2016-11-11 11:52 ` Ulf Hansson
-1 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2016-11-11 11:52 UTC (permalink / raw)
To: Rob Herring
Cc: Lina Iyer, Kevin Hilman, Rafael J. Wysocki,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Andy Gross,
Stephen Boyd, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
Brendan Jackman, Lorenzo Pieralisi, Sudeep Holla, Juri Lelli,
devicetree-u79uwXL29TY76Z2rM5mHXA
On 10 November 2016 at 20:58, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> > On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > > Re-using idle state definition provided by arm,idle-state for domain
> > > idle states creates a lot of confusion and limits further evolution of
> > > the domain idle definition. To keep things clear and simple, define a
> > > idle states for domain using a new compatible "domain-idle-state".
> > >
> > > Fix existing PM domains code to look for the newly defined compatible.
> > >
> > > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > ---
> > > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> > > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> > > drivers/base/power/domain.c | 2 +-
> > > 3 files changed, 38 insertions(+), 5 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > > new file mode 100644
> > > index 0000000..eefc7ed
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > > @@ -0,0 +1,33 @@
> > > +PM Domain Idle State Node:
> > > +
> > > +A domain idle state node represents the state parameters that will be used to
> > > +select the state when there are no active components in the domain.
> > > +
> > > +The state node has the following parameters -
> > > +
> > > +- compatible:
> > > + Usage: Required
> > > + Value type: <string>
> > > + Definition: Must be "domain-idle-state".
> > > +
> > > +- entry-latency-us
> > > + Usage: Required
> > > + Value type: <prop-encoded-array>
> > > + Definition: u32 value representing worst case latency in
> > > + microseconds required to enter the idle state.
> > > + The exit-latency-us duration may be guaranteed
> > > + only after entry-latency-us has passed.
> >
> > As we anyway are going to change this, why not use an u64 and have the
> > value in ns instead of us?
>
> I can't imagine that you would need more resolution or range. For times
> less than 1us, s/w and register access times are going to dominate the
> time.
Yep.
>
>
> Unless there is a real need, I'd keep alignment with the existing
> binding.
Agree!
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-11 11:52 ` Ulf Hansson
0 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2016-11-11 11:52 UTC (permalink / raw)
To: linux-arm-kernel
On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> > On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> > > Re-using idle state definition provided by arm,idle-state for domain
> > > idle states creates a lot of confusion and limits further evolution of
> > > the domain idle definition. To keep things clear and simple, define a
> > > idle states for domain using a new compatible "domain-idle-state".
> > >
> > > Fix existing PM domains code to look for the newly defined compatible.
> > >
> > > Cc: <devicetree@vger.kernel.org>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> > > ---
> > > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> > > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> > > drivers/base/power/domain.c | 2 +-
> > > 3 files changed, 38 insertions(+), 5 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > > new file mode 100644
> > > index 0000000..eefc7ed
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > > @@ -0,0 +1,33 @@
> > > +PM Domain Idle State Node:
> > > +
> > > +A domain idle state node represents the state parameters that will be used to
> > > +select the state when there are no active components in the domain.
> > > +
> > > +The state node has the following parameters -
> > > +
> > > +- compatible:
> > > + Usage: Required
> > > + Value type: <string>
> > > + Definition: Must be "domain-idle-state".
> > > +
> > > +- entry-latency-us
> > > + Usage: Required
> > > + Value type: <prop-encoded-array>
> > > + Definition: u32 value representing worst case latency in
> > > + microseconds required to enter the idle state.
> > > + The exit-latency-us duration may be guaranteed
> > > + only after entry-latency-us has passed.
> >
> > As we anyway are going to change this, why not use an u64 and have the
> > value in ns instead of us?
>
> I can't imagine that you would need more resolution or range. For times
> less than 1us, s/w and register access times are going to dominate the
> time.
Yep.
>
>
> Unless there is a real need, I'd keep alignment with the existing
> binding.
Agree!
Kind regards
Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-10 19:58 ` Rob Herring
@ 2016-11-29 8:47 ` Ulf Hansson
-1 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2016-11-29 8:47 UTC (permalink / raw)
To: Rob Herring
Cc: Lina Iyer, Kevin Hilman, Rafael J. Wysocki,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Andy Gross,
Stephen Boyd, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
Brendan Jackman, Lorenzo Pieralisi, Sudeep Holla, Juri Lelli,
devicetree-u79uwXL29TY76Z2rM5mHXA
On 10 November 2016 at 20:58, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
>> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > Re-using idle state definition provided by arm,idle-state for domain
>> > idle states creates a lot of confusion and limits further evolution of
>> > the domain idle definition. To keep things clear and simple, define a
>> > idle states for domain using a new compatible "domain-idle-state".
>> >
>> > Fix existing PM domains code to look for the newly defined compatible.
>> >
>> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > ---
>> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
>> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
>> > drivers/base/power/domain.c | 2 +-
>> > 3 files changed, 38 insertions(+), 5 deletions(-)
>> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > new file mode 100644
>> > index 0000000..eefc7ed
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > @@ -0,0 +1,33 @@
>> > +PM Domain Idle State Node:
>> > +
>> > +A domain idle state node represents the state parameters that will be used to
>> > +select the state when there are no active components in the domain.
>> > +
>> > +The state node has the following parameters -
>> > +
>> > +- compatible:
>> > + Usage: Required
>> > + Value type: <string>
>> > + Definition: Must be "domain-idle-state".
>> > +
>> > +- entry-latency-us
>> > + Usage: Required
>> > + Value type: <prop-encoded-array>
>> > + Definition: u32 value representing worst case latency in
>> > + microseconds required to enter the idle state.
>> > + The exit-latency-us duration may be guaranteed
>> > + only after entry-latency-us has passed.
>>
>> As we anyway are going to change this, why not use an u64 and have the
>> value in ns instead of us?
>
> I can't imagine that you would need more resolution or range. For times
> less than 1us, s/w and register access times are going to dominate the
> time.
>
> Unless there is a real need, I'd keep alignment with the existing
> binding.
Rob, are you fine with this? I thought it would be great to get this
in for 4.10 rc1.
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-29 8:47 ` Ulf Hansson
0 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2016-11-29 8:47 UTC (permalink / raw)
To: linux-arm-kernel
On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
> On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
>> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
>> > Re-using idle state definition provided by arm,idle-state for domain
>> > idle states creates a lot of confusion and limits further evolution of
>> > the domain idle definition. To keep things clear and simple, define a
>> > idle states for domain using a new compatible "domain-idle-state".
>> >
>> > Fix existing PM domains code to look for the newly defined compatible.
>> >
>> > Cc: <devicetree@vger.kernel.org>
>> > Cc: Rob Herring <robh@kernel.org>
>> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> > ---
>> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
>> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
>> > drivers/base/power/domain.c | 2 +-
>> > 3 files changed, 38 insertions(+), 5 deletions(-)
>> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > new file mode 100644
>> > index 0000000..eefc7ed
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > @@ -0,0 +1,33 @@
>> > +PM Domain Idle State Node:
>> > +
>> > +A domain idle state node represents the state parameters that will be used to
>> > +select the state when there are no active components in the domain.
>> > +
>> > +The state node has the following parameters -
>> > +
>> > +- compatible:
>> > + Usage: Required
>> > + Value type: <string>
>> > + Definition: Must be "domain-idle-state".
>> > +
>> > +- entry-latency-us
>> > + Usage: Required
>> > + Value type: <prop-encoded-array>
>> > + Definition: u32 value representing worst case latency in
>> > + microseconds required to enter the idle state.
>> > + The exit-latency-us duration may be guaranteed
>> > + only after entry-latency-us has passed.
>>
>> As we anyway are going to change this, why not use an u64 and have the
>> value in ns instead of us?
>
> I can't imagine that you would need more resolution or range. For times
> less than 1us, s/w and register access times are going to dominate the
> time.
>
> Unless there is a real need, I'd keep alignment with the existing
> binding.
Rob, are you fine with this? I thought it would be great to get this
in for 4.10 rc1.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-29 8:47 ` Ulf Hansson
@ 2016-12-01 21:21 ` Rafael J. Wysocki
-1 siblings, 0 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2016-12-01 21:21 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Lina Iyer, Kevin Hilman, linux-pm, linux-arm-kernel, Andy Gross,
Stephen Boyd, linux-arm-msm, Brendan Jackman, Lorenzo Pieralisi,
Sudeep Holla, Juri Lelli, devicetree
On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
> On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
> > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> >> > Re-using idle state definition provided by arm,idle-state for domain
> >> > idle states creates a lot of confusion and limits further evolution of
> >> > the domain idle definition. To keep things clear and simple, define a
> >> > idle states for domain using a new compatible "domain-idle-state".
> >> >
> >> > Fix existing PM domains code to look for the newly defined compatible.
> >> >
> >> > Cc: <devicetree@vger.kernel.org>
> >> > Cc: Rob Herring <robh@kernel.org>
> >> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> >> > ---
> >> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> >> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> >> > drivers/base/power/domain.c | 2 +-
> >> > 3 files changed, 38 insertions(+), 5 deletions(-)
> >> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> > new file mode 100644
> >> > index 0000000..eefc7ed
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> > @@ -0,0 +1,33 @@
> >> > +PM Domain Idle State Node:
> >> > +
> >> > +A domain idle state node represents the state parameters that will be used to
> >> > +select the state when there are no active components in the domain.
> >> > +
> >> > +The state node has the following parameters -
> >> > +
> >> > +- compatible:
> >> > + Usage: Required
> >> > + Value type: <string>
> >> > + Definition: Must be "domain-idle-state".
> >> > +
> >> > +- entry-latency-us
> >> > + Usage: Required
> >> > + Value type: <prop-encoded-array>
> >> > + Definition: u32 value representing worst case latency in
> >> > + microseconds required to enter the idle state.
> >> > + The exit-latency-us duration may be guaranteed
> >> > + only after entry-latency-us has passed.
> >>
> >> As we anyway are going to change this, why not use an u64 and have the
> >> value in ns instead of us?
> >
> > I can't imagine that you would need more resolution or range. For times
> > less than 1us, s/w and register access times are going to dominate the
> > time.
> >
> > Unless there is a real need, I'd keep alignment with the existing
> > binding.
>
> Rob, are you fine with this? I thought it would be great to get this
> in for 4.10 rc1.
Rob, any objections here?
Thanks,
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-12-01 21:21 ` Rafael J. Wysocki
0 siblings, 0 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2016-12-01 21:21 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
> On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
> > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> >> > Re-using idle state definition provided by arm,idle-state for domain
> >> > idle states creates a lot of confusion and limits further evolution of
> >> > the domain idle definition. To keep things clear and simple, define a
> >> > idle states for domain using a new compatible "domain-idle-state".
> >> >
> >> > Fix existing PM domains code to look for the newly defined compatible.
> >> >
> >> > Cc: <devicetree@vger.kernel.org>
> >> > Cc: Rob Herring <robh@kernel.org>
> >> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> >> > ---
> >> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> >> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> >> > drivers/base/power/domain.c | 2 +-
> >> > 3 files changed, 38 insertions(+), 5 deletions(-)
> >> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> > new file mode 100644
> >> > index 0000000..eefc7ed
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> > @@ -0,0 +1,33 @@
> >> > +PM Domain Idle State Node:
> >> > +
> >> > +A domain idle state node represents the state parameters that will be used to
> >> > +select the state when there are no active components in the domain.
> >> > +
> >> > +The state node has the following parameters -
> >> > +
> >> > +- compatible:
> >> > + Usage: Required
> >> > + Value type: <string>
> >> > + Definition: Must be "domain-idle-state".
> >> > +
> >> > +- entry-latency-us
> >> > + Usage: Required
> >> > + Value type: <prop-encoded-array>
> >> > + Definition: u32 value representing worst case latency in
> >> > + microseconds required to enter the idle state.
> >> > + The exit-latency-us duration may be guaranteed
> >> > + only after entry-latency-us has passed.
> >>
> >> As we anyway are going to change this, why not use an u64 and have the
> >> value in ns instead of us?
> >
> > I can't imagine that you would need more resolution or range. For times
> > less than 1us, s/w and register access times are going to dominate the
> > time.
> >
> > Unless there is a real need, I'd keep alignment with the existing
> > binding.
>
> Rob, are you fine with this? I thought it would be great to get this
> in for 4.10 rc1.
Rob, any objections here?
Thanks,
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <1616414.c1s7NxTqJS-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>]
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-12-01 21:21 ` Rafael J. Wysocki
@ 2016-12-08 0:13 ` Rafael J. Wysocki
-1 siblings, 0 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2016-12-08 0:13 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Lina Iyer
Cc: Kevin Hilman, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Andy Gross,
Stephen Boyd, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
Brendan Jackman, Lorenzo Pieralisi, Sudeep Holla, Juri Lelli,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Thursday, December 01, 2016 10:21:59 PM Rafael J. Wysocki wrote:
> On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
> > On 10 November 2016 at 20:58, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> > >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > >> > Re-using idle state definition provided by arm,idle-state for domain
> > >> > idle states creates a lot of confusion and limits further evolution of
> > >> > the domain idle definition. To keep things clear and simple, define a
> > >> > idle states for domain using a new compatible "domain-idle-state".
> > >> >
> > >> > Fix existing PM domains code to look for the newly defined compatible.
> > >> >
> > >> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > >> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > >> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > >> > ---
> > >> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> > >> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> > >> > drivers/base/power/domain.c | 2 +-
> > >> > 3 files changed, 38 insertions(+), 5 deletions(-)
> > >> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> > new file mode 100644
> > >> > index 0000000..eefc7ed
> > >> > --- /dev/null
> > >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> > @@ -0,0 +1,33 @@
> > >> > +PM Domain Idle State Node:
> > >> > +
> > >> > +A domain idle state node represents the state parameters that will be used to
> > >> > +select the state when there are no active components in the domain.
> > >> > +
> > >> > +The state node has the following parameters -
> > >> > +
> > >> > +- compatible:
> > >> > + Usage: Required
> > >> > + Value type: <string>
> > >> > + Definition: Must be "domain-idle-state".
> > >> > +
> > >> > +- entry-latency-us
> > >> > + Usage: Required
> > >> > + Value type: <prop-encoded-array>
> > >> > + Definition: u32 value representing worst case latency in
> > >> > + microseconds required to enter the idle state.
> > >> > + The exit-latency-us duration may be guaranteed
> > >> > + only after entry-latency-us has passed.
> > >>
> > >> As we anyway are going to change this, why not use an u64 and have the
> > >> value in ns instead of us?
> > >
> > > I can't imagine that you would need more resolution or range. For times
> > > less than 1us, s/w and register access times are going to dominate the
> > > time.
> > >
> > > Unless there is a real need, I'd keep alignment with the existing
> > > binding.
> >
> > Rob, are you fine with this? I thought it would be great to get this
> > in for 4.10 rc1.
>
> Rob, any objections here?
Well, no objections, so applied.
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-12-08 0:13 ` Rafael J. Wysocki
0 siblings, 0 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2016-12-08 0:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday, December 01, 2016 10:21:59 PM Rafael J. Wysocki wrote:
> On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
> > On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
> > > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> > >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> > >> > Re-using idle state definition provided by arm,idle-state for domain
> > >> > idle states creates a lot of confusion and limits further evolution of
> > >> > the domain idle definition. To keep things clear and simple, define a
> > >> > idle states for domain using a new compatible "domain-idle-state".
> > >> >
> > >> > Fix existing PM domains code to look for the newly defined compatible.
> > >> >
> > >> > Cc: <devicetree@vger.kernel.org>
> > >> > Cc: Rob Herring <robh@kernel.org>
> > >> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> > >> > ---
> > >> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> > >> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> > >> > drivers/base/power/domain.c | 2 +-
> > >> > 3 files changed, 38 insertions(+), 5 deletions(-)
> > >> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> > new file mode 100644
> > >> > index 0000000..eefc7ed
> > >> > --- /dev/null
> > >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> > @@ -0,0 +1,33 @@
> > >> > +PM Domain Idle State Node:
> > >> > +
> > >> > +A domain idle state node represents the state parameters that will be used to
> > >> > +select the state when there are no active components in the domain.
> > >> > +
> > >> > +The state node has the following parameters -
> > >> > +
> > >> > +- compatible:
> > >> > + Usage: Required
> > >> > + Value type: <string>
> > >> > + Definition: Must be "domain-idle-state".
> > >> > +
> > >> > +- entry-latency-us
> > >> > + Usage: Required
> > >> > + Value type: <prop-encoded-array>
> > >> > + Definition: u32 value representing worst case latency in
> > >> > + microseconds required to enter the idle state.
> > >> > + The exit-latency-us duration may be guaranteed
> > >> > + only after entry-latency-us has passed.
> > >>
> > >> As we anyway are going to change this, why not use an u64 and have the
> > >> value in ns instead of us?
> > >
> > > I can't imagine that you would need more resolution or range. For times
> > > less than 1us, s/w and register access times are going to dominate the
> > > time.
> > >
> > > Unless there is a real need, I'd keep alignment with the existing
> > > binding.
> >
> > Rob, are you fine with this? I thought it would be great to get this
> > in for 4.10 rc1.
>
> Rob, any objections here?
Well, no objections, so applied.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-12-08 0:13 ` Rafael J. Wysocki
@ 2016-12-08 16:07 ` Rob Herring
-1 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2016-12-08 16:07 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Ulf Hansson, Lina Iyer, Kevin Hilman, linux-pm, linux-arm-kernel,
Andy Gross, Stephen Boyd, linux-arm-msm, Brendan Jackman,
Lorenzo Pieralisi, Sudeep Holla, Juri Lelli, devicetree
On Wed, Dec 7, 2016 at 6:13 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, December 01, 2016 10:21:59 PM Rafael J. Wysocki wrote:
>> On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
>> > On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
>> > > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
>> > >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
>> > >> > Re-using idle state definition provided by arm,idle-state for domain
>> > >> > idle states creates a lot of confusion and limits further evolution of
>> > >> > the domain idle definition. To keep things clear and simple, define a
>> > >> > idle states for domain using a new compatible "domain-idle-state".
>> > >> >
>> > >> > Fix existing PM domains code to look for the newly defined compatible.
>> > >> >
>> > >> > Cc: <devicetree@vger.kernel.org>
>> > >> > Cc: Rob Herring <robh@kernel.org>
>> > >> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> > >> > ---
>> > >> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
>> > >> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
>> > >> > drivers/base/power/domain.c | 2 +-
>> > >> > 3 files changed, 38 insertions(+), 5 deletions(-)
>> > >> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> >
>> > >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> > new file mode 100644
>> > >> > index 0000000..eefc7ed
>> > >> > --- /dev/null
>> > >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> > @@ -0,0 +1,33 @@
>> > >> > +PM Domain Idle State Node:
>> > >> > +
>> > >> > +A domain idle state node represents the state parameters that will be used to
>> > >> > +select the state when there are no active components in the domain.
>> > >> > +
>> > >> > +The state node has the following parameters -
>> > >> > +
>> > >> > +- compatible:
>> > >> > + Usage: Required
>> > >> > + Value type: <string>
>> > >> > + Definition: Must be "domain-idle-state".
>> > >> > +
>> > >> > +- entry-latency-us
>> > >> > + Usage: Required
>> > >> > + Value type: <prop-encoded-array>
>> > >> > + Definition: u32 value representing worst case latency in
>> > >> > + microseconds required to enter the idle state.
>> > >> > + The exit-latency-us duration may be guaranteed
>> > >> > + only after entry-latency-us has passed.
>> > >>
>> > >> As we anyway are going to change this, why not use an u64 and have the
>> > >> value in ns instead of us?
>> > >
>> > > I can't imagine that you would need more resolution or range. For times
>> > > less than 1us, s/w and register access times are going to dominate the
>> > > time.
>> > >
>> > > Unless there is a real need, I'd keep alignment with the existing
>> > > binding.
>> >
>> > Rob, are you fine with this? I thought it would be great to get this
>> > in for 4.10 rc1.
>>
>> Rob, any objections here?
>
> Well, no objections, so applied.
Sorry, just found my ack sitting in my drafts. Thought I had sent it.
Rob
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-12-08 16:07 ` Rob Herring
0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2016-12-08 16:07 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 7, 2016 at 6:13 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, December 01, 2016 10:21:59 PM Rafael J. Wysocki wrote:
>> On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
>> > On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
>> > > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
>> > >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
>> > >> > Re-using idle state definition provided by arm,idle-state for domain
>> > >> > idle states creates a lot of confusion and limits further evolution of
>> > >> > the domain idle definition. To keep things clear and simple, define a
>> > >> > idle states for domain using a new compatible "domain-idle-state".
>> > >> >
>> > >> > Fix existing PM domains code to look for the newly defined compatible.
>> > >> >
>> > >> > Cc: <devicetree@vger.kernel.org>
>> > >> > Cc: Rob Herring <robh@kernel.org>
>> > >> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> > >> > ---
>> > >> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
>> > >> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
>> > >> > drivers/base/power/domain.c | 2 +-
>> > >> > 3 files changed, 38 insertions(+), 5 deletions(-)
>> > >> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> >
>> > >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> > new file mode 100644
>> > >> > index 0000000..eefc7ed
>> > >> > --- /dev/null
>> > >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> > @@ -0,0 +1,33 @@
>> > >> > +PM Domain Idle State Node:
>> > >> > +
>> > >> > +A domain idle state node represents the state parameters that will be used to
>> > >> > +select the state when there are no active components in the domain.
>> > >> > +
>> > >> > +The state node has the following parameters -
>> > >> > +
>> > >> > +- compatible:
>> > >> > + Usage: Required
>> > >> > + Value type: <string>
>> > >> > + Definition: Must be "domain-idle-state".
>> > >> > +
>> > >> > +- entry-latency-us
>> > >> > + Usage: Required
>> > >> > + Value type: <prop-encoded-array>
>> > >> > + Definition: u32 value representing worst case latency in
>> > >> > + microseconds required to enter the idle state.
>> > >> > + The exit-latency-us duration may be guaranteed
>> > >> > + only after entry-latency-us has passed.
>> > >>
>> > >> As we anyway are going to change this, why not use an u64 and have the
>> > >> value in ns instead of us?
>> > >
>> > > I can't imagine that you would need more resolution or range. For times
>> > > less than 1us, s/w and register access times are going to dominate the
>> > > time.
>> > >
>> > > Unless there is a real need, I'd keep alignment with the existing
>> > > binding.
>> >
>> > Rob, are you fine with this? I thought it would be great to get this
>> > in for 4.10 rc1.
>>
>> Rob, any objections here?
>
> Well, no objections, so applied.
Sorry, just found my ack sitting in my drafts. Thought I had sent it.
Rob
^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <1478210075-92045-2-git-send-email-lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-03 21:54 ` Lina Iyer
@ 2016-11-21 12:37 ` Brendan Jackman
-1 siblings, 0 replies; 40+ messages in thread
From: Brendan Jackman @ 2016-11-21 12:37 UTC (permalink / raw)
To: Lina Iyer
Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
khilman-DgEjT+Ai2ygdnm+yROfE0A, rjw-LthD3rsA81gm4RdzfppkhA,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
andy.gross-QSEj5FYQhm4dnm+yROfE0A, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
lorenzo.pieralisi-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
Juri.Lelli-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
Rob Herring
Hi,
On Thu, Nov 03 2016 at 21:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
This looks good to me, pinging for review from others/queue for merge.
Best,
Brendan
>
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> .../devicetree/bindings/power/power_domain.txt | 8 +++---
> drivers/base/power/domain.c | 2 +-
> 3 files changed, 38 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> + Usage: Required
> + Value type: <string>
> + Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency in
> + microseconds required to enter the idle state.
> + The exit-latency-us duration may be guaranteed
> + only after entry-latency-us has passed.
> +
> +- exit-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency
> + in microseconds required to exit the idle state.
> +
> +- min-residency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing minimum residency duration
> + in microseconds after which the idle state will yield
> + power benefits after overcoming the overhead in entering
> +i the idle state.
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
> - 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].
> + compatible with domain-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
> @@ -85,7 +85,7 @@ Example 3:
> };
>
> DOMAIN_RET: state@0 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x0>;
> entry-latency-us = <1000>;
> exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
> };
>
> DOMAIN_PWR_DN: state@1 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x1>;
> entry-latency-us = <5000>;
> exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ 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
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
> - { .compatible = "arm,idle-state", },
> + { .compatible = "domain-idle-state", },
> { }
> };
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-21 12:37 ` Brendan Jackman
0 siblings, 0 replies; 40+ messages in thread
From: Brendan Jackman @ 2016-11-21 12:37 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Thu, Nov 03 2016 at 21:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
This looks good to me, pinging for review from others/queue for merge.
Best,
Brendan
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> .../devicetree/bindings/power/power_domain.txt | 8 +++---
> drivers/base/power/domain.c | 2 +-
> 3 files changed, 38 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> + Usage: Required
> + Value type: <string>
> + Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency in
> + microseconds required to enter the idle state.
> + The exit-latency-us duration may be guaranteed
> + only after entry-latency-us has passed.
> +
> +- exit-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency
> + in microseconds required to exit the idle state.
> +
> +- min-residency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing minimum residency duration
> + in microseconds after which the idle state will yield
> + power benefits after overcoming the overhead in entering
> +i the idle state.
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
> - 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].
> + compatible with domain-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
> @@ -85,7 +85,7 @@ Example 3:
> };
>
> DOMAIN_RET: state at 0 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x0>;
> entry-latency-us = <1000>;
> exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
> };
>
> DOMAIN_PWR_DN: state at 1 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x1>;
> entry-latency-us = <5000>;
> exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ 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
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
> - { .compatible = "arm,idle-state", },
> + { .compatible = "domain-idle-state", },
> { }
> };
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-21 12:37 ` Brendan Jackman
@ 2016-11-21 13:11 ` Rafael J. Wysocki
-1 siblings, 0 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2016-11-21 13:11 UTC (permalink / raw)
To: Brendan Jackman
Cc: Lina Iyer, Rafael Wysocki, Ulf Hansson, Kevin Hilman, Linux PM,
linux-arm-kernel, Andy Gross, Stephen Boyd, linux-arm-msm,
Lorenzo Pieralisi, Sudeep Holla, Juri Lelli, devicetree,
Rob Herring
On Mon, Nov 21, 2016 at 1:37 PM, Brendan Jackman
<brendan.jackman@arm.com> wrote:
> Hi,
>
> On Thu, Nov 03 2016 at 21:54, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Re-using idle state definition provided by arm,idle-state for domain
>> idle states creates a lot of confusion and limits further evolution of
>> the domain idle definition. To keep things clear and simple, define a
>> idle states for domain using a new compatible "domain-idle-state".
>>
>> Fix existing PM domains code to look for the newly defined compatible.
>
> This looks good to me, pinging for review from others/queue for merge.
Well, I need an ACK from at least one DT bindings maintainer so that I
can queue it up.
>> Cc: <devicetree@vger.kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
>> .../devicetree/bindings/power/power_domain.txt | 8 +++---
>> drivers/base/power/domain.c | 2 +-
>> 3 files changed, 38 insertions(+), 5 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> new file mode 100644
>> index 0000000..eefc7ed
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> @@ -0,0 +1,33 @@
>> +PM Domain Idle State Node:
>> +
>> +A domain idle state node represents the state parameters that will be used to
>> +select the state when there are no active components in the domain.
>> +
>> +The state node has the following parameters -
>> +
>> +- compatible:
>> + Usage: Required
>> + Value type: <string>
>> + Definition: Must be "domain-idle-state".
>> +
>> +- entry-latency-us
>> + Usage: Required
>> + Value type: <prop-encoded-array>
>> + Definition: u32 value representing worst case latency in
>> + microseconds required to enter the idle state.
>> + The exit-latency-us duration may be guaranteed
>> + only after entry-latency-us has passed.
>> +
>> +- exit-latency-us
>> + Usage: Required
>> + Value type: <prop-encoded-array>
>> + Definition: u32 value representing worst case latency
>> + in microseconds required to exit the idle state.
>> +
>> +- min-residency-us
>> + Usage: Required
>> + Value type: <prop-encoded-array>
>> + Definition: u32 value representing minimum residency duration
>> + in microseconds after which the idle state will yield
>> + power benefits after overcoming the overhead in entering
>> +i the idle state.
>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>> index e165036..723e1ad 100644
>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>> @@ -31,7 +31,7 @@ Optional properties:
>>
>> - 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].
>> + compatible with domain-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
>> @@ -85,7 +85,7 @@ Example 3:
>> };
>>
>> DOMAIN_RET: state@0 {
>> - compatible = "arm,idle-state";
>> + compatible = "domain-idle-state";
>> reg = <0x0>;
>> entry-latency-us = <1000>;
>> exit-latency-us = <2000>;
>> @@ -93,7 +93,7 @@ Example 3:
>> };
>>
>> DOMAIN_PWR_DN: state@1 {
>> - compatible = "arm,idle-state";
>> + compatible = "domain-idle-state";
>> reg = <0x1>;
>> entry-latency-us = <5000>;
>> exit-latency-us = <8000>;
>> @@ -118,4 +118,4 @@ 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
>> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 661737c..f0bc672 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
>> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>
>> static const struct of_device_id idle_state_match[] = {
>> - { .compatible = "arm,idle-state", },
>> + { .compatible = "domain-idle-state", },
>> { }
>> };
> --
Thanks,
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-21 13:11 ` Rafael J. Wysocki
0 siblings, 0 replies; 40+ messages in thread
From: Rafael J. Wysocki @ 2016-11-21 13:11 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Nov 21, 2016 at 1:37 PM, Brendan Jackman
<brendan.jackman@arm.com> wrote:
> Hi,
>
> On Thu, Nov 03 2016 at 21:54, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Re-using idle state definition provided by arm,idle-state for domain
>> idle states creates a lot of confusion and limits further evolution of
>> the domain idle definition. To keep things clear and simple, define a
>> idle states for domain using a new compatible "domain-idle-state".
>>
>> Fix existing PM domains code to look for the newly defined compatible.
>
> This looks good to me, pinging for review from others/queue for merge.
Well, I need an ACK from at least one DT bindings maintainer so that I
can queue it up.
>> Cc: <devicetree@vger.kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
>> .../devicetree/bindings/power/power_domain.txt | 8 +++---
>> drivers/base/power/domain.c | 2 +-
>> 3 files changed, 38 insertions(+), 5 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> new file mode 100644
>> index 0000000..eefc7ed
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> @@ -0,0 +1,33 @@
>> +PM Domain Idle State Node:
>> +
>> +A domain idle state node represents the state parameters that will be used to
>> +select the state when there are no active components in the domain.
>> +
>> +The state node has the following parameters -
>> +
>> +- compatible:
>> + Usage: Required
>> + Value type: <string>
>> + Definition: Must be "domain-idle-state".
>> +
>> +- entry-latency-us
>> + Usage: Required
>> + Value type: <prop-encoded-array>
>> + Definition: u32 value representing worst case latency in
>> + microseconds required to enter the idle state.
>> + The exit-latency-us duration may be guaranteed
>> + only after entry-latency-us has passed.
>> +
>> +- exit-latency-us
>> + Usage: Required
>> + Value type: <prop-encoded-array>
>> + Definition: u32 value representing worst case latency
>> + in microseconds required to exit the idle state.
>> +
>> +- min-residency-us
>> + Usage: Required
>> + Value type: <prop-encoded-array>
>> + Definition: u32 value representing minimum residency duration
>> + in microseconds after which the idle state will yield
>> + power benefits after overcoming the overhead in entering
>> +i the idle state.
>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>> index e165036..723e1ad 100644
>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>> @@ -31,7 +31,7 @@ Optional properties:
>>
>> - 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].
>> + compatible with domain-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
>> @@ -85,7 +85,7 @@ Example 3:
>> };
>>
>> DOMAIN_RET: state at 0 {
>> - compatible = "arm,idle-state";
>> + compatible = "domain-idle-state";
>> reg = <0x0>;
>> entry-latency-us = <1000>;
>> exit-latency-us = <2000>;
>> @@ -93,7 +93,7 @@ Example 3:
>> };
>>
>> DOMAIN_PWR_DN: state at 1 {
>> - compatible = "arm,idle-state";
>> + compatible = "domain-idle-state";
>> reg = <0x1>;
>> entry-latency-us = <5000>;
>> exit-latency-us = <8000>;
>> @@ -118,4 +118,4 @@ 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
>> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 661737c..f0bc672 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
>> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>
>> static const struct of_device_id idle_state_match[] = {
>> - { .compatible = "arm,idle-state", },
>> + { .compatible = "domain-idle-state", },
>> { }
>> };
> --
Thanks,
Rafael
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-03 21:54 ` Lina Iyer
@ 2016-11-21 13:27 ` Ulf Hansson
-1 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2016-11-21 13:27 UTC (permalink / raw)
To: Lina Iyer
Cc: devicetree, Lorenzo Pieralisi, Juri Lelli, linux-pm,
Rafael J. Wysocki, Kevin Hilman, Stephen Boyd, Sudeep Holla,
Brendan Jackman, linux-arm-msm, Andy Gross, linux-arm-kernel
On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> .../devicetree/bindings/power/power_domain.txt | 8 +++---
> drivers/base/power/domain.c | 2 +-
> 3 files changed, 38 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> + Usage: Required
> + Value type: <string>
> + Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency in
> + microseconds required to enter the idle state.
> + The exit-latency-us duration may be guaranteed
> + only after entry-latency-us has passed.
> +
> +- exit-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency
> + in microseconds required to exit the idle state.
> +
> +- min-residency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing minimum residency duration
> + in microseconds after which the idle state will yield
> + power benefits after overcoming the overhead in entering
> +i the idle state.
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
> - 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].
> + compatible with domain-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
> @@ -85,7 +85,7 @@ Example 3:
> };
>
> DOMAIN_RET: state@0 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x0>;
> entry-latency-us = <1000>;
> exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
> };
>
> DOMAIN_PWR_DN: state@1 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x1>;
> entry-latency-us = <5000>;
> exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ 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
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
> - { .compatible = "arm,idle-state", },
> + { .compatible = "domain-idle-state", },
> { }
> };
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-21 13:27 ` Ulf Hansson
0 siblings, 0 replies; 40+ messages in thread
From: Ulf Hansson @ 2016-11-21 13:27 UTC (permalink / raw)
To: linux-arm-kernel
On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> .../devicetree/bindings/power/power_domain.txt | 8 +++---
> drivers/base/power/domain.c | 2 +-
> 3 files changed, 38 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> + Usage: Required
> + Value type: <string>
> + Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency in
> + microseconds required to enter the idle state.
> + The exit-latency-us duration may be guaranteed
> + only after entry-latency-us has passed.
> +
> +- exit-latency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing worst case latency
> + in microseconds required to exit the idle state.
> +
> +- min-residency-us
> + Usage: Required
> + Value type: <prop-encoded-array>
> + Definition: u32 value representing minimum residency duration
> + in microseconds after which the idle state will yield
> + power benefits after overcoming the overhead in entering
> +i the idle state.
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
> - 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].
> + compatible with domain-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
> @@ -85,7 +85,7 @@ Example 3:
> };
>
> DOMAIN_RET: state at 0 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x0>;
> entry-latency-us = <1000>;
> exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
> };
>
> DOMAIN_PWR_DN: state at 1 {
> - compatible = "arm,idle-state";
> + compatible = "domain-idle-state";
> reg = <0x1>;
> entry-latency-us = <5000>;
> exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ 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
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
> - { .compatible = "arm,idle-state", },
> + { .compatible = "domain-idle-state", },
> { }
> };
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
2016-11-03 21:54 ` Lina Iyer
@ 2016-11-21 16:30 ` Sudeep Holla
-1 siblings, 0 replies; 40+ messages in thread
From: Sudeep Holla @ 2016-11-21 16:30 UTC (permalink / raw)
To: Lina Iyer, ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
Cc: devicetree, lorenzo.pieralisi, Juri.Lelli, linux-arm-msm, sboyd,
brendan.jackman, Sudeep Holla, andy.gross
Hi Lina,
On 03/11/16 21:54, Lina Iyer wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
Thanks for doing this(reluctantly? :-)). Looks good to me.
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] PM / Domains: Fix compatible for domain idle state
@ 2016-11-21 16:30 ` Sudeep Holla
0 siblings, 0 replies; 40+ messages in thread
From: Sudeep Holla @ 2016-11-21 16:30 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lina,
On 03/11/16 21:54, Lina Iyer wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
Thanks for doing this(reluctantly? :-)). Looks good to me.
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 40+ messages in thread