ARM generic idle states

ARM based platforms implement a variety of power management schemes that allow processors to enter idle states at run-time.
The parameters defining these idle states vary on a per-platform basis forcing the OS to hardcode the state parameters in platform specific static tables whose size grows as the number of platforms supported in the kernel increases and hampers device drivers standardization.

Therefore, this patch aims at standardizing idle state device tree bindings for ARM platforms. Bindings define idle state parameters inclusive of entry methods and state latencies, to allow operating systems to retrieve the configuration entries from the device tree and initialize the related power management drivers, paving the way for common code in the kernel to deal with idle states and removing the need for static data in current and previous kernel versions.

Reviewed-by Sebastian Capella <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 Documentation/devicetree/bindings/arm/cpus.txt     |   8 +
 .../devicetree/bindings/arm/idle-states.txt        | 733 +++++++++++++++++++++
 2 files changed, 741 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/idle-states.txt

+===========================================
+3 - idle-states node
+===========================================

Documentation/devicetree/bindings/arm/idle-states.txt new file +71 -0

+
+ARM processor idle states are defined within the idle-states node, which is
+a direct child of the cpus node [1] and provides a container where the
+processor idle states, defined as device tree nodes, are listed.
+
+- idle-states node
+
+       Usage: Optional - On ARM systems, it is a container of processor idle
+                         states nodes. If the system does not provide CPU
+                         power management capabilities or the processor just
+                         supports idle_standby an idle-states node is not
+                         required.
+
+       Description: idle-states node is a container node, where its
+                    subnodes describe the CPU idle states.
+
+       Node name must be "idle-states".
+
+       The idle-states node's parent node must be the cpus node.
+
+       The idle-states node's child nodes can be:
+
+       - one or more state nodes
+
+       Any other configuration is considered invalid.
+
+       An idle-states node defines the following properties:
+
+       - entry-method
+               Usage: Required
+               Value type: <stringlist>
+               Definition: Describes the method by which a CPU enters the
+                           idle states. This property is required and must be
+                           one of:
+
+                           - "arm,psci"
+                             ARM PSCI firmware interface [2].
+
+                           - "[vendor],[method]"
+                             An implementation dependent string with
+                             format "vendor,method", where vendor is a string
+                             denoting the name of the manufacturer and
+                             method is a string specifying the mechanism
+                             used to enter the idle state.
+
+The nodes describing the idle states (state) can only be defined within the
+idle-states node, any other configuration is considered invalid and therefore
+must be ignored.
+
+===========================================
+4 - state node
+===========================================
+
+A state node represents an idle state description and must be defined as
+follows:
+
+- state node
+
+       Description: must be child of the idle-states node
+
+       The state node name shall follow standard device tree naming
+       rules ([5], 2.2.1 "Node names"), in particular state nodes which
+       are siblings within a single common parent must be given a unique name.
+
+       The idle state entered by executing the wfi instruction (idle_standby
+       SBSA,[3][4]) is considered standard on all ARM platforms and therefore
+       must not be listed.
+
+       With the definitions provided above, the following list represents
+       the valid properties for a state node:
+

Hi Lorenzo,

Quoted 22 lines · Lorenzo Pieralisi

ARM based platforms implement a variety of power management schemes that allow processors to enter idle states at run-time.
The parameters defining these idle states vary on a per-platform basis forcing the OS to hardcode the state parameters in platform specific static tables whose size grows as the number of platforms supported in the kernel increases and hampers device drivers standardization.

Therefore, this patch aims at standardizing idle state device tree bindings for ARM platforms. Bindings define idle state parameters inclusive of entry methods and state latencies, to allow operating systems to retrieve the configuration entries from the device tree and initialize the related power management drivers, paving the way for common code in the kernel to deal with idle states and removing the need for static data in current and previous kernel versions.

Reviewed-by Sebastian Capella <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 Documentation/devicetree/bindings/arm/cpus.txt     |   8 +
 .../devicetree/bindings/arm/idle-states.txt        | 733 +++++++++++++++++++++
 2 files changed, 741 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/idle-states.txt

[...]

Quoted 74 lines

+===========================================
+3 - idle-states node
+===========================================

Documentation/devicetree/bindings/arm/idle-states.txt new file +71 -0

+
+ARM processor idle states are defined within the idle-states node, which is
+a direct child of the cpus node [1] and provides a container where the
+processor idle states, defined as device tree nodes, are listed.
+
+- idle-states node
+
+       Usage: Optional - On ARM systems, it is a container of processor idle
+                         states nodes. If the system does not provide CPU
+                         power management capabilities or the processor just
+                         supports idle_standby an idle-states node is not
+                         required.
+
+       Description: idle-states node is a container node, where its
+                    subnodes describe the CPU idle states.
+
+       Node name must be "idle-states".
+
+       The idle-states node's parent node must be the cpus node.
+
+       The idle-states node's child nodes can be:
+
+       - one or more state nodes
+
+       Any other configuration is considered invalid.
+
+       An idle-states node defines the following properties:
+
+       - entry-method
+               Usage: Required
+               Value type: <stringlist>
+               Definition: Describes the method by which a CPU enters the
+                           idle states. This property is required and must be
+                           one of:
+
+                           - "arm,psci"
+                             ARM PSCI firmware interface [2].
+
+                           - "[vendor],[method]"
+                             An implementation dependent string with
+                             format "vendor,method", where vendor is a string
+                             denoting the name of the manufacturer and
+                             method is a string specifying the mechanism
+                             used to enter the idle state.
+
+The nodes describing the idle states (state) can only be defined within the
+idle-states node, any other configuration is considered invalid and therefore
+must be ignored.
+
+===========================================
+4 - state node
+===========================================
+
+A state node represents an idle state description and must be defined as
+follows:
+
+- state node
+
+       Description: must be child of the idle-states node
+
+       The state node name shall follow standard device tree naming
+       rules ([5], 2.2.1 "Node names"), in particular state nodes which
+       are siblings within a single common parent must be given a unique name.
+
+       The idle state entered by executing the wfi instruction (idle_standby
+       SBSA,[3][4]) is considered standard on all ARM platforms and therefore
+       must not be listed.
+
+       With the definitions provided above, the following list represents
+       the valid properties for a state node:
+

Documentation/devicetree/bindings/arm/idle-states.txt line 288

Quoted 10 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +10 -0

+       - compatible
+               Usage: Required
+               Value type: <stringlist>
+               Definition: Must be "arm,idle-state".
+
+       - logic-state-retained
+               Usage: See definition
+               Value type: <none>
+               Definition: if present logic is retained on state entry,
+                           otherwise it is lost.

What logic state is retained? All system registers?

Documentation/devicetree/bindings/arm/idle-states.txt line 294

Quoted 5 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +5 -0

+       - cache-state-retained
+               Usage: See definition
+               Value type: <none>
+               Definition: if present cache memory is retained on state entry,
+                           otherwise it is lost.

Likewise, how much of the cache hierarchy is affected? Any of it? All of it?

Documentation/devicetree/bindings/arm/idle-states.txt line 300

Quoted 5 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +5 -0

+       - timer-state-retained
+               Usage: See definition
+               Value type: <none>
+               Definition: if present the timer control logic is retained on
+                            state entry, otherwise it is lost.

The architected generic timers? Any CPU-local timers? Or any timers whatsoever?

Documentation/devicetree/bindings/arm/idle-states.txt line 311

Quoted 10 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +10 -0

+       - power-rank
+               Usage: Required
+               Value type: <u32>
+               Definition: It represents the idle state power-rank.
+                           An increasing value implies less power
+                           consumption. It must be given a sequential
+                           value = {0, 1, ....}, starting from 0.
+                           Phandles in the cpu nodes [1] cpu-idle-states
+                           array property are not allowed to point at idle
+                           state nodes having the same power-rank value.

Why can't this be implicit in the order of the cpu-idle-states list? That way it's impossible to violate the ordering requirement.

Documentation/devicetree/bindings/arm/idle-states.txt line 318

Quoted 7 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +6 -0

+       - entry-method-param
+               Usage: See definition.
+               Value type: <u32>
+               Definition: Depends on the idle-states node entry-method
+                           property value. Refer to the entry-method bindings
+                           for this property value definition.

Should this not be left up to the particular mechanism to describe? e.g. for PSCI we could have a arm,psci-suspend-param property.

Are we sure a single u32 value is going to be sufficient?

Quoted 40 lines · Mark Rutland

Hi Lorenzo,

Quoted 22 lines · Lorenzo Pieralisi

ARM based platforms implement a variety of power management schemes that allow processors to enter idle states at run-time.
The parameters defining these idle states vary on a per-platform basis forcing the OS to hardcode the state parameters in platform specific static tables whose size grows as the number of platforms supported in the kernel increases and hampers device drivers standardization.

Therefore, this patch aims at standardizing idle state device tree bindings for ARM platforms. Bindings define idle state parameters inclusive of entry methods and state latencies, to allow operating systems to retrieve the configuration entries from the device tree and initialize the related power management drivers, paving the way for common code in the kernel to deal with idle states and removing the need for static data in current and previous kernel versions.

Reviewed-by Sebastian Capella <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 Documentation/devicetree/bindings/arm/cpus.txt     |   8 +
 .../devicetree/bindings/arm/idle-states.txt        | 733 +++++++++++++++++++++
 2 files changed, 741 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/idle-states.txt

[...]

Documentation/devicetree/bindings/arm/idle-states.txt line 311

Quoted 10 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +10 -0

+       - power-rank
+               Usage: Required
+               Value type: <u32>
+               Definition: It represents the idle state power-rank.
+                           An increasing value implies less power
+                           consumption. It must be given a sequential
+                           value = {0, 1, ....}, starting from 0.
+                           Phandles in the cpu nodes [1] cpu-idle-states
+                           array property are not allowed to point at idle
+                           state nodes having the same power-rank value.

Quoted 2 lines

Why can't this be implicit in the order of the cpu-idle-states list? That way it's impossible to violate the ordering requirement.

You mean the phandles list in the cpu nodes ? Maybe, but this would require the list to be the same order for all cpu nodes on which the idle states are valid, or just take one and use that.

It can be viable, as long as everyone agrees, every time I post this code someone comes up with a new idea on how to sort the states and honestly I would like to be done with that.

Quoted 7 lines

Quoted 6 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +6 -0

+       - entry-method-param
+               Usage: See definition.
+               Value type: <u32>
+               Definition: Depends on the idle-states node entry-method
+                           property value. Refer to the entry-method bindings
+                           for this property value definition.

Quoted 2 lines

Should this not be left up to the particular mechanism to describe? e.g. for PSCI we could have a arm,psci-suspend-param property.

It was like that in early postings, and probably was better than the current definition. I need to think about that but I am almost convinced you are right.

Quoted 22 lines

+===========================================
+4 - state node
+===========================================

Documentation/devicetree/bindings/arm/idle-states.txt new file +19 -0

