All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V5 0/6] PSCI: Support hierarchical PM domains
@ 2017-03-03 21:48 ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
  Cc: lorenzo.pieralisi, Juri.Lelli, linux-arm-msm, sboyd,
	brendan.jackman, sudeep.holla, andy.gross, Lina Iyer

Hi all,

This series adds support for hierarchical idle states in PSCI. Up until and
including PSCI v1.0, CPUs idle states could only be represented as flattened
model with CPU as part of the cpu-idle-states property in DT. For ex.,
	CPU0: cpu@0 {
		device_type = "cpu";
		cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
	};

This patchset adds support to split CPU and cluster idle states and
present them in hierarchy. The documentation in patch 5 explains this difference.
 
In order to be backwards compatible, the cpuidle driver and the PSCI driver has
been updated to look for hierarchy first and if not present will revert to
initializing using the flattened model.

This patch uses CPU PM domains published earlier.

Changes since last submission -
- Support both hierarchical model and flattened model
- Documentation updates.
- Rebase on top of Rafael's linux-next

Thanks,
Lina

Lina Iyer (6):
  drivers: cpuidle: Read CPU's idle state from PM domain
  drivers: firmware: psci: Allow OS Initiated suspend mode
  drivers: firmware: psci: Support cluster idle states for OS-Initiated
  drivers: firmwware: psci: Support hierachical idle states
  dt/bindings: Update binding for hierarchical PSCI states
  ARM64: dts: Define CPU power domain for MSM8916

 Documentation/devicetree/bindings/arm/psci.txt | 156 +++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/msm8916.dtsi          |  53 +++++++-
 drivers/cpuidle/dt_idle_states.c               |  38 +++++-
 drivers/firmware/psci.c                        | 167 +++++++++++++++++++++----
 include/uapi/linux/psci.h                      |   5 +
 5 files changed, 388 insertions(+), 31 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH V5 0/6] PSCI: Support hierarchical PM domains
@ 2017-03-03 21:48 ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hi all,

This series adds support for hierarchical idle states in PSCI. Up until and
including PSCI v1.0, CPUs idle states could only be represented as flattened
model with CPU as part of the cpu-idle-states property in DT. For ex.,
	CPU0: cpu at 0 {
		device_type = "cpu";
		cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
	};

This patchset adds support to split CPU and cluster idle states and
present them in hierarchy. The documentation in patch 5 explains this difference.
 
In order to be backwards compatible, the cpuidle driver and the PSCI driver has
been updated to look for hierarchy first and if not present will revert to
initializing using the flattened model.

This patch uses CPU PM domains published earlier.

Changes since last submission -
- Support both hierarchical model and flattened model
- Documentation updates.
- Rebase on top of Rafael's linux-next

Thanks,
Lina

Lina Iyer (6):
  drivers: cpuidle: Read CPU's idle state from PM domain
  drivers: firmware: psci: Allow OS Initiated suspend mode
  drivers: firmware: psci: Support cluster idle states for OS-Initiated
  drivers: firmwware: psci: Support hierachical idle states
  dt/bindings: Update binding for hierarchical PSCI states
  ARM64: dts: Define CPU power domain for MSM8916

 Documentation/devicetree/bindings/arm/psci.txt | 156 +++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/msm8916.dtsi          |  53 +++++++-
 drivers/cpuidle/dt_idle_states.c               |  38 +++++-
 drivers/firmware/psci.c                        | 167 +++++++++++++++++++++----
 include/uapi/linux/psci.h                      |   5 +
 5 files changed, 388 insertions(+), 31 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH V5 1/6] drivers: cpuidle: Read CPU's idle state from PM domain
  2017-03-03 21:48 ` Lina Iyer
@ 2017-03-03 21:48   ` Lina Iyer
  -1 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
  Cc: lorenzo.pieralisi, Juri.Lelli, linux-arm-msm, sboyd,
	brendan.jackman, sudeep.holla, andy.gross, Lina Iyer

Currently CPUs idle states and idle states of its parent are represented
in a flattened model by the cpu-dile-states property of the CPU node.
The CPUs idle states are followed by its cluster idle states. With the
introduction of CPU PM domains, the CPUs and domain idle states may be
represented hierarchically as part of the domain DT definition. This
would mean presenting idle state information in 2 places - CPU nodes for
the CPU and the cluster's with the PM domains.

Also, it makes sense to define domains around each individual CPU since
each of them is a power domain in its own right. The CPU idle states can
now be represented as its domain's idle state, defined by the
domain-idle-states property. This avoids presenting idle states in
multiple places in the DT.

Modify the DT-based cpuidle driver to check for the presence of a CPU's
domain and if present read the domain-idle-states of the PM domain and
if the CPU's domain is absent, revert to reading in the cpu-idle-states
property of the CPU DT node.

Suggested-by: Sudeep Holla <sudeep.holla@qrm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/cpuidle/dt_idle_states.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c
index ffca4fc..4df7d45 100644
--- a/drivers/cpuidle/dt_idle_states.c
+++ b/drivers/cpuidle/dt_idle_states.c
@@ -98,6 +98,39 @@ static int init_state_node(struct cpuidle_state *idle_state,
 }
 
 /*
+ * Get the state node at @idx. State node may be defined as domain's idle state
+ * if the CPU has its own domain or defined as CPU's idle state if it doesn't
+ * have a domain provider.
+ */
+static struct device_node *get_state_node(struct device_node *cpu_node,
+				unsigned int idx)
+{
+	struct device_node *dn;
+	bool cpu_has_domain = false;
+	struct of_phandle_args args;
+	const char *property;
+	int err;
+
+	err = of_parse_phandle_with_args(cpu_node, "power-domains",
+					"#power-domain-cells", 0, &args);
+	if (!err) {
+		dn = args.np;
+		err = of_count_phandle_with_args(dn, "domain-idle-states",
+								NULL);
+		cpu_has_domain = (err > 0);
+	}
+
+	if (cpu_has_domain) {
+		property = "domain-idle-states";
+	} else {
+		property = "cpu-idle-states";
+		dn = cpu_node;
+	}
+
+	return of_parse_phandle(dn, property, idx);
+}
+
+/*
  * Check that the idle state is uniform across all CPUs in the CPUidle driver
  * cpumask
  */
@@ -118,8 +151,7 @@ static bool idle_state_valid(struct device_node *state_node, unsigned int idx,
 	for (cpu = cpumask_next(cpumask_first(cpumask), cpumask);
 	     cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpumask)) {
 		cpu_node = of_cpu_device_node_get(cpu);
-		curr_state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
-						   idx);
+		curr_state_node = get_state_node(cpu_node, idx);
 		if (state_node != curr_state_node)
 			valid = false;
 
