allow timer interrupt to be shared.

Hi,

For review, you will find attached to this mail a patch which allows Xenomai to run on ARM platforms where the timer interrupt is shared. Such a platform have to define the constant IPIPE_HAVE_SHARED_TIMER_IRQ, as well as two functions or macros:
- ipipe_timer_irq_p(), used by xnpod_announce_tick to test whether a timer interrupt is pending, in which case it is handled, otherwise xnpod_announce_tick returns with the XN_ISR_PROPAGATE so that the interrupt is propagated to the Linux domain.
- ipipe_mark_root_timer_irq(), used by xnarch_relay_tick so that the interrupt propagated by xnarch_relay_tick is treated by the root domain as a timer irq. As a matter of fact, since the interrupt is shared, the root domain has to know whether the irq is a timer irq, and since this interrupt is forged, the root domain can not rely on the same hardware mechanism as the xenomai domain irq.

The patch also contains a tiny patch of Xenomai syslib so that __tickAnnounce has the same return value as xnpod_announce_tick.

plain text document attachment (xeno-shared-timer-irq.diff)
Index: include/asm-generic/hal.h
===================================================================

asm-generic/hal.h +0 -0

 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Index: include/asm-generic/system.h
===================================================================

asm-generic/system.h +1 -0

+
 #endif /* !_XENO_ASM_GENERIC_SYSTEM_H */

Index: include/asm-arm/bits/intr.h
===================================================================

asm-arm/bits/intr.h +3 -0

@@ -27,6 +27,9 @@
 
 static inline void xnarch_relay_tick(void)
 {
+#ifdef RTHAL_HAVE_SHARED_TIMER_IRQ
+	rthal_mark_root_timer_irq();
+#endif /* RTHAL_HAVE_SHARED_TIMER_IRQ */
 	rthal_irq_host_pend(RTHAL_TIMER_IRQ);
 }
 

Index: ksrc/skins/vxworks/sysLib.c
===================================================================

skins/vxworks/sysLib.c +3 -2

@@ -24,19 +24,20 @@
 
 static wind_tick_handler_t tick_handler;
 static long tick_handler_arg;
+static int tick_status [XNARCH_NR_CPUS];
 
 void tickAnnounce(void)
 {
 	if (tick_handler != NULL)
 		tick_handler(tick_handler_arg);
 
-	xnpod_announce_tick(&nkclock);
+	tick_status[xnarch_current_cpu()] = xnpod_announce_tick(&nkclock);
 }
 
 static int __tickAnnounce(xnintr_t *intr)
 {
 	tickAnnounce();
-	return XN_ISR_HANDLED | XN_ISR_NOENABLE;
+	return tick_status[xnarch_current_cpu()];
 }
 
 int wind_sysclk_init(u_long init_rate)

Index: ksrc/nucleus/pod.c
===================================================================

nucleus/pod.c +5 -0

