From patchwork Tue May 21 08:41:19 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chen Gang X-Patchwork-Id: 2598011 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) by patchwork2.kernel.org (Postfix) with ESMTP id DDB45DFB79 for ; Tue, 21 May 2013 16:04:38 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uemdm-00041a-I3; Tue, 21 May 2013 13:30:12 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uei9T-0003h7-Fz; Tue, 21 May 2013 08:42:35 +0000 Received: from intranet.asianux.com ([58.214.24.6]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uei9P-0003g5-GH for linux-arm-kernel@lists.infradead.org; Tue, 21 May 2013 08:42:32 +0000 Received: by intranet.asianux.com (Postfix, from userid 103) id 266F2184026F; Tue, 21 May 2013 16:42:09 +0800 (CST) X-Spam-Score: -100.8 X-Spam-Checker-Version: SpamAssassin 3.1.9 (2007-02-13) on intranet.asianux.com X-Spam-Level: X-Spam-Status: No, score=-100.8 required=5.0 tests=AWL,BAYES_00, RATWARE_GECKO_BUILD,USER_IN_WHITELIST autolearn=no version=3.1.9 Received: from [10.1.0.143] (unknown [219.143.36.82]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by intranet.asianux.com (Postfix) with ESMTP id 93718184024F; Tue, 21 May 2013 16:42:08 +0800 (CST) Message-ID: <519B332F.1000404@asianux.com> Date: Tue, 21 May 2013 16:41:19 +0800 From: Chen Gang User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Marc Zyngier Subject: Re: [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' References: <5199C725.8050102@asianux.com> <168084c28e8ba5124c05553e97463172@localhost> <20130520095606.GH31359@mudshark.cambridge.arm.com> <519AF2DC.2040200@asianux.com> <5bf8a8dd20ecd01c14f0ed9aa054f2df@localhost> In-Reply-To: <5bf8a8dd20ecd01c14f0ed9aa054f2df@localhost> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130521_044232_086375_6E7213F7 X-CRM114-Status: GOOD ( 28.64 ) X-Spam-Score: -3.0 (---) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-3.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.1 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Tony Lindgren , Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, Santosh Shilimkar , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On 05/21/2013 02:13 PM, Marc Zyngier wrote: > On Tue, 21 May 2013 12:06:52 +0800, Chen Gang > wrote: >> On 05/20/2013 05:56 PM, Will Deacon wrote: >>> On Mon, May 20, 2013 at 08:15:04AM +0100, Marc Zyngier wrote: >>>> On Mon, 20 May 2013 14:48:05 +0800, Chen Gang >>>> wrote: >>>>> Need 'EXPORT_SYMBOL_GPL(read_current_timer)' if build with >>>>> allmodconfig. >>>>> >>>>> The related error: >>>>> ERROR: "read_current_timer" [lib/rbtree_test.ko] undefined! >>>>> ERROR: "read_current_timer" [lib/interval_tree_test.ko] undefined! >>>>> ERROR: "read_current_timer" [fs/ext4/ext4.ko] undefined! >>>>> ERROR: "read_current_timer" [crypto/tcrypt.ko] undefined! >>>>> >>>>> >>>>> Signed-off-by: Chen Gang >>>>> --- >>>>> arch/arm64/kernel/time.c | 1 + >>>>> 1 files changed, 1 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c >>>>> index a551f88..7fcba80 100644 >>>>> --- a/arch/arm64/kernel/time.c >>>>> +++ b/arch/arm64/kernel/time.c >>>>> @@ -73,6 +73,7 @@ int read_current_timer(unsigned long *timer_value) >>>>> *timer_value = arch_timer_read_counter(); >>>>> return 0; >>>>> } >>>>> +EXPORT_SYMBOL_GPL(read_current_timer); >>>>> >>>>> void __init time_init(void) >>>>> { >>>> >>>> While this solves the problem, I'm not sure this is the best fix. The >>>> real >>>> issue is with get_cycles, which is a macro around read_current_timer. >>>> >>>> AArch32 exports it because of the number of timer implementations. On >>>> arm64, we should be able to just return CNTVCT_EL0. >>>> >>>> Catalin, Will, what do you think? >>> >>> Should be ok once the arch timer driver has moved exclusively to > virtual >>> time. I'm also not sure we even need to implement read_current_timer() > -- >>> it's only used for delay-loop calibration, which we don't need for the >>> arch timer. >>> >> >> For whether we need implement read_current_timer(): >> >> many platforms have implemented it (openrisc, arm, sparc, hexagon, >> avr32, x86). >> it is called by init/calibrate.c when 'ARCH_HAS_READ_CURRENT_TIMER' is >> defined. >> since arm64 can implement it, better to provide it as an architect >> features to let outside use. > > Nobody disputes the interest of read_current_timer. > It is for Will said "I'm also not sure we even need to implement read_current_timer()". I think we still need it. >> For the implementation of read_current_timer(): >> >> it has to face various configurations >> (e.g. CONFIG_ARM_ARCH_TIMER, arch_timer_read_zero, >> arch_counter_get_cntvct, arch_counter_get_cntpct) >> so better still use variable instead of. >> (excuse me, I do not know what is 'CNTVCT_EL0', is it like a > constant >> number ?) > > Architected timer is mandatory on arm64, so we can always rely on it it be > present. CNTVCT_EL0 is the system register accessing the Virtual Counter, > which is basically what read_current_timer() returns. > OK, thanks. for CNTVCT_EL0, can we use arch_counter_get_cntvct() which is defined in "arch/arm64/include/asm/arch_timer.h" ? >> For the implementation of get_cycles() >> >> if read_current_timer() is provided, >> better to let get_cycles() to call it, instead of implement once > again. > > There is certainly some value in reusing existing code, but in this > particular case we can simply inline two instructions (isb + mrs > cntvct_el0), and I'm not even completely sure about the isb. > OK, thanks. So, how about the fix below :) ------------------------diff begin------------------------------- ------------------------diff end--------------------------------- diff --git a/arch/arm64/include/asm/timex.h b/arch/arm64/include/asm/timex.h index b24a31a..768ba44 100644 --- a/arch/arm64/include/asm/timex.h +++ b/arch/arm64/include/asm/timex.h @@ -16,11 +16,13 @@ #ifndef __ASM_TIMEX_H #define __ASM_TIMEX_H +#include + /* * Use the current timer as a cycle counter since this is what we use for * the delay loop. */ -#define get_cycles() ({ cycles_t c; read_current_timer(&c); c; }) +#define get_cycles() arch_counter_get_cntvct() #include diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c index a551f88..6d7ce08 100644 --- a/arch/arm64/kernel/time.c +++ b/arch/arm64/kernel/time.c @@ -38,6 +38,7 @@ #include #include +#include #ifdef CONFIG_SMP unsigned long profile_pc(struct pt_regs *regs) @@ -70,7 +71,7 @@ unsigned long long notrace sched_clock(void) int read_current_timer(unsigned long *timer_value) { - *timer_value = arch_timer_read_counter(); + *timer_value = arch_counter_get_cntvct(); return 0; }