@@ -176,7 +208,7 @@ int dt_init_idle_driver(struct cpuidle_driver *drv,
 	cpu_node = of_cpu_device_node_get(cpumask_first(cpumask));
 
 	for (i = 0; ; i++) {
-		state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
+		state_node = get_state_node(cpu_node, i);
 		if (!state_node)
 			break;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 1/6] drivers: cpuidle: Read CPU's idle state from PM domain
@ 2017-03-03 21:48   ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

Currently CPUs idle states and idle states of its parent are represented
in a flattened model by the cpu-dile-states property of the CPU node.
The CPUs idle states are followed by its cluster idle states. With the
introduction of CPU PM domains, the CPUs and domain idle states may be
represented hierarchically as part of the domain DT definition. This
would mean presenting idle state information in 2 places - CPU nodes for
the CPU and the cluster's with the PM domains.

Also, it makes sense to define domains around each individual CPU since
each of them is a power domain in its own right. The CPU idle states can
now be represented as its domain's idle state, defined by the
domain-idle-states property. This avoids presenting idle states in
multiple places in the DT.

Modify the DT-based cpuidle driver to check for the presence of a CPU's
domain and if present read the domain-idle-states of the PM domain and
if the CPU's domain is absent, revert to reading in the cpu-idle-states
property of the CPU DT node.

Suggested-by: Sudeep Holla <sudeep.holla@qrm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/cpuidle/dt_idle_states.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c
index ffca4fc..4df7d45 100644
--- a/drivers/cpuidle/dt_idle_states.c
+++ b/drivers/cpuidle/dt_idle_states.c
@@ -98,6 +98,39 @@ static int init_state_node(struct cpuidle_state *idle_state,
 }
 
 /*
+ * Get the state node at @idx. State node may be defined as domain's idle state
+ * if the CPU has its own domain or defined as CPU's idle state if it doesn't
+ * have a domain provider.
+ */
+static struct device_node *get_state_node(struct device_node *cpu_node,
+				unsigned int idx)
+{
+	struct device_node *dn;
+	bool cpu_has_domain = false;
+	struct of_phandle_args args;
+	const char *property;
+	int err;
+
+	err = of_parse_phandle_with_args(cpu_node, "power-domains",
+					"#power-domain-cells", 0, &args);
+	if (!err) {
+		dn = args.np;
+		err = of_count_phandle_with_args(dn, "domain-idle-states",
+								NULL);
+		cpu_has_domain = (err > 0);
+	}
+
+	if (cpu_has_domain) {
+		property = "domain-idle-states";
+	} else {
+		property = "cpu-idle-states";
+		dn = cpu_node;
+	}
+
+	return of_parse_phandle(dn, property, idx);
+}
+
+/*
  * Check that the idle state is uniform across all CPUs in the CPUidle driver
  * cpumask
  */
@@ -118,8 +151,7 @@ static bool idle_state_valid(struct device_node *state_node, unsigned int idx,
 	for (cpu = cpumask_next(cpumask_first(cpumask), cpumask);
 	     cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpumask)) {
 		cpu_node = of_cpu_device_node_get(cpu);
-		curr_state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
-						   idx);
+		curr_state_node = get_state_node(cpu_node, idx);
 		if (state_node != curr_state_node)
 			valid = false;
 
@@ -176,7 +208,7 @@ int dt_init_idle_driver(struct cpuidle_driver *drv,
 	cpu_node = of_cpu_device_node_get(cpumask_first(cpumask));
 
 	for (i = 0; ; i++) {
-		state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
+		state_node = get_state_node(cpu_node, i);
 		if (!state_node)
 			break;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 2/6] drivers: firmware: psci: Allow OS Initiated suspend mode
  2017-03-03 21:48 ` Lina Iyer
@ 2017-03-03 21:48   ` Lina Iyer
  -1 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
  Cc: Mark Rutland, lorenzo.pieralisi, Juri.Lelli, linux-arm-msm,
	sboyd, brendan.jackman, sudeep.holla, andy.gross, Lina Iyer

PSCI firmware v1.0 onwards may support 2 different modes for
CPU_SUSPEND. Platform coordinated mode is the default and every firmware
should support it. OS Initiated mode is optional for the firmware to
implement and allow Linux to make an better decision on the state of
the CPU cluster heirarchy.

With the kernel capable of deciding the state for CPU cluster and
coherency domains, the OS Initiated mode may now be used by the kernel,
provided the firmware supports it. SET_SUSPEND_MODE is a PSCI function
available on v1.0 onwards and can be used to set the mode in the
firmware.

Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
[Ulf: Rebased on 4.7 rc1]
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/firmware/psci.c   | 42 +++++++++++++++++++++++++++++-------------
 include/uapi/linux/psci.h |  5 +++++
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 6c60a50..ec922b8 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -53,6 +53,7 @@
  * require cooperation with a Trusted OS driver.
  */
 static int resident_cpu = -1;
+static bool psci_has_osi;
 
 bool psci_tos_resident_on(int cpu)
 {
@@ -558,9 +559,8 @@ static int __init psci_0_2_init(struct device_node *np)
 	int err;
 
 	err = get_set_conduit_method(np);
-
 	if (err)
-		goto out_put_node;
+		return err;
 	/*
 	 * Starting with v0.2, the PSCI specification introduced a call
 	 * (PSCI_VERSION) that allows probing the firmware version, so
@@ -568,11 +568,7 @@ static int __init psci_0_2_init(struct device_node *np)
 	 * can be carried out according to the specific version reported
 	 * by firmware
 	 */
-	err = psci_probe();
-
-out_put_node:
-	of_node_put(np);
-	return err;
+	return psci_probe();
 }
 
 /*
@@ -584,9 +580,8 @@ static int __init psci_0_1_init(struct device_node *np)
 	int err;
 
 	err = get_set_conduit_method(np);
-
 	if (err)
-		goto out_put_node;
+		return err;
 
 	pr_info("Using PSCI v0.1 Function IDs from DT\n");
 
@@ -610,15 +605,31 @@ static int __init psci_0_1_init(struct device_node *np)
 		psci_ops.migrate = psci_migrate;
 	}
 
-out_put_node:
-	of_node_put(np);
 	return err;
 }
 
+static int __init psci_1_0_init(struct device_node *np)
+{
+	int ret;
+
+	ret = psci_0_2_init(np);
+	if (ret)
+		return ret;
+
+	/* Check if PSCI OSI mode is available */
+	ret = psci_features(psci_function_id[PSCI_FN_CPU_SUSPEND]);
+	if (ret & PSCI_1_0_OS_INITIATED) {
+		if (!psci_features(PSCI_1_0_FN_SET_SUSPEND_MODE))
+			psci_has_osi = true;
+	}
+
+	return 0;
+}
+
 static const struct of_device_id psci_of_match[] __initconst = {
 	{ .compatible = "arm,psci",	.data = psci_0_1_init},
 	{ .compatible = "arm,psci-0.2",	.data = psci_0_2_init},
-	{ .compatible = "arm,psci-1.0",	.data = psci_0_2_init},
+	{ .compatible = "arm,psci-1.0",	.data = psci_1_0_init},
 	{},
 };
 
@@ -627,6 +638,7 @@ int __init psci_dt_init(void)
 	struct device_node *np;
 	const struct of_device_id *matched_np;
 	psci_initcall_t init_fn;
+	int ret;
 
 	np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
 
@@ -634,7 +646,11 @@ int __init psci_dt_init(void)
 		return -ENODEV;
 
 	init_fn = (psci_initcall_t)matched_np->data;
-	return init_fn(np);
+	ret = init_fn(np);
+
+	of_node_put(np);
+
+	return ret;
 }
 
 #ifdef CONFIG_ACPI
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 3d7a0fc..7dd778e 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -48,6 +48,7 @@
 
 #define PSCI_1_0_FN_PSCI_FEATURES		PSCI_0_2_FN(10)
 #define PSCI_1_0_FN_SYSTEM_SUSPEND		PSCI_0_2_FN(14)
+#define PSCI_1_0_FN_SET_SUSPEND_MODE		PSCI_0_2_FN(15)
 
 #define PSCI_1_0_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)
 
@@ -93,6 +94,10 @@
 #define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK	\
 			(0x1 << PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT)
 
+#define PSCI_1_0_OS_INITIATED			BIT(0)
+#define PSCI_1_0_SUSPEND_MODE_PC		0
+#define PSCI_1_0_SUSPEND_MODE_OSI		1
+
 /* PSCI return values (inclusive of all PSCI versions) */
 #define PSCI_RET_SUCCESS			0
 #define PSCI_RET_NOT_SUPPORTED			-1
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 2/6] drivers: firmware: psci: Allow OS Initiated suspend mode
@ 2017-03-03 21:48   ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

PSCI firmware v1.0 onwards may support 2 different modes for
CPU_SUSPEND. Platform coordinated mode is the default and every firmware
should support it. OS Initiated mode is optional for the firmware to
implement and allow Linux to make an better decision on the state of
the CPU cluster heirarchy.

With the kernel capable of deciding the state for CPU cluster and
coherency domains, the OS Initiated mode may now be used by the kernel,
provided the firmware supports it. SET_SUSPEND_MODE is a PSCI function
available on v1.0 onwards and can be used to set the mode in the
firmware.

Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
[Ulf: Rebased on 4.7 rc1]
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/firmware/psci.c   | 42 +++++++++++++++++++++++++++++-------------
 include/uapi/linux/psci.h |  5 +++++
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 6c60a50..ec922b8 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -53,6 +53,7 @@
  * require cooperation with a Trusted OS driver.
  */
 static int resident_cpu = -1;
