Make PPC macro spin_event_timeout() architecture independent
Subsystems (6)
cover Make PPC macro spin_event_timeout() architecture independent
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(-)
Signature · 1 line
1.8.1.4
PATCH 1/2 Make USECS_PER_JIFFY available for generic use
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.
--- 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(-)
arch/arm/include/asm/timex.h +2 -0
@@ -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; })
arch/arm/kernel/time.c +0 -3
@@ -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) {
arch/m32r/include/asm/timex.h +2 -0
@@ -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__/*
arch/m32r/kernel/time.c +0 -3
@@ -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 )
arch/m68k/hp300/time.c +1 -3
@@ -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)
arch/m68k/include/asm/timex.h +2 -0
@@ -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 )
arch/s390/include/asm/timex.h +2 -0
@@ -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)
arch/s390/kernel/time.c +0 -2
@@ -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. */
arch/sparc/include/asm/timex_32.h +2 -0
@@ -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@some point... -DaveM */ typedef unsigned long cycles_t ;
arch/sparc/include/asm/timex_64.h +2 -0
@@ -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 ;
arch/sparc/kernel/pcic.c +0 -1
@@ -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 )
Signature · 1 line
1.8.1.4
PATCH 2/2 Convert PowerPC macro spin_event_timeout() to architecture independent macro
In reply to Arpit GoelThis 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.
--- include/linux/delay.h | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
include/linux/delay.h +40 -0
@@ -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) */
Signature · 1 line
1.8.1.4
Re: [PATCH 2/2] Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 6 lines · Arpit Goel
This patch ports PowerPC implementation of
spin_event_timeout()for generic use. Architecture specific implementation can be added toasm/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.
Quoted 30 lines
include/linux/delay.h +29
-0+#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; \
include/linux/delay.h static inline void ssleep(unsigned int seconds) · line 93
Quoted 10 lines
include/linux/delay.h +10
-0+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?
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 6 lines · Stephen Boyd
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)
Re: [PATCH 2/2] Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 8 lines · Timur Tabi
Quoted 5 lines · Stephen Boyd
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.
Quoted 4 lines
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()?
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 3 lines · Stephen Boyd
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.
Signature · 2 lines
-- Timur Tabi
Re: [PATCH 2/2] Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 7 lines · Timur Tabi
Quoted 3 lines · Stephen Boyd
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 tocpu_relax(). Is it possible thatcpu_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.
Quoted 4 lines
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.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 3 lines · Stephen Boyd
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?
Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
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
Signature · 1 line
Timur Tabi
Re: [PATCH 2/2] Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 6 lines · Timur Tabi
Quoted 3 lines · Stephen Boyd
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.
Quoted 3 lines
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.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
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
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 2/2 Convert PowerPC macro spin_event_timeout() to architecture independent macro
In reply to Stephen Boyd-----Original Message-----
From: [email protected] [mailto:linux-kernel-owner at vger.kernel.org] On Behalf Of Stephen Boyd
Sent: Thursday, August 01, 2013 5:34 AM
Quoted 4 lines
Quoted 2 lines
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.
Looks like the macro yields when 'delay', the third arg to the macro, is zero; otherwise the macro performs one or more udelays. If this is correct, can usleep_range() help?
PATCH 2/2 Convert PowerPC macro spin_event_timeout() to architecture independent macro
In reply to Stephen BoydQuoted 1 line · Thursday, August 01, 2013 5:34 AM Stephen Boyd [mailto:sboyd at…
BTW, couldn't we skip the first patch and just use
usecs_to_jiffies()?
[Arpit] It is a good idea to use usecs_to_jiffies() in spin_event_timeout() implementation. But wont moving USECS_PER_JIFFY from
C <arch/time.c> files to include <arch/timex.h> files also required as this would enable using USECS_PER_JIFFY.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] Convert PowerPC macro spin_event_timeout() to architecture independent macro
In reply to Arpit Goelinclude/linux/delay.h static inline void ssleep(unsigned int seconds) · line 56
Quoted 3 lines · Arpit Goel
include/linux/delay.h +2
-0+#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?
PATCH 2/2 Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 3 lines · Arpit Goel
include/linux/delay.h +2
-0+#ifndef spin_event_timeout +/**
Quoted 1 line · On Thursday, August 01, 2013 5:32 AM Timur Tabi [mailto:timur at tabi.org]…
Why don't you make PowerPC also use this generic version? It seems silly to have two virtually identical implementations of this macro?
PowerPC uses hardware timer registers provided by PowerPC architecture to implement spin_event_timeout(). These registers make PowerPC implementation of spin_event_timeout() more granular in comparison to ones using generic USECS_PER_JIFFY framework. This would handle those cases more granularly in which evaluation of "condition" is taking time or fails.
PATCH 2/2 Convert PowerPC macro spin_event_timeout() to architecture independent macro
Quoted 3 lines · Goel Arpit-B44344 wrote:
PowerPC uses hardware timer registers provided by PowerPC architecture to implement
spin_event_timeout(). These registers make PowerPC implementation ofspin_event_timeout()more granular in comparison to ones using genericUSECS_PER_JIFFYframework. This would handle those cases more granularly in which evaluation of "condition" is taking time or fails.
LOL, it seems I forgot how my own code works. Thanks for the reminder.
For some reason, I thought the PowerPC version was also using
'jiffies'. The concern about using 'jiffies' makes a lot more sense now.
Almost every architecture has a timebase counter. Wouldn't it make more sense to port tb_ticks_per_usec, get_tbl(), and tb_ticks_since() to other architectures instead? I don't think 'jiffies' is granular enough for spin_event_timeout().
Geert Uytterhoeven
maintainerQuoted 10 lines · Arpit Goel
This patch-set moves
USECS_PER_JIFFYto architecturetimex.hfrom architecture specific C files. Patch-set further uses this to make PPC macrospin_event_timeout()architecture independent. This change enables drivers to usespin_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_JIFFYto architecture specifictimex.hfiles from
architecture specific C files.
Quoted 11 lines
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>?
Quoted 4 lines
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 -- geert at linux-m68k.org
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
cover Make PPC macro spin_event_timeout() architecture independent
Quoted 11 lines · Arpit Goel
This patch-set moves
USECS_PER_JIFFYto architecturetimex.hfrom architecture specific C files. Patch-set further uses this to make PPC macrospin_event_timeout()architecture independent. This change enables drivers to usespin_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_JIFFYto architecture specifictimex.hfiles from
architecture specific C files.
Quoted 11 lines
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>?
[Arpit] I will add USECS_PER_JIFFY definition to <linux/timex.h> and will keep
definitions in <arch/timex.h> as it is. So that USECS_PER_JIFFY is available
for generic use and architecture specific implementation can override it.
Quoted 4 lines
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.
[Arpit] After adding USEC_PER_JIFFY to <linux/timex.h>, this issue can be addressed as well.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
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