+
+A state node represents an idle state description and must be defined as
+follows:
+
+- state node
+
+       Description: must be child of the idle-states node
+
+       The state node name shall follow standard device tree naming
+       rules ([5], 2.2.1 "Node names"), in particular state nodes which
+       are siblings within a single common parent must be given a unique name.
+
+       The idle state entered by executing the wfi instruction (idle_standby
+       SBSA,[3][4]) is considered standard on all ARM platforms and therefore
+       must not be listed.
+
+       With the definitions provided above, the following list represents
+       the valid properties for a state node:
+

Documentation/devicetree/bindings/arm/idle-states.txt line 288

Quoted 10 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +10 -0

+       - compatible
+               Usage: Required
+               Value type: <stringlist>
+               Definition: Must be "arm,idle-state".
+
+       - logic-state-retained
+               Usage: See definition
+               Value type: <none>
+               Definition: if present logic is retained on state entry,
+                           otherwise it is lost.

What logic state is retained? All system registers?

Documentation/devicetree/bindings/arm/idle-states.txt line 294

Quoted 5 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +5 -0

+       - cache-state-retained
+               Usage: See definition
+               Value type: <none>
+               Definition: if present cache memory is retained on state entry,
+                           otherwise it is lost.

Likewise, how much of the cache hierarchy is affected? Any of it? All of it?

Documentation/devicetree/bindings/arm/idle-states.txt line 300

Quoted 5 lines

Documentation/devicetree/bindings/arm/idle-states.txt new file +5 -0

+       - timer-state-retained
+               Usage: See definition
+               Value type: <none>
+               Definition: if present the timer control logic is retained on
+                            state entry, otherwise it is lost.

The architected generic timers? Any CPU-local timers? Or any timers whatsoever?

ARM based platforms implement a variety of power management schemes that allow processors to enter idle states at run-time.
The parameters defining these idle states vary on a per-platform basis forcing the OS to hardcode the state parameters in platform specific static tables whose size grows as the number of platforms supported in the kernel increases and hampers device drivers standardization.

Therefore, this patch aims at standardizing idle state device tree bindings for ARM platforms. Bindings define idle state parameters inclusive of entry methods and state latencies, to allow operating systems to retrieve the configuration entries from the device tree and initialize the related power management drivers, paving the way for common code in the kernel to deal with idle states and removing the need for static data in current and previous kernel versions.

Reviewed-by Sebastian Capella <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 Documentation/devicetree/bindings/arm/cpus.txt     |   8 +
 .../devicetree/bindings/arm/idle-states.txt        | 733 +++++++++++++++++++++
 2 files changed, 741 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/idle-states.txt

Documentation/devicetree/bindings/arm/cpus.txt +8 -0

index 1fe72a0..a44d4fd 100644
@@ -215,6 +215,12 @@ nodes to be present and contain the properties described below.
 		Value type: <phandle>
 		Definition: Specifies the ACC[2] node associated with this CPU.
 
+	- cpu-idle-states
+		Usage: Optional
+		Value type: <prop-encoded-array>
+		Definition:
+			# List of phandles to idle state nodes supported
+			  by this cpu [3].
 
 Example 1 (dual-cluster big.LITTLE system 32-bit):
 

@@ -411,3 +417,5 @@ cpus {
 --
 [1] arm/msm/qcom,saw2.txt
 [2] arm/msm/qcom,kpss-acc.txt
+[3] ARM Linux kernel documentation - idle states bindings
+    Documentation/devicetree/bindings/arm/idle-states.txt

Documentation/devicetree/bindings/arm/idle-states.txt new file +733 -0

new file mode 100644
index 0000000..5efd198
@@ -0,0 +1,733 @@
+==========================================
+ARM idle states binding description
+==========================================
+
+==========================================
+1 - Introduction
+==========================================
+
+ARM systems contain HW capable of managing power consumption dynamically,
+where cores can be put in different low-power states (ranging from simple
+wfi to power gating) according to OS PM policies. The CPU states representing
+the range of dynamic idle states that a processor can enter at run-time, can be
+specified through device tree bindings representing the parameters required
+to enter/exit specific idle states on a given processor.
+
+According to the Server Base System Architecture document (SBSA, [3]), the
+power states an ARM CPU can be put into are identified by the following list:
+
+- Running
+- Idle_standby
+- Idle_retention
+- Sleep
+- Off
+
+The power states described in the SBSA document define the basic CPU states on
+top of which ARM platforms implement power management schemes that allow an OS
+PM implementation to put the processor in different idle states (which include
+states listed above; "off" state is not an idle state since it does not have
+wake-up capabilities, hence it is not considered in this document).
+
+Idle state parameters (eg entry latency) are platform specific and need to be
+characterized with bindings that provide the required information to OS PM
+code so that it can build the required tables and use them at runtime.
+
+The device tree binding definition for ARM idle states is the subject of this
+document.
+
+===========================================
+2 - idle-states definitions
+===========================================
+
+Idle states are characterized for a specific system through a set of
+timing and energy related properties, that underline the HW behaviour
+triggered upon idle states entry and exit.
+
+The following diagram depicts the CPU execution phases and related timing
+properties required to enter and exit an idle state:
+
+..__[EXEC]__|__[PREP]__|__[ENTRY]__|__[IDLE]__|__[EXIT]__|__[EXEC]__..
+	    |          |           |          |          |
+
+	    |<------ entry ------->|
+	    |       latency        |
+					      |<- exit ->|
+					      |  latency |
+	    |<-------- min-residency -------->|
+		       |<-------  wakeup-latency ------->|
+
+		Diagram 1: CPU idle state execution phases
+
+EXEC:	Normal CPU execution.
+
+PREP:	Preparation phase before committing the hardware to idle mode
+	like cache flushing. This is abortable on pending wake-up
+	event conditions. The abort latency is assumed to be negligible
+	(i.e. less than the ENTRY + EXIT duration). If aborted, CPU
+	goes back to EXEC. This phase is optional. If not abortable,
+	this should be included in the ENTRY phase instead.
+
+ENTRY:	The hardware is committed to idle mode. This period must run
+	to completion up to IDLE before anything else can happen.
+
+IDLE:	This is the actual energy-saving idle period. This may last
+	between 0 and infinite time, until a wake-up event occurs.
+
+EXIT:	Period during which the CPU is brought back to operational
+	mode (EXEC).
+
+entry-latency: Worst case latency required to enter the idle state. The
+exit-latency may be guaranteed only after entry-latency has passed.
+
+min-residency: Minimum period, including preparation and entry, for a given
+idle state to be worthwhile energywise.
+
+wakeup-latency: Maximum delay between the signaling of a wake-up event and the
+CPU being able to execute normal code again. If not specified, this is assumed
+to be entry-latency + exit-latency.
+
+These timing parameters can be used by an OS in different circumstances.
+
+An idle CPU requires the expected min-residency time to select the most
+appropriate idle state based on the expected expiry time of the next IRQ
+(ie wake-up) that causes the CPU to return to the EXEC phase.
+
+An operating system scheduler may need to compute the shortest wake-up delay
+for CPUs in the system by detecting how long will it take to get a CPU out
+of an idle state, eg:
+
+wakeup-delay = exit-latency + max(entry-latency - (now - entry-timestamp), 0)
+
+In other words, the scheduler can make its scheduling decision by selecting
+(eg waking-up) the CPU with the shortest wake-up latency.
+The wake-up latency must take into account the entry latency if that period
+has not expired. The abortable nature of the PREP period can be ignored
+if it cannot be relied upon (e.g. the PREP deadline may occur much sooner than
+the worst case since it depends on the CPU operating conditions, ie caches
+state).
+
+An OS has to reliably probe the wakeup-latency since some devices can enforce
+latency constraints guarantees to work properly, so the OS has to detect the
+worst case wake-up latency it can incur if a CPU is allowed to enter an
+idle state, and possibly to prevent that to guarantee reliable device
+functioning.
+
+The min-residency time parameter deserves further explanation since it is
+expressed in time units but must factor in energy consumption coefficients.
+
+The energy consumption of a cpu when it enters a power state can be roughly
+characterised by the following graph:
+
+               |
+               |
+               |
+           e   |
+           n   |                                      /---
+           e   |                               /------
+           r   |                        /------
+           g   |                  /-----
+           y   |           /------
+               |       ----
+               |      /|
+               |     / |
+               |    /  |
+               |   /   |
+               |  /    |
+               | /     |
+               |/      |
+          -----|-------+----------------------------------
+              0|       1                              time(ms)
+
+		Graph 1: Energy vs time example
+
+The graph is split in two parts delimited by time 1ms on the X-axis.
+The graph curve with X-axis values = { x | 0 < x < 1ms } has a steep slope
+and denotes the energy costs incurred whilst entering and leaving the idle
+state.
+The graph curve in the area delimited by X-axis values = {x | x > 1ms } has
+shallower slope and essentially represents the energy consumption of the idle
+state.
+
+min-residency is defined for a given idle state as the minimum expected
+residency time for a state (inclusive of preparation and entry) after
+which choosing that state become the most energy efficient option. A good
+way to visualise this, is by taking the same graph above and comparing some
+states energy consumptions plots.
+
+For sake of simplicity, let's consider a system with two idle states IDLE1,
+and IDLE2:
+
+          |
+          |
+          |
+          |                                                  /-- IDLE1
+       e  |                                              /---
+       n  |                                         /----
+       e  |                                     /---
+       r  |                                /-----/--------- IDLE2
+       g  |                    /-------/---------
+       y  |        ------------    /---|
+          |       /           /----    |
+          |      /        /---         |
+          |     /    /----             |
+          |    / /---                  |
+          |   ---                      |
+          |  /                         |
+          | /                          |
+          |/                           |                  time
+       ---/----------------------------+------------------------
+          |IDLE1-energy < IDLE2-energy | IDLE2-energy < IDLE1-energy
+                                       |
+                                IDLE2-min-residency
+
+		Graph 2: idle states min-residency example
+
+In graph 2 above, that takes into account idle states entry/exit energy
+costs, it is clear that if the idle state residency time (ie time till next
+wake-up IRQ) is less than IDLE2-min-residency, IDLE1 is the better idle state
+choice energywise.
+
+This is mainly down to the fact that IDLE1 entry/exit energy costs are lower
+than IDLE2.
+
+However, the lower power consumption (ie shallower energy curve slope) of idle
+state IDLE2 implies that after a suitable time, IDLE2 becomes more energy
+efficient.
+
+The time at which IDLE2 becomes more energy efficient than IDLE1 (and other
+shallower states in a system with multiple idle states) is defined
+IDLE2-min-residency and corresponds to the time when energy consumption of
+IDLE1 and IDLE2 states breaks even.
+
+The definitions provided in this section underpin the idle states
+properties specification that is the subject of the following sections.
+
+===========================================
+3 - idle-states node
+===========================================
+
+ARM processor idle states are defined within the idle-states node, which is
+a direct child of the cpus node [1] and provides a container where the
+processor idle states, defined as device tree nodes, are listed.
+
+- idle-states node
+
+	Usage: Optional - On ARM systems, it is a container of processor idle
+			  states nodes. If the system does not provide CPU
+			  power management capabilities or the processor just
+			  supports idle_standby an idle-states node is not
+			  required.
+
+	Description: idle-states node is a container node, where its
+		     subnodes describe the CPU idle states.
+
+	Node name must be "idle-states".
+
+	The idle-states node's parent node must be the cpus node.
+
+	The idle-states node's child nodes can be:
+
+	- one or more state nodes
+
+	Any other configuration is considered invalid.
+
+	An idle-states node defines the following properties:
+
+	- entry-method
+		Usage: Required
+		Value type: <stringlist>
+		Definition: Describes the method by which a CPU enters the
+			    idle states. This property is required and must be
+			    one of:
+
+			    - "arm,psci"
+			      ARM PSCI firmware interface [2].
+
+			    - "[vendor],[method]"
+			      An implementation dependent string with
+			      format "vendor,method", where vendor is a string
+			      denoting the name of the manufacturer and
+			      method is a string specifying the mechanism
+			      used to enter the idle state.
+
+The nodes describing the idle states (state) can only be defined within the
+idle-states node, any other configuration is considered invalid and therefore
+must be ignored.
+
+===========================================
+4 - state node
+===========================================
+
+A state node represents an idle state description and must be defined as
+follows:
+
+- state node
+
+	Description: must be child of the idle-states node
+
+	The state node name shall follow standard device tree naming
+	rules ([5], 2.2.1 "Node names"), in particular state nodes which
+	are siblings within a single common parent must be given a unique name.
+
+	The idle state entered by executing the wfi instruction (idle_standby
+	SBSA,[3][4]) is considered standard on all ARM platforms and therefore
+	must not be listed.
+
+	With the definitions provided above, the following list represents
+	the valid properties for a state node:
+
+	- compatible
+		Usage: Required
+		Value type: <stringlist>
+		Definition: Must be "arm,idle-state".
+
+	- logic-state-retained
+		Usage: See definition
+		Value type: <none>
+		Definition: if present logic is retained on state entry,
+			    otherwise it is lost.
+
+	- cache-state-retained
+		Usage: See definition
+		Value type: <none>
+		Definition: if present cache memory is retained on state entry,
+			    otherwise it is lost.
+
+	- timer-state-retained
+		Usage: See definition
+		Value type: <none>
+		Definition: if present the timer control logic is retained on
+                            state entry, otherwise it is lost.
+
+	- power-rank
+		Usage: Required
+		Value type: <u32>
+		Definition: It represents the idle state power-rank.
+			    An increasing value implies less power
+			    consumption. It must be given a sequential
+			    value = {0, 1, ....}, starting from 0.
+			    Phandles in the cpu nodes [1] cpu-idle-states
+			    array property are not allowed to point at idle
+			    state nodes having the same power-rank value.
+
+	- entry-method-param
+		Usage: See definition.
+		Value type: <u32>
+		Definition: Depends on the idle-states node entry-method
+			    property value. Refer to the entry-method bindings
+			    for this property value definition.
+
+	- 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, inclusive of preparation and
+			    entry, for this idle state to be considered
+			    worthwhile energy wise (refer to section 2 of
+			    this document for a complete description).
+
+	- wakeup-latency-us:
+		Usage: Optional
+		Value type: <prop-encoded-array>
+		Definition: u32 value representing maximum delay between the
+			    signaling of a wake-up event and the CPU being
+			    able to execute normal code again. If omitted,
+			    this is assumed to be equal to:
+
+				entry-latency-us + exit-latency-us
+
+			    It is important to supply this value on systems
+			    where the duration of PREP phase (see diagram 1,
+			    section 2) is non-neglibigle.
+			    In such systems entry-latency-us + exit-latency-us
+			    will exceed wakeup-latency-us by this duration.
+
+===========================================
+4 - Examples
+===========================================
+
+Example 1 (ARM 64-bit, 16-cpu system):
+
+cpus {
+	#size-cells = <0>;
+	#address-cells = <2>;
+
+	idle-states {
+		entry-method = "arm,psci";
+
+		CPU_RETENTION_0_0: cpu-retention-0-0 {
+			compatible = "arm,idle-state";
+			power-rank = <0>;
+			logic-state-retained;
+			cache-state-retained;
+			entry-method-param = <0x0010000>;
+			entry-latency-us = <20>;
+			exit-latency-us = <40>;
+			min-residency-us = <80>;
+		};
+
+		CLUSTER_RETENTION_0: cluster-retention-0 {
+			compatible = "arm,idle-state";
+			power-rank = <2>;
+			cache-state-retained;
+			entry-method-param = <0x1010000>;
+			entry-latency-us = <50>;
+			exit-latency-us = <100>;
+			min-residency-us = <250>;
+			wakeup-latency-us = <130>;
+		};
+
+		CPU_SLEEP_0_0: cpu-sleep-0-0 {
+			compatible = "arm,idle-state";
+			power-rank = <1>;
+			entry-method-param = <0x0010000>;
+			entry-latency-us = <250>;
+			exit-latency-us = <500>;
+			min-residency-us = <950>;
+		};
+
+		CLUSTER_SLEEP_0: cluster-sleep-0 {
+			compatible = "arm,idle-state";
+			power-rank = <3>;
+			entry-method-param = <0x1010000>;
+			entry-latency-us = <600>;
+			exit-latency-us = <1100>;
+			min-residency-us = <2700>;
+			wakeup-latency-us = <1500>;
+		};
+
+		CPU_RETENTION_1_0: cpu-retention-1-0 {
+			compatible = "arm,idle-state";
+			power-rank = <0>;
+			logic-state-retained;
+			cache-state-retained;
+			entry-method-param = <0x0010000>;
+			entry-latency-us = <20>;
+			exit-latency-us = <40>;
+			min-residency-us = <90>;
+		};
+
+		CLUSTER_RETENTION_1: cluster-retention-1 {
+			compatible = "arm,idle-state";
+			power-rank = <2>;
+			cache-state-retained;
+			entry-method-param = <0x1010000>;
+			entry-latency-us = <50>;
+			exit-latency-us = <100>;
+			min-residency-us = <270>;
+			wakeup-latency-us = <100>;
+		};
+
+		CPU_SLEEP_1_0: cpu-sleep-1-0 {
+			compatible = "arm,idle-state";
+			power-rank = <1>;
+			entry-method-param = <0x0010000>;
+			entry-latency-us = <70>;
+			exit-latency-us = <100>;
+			min-residency-us = <300>;
+			wakeup-latency-us = <150>;
+		};
+
+		CLUSTER_SLEEP_1: cluster-sleep-1 {
+			compatible = "arm,idle-state";
+			power-rank = <3>;
+			entry-method-param = <0x1010000>;
+			entry-latency-us = <500>;
+			exit-latency-us = <1200>;
+			min-residency-us = <3500>;
+			wakeup-latency-us = <1300>;
+		};
+	};
+
+	CPU0: cpu at 0 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x0>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_0_0 &CPU_SLEEP_0_0
+				   &CLUSTER_RETENTION_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU1: cpu at 1 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x1>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_0_0 &CPU_SLEEP_0_0
+				   &CLUSTER_RETENTION_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU2: cpu at 100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x100>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_0_0 &CPU_SLEEP_0_0
+				   &CLUSTER_RETENTION_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU3: cpu at 101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x101>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_0_0 &CPU_SLEEP_0_0
+				   &CLUSTER_RETENTION_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU4: cpu at 10000 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x10000>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_0_0 &CPU_SLEEP_0_0
+				   &CLUSTER_RETENTION_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU5: cpu at 10001 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x10001>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_0_0 &CPU_SLEEP_0_0
+				   &CLUSTER_RETENTION_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU6: cpu at 10100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x10100>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_0_0 &CPU_SLEEP_0_0
+				   &CLUSTER_RETENTION_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU7: cpu at 10101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x10101>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_0_0 &CPU_SLEEP_0_0
+				   &CLUSTER_RETENTION_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU8: cpu at 100000000 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a53";
+		reg = <0x1 0x0>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_1_0 &CPU_SLEEP_1_0
+				   &CLUSTER_RETENTION_1 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU9: cpu at 100000001 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a53";
+		reg = <0x1 0x1>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_1_0 &CPU_SLEEP_1_0
+				   &CLUSTER_RETENTION_1 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU10: cpu at 100000100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a53";
+		reg = <0x1 0x100>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_1_0 &CPU_SLEEP_1_0
+				   &CLUSTER_RETENTION_1 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU11: cpu at 100000101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a53";
+		reg = <0x1 0x101>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_1_0 &CPU_SLEEP_1_0
+				   &CLUSTER_RETENTION_1 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU12: cpu at 100010000 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a53";
+		reg = <0x1 0x10000>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_1_0 &CPU_SLEEP_1_0
+				   &CLUSTER_RETENTION_1 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU13: cpu at 100010001 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a53";
+		reg = <0x1 0x10001>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_1_0 &CPU_SLEEP_1_0
+				   &CLUSTER_RETENTION_1 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU14: cpu at 100010100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a53";
+		reg = <0x1 0x10100>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_1_0 &CPU_SLEEP_1_0
+				   &CLUSTER_RETENTION_1 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU15: cpu at 100010101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a53";
+		reg = <0x1 0x10101>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_RETENTION_1_0 &CPU_SLEEP_1_0
+				   &CLUSTER_RETENTION_1 &CLUSTER_SLEEP_1>;
+	};
+};
+
+Example 2 (ARM 32-bit, 8-cpu system, two clusters):
+
+cpus {
+	#size-cells = <0>;
+	#address-cells = <1>;
+
+	idle-states {
+		entry-method = "arm,psci";
+
+		CPU_SLEEP_0_0: cpu-sleep-0-0 {
+			compatible = "arm,idle-state";
+			power-rank = <0>;
+			entry-method-param = <0x0010000>;
+			entry-latency-us = <200>;
+			exit-latency-us = <100>;
+			min-residency-us = <400>;
+			wakeup-latency-us = <250>;
+		};
+
+		CLUSTER_SLEEP_0: cluster-sleep-0 {
+			compatible = "arm,idle-state";
+			power-rank = <2>;
+			entry-method-param = <0x1010000>;
+			entry-latency-us = <500>;
+			exit-latency-us = <1500>;
+			min-residency-us = <2500>;
+			wakeup-latency-us = <1700>;
+		};
+
+		CPU_SLEEP_1_0: cpu-sleep-1-0 {
+			compatible = "arm,idle-state";
+			power-rank = <1>;
+			entry-method-param = <0x0010000>;
+			entry-latency-us = <300>;
+			exit-latency-us = <500>;
+			min-residency-us = <900>;
+			wakeup-latency-us = <600>;
+		};
+
+		CLUSTER_SLEEP_1: cluster-sleep-1 {
+			compatible = "arm,idle-state";
+			power-rank = <3>;
+			entry-method-param = <0x1010000>;
+			entry-latency-us = <800>;
+			exit-latency-us = <2000>;
+			min-residency-us = <6500>;
+			wakeup-latency-us = <2300>;
+		};
+	};
+
+	CPU0: cpu at 0 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a15";
+		reg = <0x0>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_SLEEP_0_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU1: cpu at 1 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a15";
+		reg = <0x1>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_SLEEP_0_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU2: cpu at 2 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a15";
+		reg = <0x2>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_SLEEP_0_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU3: cpu at 3 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a15";
+		reg = <0x3>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_SLEEP_0_0 &CLUSTER_SLEEP_0>;
+	};
+
+	CPU4: cpu at 100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a7";
+		reg = <0x100>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_SLEEP_1_0 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU5: cpu at 101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a7";
+		reg = <0x101>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_SLEEP_1_0 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU6: cpu at 102 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a7";
+		reg = <0x102>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_SLEEP_1_0 &CLUSTER_SLEEP_1>;
+	};
+
+	CPU7: cpu at 103 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a7";
+		reg = <0x103>;
+		enable-method = "psci";
+		cpu-idle-states = <&CPU_SLEEP_1_0 &CLUSTER_SLEEP_1>;
+	};
+};
+
+===========================================
+5 - References
+===========================================
+
+[1] ARM Linux Kernel documentation - CPUs bindings
+    Documentation/devicetree/bindings/arm/cpus.txt
+
+[2] ARM Linux Kernel documentation - PSCI bindings
+    Documentation/devicetree/bindings/arm/psci.txt
+
+[3] ARM Server Base System Architecture (SBSA)
+    http://infocenter.arm.com/help/index.jsp
+
+[4] ARM Architecture Reference Manuals
+    http://infocenter.arm.com/help/index.jsp
+
+[5] ePAPR standard
+    https://www.power.org/documentation/epapr-version-1-1/

