From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754742AbbBTOEj (ORCPT ); Fri, 20 Feb 2015 09:04:39 -0500 Received: from mail-wg0-f47.google.com ([74.125.82.47]:39244 "EHLO mail-wg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754193AbbBTOEi (ORCPT ); Fri, 20 Feb 2015 09:04:38 -0500 Date: Fri, 20 Feb 2015 15:04:32 +0100 From: Ingo Molnar To: Viresh Kumar Cc: Peter Zijlstra , Thomas Gleixner , Ingo Molnar , Linaro Kernel Mailman List , Kevin Hilman , Preeti U Murthy , Daniel Lezcano , Linux Kernel Mailing List , "linux-arm-kernel@lists.infradead.org" , Frederic Weisbecker , Linaro Networking , Steven Miao , Mark Salter , Michal Simek , Ralf Baechle , Ley Foon Tan , Jonas Bonn , "David S. Miller" , Jeff Dike , Guan Xuetao Subject: Re: [PATCH] clockevents: Add (missing) default case for switch blocks Message-ID: <20150220140432.GA31928@gmail.com> References: <6291e308ab77a480c6b1732e16108c5fe6f66afa.1424412815.git.viresh.kumar@linaro.org> <20150220083842.GA20387@gmail.com> <20150220084807.GJ21418@twins.programming.kicks-ass.net> <20150220093659.GA23469@gmail.com> <20150220113753.GP5029@twins.programming.kicks-ass.net> <20150220114136.GA27483@gmail.com> <20150220132220.GB28882@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Viresh Kumar wrote: > On 20 February 2015 at 18:52, Ingo Molnar wrote: > > > > * Viresh Kumar wrote: > >> + CLOCK_EVT_DEV_MODE_UNUSED = 0, > > > > What is 'unused' - not initialized yet? > > Unused. Initially all clockevent devices are supposed to > be in this mode but later if another device replaces an > existing one, the existing one is put into this mode. I'd suggest to rename it to MODE_INIT - at first glance it gave me the impression that it's some sort of API placeholder - i.e. an unused flag or so. Also, I'd suggest to rename all 'modes' to true state machine naming: STATE_INITIALIZED, STATE_SHUT_DOWN, STATE_PERIODIC, STATE_RESUMED, etc.: if these are enums for states and not state transition names, see my later questions: > >> + CLOCK_EVT_DEV_MODE_SHUTDOWN, > >> + CLOCK_EVT_DEV_MODE_PERIODIC, > >> + CLOCK_EVT_DEV_MODE_ONESHOT, > >> + CLOCK_EVT_DEV_MODE_RESUME, > > > > What is 'resume' mode? > > Introduced with: 18de5bc4c1f1 ("clockevents: fix resume > logic") and is only called during system resume to resume > the clockevent devices before resuming the tick. Only few > implementations do meaningful stuff here. So is it a state that a clockevents device reaches, or a state transition? The two purposes seem to be mixed up in the nomenclature. > >> + CLOCK_EVT_DEV_MODE_ONESHOT_STOPPED, /* This would be the new > >> mode which I will add later */ > > > > What does this mode express? > > I have added it here to show how things would look like > eventually, but it wouldn't be present in the patch which > splits the enum into two parts.. Yeah. > Its only important for NOHZ_FULL (IDLE ? Maybe). When we > decide that the tick (LOWRES) or hrtimer interrupt > (HIGHRES) isn't required for indefinite period of time > (i.e. no timers/hrtimers are present to serve), we skip > reprogramming the clockevent device. But its already > reprogrammed from the tick-handler and so will fire > atleast once again. So this new 'mode' appears to be a true state of the device? Thanks, Ingo