Patchwork [V4,1/5] The macro spin_event_timeout() takes a condition and timeout value

login
register
mail settings
Submitter Jon Smirl
Date May 25, 2009, 10:15 p.m.
Message ID <20090525221507.21370.68301.stgit@terra>
Download mbox | patch
Permalink /patch/27622/
State Superseded
Headers show

Comments

Jon Smirl - May 25, 2009, 10:15 p.m.
(in microseconds) as parameters.  It spins until either the condition is true
or the timeout expires.  It returns the result of the condition when the loop
was terminated.

This primary purpose of this macro is to poll on a hardware register until a
status bit changes.  The timeout ensures that the loop still terminates if the
bit doesn't change as expected.  This macro makes it easier for driver
developers to perform this kind of operation properly.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 arch/powerpc/include/asm/delay.h |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
Geert Uytterhoeven - May 26, 2009, 7:29 a.m.
On Mon, 25 May 2009, Jon Smirl wrote:
> (in microseconds) as parameters.  It spins until either the condition is true
> or the timeout expires.  It returns the result of the condition when the loop
> was terminated.
> 
> This primary purpose of this macro is to poll on a hardware register until a
> status bit changes.  The timeout ensures that the loop still terminates if the
> bit doesn't change as expected.  This macro makes it easier for driver
> developers to perform this kind of operation properly.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>  arch/powerpc/include/asm/delay.h |   33 +++++++++++++++++++++++++++++++++
>  1 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
> index f9200a6..fedf037 100644
> --- a/arch/powerpc/include/asm/delay.h
> +++ b/arch/powerpc/include/asm/delay.h
> @@ -2,8 +2,11 @@
>  #define _ASM_POWERPC_DELAY_H
>  #ifdef __KERNEL__
>  
> +#include <asm/time.h>
> +
>  /*
>   * Copyright 1996, Paul Mackerras.
> + * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -30,5 +33,35 @@ extern void udelay(unsigned long usecs);
>  #define mdelay(n)	udelay((n) * 1000)
>  #endif
>  
> +/**
> + * spin_event_timeout - spin until a condition gets true or a timeout elapses
> + * @condition: a C expression to evalate
> + * @timeout: timeout, in microseconds
> + * @delay: the number of microseconds to delay between eache evaluation of
> + *         @condition
> + * @rc: the last value of the condition
> + *
> + * The process spins until the condition evaluates to true (non-zero) or the
> + * timeout elapses.  Upon exit, @rc contains the value of the condition. This
> + * allows you to test the condition without incurring any side effects.
> + *
> + * This primary purpose of this macro is to poll on a hardware register
> + * until a status bit changes.  The timeout ensures that the loop still
> + * terminates even if the bit never changes.  The delay is for devices that
> + * need a delay in between successive reads.
> + *
> + * gcc will optimize out the if-statement if @delay is a constant.
> + */
> +#define spin_event_timeout(condition, timeout, delay, rc)                   \

static inline function, returning rc, instead of a macro?

> +{                                                                           \
> +	unsigned long __loops = tb_ticks_per_usec * timeout;                \
> +	unsigned long __start = get_tbl();                                  \
> +	while ((rc = (condition)) && (tb_ticks_since(__start) <= __loops)) \
> +		if (delay)                                                  \
> +			udelay(delay);                                      \
> +		else	                                                    \
> +			cpu_relax();                                        \
> +}
> +
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_DELAY_H */

With kind regards,

Geert Uytterhoeven
Software Architect
Techsoft Centre

Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
Timur Tabi - May 26, 2009, 11:32 a.m.
On Tue, May 26, 2009 at 2:29 AM, Geert Uytterhoeven
<Geert.Uytterhoeven@sonycom.com> wrote:

> static inline function, returning rc, instead of a macro?

It won't work as an inline function ...

>> +     unsigned long __loops = tb_ticks_per_usec * timeout;                \
>> +     unsigned long __start = get_tbl();                                  \
>> +     while ((rc = (condition)) && (tb_ticks_since(__start) <= __loops)) \

