RT: Misc. fixups for ARM/OMAP against 2.6.25.4-rt3

What follows are a series of minor fixes to get -rt3 building/booting on
ARM. The first 3 are ready do be included into the -rt patch, while
the last 4th one (cmpxchg duplication) needs a little investigating. The 5th one needs to go upstream via the OMAP tree.

This was build/boot tested on the TI OMAP3430 SDP platform

Kevin

---
 arch/arm/kernel/process.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

index 9308346..a0aad84 100644
@@ -171,12 +171,10 @@ void cpu_idle(void)
 			idle();
 		leds_event(led_idle_end);
 		local_irq_disable();
-		trace_preempt_exit_idle();
 		tick_nohz_restart_sched_tick();
 		__preempt_enable_no_resched();
 		__schedule();
 		preempt_disable();
-		trace_preempt_enter_idle();
 		local_irq_enable();
 	}
 }

1.5.4.1.166.g6706d

The macros used by BUILD_CHECK_IRQ_FLAGS() are defined in kernel.h, but this file is sometimes included before kernel.h (or even by kernel.h)

---
 include/linux/irqflags.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

index 6ada2b9..e3bf9d0 100644
@@ -11,11 +11,15 @@
 #ifndef _LINUX_TRACE_IRQFLAGS_H
 #define _LINUX_TRACE_IRQFLAGS_H
 
+#if defined(BUILD_CHECK_IRQ_FLAGS) && defined(typecheck)
 #define BUILD_CHECK_IRQ_FLAGS(flags)					\
 	do {								\
 		BUILD_BUG_ON(sizeof(flags) != sizeof(unsigned long));	\
 		typecheck(unsigned long, flags);			\
 	} while (0)
+#else
+#define BUILD_CHECK_IRQ_FLAGS(flags)
+#endif
 
 #ifdef CONFIG_TRACE_IRQFLAGS
   extern void trace_softirqs_on(unsigned long ip);

1.5.4.1.166.g6706d

---
 arch/arm/kernel/semaphore.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

index 6036f57..11f7d04 100644
@@ -49,7 +49,7 @@
  *    we cannot lose wakeup events.
  */
 
- void __attribute_used__ __compat_up(struct compat_semaphore *sem)
+ void __used __compat_up(struct compat_semaphore *sem)
 {
 	wake_up(&sem->wait);
 }

@@ -58,7 +58,7 @@ EXPORT_SYMBOL(__compat_up);
 
 static DEFINE_SPINLOCK(semaphore_lock);
 