Signature · 1 line

1.9.1

Quoted 15 lines · Lorenzo Pieralisi

ARM based platforms implement a variety of power management schemes that allow processors to enter idle states at run-time.
The parameters defining these idle states vary on a per-platform basis forcing the OS to hardcode the state parameters in platform specific static tables whose size grows as the number of platforms supported in the kernel increases and hampers device drivers standardization.

Therefore, this patch aims at standardizing idle state device tree bindings for ARM platforms. Bindings define idle state parameters inclusive of entry methods and state latencies, to allow operating systems to retrieve the configuration entries from the device tree and initialize the related power management drivers, paving the way for common code in the kernel to deal with idle states and removing the need for static data in current and previous kernel versions.

Quoted 2 lines

Reviewed-by Sebastian Capella <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

Excellent.

Reviewed-by Nicolas Pitre <[email protected]>

Quoted 22 lines · Nicolas Pitre

Quoted 15 lines · Lorenzo Pieralisi

ARM based platforms implement a variety of power management schemes that allow processors to enter idle states at run-time.
The parameters defining these idle states vary on a per-platform basis forcing the OS to hardcode the state parameters in platform specific static tables whose size grows as the number of platforms supported in the kernel increases and hampers device drivers standardization.

Therefore, this patch aims at standardizing idle state device tree bindings for ARM platforms. Bindings define idle state parameters inclusive of entry methods and state latencies, to allow operating systems to retrieve the configuration entries from the device tree and initialize the related power management drivers, paving the way for common code in the kernel to deal with idle states and removing the need for static data in current and previous kernel versions.

Quoted 2 lines

Reviewed-by Sebastian Capella <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

Excellent.

Quoted 1 line

Reviewed-by Nicolas Pitre <[email protected]>

Thanks Nico, there are still a couple of niggles to sort out (ie local timer state), but the bulk of the document should be complete I hope.

I will postpone adding your (and Seb's) Reviewed-by until we have a final agreement if it is ok with you.

On most common ARM systems, the low-power states a CPU can be put into are not discoverable in HW and require device tree bindings to describe power down suspend operations and idle states parameters.

In order to enable DT based idle states and configure idle drivers, this patch implements the bulk infrastructure required to parse the device tree idle states bindings and initialize the corresponding CPUidle driver states data.

Code that initializes idle states checks the CPU idle driver cpumask so that multiple CPU idle drivers can be initialized through it in the kernel. The CPU idle driver cpumask defines which idle states should be considered valid for the driver, ie idle states that are valid on a set of cpus the idle driver manages.

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 drivers/cpuidle/Kconfig          |   8 ++
 drivers/cpuidle/Makefile         |   1 +
 drivers/cpuidle/dt_idle_states.c | 283 +++++++++++++++++++++++++++++++++++++++
 drivers/cpuidle/dt_idle_states.h |   8 ++
 4 files changed, 300 insertions(+)
 create mode 100644 drivers/cpuidle/dt_idle_states.c
 create mode 100644 drivers/cpuidle/dt_idle_states.h

drivers/cpuidle/Kconfig +8 -0

index 1b96fb9..414e7a96 100644
@@ -30,6 +30,14 @@ config CPU_IDLE_GOV_MENU
 	bool "Menu governor (for tickless system)"
 	default y
 
+config DT_IDLE_STATES
+        bool "Idle states DT support"
+	depends on ARM || ARM64
+	help
+	 Allows the CPU idle framework to initialize CPU idle drivers
+	 state data by using DT provided nodes compliant with idle states
+	 device tree bindings.
+
 menu "ARM CPU Idle Drivers"
 depends on ARM
 source "drivers/cpuidle/Kconfig.arm"

drivers/cpuidle/Makefile +1 -0

index d8bb1ff..b27a062 100644
@@ -4,6 +4,7 @@
 
 obj-y += cpuidle.o driver.o governor.o sysfs.o governors/
 obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
+obj-$(CONFIG_DT_IDLE_STATES)		  += dt_idle_states.o
 
 ##################################################################################
 # ARM SoC drivers

drivers/cpuidle/dt_idle_states.c new file +20 -0

new file mode 100644
index 0000000..5c16001c
@@ -0,0 +1,283 @@
+/*
+ * DT idle states parsing code.
+ *
+ * Copyright (C) 2014 ARM Ltd.
+ * Author: Lorenzo Pieralisi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "DT idle-states: " fmt
+
+#include <linux/cpuidle.h>
+#include <linux/cpumask.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/list_sort.h>
+#include <linux/module.h>

drivers/cpuidle/dt_idle_states.c new file +13 -0

+static bool __init state_cpus_valid(const cpumask_t *cpus,
+				    struct device_node *state_node)
+{
+	int cpu;
+	struct device_node *cpu_node;
+
+	/*
+	 * Check if state is valid on driver cpumask cpus
+	 */
+	for_each_cpu(cpu, cpus) {
+		cpu_node = of_get_cpu_node(cpu, NULL);
+
+		if (!cpu_node) {

drivers/cpuidle/dt_idle_states.c new file +16 -0

+
+		if (of_property_read_u32(state_node, "exit-latency-us",
+					 &exit_latency)) {
+			pr_debug(" * %s missing exit-latency-us property\n",
+				 state_node->full_name);
+			return;
+		}
+		/*
+		 * If wakeup-latency-us is missing, default to entry+exit
+		 * latencies as defined in idle states bindings
+		 */
+		idle_state->exit_latency = entry_latency + exit_latency;
+	}
+
+	if (of_property_read_u32(state_node, "min-residency-us",
+				 &idle_state->target_residency)) {

drivers/cpuidle/dt_idle_states.c new file +33 -0

+	}
+	/*
+	 * Sort the states list before initializing the CPUidle driver
+	 * states array.
+	 */
+	list_sort(NULL, &head, state_cmp);
+}
+
+/**
+ * dt_init_idle_driver() - Parse the DT idle states and initialize the
+ *			   idle driver states array
+ *
+ * @drv:	  Pointer to CPU idle driver to be initialized
+ * @state_nodes:  Array of struct device_nodes to be initialized if
+ *		  init_nodes == true. Must be sized CPUIDLE_STATE_MAX
+ * @start_idx:    First idle state index to be initialized
+ * @init_nodes:   Boolean to request device nodes initialization
+ *
+ * On success the states array in the cpuidle driver contains
+ * initialized entries in the states array, starting from index start_idx.
+ * If init_nodes == true, on success the state_nodes array is initialized
+ * with idle state DT node pointers, starting from index start_idx,
+ * in a 1:1 relation with the idle driver states array.
+ *
+ * Return:
+ *	0 on success
+ *	<0 on failure
+ */
+int __init dt_init_idle_driver(struct cpuidle_driver *drv,
+			       struct device_node *state_nodes[],
+			       unsigned int start_idx, bool init_nodes)
+{
+	struct device_node *idle_states_node;

On Wed, Jun 25, 2014 at 03:10:16PM +0100, Lorenzo Pieralisi wrote:

Quoted 31 lines

drivers/cpuidle/dt_idle_states.c new file +25 -0

new file mode 100644
index 0000000..5c16001c
@@ -0,0 +1,283 @@
+/*
+ * DT idle states parsing code.
+ *
+ * Copyright (C) 2014 ARM Ltd.
+ * Author: Lorenzo Pieralisi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "DT idle-states: " fmt
+
+#include <linux/cpuidle.h>
+#include <linux/cpumask.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/list_sort.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+#include "dt_idle_states.h"
+

drivers/cpuidle/dt_idle_states.c line 30

Quoted 5 lines

drivers/cpuidle/dt_idle_states.c new file +5 -0

+struct state_elem {
+	struct list_head list;
+	struct device_node *node;
+	u32 val;
+};

Ah. So the fixed-size entry parameter requirement is because this code is in charge of allocating and freeing these structs?

Quoted 6 lines

drivers/cpuidle/dt_idle_states.c new file +6 -0

+
+static struct list_head head __initdata = LIST_HEAD_INIT(head);
+
+static bool __init state_cpu_valid(struct device_node *state_node,
+				   struct device_node *cpu_node)
+{

drivers/cpuidle/dt_idle_states.c line 42

Quoted 6 lines

drivers/cpuidle/dt_idle_states.c new file +6 -0

+	int i = 0;
+	struct device_node *cpu_state;
+
+	while ((cpu_state = of_parse_phandle(cpu_node,
+					     "cpu-idle-states", i++))) {
+		if (cpu_state && state_node == cpu_state) {

You can drop the cpu_state NULL check, it's implicit in the while loop.

drivers/cpuidle/dt_idle_states.c line 49

Quoted 7 lines

drivers/cpuidle/dt_idle_states.c new file +7 -0

+			of_node_put(cpu_state);
+			return true;
+		}
+		of_node_put(cpu_state);
+	}
+	return false;
+}

Is it possible to use a bool ret variable to avoid the two of_node_put cases? Or does that end up making this larger?

Quoted 13 lines

drivers/cpuidle/dt_idle_states.c new file +13 -0

+static bool __init state_cpus_valid(const cpumask_t *cpus,
+				    struct device_node *state_node)
+{
+	int cpu;
+	struct device_node *cpu_node;
+
+	/*
+	 * Check if state is valid on driver cpumask cpus
+	 */
+	for_each_cpu(cpu, cpus) {
+		cpu_node = of_get_cpu_node(cpu, NULL);
+
+		if (!cpu_node) {

drivers/cpuidle/dt_idle_states.c line 73

Quoted 10 lines

drivers/cpuidle/dt_idle_states.c new file +10 -0

+			pr_err("Missing device node for CPU %d\n", cpu);
+			return false;
+		}
+
+		if (!state_cpu_valid(state_node, cpu_node))
+			return false;
+	}
+
+	return true;
+}

Doesn't this leave all the cpu node refcounts incremented? (it's painful to get device node refcounting right, I know).