There's no way to pass a expression to an inline function.  It has to
be a macro.  Here, the loop evaluates "condition" at every pass.  If
this were an inline function, "condition" would be evaluated once when
the function were called, and never again.
Geert Uytterhoeven - May 26, 2009, 11:51 a.m.
On Tue, 26 May 2009, Timur Tabi wrote:
> On Tue, May 26, 2009 at 2:29 AM, Geert Uytterhoeven
> <Geert.Uytterhoeven@sonycom.com> wrote:
> 
> > static inline function, returning rc, instead of a macro?
> 
> It won't work as an inline function ...
> 
> >> +     unsigned long __loops = tb_ticks_per_usec * timeout;                \
> >> +     unsigned long __start = get_tbl();                                  \
> >> +     while ((rc = (condition)) && (tb_ticks_since(__start) <= __loops)) \
> 
> There's no way to pass a expression to an inline function.  It has to
> be a macro.  Here, the loop evaluates "condition" at every pass.  If
> this were an inline function, "condition" would be evaluated once when
> the function were called, and never again.

You're right, I missed that part. Sorry about that.

However, you can still improve useability by making the macro return the rc,
instead of letting the caller pass it, cfr. wait_event_timeout() and friends.

With kind regards,

Geert Uytterhoeven
Software Architect
Techsoft Centre

Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
Arnd Bergmann - May 26, 2009, 12:49 p.m.
On Tuesday 26 May 2009, Geert Uytterhoeven wrote:
> However, you can still improve useability by making the macro return the rc,
> instead of letting the caller pass it, cfr. wait_event_timeout() and friends.

Either that, or it should at least use the do { ... } while (0) construct
to make the macro a statement. All multi-line macros need to either
use ({ ... }) or do { ... } while (0) to make sure they behave well.

	Arnd <><
Timur Tabi - May 26, 2009, 2:20 p.m.
Arnd Bergmann wrote:
> On Tuesday 26 May 2009, Geert Uytterhoeven wrote:
>> However, you can still improve useability by making the macro return the rc,
>> instead of letting the caller pass it, cfr. wait_event_timeout() and friends.

I had that originally, but somewhere during the seven revisions of my macro, it got lost.

I'll post a v9 soon.  This version will take 3 parameters and return __ret, so Jon will need to change his code (sorry).

Patch

diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
index f9200a6..fedf037 100644
--- a/arch/powerpc/include/asm/delay.h
+++ b/arch/powerpc/include/asm/delay.h
@@ -2,8 +2,11 @@ 
 #define _ASM_POWERPC_DELAY_H
 #ifdef __KERNEL__
 
+#include <asm/time.h>
+
 /*
  * Copyright 1996, Paul Mackerras.
+ * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -30,5 +33,35 @@  extern void udelay(unsigned long usecs);
 #define mdelay(n)	udelay((n) * 1000)
 #endif
 
+/**
+ * spin_event_timeout - spin until a condition gets true or a timeout elapses
+ * @condition: a C expression to evalate
+ * @timeout: timeout, in microseconds
+ * @delay: the number of microseconds to delay between eache evaluation of
+ *         @condition
+ * @rc: the last value of the condition
+ *
+ * The process spins until the condition evaluates to true (non-zero) or the
+ * timeout elapses.  Upon exit, @rc contains the value of the condition. This
+ * allows you to test the condition without incurring any side effects.
+ *
+ * This primary purpose of this macro is to poll on a hardware register
+ * until a status bit changes.  The timeout ensures that the loop still
+ * terminates even if the bit never changes.  The delay is for devices that
+ * need a delay in between successive reads.
+ *
+ * gcc will optimize out the if-statement if @delay is a constant.
+ */
+#define spin_event_timeout(condition, timeout, delay, rc)                   \
+{                                                                           \
+	unsigned long __loops = tb_ticks_per_usec * timeout;                \
+	unsigned long __start = get_tbl();                                  \
+	while ((rc = (condition)) && (tb_ticks_since(__start) <= __loops)) \
+		if (delay)                                                  \
+			udelay(delay);                                      \
+		else	                                                    \
+			cpu_relax();                                        \
+}
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DELAY_H */