+static bool psci_has_osi;
 
 bool psci_tos_resident_on(int cpu)
 {
@@ -558,9 +559,8 @@ static int __init psci_0_2_init(struct device_node *np)
 	int err;
 
 	err = get_set_conduit_method(np);
-
 	if (err)
-		goto out_put_node;
+		return err;
 	/*
 	 * Starting with v0.2, the PSCI specification introduced a call
 	 * (PSCI_VERSION) that allows probing the firmware version, so
@@ -568,11 +568,7 @@ static int __init psci_0_2_init(struct device_node *np)
 	 * can be carried out according to the specific version reported
 	 * by firmware
 	 */
-	err = psci_probe();
-
-out_put_node:
-	of_node_put(np);
-	return err;
+	return psci_probe();
 }
 
 /*
@@ -584,9 +580,8 @@ static int __init psci_0_1_init(struct device_node *np)
 	int err;
 
 	err = get_set_conduit_method(np);
-
 	if (err)
-		goto out_put_node;
+		return err;
 
 	pr_info("Using PSCI v0.1 Function IDs from DT\n");
 
@@ -610,15 +605,31 @@ static int __init psci_0_1_init(struct device_node *np)
 		psci_ops.migrate = psci_migrate;
 	}
 
-out_put_node:
-	of_node_put(np);
 	return err;
 }
 
+static int __init psci_1_0_init(struct device_node *np)
+{
+	int ret;
+
+	ret = psci_0_2_init(np);
+	if (ret)
+		return ret;
+
+	/* Check if PSCI OSI mode is available */
+	ret = psci_features(psci_function_id[PSCI_FN_CPU_SUSPEND]);
+	if (ret & PSCI_1_0_OS_INITIATED) {
+		if (!psci_features(PSCI_1_0_FN_SET_SUSPEND_MODE))
+			psci_has_osi = true;
+	}
+
+	return 0;
+}
+
 static const struct of_device_id psci_of_match[] __initconst = {
 	{ .compatible = "arm,psci",	.data = psci_0_1_init},
 	{ .compatible = "arm,psci-0.2",	.data = psci_0_2_init},
-	{ .compatible = "arm,psci-1.0",	.data = psci_0_2_init},
+	{ .compatible = "arm,psci-1.0",	.data = psci_1_0_init},
 	{},
 };
 
@@ -627,6 +638,7 @@ int __init psci_dt_init(void)
 	struct device_node *np;
 	const struct of_device_id *matched_np;
 	psci_initcall_t init_fn;
+	int ret;
 
 	np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
 
@@ -634,7 +646,11 @@ int __init psci_dt_init(void)
 		return -ENODEV;
 
 	init_fn = (psci_initcall_t)matched_np->data;
-	return init_fn(np);
+	ret = init_fn(np);
+
+	of_node_put(np);
+
+	return ret;
 }
 
 #ifdef CONFIG_ACPI
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 3d7a0fc..7dd778e 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -48,6 +48,7 @@
 
 #define PSCI_1_0_FN_PSCI_FEATURES		PSCI_0_2_FN(10)
 #define PSCI_1_0_FN_SYSTEM_SUSPEND		PSCI_0_2_FN(14)
+#define PSCI_1_0_FN_SET_SUSPEND_MODE		PSCI_0_2_FN(15)
 
 #define PSCI_1_0_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)
 
@@ -93,6 +94,10 @@
 #define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK	\
 			(0x1 << PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT)
 
+#define PSCI_1_0_OS_INITIATED			BIT(0)
+#define PSCI_1_0_SUSPEND_MODE_PC		0
+#define PSCI_1_0_SUSPEND_MODE_OSI		1
+
 /* PSCI return values (inclusive of all PSCI versions) */
 #define PSCI_RET_SUCCESS			0
 #define PSCI_RET_NOT_SUPPORTED			-1
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 3/6] drivers: firmware: psci: Support cluster idle states for OS-Initiated
  2017-03-03 21:48 ` Lina Iyer
@ 2017-03-03 21:48   ` Lina Iyer
  -1 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
  Cc: Mark Rutland, lorenzo.pieralisi, Juri.Lelli, linux-arm-msm,
	sboyd, brendan.jackman, sudeep.holla, andy.gross, Lina Iyer

PSCI OS initiated firmware may allow Linux to determine the state of the
CPU cluster and the cluster at coherency level to enter idle states when
there are no active CPUs. Since Linux has a better idea of the QoS and
the wakeup pattern of the CPUs, the cluster idle states may be better
determined by the OS instead of the firmware.

The last CPU entering idle in a cluster, is responsible for selecting
the state of the cluster. Only one CPU in a cluster may provide the
cluster idle state to the firmware. Similarly, the last CPU in the
system may provide the state of the coherency domain along with the
cluster and the CPU state IDs.

Utilize the CPU PM domain framework's helper functions to build up the
hierarchy of cluster topology using Generic PM domains. We provide
callbacks for domain power_on and power_off. By appending the state IDs
at each domain level in the -power_off() callbacks, we build up a
composite state ID that can be passed onto the firmware to idle the CPU,
the cluster and the coherency interface.

Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/firmware/psci.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 90 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index ec922b8..18ae62d 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/arm-smccc.h>
 #include <linux/cpuidle.h>
+#include <linux/cpu_domains.h>
 #include <linux/errno.h>
 #include <linux/linkage.h>
 #include <linux/of.h>
@@ -54,6 +55,18 @@
  */
 static int resident_cpu = -1;
 static bool psci_has_osi;
+static bool psci_has_osi_pd;
+static DEFINE_PER_CPU(u32, cluster_state_id);
+
+static inline u32 psci_get_composite_state_id(u32 cpu_state)
+{
+	return cpu_state | this_cpu_read(cluster_state_id);
+}
+
+static inline void psci_reset_composite_state_id(void)
+{
+	this_cpu_write(cluster_state_id, 0);
+}
 
 bool psci_tos_resident_on(int cpu)
 {
@@ -180,6 +193,8 @@ static int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
 
 	fn = psci_function_id[PSCI_FN_CPU_ON];
 	err = invoke_psci_fn(fn, cpuid, entry_point, 0);
+	/* Reset CPU cluster states */
+	psci_reset_composite_state_id();
 	return psci_to_linux_errno(err);
 }
 
@@ -251,6 +266,27 @@ static int __init psci_features(u32 psci_func_id)
 
 #ifdef CONFIG_CPU_IDLE
 static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
+static bool psci_suspend_mode_is_osi;
+
+static int psci_set_suspend_mode_osi(bool enable)
+{
+	int ret;
+	int mode;
+
+	if (enable && !psci_has_osi)
+		return -ENODEV;
+
+	if (enable == psci_suspend_mode_is_osi)
+		return 0;
+
+	mode = enable ? PSCI_1_0_SUSPEND_MODE_OSI : PSCI_1_0_SUSPEND_MODE_PC;
+	ret = invoke_psci_fn(PSCI_1_0_FN_SET_SUSPEND_MODE,
+			     mode, 0, 0);
+	if (!ret)
+		psci_suspend_mode_is_osi = enable;
+
+	return psci_to_linux_errno(ret);
+}
 
 static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
 {
@@ -353,6 +389,39 @@ static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
 }
 #endif
 
