This patch-set moves USECS_PER_JIFFY to architecture timex.h from architecture
specific C files. Patch-set further uses this to make PPC macro
spin_event_timeout() architecture independent. This change enables drivers to
use spin_event_timeout() even in non-PowerPC based SoC's.
This patchset has been compiled for ARM, PowerPC and x86.
Patch[1/2] : Moves USECS_PER_JIFFY to architecture specific timex.h files from
architecture specific C files.
Patch[2/2] : Converts PPC macro spin_event_timeout() in architecture independent
macro.
Arpit Goel (2):
Make USECS_PER_JIFFY available for generic use
Convert PowerPC macro spin_event_timeout() to architecture independent
macro
arch/arm/include/asm/timex.h | 2 ++
arch/arm/kernel/time.c | 3 ---
arch/m32r/include/asm/timex.h | 2 ++
arch/m32r/kernel/time.c | 3 ---
arch/m68k/hp300/time.c | 4 +---
arch/m68k/include/asm/timex.h | 2 ++
arch/s390/include/asm/timex.h | 2 ++
arch/s390/kernel/time.c | 2 --
arch/sparc/include/asm/timex_32.h | 2 ++
arch/sparc/include/asm/timex_64.h | 2 ++
arch/sparc/kernel/pcic.c | 1 -
include/linux/delay.h | 40 +++++++++++++++++++++++++++++++++++++++
12 files changed, 53 insertions(+), 12 deletions(-)
--
1.8.1.4
This patch ports PowerPC implementation of spin_event_timeout() for generic
use. Architecture specific implementation can be added to asm/delay.h, which
will override the generic linux implementation.
Signed-off-by: Arpit Goel <[email protected]>
---
include/linux/delay.h | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/include/linux/delay.h b/include/linux/delay.h
index a6ecb34..e975994 100644
--- a/include/linux/delay.h
+++ b/include/linux/delay.h
@@ -52,4 +52,44 @@ static inline void ssleep(unsigned int seconds)
msleep(seconds * 1000);
}
+#ifndef spin_event_timeout
+/**
+ * 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.
+ *
+ * This is copied from PowerPC based spin_event_timeout() implementation
+ * and modified for generic usecase.
+ */
+#define spin_event_timeout(condition, timeout, delay) \
+({ \
+ typeof(condition) __ret; \
+ unsigned long __loops = timeout/USECS_PER_JIFFY; \
+ unsigned long __start = jiffies; \
+ while (!(__ret = (condition)) && \
+ time_before(jiffies, __start + __loops + 1)) \
+ if (delay) \
+ udelay(delay); \
+ else \
+ schedule(); \
+ if (!__ret) \
+ __ret = (condition); \
+ __ret; \
+})
+#endif
#endif /* defined(_LINUX_DELAY_H) */
--
1.8.1.4
USECS_PER_JIFFY is used in several ARCHITECTURE C files.Now USECS_PER_JIFFY
has been moved from architecture specific C files to architecture specific
timex.h. This enables users to include timex.h and make use of USECS_PER_JIFFY.
Signed-off-by: Arpit Goel <[email protected]>
---
arch/arm/include/asm/timex.h | 2 ++
arch/arm/kernel/time.c | 3 ---
arch/m32r/include/asm/timex.h | 2 ++
arch/m32r/kernel/time.c | 3 ---
arch/m68k/hp300/time.c | 4 +---
arch/m68k/include/asm/timex.h | 2 ++
arch/s390/include/asm/timex.h | 2 ++
arch/s390/kernel/time.c | 2 --
arch/sparc/include/asm/timex_32.h | 2 ++
arch/sparc/include/asm/timex_64.h | 2 ++
arch/sparc/kernel/pcic.c | 1 -
11 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/arch/arm/include/asm/timex.h b/arch/arm/include/asm/timex.h
index 83f2aa8..55a4f34 100644
--- a/arch/arm/include/asm/timex.h
+++ b/arch/arm/include/asm/timex.h
@@ -18,6 +18,8 @@
#include <mach/timex.h>
#endif
+/* change this if you have some constant time drift */
+#define USECS_PER_JIFFY (1000000/HZ)
typedef unsigned long cycles_t;
#define get_cycles() ({ cycles_t c; read_current_timer(&c) ? 0 : c; })
diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index 98aee32..7aaead4 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -38,9 +38,6 @@ DEFINE_SPINLOCK(rtc_lock);
EXPORT_SYMBOL(rtc_lock);
#endif /* pc-style 'CMOS' RTC support */
-/* change this if you have some constant time drift */
-#define USECS_PER_JIFFY (1000000/HZ)
-
#ifdef CONFIG_SMP
unsigned long profile_pc(struct pt_regs *regs)
{
diff --git a/arch/m32r/include/asm/timex.h b/arch/m32r/include/asm/timex.h
index bb9fe4f..ff8e6b1 100644
--- a/arch/m32r/include/asm/timex.h
+++ b/arch/m32r/include/asm/timex.h
@@ -9,6 +9,8 @@
#define CLOCK_TICK_RATE (CONFIG_BUS_CLOCK / CONFIG_TIMER_DIVIDE)
#define CLOCK_TICK_FACTOR 20 /* Factor of both 1000000 and CLOCK_TICK_RATE */
+/* change this if you have some constant time drift */
+#define USECS_PER_JIFFY (1000000/HZ)
#ifdef __KERNEL__
/*
diff --git a/arch/m32r/kernel/time.c b/arch/m32r/kernel/time.c
index 1a15f81..0608e02 100644
--- a/arch/m32r/kernel/time.c
+++ b/arch/m32r/kernel/time.c
@@ -52,9 +52,6 @@ extern void smp_local_timer_interrupt(void);
* Change this if you have some constant time drift
*/
-/* This is for machines which generate the exact clock. */
-#define USECS_PER_JIFFY (1000000/HZ)
-
static unsigned long latch;
static u32 m32r_gettimeoffset(void)
diff --git a/arch/m68k/hp300/time.c b/arch/m68k/hp300/time.c
index 749543b..9d751d3 100644
--- a/arch/m68k/hp300/time.c
+++ b/arch/m68k/hp300/time.c
@@ -12,6 +12,7 @@
#include <linux/sched.h>
#include <linux/kernel_stat.h>
#include <linux/interrupt.h>
+#include <linux/time.h>
#include <asm/machdep.h>
#include <asm/irq.h>
#include <asm/io.h>
@@ -30,9 +31,6 @@
#define CLKMSB2 0x9
#define CLKMSB3 0xD
-/* This is for machines which generate the exact clock. */
-#define USECS_PER_JIFFY (1000000/HZ)
-
#define INTVAL ((10000 / 4) - 1)
static irqreturn_t hp300_tick(int irq, void *dev_id)
diff --git a/arch/m68k/include/asm/timex.h b/arch/m68k/include/asm/timex.h
index 6759dad..44f2603 100644
--- a/arch/m68k/include/asm/timex.h
+++ b/arch/m68k/include/asm/timex.h
@@ -21,6 +21,8 @@
#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
#endif
+/* change this if you have some constant time drift */
+#define USECS_PER_JIFFY (1000000/HZ)
typedef unsigned long cycles_t;
static inline cycles_t get_cycles(void)
diff --git a/arch/s390/include/asm/timex.h b/arch/s390/include/asm/timex.h
index 8ad8af9..91175c9 100644
--- a/arch/s390/include/asm/timex.h
+++ b/arch/s390/include/asm/timex.h
@@ -13,6 +13,8 @@
/* The value of the TOD clock for 1.1.1970. */
#define TOD_UNIX_EPOCH 0x7d91048bca000000ULL
+/* change this if you have some constant time drift */
+#define USECS_PER_JIFFY (1000000/HZ)
/* Inline functions for clock register access. */
static inline int set_tod_clock(__u64 time)
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 876546b..eb659b6 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -49,8 +49,6 @@
#include <asm/cio.h>
#include "entry.h"
-/* change this if you have some constant time drift */
-#define USECS_PER_JIFFY ((unsigned long) 1000000/HZ)
#define CLK_TICKS_PER_JIFFY ((unsigned long) USECS_PER_JIFFY << 12)
u64 sched_clock_base_cc = -1; /* Force to data section. */
diff --git a/arch/sparc/include/asm/timex_32.h b/arch/sparc/include/asm/timex_32.h
index b6ccdb0..cb7f412 100644
--- a/arch/sparc/include/asm/timex_32.h
+++ b/arch/sparc/include/asm/timex_32.h
@@ -7,6 +7,8 @@
#define _ASMsparc_TIMEX_H
#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
+/* change this if you have some constant time drift */
+#define USECS_PER_JIFFY (1000000/HZ)
/* XXX Maybe do something better at some point... -DaveM */
typedef unsigned long cycles_t;
diff --git a/arch/sparc/include/asm/timex_64.h b/arch/sparc/include/asm/timex_64.h
index 18b30bc..1a5738a 100644
--- a/arch/sparc/include/asm/timex_64.h
+++ b/arch/sparc/include/asm/timex_64.h
@@ -9,6 +9,8 @@
#include <asm/timer.h>
#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
+/* change this if you have some constant time drift */
+#define USECS_PER_JIFFY (1000000/HZ)
/* Getting on the cycle counter on sparc64. */
typedef unsigned long cycles_t;
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 09f4fdd..2a62932 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -703,7 +703,6 @@ static void pcic_clear_clock_irq(void)
}
/* CPU frequency is 100 MHz, timer increments every 4 CPU clocks */
-#define USECS_PER_JIFFY (1000000 / HZ)
#define TICK_TIMER_LIMIT ((100 * 1000000 / 4) / HZ)
static unsigned int pcic_cycles_offset(void)
--
1.8.1.4
On Tue, Jul 30, 2013 at 2:38 PM, Arpit Goel <[email protected]> wrote:
> This patch-set moves USECS_PER_JIFFY to architecture timex.h from architecture
> specific C files. Patch-set further uses this to make PPC macro
> spin_event_timeout() architecture independent. This change enables drivers to
> use spin_event_timeout() even in non-PowerPC based SoC's.
>
> This patchset has been compiled for ARM, PowerPC and x86.
>
> Patch[1/2] : Moves USECS_PER_JIFFY to architecture specific timex.h files from
> architecture specific C files.
> arch/arm/include/asm/timex.h | 2 ++
> arch/arm/kernel/time.c | 3 ---
> arch/m32r/include/asm/timex.h | 2 ++
> arch/m32r/kernel/time.c | 3 ---
> arch/m68k/hp300/time.c | 4 +---
> arch/m68k/include/asm/timex.h | 2 ++
> arch/s390/include/asm/timex.h | 2 ++
> arch/s390/kernel/time.c | 2 --
> arch/sparc/include/asm/timex_32.h | 2 ++
> arch/sparc/include/asm/timex_64.h | 2 ++
> arch/sparc/kernel/pcic.c | 1 -
As the definition is the same on all architectures, why not move it to
<linux/delay.h>?
> Patch[2/2] : Converts PPC macro spin_event_timeout() in architecture independent
> macro.
> include/linux/delay.h | 40 +++++++++++++++++++++++++++++++++++++++
After your first patch, USECS_PER_JIFFY is still available only on
arm/m32r/m68k/s390/sparc. What about other architectures?
If anyone uses spin_event_timeout() on e.g. x86, it will fail to compile.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 07/30, Arpit Goel wrote:
> This patch ports PowerPC implementation of spin_event_timeout() for generic
> use. Architecture specific implementation can be added to asm/delay.h, which
> will override the generic linux implementation.
>
> Signed-off-by: Arpit Goel <[email protected]>
> ---
We use something similar internally but it's tied specifically to
readl.
>
> +#ifndef spin_event_timeout
> +/**
> + * 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.
> + *
> + * This is copied from PowerPC based spin_event_timeout() implementation
> + * and modified for generic usecase.
> + */
> +#define spin_event_timeout(condition, timeout, delay) \
> +({ \
> + typeof(condition) __ret; \
> + unsigned long __loops = timeout/USECS_PER_JIFFY; \
> + unsigned long __start = jiffies; \
> + while (!(__ret = (condition)) && \
> + time_before(jiffies, __start + __loops + 1)) \
> + if (delay) \
> + udelay(delay); \
> + else \
> + schedule(); \
> + if (!__ret) \
> + __ret = (condition); \
> + __ret; \
> +})
What do you do here if jiffies aren't incrementing (i.e
interrupts are disabled). The time_before() check won't work
there and it would be nice if we were able to use this in such
situations. I think powerpc gets around this by reading the
hardware timer directly?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On Wed, Jul 31, 2013 at 2:16 AM, Stephen Boyd <[email protected]> wrote:
>
> What do you do here if jiffies aren't incrementing (i.e
> interrupts are disabled). The time_before() check won't work
> there and it would be nice if we were able to use this in such
> situations. I think powerpc gets around this by reading the
> hardware timer directly?
I believe that jiffies is always a global variable. It should behave
the same on PowerPC as on other architectures.
The answer to your question is that you should not use
spin_event_timeout() in interrupt context, because it yields.
(FYI, I'm the author of spin_event_timeout(), so please CC: me on all
changes to it)
On Tue, Jul 30, 2013 at 7:38 AM, Arpit Goel <[email protected]> wrote:
>
> +#ifndef spin_event_timeout
> +/**
Why don't you make PowerPC also use this generic version? It seems
silly to have two virtually identical implementations of this macro?
On 07/31/13 16:44, Timur Tabi wrote:
> On Wed, Jul 31, 2013 at 2:16 AM, Stephen Boyd <[email protected]> wrote:
>> What do you do here if jiffies aren't incrementing (i.e
>> interrupts are disabled). The time_before() check won't work
>> there and it would be nice if we were able to use this in such
>> situations. I think powerpc gets around this by reading the
>> hardware timer directly?
> I believe that jiffies is always a global variable. It should behave
> the same on PowerPC as on other architectures.
Yes it's global but it doesn't increment while interrupts are off.
>
> The answer to your question is that you should not use
> spin_event_timeout() in interrupt context, because it yields.
>
If it yields why are we using udelay? Why not usleep_range()? It would
be useful to have a variant that worked in interrupt context and it
looked like that was almost possible.
BTW, couldn't we skip the first patch and just use usecs_to_jiffies()?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On 07/31/2013 07:04 PM, Stephen Boyd wrote:
> If it yields why are we using udelay? Why not usleep_range()? It would
> be useful to have a variant that worked in interrupt context and it
> looked like that was almost possible.
I've never heard of usleep_range() before, so I don't know if it
applies. Apparently, udelay() includes its own call to cpu_relax(). Is
it possible that cpu_relax() is a "lightweight" yield, compared to sleeping?
FYI, you might want to look at the code reviews for spin_event_timeout()
on the linuxppc-dev mailing list, back in March 2009.
--
--
Timur Tabi
On 07/31/13 17:13, Timur Tabi wrote:
> On 07/31/2013 07:04 PM, Stephen Boyd wrote:
>> If it yields why are we using udelay? Why not usleep_range()? It would
>> be useful to have a variant that worked in interrupt context and it
>> looked like that was almost possible.
> I've never heard of usleep_range() before, so I don't know if it
> applies. Apparently, udelay() includes its own call to cpu_relax(). Is
> it possible that cpu_relax() is a "lightweight" yield, compared to sleeping?
cpu_relax() is usually just a compiler barrier or an instruction hint to
the cpu that it should cool down because we're spinning in a tight loop.
It certainly shouldn't be calling into the scheduler.
>
> FYI, you might want to look at the code reviews for spin_event_timeout()
> on the linuxppc-dev mailing list, back in March 2009.
>
Sure. Any pointers? Otherwise I'll go digging around the archives.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On 07/31/2013 07:16 PM, Stephen Boyd wrote:
> cpu_relax() is usually just a compiler barrier or an instruction hint to
> the cpu that it should cool down because we're spinning in a tight loop.
> It certainly shouldn't be calling into the scheduler.
Ah yes, I remember now. So it does seem that if we can fix the problem
of non-incrementing 'jiffies', then this macro can be used in interrupts.
Of course, that assumes that spinning in interrupt context is a good
idea to begin with. Maybe we shouldn't be encouraging it?
>> > FYI, you might want to look at the code reviews for spin_event_timeout()
>> > on the linuxppc-dev mailing list, back in March 2009.
>> >
> Sure. Any pointers? Otherwise I'll go digging around the archives.
https://lists.ozlabs.org/pipermail/linuxppc-dev/2009-March/thread.html
--
Timur Tabi
On 07/31/13 17:20, Timur Tabi wrote:
> On 07/31/2013 07:16 PM, Stephen Boyd wrote:
>> cpu_relax() is usually just a compiler barrier or an instruction hint to
>> the cpu that it should cool down because we're spinning in a tight loop.
>> It certainly shouldn't be calling into the scheduler.
> Ah yes, I remember now. So it does seem that if we can fix the problem
> of non-incrementing 'jiffies', then this macro can be used in interrupts.
That's encouraging. It looks like you introduced it to use in interrupt
context but then it got shot down[1]? I lost track in all the versions.
>
> Of course, that assumes that spinning in interrupt context is a good
> idea to begin with. Maybe we shouldn't be encouraging it?
I read through the v5 discussion and it seems I'm about to walk through
some tall grass on the way to Cerulean City.
Andrew Morton, I choose you! Use your mind-power move to convince
everyone that having a macro for spinning on a register in interrupt
context is a good thing. At least it will be more obvious.
>
>>>> FYI, you might want to look at the code reviews for spin_event_timeout()
>>>> on the linuxppc-dev mailing list, back in March 2009.
>>>>
>> Sure. Any pointers? Otherwise I'll go digging around the archives.
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2009-March/thread.html
>
[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2009-May/072521.html
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation