From: xuwei5@hisilicon.com (Wei Xu)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
Date: Fri, 22 Dec 2017 09:37:00 +0000 [thread overview]
Message-ID: <5A3CD23C.3080307@hisilicon.com> (raw)
In-Reply-To: <1513069954-22827-1-git-send-email-leo.yan@linaro.org>
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>;
>
WARNING: multiple messages have this Message-ID (diff)
From: Wei Xu <xuwei5-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
To: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>,
Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Vincent Guittot
<vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
Daniel Lezcano
<daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>,
Soby Mathew <Soby.Mathew-5wv7dgnIgG8@public.gmane.org>
Subject: Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
Date: Fri, 22 Dec 2017 09:37:00 +0000 [thread overview]
Message-ID: <5A3CD23C.3080307@hisilicon.com> (raw)
In-Reply-To: <1513069954-22827-1-git-send-email-leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
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
WARNING: multiple messages have this Message-ID (diff)
From: Wei Xu <xuwei5@hisilicon.com>
To: Leo Yan <leo.yan@linaro.org>, Rob Herring <robh+dt@kernel.org>,
"Mark Rutland" <mark.rutland@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
<linux-arm-kernel@lists.infradead.org>,
<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Sudeep Holla <sudeep.holla@arm.com>,
Soby Mathew <Soby.Mathew@arm.com>
Subject: Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
Date: Fri, 22 Dec 2017 09:37:00 +0000 [thread overview]
Message-ID: <5A3CD23C.3080307@hisilicon.com> (raw)
In-Reply-To: <1513069954-22827-1-git-send-email-leo.yan@linaro.org>
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>;
>
next prev parent reply other threads:[~2017-12-22 9:37 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5A3CD23C.3080307@hisilicon.com \
--to=xuwei5@hisilicon.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.