I think you can use the similarly named of_cpu_device_node_get to find the CPU node. It uses the pointer stored in cpu->dev.of_node, so it doesn't have to walk the tree to find the CPU node. It also doesn't increment the refcount.

Unless this is too early for that?

drivers/cpuidle/dt_idle_states.c line 130

Quoted 12 lines

drivers/cpuidle/dt_idle_states.c new file +12 -0

+static void __init init_state_node(struct cpuidle_driver *drv,
+				   struct device_node *state_node,
+				   int *cnt)
+{
+	struct cpuidle_state *idle_state;
+
+	pr_debug(" * %s...\n", state_node->full_name);
+
+	idle_state = &drv->states[*cnt];
+
+	if (of_property_read_u32(state_node, "wakeup-latency-us",
+				 &idle_state->exit_latency)) {

I'm not a fan of this construction, as the obvious reading is that we take the branch if we succeeded (which obviously isn't true as of_property_read_* return error codes).

Could we change it to something like:

	err = of_property_read_u32(state_node, "wakeup-latency-us",
				   &idle_state->exit_latency);
	if (err) {

drivers/cpuidle/dt_idle_states.c line 138

Quoted 8 lines

drivers/cpuidle/dt_idle_states.c new file +8 -0

+		u32 entry_latency, exit_latency;
+
+		if (of_property_read_u32(state_node, "entry-latency-us",
+					 &entry_latency)) {
+			pr_debug(" * %s missing entry-latency-us property\n",
+				 state_node->full_name);
+			return;
+		}

Returning without error code? Do the fields have sane default values?

Or is this safe because we didn't increment cnt?

Quoted 16 lines

drivers/cpuidle/dt_idle_states.c new file +16 -0

+
+		if (of_property_read_u32(state_node, "exit-latency-us",
+					 &exit_latency)) {
+			pr_debug(" * %s missing exit-latency-us property\n",
+				 state_node->full_name);
+			return;
+		}
+		/*
+		 * If wakeup-latency-us is missing, default to entry+exit
+		 * latencies as defined in idle states bindings
+		 */
+		idle_state->exit_latency = entry_latency + exit_latency;
+	}
+
+	if (of_property_read_u32(state_node, "min-residency-us",
+				 &idle_state->target_residency)) {

drivers/cpuidle/dt_idle_states.c line 165

Quoted 10 lines

drivers/cpuidle/dt_idle_states.c new file +10 -0

+		pr_debug(" * %s missing min-residency-us property\n",
+			     state_node->full_name);
+		return;
+	}
+
+	idle_state->flags = CPUIDLE_FLAG_TIME_VALID;
+	if (!of_property_read_bool(state_node, "timer-state-retained"))
+		idle_state->flags |= CPUIDLE_FLAG_TIMER_STOP;
+ 	strncpy(idle_state->name, state_node->name, CPUIDLE_NAME_LEN);
+	strncpy(idle_state->desc, state_node->name, CPUIDLE_NAME_LEN);

Does the name make sense as a desc? Is a desc necessary?

CPUIDLE_DESC_LEN seems to exist, and is double CPUIDLE_NAME_LEN.

drivers/cpuidle/dt_idle_states.c line 216

Quoted 7 lines

drivers/cpuidle/dt_idle_states.c new file +7 -0

+static void __init add_idle_states(struct cpuidle_driver *drv,
+				   struct device_node *idle_states)
+{
+	struct device_node *state_node;
+
+	for_each_child_of_node(idle_states, state_node) {
+		if ((!of_device_is_compatible(state_node, "arm,idle-state"))) {

Holy brackets batman! I think we can drop the outer ones given there's no assignment we want to supress warnings for.

drivers/cpuidle/dt_idle_states.c line 218

Quoted 2 lines

drivers/cpuidle/dt_idle_states.c new file +2 -0

+			pr_warn(" * %s: children of /cpus/idle-states must be \"arm,idle-state\" compatible\n",
+				     state_node->full_name);

Presumably the entire reason for having the compatible string is for future extensibility.

It would probably be better to have something like:

	pr_warn("Node %s has unrecognised/missing compatible string\n",
		state_node->full_name);

drivers/cpuidle/dt_idle_states.c line 227

Quoted 9 lines

drivers/cpuidle/dt_idle_states.c new file +9 -0

+			continue;
+		}
+		/*
+		 * If memory allocation fails, better bail out.
+		 * Initialized nodes are freed at initialization
+		 * completion in of_init_idle_driver().
+		 */
+		if ((add_state_node(drv->cpumask, state_node) == -ENOMEM))
+			break;

Can we not return? Or is the list sort important in the error case too?

Quoted 38 lines

drivers/cpuidle/dt_idle_states.c new file +38 -0

+	}
+	/*
+	 * Sort the states list before initializing the CPUidle driver
+	 * states array.
+	 */
+	list_sort(NULL, &head, state_cmp);
+}
+
+/**
+ * dt_init_idle_driver() - Parse the DT idle states and initialize the
+ *			   idle driver states array
+ *
+ * @drv:	  Pointer to CPU idle driver to be initialized
+ * @state_nodes:  Array of struct device_nodes to be initialized if
+ *		  init_nodes == true. Must be sized CPUIDLE_STATE_MAX
+ * @start_idx:    First idle state index to be initialized
+ * @init_nodes:   Boolean to request device nodes initialization
+ *
+ * On success the states array in the cpuidle driver contains
+ * initialized entries in the states array, starting from index start_idx.
+ * If init_nodes == true, on success the state_nodes array is initialized
+ * with idle state DT node pointers, starting from index start_idx,
+ * in a 1:1 relation with the idle driver states array.
+ *
+ * Return:
+ *	0 on success
+ *	<0 on failure
+ */
+int __init dt_init_idle_driver(struct cpuidle_driver *drv,
+			       struct device_node *state_nodes[],
+			       unsigned int start_idx, bool init_nodes)
+{
+	struct device_node *idle_states_node;
+	int ret;
+
+	if (start_idx >= CPUIDLE_STATE_MAX) {
+		pr_warn("State index exceeds static CPU idle driver states array size\n");
+		return -EINVAL;

drivers/cpuidle/dt_idle_states.c line 270

Quoted 5 lines

drivers/cpuidle/dt_idle_states.c new file +5 -0

+	}
+
+	if (WARN(init_nodes && !state_nodes,
+		"Requested nodes stashing in an invalid nodes container\n"))
+		return -EINVAL;

That warning message is somewhat confusing, and I'm not sure I follow the logic.

This patch implements the cpu_suspend cpu operations method through the PSCI CPU_SUSPEND API. The PSCI implementation translates the idle state index passed by the cpu_suspend core call into a valid PSCI state according to the PSCI states initialized at boot by the PSCI suspend backend.

Entry point is set to cpu_resume physical address, that represents the default kernel execution address following a CPU reset.

Idle state indices missing a DT node description are initialized to power state standby WFI so that if called by the idle driver they provide the default behaviour.

Reviewed-by Sebastian Capella <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 arch/arm64/include/asm/psci.h |   4 ++
 arch/arm64/kernel/psci.c      | 103 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

Quoted 18 lines · Lorenzo Pieralisi

This patch implements the cpu_suspend cpu operations method through the PSCI CPU_SUSPEND API. The PSCI implementation translates the idle state index passed by the cpu_suspend core call into a valid PSCI state according to the PSCI states initialized at boot by the PSCI suspend backend.

Entry point is set to cpu_resume physical address, that represents the default kernel execution address following a CPU reset.

Idle state indices missing a DT node description are initialized to power state standby WFI so that if called by the idle driver they provide the default behaviour.

Reviewed-by Sebastian Capella <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 arch/arm64/include/asm/psci.h |   4 ++
 arch/arm64/kernel/psci.c      | 103 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

[...]

arch/arm64/kernel/psci.c static u32 psci_power_state_pack(struct psci_power_state state) · line 111

Quoted 11 lines

arch/arm64/kernel/psci.c +11 -0

+static void psci_power_state_unpack(u32 power_state,
+				    struct psci_power_state *state)
+{
+	state->id = (power_state & PSCI_0_2_POWER_STATE_ID_MASK) >>
+			PSCI_0_2_POWER_STATE_ID_SHIFT;
+	state->type = (power_state & PSCI_0_2_POWER_STATE_TYPE_MASK) >>
+			PSCI_0_2_POWER_STATE_TYPE_SHIFT;
+	state->affinity_level =
+			(power_state & PSCI_0_2_POWER_STATE_AFFL_MASK) >>
+			PSCI_0_2_POWER_STATE_AFFL_SHIFT;
+}

Is this valid for PSCI versions prior to 0.2?

Quoted 7 lines

/*
* The following two functions are invoked via the invoke_psci_fn pointer
* and will not be inlined, allowing us to piggyback on the AAPCS.

arch/arm64/kernel/psci.c +0 -0

@@ -199,6 +216,77 @@ static int psci_migrate_info_type(void)
 	return err;
 }
 

arch/arm64/kernel/psci.c static int psci_migrate_info_type(void) · line 222

Quoted 4 lines

arch/arm64/kernel/psci.c +4 -0

+int __init psci_dt_register_idle_states(struct cpuidle_driver *drv,
+					struct device_node *state_nodes[])
+{
+	int cpu, i;

Perhaps unsigned int? You print i with %u below.

Quoted 6 lines

arch/arm64/kernel/psci.c +6 -0

+	for (i = 0; i < drv->state_count; i++) {
+		u32 psci_power_state;
+
+		if (!state_nodes[i]) {
+			/*
+			 * An index with a missing node pointer falls back to

arch/arm64/kernel/psci.c static int psci_migrate_info_type(void) · line 268

Quoted 5 lines

arch/arm64/kernel/psci.c +5 -0

+			 * simple STANDBYWFI
+			 */
+			psci_states[i].type = PSCI_POWER_STATE_TYPE_STANDBY;
+			continue;
+		}

Does this make sense? Are there any limitations on which state nodes could be missing?

arch/arm64/kernel/psci.c static int psci_migrate_info_type(void) · line 279

Quoted 11 lines

arch/arm64/kernel/psci.c +11 -0

+
+		if (of_property_read_u32(state_nodes[i], "entry-method-param",
+					 &psci_power_state)) {
+			pr_warn(" * %s missing entry-method-param property\n",
+				state_nodes[i]->full_name);
+			/*
+			 * If entry-method-param property is missing, fall
+			 * back to STANDBYWFI state
+			 */
+			psci_states[i].type = PSCI_POWER_STATE_TYPE_STANDBY;
+			continue;

Surely we want to throw away these states instead?

Otherwise we can get into a mess like:

psci_states[0] => low power state psci_states[1] => lower power state psci_states[2] => WFI / not low power psci_states[3] => lowest power state

Where power usage and latency would jump around rather than follow monotonic patterns.

Hi Lorenzo,

Quoted 1 line · Lorenzo Pieralisi

arch/arm64/kernel/psci.c +0 -0

arch/arm64/kernel/psci.c static int psci_migrate_info_type(void) · line 238

Quoted 11 lines

arch/arm64/kernel/psci.c +11 -0

+	/*
+	 * This is belt-and-braces: make sure that if the idle
+	 * specified protocol is psci, the cpu_ops have been
+	 * initialized to psci operations. Anything else is
+	 * a recipe for mayhem.
+	 */
+	for_each_cpu(cpu, drv->cpumask) {
+		cpu_ops_ptr = cpu_ops[cpu];
+		if (WARN_ON(!cpu_ops_ptr || strcmp(cpu_ops_ptr->name, "psci")))
+			return -EOPNOTSUPP;
+	}

I'm not sure how drv->cpumask is setup, but if a system has mixed enable methods, say some cpus 'spin-table' and some 'psci', will this give a false error?

If drv->cpumask should only include 'psci' cpus, then should this be a BUG()?

Quoted 8 lines

arch/arm64/kernel/psci.c +8 -0

+
+	psci_states = kcalloc(drv->state_count, sizeof(*psci_states),
+			      GFP_KERNEL);
+
+	if (!psci_states) {
+		pr_warn("psci idle state allocation failed\n");
+		return -ENOMEM;
+	}

arch/arm64/kernel/psci.c static int psci_migrate_info_type(void) · line 251

Quoted 5 lines

arch/arm64/kernel/psci.c +5 -0

+
+	for_each_cpu(cpu, drv->cpumask) {
+		if (per_cpu(psci_power_state, cpu)) {
+			pr_warn("idle states already initialized on cpu %u\n",
+				cpu);

This seems like an implementation problem, if so, shouldn't this be pr_debug()?

Quoted 11 lines

#endif

arch/arm64/kernel/psci.c +9 -0

+#ifdef CONFIG_ARM64_CPU_SUSPEND
+static int cpu_psci_cpu_suspend(unsigned long index)
+{
+	struct psci_power_state *state = __get_cpu_var(psci_power_state);
+
+	if (!state)
+		return -EOPNOTSUPP;
+
+	return psci_ops.cpu_suspend(state[index], virt_to_phys(cpu_resume));

arch/arm64/kernel/psci.c static int cpu_psci_cpu_kill(unsigned int cpu) · line 536

Quoted 2 lines

arch/arm64/kernel/psci.c +2 -0

+}
+#endif

Why not put a __maybe_unused attribute on cpu_psci_cpu_suspend() and remove the preprocessor conditional. That way this code will always be compiled, and with therefor always get a build test. The linker should strip out the unused code when CONFIG_ARM64_CPU_SUSPEND=n and the code below is not compiled.

This patch implements a generic CPU idle driver for ARM64 machines.

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the big.LITTLE CPUidle driver to dynamically initialize idle states data through the updated device tree source file.

Cc Chander Kashyap <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 Documentation/devicetree/bindings/arm/vexpress.txt | 25 +++++++++++++
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts         | 25 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-big_little.c               | 43 +++++++++++-----------
 4 files changed, 73 insertions(+), 21 deletions(-)

Documentation/devicetree/bindings/arm/vexpress.txt +0 -0

index 39844cd..7f52ac8 100644
@@ -67,6 +67,28 @@ with device_type = "cpu" property for every available core, eg.:
 		};
 	};

Documentation/devicetree/bindings/arm/vexpress.txt +16 -0

+
+Versatile Express idle-states nodes example:
+
+	idle-states {
+		entry-method = "arm,vexpress-v2p-ca15_a7";
+
+		cluster-sleep-0 {
+			compatible = "arm,idle-state";
+			entry-latency-us = <1000>;
+			exit-latency-us = <700>;
+			min-residency-us = <3500>;
+		};
+	};
 
 Configuration infrastructure
 ----------------------------

@@ -227,3 +249,6 @@ Example of a VE tile description (simplified)
 	};
 };
 
