linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7] introduce macro spin_event_timeout()
@ 2009-05-05 21:53 Timur Tabi
  2009-05-06  0:43 ` Sean MacLennan
  2009-05-14 15:13 ` Grant Likely
  0 siblings, 2 replies; 9+ messages in thread
From: Timur Tabi @ 2009-05-05 21:53 UTC (permalink / raw)
  To: linuxppc-dev, galak, benh, scottwood, smaclennan, jwboyer

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

v7: add if-statement to use cpu_relax() if the delay is 0.  gcc will optimize
out the if-statement if 'delay' is a constant.

I'm making this a PowerPC-specific patch because I want to use
tb_ticks_per_usec, which does not exist on all other platforms.  I don't want
to use jiffies because jiffies works only when interrupts are enabled, and
the resolution may not be fine enough.

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

diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
index f9200a6..2bde26f 100644
--- a/arch/powerpc/include/asm/delay.h
+++ b/arch/powerpc/include/asm/delay.h
@@ -2,6 +2,8 @@
 #define _ASM_POWERPC_DELAY_H
 #ifdef __KERNEL__
 
+#include <asm/time.h>
+
 /*
  * Copyright 1996, Paul Mackerras.
  *
@@ -30,5 +32,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 */
-- 
1.6.0.6

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v7] introduce macro spin_event_timeout()
  2009-05-05 21:53 [PATCH v7] introduce macro spin_event_timeout() Timur Tabi
@ 2009-05-06  0:43 ` Sean MacLennan
  2009-05-14 15:13 ` Grant Likely
  1 sibling, 0 replies; 9+ messages in thread
From: Sean MacLennan @ 2009-05-06  0:43 UTC (permalink / raw)
  To: Timur Tabi; +Cc: scottwood, linuxppc-dev

On Tue,  5 May 2009 16:53:05 -0500
"Timur Tabi" <timur@freescale.com> 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>

Nice. I could have used a routine like this in a couple of our drivers.
So, for what it is worth:

Acked-by: Sean MacLennan <smaclennan@pikatech.com>

Cheers,
   Sean

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7] introduce macro spin_event_timeout()
  2009-05-05 21:53 [PATCH v7] introduce macro spin_event_timeout() Timur Tabi
  2009-05-06  0:43 ` Sean MacLennan
@ 2009-05-14 15:13 ` Grant Likely
  2009-05-14 16:03   ` Timur Tabi
  1 sibling, 1 reply; 9+ messages in thread
From: Grant Likely @ 2009-05-14 15:13 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, smaclennan, scottwood

On Tue, May 5, 2009 at 3:53 PM, Timur Tabi <timur@freescale.com> wrote:
> The macro spin_event_timeout() takes a condition and timeout value
> (in microseconds) as parameters. =A0It spins until either the condition i=
s true
> or the timeout expires. =A0It returns the result of the condition when th=
e loop
> was terminated.
>
> This primary purpose of this macro is to poll on a hardware register unti=
l a
> status bit changes. =A0The timeout ensures that the loop still terminates=
 if the
> bit doesn't change as expected. =A0This macro makes it easier for driver
> developers to perform this kind of operation properly.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>

This version looks good to me.  Who's the first user? (I'd like to see
that something is ready to use this before merging it)

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>
> v7: add if-statement to use cpu_relax() if the delay is 0. =A0gcc will op=
timize
> out the if-statement if 'delay' is a constant.
>
> I'm making this a PowerPC-specific patch because I want to use
> tb_ticks_per_usec, which does not exist on all other platforms. =A0I don'=
t want
> to use jiffies because jiffies works only when interrupts are enabled, an=
d
> the resolution may not be fine enough.
>
> =A0arch/powerpc/include/asm/delay.h | =A0 32 ++++++++++++++++++++++++++++=
++++
> =A01 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/=
delay.h
> index f9200a6..2bde26f 100644
> --- a/arch/powerpc/include/asm/delay.h
> +++ b/arch/powerpc/include/asm/delay.h
> @@ -2,6 +2,8 @@
> =A0#define _ASM_POWERPC_DELAY_H
> =A0#ifdef __KERNEL__
>
> +#include <asm/time.h>
> +
> =A0/*
> =A0* Copyright 1996, Paul Mackerras.
> =A0*
> @@ -30,5 +32,35 @@ extern void udelay(unsigned long usecs);
> =A0#define mdelay(n) =A0 =A0 =A0udelay((n) * 1000)
> =A0#endif
>
> +/**
> + * spin_event_timeout - spin until a condition gets true or a timeout el=
apses
> + * @condition: a C expression to evalate
> + * @timeout: timeout, in microseconds
> + * @delay: the number of microseconds to delay between eache evaluation =
of
> + * =A0 =A0 =A0 =A0 @condition
> + * @rc: the last value of the condition
> + *
> + * The process spins until the condition evaluates to true (non-zero) or=
 the
> + * timeout elapses. =A0Upon exit, @rc contains the value of the conditio=
n. 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. =A0The timeout ensures that the loop stil=
l
> + * terminates even if the bit never changes. =A0The 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) =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 \
> +{ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 \
> + =A0 =A0 =A0 unsigned long __loops =3D tb_ticks_per_usec * timeout; =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0\
> + =A0 =A0 =A0 unsigned long __start =3D get_tbl(); =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
> + =A0 =A0 =A0 while (!(rc =3D (condition)) && (tb_ticks_since(__start) <=
=3D __loops)) \
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (delay) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 udelay(delay); =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
\
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cpu_relax(); =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
> +}
> +
> =A0#endif /* __KERNEL__ */
> =A0#endif /* _ASM_POWERPC_DELAY_H */
> --
> 1.6.0.6
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7] introduce macro spin_event_timeout()
  2009-05-14 15:13 ` Grant Likely
