| Message ID | 1347380341-28145-1-git-send-email-cyril@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 11, 2012 at 05:19:00PM +0100, Cyril Chemparathy wrote: > With the inclusion of asm-generic/timex.h, the ARM arch timer implementation > breaks on build. This is because asm/arch_timer.h now defines > ARCH_HAS_READ_CURRENT_TIMER, only to have this macro undefined by the > subsequent inclusion of asm-generic/timex.h. > > This patch fixes the problem in asm/timex.h by including asm-generic/timex.h > early, and by defining get_cycles even earlier. > > This patch has been tested against linux-next-20120910, both with and without > arch timer support. Is this still a problem with 7530/1 ("delay: add registration mechanism for delay timer sources") applied? http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7530/1 Will
On 9/11/2012 12:25 PM, Will Deacon wrote: > On Tue, Sep 11, 2012 at 05:19:00PM +0100, Cyril Chemparathy wrote: >> With the inclusion of asm-generic/timex.h, the ARM arch timer implementation >> breaks on build. This is because asm/arch_timer.h now defines >> ARCH_HAS_READ_CURRENT_TIMER, only to have this macro undefined by the >> subsequent inclusion of asm-generic/timex.h. >> >> This patch fixes the problem in asm/timex.h by including asm-generic/timex.h >> early, and by defining get_cycles even earlier. >> >> This patch has been tested against linux-next-20120910, both with and without >> arch timer support. > > Is this still a problem with 7530/1 ("delay: add registration mechanism for > delay timer sources") applied? > > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7530/1 > Excellent. This conflicts with linux-next-20120910, and needs an added include of <asm/delay.h> in timex.h. With that in place, it works for me.
On Tue, Sep 11, 2012 at 05:59:39PM +0100, Cyril Chemparathy wrote: > On 9/11/2012 12:25 PM, Will Deacon wrote: > > On Tue, Sep 11, 2012 at 05:19:00PM +0100, Cyril Chemparathy wrote: > >> With the inclusion of asm-generic/timex.h, the ARM arch timer implementation > >> breaks on build. This is because asm/arch_timer.h now defines > >> ARCH_HAS_READ_CURRENT_TIMER, only to have this macro undefined by the > >> subsequent inclusion of asm-generic/timex.h. > >> > >> This patch fixes the problem in asm/timex.h by including asm-generic/timex.h > >> early, and by defining get_cycles even earlier. > >> > >> This patch has been tested against linux-next-20120910, both with and without > >> arch timer support. > > > > Is this still a problem with 7530/1 ("delay: add registration mechanism for > > delay timer sources") applied? > > > > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7530/1 > > > > Excellent. > > This conflicts with linux-next-20120910, and needs an added include of > <asm/delay.h> in timex.h. With that in place, it works for me. Like below? If that's the fix, then I may as well respin 7530/1 given that it hasn't yet been merged. Will --->8 diff --cc arch/arm/include/asm/timex.h index 5e71172,9acc135..0000000 --- a/arch/arm/include/asm/timex.h +++ b/arch/arm/include/asm/timex.h @@@ -12,13 -12,9 +12,11 @@@ #ifndef _ASMARM_TIMEX_H #define _ASMARM_TIMEX_H - #include <asm/arch_timer.h> #include <mach/timex.h> - #ifdef ARCH_HAS_READ_CURRENT_TIMER -typedef unsigned long cycles_t; #define get_cycles() ({ cycles_t c; read_current_timer(&c) ? 0 : c; }) - #endif +#include <asm-generic/timex.h> ++#include <asm/delay.h> + #endif
On 9/11/2012 1:20 PM, Will Deacon wrote: > On Tue, Sep 11, 2012 at 05:59:39PM +0100, Cyril Chemparathy wrote: >> On 9/11/2012 12:25 PM, Will Deacon wrote: >>> On Tue, Sep 11, 2012 at 05:19:00PM +0100, Cyril Chemparathy wrote: >>>> With the inclusion of asm-generic/timex.h, the ARM arch timer implementation >>>> breaks on build. This is because asm/arch_timer.h now defines >>>> ARCH_HAS_READ_CURRENT_TIMER, only to have this macro undefined by the >>>> subsequent inclusion of asm-generic/timex.h. >>>> >>>> This patch fixes the problem in asm/timex.h by including asm-generic/timex.h >>>> early, and by defining get_cycles even earlier. >>>> >>>> This patch has been tested against linux-next-20120910, both with and without >>>> arch timer support. >>> >>> Is this still a problem with 7530/1 ("delay: add registration mechanism for >>> delay timer sources") applied? >>> >>> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7530/1 >>> >> >> Excellent. >> >> This conflicts with linux-next-20120910, and needs an added include of >> <asm/delay.h> in timex.h. With that in place, it works for me. > > Like below? > > If that's the fix, then I may as well respin 7530/1 given that it hasn't yet > been merged. > > Will > > --->8 > > diff --cc arch/arm/include/asm/timex.h > index 5e71172,9acc135..0000000 > --- a/arch/arm/include/asm/timex.h > +++ b/arch/arm/include/asm/timex.h > @@@ -12,13 -12,9 +12,11 @@@ > #ifndef _ASMARM_TIMEX_H > #define _ASMARM_TIMEX_H > > - #include <asm/arch_timer.h> > #include <mach/timex.h> > > - #ifdef ARCH_HAS_READ_CURRENT_TIMER > -typedef unsigned long cycles_t; > #define get_cycles() ({ cycles_t c; read_current_timer(&c) ? 0 : c; }) > - #endif > > +#include <asm-generic/timex.h> > ++#include <asm/delay.h> > + > #endif > Perfect. I can re-verify with the respun 7530/1 if you'd like.
On Tue, Sep 11, 2012 at 06:28:37PM +0100, Cyril Chemparathy wrote:
> I can re-verify with the respun 7530/1 if you'd like.
Sure, submitted as 7533/1:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7533/1
Will
On Tue, Sep 11, 2012 at 12:19:00PM -0400, Cyril Chemparathy wrote: > With the inclusion of asm-generic/timex.h, the ARM arch timer implementation > breaks on build. This is because asm/arch_timer.h now defines > ARCH_HAS_READ_CURRENT_TIMER, only to have this macro undefined by the > subsequent inclusion of asm-generic/timex.h. > > This patch fixes the problem in asm/timex.h by including asm-generic/timex.h > early, and by defining get_cycles even earlier. > > This patch has been tested against linux-next-20120910, both with and without > arch timer support. Given the complexity this introduces, just for the sake of using one thing from asm-generic/timex.h: typedef unsigned long cycles_t; it seems utterly pointless to use the asm-generic version at all, especially if we have to play games to work around what it's doing. What this means is one of two things. Either we should not be using asm-generic/timex.h, or asm-generic/timex.h as currently exists is broken.
On Tue, Sep 11, 2012 at 07:01:00PM +0100, Russell King - ARM Linux wrote: > On Tue, Sep 11, 2012 at 12:19:00PM -0400, Cyril Chemparathy wrote: > > With the inclusion of asm-generic/timex.h, the ARM arch timer implementation > > breaks on build. This is because asm/arch_timer.h now defines > > ARCH_HAS_READ_CURRENT_TIMER, only to have this macro undefined by the > > subsequent inclusion of asm-generic/timex.h. > > > > This patch fixes the problem in asm/timex.h by including asm-generic/timex.h > > early, and by defining get_cycles even earlier. > > > > This patch has been tested against linux-next-20120910, both with and without > > arch timer support. > > Given the complexity this introduces, just for the sake of using one > thing from asm-generic/timex.h: > > typedef unsigned long cycles_t; > > it seems utterly pointless to use the asm-generic version at all, > especially if we have to play games to work around what it's doing. > What this means is one of two things. Either we should not be using > asm-generic/timex.h, or asm-generic/timex.h as currently exists is > broken. Given that we have to have our own asm/timex.h anyway, I'd be happier not using the generic version for ARM. That said, it's sitting in your for-next tree so I thought I'd better work with it. If you drop the change, then 7530/1 (the original patch) should be fine, otherwise 7533/1 caters for the generic code. Will
On 9/11/2012 1:53 PM, Will Deacon wrote: > On Tue, Sep 11, 2012 at 06:28:37PM +0100, Cyril Chemparathy wrote: >> I can re-verify with the respun 7530/1 if you'd like. > > Sure, submitted as 7533/1: > > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7533/1 > Works great.
On Tue, Sep 11, 2012 at 02:31:09PM -0400, Cyril Chemparathy wrote: > On 9/11/2012 1:53 PM, Will Deacon wrote: >> On Tue, Sep 11, 2012 at 06:28:37PM +0100, Cyril Chemparathy wrote: >>> I can re-verify with the respun 7530/1 if you'd like. >> >> Sure, submitted as 7533/1: >> >> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7533/1 >> > > Works great. Cyril, I notice you're trying repeatedly to get a new password out of the system. TI's mail system is known to silently drop the password reminders. Please bug your IT provider about this, there's nothing I can do about it at my end - (and as such you won't receive a password reminder until something can be done about this.) 2012-09-11 19:45:53 1TBVT2-0008P4-2U => cyril@ti.com R=verp_domain_dnslookup T=verp_domain_smtp S=1041 H=ti.com.s9a1.psmtp.com [74.125.148.10] X=TLSv1:AES256-SHA:256 DN="/C=US/ST=California/L=Mountain View/O=Google Inc/CN=*.psmtp.com" C="250 Thanks" 2012-09-11 20:14:27 1TBVuf-0008S4-EC => cyril@ti.com R=verp_domain_dnslookup T=verp_domain_smtp S=1041 H=ti.com.s9a1.psmtp.com [74.125.148.10] X=TLSv1:AES256-SHA:256 DN="/C=US/ST=California/L=Mountain View/O=Google Inc/CN=*.psmtp.com" C="250 Thanks" (Note that *.psmtp.com seems to actually be more trouble than its worth... previous history has shown it to be quite broken, silently dropping _real_ emails as well. I'd call it an anti-HAM service rather than an anti-SPAM service. The more people complain to Google about it the better chance it will get fixed.)
On 9/11/2012 3:28 PM, Russell King - ARM Linux wrote: > On Tue, Sep 11, 2012 at 02:31:09PM -0400, Cyril Chemparathy wrote: >> On 9/11/2012 1:53 PM, Will Deacon wrote: >>> On Tue, Sep 11, 2012 at 06:28:37PM +0100, Cyril Chemparathy wrote: >>>> I can re-verify with the respun 7530/1 if you'd like. >>> >>> Sure, submitted as 7533/1: >>> >>> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7533/1 >>> >> >> Works great. > > Cyril, I notice you're trying repeatedly to get a new password out of > the system. > > TI's mail system is known to silently drop the password reminders. > > Please bug your IT provider about this, there's nothing I can do about > it at my end - (and as such you won't receive a password reminder until > something can be done about this.) > *Sigh* Thanks Russell. I'll try to poke folks to see if this can be fixed. > 2012-09-11 19:45:53 1TBVT2-0008P4-2U => cyril@ti.com R=verp_domain_dnslookup T=verp_domain_smtp S=1041 H=ti.com.s9a1.psmtp.com [74.125.148.10] X=TLSv1:AES256-SHA:256 DN="/C=US/ST=California/L=Mountain View/O=Google Inc/CN=*.psmtp.com" C="250 Thanks" > 2012-09-11 20:14:27 1TBVuf-0008S4-EC => cyril@ti.com R=verp_domain_dnslookup T=verp_domain_smtp S=1041 H=ti.com.s9a1.psmtp.com [74.125.148.10] X=TLSv1:AES256-SHA:256 DN="/C=US/ST=California/L=Mountain View/O=Google Inc/CN=*.psmtp.com" C="250 Thanks" > > (Note that *.psmtp.com seems to actually be more trouble than its > worth... previous history has shown it to be quite broken, silently > dropping _real_ emails as well. I'd call it an anti-HAM service rather > than an anti-SPAM service. The more people complain to Google about it > the better chance it will get fixed.) >
diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h index 62e7547..6beb06a 100644 --- a/arch/arm/include/asm/arch_timer.h +++ b/arch/arm/include/asm/arch_timer.h @@ -7,6 +7,7 @@ #define ARCH_HAS_READ_CURRENT_TIMER int arch_timer_of_register(void); int arch_timer_sched_clock_init(void); +int read_current_timer(cycles_t *c); #else static inline int arch_timer_of_register(void) { diff --git a/arch/arm/include/asm/timex.h b/arch/arm/include/asm/timex.h index 5e71172..76f4217 100644 --- a/arch/arm/include/asm/timex.h +++ b/arch/arm/include/asm/timex.h @@ -12,13 +12,21 @@ #ifndef _ASMARM_TIMEX_H #define _ASMARM_TIMEX_H +#define get_cycles get_cycles + +#include <asm-generic/timex.h> #include <asm/arch_timer.h> #include <mach/timex.h> +static inline cycles_t get_cycles(void) +{ #ifdef ARCH_HAS_READ_CURRENT_TIMER -#define get_cycles() ({ cycles_t c; read_current_timer(&c) ? 0 : c; }) -#endif + cycles_t c; -#include <asm-generic/timex.h> + if (!read_current_timer(&c)) + return c; +#endif + return 0; +} #endif
With the inclusion of asm-generic/timex.h, the ARM arch timer implementation breaks on build. This is because asm/arch_timer.h now defines ARCH_HAS_READ_CURRENT_TIMER, only to have this macro undefined by the subsequent inclusion of asm-generic/timex.h. This patch fixes the problem in asm/timex.h by including asm-generic/timex.h early, and by defining get_cycles even earlier. This patch has been tested against linux-next-20120910, both with and without arch timer support. Signed-off-by: Cyril Chemparathy <cyril@ti.com> --- arch/arm/include/asm/arch_timer.h | 1 + arch/arm/include/asm/timex.h | 14 +++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-)