+
+[1] ARM Linux Kernel documentation - Idle states bindings
+    Documentation/devicetree/bindings/arm/idle-states.txt

arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts +25 -0

index a25c262..ad28242 100644
@@ -33,11 +33,32 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		idle-states {
+			entry-method = "arm,vexpress-v2p-ca15_a7";
+
+			CLUSTER_SLEEP_BIG: cluster-sleep-big {
+				compatible = "arm,idle-state";
+				power-rank = <0>;
+				entry-latency-us = <1000>;
+				exit-latency-us = <700>;
+				min-residency-us = <3500>;
+			};
+
+			CLUSTER_SLEEP_LITTLE: cluster-sleep-little {
+				compatible = "arm,idle-state";
+				power-rank = <0>;
+				entry-latency-us = <1000>;
+				exit-latency-us = <500>;
+				min-residency-us = <3000>;
+			};
+		};
+
 		cpu0: cpu at 0 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a15";
 			reg = <0>;
 			cci-control-port = <&cci_control1>;
+			cpu-idle-states = <&CLUSTER_SLEEP_BIG>;
 		};
 
 		cpu1: cpu at 1 {

@@ -45,6 +66,7 @@
 			compatible = "arm,cortex-a15";
 			reg = <1>;
 			cci-control-port = <&cci_control1>;
+			cpu-idle-states = <&CLUSTER_SLEEP_BIG>;
 		};
 
 		cpu2: cpu at 2 {

@@ -52,6 +74,7 @@
 			compatible = "arm,cortex-a7";
 			reg = <0x100>;
 			cci-control-port = <&cci_control2>;
+			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
 		};
 
 		cpu3: cpu at 3 {

@@ -59,6 +82,7 @@
 			compatible = "arm,cortex-a7";
 			reg = <0x101>;
 			cci-control-port = <&cci_control2>;
+			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
 		};
 
 		cpu4: cpu at 4 {

@@ -66,6 +90,7 @@
 			compatible = "arm,cortex-a7";
 			reg = <0x102>;
 			cci-control-port = <&cci_control2>;
+			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
 		};
 	};
 

drivers/cpuidle/Kconfig.arm +1 -0

index b6d69e8..a9b089c 100644
@@ -12,6 +12,7 @@ config ARM_BIG_LITTLE_CPUIDLE
 	depends on ARCH_VEXPRESS_TC2_PM
 	select ARM_CPU_SUSPEND
 	select CPU_IDLE_MULTIPLE_DRIVERS
+	select DT_IDLE_STATES
 	help
 	  Select this option to enable CPU idle driver for big.LITTLE based
 	  ARM systems. Driver manages CPUs coordination through MCPM and

drivers/cpuidle/cpuidle-big_little.c +22 -21

index b45fc62..6712441 100644
@@ -24,6 +24,8 @@
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
 
+#include "dt_idle_states.h"
+
 static int bl_enter_powerdown(struct cpuidle_device *dev,
 			      struct cpuidle_driver *drv, int idx);
 

@@ -61,32 +63,12 @@ static struct cpuidle_driver bl_idle_little_driver = {
 	.name = "little_idle",
 	.owner = THIS_MODULE,
 	.states[0] = ARM_CPUIDLE_WFI_STATE,
-	.states[1] = {
-		.enter			= bl_enter_powerdown,
-		.exit_latency		= 700,
-		.target_residency	= 2500,
-		.flags			= CPUIDLE_FLAG_TIME_VALID |
-					  CPUIDLE_FLAG_TIMER_STOP,
-		.name			= "C1",
-		.desc			= "ARM little-cluster power down",
-	},
-	.state_count = 2,
 };
 
 static struct cpuidle_driver bl_idle_big_driver = {
 	.name = "big_idle",
 	.owner = THIS_MODULE,
 	.states[0] = ARM_CPUIDLE_WFI_STATE,
-	.states[1] = {
-		.enter			= bl_enter_powerdown,
-		.exit_latency		= 500,
-		.target_residency	= 2000,
-		.flags			= CPUIDLE_FLAG_TIME_VALID |
-					  CPUIDLE_FLAG_TIMER_STOP,
-		.name			= "C1",
-		.desc			= "ARM big-cluster power down",
-	},
-	.state_count = 2,
 };
 
 /*

@@ -165,7 +147,8 @@ static int __init bl_idle_driver_init(struct cpuidle_driver *drv, int cpu_id)
 
 static int __init bl_idle_init(void)
 {
-	int ret;
+	int ret, i;
+	struct cpuidle_driver *drv;
 
 	/*
 	 * Initialize the driver just for a compliant set of machines

@@ -187,6 +170,24 @@ static int __init bl_idle_init(void)
 	if (ret)
 		goto out_uninit_little;
 
+	 /* Start at index 1, index 0 standard WFI */
+	ret = dt_init_idle_driver(&bl_idle_big_driver, NULL, 1, false);
+	if (ret)
+		goto out_uninit_big;
+
+	 /* Start at index 1, index 0 standard WFI */
+	ret = dt_init_idle_driver(&bl_idle_little_driver, NULL, 1, false);
+	if (ret)
+		goto out_uninit_big;
+
+	drv = &bl_idle_big_driver;
+	for (i = 1; i < drv->state_count; i++)
+		drv->states[i].enter = bl_enter_powerdown;
+
+	drv = &bl_idle_little_driver;
+	for (i = 1; i < drv->state_count; i++)
+		drv->states[i].enter = bl_enter_powerdown;
+
 	ret = cpuidle_register(&bl_idle_little_driver, NULL);
 	if (ret)
 		goto out_uninit_big;

Signature · 1 line

1.9.1

Quoted 23 lines · Lorenzo Pieralisi

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the big.LITTLE CPUidle driver to dynamically initialize idle states data through the updated device tree source file.

Cc Chander Kashyap <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 Documentation/devicetree/bindings/arm/vexpress.txt | 25 +++++++++++++
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts         | 25 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-big_little.c               | 43 +++++++++++-----------
 4 files changed, 73 insertions(+), 21 deletions(-)

