From mboxrd@z Thu Jan 1 00:00:00 1970 From: Catalin Marinas Subject: Re: [PATCH v7 1/8] arm64: kernel: refactor the CPU suspend API for retention states Date: Mon, 18 Aug 2014 15:20:30 +0100 Message-ID: <20140818142030.GB20043@localhost> References: <1407945127-27554-1-git-send-email-lorenzo.pieralisi@arm.com> <1407945127-27554-2-git-send-email-lorenzo.pieralisi@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1407945127-27554-2-git-send-email-lorenzo.pieralisi@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Lorenzo Pieralisi Cc: Mark Rutland , Tomasz Figa , Lina Iyer , Chander Kashyap , Vincent Guittot , Nicolas Pitre , Daniel Lezcano , "linux-arm-kernel@lists.infradead.org" , "grant.likely@linaro.org" , Charles Garcia-Tobin , "devicetree@vger.kernel.org" , Kevin Hilman , "linux-pm@vger.kernel.org" , Sebastian Capella , Mark Brown , Antti Miettinen , Paul Walmsley , Geoff Levand , Peter De Schrijver , Stephen Boyd , Amit Kucheria List-Id: devicetree@vger.kernel.org On Wed, Aug 13, 2014 at 04:52:00PM +0100, Lorenzo Pieralisi wrote: > CPU suspend is the standard kernel interface to be used to enter > low-power states on ARM64 systems. Current cpu_suspend implementation > by default assumes that all low power states are losing the CPU context, > so the CPU registers must be saved and cleaned to DRAM upon state > entry. Furthermore, the current cpu_suspend() implementation assumes > that if the CPU suspend back-end method returns when called, this has > to be considered an error regardless of the return code (which can be > successful) since the CPU was not expected to return from a code path that > is different from cpu_resume code path - eg returning from the reset vector. > > All in all this means that the current API does not cope well with low-power > states that preserve the CPU context when entered (ie retention states), > since first of all the context is saved for nothing on state entry for > those states and a successful state entry can return as a normal function > return, which is considered an error by the current CPU suspend > implementation. > > This patch refactors the cpu_suspend() API so that it can be split in > two separate functionalities. The arm64 cpu_suspend API just provides > a wrapper around CPU suspend operation hook. A new function is > introduced (for architecture code use only) for states that require > context saving upon entry: > > __cpu_suspend(unsigned long arg, int (*fn)(unsigned long)) > > __cpu_suspend() saves the context on function entry and calls the > so called suspend finisher (ie fn) to complete the suspend operation. > The finisher is not expected to return, unless it fails in which case > the error is propagated back to the __cpu_suspend caller. > > The API refactoring results in the following pseudo code call sequence for a > suspending CPU, when triggered from a kernel subsystem: > > /* > * int cpu_suspend(unsigned long idx) > * @idx: idle state index > */ > { > -> cpu_suspend(idx) > |---> CPU operations suspend hook called, if present > |--> if (retention_state) > |--> direct suspend back-end call (eg PSCI suspend) > else > |--> __cpu_suspend(idx, &back_end_finisher); > } > > By refactoring the cpu_suspend API this way, the CPU operations back-end > has a chance to detect whether idle states require state saving or not > and can call the required suspend operations accordingly either through > simple function call or indirectly through __cpu_suspend() which carries out > state saving and suspend finisher dispatching to complete idle state entry. > > Signed-off-by: Lorenzo Pieralisi > --- > arch/arm64/include/asm/suspend.h | 1 + > arch/arm64/kernel/sleep.S | 47 +++++++++++++++++++++++++++++----------- > arch/arm64/kernel/suspend.c | 47 ++++++++++++++++++++++++---------------- > 3 files changed, 63 insertions(+), 32 deletions(-) Reviewed-by: Catalin Marinas From mboxrd@z Thu Jan 1 00:00:00 1970 From: catalin.marinas@arm.com (Catalin Marinas) Date: Mon, 18 Aug 2014 15:20:30 +0100 Subject: [PATCH v7 1/8] arm64: kernel: refactor the CPU suspend API for retention states In-Reply-To: <1407945127-27554-2-git-send-email-lorenzo.pieralisi@arm.com> References: <1407945127-27554-1-git-send-email-lorenzo.pieralisi@arm.com> <1407945127-27554-2-git-send-email-lorenzo.pieralisi@arm.com> Message-ID: <20140818142030.GB20043@localhost> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Aug 13, 2014 at 04:52:00PM +0100, Lorenzo Pieralisi wrote: > CPU suspend is the standard kernel interface to be used to enter > low-power states on ARM64 systems. Current cpu_suspend implementation > by default assumes that all low power states are losing the CPU context, > so the CPU registers must be saved and cleaned to DRAM upon state > entry. Furthermore, the current cpu_suspend() implementation assumes > that if the CPU suspend back-end method returns when called, this has > to be considered an error regardless of the return code (which can be > successful) since the CPU was not expected to return from a code path that > is different from cpu_resume code path - eg returning from the reset vector. > > All in all this means that the current API does not cope well with low-power > states that preserve the CPU context when entered (ie retention states), > since first of all the context is saved for nothing on state entry for > those states and a successful state entry can return as a normal function > return, which is considered an error by the current CPU suspend > implementation. > > This patch refactors the cpu_suspend() API so that it can be split in > two separate functionalities. The arm64 cpu_suspend API just provides > a wrapper around CPU suspend operation hook. A new function is > introduced (for architecture code use only) for states that require > context saving upon entry: > > __cpu_suspend(unsigned long arg, int (*fn)(unsigned long)) > > __cpu_suspend() saves the context on function entry and calls the > so called suspend finisher (ie fn) to complete the suspend operation. > The finisher is not expected to return, unless it fails in which case > the error is propagated back to the __cpu_suspend caller. > > The API refactoring results in the following pseudo code call sequence for a > suspending CPU, when triggered from a kernel subsystem: > > /* > * int cpu_suspend(unsigned long idx) > * @idx: idle state index > */ > { > -> cpu_suspend(idx) > |---> CPU operations suspend hook called, if present > |--> if (retention_state) > |--> direct suspend back-end call (eg PSCI suspend) > else > |--> __cpu_suspend(idx, &back_end_finisher); > } > > By refactoring the cpu_suspend API this way, the CPU operations back-end > has a chance to detect whether idle states require state saving or not > and can call the required suspend operations accordingly either through > simple function call or indirectly through __cpu_suspend() which carries out > state saving and suspend finisher dispatching to complete idle state entry. > > Signed-off-by: Lorenzo Pieralisi > --- > arch/arm64/include/asm/suspend.h | 1 + > arch/arm64/kernel/sleep.S | 47 +++++++++++++++++++++++++++++----------- > arch/arm64/kernel/suspend.c | 47 ++++++++++++++++++++++++---------------- > 3 files changed, 63 insertions(+), 32 deletions(-) Reviewed-by: Catalin Marinas