* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-12 9:12 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-12 9:12 UTC (permalink / raw)
To: Wei Xu, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
linux-arm-kernel, devicetree, linux-kernel
Cc: Leo Yan, Vincent Guittot, Daniel Lezcano, Sudeep Holla, Soby Mathew
Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
idle state. From ftrace log we can observe CA73 CPUs can be easily
waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
anything and sleep again; so there have tons of trace events for CA73
CPUs entering and exiting idle state.
On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
set its psci parameter as '0x0000001' and from this parameter it can
calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
takes 1 as a invalid value for state id, so the CPU cannot enter idle
state and directly bail out to kernel.
We want to create good practice for psci parameters platform definition,
so review the psci specification. The spec "ARM Power State Coordination
Interface - Platform Design Document (ARM DEN 0022D)" recommends state
ID in chapter "6.5 Recommended StateID Encoding". The recommended power
state IDs can be presented by below listed values; and it divides into
three fields, every field can use 4 bits to present power states
corresponding to core level, cluster level and system level:
0: Run
1: Standby
2: Retention
3: Powerdown
This commit changes psci parameter to compliance with the suggested
state ID in the doc. Except we change 'CPU_NAP' state psci parameter
to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
state parameters to '0x0010003' and '0x1010033' respectively.
Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Soby Mathew <Soby.Mathew@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index ab0b95b..99d5a46 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -147,7 +147,7 @@
CPU_NAP: cpu-nap {
compatible = "arm,idle-state";
- arm,psci-suspend-param = <0x0000001>;
+ arm,psci-suspend-param = <0x0000002>;
entry-latency-us = <7>;
exit-latency-us = <2>;
min-residency-us = <15>;
@@ -156,7 +156,7 @@
CPU_SLEEP: cpu-sleep {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x0010000>;
+ arm,psci-suspend-param = <0x0010003>;
entry-latency-us = <40>;
exit-latency-us = <70>;
min-residency-us = <3000>;
@@ -165,7 +165,7 @@
CLUSTER_SLEEP_0: cluster-sleep-0 {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x1010000>;
+ arm,psci-suspend-param = <0x1010033>;
entry-latency-us = <500>;
exit-latency-us = <5000>;
min-residency-us = <20000>;
@@ -174,7 +174,7 @@
CLUSTER_SLEEP_1: cluster-sleep-1 {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x1010000>;
+ arm,psci-suspend-param = <0x1010033>;
entry-latency-us = <1000>;
exit-latency-us = <5000>;
min-residency-us = <20000>;
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-12 9:12 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-12 9:12 UTC (permalink / raw)
To: Wei Xu, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Leo Yan, Vincent Guittot, Daniel Lezcano, Sudeep Holla, Soby Mathew
Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
idle state. From ftrace log we can observe CA73 CPUs can be easily
waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
anything and sleep again; so there have tons of trace events for CA73
CPUs entering and exiting idle state.
On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
set its psci parameter as '0x0000001' and from this parameter it can
calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
takes 1 as a invalid value for state id, so the CPU cannot enter idle
state and directly bail out to kernel.
We want to create good practice for psci parameters platform definition,
so review the psci specification. The spec "ARM Power State Coordination
Interface - Platform Design Document (ARM DEN 0022D)" recommends state
ID in chapter "6.5 Recommended StateID Encoding". The recommended power
state IDs can be presented by below listed values; and it divides into
three fields, every field can use 4 bits to present power states
corresponding to core level, cluster level and system level:
0: Run
1: Standby
2: Retention
3: Powerdown
This commit changes psci parameter to compliance with the suggested
state ID in the doc. Except we change 'CPU_NAP' state psci parameter
to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
state parameters to '0x0010003' and '0x1010033' respectively.
Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
Cc: Vincent Guittot <vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
Cc: Soby Mathew <Soby.Mathew-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index ab0b95b..99d5a46 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -147,7 +147,7 @@
CPU_NAP: cpu-nap {
compatible = "arm,idle-state";
- arm,psci-suspend-param = <0x0000001>;
+ arm,psci-suspend-param = <0x0000002>;
entry-latency-us = <7>;
exit-latency-us = <2>;
min-residency-us = <15>;
@@ -156,7 +156,7 @@
CPU_SLEEP: cpu-sleep {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x0010000>;
+ arm,psci-suspend-param = <0x0010003>;
entry-latency-us = <40>;
exit-latency-us = <70>;
min-residency-us = <3000>;
@@ -165,7 +165,7 @@
CLUSTER_SLEEP_0: cluster-sleep-0 {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x1010000>;
+ arm,psci-suspend-param = <0x1010033>;
entry-latency-us = <500>;
exit-latency-us = <5000>;
min-residency-us = <20000>;
@@ -174,7 +174,7 @@
CLUSTER_SLEEP_1: cluster-sleep-1 {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x1010000>;
+ arm,psci-suspend-param = <0x1010033>;
entry-latency-us = <1000>;
exit-latency-us = <5000>;
min-residency-us = <20000>;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-12 9:12 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-12 9:12 UTC (permalink / raw)
To: linux-arm-kernel
Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
idle state. From ftrace log we can observe CA73 CPUs can be easily
waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
anything and sleep again; so there have tons of trace events for CA73
CPUs entering and exiting idle state.
On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
set its psci parameter as '0x0000001' and from this parameter it can
calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
takes 1 as a invalid value for state id, so the CPU cannot enter idle
state and directly bail out to kernel.
We want to create good practice for psci parameters platform definition,
so review the psci specification. The spec "ARM Power State Coordination
Interface - Platform Design Document (ARM DEN 0022D)" recommends state
ID in chapter "6.5 Recommended StateID Encoding". The recommended power
state IDs can be presented by below listed values; and it divides into
three fields, every field can use 4 bits to present power states
corresponding to core level, cluster level and system level:
0: Run
1: Standby
2: Retention
3: Powerdown
This commit changes psci parameter to compliance with the suggested
state ID in the doc. Except we change 'CPU_NAP' state psci parameter
to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
state parameters to '0x0010003' and '0x1010033' respectively.
Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Soby Mathew <Soby.Mathew@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index ab0b95b..99d5a46 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -147,7 +147,7 @@
CPU_NAP: cpu-nap {
compatible = "arm,idle-state";
- arm,psci-suspend-param = <0x0000001>;
+ arm,psci-suspend-param = <0x0000002>;
entry-latency-us = <7>;
exit-latency-us = <2>;
min-residency-us = <15>;
@@ -156,7 +156,7 @@
CPU_SLEEP: cpu-sleep {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x0010000>;
+ arm,psci-suspend-param = <0x0010003>;
entry-latency-us = <40>;
exit-latency-us = <70>;
min-residency-us = <3000>;
@@ -165,7 +165,7 @@
CLUSTER_SLEEP_0: cluster-sleep-0 {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x1010000>;
+ arm,psci-suspend-param = <0x1010033>;
entry-latency-us = <500>;
exit-latency-us = <5000>;
min-residency-us = <20000>;
@@ -174,7 +174,7 @@
CLUSTER_SLEEP_1: cluster-sleep-1 {
compatible = "arm,idle-state";
local-timer-stop;
- arm,psci-suspend-param = <0x1010000>;
+ arm,psci-suspend-param = <0x1010033>;
entry-latency-us = <1000>;
exit-latency-us = <5000>;
min-residency-us = <20000>;
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-22 9:37 ` Wei Xu
0 siblings, 0 replies; 20+ messages in thread
From: Wei Xu @ 2017-12-22 9:37 UTC (permalink / raw)
To: Leo Yan, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
linux-arm-kernel, devicetree, linux-kernel
Cc: Vincent Guittot, Daniel Lezcano, Sudeep Holla, Soby Mathew
Hi Leo,
On 2017/12/12 9:12, Leo Yan wrote:
> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> idle state. From ftrace log we can observe CA73 CPUs can be easily
> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> anything and sleep again; so there have tons of trace events for CA73
> CPUs entering and exiting idle state.
>
> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> set its psci parameter as '0x0000001' and from this parameter it can
> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> takes 1 as a invalid value for state id, so the CPU cannot enter idle
> state and directly bail out to kernel.
>
> We want to create good practice for psci parameters platform definition,
> so review the psci specification. The spec "ARM Power State Coordination
> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> state IDs can be presented by below listed values; and it divides into
> three fields, every field can use 4 bits to present power states
> corresponding to core level, cluster level and system level:
> 0: Run
> 1: Standby
> 2: Retention
> 3: Powerdown
>
> This commit changes psci parameter to compliance with the suggested
> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> state parameters to '0x0010003' and '0x1010033' respectively.
>
> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Soby Mathew <Soby.Mathew@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
Applied into hisilicon dt tree.
Thanks!
Best Regards,
Wei
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index ab0b95b..99d5a46 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -147,7 +147,7 @@
>
> CPU_NAP: cpu-nap {
> compatible = "arm,idle-state";
> - arm,psci-suspend-param = <0x0000001>;
> + arm,psci-suspend-param = <0x0000002>;
> entry-latency-us = <7>;
> exit-latency-us = <2>;
> min-residency-us = <15>;
> @@ -156,7 +156,7 @@
> CPU_SLEEP: cpu-sleep {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x0010000>;
> + arm,psci-suspend-param = <0x0010003>;
> entry-latency-us = <40>;
> exit-latency-us = <70>;
> min-residency-us = <3000>;
> @@ -165,7 +165,7 @@
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <500>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> @@ -174,7 +174,7 @@
> CLUSTER_SLEEP_1: cluster-sleep-1 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <1000>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-22 9:37 ` Wei Xu
0 siblings, 0 replies; 20+ messages in thread
From: Wei Xu @ 2017-12-22 9:37 UTC (permalink / raw)
To: Leo Yan, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Vincent Guittot, Daniel Lezcano, Sudeep Holla, Soby Mathew
Hi Leo,
On 2017/12/12 9:12, Leo Yan wrote:
> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> idle state. From ftrace log we can observe CA73 CPUs can be easily
> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> anything and sleep again; so there have tons of trace events for CA73
> CPUs entering and exiting idle state.
>
> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> set its psci parameter as '0x0000001' and from this parameter it can
> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> takes 1 as a invalid value for state id, so the CPU cannot enter idle
> state and directly bail out to kernel.
>
> We want to create good practice for psci parameters platform definition,
> so review the psci specification. The spec "ARM Power State Coordination
> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> state IDs can be presented by below listed values; and it divides into
> three fields, every field can use 4 bits to present power states
> corresponding to core level, cluster level and system level:
> 0: Run
> 1: Standby
> 2: Retention
> 3: Powerdown
>
> This commit changes psci parameter to compliance with the suggested
> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> state parameters to '0x0010003' and '0x1010033' respectively.
>
> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>
> Cc: Vincent Guittot <vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Soby Mathew <Soby.Mathew-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
Applied into hisilicon dt tree.
Thanks!
Best Regards,
Wei
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index ab0b95b..99d5a46 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -147,7 +147,7 @@
>
> CPU_NAP: cpu-nap {
> compatible = "arm,idle-state";
> - arm,psci-suspend-param = <0x0000001>;
> + arm,psci-suspend-param = <0x0000002>;
> entry-latency-us = <7>;
> exit-latency-us = <2>;
> min-residency-us = <15>;
> @@ -156,7 +156,7 @@
> CPU_SLEEP: cpu-sleep {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x0010000>;
> + arm,psci-suspend-param = <0x0010003>;
> entry-latency-us = <40>;
> exit-latency-us = <70>;
> min-residency-us = <3000>;
> @@ -165,7 +165,7 @@
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <500>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> @@ -174,7 +174,7 @@
> CLUSTER_SLEEP_1: cluster-sleep-1 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <1000>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-22 9:37 ` Wei Xu
0 siblings, 0 replies; 20+ messages in thread
From: Wei Xu @ 2017-12-22 9:37 UTC (permalink / raw)
To: linux-arm-kernel
Hi Leo,
On 2017/12/12 9:12, Leo Yan wrote:
> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> idle state. From ftrace log we can observe CA73 CPUs can be easily
> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> anything and sleep again; so there have tons of trace events for CA73
> CPUs entering and exiting idle state.
>
> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> set its psci parameter as '0x0000001' and from this parameter it can
> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> takes 1 as a invalid value for state id, so the CPU cannot enter idle
> state and directly bail out to kernel.
>
> We want to create good practice for psci parameters platform definition,
> so review the psci specification. The spec "ARM Power State Coordination
> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> state IDs can be presented by below listed values; and it divides into
> three fields, every field can use 4 bits to present power states
> corresponding to core level, cluster level and system level:
> 0: Run
> 1: Standby
> 2: Retention
> 3: Powerdown
>
> This commit changes psci parameter to compliance with the suggested
> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> state parameters to '0x0010003' and '0x1010033' respectively.
>
> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Soby Mathew <Soby.Mathew@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
Applied into hisilicon dt tree.
Thanks!
Best Regards,
Wei
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index ab0b95b..99d5a46 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -147,7 +147,7 @@
>
> CPU_NAP: cpu-nap {
> compatible = "arm,idle-state";
> - arm,psci-suspend-param = <0x0000001>;
> + arm,psci-suspend-param = <0x0000002>;
> entry-latency-us = <7>;
> exit-latency-us = <2>;
> min-residency-us = <15>;
> @@ -156,7 +156,7 @@
> CPU_SLEEP: cpu-sleep {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x0010000>;
> + arm,psci-suspend-param = <0x0010003>;
> entry-latency-us = <40>;
> exit-latency-us = <70>;
> min-residency-us = <3000>;
> @@ -165,7 +165,7 @@
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <500>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> @@ -174,7 +174,7 @@
> CLUSTER_SLEEP_1: cluster-sleep-1 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <1000>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-22 14:22 ` Vincent Guittot
0 siblings, 0 replies; 20+ messages in thread
From: Vincent Guittot @ 2017-12-22 14:22 UTC (permalink / raw)
To: Leo Yan
Cc: Wei Xu, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
LAK, devicetree, linux-kernel, Daniel Lezcano, Sudeep Holla,
Soby Mathew
Hi Leo,
Sorry for jumping late in the discussion but should we also remove
the NAP state from the property cpu-idle-states of the CPUs because
this state not supported by the platform at least for now and may be
not in a near future ?
Then, I have another question regarding the update of the
psci-suspend-parameter. These changes implies an update of the psci
firmawre which means that we will now have 2 different firmware
version compatible with 2 different dt.
Is there any way to check that the ATF on the board is the one that
compatible with the parameter with something like a version ? I
currently use the previous firmware which works fine with current
kernel and dt binding once the NAP state is removed from the table.
When moving on recent kernel, I will have to take care of updating the
firmware and if i need to go back on a previous kernel, i will have to
make sure that i have the right ATF version. This make a lot of chance
of having the wrong configuration
Regards,
Vincent
On 12 December 2017 at 10:12, Leo Yan <leo.yan@linaro.org> wrote:
> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> idle state. From ftrace log we can observe CA73 CPUs can be easily
> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> anything and sleep again; so there have tons of trace events for CA73
> CPUs entering and exiting idle state.
>
> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> set its psci parameter as '0x0000001' and from this parameter it can
> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> takes 1 as a invalid value for state id, so the CPU cannot enter idle
> state and directly bail out to kernel.
>
> We want to create good practice for psci parameters platform definition,
> so review the psci specification. The spec "ARM Power State Coordination
> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> state IDs can be presented by below listed values; and it divides into
> three fields, every field can use 4 bits to present power states
> corresponding to core level, cluster level and system level:
> 0: Run
> 1: Standby
> 2: Retention
> 3: Powerdown
>
> This commit changes psci parameter to compliance with the suggested
> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> state parameters to '0x0010003' and '0x1010033' respectively.
>
> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Soby Mathew <Soby.Mathew@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index ab0b95b..99d5a46 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -147,7 +147,7 @@
>
> CPU_NAP: cpu-nap {
> compatible = "arm,idle-state";
> - arm,psci-suspend-param = <0x0000001>;
> + arm,psci-suspend-param = <0x0000002>;
> entry-latency-us = <7>;
> exit-latency-us = <2>;
> min-residency-us = <15>;
> @@ -156,7 +156,7 @@
> CPU_SLEEP: cpu-sleep {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x0010000>;
> + arm,psci-suspend-param = <0x0010003>;
> entry-latency-us = <40>;
> exit-latency-us = <70>;
> min-residency-us = <3000>;
> @@ -165,7 +165,7 @@
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <500>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> @@ -174,7 +174,7 @@
> CLUSTER_SLEEP_1: cluster-sleep-1 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <1000>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-22 14:22 ` Vincent Guittot
0 siblings, 0 replies; 20+ messages in thread
From: Vincent Guittot @ 2017-12-22 14:22 UTC (permalink / raw)
To: Leo Yan
Cc: Wei Xu, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel,
Daniel Lezcano, Sudeep Holla, Soby Mathew
Hi Leo,
Sorry for jumping late in the discussion but should we also remove
the NAP state from the property cpu-idle-states of the CPUs because
this state not supported by the platform at least for now and may be
not in a near future ?
Then, I have another question regarding the update of the
psci-suspend-parameter. These changes implies an update of the psci
firmawre which means that we will now have 2 different firmware
version compatible with 2 different dt.
Is there any way to check that the ATF on the board is the one that
compatible with the parameter with something like a version ? I
currently use the previous firmware which works fine with current
kernel and dt binding once the NAP state is removed from the table.
When moving on recent kernel, I will have to take care of updating the
firmware and if i need to go back on a previous kernel, i will have to
make sure that i have the right ATF version. This make a lot of chance
of having the wrong configuration
Regards,
Vincent
On 12 December 2017 at 10:12, Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> idle state. From ftrace log we can observe CA73 CPUs can be easily
> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> anything and sleep again; so there have tons of trace events for CA73
> CPUs entering and exiting idle state.
>
> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> set its psci parameter as '0x0000001' and from this parameter it can
> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> takes 1 as a invalid value for state id, so the CPU cannot enter idle
> state and directly bail out to kernel.
>
> We want to create good practice for psci parameters platform definition,
> so review the psci specification. The spec "ARM Power State Coordination
> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> state IDs can be presented by below listed values; and it divides into
> three fields, every field can use 4 bits to present power states
> corresponding to core level, cluster level and system level:
> 0: Run
> 1: Standby
> 2: Retention
> 3: Powerdown
>
> This commit changes psci parameter to compliance with the suggested
> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> state parameters to '0x0010003' and '0x1010033' respectively.
>
> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>
> Cc: Vincent Guittot <vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Soby Mathew <Soby.Mathew-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index ab0b95b..99d5a46 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -147,7 +147,7 @@
>
> CPU_NAP: cpu-nap {
> compatible = "arm,idle-state";
> - arm,psci-suspend-param = <0x0000001>;
> + arm,psci-suspend-param = <0x0000002>;
> entry-latency-us = <7>;
> exit-latency-us = <2>;
> min-residency-us = <15>;
> @@ -156,7 +156,7 @@
> CPU_SLEEP: cpu-sleep {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x0010000>;
> + arm,psci-suspend-param = <0x0010003>;
> entry-latency-us = <40>;
> exit-latency-us = <70>;
> min-residency-us = <3000>;
> @@ -165,7 +165,7 @@
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <500>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> @@ -174,7 +174,7 @@
> CLUSTER_SLEEP_1: cluster-sleep-1 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <1000>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-22 14:22 ` Vincent Guittot
0 siblings, 0 replies; 20+ messages in thread
From: Vincent Guittot @ 2017-12-22 14:22 UTC (permalink / raw)
To: linux-arm-kernel
Hi Leo,
Sorry for jumping late in the discussion but should we also remove
the NAP state from the property cpu-idle-states of the CPUs because
this state not supported by the platform at least for now and may be
not in a near future ?
Then, I have another question regarding the update of the
psci-suspend-parameter. These changes implies an update of the psci
firmawre which means that we will now have 2 different firmware
version compatible with 2 different dt.
Is there any way to check that the ATF on the board is the one that
compatible with the parameter with something like a version ? I
currently use the previous firmware which works fine with current
kernel and dt binding once the NAP state is removed from the table.
When moving on recent kernel, I will have to take care of updating the
firmware and if i need to go back on a previous kernel, i will have to
make sure that i have the right ATF version. This make a lot of chance
of having the wrong configuration
Regards,
Vincent
On 12 December 2017 at 10:12, Leo Yan <leo.yan@linaro.org> wrote:
> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> idle state. From ftrace log we can observe CA73 CPUs can be easily
> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> anything and sleep again; so there have tons of trace events for CA73
> CPUs entering and exiting idle state.
>
> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> set its psci parameter as '0x0000001' and from this parameter it can
> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> takes 1 as a invalid value for state id, so the CPU cannot enter idle
> state and directly bail out to kernel.
>
> We want to create good practice for psci parameters platform definition,
> so review the psci specification. The spec "ARM Power State Coordination
> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> state IDs can be presented by below listed values; and it divides into
> three fields, every field can use 4 bits to present power states
> corresponding to core level, cluster level and system level:
> 0: Run
> 1: Standby
> 2: Retention
> 3: Powerdown
>
> This commit changes psci parameter to compliance with the suggested
> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> state parameters to '0x0010003' and '0x1010033' respectively.
>
> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Soby Mathew <Soby.Mathew@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index ab0b95b..99d5a46 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -147,7 +147,7 @@
>
> CPU_NAP: cpu-nap {
> compatible = "arm,idle-state";
> - arm,psci-suspend-param = <0x0000001>;
> + arm,psci-suspend-param = <0x0000002>;
> entry-latency-us = <7>;
> exit-latency-us = <2>;
> min-residency-us = <15>;
> @@ -156,7 +156,7 @@
> CPU_SLEEP: cpu-sleep {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x0010000>;
> + arm,psci-suspend-param = <0x0010003>;
> entry-latency-us = <40>;
> exit-latency-us = <70>;
> min-residency-us = <3000>;
> @@ -165,7 +165,7 @@
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <500>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> @@ -174,7 +174,7 @@
> CLUSTER_SLEEP_1: cluster-sleep-1 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <1000>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-25 2:22 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-25 2:22 UTC (permalink / raw)
To: Vincent Guittot
Cc: Wei Xu, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
LAK, devicetree, linux-kernel, Daniel Lezcano, Sudeep Holla,
Soby Mathew, John Stultz, kevin.wangtao
Hi Vincent,
[ + John, Kevin Wang ]
On Fri, Dec 22, 2017 at 03:22:51PM +0100, Vincent Guittot wrote:
> Hi Leo,
>
> Sorry for jumping late in the discussion but should we also remove
> the NAP state from the property cpu-idle-states of the CPUs because
> this state not supported by the platform at least for now and may be
> not in a near future ?
Thanks for bringing up this.
I don't want to hide anything for patch discussion :) this patch is to
resolve the PSCI parameter mismatching issue between kernel and ARM-TF
and it's not used to resolve the bug for CPU_NAP, so I didn't mention
the CPU_NAP malfunction issue to avoid complex discussion context.
I want to keep CPU_NAP state and track bug for CPU_NAP fixing; if we
remove this state, I suspect we might have no chance to enable it
anymore. Finally this is up to Hisilicon colleague decision and if they
have time to fix this.
I will offline to check with Daniel and Kevin for this; and if we
finally decide to remove it we can commit extra patch for this later,
how about you think?
> Then, I have another question regarding the update of the
> psci-suspend-parameter. These changes implies an update of the psci
> firmawre which means that we will now have 2 different firmware
> version compatible with 2 different dt.
>
> Is there any way to check that the ATF on the board is the one that
> compatible with the parameter with something like a version ? I
> currently use the previous firmware which works fine with current
> kernel and dt binding once the NAP state is removed from the table.
> When moving on recent kernel, I will have to take care of updating the
> firmware and if i need to go back on a previous kernel, i will have to
> make sure that i have the right ATF version. This make a lot of chance
> of having the wrong configuration
AFAIK, we cannot distinguish the PSCI parameter by PSCI version or
ARM-TF version number; alternatively one simple way for checking ARM-TF
is we can get commit ID (e.g. 83df7ce) from the ARM-TF log; so any
ARM-TF commit ID is newer than the patch fdae60b6ba27: "Hikey960:
Change to use recommended power state id format" should apply this
kernel patch.
NOTICE: BL1: Booting BL31
NOTICE: BL31: v1.4(debug):v1.4-441-g83df7ce-dirty
NOTICE: BL31: Built : 17:31:35, Dec 22 2017
BTW, I hope we can upgrade Linux kernel and ARM-TF to latest code base
to avoid compatible issue; for Android offical releasing it uses the
old PSCI parameters with Hisilicon legacy booting images, so they can
work well, but if someone uses ARM-TF mainline code + Android kernel
4.4/4.9, there must have compatible issue.
I am monitoring the integration ARM-TF/UEFI into Android on Hikey960,
we need backport this patch onto Android kernel 4.4/4.9 ASAP after
integration ARM-TF/UEFI.
Thanks,
Leo Yan
> Regards,
> Vincent
>
> On 12 December 2017 at 10:12, Leo Yan <leo.yan@linaro.org> wrote:
> > Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> > idle state. From ftrace log we can observe CA73 CPUs can be easily
> > waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> > anything and sleep again; so there have tons of trace events for CA73
> > CPUs entering and exiting idle state.
> >
> > On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> > set its psci parameter as '0x0000001' and from this parameter it can
> > calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> > takes 1 as a invalid value for state id, so the CPU cannot enter idle
> > state and directly bail out to kernel.
> >
> > We want to create good practice for psci parameters platform definition,
> > so review the psci specification. The spec "ARM Power State Coordination
> > Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> > ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> > state IDs can be presented by below listed values; and it divides into
> > three fields, every field can use 4 bits to present power states
> > corresponding to core level, cluster level and system level:
> > 0: Run
> > 1: Standby
> > 2: Retention
> > 3: Powerdown
> >
> > This commit changes psci parameter to compliance with the suggested
> > state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> > to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> > state parameters to '0x0010003' and '0x1010033' respectively.
> >
> > Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
> >
> > Cc: Vincent Guittot <vincent.guittot@linaro.org>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Soby Mathew <Soby.Mathew@arm.com>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > index ab0b95b..99d5a46 100644
> > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > @@ -147,7 +147,7 @@
> >
> > CPU_NAP: cpu-nap {
> > compatible = "arm,idle-state";
> > - arm,psci-suspend-param = <0x0000001>;
> > + arm,psci-suspend-param = <0x0000002>;
> > entry-latency-us = <7>;
> > exit-latency-us = <2>;
> > min-residency-us = <15>;
> > @@ -156,7 +156,7 @@
> > CPU_SLEEP: cpu-sleep {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x0010000>;
> > + arm,psci-suspend-param = <0x0010003>;
> > entry-latency-us = <40>;
> > exit-latency-us = <70>;
> > min-residency-us = <3000>;
> > @@ -165,7 +165,7 @@
> > CLUSTER_SLEEP_0: cluster-sleep-0 {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x1010000>;
> > + arm,psci-suspend-param = <0x1010033>;
> > entry-latency-us = <500>;
> > exit-latency-us = <5000>;
> > min-residency-us = <20000>;
> > @@ -174,7 +174,7 @@
> > CLUSTER_SLEEP_1: cluster-sleep-1 {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x1010000>;
> > + arm,psci-suspend-param = <0x1010033>;
> > entry-latency-us = <1000>;
> > exit-latency-us = <5000>;
> > min-residency-us = <20000>;
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-25 2:22 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-25 2:22 UTC (permalink / raw)
To: Vincent Guittot
Cc: Wei Xu, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel,
Daniel Lezcano, Sudeep Holla, Soby Mathew, John Stultz,
kevin.wangtao-C8/M+/jPZTeaMJb+Lgu22Q
Hi Vincent,
[ + John, Kevin Wang ]
On Fri, Dec 22, 2017 at 03:22:51PM +0100, Vincent Guittot wrote:
> Hi Leo,
>
> Sorry for jumping late in the discussion but should we also remove
> the NAP state from the property cpu-idle-states of the CPUs because
> this state not supported by the platform at least for now and may be
> not in a near future ?
Thanks for bringing up this.
I don't want to hide anything for patch discussion :) this patch is to
resolve the PSCI parameter mismatching issue between kernel and ARM-TF
and it's not used to resolve the bug for CPU_NAP, so I didn't mention
the CPU_NAP malfunction issue to avoid complex discussion context.
I want to keep CPU_NAP state and track bug for CPU_NAP fixing; if we
remove this state, I suspect we might have no chance to enable it
anymore. Finally this is up to Hisilicon colleague decision and if they
have time to fix this.
I will offline to check with Daniel and Kevin for this; and if we
finally decide to remove it we can commit extra patch for this later,
how about you think?
> Then, I have another question regarding the update of the
> psci-suspend-parameter. These changes implies an update of the psci
> firmawre which means that we will now have 2 different firmware
> version compatible with 2 different dt.
>
> Is there any way to check that the ATF on the board is the one that
> compatible with the parameter with something like a version ? I
> currently use the previous firmware which works fine with current
> kernel and dt binding once the NAP state is removed from the table.
> When moving on recent kernel, I will have to take care of updating the
> firmware and if i need to go back on a previous kernel, i will have to
> make sure that i have the right ATF version. This make a lot of chance
> of having the wrong configuration
AFAIK, we cannot distinguish the PSCI parameter by PSCI version or
ARM-TF version number; alternatively one simple way for checking ARM-TF
is we can get commit ID (e.g. 83df7ce) from the ARM-TF log; so any
ARM-TF commit ID is newer than the patch fdae60b6ba27: "Hikey960:
Change to use recommended power state id format" should apply this
kernel patch.
NOTICE: BL1: Booting BL31
NOTICE: BL31: v1.4(debug):v1.4-441-g83df7ce-dirty
NOTICE: BL31: Built : 17:31:35, Dec 22 2017
BTW, I hope we can upgrade Linux kernel and ARM-TF to latest code base
to avoid compatible issue; for Android offical releasing it uses the
old PSCI parameters with Hisilicon legacy booting images, so they can
work well, but if someone uses ARM-TF mainline code + Android kernel
4.4/4.9, there must have compatible issue.
I am monitoring the integration ARM-TF/UEFI into Android on Hikey960,
we need backport this patch onto Android kernel 4.4/4.9 ASAP after
integration ARM-TF/UEFI.
Thanks,
Leo Yan
> Regards,
> Vincent
>
> On 12 December 2017 at 10:12, Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> > idle state. From ftrace log we can observe CA73 CPUs can be easily
> > waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> > anything and sleep again; so there have tons of trace events for CA73
> > CPUs entering and exiting idle state.
> >
> > On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> > set its psci parameter as '0x0000001' and from this parameter it can
> > calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> > takes 1 as a invalid value for state id, so the CPU cannot enter idle
> > state and directly bail out to kernel.
> >
> > We want to create good practice for psci parameters platform definition,
> > so review the psci specification. The spec "ARM Power State Coordination
> > Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> > ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> > state IDs can be presented by below listed values; and it divides into
> > three fields, every field can use 4 bits to present power states
> > corresponding to core level, cluster level and system level:
> > 0: Run
> > 1: Standby
> > 2: Retention
> > 3: Powerdown
> >
> > This commit changes psci parameter to compliance with the suggested
> > state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> > to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> > state parameters to '0x0010003' and '0x1010033' respectively.
> >
> > Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
> >
> > Cc: Vincent Guittot <vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> > Cc: Soby Mathew <Soby.Mathew-5wv7dgnIgG8@public.gmane.org>
> > Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > index ab0b95b..99d5a46 100644
> > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > @@ -147,7 +147,7 @@
> >
> > CPU_NAP: cpu-nap {
> > compatible = "arm,idle-state";
> > - arm,psci-suspend-param = <0x0000001>;
> > + arm,psci-suspend-param = <0x0000002>;
> > entry-latency-us = <7>;
> > exit-latency-us = <2>;
> > min-residency-us = <15>;
> > @@ -156,7 +156,7 @@
> > CPU_SLEEP: cpu-sleep {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x0010000>;
> > + arm,psci-suspend-param = <0x0010003>;
> > entry-latency-us = <40>;
> > exit-latency-us = <70>;
> > min-residency-us = <3000>;
> > @@ -165,7 +165,7 @@
> > CLUSTER_SLEEP_0: cluster-sleep-0 {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x1010000>;
> > + arm,psci-suspend-param = <0x1010033>;
> > entry-latency-us = <500>;
> > exit-latency-us = <5000>;
> > min-residency-us = <20000>;
> > @@ -174,7 +174,7 @@
> > CLUSTER_SLEEP_1: cluster-sleep-1 {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x1010000>;
> > + arm,psci-suspend-param = <0x1010033>;
> > entry-latency-us = <1000>;
> > exit-latency-us = <5000>;
> > min-residency-us = <20000>;
> > --
> > 2.7.4
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-25 2:22 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-25 2:22 UTC (permalink / raw)
To: linux-arm-kernel
Hi Vincent,
[ + John, Kevin Wang ]
On Fri, Dec 22, 2017 at 03:22:51PM +0100, Vincent Guittot wrote:
> Hi Leo,
>
> Sorry for jumping late in the discussion but should we also remove
> the NAP state from the property cpu-idle-states of the CPUs because
> this state not supported by the platform at least for now and may be
> not in a near future ?
Thanks for bringing up this.
I don't want to hide anything for patch discussion :) this patch is to
resolve the PSCI parameter mismatching issue between kernel and ARM-TF
and it's not used to resolve the bug for CPU_NAP, so I didn't mention
the CPU_NAP malfunction issue to avoid complex discussion context.
I want to keep CPU_NAP state and track bug for CPU_NAP fixing; if we
remove this state, I suspect we might have no chance to enable it
anymore. Finally this is up to Hisilicon colleague decision and if they
have time to fix this.
I will offline to check with Daniel and Kevin for this; and if we
finally decide to remove it we can commit extra patch for this later,
how about you think?
> Then, I have another question regarding the update of the
> psci-suspend-parameter. These changes implies an update of the psci
> firmawre which means that we will now have 2 different firmware
> version compatible with 2 different dt.
>
> Is there any way to check that the ATF on the board is the one that
> compatible with the parameter with something like a version ? I
> currently use the previous firmware which works fine with current
> kernel and dt binding once the NAP state is removed from the table.
> When moving on recent kernel, I will have to take care of updating the
> firmware and if i need to go back on a previous kernel, i will have to
> make sure that i have the right ATF version. This make a lot of chance
> of having the wrong configuration
AFAIK, we cannot distinguish the PSCI parameter by PSCI version or
ARM-TF version number; alternatively one simple way for checking ARM-TF
is we can get commit ID (e.g. 83df7ce) from the ARM-TF log; so any
ARM-TF commit ID is newer than the patch fdae60b6ba27: "Hikey960:
Change to use recommended power state id format" should apply this
kernel patch.
NOTICE: BL1: Booting BL31
NOTICE: BL31: v1.4(debug):v1.4-441-g83df7ce-dirty
NOTICE: BL31: Built : 17:31:35, Dec 22 2017
BTW, I hope we can upgrade Linux kernel and ARM-TF to latest code base
to avoid compatible issue; for Android offical releasing it uses the
old PSCI parameters with Hisilicon legacy booting images, so they can
work well, but if someone uses ARM-TF mainline code + Android kernel
4.4/4.9, there must have compatible issue.
I am monitoring the integration ARM-TF/UEFI into Android on Hikey960,
we need backport this patch onto Android kernel 4.4/4.9 ASAP after
integration ARM-TF/UEFI.
Thanks,
Leo Yan
> Regards,
> Vincent
>
> On 12 December 2017 at 10:12, Leo Yan <leo.yan@linaro.org> wrote:
> > Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> > idle state. From ftrace log we can observe CA73 CPUs can be easily
> > waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> > anything and sleep again; so there have tons of trace events for CA73
> > CPUs entering and exiting idle state.
> >
> > On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> > set its psci parameter as '0x0000001' and from this parameter it can
> > calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> > takes 1 as a invalid value for state id, so the CPU cannot enter idle
> > state and directly bail out to kernel.
> >
> > We want to create good practice for psci parameters platform definition,
> > so review the psci specification. The spec "ARM Power State Coordination
> > Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> > ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> > state IDs can be presented by below listed values; and it divides into
> > three fields, every field can use 4 bits to present power states
> > corresponding to core level, cluster level and system level:
> > 0: Run
> > 1: Standby
> > 2: Retention
> > 3: Powerdown
> >
> > This commit changes psci parameter to compliance with the suggested
> > state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> > to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> > state parameters to '0x0010003' and '0x1010033' respectively.
> >
> > Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
> >
> > Cc: Vincent Guittot <vincent.guittot@linaro.org>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Soby Mathew <Soby.Mathew@arm.com>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > index ab0b95b..99d5a46 100644
> > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > @@ -147,7 +147,7 @@
> >
> > CPU_NAP: cpu-nap {
> > compatible = "arm,idle-state";
> > - arm,psci-suspend-param = <0x0000001>;
> > + arm,psci-suspend-param = <0x0000002>;
> > entry-latency-us = <7>;
> > exit-latency-us = <2>;
> > min-residency-us = <15>;
> > @@ -156,7 +156,7 @@
> > CPU_SLEEP: cpu-sleep {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x0010000>;
> > + arm,psci-suspend-param = <0x0010003>;
> > entry-latency-us = <40>;
> > exit-latency-us = <70>;
> > min-residency-us = <3000>;
> > @@ -165,7 +165,7 @@
> > CLUSTER_SLEEP_0: cluster-sleep-0 {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x1010000>;
> > + arm,psci-suspend-param = <0x1010033>;
> > entry-latency-us = <500>;
> > exit-latency-us = <5000>;
> > min-residency-us = <20000>;
> > @@ -174,7 +174,7 @@
> > CLUSTER_SLEEP_1: cluster-sleep-1 {
> > compatible = "arm,idle-state";
> > local-timer-stop;
> > - arm,psci-suspend-param = <0x1010000>;
> > + arm,psci-suspend-param = <0x1010033>;
> > entry-latency-us = <1000>;
> > exit-latency-us = <5000>;
> > min-residency-us = <20000>;
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
2017-12-25 2:22 ` Leo Yan
@ 2017-12-27 8:29 ` Vincent Guittot
-1 siblings, 0 replies; 20+ messages in thread
From: Vincent Guittot @ 2017-12-27 8:29 UTC (permalink / raw)
To: Leo Yan
Cc: Wei Xu, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
LAK, devicetree, linux-kernel, Daniel Lezcano, Sudeep Holla,
Soby Mathew, John Stultz, Wangtao (Kevin, Kirin)
Hi Leo
On 25 December 2017 at 03:22, Leo Yan <leo.yan@linaro.org> wrote:
> Hi Vincent,
>
> [ + John, Kevin Wang ]
>
> On Fri, Dec 22, 2017 at 03:22:51PM +0100, Vincent Guittot wrote:
>> Hi Leo,
>>
>> Sorry for jumping late in the discussion but should we also remove
>> the NAP state from the property cpu-idle-states of the CPUs because
>> this state not supported by the platform at least for now and may be
>> not in a near future ?
>
> Thanks for bringing up this.
>
> I don't want to hide anything for patch discussion :) this patch is to
> resolve the PSCI parameter mismatching issue between kernel and ARM-TF
> and it's not used to resolve the bug for CPU_NAP, so I didn't mention
> the CPU_NAP malfunction issue to avoid complex discussion context.
>
> I want to keep CPU_NAP state and track bug for CPU_NAP fixing; if we
> remove this state, I suspect we might have no chance to enable it
> anymore. Finally this is up to Hisilicon colleague decision and if they
> have time to fix this.
>
> I will offline to check with Daniel and Kevin for this; and if we
> finally decide to remove it we can commit extra patch for this later,
> how about you think?
I would prefer to remove it right now.
Removing NAP from c-state table makes the hikey960 working correctly;
I mean even with current ATF and current state id. So it's the best
solution to the NAP problem IMO and I don't see the benefit of keeping
NAP in the table until this state has been fixed. This will just add
uncertainties in the behavior of the board.
I don't see why you can't re-add it once it has been fixed.
>
>> Then, I have another question regarding the update of the
>> psci-suspend-parameter. These changes implies an update of the psci
>> firmawre which means that we will now have 2 different firmware
>> version compatible with 2 different dt.
>>
>> Is there any way to check that the ATF on the board is the one that
>> compatible with the parameter with something like a version ? I
>> currently use the previous firmware which works fine with current
>> kernel and dt binding once the NAP state is removed from the table.
>> When moving on recent kernel, I will have to take care of updating the
>> firmware and if i need to go back on a previous kernel, i will have to
>> make sure that i have the right ATF version. This make a lot of chance
>> of having the wrong configuration
>
> AFAIK, we cannot distinguish the PSCI parameter by PSCI version or
And that's my main concern because this adds a new possible regression
factor when switching between different kernel version
> ARM-TF version number; alternatively one simple way for checking ARM-TF
> is we can get commit ID (e.g. 83df7ce) from the ARM-TF log; so any
> ARM-TF commit ID is newer than the patch fdae60b6ba27: "Hikey960:
> Change to use recommended power state id format" should apply this
> kernel patch.
>
> NOTICE: BL1: Booting BL31
> NOTICE: BL31: v1.4(debug):v1.4-441-g83df7ce-dirty
> NOTICE: BL31: Built : 17:31:35, Dec 22 2017
>
> BTW, I hope we can upgrade Linux kernel and ARM-TF to latest code base
> to avoid compatible issue; for Android offical releasing it uses the
> old PSCI parameters with Hisilicon legacy booting images, so they can
> work well, but if someone uses ARM-TF mainline code + Android kernel
> 4.4/4.9, there must have compatible issue.
>
> I am monitoring the integration ARM-TF/UEFI into Android on Hikey960,
> we need backport this patch onto Android kernel 4.4/4.9 ASAP after
> integration ARM-TF/UEFI.
>
> Thanks,
> Leo Yan
>
>> Regards,
>> Vincent
>>
>> On 12 December 2017 at 10:12, Leo Yan <leo.yan@linaro.org> wrote:
>> > Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
>> > idle state. From ftrace log we can observe CA73 CPUs can be easily
>> > waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
>> > anything and sleep again; so there have tons of trace events for CA73
>> > CPUs entering and exiting idle state.
>> >
>> > On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
>> > set its psci parameter as '0x0000001' and from this parameter it can
>> > calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
>> > takes 1 as a invalid value for state id, so the CPU cannot enter idle
>> > state and directly bail out to kernel.
>> >
>> > We want to create good practice for psci parameters platform definition,
>> > so review the psci specification. The spec "ARM Power State Coordination
>> > Interface - Platform Design Document (ARM DEN 0022D)" recommends state
>> > ID in chapter "6.5 Recommended StateID Encoding". The recommended power
>> > state IDs can be presented by below listed values; and it divides into
>> > three fields, every field can use 4 bits to present power states
>> > corresponding to core level, cluster level and system level:
>> > 0: Run
>> > 1: Standby
>> > 2: Retention
>> > 3: Powerdown
>> >
>> > This commit changes psci parameter to compliance with the suggested
>> > state ID in the doc. Except we change 'CPU_NAP' state psci parameter
>> > to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
>> > state parameters to '0x0010003' and '0x1010033' respectively.
>> >
>> > Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>> >
>> > Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> > Cc: Sudeep Holla <sudeep.holla@arm.com>
>> > Cc: Soby Mathew <Soby.Mathew@arm.com>
>> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> > ---
>> > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
>> > 1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > index ab0b95b..99d5a46 100644
>> > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > @@ -147,7 +147,7 @@
>> >
>> > CPU_NAP: cpu-nap {
>> > compatible = "arm,idle-state";
>> > - arm,psci-suspend-param = <0x0000001>;
>> > + arm,psci-suspend-param = <0x0000002>;
>> > entry-latency-us = <7>;
>> > exit-latency-us = <2>;
>> > min-residency-us = <15>;
>> > @@ -156,7 +156,7 @@
>> > CPU_SLEEP: cpu-sleep {
>> > compatible = "arm,idle-state";
>> > local-timer-stop;
>> > - arm,psci-suspend-param = <0x0010000>;
>> > + arm,psci-suspend-param = <0x0010003>;
>> > entry-latency-us = <40>;
>> > exit-latency-us = <70>;
>> > min-residency-us = <3000>;
>> > @@ -165,7 +165,7 @@
>> > CLUSTER_SLEEP_0: cluster-sleep-0 {
>> > compatible = "arm,idle-state";
>> > local-timer-stop;
>> > - arm,psci-suspend-param = <0x1010000>;
>> > + arm,psci-suspend-param = <0x1010033>;
>> > entry-latency-us = <500>;
>> > exit-latency-us = <5000>;
>> > min-residency-us = <20000>;
>> > @@ -174,7 +174,7 @@
>> > CLUSTER_SLEEP_1: cluster-sleep-1 {
>> > compatible = "arm,idle-state";
>> > local-timer-stop;
>> > - arm,psci-suspend-param = <0x1010000>;
>> > + arm,psci-suspend-param = <0x1010033>;
>> > entry-latency-us = <1000>;
>> > exit-latency-us = <5000>;
>> > min-residency-us = <20000>;
>> > --
>> > 2.7.4
>> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-27 8:29 ` Vincent Guittot
0 siblings, 0 replies; 20+ messages in thread
From: Vincent Guittot @ 2017-12-27 8:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi Leo
On 25 December 2017 at 03:22, Leo Yan <leo.yan@linaro.org> wrote:
> Hi Vincent,
>
> [ + John, Kevin Wang ]
>
> On Fri, Dec 22, 2017 at 03:22:51PM +0100, Vincent Guittot wrote:
>> Hi Leo,
>>
>> Sorry for jumping late in the discussion but should we also remove
>> the NAP state from the property cpu-idle-states of the CPUs because
>> this state not supported by the platform at least for now and may be
>> not in a near future ?
>
> Thanks for bringing up this.
>
> I don't want to hide anything for patch discussion :) this patch is to
> resolve the PSCI parameter mismatching issue between kernel and ARM-TF
> and it's not used to resolve the bug for CPU_NAP, so I didn't mention
> the CPU_NAP malfunction issue to avoid complex discussion context.
>
> I want to keep CPU_NAP state and track bug for CPU_NAP fixing; if we
> remove this state, I suspect we might have no chance to enable it
> anymore. Finally this is up to Hisilicon colleague decision and if they
> have time to fix this.
>
> I will offline to check with Daniel and Kevin for this; and if we
> finally decide to remove it we can commit extra patch for this later,
> how about you think?
I would prefer to remove it right now.
Removing NAP from c-state table makes the hikey960 working correctly;
I mean even with current ATF and current state id. So it's the best
solution to the NAP problem IMO and I don't see the benefit of keeping
NAP in the table until this state has been fixed. This will just add
uncertainties in the behavior of the board.
I don't see why you can't re-add it once it has been fixed.
>
>> Then, I have another question regarding the update of the
>> psci-suspend-parameter. These changes implies an update of the psci
>> firmawre which means that we will now have 2 different firmware
>> version compatible with 2 different dt.
>>
>> Is there any way to check that the ATF on the board is the one that
>> compatible with the parameter with something like a version ? I
>> currently use the previous firmware which works fine with current
>> kernel and dt binding once the NAP state is removed from the table.
>> When moving on recent kernel, I will have to take care of updating the
>> firmware and if i need to go back on a previous kernel, i will have to
>> make sure that i have the right ATF version. This make a lot of chance
>> of having the wrong configuration
>
> AFAIK, we cannot distinguish the PSCI parameter by PSCI version or
And that's my main concern because this adds a new possible regression
factor when switching between different kernel version
> ARM-TF version number; alternatively one simple way for checking ARM-TF
> is we can get commit ID (e.g. 83df7ce) from the ARM-TF log; so any
> ARM-TF commit ID is newer than the patch fdae60b6ba27: "Hikey960:
> Change to use recommended power state id format" should apply this
> kernel patch.
>
> NOTICE: BL1: Booting BL31
> NOTICE: BL31: v1.4(debug):v1.4-441-g83df7ce-dirty
> NOTICE: BL31: Built : 17:31:35, Dec 22 2017
>
> BTW, I hope we can upgrade Linux kernel and ARM-TF to latest code base
> to avoid compatible issue; for Android offical releasing it uses the
> old PSCI parameters with Hisilicon legacy booting images, so they can
> work well, but if someone uses ARM-TF mainline code + Android kernel
> 4.4/4.9, there must have compatible issue.
>
> I am monitoring the integration ARM-TF/UEFI into Android on Hikey960,
> we need backport this patch onto Android kernel 4.4/4.9 ASAP after
> integration ARM-TF/UEFI.
>
> Thanks,
> Leo Yan
>
>> Regards,
>> Vincent
>>
>> On 12 December 2017 at 10:12, Leo Yan <leo.yan@linaro.org> wrote:
>> > Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
>> > idle state. From ftrace log we can observe CA73 CPUs can be easily
>> > waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
>> > anything and sleep again; so there have tons of trace events for CA73
>> > CPUs entering and exiting idle state.
>> >
>> > On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
>> > set its psci parameter as '0x0000001' and from this parameter it can
>> > calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
>> > takes 1 as a invalid value for state id, so the CPU cannot enter idle
>> > state and directly bail out to kernel.
>> >
>> > We want to create good practice for psci parameters platform definition,
>> > so review the psci specification. The spec "ARM Power State Coordination
>> > Interface - Platform Design Document (ARM DEN 0022D)" recommends state
>> > ID in chapter "6.5 Recommended StateID Encoding". The recommended power
>> > state IDs can be presented by below listed values; and it divides into
>> > three fields, every field can use 4 bits to present power states
>> > corresponding to core level, cluster level and system level:
>> > 0: Run
>> > 1: Standby
>> > 2: Retention
>> > 3: Powerdown
>> >
>> > This commit changes psci parameter to compliance with the suggested
>> > state ID in the doc. Except we change 'CPU_NAP' state psci parameter
>> > to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
>> > state parameters to '0x0010003' and '0x1010033' respectively.
>> >
>> > Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>> >
>> > Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> > Cc: Sudeep Holla <sudeep.holla@arm.com>
>> > Cc: Soby Mathew <Soby.Mathew@arm.com>
>> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> > ---
>> > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
>> > 1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > index ab0b95b..99d5a46 100644
>> > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > @@ -147,7 +147,7 @@
>> >
>> > CPU_NAP: cpu-nap {
>> > compatible = "arm,idle-state";
>> > - arm,psci-suspend-param = <0x0000001>;
>> > + arm,psci-suspend-param = <0x0000002>;
>> > entry-latency-us = <7>;
>> > exit-latency-us = <2>;
>> > min-residency-us = <15>;
>> > @@ -156,7 +156,7 @@
>> > CPU_SLEEP: cpu-sleep {
>> > compatible = "arm,idle-state";
>> > local-timer-stop;
>> > - arm,psci-suspend-param = <0x0010000>;
>> > + arm,psci-suspend-param = <0x0010003>;
>> > entry-latency-us = <40>;
>> > exit-latency-us = <70>;
>> > min-residency-us = <3000>;
>> > @@ -165,7 +165,7 @@
>> > CLUSTER_SLEEP_0: cluster-sleep-0 {
>> > compatible = "arm,idle-state";
>> > local-timer-stop;
>> > - arm,psci-suspend-param = <0x1010000>;
>> > + arm,psci-suspend-param = <0x1010033>;
>> > entry-latency-us = <500>;
>> > exit-latency-us = <5000>;
>> > min-residency-us = <20000>;
>> > @@ -174,7 +174,7 @@
>> > CLUSTER_SLEEP_1: cluster-sleep-1 {
>> > compatible = "arm,idle-state";
>> > local-timer-stop;
>> > - arm,psci-suspend-param = <0x1010000>;
>> > + arm,psci-suspend-param = <0x1010033>;
>> > entry-latency-us = <1000>;
>> > exit-latency-us = <5000>;
>> > min-residency-us = <20000>;
>> > --
>> > 2.7.4
>> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-28 21:14 ` Wei Xu
0 siblings, 0 replies; 20+ messages in thread
From: Wei Xu @ 2017-12-28 21:14 UTC (permalink / raw)
To: Leo Yan, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
linux-arm-kernel, devicetree, linux-kernel
Cc: Vincent Guittot, Daniel Lezcano, Sudeep Holla, Soby Mathew
Hi Leo,
On 2017/12/22 9:37, Wei Xu wrote:
> Hi Leo,
>
> On 2017/12/12 9:12, Leo Yan wrote:
>> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
>> idle state. From ftrace log we can observe CA73 CPUs can be easily
>> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
>> anything and sleep again; so there have tons of trace events for CA73
>> CPUs entering and exiting idle state.
>>
>> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
>> set its psci parameter as '0x0000001' and from this parameter it can
>> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
>> takes 1 as a invalid value for state id, so the CPU cannot enter idle
>> state and directly bail out to kernel.
>>
>> We want to create good practice for psci parameters platform definition,
>> so review the psci specification. The spec "ARM Power State Coordination
>> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
>> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
>> state IDs can be presented by below listed values; and it divides into
>> three fields, every field can use 4 bits to present power states
>> corresponding to core level, cluster level and system level:
>> 0: Run
>> 1: Standby
>> 2: Retention
>> 3: Powerdown
>>
>> This commit changes psci parameter to compliance with the suggested
>> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
>> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
>> state parameters to '0x0010003' and '0x1010033' respectively.
>>
>> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>>
>> Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Soby Mathew <Soby.Mathew@arm.com>
>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> ---
>
> Applied into hisilicon dt tree.
> Thanks!
Sorry, since this patch is still under discussion,
I will drop it firstly.
Thanks!
Best Regards,
Wei
>
> Best Regards,
> Wei
>
>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> index ab0b95b..99d5a46 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> @@ -147,7 +147,7 @@
>>
>> CPU_NAP: cpu-nap {
>> compatible = "arm,idle-state";
>> - arm,psci-suspend-param = <0x0000001>;
>> + arm,psci-suspend-param = <0x0000002>;
>> entry-latency-us = <7>;
>> exit-latency-us = <2>;
>> min-residency-us = <15>;
>> @@ -156,7 +156,7 @@
>> CPU_SLEEP: cpu-sleep {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x0010000>;
>> + arm,psci-suspend-param = <0x0010003>;
>> entry-latency-us = <40>;
>> exit-latency-us = <70>;
>> min-residency-us = <3000>;
>> @@ -165,7 +165,7 @@
>> CLUSTER_SLEEP_0: cluster-sleep-0 {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x1010000>;
>> + arm,psci-suspend-param = <0x1010033>;
>> entry-latency-us = <500>;
>> exit-latency-us = <5000>;
>> min-residency-us = <20000>;
>> @@ -174,7 +174,7 @@
>> CLUSTER_SLEEP_1: cluster-sleep-1 {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x1010000>;
>> + arm,psci-suspend-param = <0x1010033>;
>> entry-latency-us = <1000>;
>> exit-latency-us = <5000>;
>> min-residency-us = <20000>;
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-28 21:14 ` Wei Xu
0 siblings, 0 replies; 20+ messages in thread
From: Wei Xu @ 2017-12-28 21:14 UTC (permalink / raw)
To: Leo Yan, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Vincent Guittot, Daniel Lezcano, Sudeep Holla, Soby Mathew
Hi Leo,
On 2017/12/22 9:37, Wei Xu wrote:
> Hi Leo,
>
> On 2017/12/12 9:12, Leo Yan wrote:
>> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
>> idle state. From ftrace log we can observe CA73 CPUs can be easily
>> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
>> anything and sleep again; so there have tons of trace events for CA73
>> CPUs entering and exiting idle state.
>>
>> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
>> set its psci parameter as '0x0000001' and from this parameter it can
>> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
>> takes 1 as a invalid value for state id, so the CPU cannot enter idle
>> state and directly bail out to kernel.
>>
>> We want to create good practice for psci parameters platform definition,
>> so review the psci specification. The spec "ARM Power State Coordination
>> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
>> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
>> state IDs can be presented by below listed values; and it divides into
>> three fields, every field can use 4 bits to present power states
>> corresponding to core level, cluster level and system level:
>> 0: Run
>> 1: Standby
>> 2: Retention
>> 3: Powerdown
>>
>> This commit changes psci parameter to compliance with the suggested
>> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
>> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
>> state parameters to '0x0010003' and '0x1010033' respectively.
>>
>> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>>
>> Cc: Vincent Guittot <vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>> Cc: Soby Mathew <Soby.Mathew-5wv7dgnIgG8@public.gmane.org>
>> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>
> Applied into hisilicon dt tree.
> Thanks!
Sorry, since this patch is still under discussion,
I will drop it firstly.
Thanks!
Best Regards,
Wei
>
> Best Regards,
> Wei
>
>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> index ab0b95b..99d5a46 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> @@ -147,7 +147,7 @@
>>
>> CPU_NAP: cpu-nap {
>> compatible = "arm,idle-state";
>> - arm,psci-suspend-param = <0x0000001>;
>> + arm,psci-suspend-param = <0x0000002>;
>> entry-latency-us = <7>;
>> exit-latency-us = <2>;
>> min-residency-us = <15>;
>> @@ -156,7 +156,7 @@
>> CPU_SLEEP: cpu-sleep {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x0010000>;
>> + arm,psci-suspend-param = <0x0010003>;
>> entry-latency-us = <40>;
>> exit-latency-us = <70>;
>> min-residency-us = <3000>;
>> @@ -165,7 +165,7 @@
>> CLUSTER_SLEEP_0: cluster-sleep-0 {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x1010000>;
>> + arm,psci-suspend-param = <0x1010033>;
>> entry-latency-us = <500>;
>> exit-latency-us = <5000>;
>> min-residency-us = <20000>;
>> @@ -174,7 +174,7 @@
>> CLUSTER_SLEEP_1: cluster-sleep-1 {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x1010000>;
>> + arm,psci-suspend-param = <0x1010033>;
>> entry-latency-us = <1000>;
>> exit-latency-us = <5000>;
>> min-residency-us = <20000>;
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-28 21:14 ` Wei Xu
0 siblings, 0 replies; 20+ messages in thread
From: Wei Xu @ 2017-12-28 21:14 UTC (permalink / raw)
To: linux-arm-kernel
Hi Leo,
On 2017/12/22 9:37, Wei Xu wrote:
> Hi Leo,
>
> On 2017/12/12 9:12, Leo Yan wrote:
>> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
>> idle state. From ftrace log we can observe CA73 CPUs can be easily
>> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
>> anything and sleep again; so there have tons of trace events for CA73
>> CPUs entering and exiting idle state.
>>
>> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
>> set its psci parameter as '0x0000001' and from this parameter it can
>> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
>> takes 1 as a invalid value for state id, so the CPU cannot enter idle
>> state and directly bail out to kernel.
>>
>> We want to create good practice for psci parameters platform definition,
>> so review the psci specification. The spec "ARM Power State Coordination
>> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
>> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
>> state IDs can be presented by below listed values; and it divides into
>> three fields, every field can use 4 bits to present power states
>> corresponding to core level, cluster level and system level:
>> 0: Run
>> 1: Standby
>> 2: Retention
>> 3: Powerdown
>>
>> This commit changes psci parameter to compliance with the suggested
>> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
>> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
>> state parameters to '0x0010003' and '0x1010033' respectively.
>>
>> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>>
>> Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Soby Mathew <Soby.Mathew@arm.com>
>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> ---
>
> Applied into hisilicon dt tree.
> Thanks!
Sorry, since this patch is still under discussion,
I will drop it firstly.
Thanks!
Best Regards,
Wei
>
> Best Regards,
> Wei
>
>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> index ab0b95b..99d5a46 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> @@ -147,7 +147,7 @@
>>
>> CPU_NAP: cpu-nap {
>> compatible = "arm,idle-state";
>> - arm,psci-suspend-param = <0x0000001>;
>> + arm,psci-suspend-param = <0x0000002>;
>> entry-latency-us = <7>;
>> exit-latency-us = <2>;
>> min-residency-us = <15>;
>> @@ -156,7 +156,7 @@
>> CPU_SLEEP: cpu-sleep {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x0010000>;
>> + arm,psci-suspend-param = <0x0010003>;
>> entry-latency-us = <40>;
>> exit-latency-us = <70>;
>> min-residency-us = <3000>;
>> @@ -165,7 +165,7 @@
>> CLUSTER_SLEEP_0: cluster-sleep-0 {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x1010000>;
>> + arm,psci-suspend-param = <0x1010033>;
>> entry-latency-us = <500>;
>> exit-latency-us = <5000>;
>> min-residency-us = <20000>;
>> @@ -174,7 +174,7 @@
>> CLUSTER_SLEEP_1: cluster-sleep-1 {
>> compatible = "arm,idle-state";
>> local-timer-stop;
>> - arm,psci-suspend-param = <0x1010000>;
>> + arm,psci-suspend-param = <0x1010033>;
>> entry-latency-us = <1000>;
>> exit-latency-us = <5000>;
>> min-residency-us = <20000>;
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-28 23:13 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-28 23:13 UTC (permalink / raw)
To: Wei Xu
Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
linux-arm-kernel, devicetree, linux-kernel, Vincent Guittot,
Daniel Lezcano, Sudeep Holla, Soby Mathew
On Thu, Dec 28, 2017 at 09:14:03PM +0000, Wei Xu wrote:
> Hi Leo,
[...]
> Sorry, since this patch is still under discussion,
> I will drop it firstly.
> Thanks!
Thanks, Wei.
> Best Regards,
> Wei
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-28 23:13 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-28 23:13 UTC (permalink / raw)
To: Wei Xu
Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
Daniel Lezcano, Sudeep Holla, Soby Mathew
On Thu, Dec 28, 2017 at 09:14:03PM +0000, Wei Xu wrote:
> Hi Leo,
[...]
> Sorry, since this patch is still under discussion,
> I will drop it firstly.
> Thanks!
Thanks, Wei.
> Best Regards,
> Wei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
@ 2017-12-28 23:13 ` Leo Yan
0 siblings, 0 replies; 20+ messages in thread
From: Leo Yan @ 2017-12-28 23:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 28, 2017 at 09:14:03PM +0000, Wei Xu wrote:
> Hi Leo,
[...]
> Sorry, since this patch is still under discussion,
> I will drop it firstly.
> Thanks!
Thanks, Wei.
> Best Regards,
> Wei
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-12-28 23:14 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-12 9:12 [PATCH v2] arm64: dts: Hi3660: Fix up psci state id Leo Yan
2017-12-12 9:12 ` Leo Yan
2017-12-12 9:12 ` Leo Yan
2017-12-22 9:37 ` Wei Xu
2017-12-22 9:37 ` Wei Xu
2017-12-22 9:37 ` Wei Xu
2017-12-28 21:14 ` Wei Xu
2017-12-28 21:14 ` Wei Xu
2017-12-28 21:14 ` Wei Xu
2017-12-28 23:13 ` Leo Yan
2017-12-28 23:13 ` Leo Yan
2017-12-28 23:13 ` Leo Yan
2017-12-22 14:22 ` Vincent Guittot
2017-12-22 14:22 ` Vincent Guittot
2017-12-22 14:22 ` Vincent Guittot
2017-12-25 2:22 ` Leo Yan
2017-12-25 2:22 ` Leo Yan
2017-12-25 2:22 ` Leo Yan
2017-12-27 8:29 ` Vincent Guittot
2017-12-27 8:29 ` Vincent Guittot
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.