- void __attribute_used__ __sched __compat_down(struct compat_semaphore * sem)
+ void __used __sched __compat_down(struct compat_semaphore * sem)
 {
 	struct task_struct *tsk = current;
 	DECLARE_WAITQUEUE(wait, tsk);

@@ -93,7 +93,7 @@ static DEFINE_SPINLOCK(semaphore_lock);
 
 EXPORT_SYMBOL(__compat_down);
 
- int __attribute_used__ __sched __compat_down_interruptible(struct compat_semaphore * sem)
+ int __used __sched __compat_down_interruptible(struct compat_semaphore * sem)
 {
 	int retval = 0;
 	struct task_struct *tsk = current;

@@ -154,7 +154,7 @@ EXPORT_SYMBOL(__compat_down_interruptible);
  * single "cmpxchg" without failure cases,
  * but then it wouldn't work on a 386.
  */
- int __attribute_used__ __sched __compat_down_trylock(struct compat_semaphore * sem)
+ int __used __sched __compat_down_trylock(struct compat_semaphore * sem)
 {
 	int sleepers;
 	unsigned long flags;

1.5.4.1.166.g6706d

FIXME: The ARM cmpxchg macro conflicts with the generic ones from cmpxchg-local.

---
 include/asm-arm/atomic.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index b5ba735..7cdc2fd 100644
@@ -146,7 +146,7 @@ static inline unsigned long __cmpxchg(volatile void *ptr,
 		return wrong_size_cmpxchg(ptr);
 }
 
-#define cmpxchg(ptr,o,n)						\
+#define cmpxchg__disabled(ptr,o,n)						\
 ({									\
 	__typeof__(*(ptr)) _o_ = (o);					\
 	__typeof__(*(ptr)) _n_ = (n);					\

1.5.4.1.166.g6706d

FIXME: The ARM cmpxchg macro conflicts with the generic ones from cmpxchg-local.

---
 include/asm-arm/atomic.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index b5ba735..7cdc2fd 100644
@@ -146,7 +146,7 @@ static inline unsigned long __cmpxchg(volatile void *ptr,
 		return wrong_size_cmpxchg(ptr);
 }
 
-#define cmpxchg(ptr,o,n)						\
+#define cmpxchg__disabled(ptr,o,n)						\
 ({									\
 	__typeof__(*(ptr)) _o_ = (o);					\
 	__typeof__(*(ptr)) _n_ = (n);					\

1.5.4.1.166.g6706d

The locking in the get_rate() hook is unnecessary, and causes problems when used with the -rt patch, since it may be called recursively.

---
 arch/arm/plat-omap/clock.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

index 714dbbf..3842540 100644
@@ -134,15 +134,12 @@ EXPORT_SYMBOL(clk_get_usecount);
 
 unsigned long clk_get_rate(struct clk *clk)
 {
-	unsigned long flags;
 	unsigned long ret = 0;
 
 	if (clk == NULL || IS_ERR(clk))
 		return 0;
 
-	spin_lock_irqsave(&clockfw_lock, flags);
 	ret = clk->rate;
-	spin_unlock_irqrestore(&clockfw_lock, flags);
 
 	return ret;
 }

1.5.4.1.166.g6706d

Hi Kevin,

BTW, looks like we could probably get rid of some of the other spinlocking in plat-omap/clock.c also -- clk_get_usecount() ?

- Paul

I agree, most of the locking in there seems unnecessary, but this was the only one that could be removed without looking closer at at all the platform specific clock code.

Basically, I think there minimal (maybe no) locking in the generic layer, and the locking should be done if necessary in the platform specific parts.

Kevin

To me it seems locking is needed. What if something is changing a clock's rate and then something else calls get_rate() while the other thread is changing the rate?

If get_rate() is not locking, get_rate() may get invalid rate between something else programming the hardware and changing clk->rate. I guess just locking set_rate() would still work with local irq disabled, but would not be SMP safe.

Anyways, ideally the spin_lock would be specific to the clocks accessing the same clock registers... But that would mean adding yet another field to struct clk or clock domains. Anybody got better ideas?

And of course anything going through the clocks list needs to be protected by the mutex.

Tony

Then the caller of get_rate() has an out of date value.

The same can happen even with the locking. If someone asks for the rate, and then another thread changes the rate right after, the thread who asked for the rate has an out-of-date value.

That being said, there may be some locks that need to stay, I haven't done a thorough analysis of this. I just think that there is currently to much unnecessary locking.

Agreed, the mutex needs to stay.

Kevin

Hi,

I don't understand how this would materialize in practice:

- if the rate is beign changed by 2 threads from the same driver, then the driver itself needs to take care of its internal state and coherency

- if 2 different drivers are contending the same clock, then 2 children clocks should be created and the clock fw would take care of failing the request of the 2nd child, if it is incompatible with the rate set by the 1st one

- if a dvfs change is responsible of changing the rate supplied to a driver which asked it with clk_get_rate(), the dvfs transition has no reason to happen in case it would violate the constraints set by clock users.

Does this make sense?

Cheers, Igor

---

Igor Stoppa
Nokia Devices R&D - Helsinki

Hello,

There aren't any such constraints in the current tree.

clk_set_rate() is mostly only useful for code that doesn't expect the clock rate to be changed by other code. Pre- and post-clk-rate-change notifiers should fix this for code that does need to know clock rates precisely.

- Paul

well I get the following error in my source code compilation time:
à partir de kernel/rtmutex.c:13:
include/asm/atomic.h:241:1: attention : « cmpxchg » redéfini
Dans le fichier inclus à partir de include/asm/system.h:378,
à partir de include/asm/bitops.h:27,
à partir de include/linux/bitops.h:17,
à partir de include/linux/thread_info.h:34,
à partir de include/linux/preempt.h:9,
à partir de include/linux/spinlock.h:85,
à partir de kernel/rtmutex.c:13:
include/asm-generic/cmpxchg.h:19:1: attention : ceci est la localisation d'une précédente définition
kernel/rtmutex.c:124: erreur: redefinition of 'init_lists' kernel/rtmutex.c:110: erreur: previous definition of 'init_lists' was here kernel/rtmutex.c: In function 'do_try_to_take_rt_mutex':
kernel/rtmutex.c:425: erreur: too many arguments to function 'try_to_steal_lock' kernel/rtmutex.c: In function 'rt_spin_lock_slowlock':
kernel/rtmutex.c:787: erreur: 'saved_lock_depth' undeclared (first use in this function)
kernel/rtmutex.c:787: erreur: (Each undeclared identifier is reported only once kernel/rtmutex.c:787: erreur: for each function it appears in.) kernel/rtmutex.c:788: erreur: implicit declaration of function 'rt_reacquire_bkl'
kernel/rtmutex.c: Hors de toute fonction :
kernel/rtmutex.c:1239: erreur: redefinition of 'rt_mutex_lock_killable' kernel/rtmutex.c:1218: erreur: previous definition of 'rt_mutex_lock_killable' was here
kernel/rtmutex.c:1245: erreur: redefinition of '__kstrtab_rt_mutex_lock_killable'
kernel/rtmutex.c:1224: erreur: previous definition of '__kstrtab_rt_mutex_lock_killable' was here
kernel/rtmutex.c:1245: erreur: redefinition of '__ksymtab_rt_mutex_lock_killable'
kernel/rtmutex.c:1224: erreur: previous definition of '__ksymtab_rt_mutex_lock_killable' was here
make[1]: *** [kernel/rtmutex.o] Error 1
make: *** [kernel] Error 2

and the following warning is noisy been shown:
include/asm/atomic.h:241:1: attention : « cmpxchg » redéfini Dans le fichier inclus à partir de include/asm/system.h:378,

------------------------------------------------------------------------------
Thanks && Regards
M.Sc. B.Sc. Luís Vitório Cargnini
IEEE Member
Electrical Engineer Faculty @ PUCRS
Ipiranga Avenue, 6681 – Building 30
P.O. Box: 90619-900 – Porto Alegre/RS
Phone: +55 51 3320 3500 extension: 7696
---------------------------------------------------------------------------------