From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756109AbdLVJhn (ORCPT ); Fri, 22 Dec 2017 04:37:43 -0500 Received: from szxga04-in.huawei.com ([45.249.212.190]:2714 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932097AbdLVJhY (ORCPT ); Fri, 22 Dec 2017 04:37:24 -0500 Subject: Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id To: Leo Yan , Rob Herring , "Mark Rutland" , Catalin Marinas , Will Deacon , , , References: <1513069954-22827-1-git-send-email-leo.yan@linaro.org> CC: Vincent Guittot , Daniel Lezcano , Sudeep Holla , Soby Mathew From: Wei Xu Message-ID: <5A3CD23C.3080307@hisilicon.com> Date: Fri, 22 Dec 2017 09:37:00 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1513069954-22827-1-git-send-email-leo.yan@linaro.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.202.226.123] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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 > Cc: Daniel Lezcano > Cc: Sudeep Holla > Cc: Soby Mathew > Signed-off-by: Leo Yan > --- 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>; >