+static int psci_pd_populate_state_data(struct device_node *np, u32 *param)
+{
+	return of_property_read_u32(np, "arm,psci-suspend-param", param);
+}
+
+static int psci_pd_power_off(u32 idx, u32 param, const struct cpumask *mask)
+{
+	__this_cpu_add(cluster_state_id, param);
+	return 0;
+}
+
+const struct cpu_pd_ops psci_pd_ops = {
+	.populate_state_data = psci_pd_populate_state_data,
+	.power_off = psci_pd_power_off,
+};
+
+static int psci_cpu_osi_pd_init(int cpu)
+{
+	int ret;
+
+	if (!psci_has_osi_pd)
+		return 0;
+
+	ret = of_setup_cpu_pd_single(cpu, &psci_pd_ops);
+	if (!ret) {
+		ret = psci_set_suspend_mode_osi(true);
+		if (ret)
+			pr_warn("CPU%d: Error setting PSCI OSI mode\n", cpu);
+	}
+
+	return ret;
+}
+
 int psci_cpu_init_idle(unsigned int cpu)
 {
 	struct device_node *cpu_node;
@@ -368,6 +437,10 @@ int psci_cpu_init_idle(unsigned int cpu)
 	if (!acpi_disabled)
 		return psci_acpi_cpu_init_idle(cpu);
 
+	ret = psci_cpu_osi_pd_init(cpu);
+	if (ret)
+		return ret;
+
 	cpu_node = of_get_cpu_node(cpu, NULL);
 	if (!cpu_node)
 		return -ENODEV;
@@ -382,15 +455,17 @@ int psci_cpu_init_idle(unsigned int cpu)
 static int psci_suspend_finisher(unsigned long index)
 {
 	u32 *state = __this_cpu_read(psci_power_state);
+	u32 ext_state = psci_get_composite_state_id(state[index - 1]);
 
-	return psci_ops.cpu_suspend(state[index - 1],
-				    virt_to_phys(cpu_resume));
+	return psci_ops.cpu_suspend(ext_state, virt_to_phys(cpu_resume));
 }
 
 int psci_cpu_suspend_enter(unsigned long index)
 {
 	int ret;
 	u32 *state = __this_cpu_read(psci_power_state);
+	u32 ext_state = psci_get_composite_state_id(state[index - 1]);
+
 	/*
 	 * idle state index 0 corresponds to wfi, should never be called
 	 * from the cpu_suspend operations
@@ -399,10 +474,16 @@ int psci_cpu_suspend_enter(unsigned long index)
 		return -EINVAL;
 
 	if (!psci_power_state_loses_context(state[index - 1]))
-		ret = psci_ops.cpu_suspend(state[index - 1], 0);
+		ret = psci_ops.cpu_suspend(ext_state, 0);
 	else
 		ret = cpu_suspend(index, psci_suspend_finisher);
 
+	/*
+	 * Clear the CPU's cluster states, we start afresh after coming
+	 * out of idle.
+	 */
+	psci_reset_composite_state_id();
+
 	return ret;
 }
 
@@ -610,6 +691,7 @@ static int __init psci_0_1_init(struct device_node *np)
 
 static int __init psci_1_0_init(struct device_node *np)
 {
+	struct device_node *dn;
 	int ret;
 
 	ret = psci_0_2_init(np);
@@ -621,6 +703,11 @@ static int __init psci_1_0_init(struct device_node *np)
 	if (ret & PSCI_1_0_OS_INITIATED) {
 		if (!psci_features(PSCI_1_0_FN_SET_SUSPEND_MODE))
 			psci_has_osi = true;
+		/* Check if we power domains defined in the PSCI node */
+		dn = of_find_node_with_property(np, "#power-domain-cells");
+		if (dn)
+			psci_has_osi_pd = true;
+		of_node_put(dn);
 	}
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 3/6] drivers: firmware: psci: Support cluster idle states for OS-Initiated
@ 2017-03-03 21:48   ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

PSCI OS initiated firmware may allow Linux to determine the state of the
CPU cluster and the cluster at coherency level to enter idle states when
there are no active CPUs. Since Linux has a better idea of the QoS and
the wakeup pattern of the CPUs, the cluster idle states may be better
determined by the OS instead of the firmware.

The last CPU entering idle in a cluster, is responsible for selecting
the state of the cluster. Only one CPU in a cluster may provide the
cluster idle state to the firmware. Similarly, the last CPU in the
system may provide the state of the coherency domain along with the
cluster and the CPU state IDs.

Utilize the CPU PM domain framework's helper functions to build up the
hierarchy of cluster topology using Generic PM domains. We provide
callbacks for domain power_on and power_off. By appending the state IDs
at each domain level in the -power_off() callbacks, we build up a
composite state ID that can be passed onto the firmware to idle the CPU,
the cluster and the coherency interface.

Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/firmware/psci.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 90 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index ec922b8..18ae62d 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/arm-smccc.h>
 #include <linux/cpuidle.h>
+#include <linux/cpu_domains.h>
 #include <linux/errno.h>
 #include <linux/linkage.h>
 #include <linux/of.h>
@@ -54,6 +55,18 @@
  */
 static int resident_cpu = -1;
 static bool psci_has_osi;
+static bool psci_has_osi_pd;
+static DEFINE_PER_CPU(u32, cluster_state_id);
+
+static inline u32 psci_get_composite_state_id(u32 cpu_state)
+{
+	return cpu_state | this_cpu_read(cluster_state_id);
+}
+
+static inline void psci_reset_composite_state_id(void)
+{
+	this_cpu_write(cluster_state_id, 0);
+}
 
 bool psci_tos_resident_on(int cpu)
 {
@@ -180,6 +193,8 @@ static int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
 
 	fn = psci_function_id[PSCI_FN_CPU_ON];
 	err = invoke_psci_fn(fn, cpuid, entry_point, 0);
+	/* Reset CPU cluster states */
+	psci_reset_composite_state_id();
 	return psci_to_linux_errno(err);
 }
 
@@ -251,6 +266,27 @@ static int __init psci_features(u32 psci_func_id)
 
 #ifdef CONFIG_CPU_IDLE
 static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
+static bool psci_suspend_mode_is_osi;
+
+static int psci_set_suspend_mode_osi(bool enable)
+{
+	int ret;
+	int mode;
+
+	if (enable && !psci_has_osi)
+		return -ENODEV;
+
+	if (enable == psci_suspend_mode_is_osi)
+		return 0;
+
+	mode = enable ? PSCI_1_0_SUSPEND_MODE_OSI : PSCI_1_0_SUSPEND_MODE_PC;
+	ret = invoke_psci_fn(PSCI_1_0_FN_SET_SUSPEND_MODE,
+			     mode, 0, 0);
+	if (!ret)
+		psci_suspend_mode_is_osi = enable;
+
+	return psci_to_linux_errno(ret);
+}
 
 static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
 {
@@ -353,6 +389,39 @@ static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
 }
 #endif
 
+static int psci_pd_populate_state_data(struct device_node *np, u32 *param)
+{
+	return of_property_read_u32(np, "arm,psci-suspend-param", param);
+}
+
+static int psci_pd_power_off(u32 idx, u32 param, const struct cpumask *mask)
+{
+	__this_cpu_add(cluster_state_id, param);
+	return 0;
+}
+
+const struct cpu_pd_ops psci_pd_ops = {
+	.populate_state_data = psci_pd_populate_state_data,
+	.power_off = psci_pd_power_off,
+};
+
+static int psci_cpu_osi_pd_init(int cpu)
+{
+	int ret;
+
+	if (!psci_has_osi_pd)
+		return 0;
+
+	ret = of_setup_cpu_pd_single(cpu, &psci_pd_ops);
+	if (!ret) {
+		ret = psci_set_suspend_mode_osi(true);
+		if (ret)
+			pr_warn("CPU%d: Error setting PSCI OSI mode\n", cpu);
+	}
+
+	return ret;
+}
+
 int psci_cpu_init_idle(unsigned int cpu)
 {
 	struct device_node *cpu_node;
@@ -368,6 +437,10 @@ int psci_cpu_init_idle(unsigned int cpu)
 	if (!acpi_disabled)
 		return psci_acpi_cpu_init_idle(cpu);
 
+	ret = psci_cpu_osi_pd_init(cpu);
+	if (ret)
+		return ret;
+
 	cpu_node = of_get_cpu_node(cpu, NULL);
 	if (!cpu_node)
 		return -ENODEV;
@@ -382,15 +455,17 @@ int psci_cpu_init_idle(unsigned int cpu)
 static int psci_suspend_finisher(unsigned long index)
 {
 	u32 *state = __this_cpu_read(psci_power_state);
+	u32 ext_state = psci_get_composite_state_id(state[index - 1]);
 
-	return psci_ops.cpu_suspend(state[index - 1],
-				    virt_to_phys(cpu_resume));
+	return psci_ops.cpu_suspend(ext_state, virt_to_phys(cpu_resume));
 }
 
 int psci_cpu_suspend_enter(unsigned long index)
 {
 	int ret;
 	u32 *state = __this_cpu_read(psci_power_state);
+	u32 ext_state = psci_get_composite_state_id(state[index - 1]);
+
 	/*
 	 * idle state index 0 corresponds to wfi, should never be called
 	 * from the cpu_suspend operations
@@ -399,10 +474,16 @@ int psci_cpu_suspend_enter(unsigned long index)
 		return -EINVAL;
 
 	if (!psci_power_state_loses_context(state[index - 1]))
-		ret = psci_ops.cpu_suspend(state[index - 1], 0);
+		ret = psci_ops.cpu_suspend(ext_state, 0);
 	else
 		ret = cpu_suspend(index, psci_suspend_finisher);
 
+	/*
+	 * Clear the CPU's cluster states, we start afresh after coming
+	 * out of idle.
+	 */
+	psci_reset_composite_state_id();
+
 	return ret;
 }
 
@@ -610,6 +691,7 @@ static int __init psci_0_1_init(struct device_node *np)
 
 static int __init psci_1_0_init(struct device_node *np)
 {
+	struct device_node *dn;
 	int ret;
 
 	ret = psci_0_2_init(np);
@@ -621,6 +703,11 @@ static int __init psci_1_0_init(struct device_node *np)
 	if (ret & PSCI_1_0_OS_INITIATED) {
 		if (!psci_features(PSCI_1_0_FN_SET_SUSPEND_MODE))
 			psci_has_osi = true;
+		/* Check if we power domains defined in the PSCI node */
+		dn = of_find_node_with_property(np, "#power-domain-cells");
+		if (dn)
+			psci_has_osi_pd = true;
+		of_node_put(dn);
 	}
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 4/6] drivers: firmwware: psci: Support hierachical idle states
  2017-03-03 21:48 ` Lina Iyer
@ 2017-03-03 21:48   ` Lina Iyer
  -1 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
  Cc: lorenzo.pieralisi, Juri.Lelli, linux-arm-msm, sboyd,
	brendan.jackman, sudeep.holla, andy.gross, Lina Iyer

Read in the idle state properties for a CPU's idle state from its PM
domain, if such a domain exists or use the existing CPU node property.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/firmware/psci.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 18ae62d..190d3a7 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -292,15 +292,24 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
 {
 	int i, ret, count = 0;
 	u32 *psci_states;
-	struct device_node *state_node;
-
-	/* Count idle states */
-	while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
-					      count))) {
-		count++;
-		of_node_put(state_node);
+	struct device_node *state_node, *dn;
+	struct of_phandle_args args;
+	bool cpu_has_domain = false;
+	const char *property;
+
+	/* Is there a domain provider for this CPU? */
+	ret = of_parse_phandle_with_args(cpu_node, "power-domains",
+					"#power-domain-cells", 0, &args);
+	if (!ret) {
+		dn = args.np;
+		ret = of_count_phandle_with_args(dn, "domain-idle-states",
+								NULL);
+		cpu_has_domain = (ret > 0);
 	}
 
+	count = (cpu_has_domain) ? ret :
+		of_count_phandle_with_args(cpu_node, "cpu-idle-states", NULL);
+
 	if (!count)
 		return -ENODEV;
 
@@ -308,10 +317,17 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
 	if (!psci_states)
 		return -ENOMEM;
 
+	if (cpu_has_domain) {
+		property = "domain-idle-states";
+	} else {
+		property = "cpu-idle-states";
+		dn = cpu_node;
+	}
+
 	for (i = 0; i < count; i++) {
 		u32 state;
 
-		state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
+		state_node = of_parse_phandle(dn, property, i);
 
 		ret = of_property_read_u32(state_node,
 					   "arm,psci-suspend-param",
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 4/6] drivers: firmwware: psci: Support hierachical idle states
@ 2017-03-03 21:48   ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

Read in the idle state properties for a CPU's idle state from its PM
domain, if such a domain exists or use the existing CPU node property.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/firmware/psci.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 18ae62d..190d3a7 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -292,15 +292,24 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
 {
 	int i, ret, count = 0;
 	u32 *psci_states;
-	struct device_node *state_node;
-
-	/* Count idle states */
-	while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
-					      count))) {
-		count++;
-		of_node_put(state_node);
+	struct device_node *state_node, *dn;
+	struct of_phandle_args args;
+	bool cpu_has_domain = false;
+	const char *property;
+
+	/* Is there a domain provider for this CPU? */
+	ret = of_parse_phandle_with_args(cpu_node, "power-domains",
+					"#power-domain-cells", 0, &args);
+	if (!ret) {
+		dn = args.np;
+		ret = of_count_phandle_with_args(dn, "domain-idle-states",
+								NULL);
+		cpu_has_domain = (ret > 0);
 	}
 
+	count = (cpu_has_domain) ? ret :
+		of_count_phandle_with_args(cpu_node, "cpu-idle-states", NULL);
+
 	if (!count)
 		return -ENODEV;
 
@@ -308,10 +317,17 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
 	if (!psci_states)
 		return -ENOMEM;
 
+	if (cpu_has_domain) {
+		property = "domain-idle-states";
+	} else {
+		property = "cpu-idle-states";
+		dn = cpu_node;
+	}
+
 	for (i = 0; i < count; i++) {
 		u32 state;
 
-		state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
+		state_node = of_parse_phandle(dn, property, i);
 
 		ret = of_property_read_u32(state_node,
 					   "arm,psci-suspend-param",
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 5/6] dt/bindings: Update binding for hierarchical PSCI states
  2017-03-03 21:48 ` Lina Iyer
@ 2017-03-03 21:48   ` Lina Iyer
  -1 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
  Cc: Mark Rutland, devicetree, lorenzo.pieralisi, Juri.Lelli,
	linux-arm-msm, sboyd, brendan.jackman, sudeep.holla, andy.gross,
	Lina Iyer

Update device bindings to represent hierarchical CPU and CPU domain idle
states. Also update the examples to clearly show how flattened and
hierarchical states can be represented in DT.

Cc: <devicetree@vger.kernel.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/devicetree/bindings/arm/psci.txt | 156 +++++++++++++++++++++++++
 1 file changed, 156 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt
index a2c4f1d..e0e9281 100644
--- a/Documentation/devicetree/bindings/arm/psci.txt
+++ b/Documentation/devicetree/bindings/arm/psci.txt
@@ -105,7 +105,163 @@ Case 3: PSCI v0.2 and PSCI v0.1.
 		...
 	};
 
+PSCI v1.0 onwards, supports OS-Initiated mode for powering off CPU domains
+from the firmware. Such PM domains for which the PSCI firmware driver acts as
+pseudo-controller, may also be specified in the DT under the psci node. The
+domain definitions must follow the domain idle state specifications per [3].
+The domain states themselves must be compatible with 'domain-idle-state'
+defined in [1] and need to specify the arm,psci-suspend-param property for
+each idle state.
+
+DT allows representing CPU and CPU cluster idle states in two different ways -
+
+The flattened model as given in Example 1, lists CPU's idle states followed by
+the domain idle state that the CPUs may choose. This is the general practice
+followed in PSCI firmwares that support Platform Coordinated mode. Note that
+the idle states are all compatible with "arm,idle-state".
+
+Example 2 represents the hierarchical model of CPU and domain idle states.
+CPUs define their domain provider in their DT node. The domain controls the
+power to the CPU and possibly other h/w blocks that would be powered off when
+the CPU is powered off. The CPU's idle states may therefore be considered as
+the domain's idle states and have the compatible "arm,idle-state". Such
+domains may be embedded within another domain that represents common h/w
+blocks between these CPUs viz., the cluster. The idle states of the cluster
+would be represented as the domain's idle states. In order to use OS-Initiated
+mode of PSCI in the firmware, the hierarchial representation must be used.
+
+More information on defining CPU PM domains is available in [4].
+
+Example 1: Flattened representation of CPU and domain idle states
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		CPU0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x0>;
+			enable-method = "psci";
+			cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>,
+					  <&CLUSTER_PWR_DWN>;
+		};
+
+		CPU1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a57", "arm,armv8";
+			reg = <0x100>;
+			enable-method = "psci";
+			cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>,
+					  <&CLUSTER_PWR_DWN>;
+		};
+
+		idle-states {
+			CPU_PWRDN: cpu_power_down{
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x000001>;
+				entry-latency-us = <10>;
+				exit-latency-us = <10>;
+				min-residency-us = <100>;
+			};
+
+			CLUSTER_RET: domain_ret {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x1000010>;
+				entry-latency-us = <500>;
+				exit-latency-us = <500>;
+				min-residency-us = <2000>;
+			};
+
+			CLUSTER_PWR_DWN: domain_off {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x1000030>;
+				entry-latency-us = <2000>;
+				exit-latency-us = <2000>;
+				min-residency-us = <6000>;
+			};
+	};
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+Example 2: Hierarchical representation of CPU and domain idle states
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		CPU0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x0>;
+			enable-method = "psci";
+			power-domains = <&CPU_PD0>;
+		};
+
+		CPU1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a57", "arm,armv8";
+			reg = <0x100>;
+			enable-method = "psci";
+			power-domains = <&CPU_PD1>;
+		};
+
+		idle-states {
+			CPU_PWRDN: cpu_power_down{
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x000001>;
+				entry-latency-us = <10>;
+				exit-latency-us = <10>;
+				min-residency-us = <100>;
+			};
+
+			CLUSTER_RET: domain_ret {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x1000010>;
+				entry-latency-us = <500>;
+				exit-latency-us = <500>;
+				min-residency-us = <2000>;
+			};
+
+			CLUSTER_PWR_DWN: domain_off {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x1000030>;
+				entry-latency-us = <2000>;
+				exit-latency-us = <2000>;
+				min-residency-us = <6000>;
+			};
+		};
+	};
+
+	psci {
+		compatible = "arm,psci-1.0";
+		method = "smc";
+
+		CPU_PD0: cpu-pd@0 {
+			#power-domain-cells = <0>;
+			domain-idle-states = <&CPU_PWRDN>;
+			power-domains = <&CLUSTER_PD>;
+		};
+
+		CPU_PD1: cpu-pd@1 {
+			#power-domain-cells = <0>;
+			domain-idle-states =  <&CPU_PWRDN>;
+			power-domains = <&CLUSTER_PD>;
+		};
+
+		CLUSTER_PD: cluster-pd@0 {
+			#power-domain-cells = <0>;
+			domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
+		};
+	};
+
 [1] Kernel documentation - ARM idle states bindings
     Documentation/devicetree/bindings/arm/idle-states.txt
 [2] Power State Coordination Interface (PSCI) specification
     http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
