Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754372Ab3FRIYa (ORCPT ); Tue, 18 Jun 2013 04:24:30 -0400 Received: from mail-bk0-f50.google.com ([209.85.214.50]:52258 "EHLO mail-bk0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754458Ab3FRIYW (ORCPT ); Tue, 18 Jun 2013 04:24:22 -0400 Message-ID: <51C0193A.40607@linaro.org> Date: Tue, 18 Jun 2013 10:24:26 +0200 From: Daniel Lezcano User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 MIME-Version: 1.0 To: Magnus Damm CC: linux-kernel , Mark Rutland , SH-Linux , Marc Zyngier , Catalin Marinas , "Simon Horman [Horms]" , John Stultz , Shinya Kuribayashi , Thomas Gleixner Subject: Re: [PATCH/RFC] clockevents: Ignore C3STOP when CPUIdle is disabled References: <20130618071756.22598.51046.sendpatchset@w520> <51C00D08.3080406@linaro.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3187 Lines: 80 On 06/18/2013 09:39 AM, Magnus Damm wrote: > On Tue, Jun 18, 2013 at 4:32 PM, Daniel Lezcano > wrote: >> On 06/18/2013 09:17 AM, Magnus Damm wrote: >>> From: Magnus Damm >>> >>> Introduce the function tick_device_may_c3stop() that >>> ignores the C3STOP flag in case CPUIdle is disabled. >>> >>> The C3STOP flag tells the system that a clock event >>> device may be stopped during deep sleep, but if this >>> will happen or not depends on things like if CPUIdle >>> is enabled and if a CPUIdle driver is available. >>> >>> This patch assumes that if CPUIdle is disabled then >>> the sleep mode triggering C3STOP will never be entered. >>> So by ignoring C3STOP when CPUIdle is disabled then it >>> becomes possible to use high resolution timers with only >>> per-cpu local timers - regardless if they have the >>> C3STOP flag set or not. >>> >>> Observed on the r8a73a4 SoC that at this point only uses >>> ARM architected timers for clock event and clock sources. >>> >>> Without this patch high resolution timers are run time >>> disabled on the r8a73a4 SoC - this regardless of CPUIdle >>> is disabled or not. >>> >>> The less short term fix is to add support for more timers >>> on the r8a73a4 SoC, but until CPUIdle support is enabled >>> it must be possible to use high resoultion timers without >>> additional timers. >>> >>> I'd like to hear some feedback and also test this on more >>> systems before merging the code, see the non-SOB below. >> >> Do we need a broadcast timer when cpuidle is not compiled in the kernel ? > > Yes, if there is no per-cpu timer available. It depends on what the > SMP support code for a particular SoC or architecture happen to > enable. Ok thanks for the information. There is here a multiple occurrence of the information "the timer will stop when power is saved": CLOCK_EVT_FEAT_C3STOP and CPUIDLE_FLAG_TIMER_STOP, so I am wondering if some code simplification couldn't be done before your patch. The function: tick_broadcast_oneshot_control is called from clockevents_notify. This one is called from the cpuidle framework or the back-end cpuidle driver. The caller knows the timer will be stop and this is why it is switching to the broadcast mode. But we have a sanity check in tick_broadcast_oneshot_control function: if (!(dev->features & CLOCK_EVT_FEAT_C3STOP)) return; In other words, CPUIDLE_FLAG_TIMER_STOP will tell the framework to call clockevents_notify and the tick broadcast code will re-check the device will effectively go down. IMHO, we can get rid of this check. The same happens for the tick_do_broadcast_on_off function. That reduces the number of C3STOP usage. -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/