Skip to contentSkip to conversation

ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

PATCH New Cyril Chemparathy 3 participants 11 messages Sep 11, 2012

Subsystems (2)
Arm (32-bit) Arm architected timer driver
lore
Share & tools
lore

Cyril Chemparathy

author
#0 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

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 <[email protected]>
---
 arch/arm/include/asm/arch_timer.h |    1 +
 arch/arm/include/asm/timex.h      |   14 +++++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)
arch/arm/include/asm/arch_timer.h +1 -0
index 62e7547..6beb06a 100644
@@ -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)
 {
arch/arm/include/asm/timex.h +11 -3
index 5e71172..76f4217 100644
@@ -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
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

Quoted 10 lines · Cyril Chemparathy

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

author
#2 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

Quoted 9 lines · Will Deacon
Quoted 8 lines · Cyril Chemparathy

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.

Quoted 8 lines
Quoted 2 lines

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.

Signature · 2 lines
Thanks
- Cyril
#3 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

Quoted 16 lines · Cyril Chemparathy
Quoted 15 lines · Will Deacon
Quoted 10 lines · Cyril Chemparathy

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?

Quoted 7 lines
Quoted 2 lines

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

arch/arm/include/asm/timex.h +0 -0
diff --cc arch/arm/include/asm/timex.h
index 5e71172,9acc135..0000000
@@@ -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
author
#4 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

Quoted 45 lines · Will Deacon
Quoted 16 lines · Cyril Chemparathy
Quoted 15 lines · Will Deacon
Quoted 10 lines · Cyril Chemparathy

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?

Quoted 7 lines
Quoted 2 lines

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

arch/arm/include/asm/timex.h +0 -1
diff --cc arch/arm/include/asm/timex.h
index 5e71172,9acc135..0000000
@@@ -12,13 -12,9 +12,11 @@@
   #ifndef _ASMARM_TIMEX_H
   #define _ASMARM_TIMEX_H
 
- #include <asm/arch_timer.h>
   #include <mach/timex.h>

arch/arm/include/asm/timex.h

Quoted 10 lines
arch/arm/include/asm/timex.h +1 -2
 
- #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.

Signature · 2 lines
Thanks
- Cyril
#7 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

Quoted 10 lines · Cyril Chemparathy
Quoted 7 lines · Will Deacon
Quoted 1 line · Cyril Chemparathy

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 at 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 at 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.)

author
#8 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

Quoted 12 lines · Russell King - ARM Linux
Quoted 10 lines · Cyril Chemparathy
Quoted 7 lines · Will Deacon
Quoted 1 line · Cyril Chemparathy

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.

Quoted 9 lines

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.

Quoted 9 lines
2012-09-11 19:45:53 1TBVT2-0008P4-2U => cyril at 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 at 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.)

Signature · 2 lines
Thanks
- Cyril
#9 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

In reply to Cyril Chemparathy
Quoted 10 lines · Cyril Chemparathy

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.

#10 lore raw

PATCH 1/1 ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

Quoted 15 lines · Russell King - ARM Linux
Quoted 10 lines · Cyril Chemparathy

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:

Quoted 7 lines

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