From patchwork Fri May 27 19:30:17 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 825302 Received: from bombadil.infradead.org (173-166-109-252-newengland.hfc.comcastbusiness.net [173.166.109.252]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p4RJrIk1011769 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 27 May 2011 19:53:45 GMT Received: from canuck.infradead.org ([2001:4978:20e::1]) by bombadil.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QQ2jv-0002Kv-Ll; Fri, 27 May 2011 19:30:31 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QQ2js-0006ba-Kf; Fri, 27 May 2011 19:30:28 +0000 Received: from wolverine01.qualcomm.com ([199.106.114.254]) by canuck.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QQ2jk-0006aq-Aq for linux-arm-kernel@lists.infradead.org; Fri, 27 May 2011 19:30:21 +0000 X-IronPort-AV: E=McAfee;i="5400,1158,6358"; a="94154121" Received: from pdmz-css-vrrp.qualcomm.com (HELO mostmsg01.qualcomm.com) ([199.106.114.130]) by wolverine01.qualcomm.com with ESMTP/TLS/ADH-AES256-SHA; 27 May 2011 12:30:17 -0700 Received: from [10.46.164.20] (pdmz-snip-v218.qualcomm.com [192.168.218.1]) by mostmsg01.qualcomm.com (Postfix) with ESMTPA id 0560A10004D7; Fri, 27 May 2011 12:29:58 -0700 (PDT) Message-ID: <4DDFFBC9.30002@codeaurora.org> Date: Fri, 27 May 2011 12:30:17 -0700 From: Stephen Boyd User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9 MIME-Version: 1.0 To: Marc Zyngier Subject: Re: [RFC PATCH v4 09/13] ARM: msm: use remapped PPI interrupts for local timer References: <1306513671-12206-1-git-send-email-marc.zyngier@arm.com> <1306513671-12206-10-git-send-email-marc.zyngier@arm.com> In-Reply-To: <1306513671-12206-10-git-send-email-marc.zyngier@arm.com> X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110527_153020_640492_D7C58523 X-CRM114-Status: GOOD ( 22.45 ) X-Spam-Score: -2.3 (--) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (-2.3 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [199.106.114.254 listed in list.dnswl.org] Cc: Daniel Walker , David Brown , Bryan Huntsman , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Fri, 27 May 2011 19:54:02 +0000 (UTC) On 05/27/2011 09:27 AM, Marc Zyngier wrote: > Use the normal interrupt scheme for the local timers by using > a remapped PPI interrupt. > > MSM already had a very similar scheme, though still mixing both > GIC-specific and generic APIs. > > Fixes and ideas courtesy of Stephen Boyd. > > Cc: David Brown > Cc: Daniel Walker > Cc: Bryan Huntsman Reviewed-and-Tested-by: Stephen Boyd > > +static bool local_timer_inited; Except this is unused if SMP=n. Here's a fix: ---8<---- --- But now I really want to know. Why can't we use system_state == SYSTEM_BOOTING? Perhaps I'm thinking ahead too much, but I'm concerned that if we have more than two cores we're going to have to make a percpu variable here just to avoid requesting the irq again. It would be simpler to just accept the fact that we can't boot a secondary core for the first time after kernel init due to the way the code is written. If/when we decide to allow such a thing we can revisit this code and make it into a percpu variable. Or another idea is to pass a "first_boot" flag or something to local_timer_setup() to indicate this is the first boot. Then we could extend the percpu_clockevent variable in the localtimer core to have this flag. Either way it seems like something we should centralize. diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c index 186abdf..a242b89 100644 --- a/arch/arm/mach-msm/timer.c +++ b/arch/arm/mach-msm/timer.c @@ -84,7 +84,6 @@ enum { static struct msm_clock msm_clocks[]; -static bool local_timer_inited; static irqreturn_t msm_timer_interrupt(int irq, void *dev_id) { @@ -259,6 +258,7 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt) { struct msm_clock *clock = &msm_clocks[MSM_GLOBAL_TIMER]; int res; + static bool local_timer_inited; /* Use existing clock_event for cpu 0 */ if (!smp_processor_id())