Documentation/devicetree/bindings/arm/vexpress.txt +0 -0

index 39844cd..7f52ac8 100644
@@ -67,6 +67,28 @@ with device_type = "cpu" property for every available core, eg.:
 		};
 	};
 

Documentation/devicetree/bindings/arm/vexpress.txt with device_type = "cpu" property for every available core, eg.: · line 78

Quoted 9 lines

Documentation/devicetree/bindings/arm/vexpress.txt +9 -0

+idle-states node
+----------------
+
+On Versatile Express platforms with power management capabilities, the device
+tree source file must contain the idle-states node[1]. As defined in [1] the
+idle-states node must contain an entry-method property that for Versatile
+Express platforms can be one of:
+
+	- "arm,vexpress-v2p-ca15_a7"

... and what does this mean? It's the name we've assigned the platform in the Linux DT bindings, but this binding document tells me nothing about how this method works.

This feels like leaking Linux internals rather than a reusable interface.

Mark.

Quoted 188 lines

Documentation/devicetree/bindings/arm/vexpress.txt +16 -0

+
+Versatile Express idle-states nodes example:
+
+	idle-states {
+		entry-method = "arm,vexpress-v2p-ca15_a7";
+
+		cluster-sleep-0 {
+			compatible = "arm,idle-state";
+			entry-latency-us = <1000>;
+			exit-latency-us = <700>;
+			min-residency-us = <3500>;
+		};
+	};
 
 Configuration infrastructure
 ----------------------------

@@ -227,3 +249,6 @@ Example of a VE tile description (simplified)
 	};
 };
 
+
+[1] ARM Linux Kernel documentation - Idle states bindings
+    Documentation/devicetree/bindings/arm/idle-states.txt

arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts +25 -0

index a25c262..ad28242 100644
@@ -33,11 +33,32 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		idle-states {
+			entry-method = "arm,vexpress-v2p-ca15_a7";
+
+			CLUSTER_SLEEP_BIG: cluster-sleep-big {
+				compatible = "arm,idle-state";
+				power-rank = <0>;
+				entry-latency-us = <1000>;
+				exit-latency-us = <700>;
+				min-residency-us = <3500>;
+			};
+
+			CLUSTER_SLEEP_LITTLE: cluster-sleep-little {
+				compatible = "arm,idle-state";
+				power-rank = <0>;
+				entry-latency-us = <1000>;
+				exit-latency-us = <500>;
+				min-residency-us = <3000>;
+			};
+		};
+
 		cpu0: cpu at 0 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a15";
 			reg = <0>;
 			cci-control-port = <&cci_control1>;
+			cpu-idle-states = <&CLUSTER_SLEEP_BIG>;
 		};
 
 		cpu1: cpu at 1 {

@@ -45,6 +66,7 @@
 			compatible = "arm,cortex-a15";
 			reg = <1>;
 			cci-control-port = <&cci_control1>;
+			cpu-idle-states = <&CLUSTER_SLEEP_BIG>;
 		};
 
 		cpu2: cpu at 2 {

@@ -52,6 +74,7 @@
 			compatible = "arm,cortex-a7";
 			reg = <0x100>;
 			cci-control-port = <&cci_control2>;
+			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
 		};
 
 		cpu3: cpu at 3 {

@@ -59,6 +82,7 @@
 			compatible = "arm,cortex-a7";
 			reg = <0x101>;
 			cci-control-port = <&cci_control2>;
+			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
 		};
 
 		cpu4: cpu at 4 {

@@ -66,6 +90,7 @@
 			compatible = "arm,cortex-a7";
 			reg = <0x102>;
 			cci-control-port = <&cci_control2>;
+			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
 		};
 	};
 

drivers/cpuidle/Kconfig.arm +1 -0

index b6d69e8..a9b089c 100644
@@ -12,6 +12,7 @@ config ARM_BIG_LITTLE_CPUIDLE
 	depends on ARCH_VEXPRESS_TC2_PM
 	select ARM_CPU_SUSPEND
 	select CPU_IDLE_MULTIPLE_DRIVERS
+	select DT_IDLE_STATES
 	help
 	  Select this option to enable CPU idle driver for big.LITTLE based
 	  ARM systems. Driver manages CPUs coordination through MCPM and

drivers/cpuidle/cpuidle-big_little.c +22 -21

index b45fc62..6712441 100644
@@ -24,6 +24,8 @@
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
 
+#include "dt_idle_states.h"
+
 static int bl_enter_powerdown(struct cpuidle_device *dev,
 			      struct cpuidle_driver *drv, int idx);
 

@@ -61,32 +63,12 @@ static struct cpuidle_driver bl_idle_little_driver = {
 	.name = "little_idle",
 	.owner = THIS_MODULE,
 	.states[0] = ARM_CPUIDLE_WFI_STATE,
-	.states[1] = {
-		.enter			= bl_enter_powerdown,
-		.exit_latency		= 700,
-		.target_residency	= 2500,
-		.flags			= CPUIDLE_FLAG_TIME_VALID |
-					  CPUIDLE_FLAG_TIMER_STOP,
-		.name			= "C1",
-		.desc			= "ARM little-cluster power down",
-	},
-	.state_count = 2,
 };
 
 static struct cpuidle_driver bl_idle_big_driver = {
 	.name = "big_idle",
 	.owner = THIS_MODULE,
 	.states[0] = ARM_CPUIDLE_WFI_STATE,
-	.states[1] = {
-		.enter			= bl_enter_powerdown,
-		.exit_latency		= 500,
-		.target_residency	= 2000,
-		.flags			= CPUIDLE_FLAG_TIME_VALID |
-					  CPUIDLE_FLAG_TIMER_STOP,
-		.name			= "C1",
-		.desc			= "ARM big-cluster power down",
-	},
-	.state_count = 2,
 };
 
 /*

@@ -165,7 +147,8 @@ static int __init bl_idle_driver_init(struct cpuidle_driver *drv, int cpu_id)
 
 static int __init bl_idle_init(void)
 {
-	int ret;
+	int ret, i;
+	struct cpuidle_driver *drv;
 
 	/*
 	 * Initialize the driver just for a compliant set of machines

@@ -187,6 +170,24 @@ static int __init bl_idle_init(void)
 	if (ret)
 		goto out_uninit_little;
 
+	 /* Start at index 1, index 0 standard WFI */
+	ret = dt_init_idle_driver(&bl_idle_big_driver, NULL, 1, false);
+	if (ret)
+		goto out_uninit_big;
+
+	 /* Start at index 1, index 0 standard WFI */
+	ret = dt_init_idle_driver(&bl_idle_little_driver, NULL, 1, false);
+	if (ret)
+		goto out_uninit_big;
+
+	drv = &bl_idle_big_driver;
+	for (i = 1; i < drv->state_count; i++)
+		drv->states[i].enter = bl_enter_powerdown;
+
+	drv = &bl_idle_little_driver;
+	for (i = 1; i < drv->state_count; i++)
+		drv->states[i].enter = bl_enter_powerdown;
+
 	ret = cpuidle_register(&bl_idle_little_driver, NULL);
 	if (ret)
 		goto out_uninit_big;

Signature · 1 line

1.9.1

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

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the Exynos CPUidle driver to dynamically initialize idle states data through the updated device tree source files.

Cc Kukjin Kim <[email protected]>

Cc Tomasz Figa <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
Compile tested, I am not sure I patched the right dts files, please check.

 .../devicetree/bindings/arm/exynos/idle-states.txt | 27 ++++++++++++++++++++
 arch/arm/boot/dts/exynos3250.dtsi                  | 16 ++++++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  | 15 +++++++++++
 arch/arm/boot/dts/exynos5410.dtsi                  | 17 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-exynos.c                   | 29 +++++++++++++---------
 6 files changed, 93 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/exynos/idle-states.txt

Documentation/devicetree/bindings/arm/exynos/idle-states.txt new file +0 -0

new file mode 100644
index 0000000..342ecb4

Quoted 29 lines · Lorenzo Pieralisi

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the Exynos CPUidle driver to dynamically initialize idle states data through the updated device tree source files.

Cc Kukjin Kim <[email protected]>

Cc Tomasz Figa <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
Compile tested, I am not sure I patched the right dts files, please check.

 .../devicetree/bindings/arm/exynos/idle-states.txt | 27 ++++++++++++++++++++
 arch/arm/boot/dts/exynos3250.dtsi                  | 16 ++++++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  | 15 +++++++++++
 arch/arm/boot/dts/exynos5410.dtsi                  | 17 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-exynos.c                   | 29 +++++++++++++---------
 6 files changed, 93 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/exynos/idle-states.txt

Documentation/devicetree/bindings/arm/exynos/idle-states.txt new file +0 -0

new file mode 100644
index 0000000..342ecb4

Documentation/devicetree/bindings/arm/exynos/idle-states.txt line 9

Quoted 9 lines

Documentation/devicetree/bindings/arm/exynos/idle-states.txt new file +9 -0

@@ -0,0 +1,27 @@
+idle-states node
+----------------
+
+On Exynos platforms with power management capabilities, the device
+tree source file must contain the idle-states node[1]. As defined in [1] the
+idle-states node must contain an entry-method property that for Exynos
+platforms can be one of:
+
+	- "samsung,exynos"

Similarly to the TC2 binding, what does this mean?

What is a kernel expected to do when it sees this entry-method?

Using "samsung,exynos" as the entry-method feels like something that's going to bite us; it sounds far too wide-reaching.

Quoted 6 lines