+[3]. PM Domains description
+    Documentation/devicetree/bindings/power/power_domain.txt
+[4]. CPU PM Domains description
+    Documentation/power/cpu_domains.txt
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 5/6] dt/bindings: Update binding for hierarchical PSCI states
@ 2017-03-03 21:48   ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

Update device bindings to represent hierarchical CPU and CPU domain idle
states. Also update the examples to clearly show how flattened and
hierarchical states can be represented in DT.

Cc: <devicetree@vger.kernel.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/devicetree/bindings/arm/psci.txt | 156 +++++++++++++++++++++++++
 1 file changed, 156 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt
index a2c4f1d..e0e9281 100644
--- a/Documentation/devicetree/bindings/arm/psci.txt
+++ b/Documentation/devicetree/bindings/arm/psci.txt
@@ -105,7 +105,163 @@ Case 3: PSCI v0.2 and PSCI v0.1.
 		...
 	};
 
+PSCI v1.0 onwards, supports OS-Initiated mode for powering off CPU domains
+from the firmware. Such PM domains for which the PSCI firmware driver acts as
+pseudo-controller, may also be specified in the DT under the psci node. The
+domain definitions must follow the domain idle state specifications per [3].
+The domain states themselves must be compatible with 'domain-idle-state'
+defined in [1] and need to specify the arm,psci-suspend-param property for
+each idle state.
+
+DT allows representing CPU and CPU cluster idle states in two different ways -
+
+The flattened model as given in Example 1, lists CPU's idle states followed by
+the domain idle state that the CPUs may choose. This is the general practice
+followed in PSCI firmwares that support Platform Coordinated mode. Note that
+the idle states are all compatible with "arm,idle-state".
+
+Example 2 represents the hierarchical model of CPU and domain idle states.
+CPUs define their domain provider in their DT node. The domain controls the
+power to the CPU and possibly other h/w blocks that would be powered off when
+the CPU is powered off. The CPU's idle states may therefore be considered as
+the domain's idle states and have the compatible "arm,idle-state". Such
+domains may be embedded within another domain that represents common h/w
+blocks between these CPUs viz., the cluster. The idle states of the cluster
+would be represented as the domain's idle states. In order to use OS-Initiated
+mode of PSCI in the firmware, the hierarchial representation must be used.
+
+More information on defining CPU PM domains is available in [4].
+
+Example 1: Flattened representation of CPU and domain idle states
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		CPU0: cpu at 0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x0>;
+			enable-method = "psci";
+			cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>,
+					  <&CLUSTER_PWR_DWN>;
+		};
+
+		CPU1: cpu at 1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a57", "arm,armv8";
+			reg = <0x100>;
+			enable-method = "psci";
+			cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>,
+					  <&CLUSTER_PWR_DWN>;
+		};
+
+		idle-states {
+			CPU_PWRDN: cpu_power_down{
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x000001>;
+				entry-latency-us = <10>;
+				exit-latency-us = <10>;
+				min-residency-us = <100>;
+			};
+
+			CLUSTER_RET: domain_ret {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x1000010>;
+				entry-latency-us = <500>;
+				exit-latency-us = <500>;
+				min-residency-us = <2000>;
+			};
+
+			CLUSTER_PWR_DWN: domain_off {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x1000030>;
+				entry-latency-us = <2000>;
+				exit-latency-us = <2000>;
+				min-residency-us = <6000>;
+			};
+	};
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+Example 2: Hierarchical representation of CPU and domain idle states
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		CPU0: cpu at 0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x0>;
+			enable-method = "psci";
+			power-domains = <&CPU_PD0>;
+		};
+
+		CPU1: cpu at 1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a57", "arm,armv8";
+			reg = <0x100>;
+			enable-method = "psci";
+			power-domains = <&CPU_PD1>;
+		};
+
+		idle-states {
+			CPU_PWRDN: cpu_power_down{
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x000001>;
+				entry-latency-us = <10>;
+				exit-latency-us = <10>;
+				min-residency-us = <100>;
+			};
+
+			CLUSTER_RET: domain_ret {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x1000010>;
+				entry-latency-us = <500>;
+				exit-latency-us = <500>;
+				min-residency-us = <2000>;
+			};
+
+			CLUSTER_PWR_DWN: domain_off {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x1000030>;
+				entry-latency-us = <2000>;
+				exit-latency-us = <2000>;
+				min-residency-us = <6000>;
+			};
+		};
+	};
+
+	psci {
+		compatible = "arm,psci-1.0";
+		method = "smc";
+
+		CPU_PD0: cpu-pd at 0 {
+			#power-domain-cells = <0>;
+			domain-idle-states = <&CPU_PWRDN>;
+			power-domains = <&CLUSTER_PD>;
+		};
+
+		CPU_PD1: cpu-pd at 1 {
+			#power-domain-cells = <0>;
+			domain-idle-states =  <&CPU_PWRDN>;
+			power-domains = <&CLUSTER_PD>;
+		};
+
+		CLUSTER_PD: cluster-pd at 0 {
+			#power-domain-cells = <0>;
+			domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
+		};
+	};
+
 [1] Kernel documentation - ARM idle states bindings
     Documentation/devicetree/bindings/arm/idle-states.txt
 [2] Power State Coordination Interface (PSCI) specification
     http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
