Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754205AbcCXQDm (ORCPT ); Thu, 24 Mar 2016 12:03:42 -0400 Received: from foss.arm.com ([217.140.101.70]:52253 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751122AbcCXQDc (ORCPT ); Thu, 24 Mar 2016 12:03:32 -0400 Date: Thu, 24 Mar 2016 16:06:00 +0000 From: Lorenzo Pieralisi To: Jisheng Zhang Cc: rjw@rjwysocki.net, daniel.lezcano@linaro.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] cpuidle: arm: make enter idle operation a bit more efficient Message-ID: <20160324160600.GA21749@red-moon> References: <1458796038-6062-1-git-send-email-jszhang@marvell.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1458796038-6062-1-git-send-email-jszhang@marvell.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1916 Lines: 65 On Thu, Mar 24, 2016 at 01:07:18PM +0800, Jisheng Zhang wrote: > Currently, entering idle need to check the idx every time to choose the > real entering idle routine. But this check could be avoided by pointing > the idle enter function pointer of each idle states to the routines > suitable for each states directly. > > Signed-off-by: Jisheng Zhang > --- > drivers/cpuidle/cpuidle-arm.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c > index 545069d..48a620f 100644 > --- a/drivers/cpuidle/cpuidle-arm.c > +++ b/drivers/cpuidle/cpuidle-arm.c > @@ -23,6 +23,13 @@ > > #include "dt_idle_states.h" > > +static int arm_enter_wfi_state(struct cpuidle_device *dev, > + struct cpuidle_driver *drv, int idx) > +{ > + cpu_do_idle(); > + return 0; > +} > + > /* > * arm_enter_idle_state - Programs CPU to enter the specified state > * > @@ -38,11 +45,6 @@ static int arm_enter_idle_state(struct cpuidle_device *dev, > { > int ret; > > - if (!idx) { > - cpu_do_idle(); > - return idx; > - } Mmm...if I wanted to paint your bikeshed I would say idx is in a register and you are removing a simple comparison to exchange it with a function that adds to code footprint and may even make performance worse instead of improving anything. I am not sure this patch makes anything more efficient, happy to be proven wrong, with significant data. Thanks, Lorenzo > - > ret = cpu_pm_enter(); > if (!ret) { > /* > @@ -69,7 +71,7 @@ static struct cpuidle_driver arm_idle_driver = { > * handler for idle state index 0. > */ > .states[0] = { > - .enter = arm_enter_idle_state, > + .enter = arm_enter_wfi_state, > .exit_latency = 1, > .target_residency = 1, > .power_usage = UINT_MAX, > -- > 2.8.0.rc3 >