Skip to contentSkip to conversation

PM / Domains: DT bindings for domain idle states and doc updates

PATCH New Lina Iyer 5 participants 12 messages Oct 25, 2016 – Nov 1, 2016

Subsystems (7)
lore
Share & tools
lore

Lina Iyer

author
#0 lore raw

cover PM / Domains: DT bindings for domain idle states and doc updates

Hi Rafael,

This patchset adds the following on top of the patches [0-7] of v3[1], that you have already queued up for linux-next.

- New DT compatible for PM Domain idle states.
- Update documentation for PM domain idle state
(Pls. note that this new compatible is yet to be approved by DT reviewers)
- Update device/domain documentation per your suggestion

Thanks,
Lina

[1]. http://www.spinics.net/lists/arm-kernel/msg536181.html

Lina Iyer (4):
PM / doc: Update device documentation for devices in IRQ safe PM
domains
dt-bindings: add domain-idle-state compatible to arm,idle-state
dt-bindings: Update domain-idle-state binding to use correct
compatibles
PM / Domains: Use domain-idle-state compatible

 Documentation/devicetree/bindings/arm/idle-states.txt    |  3 +++
 Documentation/devicetree/bindings/power/power_domain.txt |  9 +++++----
 Documentation/power/devices.txt                          | 14 +++++++++++++-
 drivers/base/power/domain.c                              |  2 +-
 4 files changed, 22 insertions(+), 6 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 1/4 PM / doc: Update device documentation for devices in IRQ safe PM domains

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

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

PATCH 2/4 dt-bindings: add domain-idle-state compatible to arm, idle-state

In reply to Lina Iyer

CPU's idle states are defined by the arm,idle-state compatible flag. PM domains that can contains devices and other domains also has similar definition for its idle state. Reuse the definition of arm,idle-state for PM domains by allowing an addition compatible string ("domain-idle-state") to denote idle states that are specific to PM Domains.

---
 Documentation/devicetree/bindings/arm/idle-states.txt | 3 +++
 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/arm/idle-states.txt +3 -0
index b8e41c1..4b0ff59 100644
@@ -271,6 +271,9 @@ follows:
 		Usage: Required
 		Value type: <stringlist>
 		Definition: Must be "arm,idle-state".
+			    Additionally, nodes that are used to describe a
+			    idle-state of PM domain must also define
+			    "domain-idle-state" as compatible string.
 
 	- local-timer-stop
 		Usage: See definition
Signature · 1 line
2.7.4
author
#4 lore raw

PATCH 3/4 dt-bindings: Update domain-idle-state binding to use correct compatibles

In reply to Lina Iyer

Update domain-idle-state binding to use "domain-idle-state" compatible from Documentation/devicetree/bindings/arm/idle-states.txt.

---
 Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/power/power_domain.txt +5 -4
index e165036..6fb53a3 100644
@@ -30,8 +30,9 @@ Optional properties:
    available in the next section.
 
 - domain-idle-states : A phandle of an idle-state that shall be soaked into a
