[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[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.

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(-)

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
-- 
1.7.9.5

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Will Deacon]

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

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Cyril Chemparathy]

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.

-- 
Thanks
- Cyril

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Will Deacon]

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

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Cyril Chemparathy]

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.

-- 
Thanks
- Cyril

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Will Deacon]

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

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Cyril Chemparathy]

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.

-- 
Thanks
- Cyril

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Russell King - ARM Linux]

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

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Cyril Chemparathy]

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

-- 
Thanks
- Cyril

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Russell King - ARM Linux]

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.

[PATCH] ARM: avoid undef on ARCH_HAS_READ_CURRENT_TIMER

[Will Deacon]

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