From patchwork Fri Nov 24 06:56:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leo Yan X-Patchwork-Id: 10073649 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DF9F860375 for ; Fri, 24 Nov 2017 07:13:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D1B402A321 for ; Fri, 24 Nov 2017 07:13:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C65CE2A349; Fri, 24 Nov 2017 07:13:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3F6DA2A321 for ; Fri, 24 Nov 2017 07:13:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=h6XR2jYQtyB61/ri13KZxJThbZWPmawSQVanflwot1c=; b=j4AdDG+RuDeOYr jsaFCRFFjrE0ivrya6zgKVnEJdJ58HjwXqt9ykgqvL+WU7IqU+HrfvGbfJ5pAlZlZ6x7mLvYk6iZ2 pYHxI8U71h5/R/haf9F7/V6oalOQlleFexKS0RShZtCGYAWOLrmWk8YIY6L/n1OBT+y+nNRiFKlgS 6wNoahWBWvMF2vd8qDQH4hKQo9DBb2SR37lgMgy4tbEUO9CQXCGLu3zoeSK9BnH/2exchJhIM/Wzt JmBzAGIAKZKnC2iT4/zWtv6P/pzf5K2pepPFLsN4iK0zrl8Po7IshgamZAvQsqZ5vNz3CZsXULiRP CnzHCW0u47tkhC8crcSg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eI8B6-0006KB-AT; Fri, 24 Nov 2017 07:13:36 +0000 Received: from casper.infradead.org ([2001:8b0:10b:1236::1]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1eI8B3-0006Hx-MK for linux-arm-kernel@bombadil.infradead.org; Fri, 24 Nov 2017 07:13:33 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=AKCkwbYOD4/uQws2ONVIU/zjT7/t46WzLXTs9cm4HkA=; b=W45uhERjqp4ZzxqMJ5h3W95KM VUa+19nZpwJ+JBMpSLKqYhL4mtKe031Cafqpr2pLQAR3CDgW4qOIBhmkEdtHHi0xGKomYPPeEOYJr DfQIE/sTDPOdtbfio4t1DdEEZ5YIGIYmAGk1BUgE5nxy5xOoikVv15jeh5NOlKF8ptn5EhvG93V4K Vp2xpsEVNEhq+PkthIia9BpbB01VaWDr0/bPeERDZbamB0B/FBGegrgt/yJh+VuGQikr4wmc3QzC8 JeyUwYA5umLoyBkXyi3p9e/Etv0be+C/NaR9EhvIdLrjwPBHlg7zWjHpC2LXHN8yRFMq+VugIx4N+ IPG6rxoSA==; Received: from mail-wm0-x241.google.com ([2a00:1450:400c:c09::241]) by casper.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1eI7v8-0002y0-Jf for linux-arm-kernel@lists.infradead.org; Fri, 24 Nov 2017 06:57:10 +0000 Received: by mail-wm0-x241.google.com with SMTP id g130so19339211wme.0 for ; Thu, 23 Nov 2017 22:56:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=AKCkwbYOD4/uQws2ONVIU/zjT7/t46WzLXTs9cm4HkA=; b=L9I0vIEgQTeHvwtntzOIpdOlMYp7/EpMGRaagLbrJkw2fTl/4SH7ca8PDGQA/7sOss tKYwMwhP1ygo1/x1Z+STFhgfzTZIKMZ+Q0d/0LmMsLO3yEn5UoAln+ArV/I9wAgov20H osk1BXv1G3qeeIOb4lyIYdLBeJkwrE2dlmkIw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=AKCkwbYOD4/uQws2ONVIU/zjT7/t46WzLXTs9cm4HkA=; b=ITUfUQKRH4yLTXOcCocSM66yQ/tbs4PQ0/JpmL4a+k+/rWE56KTMVKe7/mc8cnUgro uawiOuo4AkagmmL5PyQdH47Udt+hZ+0nkRygyeat2VxMKl1LL9iDPi3zIFAHcfrfANe1 8UdjxxfUIS33CsFSd8fP41YChir+Jslo5l0pWL5lN/yi5ev4DBhudH7poJA7zcsbDVnQ NepXTqqcyoFpcRTPZw5kbVcLWgWeTH/SN3NQX2KA/DwhHjmc6TitRVWnfJ+AIrIL/+xp Lf7hYXq1QU7adHJucWCdq9zf8je9dXSMAJm7/BIcll5Ygu5xqtG8IYsHjeRpiHbOpAso 6DfA== X-Gm-Message-State: AJaThX6W0nRdznx/LAw4Lc/vgMtEval8qn1nJ2BI+QjXGZimkc6L7UCo FmXpC8+s7Biu21HqIy7q24E5rA== X-Google-Smtp-Source: AGs4zMZ8T2WRreE2VqlGa4DBXnmF2dWqxg8Ko9XdC/ShnqK/zOfCI6rS7LVVr6YgZ981awiyIlxaJA== X-Received: by 10.80.245.102 with SMTP id w35mr38093815edm.179.1511506604471; Thu, 23 Nov 2017 22:56:44 -0800 (PST) Received: from leoy-linaro (li1530-42.members.linode.com. [139.162.245.42]) by smtp.gmail.com with ESMTPSA id f36sm5755344edd.82.2017.11.23.22.56.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 23 Nov 2017 22:56:42 -0800 (PST) Date: Fri, 24 Nov 2017 14:56:23 +0800 From: Leo Yan To: Sudeep Holla Subject: Re: [PATCH] arm64: dts: Hi3660: Fix state id for 'CPU_NAP' state Message-ID: <20171124065623.GD13184@leoy-linaro> References: <1511415614-9859-1-git-send-email-leo.yan@linaro.org> <17639ecc-8ff8-e118-f6e1-51e2cfe4342b@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <17639ecc-8ff8-e118-f6e1-51e2cfe4342b@arm.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171124_065706_672335_28C5ADD8 X-CRM114-Status: GOOD ( 35.00 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree@vger.kernel.org, Vincent Guittot , Daniel Lezcano , linux-kernel@vger.kernel.org, Wei Xu , Rob Herring , linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Sudeep, On Thu, Nov 23, 2017 at 02:03:51PM +0000, Sudeep Holla wrote: > Hi Daniel, > > Thanks a lot for pointing me to this and having some useful discussion > in private. That helped to dig a bit further on this. > > On 23/11/17 05:40, 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. > > > > This commit changes psci parameter to '0x00000000' for state id = 0; > > this id is accepted by ARM trusted firmware and finally CPU can stay > > properly in 'CPU_NAP' state. > > > > I would like to conditionally NACK this patch. If we can't update the > ARM TF at all then, I will agree with this change reluctantly. Thanks for reviewing. Just like Daniel said, we need to figure out the right method for this. So suggestions are very welcome! > This looks like an artifact of copy paste in ARM TF port for this > platform. If you look as PSCI specification, CPU suspend parameter has > some recommendations and it's good to follow then unless you have strong > reasons not to. > > As Daniel pointed to me, this patch is required to satisfy TF > particularly [1]. Now that looks like copy pasted from Juno or FVP port > and if you look deeper, it's clearly under !ARM_RECOM_STATE_ID_ENC [2] > which was not copied IIUC :). Thanks for sharing pointers. It's shame that the copying is not correct for Hikey960 :) Come back to recommended state id, I reviewed Juno board defintion and I found it's not align with PSCI spec defintion, in ARM-TF Juno code defines state as below [1]: #define ARM_LOCAL_STATE_RUN 0 #define ARM_LOCAL_STATE_RET 1 #define ARM_LOCAL_STATE_OFF 2 In PSCI spec chapter "6.5 Recommended StateID Encoding" recommends power state id as below: 0: Run 1: Standby 2: Retention 3: Powerdown So could you confirm on Hikey960 we should follow PSCI definition for state id definition? > Juno's implementation is legacy as these recommendations were added > later in the specification while Juno is 3 year old platform now. > > Though strictly speaking it's not violation of the PSCI specification, > but I would rather get this fixed not before it's too late and copied to > the next generation of platforms. Since the firmware can be easily > upgraded that shouldn't be that difficult. If completely compliant with PSCI recommended state id, we need change both for ARM-TF and kernel for this. In ARM-TF, I have sent PR [2]. For the kernel patch, we should change state id as below. Please let me know if you have suggestion for this. [1] https://github.com/ARM-software/arm-trusted-firmware/blob/master/include/plat/arm/common/arm_def.h#L43 [2] https://github.com/ARM-software/arm-trusted-firmware/pull/1171/commits/13d30c1c33609eb6cffadd50954e4301d2cab909 Thanks, Leo Yan diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi index 12544c3..812437a 100644 --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi @@ -179,7 +179,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>; @@ -188,7 +188,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>; @@ -197,7 +197,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>; @@ -206,7 +206,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>;