From patchwork Sat Oct 3 08:24:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King X-Patchwork-Id: 7320461 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 546089F32B for ; Sat, 3 Oct 2015 08:24:53 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 73278208B2 for ; Sat, 3 Oct 2015 08:24:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 41F8B208AF for ; Sat, 3 Oct 2015 08:24:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751961AbbJCIYs (ORCPT ); Sat, 3 Oct 2015 04:24:48 -0400 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:35999 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751911AbbJCIYp (ORCPT ); Sat, 3 Oct 2015 04:24:45 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora-2014; h=Date:Sender:Message-Id:Content-Type:Content-Transfer-Encoding:MIME-Version:Subject:Cc:To:From; bh=AKhgCBeLO25tV+BnhLmya4jHqr0HjLcrM5pCF2/oryQ=; b=KMuGdzHVCWJWLHFzl9dGqsBNy7y+FIbKiVfj3rkkyemGGrInpJJQSbngnIUNwJXis9R7cJZraSs9+Nrn5kroH8Oss+2SPG7MJeGSGmSlcZrkCdLXC/HILJPB0R+7QNkS0VqMweL78yRW6it+heJ5jcpm6PKlHaCR5auUxinoNdk=; Received: from e0022681537dd.dyn.arm.linux.org.uk ([fd8f:7570:feb6:1:222:68ff:fe15:37dd]:44180 helo=rmk-PC.arm.linux.org.uk) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1ZiI7U-0002LF-Jv; Sat, 03 Oct 2015 09:24:40 +0100 Received: from rmk by rmk-PC.arm.linux.org.uk with local (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1ZiI7T-0002zo-Ez; Sat, 03 Oct 2015 09:24:39 +0100 From: Russell King To: Gregory CLEMENT , "Rafael J. Wysocki" Cc: Daniel Lezcano , linux-pm@vger.kernel.org Subject: [RFC PATCH] cpuidle: mvebu: indicate failure to enter deeper sleep states MIME-Version: 1.0 Content-Disposition: inline Message-Id: Date: Sat, 03 Oct 2015 09:24:39 +0100 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The cpuidle ->enter method expects the return value to be the sleep state we entered. Returning negative numbers or other codes is not permissible since coupled CPU idle was merged. At least some of the mvebu_v7_cpu_suspend() implementations return the value from cpu_suspend(), which returns zero if the CPU vectors back into the kernel via cpu_resume() (the success case), or the non-zero return value of the suspend actor, or one (failure cases). We do not want to be returning the failure case value back to CPU idle as that indicates that we successfully entered one of the deeper idle states. Always return zero instead, indicating that we slept for the shortest amount of time. Signed-off-by: Russell King --- Please think about this - having only an Armada 388 platform, I can't test this due to CPU idle being disabled there. However, the code paths indicate that if we fail to sleep in armada_370_xp_pmsu_idle_enter() or armada_38x_do_cpu_suspend() fail to sleep, we end up reporting that we successfully slept in CPU idle state 1 in both cases. Rafael - the code in cpuidle.c prior to the addition of coupled CPU idle seems to allow the return of negative error numbers from the ->enter() method: if (entered_state >= 0) { /* Update cpuidle counters */ /* This can be moved to within driver enter routine * but that results in multiple copies of same code. */ dev->states_usage[entered_state].time += dev->last_residency; dev->states_usage[entered_state].usage++; } else { dev->last_residency = 0; } Since coupled CPU idle, negative return codes will give a negative table index inside cpuidle_state_is_coupled() - which is obviously buggy. Does this need fixing? If so, this patch below is wrong, and we should be returning a negative errno rather than zero on failure to enter a low power state. drivers/cpuidle/cpuidle-mvebu-v7.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c index 01a856971f05..18ded9e7cb34 100644 --- a/drivers/cpuidle/cpuidle-mvebu-v7.c +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c @@ -39,8 +39,12 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev, ret = mvebu_v7_cpu_suspend(deepidle); cpu_pm_exit(); + /* + * If we failed to enter the desired state, indicate that we + * slept lightly. + */ if (ret) - return ret; + return 0; return index; }