+[3]. PM Domains description
+    Documentation/devicetree/bindings/power/power_domain.txt
+[4]. CPU PM Domains description
+    Documentation/power/cpu_domains.txt
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 6/6] ARM64: dts: Define CPU power domain for MSM8916
  2017-03-03 21:48 ` Lina Iyer
@ 2017-03-03 21:48   ` Lina Iyer
  -1 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
  Cc: devicetree, lorenzo.pieralisi, Juri.Lelli, linux-arm-msm, sboyd,
	brendan.jackman, sudeep.holla, andy.gross, Lina Iyer

Define power domain and the power states for the domain as defined by
the PSCI firmware. The 8916 firmware supports OS initiated method of
powering off the CPU clusters.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 53 ++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index f8ff327..ba60276 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -103,7 +103,7 @@
 			reg = <0x0>;
 			next-level-cache = <&L2_0>;
 			enable-method = "psci";
-			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&CPU_PD0>;
 		};
 
 		CPU1: cpu@1 {
@@ -112,7 +112,7 @@
 			reg = <0x1>;
 			next-level-cache = <&L2_0>;
 			enable-method = "psci";
-			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&CPU_PD1>;
 		};
 
 		CPU2: cpu@2 {
@@ -121,7 +121,7 @@
 			reg = <0x2>;
 			next-level-cache = <&L2_0>;
 			enable-method = "psci";
-			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&CPU_PD2>;
 		};
 
 		CPU3: cpu@3 {
@@ -130,7 +130,7 @@
 			reg = <0x3>;
 			next-level-cache = <&L2_0>;
 			enable-method = "psci";
-			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&CPU_PD3>;
 		};
 
 		L2_0: l2-cache {
@@ -147,12 +147,57 @@
 				min-residency-us = <2000>;
 				local-timer-stop;
 			};
