* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' @ 2013-05-20 6:48 Chen Gang 2013-05-20 7:15 ` Marc Zyngier 0 siblings, 1 reply; 17+ messages in thread From: Chen Gang @ 2013-05-20 6:48 UTC (permalink / raw) To: linux-arm-kernel 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 <gang.chen@asianux.com> --- 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) { -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-20 6:48 [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' Chen Gang @ 2013-05-20 7:15 ` Marc Zyngier 2013-05-20 9:56 ` Will Deacon 0 siblings, 1 reply; 17+ messages in thread From: Marc Zyngier @ 2013-05-20 7:15 UTC (permalink / raw) To: linux-arm-kernel On Mon, 20 May 2013 14:48:05 +0800, Chen Gang <gang.chen@asianux.com> 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 <gang.chen@asianux.com> > --- > 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? M. -- Fast, cheap, reliable. Pick two. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-20 7:15 ` Marc Zyngier @ 2013-05-20 9:56 ` Will Deacon 2013-05-21 4:06 ` Chen Gang 0 siblings, 1 reply; 17+ messages in thread From: Will Deacon @ 2013-05-20 9:56 UTC (permalink / raw) To: linux-arm-kernel On Mon, May 20, 2013 at 08:15:04AM +0100, Marc Zyngier wrote: > On Mon, 20 May 2013 14:48:05 +0800, Chen Gang <gang.chen@asianux.com> > 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 <gang.chen@asianux.com> > > --- > > 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. Will ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-20 9:56 ` Will Deacon @ 2013-05-21 4:06 ` Chen Gang 2013-05-21 6:13 ` Marc Zyngier 2013-05-21 8:53 ` Will Deacon 0 siblings, 2 replies; 17+ messages in thread From: Chen Gang @ 2013-05-21 4:06 UTC (permalink / raw) To: linux-arm-kernel 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 <gang.chen@asianux.com> >> 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 <gang.chen@asianux.com> >>> --- >>> 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. 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 ?) 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. Thanks. -- Chen Gang Asianux Corporation ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-21 4:06 ` Chen Gang @ 2013-05-21 6:13 ` Marc Zyngier 2013-05-21 8:41 ` Chen Gang 2013-05-21 8:53 ` Will Deacon 1 sibling, 1 reply; 17+ messages in thread From: Marc Zyngier @ 2013-05-21 6:13 UTC (permalink / raw) To: linux-arm-kernel On Tue, 21 May 2013 12:06:52 +0800, Chen Gang <gang.chen@asianux.com> 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 <gang.chen@asianux.com> >>> 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 <gang.chen@asianux.com> >>>> --- >>>> 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. > 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. > 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. M. -- Fast, cheap, reliable. Pick two. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-21 6:13 ` Marc Zyngier @ 2013-05-21 8:41 ` Chen Gang 2013-05-21 8:58 ` Marc Zyngier 0 siblings, 1 reply; 17+ messages in thread From: Chen Gang @ 2013-05-21 8:41 UTC (permalink / raw) To: linux-arm-kernel On 05/21/2013 02:13 PM, Marc Zyngier wrote: > On Tue, 21 May 2013 12:06:52 +0800, Chen Gang <gang.chen@asianux.com> > 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 <gang.chen@asianux.com> >>>> 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 <gang.chen@asianux.com> >>>>> --- >>>>> 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 --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 <asm/arch_timer.h> + /* * 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 <asm-generic/timex.h> 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 <asm/thread_info.h> #include <asm/stacktrace.h> +#include <asm/arch_timer.h> #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; } ------------------------diff end--------------------------------- -- Chen Gang Asianux Corporation ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-21 8:41 ` Chen Gang @ 2013-05-21 8:58 ` Marc Zyngier 2013-05-21 9:26 ` Chen Gang 0 siblings, 1 reply; 17+ messages in thread From: Marc Zyngier @ 2013-05-21 8:58 UTC (permalink / raw) To: linux-arm-kernel On 21/05/13 09:41, Chen Gang wrote: > On 05/21/2013 02:13 PM, Marc Zyngier wrote: >> On Tue, 21 May 2013 12:06:52 +0800, Chen Gang <gang.chen@asianux.com> >> 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 <gang.chen@asianux.com> >>>>> 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 <gang.chen@asianux.com> >>>>>> --- >>>>>> 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. Not really. The only use of read_current_timer is for calibrating the delay loop, and we just do not need this, because we can actually rely on the timer to give us an accurate timing information. >>> 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" ? Yes. >>> 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 --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 <asm/arch_timer.h> > + > /* > * 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 <asm-generic/timex.h> > > 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 <asm/thread_info.h> > #include <asm/stacktrace.h> > +#include <asm/arch_timer.h> > > #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; > } > > > ------------------------diff end--------------------------------- I think you should try to implement Will's suggestion and drop read_current_timer (and the ARCH_HAS_READ_CURRENT_TIMER macro) altogether. It would be a much better fix. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-21 8:58 ` Marc Zyngier @ 2013-05-21 9:26 ` Chen Gang 0 siblings, 0 replies; 17+ messages in thread From: Chen Gang @ 2013-05-21 9:26 UTC (permalink / raw) To: linux-arm-kernel On 05/21/2013 04:58 PM, Marc Zyngier wrote: > I think you should try to implement Will's suggestion and drop > read_current_timer (and the ARCH_HAS_READ_CURRENT_TIMER macro) altogether. > > It would be a much better fix. OK, thanks. I will send patch v2. -- Chen Gang Asianux Corporation ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-21 4:06 ` Chen Gang 2013-05-21 6:13 ` Marc Zyngier @ 2013-05-21 8:53 ` Will Deacon 2013-05-21 9:27 ` Chen Gang 2013-05-21 9:46 ` [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer() Chen Gang 1 sibling, 2 replies; 17+ messages in thread From: Will Deacon @ 2013-05-21 8:53 UTC (permalink / raw) To: linux-arm-kernel On Tue, May 21, 2013 at 05:06:52AM +0100, Chen Gang wrote: > On 05/20/2013 05:56 PM, Will Deacon wrote: > > 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. No, that code is not needed on arm64 because we calibrate the delay loop statically using a known timer frequency. > 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 ?) cntvct_el0 is a system register, which provides the virtual counter value. > 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. You can implement it as a macro if you like, I'm just suggesting that we might not need read_current_timer after all. Will ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' 2013-05-21 8:53 ` Will Deacon @ 2013-05-21 9:27 ` Chen Gang 2013-05-21 9:46 ` [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer() Chen Gang 1 sibling, 0 replies; 17+ messages in thread From: Chen Gang @ 2013-05-21 9:27 UTC (permalink / raw) To: linux-arm-kernel On 05/21/2013 04:53 PM, Will Deacon wrote: > On Tue, May 21, 2013 at 05:06:52AM +0100, Chen Gang wrote: >> On 05/20/2013 05:56 PM, Will Deacon wrote: >>> 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. > > No, that code is not needed on arm64 because we calibrate the delay loop > statically using a known timer frequency. > >> 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 ?) > > cntvct_el0 is a system register, which provides the virtual counter value. > >> 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. > > You can implement it as a macro if you like, I'm just suggesting that we > might not need read_current_timer after all. > > Will > > Thanks, I should try patch v2. :-) -- Chen Gang Asianux Corporation ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer(). 2013-05-21 8:53 ` Will Deacon 2013-05-21 9:27 ` Chen Gang @ 2013-05-21 9:46 ` Chen Gang 2013-05-27 10:02 ` Chen Gang 1 sibling, 1 reply; 17+ messages in thread From: Chen Gang @ 2013-05-21 9:46 UTC (permalink / raw) To: linux-arm-kernel Under arm64, we will calibrate the delay loop statically using a known timer frequency, so delete read_current_timer(), or it will cause compiling issue 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 <gang.chen@asianux.com> --- arch/arm64/include/asm/timex.h | 6 +++--- arch/arm64/kernel/time.c | 6 ------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/arch/arm64/include/asm/timex.h b/arch/arm64/include/asm/timex.h index b24a31a..81a076e 100644 --- a/arch/arm64/include/asm/timex.h +++ b/arch/arm64/include/asm/timex.h @@ -16,14 +16,14 @@ #ifndef __ASM_TIMEX_H #define __ASM_TIMEX_H +#include <asm/arch_timer.h> + /* * 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 <asm-generic/timex.h> -#define ARCH_HAS_READ_CURRENT_TIMER - #endif diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c index a551f88..03dc371 100644 --- a/arch/arm64/kernel/time.c +++ b/arch/arm64/kernel/time.c @@ -68,12 +68,6 @@ unsigned long long notrace sched_clock(void) return arch_timer_read_counter() * sched_clock_mult; } -int read_current_timer(unsigned long *timer_value) -{ - *timer_value = arch_timer_read_counter(); - return 0; -} - void __init time_init(void) { u32 arch_timer_rate; -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer(). 2013-05-21 9:46 ` [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer() Chen Gang @ 2013-05-27 10:02 ` Chen Gang 2013-06-08 4:37 ` Chen Gang 0 siblings, 1 reply; 17+ messages in thread From: Chen Gang @ 2013-05-27 10:02 UTC (permalink / raw) To: linux-arm-kernel Hello Maintainers: Please help check this patch whether OK, when you have time. Thanks. On 05/21/2013 05:46 PM, Chen Gang wrote: > > Under arm64, we will calibrate the delay loop statically using a known > timer frequency, so delete read_current_timer(), or it will cause > compiling issue 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 <gang.chen@asianux.com> > --- > arch/arm64/include/asm/timex.h | 6 +++--- > arch/arm64/kernel/time.c | 6 ------ > 2 files changed, 3 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/include/asm/timex.h b/arch/arm64/include/asm/timex.h > index b24a31a..81a076e 100644 > --- a/arch/arm64/include/asm/timex.h > +++ b/arch/arm64/include/asm/timex.h > @@ -16,14 +16,14 @@ > #ifndef __ASM_TIMEX_H > #define __ASM_TIMEX_H > > +#include <asm/arch_timer.h> > + > /* > * 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 <asm-generic/timex.h> > > -#define ARCH_HAS_READ_CURRENT_TIMER > - > #endif > diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c > index a551f88..03dc371 100644 > --- a/arch/arm64/kernel/time.c > +++ b/arch/arm64/kernel/time.c > @@ -68,12 +68,6 @@ unsigned long long notrace sched_clock(void) > return arch_timer_read_counter() * sched_clock_mult; > } > > -int read_current_timer(unsigned long *timer_value) > -{ > - *timer_value = arch_timer_read_counter(); > - return 0; > -} > - > void __init time_init(void) > { > u32 arch_timer_rate; > -- Chen Gang Asianux Corporation ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer(). 2013-05-27 10:02 ` Chen Gang @ 2013-06-08 4:37 ` Chen Gang 2013-06-10 8:57 ` Will Deacon 2013-06-10 8:57 ` Marc Zyngier 0 siblings, 2 replies; 17+ messages in thread From: Chen Gang @ 2013-06-08 4:37 UTC (permalink / raw) To: linux-arm-kernel Hello Maintainers: Please help check it, when you have time. Thanks. On 05/27/2013 06:02 PM, Chen Gang wrote: > Hello Maintainers: > > Please help check this patch whether OK, when you have time. > > Thanks. > > On 05/21/2013 05:46 PM, Chen Gang wrote: >> >> Under arm64, we will calibrate the delay loop statically using a known >> timer frequency, so delete read_current_timer(), or it will cause >> compiling issue 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 <gang.chen@asianux.com> >> --- >> arch/arm64/include/asm/timex.h | 6 +++--- >> arch/arm64/kernel/time.c | 6 ------ >> 2 files changed, 3 insertions(+), 9 deletions(-) >> >> diff --git a/arch/arm64/include/asm/timex.h b/arch/arm64/include/asm/timex.h >> index b24a31a..81a076e 100644 >> --- a/arch/arm64/include/asm/timex.h >> +++ b/arch/arm64/include/asm/timex.h >> @@ -16,14 +16,14 @@ >> #ifndef __ASM_TIMEX_H >> #define __ASM_TIMEX_H >> >> +#include <asm/arch_timer.h> >> + >> /* >> * 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 <asm-generic/timex.h> >> >> -#define ARCH_HAS_READ_CURRENT_TIMER >> - >> #endif >> diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c >> index a551f88..03dc371 100644 >> --- a/arch/arm64/kernel/time.c >> +++ b/arch/arm64/kernel/time.c >> @@ -68,12 +68,6 @@ unsigned long long notrace sched_clock(void) >> return arch_timer_read_counter() * sched_clock_mult; >> } >> >> -int read_current_timer(unsigned long *timer_value) >> -{ >> - *timer_value = arch_timer_read_counter(); >> - return 0; >> -} >> - >> void __init time_init(void) >> { >> u32 arch_timer_rate; >> > > -- Chen Gang Asianux Corporation ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer(). 2013-06-08 4:37 ` Chen Gang @ 2013-06-10 8:57 ` Will Deacon 2013-06-13 1:12 ` Chen Gang 2013-06-10 8:57 ` Marc Zyngier 1 sibling, 1 reply; 17+ messages in thread From: Will Deacon @ 2013-06-10 8:57 UTC (permalink / raw) To: linux-arm-kernel On Sat, Jun 08, 2013 at 05:37:30AM +0100, Chen Gang wrote: > Hello Maintainers: > > Please help check it, when you have time. [...] > >> Under arm64, we will calibrate the delay loop statically using a known > >> timer frequency, so delete read_current_timer(), or it will cause > >> compiling issue 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 <gang.chen@asianux.com> > >> --- > >> arch/arm64/include/asm/timex.h | 6 +++--- > >> arch/arm64/kernel/time.c | 6 ------ > >> 2 files changed, 3 insertions(+), 9 deletions(-) Looks ok to me. We'll take it for a spin and apply it if it doesn't catch fire. Will ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer(). 2013-06-10 8:57 ` Will Deacon @ 2013-06-13 1:12 ` Chen Gang 0 siblings, 0 replies; 17+ messages in thread From: Chen Gang @ 2013-06-13 1:12 UTC (permalink / raw) To: linux-arm-kernel On 06/10/2013 04:57 PM, Will Deacon wrote: > On Sat, Jun 08, 2013 at 05:37:30AM +0100, Chen Gang wrote: >> > Hello Maintainers: >> > >> > Please help check it, when you have time. > [...] > OK, thanks. >>>> > >> Under arm64, we will calibrate the delay loop statically using a known >>>> > >> timer frequency, so delete read_current_timer(), or it will cause >>>> > >> compiling issue 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 <gang.chen@asianux.com> >>>> > >> --- >>>> > >> arch/arm64/include/asm/timex.h | 6 +++--- >>>> > >> arch/arm64/kernel/time.c | 6 ------ >>>> > >> 2 files changed, 3 insertions(+), 9 deletions(-) > Looks ok to me. We'll take it for a spin and apply it if it doesn't catch > fire. Really need a carefully checking. Thanks. -- Chen Gang Asianux Corporation ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer(). 2013-06-08 4:37 ` Chen Gang 2013-06-10 8:57 ` Will Deacon @ 2013-06-10 8:57 ` Marc Zyngier 2013-06-13 1:13 ` Chen Gang 1 sibling, 1 reply; 17+ messages in thread From: Marc Zyngier @ 2013-06-10 8:57 UTC (permalink / raw) To: linux-arm-kernel On 08/06/13 05:37, Chen Gang wrote: > Hello Maintainers: > > Please help check it, when you have time. > > Thanks. > > On 05/27/2013 06:02 PM, Chen Gang wrote: >> Hello Maintainers: >> >> Please help check this patch whether OK, when you have time. >> >> Thanks. >> >> On 05/21/2013 05:46 PM, Chen Gang wrote: >>> >>> Under arm64, we will calibrate the delay loop statically using a known >>> timer frequency, so delete read_current_timer(), or it will cause >>> compiling issue 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 <gang.chen@asianux.com> >>> --- >>> arch/arm64/include/asm/timex.h | 6 +++--- >>> arch/arm64/kernel/time.c | 6 ------ >>> 2 files changed, 3 insertions(+), 9 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/timex.h b/arch/arm64/include/asm/timex.h >>> index b24a31a..81a076e 100644 >>> --- a/arch/arm64/include/asm/timex.h >>> +++ b/arch/arm64/include/asm/timex.h >>> @@ -16,14 +16,14 @@ >>> #ifndef __ASM_TIMEX_H >>> #define __ASM_TIMEX_H >>> >>> +#include <asm/arch_timer.h> >>> + >>> /* >>> * 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 <asm-generic/timex.h> >>> >>> -#define ARCH_HAS_READ_CURRENT_TIMER >>> - >>> #endif >>> diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c >>> index a551f88..03dc371 100644 >>> --- a/arch/arm64/kernel/time.c >>> +++ b/arch/arm64/kernel/time.c >>> @@ -68,12 +68,6 @@ unsigned long long notrace sched_clock(void) >>> return arch_timer_read_counter() * sched_clock_mult; >>> } >>> >>> -int read_current_timer(unsigned long *timer_value) >>> -{ >>> - *timer_value = arch_timer_read_counter(); >>> - return 0; >>> -} >>> - >>> void __init time_init(void) >>> { >>> u32 arch_timer_rate; Sorry for the delay replying. This looks good to me. FWIW: Acked-by: Marc Zyngier <marc.zyngier@arm.com> M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer(). 2013-06-10 8:57 ` Marc Zyngier @ 2013-06-13 1:13 ` Chen Gang 0 siblings, 0 replies; 17+ messages in thread From: Chen Gang @ 2013-06-13 1:13 UTC (permalink / raw) To: linux-arm-kernel On 06/10/2013 04:57 PM, Marc Zyngier wrote: > Sorry for the delay replying. This looks good to me. > > FWIW: > Acked-by: Marc Zyngier <marc.zyngier@arm.com> Thanks, also sorry for my delay replying. -- Chen Gang Asianux Corporation ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-06-13 1:13 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-20 6:48 [PATCH] arm64: kernel: compiling issue, need 'EXPORT_SYMBOL_GPL(read_current_timer)' Chen Gang 2013-05-20 7:15 ` Marc Zyngier 2013-05-20 9:56 ` Will Deacon 2013-05-21 4:06 ` Chen Gang 2013-05-21 6:13 ` Marc Zyngier 2013-05-21 8:41 ` Chen Gang 2013-05-21 8:58 ` Marc Zyngier 2013-05-21 9:26 ` Chen Gang 2013-05-21 8:53 ` Will Deacon 2013-05-21 9:27 ` Chen Gang 2013-05-21 9:46 ` [PATCH v2] arm64: kernel: compiling issue, need delete read_current_timer() Chen Gang 2013-05-27 10:02 ` Chen Gang 2013-06-08 4:37 ` Chen Gang 2013-06-10 8:57 ` Will Deacon 2013-06-13 1:12 ` Chen Gang 2013-06-10 8:57 ` Marc Zyngier 2013-06-13 1:13 ` Chen Gang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).