@ 2009-05-14 16:03   ` Timur Tabi
  2009-05-14 16:10     ` Grant Likely
  0 siblings, 1 reply; 9+ messages in thread
From: Timur Tabi @ 2009-05-14 16:03 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, smaclennan, scottwood

Grant Likely wrote:

> This version looks good to me.  Who's the first user? (I'd like to see
> that something is ready to use this before merging it)

I was going to do it the other way around - wait until this patch is merged, and then update some code to use it.  After all, it's easier to use a macro if it already exists in a header file. :-)

Grepping for "while.*in_be" finds lots of candidates.

I was originally planning to use this macro in sound/soc/fsl/fsl_ssi.c, but I eliminated the need to poll on a register with commit a4d11fe50c238a7da5225d1399314c3505cbd792 ("ASoC: remove trigger delay in Freescale MPC8610 sound driver").  So now I need to find a new guinea pig.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7] introduce macro spin_event_timeout()
  2009-05-14 16:03   ` Timur Tabi
@ 2009-05-14 16:10     ` Grant Likely
  2009-05-18 22:49       ` Timur Tabi
  0 siblings, 1 reply; 9+ messages in thread
From: Grant Likely @ 2009-05-14 16:10 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, smaclennan, scottwood

On Thu, May 14, 2009 at 10:03 AM, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
>> This version looks good to me. =A0Who's the first user? (I'd like to see
>> that something is ready to use this before merging it)
>
> I was going to do it the other way around - wait until this patch is merg=
ed, and then update some code to use it. =A0After all, it's easier to use a=
 macro if it already exists in a header file. :-)

Heh, I think you miss my meaning.  Of course this patch must be merged
before any users.  I just want to hold off merging it until I see
viable patches on the mailing list which depend on it.

In other words, write your patches which use it and submit the lot as
a patch series with this patch as the first.  That gives some evidence
that this macro will actually be used and useful.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7] introduce macro spin_event_timeout()
  2009-05-14 16:10     ` Grant Likely
@ 2009-05-18 22:49       ` Timur Tabi
  2009-05-19  2:58         ` Grant Likely
  0 siblings, 1 reply; 9+ messages in thread
From: Timur Tabi @ 2009-05-18 22:49 UTC (permalink / raw)
  To: Grant Likely; +Cc: scottwood, linuxppc-dev, smaclennan

On Thu, May 14, 2009 at 11:10 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> In other words, write your patches which use it and submit the lot as
> a patch series with this patch as the first. =A0That gives some evidence
> that this macro will actually be used and useful.