-                generic domain power state. The idle state definitions are
-                compatible with arm,idle-state specified in [1].
+                generic domain power state. The idle state definitions must be
+                compatible with "domain-idle-state" as well as
+                "arm,idle-state" as defined 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 +86,7 @@ Example 3:
 	};
 
 	DOMAIN_RET: state at 0 {
-		compatible = "arm,idle-state";
+		compatible = "domain-idle-state", "arm,idle-state";
 		reg = <0x0>;
 		entry-latency-us = <1000>;
 		exit-latency-us = <2000>;
@@ -93,7 +94,7 @@ Example 3:
 	};
 
 	DOMAIN_PWR_DN: state at 1 {
-		compatible = "arm,idle-state";
+		compatible = "domain-idle-state", "arm,idle-state";
 		reg = <0x1>;
 		entry-latency-us = <5000>;
 		exit-latency-us = <8000>;
Signature · 1 line
2.7.4
#5 lore raw

PATCH 3/4 dt-bindings: Update domain-idle-state binding to use correct compatibles

Quoted 16 lines · Lina Iyer

Update domain-idle-state binding to use "domain-idle-state" compatible from Documentation/devicetree/bindings/arm/idle-states.txt.

---
 Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/power/power_domain.txt +0 -0
index e165036..6fb53a3 100644

Documentation/devicetree/bindings/power/power_domain.txt Optional properties: · line 34

Quoted 7 lines
Documentation/devicetree/bindings/power/power_domain.txt +2 -2
@@ -30,8 +30,9 @@ Optional properties:
    available in the next section.
 
 - domain-idle-states : A phandle of an idle-state that shall be soaked into a
-                generic domain power state. The idle state definitions are
-                compatible with arm,idle-state specified in [1].
+                generic domain power state. The idle state definitions must be
+                compatible with "domain-idle-state"

I would reword the below a bit different so that it's flexible to be reused without "arm,idle-state".

Quoted 2 lines

as well as
+ "arm,idle-state" as defined in [1].

'Idle states that are "arm,idle-state" compatible are generally "domain-idle-state" compatible as well if it's a PM domain.'

or something like that in line with what's in patch 2/4.

That would give us the scope of reuse of "domain-idle-state" in device for future. Also it aligns with your patch 4/4.

Otherwise, it looks good.

Signature · 2 lines
Regards,
Sudeep
author
#6 lore raw

PATCH 3/4 dt-bindings: Update domain-idle-state binding to use correct compatibles

Quoted 30 lines · Sudeep Holla
Quoted 16 lines · Lina Iyer

Update domain-idle-state binding to use "domain-idle-state" compatible from Documentation/devicetree/bindings/arm/idle-states.txt.

---
Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----

1 file changed, 5 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/power/power_domain.txt +0 -0
index e165036..6fb53a3 100644

Documentation/devicetree/bindings/power/power_domain.txt Optional properties: · line 33

Quoted 7 lines
Documentation/devicetree/bindings/power/power_domain.txt +2 -3
@@ -30,8 +30,9 @@ Optional properties:
   available in the next section.
 
- domain-idle-states : A phandle of an idle-state that shall be soaked into a
-                generic domain power state. The idle state definitions are
-                compatible with arm,idle-state specified in [1].
+                generic domain power state. The idle state definitions must be
+                compatible with "domain-idle-state"

I would reword the below a bit different so that it's flexible to be reused without "arm,idle-state".

Quoted 6 lines
Quoted 2 lines

as well as
+ "arm,idle-state" as defined in [1].

'Idle states that are "arm,idle-state" compatible are generally "domain-idle-state" compatible as well if it's a PM domain.'

I believe we should have both compatible strings. Per [1], any CPU that follows the idle state compatible *must* have "arm,idle-state" as a compatible. Since we are re-using the same compatible, its only correct that we retain what is already spec'd up in [1] and in addition provide this new compatible.

Thanks,
Lina

Quoted 10 lines

or something like that in line with what's in patch 2/4.

That would give us the scope of reuse of "domain-idle-state" in device for future. Also it aligns with your patch 4/4.

Otherwise, it looks good.

Signature · 2 lines
Regards,
Sudeep
#7 lore raw

PATCH 3/4 dt-bindings: Update domain-idle-state binding to use correct compatibles

Quoted 38 lines · Lina Iyer
Quoted 37 lines · Sudeep Holla
Quoted 25 lines · Lina Iyer

Update domain-idle-state binding to use "domain-idle-state" compatible from Documentation/devicetree/bindings/arm/idle-states.txt.

---
Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----

1 file changed, 5 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/power/power_domain.txt +0 -1
index e165036..6fb53a3 100644
@@ -30,8 +30,9 @@ Optional properties:
   available in the next section.
 
- domain-idle-states : A phandle of an idle-state that shall be

soaked into a
- generic domain power state. The idle state
definitions are

Documentation/devicetree/bindings/power/power_domain.txt +1 -1
-                compatible with arm,idle-state specified in [1].
+                generic domain power state. The idle state
Quoted 2 lines

definitions must be
+ compatible with "domain-idle-state"

I would reword the below a bit different so that it's flexible to be reused without "arm,idle-state".

Quoted 2 lines

as well as
+ "arm,idle-state" as defined in [1].

Quoted 6 lines
Quoted 3 lines

'Idle states that are "arm,idle-state" compatible are generally "domain-idle-state" compatible as well if it's a PM domain.'

I believe we should have both compatible strings. Per [1], any CPU that follows the idle state compatible *must* have "arm,idle-state" as a compatible.

Yes that's implicit for a CPU device. But generic power domain bindings should not have that explicitly as it *can be* used for non CPU device.

Quoted 4 lines

Since we are re-using the same compatible, its only correct that we retain what is already spec'd up in [1] and in addition provide this new compatible.

Yes [1] applies for *CPUs only* while this applies for *any device* and *any power domain*, so I would drop *must have* "arm,idle-state" here to keep this generic based on my understanding on how compatibles work.

Signature · 2 lines
Regards,
Sudeep
#8 lore raw

PATCH 3/4 dt-bindings: Update domain-idle-state binding to use correct compatibles

In reply to Lina Iyer
Quoted 10 lines · Lina Iyer

Update domain-idle-state binding to use "domain-idle-state" compatible from Documentation/devicetree/bindings/arm/idle-states.txt.

---
 Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

With no current users for this, I don't see the point of adding a compatible now.

IMO, this should wait and be added with the identified user we can discuss it then.

Kevin

#9 lore raw

PATCH 3/4 dt-bindings: Update domain-idle-state binding to use correct compatibles

Quoted 13 lines · Kevin Hilman
Quoted 10 lines · Lina Iyer

Update domain-idle-state binding to use "domain-idle-state" compatible from Documentation/devicetree/bindings/arm/idle-states.txt.

---
 Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Quoted 6 lines

With no current users for this, I don't see the point of adding a compatible now.

IMO, this should wait and be added with the identified user we can discuss it then.

No, IMO it needs to be used for the proposed SoC idle/genpd solution.

I understand the nodes that are "arm,idle-state" compatible can be used for this new SoC hierarchical idle management, but it was never defined for that use originally. So this new feature must be advertised by the firmware with the presence of "domain-idle-state".

Yes we might have other ways to detect that but I have already seen that broken on the reference platform, so we need alternate/DT way to specify that.

Not all existing "arm,idle-state" compatible nodes will be capable of supporting this new SoC idle feature. It's just better and safer for a new feature getting added that relies on DT to have a new compatible.

Signature · 2 lines
Regards,
Sudeep
maintainer
#10 lore raw

PATCH 3/4 dt-bindings: Update domain-idle-state binding to use correct compatibles

Quoted 30 lines · Sudeep Holla
Quoted 13 lines · Kevin Hilman
Quoted 10 lines · Lina Iyer

Update domain-idle-state binding to use "domain-idle-state" compatible from Documentation/devicetree/bindings/arm/idle-states.txt.

---
 Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Quoted 6 lines

With no current users for this, I don't see the point of adding a compatible now.

IMO, this should wait and be added with the identified user we can discuss it then.

No, IMO it needs to be used for the proposed SoC idle/genpd solution.

I understand the nodes that are "arm,idle-state" compatible can be used for this new SoC hierarchical idle management, but it was never defined for that use originally. So this new feature must be advertised by the firmware with the presence of "domain-idle-state".

Quoted 7 lines

Yes we might have other ways to detect that but I have already seen that broken on the reference platform, so we need alternate/DT way to specify that.

Not all existing "arm,idle-state" compatible nodes will be capable of supporting this new SoC idle feature. It's just better and safer for a new feature getting added that relies on DT to have a new compatible.

Or perhaps you should describe something new rather than trying to graft in what's there. This combination of compatible strings looks a bit odd to me. Though, I've not really spent much time thinking about this.

Rob

author
#11 lore raw

PATCH 4/4 PM / Domains: Use domain-idle-state compatible

In reply to Lina Iyer

Use the more specific "domain-idle-states" compatible property to read idle state properties from DT.

---
 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/base/power/domain.c +1 -1
index aac656a..07ed835 100644
@@ -2048,7 +2048,7 @@ out:
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
 static const struct of_device_id idle_state_match[] = {
-	{ .compatible = "arm,idle-state", },
+	{ .compatible = "domain-idle-state", },
 	{ }
 };
 
Signature · 1 line
2.7.4