[PATCH 0/2] [V2] introduce macro spin_event_timeout()

[PATCH 0/2] [V2] introduce macro spin_event_timeout()

[Timur Tabi]

Introduce the spin_event_timeout() macro, and update the QE library to use it.

This version changes 'ret' to a return value of the macro instead of a
variable that's passed as a parameter.

[PATCH 1/2] [PATCH 1/2 v9] powerpc: introduce macro spin_event_timeout()

[Timur Tabi]

The macro spin_event_timeout() takes a condition and timeout value
(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>
---

v9: changed the macro to return the value, instead of having it passed in

 arch/powerpc/include/asm/delay.h |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
index f9200a6..1e2eb41 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,38 @@ 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 each evaluation of
+ *         @condition
+ *
+ * The process spins until the condition evaluates to true (non-zero) or the
+ * timeout elapses.  The return value of this macro is the value of
+ * @condition when the loop terminates. This allows you to determine the cause
+ * of the loop terminates.  If the return value is zero, then you know a
+ * timeout has occurred.
+ *
+ * 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)                          \
+({                                                                             \
+	typeof(condition) __ret;                                               \
+	unsigned long __loops = tb_ticks_per_usec * timeout;                   \
+	unsigned long __start = get_tbl();                                     \
+	while (!(__ret = (condition)) && (tb_ticks_since(__start) <= __loops)) \
+		if (delay)                                                     \
+			udelay(delay);                                         \
+		else                                                           \
+			cpu_relax();                                           \
+	__ret;		                                                       \
+})
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DELAY_H */
-- 
1.6.0.6

[PATCH 2/2] [PATCH 2/2 v2] qe: add polling timeout to qe_issue_cmd()

[Timur Tabi]

The qe_issue_cmd() function (Freescale PowerPC QUICC Engine library) polls on
a register until a status bit changes, but does not include a timeout to
handle the situation if the bit never changes.  Change the code to use the new
spin_event_timeout() macro, which simplifies polling on a register without
a timeout.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch depends on my previous patch, "powerpc: introduce macro
spin_event_timeout()".

 arch/powerpc/sysdev/qe_lib/qe.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index 01bce37..05bdd43 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -111,6 +111,7 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input)
 {
 	unsigned long flags;
 	u8 mcn_shift = 0, dev_shift = 0;
+	u32 ret;
 
 	spin_lock_irqsave(&qe_lock, flags);
 	if (cmd == QE_RESET) {
@@ -138,11 +139,13 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input)
 	}
 
 	/* wait for the QE_CR_FLG to clear */
-	while(in_be32(&qe_immr->cp.cecr) & QE_CR_FLG)
-		cpu_relax();
+	ret = spin_event_timeout((in_be32(&qe_immr->cp.cecr) & QE_CR_FLG) == 0,
+			   100, 0);
+	/* On timeout (e.g. failure), the expression will be false (ret == 0),
+	   otherwise it will be true (ret == 1). */
 	spin_unlock_irqrestore(&qe_lock, flags);
 
-	return 0;
+	return ret == 1;
 }
 EXPORT_SYMBOL(qe_issue_cmd);
 
-- 
1.6.0.6

Re: [PATCH 2/2] [PATCH 2/2 v2] qe: add polling timeout to qe_issue_cmd()

[Kumar Gala]

On May 26, 2009, at 10:21 AM, Timur Tabi wrote:

> The qe_issue_cmd() function (Freescale PowerPC QUICC Engine library)  
> polls on
> a register until a status bit changes, but does not include a  
> timeout to
> handle the situation if the bit never changes.  Change the code to  
> use the new
> spin_event_timeout() macro, which simplifies polling on a register  
> without
> a timeout.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> This patch depends on my previous patch, "powerpc: introduce macro
> spin_event_timeout()".
>
> arch/powerpc/sysdev/qe_lib/qe.c |    9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)

applied to next

- k

Re: [PATCH 1/2] [PATCH 1/2 v9] powerpc: introduce macro spin_event_timeout()

[Geoff Thorpe]

Acked-by: Geoff Thorpe <Geoff.Thorpe@freescale.com>

Not sure that it's the most consistent formulation, but it's definitely
better than debating indefinitely.

Cheers,
Geoff

Timur Tabi wrote:
> The macro spin_event_timeout() takes a condition and timeout value
> (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>
> ---
> 
> v9: changed the macro to return the value, instead of having it passed in
[snip]