This is going to be more difficult than you think.  The problem is
that I don't have access to much hardware that uses drivers which can
take advantage of this macro.  I can find one, maybe two examples, but
if I put a timeout that's too short, I might break some other platform
without knowing it.

--=20
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7] introduce macro spin_event_timeout()
  2009-05-18 22:49       ` Timur Tabi
@ 2009-05-19  2:58         ` Grant Likely
  2009-05-19  3:57           ` Sean MacLennan
  0 siblings, 1 reply; 9+ messages in thread
From: Grant Likely @ 2009-05-19  2:58 UTC (permalink / raw)
  To: Timur Tabi; +Cc: scottwood, linuxppc-dev, smaclennan

On Mon, May 18, 2009 at 4:49 PM, Timur Tabi <timur@freescale.com> wrote:
> On Thu, May 14, 2009 at 11:10 AM, Grant Likely
> <grant.likely@secretlab.ca> wrote:
>> In other words, write your patches which use it and submit the lot as
>> a patch series with this patch as the first. =A0That gives some evidence
>> that this macro will actually be used and useful.
>
> This is going to be more difficult than you think. =A0The problem is
> that I don't have access to much hardware that uses drivers which can
> take advantage of this macro. =A0I can find one, maybe two examples, but
> if I put a timeout that's too short, I might break some other platform
> without knowing it.

Then let it lie fallow on the list and in patchwork.  It is published,
and people know that it is available (I'll certainly consider it as I
do driver work), but there must be real (not just theoretical) in
kernel users of the interface.  I someone wants to use it, then they
can add it to their series or ping the list about getting it merged.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7] introduce macro spin_event_timeout()
  2009-05-19  2:58         ` Grant Likely
@ 2009-05-19  3:57           ` Sean MacLennan
  2009-05-19 12:30             ` Grant Likely
  0 siblings, 1 reply; 9+ messages in thread
From: Sean MacLennan @ 2009-05-19  3:57 UTC (permalink / raw)
  To: Grant Likely; +Cc: scottwood, linuxppc-dev, Timur Tabi

On Mon, 18 May 2009 20:58:03 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> Then let it lie fallow on the list and in patchwork.  It is published,
> and people know that it is available (I'll certainly consider it as I
> do driver work), but there must be real (not just theoretical) in
> kernel users of the interface.  I someone wants to use it, then they
> can add it to their series or ping the list about getting it merged.

That's too bad. If it is ever merged, could you also ping the list?

Cheers,
   Sean

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7] introduce macro spin_event_timeout()
  2009-05-19  3:57           ` Sean MacLennan
@ 2009-05-19 12:30             ` Grant Likely
  0 siblings, 0 replies; 9+ messages in thread
From: Grant Likely @ 2009-05-19 12:30 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: scottwood, linuxppc-dev, Timur Tabi

On Mon, May 18, 2009 at 9:57 PM, Sean MacLennan <smaclennan@pikatech.com> w=
rote:
> On Mon, 18 May 2009 20:58:03 -0600
> Grant Likely <grant.likely@secretlab.ca> wrote:
>
>> Then let it lie fallow on the list and in patchwork. =A0It is published,
>> and people know that it is available (I'll certainly consider it as I
>> do driver work), but there must be real (not just theoretical) in
>> kernel users of the interface. =A0I someone wants to use it, then they
>> can add it to their series or ping the list about getting it merged.
>
> That's too bad. If it is ever merged, could you also ping the list?

If you intend to use it, then post a patch to do so and state that it
depends on Timur's patch.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2009-05-19 12:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-05 21:53 [PATCH v7] introduce macro spin_event_timeout() Timur Tabi
2009-05-06  0:43 ` Sean MacLennan
2009-05-14 15:13 ` Grant Likely
2009-05-14 16:03   ` Timur Tabi
2009-05-14 16:10     ` Grant Likely
2009-05-18 22:49       ` Timur Tabi
2009-05-19  2:58         ` Grant Likely
2009-05-19  3:57           ` Sean MacLennan
2009-05-19 12:30             ` Grant Likely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).