+
+			CLUSTER_RET: cluster_retention {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x1000010>;
+				entry-latency-us = <500>;
+				exit-latency-us = <500>;
+				min-residency-us = <2000>;
+			};
+
+			CLUSTER_PWR_DWN: cluster_gdhs {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x1000030>;
+				entry-latency-us = <2000>;
+				exit-latency-us = <2000>;
+				min-residency-us = <6000>;
+			};
 		};
 	};
 
 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
+
+		CPU_PD0: cpu-pd@0 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&CPU_SPC>;
+		};
+
+		CPU_PD1: cpu-pd@1 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&CPU_SPC>;
+		};
+
+		CPU_PD2: cpu-pd@2 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&CPU_SPC>;
+		};
+
+		CPU_PD3: cpu-pd@3 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&CPU_SPC>;
+		};
+
+		CLUSTER_PD: cluster_pd {
+			#power-domain-cells = <0>;
+			domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
+		};
 	};
 
 	pmu {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V5 6/6] ARM64: dts: Define CPU power domain for MSM8916
@ 2017-03-03 21:48   ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2017-03-03 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

Define power domain and the power states for the domain as defined by
the PSCI firmware. The 8916 firmware supports OS initiated method of
powering off the CPU clusters.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 53 ++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index f8ff327..ba60276 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -103,7 +103,7 @@
 			reg = <0x0>;
 			next-level-cache = <&L2_0>;
 			enable-method = "psci";
-			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&CPU_PD0>;
 		};
 
 		CPU1: cpu at 1 {
@@ -112,7 +112,7 @@
 			reg = <0x1>;
 			next-level-cache = <&L2_0>;
 			enable-method = "psci";
-			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&CPU_PD1>;
 		};
 
 		CPU2: cpu at 2 {
@@ -121,7 +121,7 @@
 			reg = <0x2>;
 			next-level-cache = <&L2_0>;
 			enable-method = "psci";
-			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&CPU_PD2>;
 		};
 
 		CPU3: cpu at 3 {
@@ -130,7 +130,7 @@
 			reg = <0x3>;
 			next-level-cache = <&L2_0>;
 			enable-method = "psci";
-			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&CPU_PD3>;
 		};
 
 		L2_0: l2-cache {
@@ -147,12 +147,57 @@
 				min-residency-us = <2000>;
 				local-timer-stop;
 			};
+
+			CLUSTER_RET: cluster_retention {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x1000010>;
+				entry-latency-us = <500>;
+				exit-latency-us = <500>;
+				min-residency-us = <2000>;
+			};
+
+			CLUSTER_PWR_DWN: cluster_gdhs {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x1000030>;
+				entry-latency-us = <2000>;
+				exit-latency-us = <2000>;
+				min-residency-us = <6000>;
+			};
 		};
 	};
 
 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
+
+		CPU_PD0: cpu-pd at 0 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&CPU_SPC>;
+		};
+
+		CPU_PD1: cpu-pd at 1 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&CPU_SPC>;
+		};
+
+		CPU_PD2: cpu-pd at 2 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&CPU_SPC>;
+		};
+
+		CPU_PD3: cpu-pd at 3 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&CPU_SPC>;
+		};
+
+		CLUSTER_PD: cluster_pd {
+			#power-domain-cells = <0>;
+			domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
+		};
 	};
 
 	pmu {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH V5 1/6] drivers: cpuidle: Read CPU's idle state from PM domain
  2017-03-03 21:48   ` Lina Iyer
@ 2017-03-13 13:58     ` Brendan Jackman
  -1 siblings, 0 replies; 16+ messages in thread
From: Brendan Jackman @ 2017-03-13 13:58 UTC (permalink / raw)
  To: Lina Iyer
  Cc: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel,
	andy.gross, sboyd, linux-arm-msm, lorenzo.pieralisi,
	sudeep.holla, Juri.Lelli


Hi Lina,

On Fri, Mar 03 2017 at 21:48, Lina Iyer wrote:
> Currently CPUs idle states and idle states of its parent are represented
> in a flattened model by the cpu-dile-states property of the CPU node.
> The CPUs idle states are followed by its cluster idle states. With the
> introduction of CPU PM domains, the CPUs and domain idle states may be
> represented hierarchically as part of the domain DT definition. This
> would mean presenting idle state information in 2 places - CPU nodes for
> the CPU and the cluster's with the PM domains.
>
> Also, it makes sense to define domains around each individual CPU since
> each of them is a power domain in its own right. The CPU idle states can
> now be represented as its domain's idle state, defined by the
> domain-idle-states property. This avoids presenting idle states in
> multiple places in the DT.
>
> Modify the DT-based cpuidle driver to check for the presence of a CPU's
> domain and if present read the domain-idle-states of the PM domain and
> if the CPU's domain is absent, revert to reading in the cpu-idle-states
> property of the CPU DT node.
>
> Suggested-by: Sudeep Holla <sudeep.holla@qrm.com>
s/qrm/arm/

> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  drivers/cpuidle/dt_idle_states.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c
> index ffca4fc..4df7d45 100644
> --- a/drivers/cpuidle/dt_idle_states.c
> +++ b/drivers/cpuidle/dt_idle_states.c
> @@ -98,6 +98,39 @@ static int init_state_node(struct cpuidle_state *idle_state,
>  }
>
>  /*
> + * Get the state node at @idx. State node may be defined as domain's idle state
> + * if the CPU has its own domain or defined as CPU's idle state if it doesn't
> + * have a domain provider.
> + */
> +static struct device_node *get_state_node(struct device_node *cpu_node,
> +				unsigned int idx)
> +{
> +	struct device_node *dn;
> +	bool cpu_has_domain = false;
> +	struct of_phandle_args args;
> +	const char *property;
> +	int err;
> +
> +	err = of_parse_phandle_with_args(cpu_node, "power-domains",
> +					"#power-domain-cells", 0, &args);
Should probably have an of_node_put to match this.
> +	if (!err) {
> +		dn = args.np;
> +		err = of_count_phandle_with_args(dn, "domain-idle-states",
> +								NULL);
> +		cpu_has_domain = (err > 0);

So if a CPU has a power domain but that domain doesn't have any idle
states, then we fall back to cpu-idle-states?

I think the presence of a power domain for a CPU should mean
cpu-idle-states is totally ignored, and this should be made clear in the
power_domain.txt binding doc.

(I have had this conversation with someone before, I think it was
 internal at ARM but if I'm wrong and we've already discussed it then
 I'm sorry!)
> +	}
> +
> +	if (cpu_has_domain) {
> +		property = "domain-idle-states";
> +	} else {
> +		property = "cpu-idle-states";
> +		dn = cpu_node;
> +	}
> +
> +	return of_parse_phandle(dn, property, idx);
> +}
> +
> +/*
>   * Check that the idle state is uniform across all CPUs in the CPUidle driver
>   * cpumask
>   */
> @@ -118,8 +151,7 @@ static bool idle_state_valid(struct device_node *state_node, unsigned int idx,
>  	for (cpu = cpumask_next(cpumask_first(cpumask), cpumask);
>  	     cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpumask)) {
>  		cpu_node = of_cpu_device_node_get(cpu);
> -		curr_state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
> -						   idx);
> +		curr_state_node = get_state_node(cpu_node, idx);
>  		if (state_node != curr_state_node)
>  			valid = false;
>
> @@ -176,7 +208,7 @@ int dt_init_idle_driver(struct cpuidle_driver *drv,
>  	cpu_node = of_cpu_device_node_get(cpumask_first(cpumask));
>
>  	for (i = 0; ; i++) {
> -		state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
> +		state_node = get_state_node(cpu_node, i);
>  		if (!state_node)
>  			break;

This patch only supports using domain-idle-states for the CPU-level idle
states, i.e. it only looks at one level of the power-domains graph
rather than walking it and "linearising"/"flattening" the discovered
states into a cpuidle-friendly list.

That's not a reason against merging this patch but we should note the
limitation and maybe even print a warning in if we find a parent for the
CPU's power domain but we're using cpuidle rather than runtime PM.

Cheers,
Brendan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH V5 1/6] drivers: cpuidle: Read CPU's idle state from PM domain
@ 2017-03-13 13:58     ` Brendan Jackman
  0 siblings, 0 replies; 16+ messages in thread
From: Brendan Jackman @ 2017-03-13 13:58 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Lina,

On Fri, Mar 03 2017 at 21:48, Lina Iyer wrote:
> Currently CPUs idle states and idle states of its parent are represented
> in a flattened model by the cpu-dile-states property of the CPU node.
> The CPUs idle states are followed by its cluster idle states. With the
> introduction of CPU PM domains, the CPUs and domain idle states may be
> represented hierarchically as part of the domain DT definition. This
> would mean presenting idle state information in 2 places - CPU nodes for
> the CPU and the cluster's with the PM domains.
>
> Also, it makes sense to define domains around each individual CPU since
> each of them is a power domain in its own right. The CPU idle states can
> now be represented as its domain's idle state, defined by the
> domain-idle-states property. This avoids presenting idle states in
> multiple places in the DT.
>
> Modify the DT-based cpuidle driver to check for the presence of a CPU's
> domain and if present read the domain-idle-states of the PM domain and
> if the CPU's domain is absent, revert to reading in the cpu-idle-states
> property of the CPU DT node.
>
> Suggested-by: Sudeep Holla <sudeep.holla@qrm.com>
s/qrm/arm/

> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  drivers/cpuidle/dt_idle_states.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c
> index ffca4fc..4df7d45 100644
> --- a/drivers/cpuidle/dt_idle_states.c
> +++ b/drivers/cpuidle/dt_idle_states.c
> @@ -98,6 +98,39 @@ static int init_state_node(struct cpuidle_state *idle_state,
>  }
>
>  /*
> + * Get the state node at @idx. State node may be defined as domain's idle state
> + * if the CPU has its own domain or defined as CPU's idle state if it doesn't
> + * have a domain provider.
> + */
> +static struct device_node *get_state_node(struct device_node *cpu_node,
> +				unsigned int idx)
> +{
> +	struct device_node *dn;
> +	bool cpu_has_domain = false;
> +	struct of_phandle_args args;
> +	const char *property;
> +	int err;
> +
> +	err = of_parse_phandle_with_args(cpu_node, "power-domains",
> +					"#power-domain-cells", 0, &args);
Should probably have an of_node_put to match this.
> +	if (!err) {
> +		dn = args.np;
> +		err = of_count_phandle_with_args(dn, "domain-idle-states",
> +								NULL);
> +		cpu_has_domain = (err > 0);

So if a CPU has a power domain but that domain doesn't have any idle
states, then we fall back to cpu-idle-states?

I think the presence of a power domain for a CPU should mean
cpu-idle-states is totally ignored, and this should be made clear in the
power_domain.txt binding doc.

(I have had this conversation with someone before, I think it was
 internal at ARM but if I'm wrong and we've already discussed it then
 I'm sorry!)
> +	}
> +
> +	if (cpu_has_domain) {
> +		property = "domain-idle-states";
> +	} else {
> +		property = "cpu-idle-states";
> +		dn = cpu_node;
> +	}
> +
> +	return of_parse_phandle(dn, property, idx);
> +}
> +
> +/*
>   * Check that the idle state is uniform across all CPUs in the CPUidle driver
>   * cpumask
>   */
> @@ -118,8 +151,7 @@ static bool idle_state_valid(struct device_node *state_node, unsigned int idx,
>  	for (cpu = cpumask_next(cpumask_first(cpumask), cpumask);
>  	     cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpumask)) {
>  		cpu_node = of_cpu_device_node_get(cpu);
> -		curr_state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
> -						   idx);
> +		curr_state_node = get_state_node(cpu_node, idx);
>  		if (state_node != curr_state_node)
>  			valid = false;
>
> @@ -176,7 +208,7 @@ int dt_init_idle_driver(struct cpuidle_driver *drv,
>  	cpu_node = of_cpu_device_node_get(cpumask_first(cpumask));
>
>  	for (i = 0; ; i++) {
> -		state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
> +		state_node = get_state_node(cpu_node, i);
>  		if (!state_node)
>  			break;

This patch only supports using domain-idle-states for the CPU-level idle
states, i.e. it only looks at one level of the power-domains graph
rather than walking it and "linearising"/"flattening" the discovered
states into a cpuidle-friendly list.

That's not a reason against merging this patch but we should note the
limitation and maybe even print a warning in if we find a parent for the
CPU's power domain but we're using cpuidle rather than runtime PM.

Cheers,
Brendan

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-03-13 13:59 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-03 21:48 [PATCH V5 0/6] PSCI: Support hierarchical PM domains Lina Iyer
2017-03-03 21:48 ` Lina Iyer
2017-03-03 21:48 ` [PATCH V5 1/6] drivers: cpuidle: Read CPU's idle state from PM domain Lina Iyer
2017-03-03 21:48   ` Lina Iyer
2017-03-13 13:58   ` Brendan Jackman
2017-03-13 13:58     ` Brendan Jackman
2017-03-03 21:48 ` [PATCH V5 2/6] drivers: firmware: psci: Allow OS Initiated suspend mode Lina Iyer
2017-03-03 21:48   ` Lina Iyer
2017-03-03 21:48 ` [PATCH V5 3/6] drivers: firmware: psci: Support cluster idle states for OS-Initiated Lina Iyer
2017-03-03 21:48   ` Lina Iyer
2017-03-03 21:48 ` [PATCH V5 4/6] drivers: firmwware: psci: Support hierachical idle states Lina Iyer
2017-03-03 21:48   ` Lina Iyer
2017-03-03 21:48 ` [PATCH V5 5/6] dt/bindings: Update binding for hierarchical PSCI states Lina Iyer
2017-03-03 21:48   ` Lina Iyer
2017-03-03 21:48 ` [PATCH V5 6/6] ARM64: dts: Define CPU power domain for MSM8916 Lina Iyer
2017-03-03 21:48   ` Lina Iyer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.