@@ -3249,6 +3249,11 @@
 {
 	xnsched_t *sched;
 
+#ifdef XNARCH_HAVE_SHARED_TIMER_IRQ
+	if (!xnarch_timer_irq_p())
+		return XN_ISR_NONE | XN_ISR_NOENABLE | XN_ISR_PROPAGATE;
+#endif /* XNARCH_HAVE_SHARED_TIMER_IRQ */
+	
 	sched = xnpod_current_sched();
 
 	xnlock_get(&nklock);

_______________________________________________
Xenomai-core mailing list
[email protected]
https://mail.gna.org/listinfo/xenomai-core

I'd rather keep the number of obscure conditional macros as low as possible; we should actually try to reduce them since we have a growing number of real and pseudo-archs to support, and those macros tend to obfuscate the generic code.

In the same vein, is there anything we could use in the compatible Adeos patch that would unambiguously identify the presence of such support without resorting to yet-another-macro like IPIPE_HAVE_SHARED_TIMER_IRQ?

Quoted 5 lines · Philippe Gerum

I'd rather keep the number of obscure conditional macros as low as possible; we should actually try to reduce them since we have a growing number of real and pseudo-archs to support, and those macros tend to obfuscate the generic code.

Quoted 3 lines

In the same vein, is there anything we could use in the compatible Adeos patch that would unambiguously identify the presence of such support without resorting to yet-another-macro like IPIPE_HAVE_SHARED_TIMER_IRQ?

We could decide that ipipe_timer_irq_p() must be implemented as a macro, and use #ifdef ipipe_timer_irq_p but I thought that #ifdef IPIPE_HAVE_SHARED_TIMER_IRQ was easier to understand.

Quoted 10 lines · Gilles Chanteperdrix

Quoted 8 lines · Philippe Gerum

I'd rather keep the number of obscure conditional macros as low as possible; we should actually try to reduce them since we have a growing number of real and pseudo-archs to support, and those macros tend to obfuscate the generic code.

In the same vein, is there anything we could use in the compatible Adeos patch that would unambiguously identify the presence of such support without resorting to yet-another-macro like IPIPE_HAVE_SHARED_TIMER_IRQ?

Quoted 4 lines

We could decide that ipipe_timer_irq_p() must be implemented as a macro, and use #ifdef ipipe_timer_irq_p but I thought that #ifdef IPIPE_HAVE_SHARED_TIMER_IRQ was easier to understand.

It is, but the point is that we should not define a normalized interface; older ARM patches are obsoleted by the very existence of the new one adding a required feature for PXA.

Therefore, at some point in
time, we are going to deprecate them, removing the conditional from the Xenomai codebase. In other words, this code is aimed at transitioning internally between two Adeos patch series, not at providing a stable interface.

Quoted 16 lines · Philippe Gerum

Quoted 14 lines · Gilles Chanteperdrix

Quoted 8 lines · Philippe Gerum

I'd rather keep the number of obscure conditional macros as low as possible; we should actually try to reduce them since we have a growing number of real and pseudo-archs to support, and those macros tend to obfuscate the generic code.

In the same vein, is there anything we could use in the compatible Adeos patch that would unambiguously identify the presence of such support without resorting to yet-another-macro like IPIPE_HAVE_SHARED_TIMER_IRQ?

We could decide that ipipe_timer_irq_p() must be implemented as a macro, and use #ifdef ipipe_timer_irq_p but I thought that #ifdef IPIPE_HAVE_SHARED_TIMER_IRQ was easier to understand.

Quoted 3 lines

It is, but the point is that we should not define a normalized interface; older ARM patches are obsoleted by the very existence of the new one adding a required feature for PXA.

AT91.

Quoted 5 lines

Therefore, at some point in
time, we are going to deprecate them, removing the conditional from the Xenomai codebase. In other words, this code is aimed at transitioning internally between two Adeos patch series, not at providing a stable interface.

Quoted 23 lines · Philippe Gerum

Quoted 11 lines · Philippe Gerum

Quoted 10 lines · Gilles Chanteperdrix

Quoted 8 lines · Philippe Gerum

I'd rather keep the number of obscure conditional macros as low as possible; we should actually try to reduce them since we have a growing number of real and pseudo-archs to support, and those macros tend to obfuscate the generic code.

In the same vein, is there anything we could use in the compatible Adeos patch that would unambiguously identify the presence of such support without resorting to yet-another-macro like IPIPE_HAVE_SHARED_TIMER_IRQ?

Quoted 8 lines

Quoted 4 lines

We could decide that ipipe_timer_irq_p() must be implemented as a macro, and use #ifdef ipipe_timer_irq_p but I thought that #ifdef IPIPE_HAVE_SHARED_TIMER_IRQ was easier to understand.

It is, but the point is that we should not define a normalized interface; older ARM patches are obsoleted by the very existence of the new one adding a required feature for PXA.

AT91.

Quoted 5 lines

Quoted 5 lines

Therefore, at some point in
time, we are going to deprecate them, removing the conditional from the Xenomai codebase. In other words, this code is aimed at transitioning internally between two Adeos patch series, not at providing a stable interface.

Here is a new version of the patch.

plain text document attachment (xeno-shared-timer-irq.2.diff)
Index: include/asm-generic/hal.h
===================================================================

asm-generic/hal.h +9 -0

@@ -679,6 +679,15 @@
 
 #endif /* CONFIG_IPIPE_TRACE */
 
+#ifdef ipipe_timer_irq_p
+/* Timer IRQ is shared. */
+#define rthal_timer_irq_p()         ipipe_timer_irq_p()
+#define rthal_mark_root_timer_irq() ipipe_mark_root_timer_irq()
+#else /* !IPIPE_SHARED_TIMER_IRQ */
+#define rthal_timer_irq_p()         1
+#define rthal_mark_root_timer_irq() do { } while(0)
+#endif /* IPIPE_SHARED_TIMER_IRQ */
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Index: include/asm-generic/system.h
===================================================================

asm-generic/system.h +2 -0

@@ -487,4 +487,6 @@
 #define xnarch_post_graph(obj,state)
 #define xnarch_post_graph_if(obj,state,cond)
 
+#define xnarch_timer_irq_p()         rthal_timer_irq_p()
+
 #endif /* !_XENO_ASM_GENERIC_SYSTEM_H */

Index: include/asm-arm/bits/intr.h
===================================================================

asm-arm/bits/intr.h +1 -0

@@ -27,6 +27,7 @@
 
 static inline void xnarch_relay_tick(void)
 {
+	rthal_mark_root_timer_irq();
 	rthal_irq_host_pend(RTHAL_TIMER_IRQ);
 }
 

Index: ksrc/skins/vxworks/sysLib.c
===================================================================

skins/vxworks/sysLib.c +3 -2

@@ -24,19 +24,20 @@
 
 static wind_tick_handler_t tick_handler;
 static long tick_handler_arg;
+static int tick_status [XNARCH_NR_CPUS];
 
 void tickAnnounce(void)
 {
 	if (tick_handler != NULL)
 		tick_handler(tick_handler_arg);
 
-	xnpod_announce_tick(&nkclock);
+	tick_status[xnarch_current_cpu()] = xnpod_announce_tick(&nkclock);
 }
 
 static int __tickAnnounce(xnintr_t *intr)
 {
 	tickAnnounce();
-	return XN_ISR_HANDLED | XN_ISR_NOENABLE;
+	return tick_status[xnarch_current_cpu()];
 }
 
 int wind_sysclk_init(u_long init_rate)

Index: ksrc/nucleus/pod.c
===================================================================

nucleus/pod.c +3 -0

@@ -3249,6 +3249,9 @@
 {
 	xnsched_t *sched;
 
+	if (!xnarch_timer_irq_p())
+		return XN_ISR_NONE | XN_ISR_NOENABLE | XN_ISR_PROPAGATE;
+	
 	sched = xnpod_current_sched();
 
 	xnlock_get(&nklock);