Documentation/devicetree/bindings/arm/exynos/idle-states.txt new file +2 -1

 static int exynos_cpuidle_probe(struct platform_device *pdev)
 {
-	int ret;
+	int ret, i;
+	struct cpuidle_driver *drv = &exynos_idle_driver;

Documentation/devicetree/bindings/arm/exynos/idle-states.txt

Quoted 4 lines

Documentation/devicetree/bindings/arm/exynos/idle-states.txt new file +1 -1

 	exynos_enter_aftr = (void *)(pdev->dev.platform_data);
 
-	ret = cpuidle_register(&exynos_idle_driver, NULL);
+	drv->cpumask = (struct cpumask *) cpu_possible_mask;

This assignment looks scary to me. Why do we need to do this, and why are we throwing away the constness of cpu_possible_mask?

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the Exynos CPUidle driver to dynamically initialize idle states data through the updated device tree source files.

Cc Kukjin Kim <[email protected]>

Cc Tomasz Figa <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
Compile tested, I am not sure I patched the right dts files, please check.

 .../devicetree/bindings/arm/exynos/idle-states.txt | 27 ++++++++++++++++++++
 arch/arm/boot/dts/exynos3250.dtsi                  | 16 ++++++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  | 15 +++++++++++
 arch/arm/boot/dts/exynos5410.dtsi                  | 17 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-exynos.c                   | 29 +++++++++++++---------
 6 files changed, 93 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/exynos/idle-states.txt

Hi,

Quoted 8 lines · Lorenzo Pieralisi

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the Exynos CPUidle driver to dynamically initialize idle states data through the updated device tree source files.

Quoted 5 lines

Cc Kukjin Kim <[email protected]>

Cc Tomasz Figa <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
Compile tested, I am not sure I patched the right dts files, please check.

cpuidle-exynos driver is currently working properly in deeper cpuidle mode (AFTR) on Exynos4210 and Exynos5250 (please also see the following patch from Tomasz Figa: [1]). There is ongoing work to AFTR mode work also on Exynos4x12 and Exynos3250 but it is not complete yet. Exynos5410 OTOH should probably use the generic big little cpuidle driver (this SoC is similar to Exynos5420 one for which Chander Kashyap has developed cpuidle-big_little support [2]).

Making long story short, I think that your patch should depend on patch
[1] and update only exynos4210.dtsi and exynos5250.dtsi. Also for your patch #6 there needs to be some coordination with merging of Chander's patchset ([2]).

[1] http://www.spinics.net/lists/arm-kernel/msg341023.html
[2] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg664470.html

Quoted 8 lines

 .../devicetree/bindings/arm/exynos/idle-states.txt | 27 ++++++++++++++++++++
 arch/arm/boot/dts/exynos3250.dtsi                  | 16 ++++++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  | 15 +++++++++++
 arch/arm/boot/dts/exynos5410.dtsi                  | 17 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-exynos.c                   | 29 +++++++++++++---------
 6 files changed, 93 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/exynos/idle-states.txt

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

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

Hi,

Quoted 8 lines · Lorenzo Pieralisi

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the Exynos CPUidle driver to dynamically initialize idle states data through the updated device tree source files.

Quoted 5 lines

Cc Kukjin Kim <[email protected]>

Cc Tomasz Figa <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
Compile tested, I am not sure I patched the right dts files, please check.

cpuidle-exynos driver is currently working properly in deeper cpuidle mode (AFTR) on Exynos4210 and Exynos5250 (please also see the following patch from Tomasz Figa: [1]). There is ongoing work to AFTR mode work also on Exynos4x12 and Exynos3250 but it is not complete yet. Exynos5410 OTOH should probably use the generic big little cpuidle driver (this SoC is similar to Exynos5420 one for which Chander Kashyap has developed cpuidle-big_little support [2]).

Making long story short, I think that your patch should depend on patch
[1] and update only exynos4210.dtsi and exynos5250.dtsi. Also for your patch #6 there needs to be some coordination with merging of Chander's patchset ([2]).

[1] http://www.spinics.net/lists/arm-kernel/msg341023.html
[2] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg664470.html

Quoted 8 lines

 .../devicetree/bindings/arm/exynos/idle-states.txt | 27 ++++++++++++++++++++
 arch/arm/boot/dts/exynos3250.dtsi                  | 16 ++++++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  | 15 +++++++++++
 arch/arm/boot/dts/exynos5410.dtsi                  | 17 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-exynos.c                   | 29 +++++++++++++---------
 6 files changed, 93 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/exynos/idle-states.txt

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

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

Quoted 31 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 8 lines · Lorenzo Pieralisi

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the Exynos CPUidle driver to dynamically initialize idle states data through the updated device tree source files.

Quoted 5 lines

Cc Kukjin Kim <[email protected]>

Cc Tomasz Figa <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
Compile tested, I am not sure I patched the right dts files, please check.

cpuidle-exynos driver is currently working properly in deeper cpuidle mode (AFTR) on Exynos4210 and Exynos5250 (please also see the following patch from Tomasz Figa: [1]). There is ongoing work to AFTR mode work also on Exynos4x12 and Exynos3250 but it is not complete yet. Exynos5410 OTOH should probably use the generic big little cpuidle driver (this SoC is similar to Exynos5420 one for which Chander Kashyap has developed cpuidle-big_little support [2]).

Making long story short, I think that your patch should depend on patch
[1] and update only exynos4210.dtsi and exynos5250.dtsi. Also for your patch #6 there needs to be some coordination with merging of Chander's patchset ([2]).

Quoted 2 lines

[1] http://www.spinics.net/lists/arm-kernel/msg341023.html
[2] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg664470.html

exynos4210.dtsi does not even have cpu nodes in it. Should I add them or this might trigger regression (ie cpu_logical_map()) ?

I will post a new version soon, should I just patch 5250 for now ?

I would need help to test this patch thanks.

Lorenzo

Quoted 21 lines

Quoted 8 lines

 .../devicetree/bindings/arm/exynos/idle-states.txt | 27 ++++++++++++++++++++
 arch/arm/boot/dts/exynos3250.dtsi                  | 16 ++++++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  | 15 +++++++++++
 arch/arm/boot/dts/exynos5410.dtsi                  | 17 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-exynos.c                   | 29 +++++++++++++---------
 6 files changed, 93 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/exynos/idle-states.txt

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

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

Hi Lorenzo,

Quoted 32 lines · Lorenzo Pieralisi

Quoted 31 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 8 lines · Lorenzo Pieralisi

With the introduction of DT based idle states, CPUidle drivers for ARM can now initialize idle states data through properties in the device tree.

This patch adds code to the Exynos CPUidle driver to dynamically initialize idle states data through the updated device tree source files.

Quoted 5 lines

Cc Kukjin Kim <[email protected]>

Cc Tomasz Figa <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
Compile tested, I am not sure I patched the right dts files, please check.

cpuidle-exynos driver is currently working properly in deeper cpuidle mode (AFTR) on Exynos4210 and Exynos5250 (please also see the following patch from Tomasz Figa: [1]). There is ongoing work to AFTR mode work also on Exynos4x12 and Exynos3250 but it is not complete yet. Exynos5410 OTOH should probably use the generic big little cpuidle driver (this SoC is similar to Exynos5420 one for which Chander Kashyap has developed cpuidle-big_little support [2]).

Making long story short, I think that your patch should depend on patch
[1] and update only exynos4210.dtsi and exynos5250.dtsi. Also for your patch #6 there needs to be some coordination with merging of Chander's patchset ([2]).

Quoted 6 lines

Quoted 2 lines

[1] http://www.spinics.net/lists/arm-kernel/msg341023.html
[2] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg664470.html

exynos4210.dtsi does not even have cpu nodes in it. Should I add them or this might trigger regression (ie cpu_logical_map()) ?

Yes that can cause regression.

Quoted 1 line

I will post a new version soon, should I just patch 5250 for now ?

Quoted 1 line

I would need help to test this patch thanks.

I can test the patch for 5250.

Quoted 11 lines

Lorenzo

Quoted 8 lines

Quoted 8 lines

 .../devicetree/bindings/arm/exynos/idle-states.txt | 27 ++++++++++++++++++++
 arch/arm/boot/dts/exynos3250.dtsi                  | 16 ++++++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  | 15 +++++++++++
 arch/arm/boot/dts/exynos5410.dtsi                  | 17 +++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  1 +
 drivers/cpuidle/cpuidle-exynos.c                   | 29 +++++++++++++---------
 6 files changed, 93 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/exynos/idle-states.txt

This patch updates the RTSM dts file with PSCI bindings and nodes describing the AEMv8 model idle states parameters.

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 arch/arm64/boot/dts/rtsm_ve-aemv8a.dts | 44 +++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 8 deletions(-)

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +0 -0

index d79de9c..4051ffb 100644

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +12 -0

+	};
+
 	cpus {
 		#address-cells = <2>;
 		#size-cells = <0>;
 
+		idle-states {
+			entry-method = "arm,psci";
+
+			CPU_SLEEP_0: cpu-sleep-0 {
+				compatible = "arm,idle-state";
+				entry-method-param = <0x0010000>;
+				entry-latency-us = <40>;
+				exit-latency-us = <100>;
+				min-residency-us = <150>;
+			};

Hi Lorenzo,

Quoted 16 lines · Lorenzo Pieralisi

This patch updates the RTSM dts file with PSCI bindings and nodes describing the AEMv8 model idle states parameters.

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
 arch/arm64/boot/dts/rtsm_ve-aemv8a.dts | 44 +++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 8 deletions(-)

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +0 -0

index d79de9c..4051ffb 100644
@@ -27,37 +27,65 @@
 		serial3 = &v2m_serial3;
 	};
 

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts line 31

Quoted 2 lines

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +2 -0

+	psci {
+		compatible = "arm,psci-0.2";

Where can I find a PSCI 0.2 implementation for the RTSM VE model? I couldn't find a link in the cover.

The upstream bootwrapper is not PSCI 0.2 compliant and it does not implement CPU_SUSPEND.

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts line 35

Quoted 4 lines

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +4 -0

+		method = "smc";
+		cpu_suspend = <0xc4000001>;
+		cpu_off = <0x84000002>;
+		cpu_on = <0xc4000003>;

Without "arm,psci" in the compatible list, these three properties are meaningless.

Quoted 21 lines

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +17 -0

+	};
+
 	cpus {
 		#address-cells = <2>;
 		#size-cells = <0>;
 
+		idle-states {
+			entry-method = "arm,psci";
+
+			CPU_SLEEP_0: cpu-sleep-0 {
+				compatible = "arm,idle-state";
+				entry-method-param = <0x0010000>;
+				entry-latency-us = <40>;
+				exit-latency-us = <100>;
+				min-residency-us = <150>;
+			};
+
+			CLUSTER_SLEEP_0: cluster-sleep-0 {
+				compatible = "arm,idle-state";
+				entry-method-param = <0x1010000>;
+				entry-latency-us = <500>;

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts line 61

Quoted 5 lines

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +5 -0

+				exit-latency-us = <1000>;
+				min-residency-us = <2500>;
+			};
+		};
+

If this is going to live in the /cpus node, could it please be placed after the cpu nodes?

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts line 66

Quoted 7 lines

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +1 -2

 		cpu at 0 {
 			device_type = "cpu";
 			compatible = "arm,armv8";
 			reg = <0x0 0x0>;
-			enable-method = "spin-table";
-			cpu-release-addr = <0x0 0x8000fff8>;
+			enable-method = "psci";

Changing the enable-method will break boot on a model when using a bootwrapper without PSCI support. Really we should leave it up to the bootwrapper to inject the enable method...

This patch updates the RTSM dts file with PSCI bindings and nodes describing the AEMv8 model idle states parameters.

Signed-off-by Lorenzo Pieralisi <[email protected]>

---
  arch/arm64/boot/dts/rtsm_ve-aemv8a.dts | 44 +++++++++++++++++++++++++++-------
  1 file changed, 36 insertions(+), 8 deletions(-)

arch/arm64/boot/dts/rtsm_ve-aemv8a.dts +0 -0

index d79de9c..4051ffb 100644