[layer1] Fixed double IRQ bug

During IRQ handling, disabling and enabling IRQ may cause the same IRQ to
fire again. This is because the condition for this IRQ may be fullfilled
again, while still handling it. Using local_firq_save() and
local_irq_resore() intead, the IRQ handling will be completed before it is
cleared, and may then fire again.

The problem was detected during process of messages from layer23 to layer1.
In the IRQ context, the TX-functions of l23_api.c are called. There,
messages are queued and events are scheduled. During access to a queue or
a scheduler from any IRQ context or from normal context, interrupts must be
locked to prevent nested calls.

If it is not desired to call l23_api.c inside IRQ context, a message queue
must be used. If a message is written to that queue, it must be locked.
Afterwards a signal must be sent to the main process. The main process
locks the queue and de-queues the message. This is how it is done by all
layer 1 drivers of mISDN.

diff --git a/src/target/firmware/include/layer1/async.h b/src/target/firmware/include/layer1/async.h
index 42c3223..16b016e 100644 (file)
--- a/src/target/firmware/include/layer1/async.h
+++ b/src/target/firmware/include/layer1/async.h
@@ -5,6 +5,10 @@
 
 #include <layer1/mframe_sched.h>
 
+#if 0
+NOTE: Re-enabling interrupts causes an IRQ while processing the same IRQ.
+      Use local_firq_save and local_irq_restore instead!
+
 /* When altering data structures used by L1 Sync part, we need to
  * make sure to temporarily disable IRQ/FIQ to keep data consistent */
 static inline void l1a_lock_sync(void)
@@ -16,6 +20,7 @@ static inline void l1a_unlock_sync(void)
 {
        arm_enable_interrupts();
 }
+#endif
 
 /* safely enable a message into the L1S TX queue */
 void l1a_txq_msgb_enq(struct llist_head *queue, struct msgb *msg);

diff --git a/src/target/firmware/layer1/async.c b/src/target/firmware/layer1/async.c
index 41f443a..68fee03 100644 (file)
--- a/src/target/firmware/layer1/async.c
+++ b/src/target/firmware/layer1/async.c
@@ -39,9 +39,11 @@ extern const struct tdma_sched_item rach_sched_set_ul[];
 /* safely enable a message into the L1S TX queue */
 void l1a_txq_msgb_enq(struct llist_head *queue, struct msgb *msg)
 {
-       l1a_lock_sync();
+       unsigned long flags;
+
+       local_firq_save(flags);
        msgb_enqueue(queue, msg);
-       l1a_unlock_sync();
+       local_irq_restore(flags);
 }
 
 /* Enable a repeating multiframe task */

diff --git a/src/target/firmware/layer1/prim_freq.c b/src/target/firmware/layer1/prim_freq.c
index 4b702ff..c6e614f 100644 (file)
--- a/src/target/firmware/layer1/prim_freq.c
+++ b/src/target/firmware/layer1/prim_freq.c
@@ -38,6 +38,7 @@
 #include <calypso/dsp.h>
 #include <calypso/timer.h>
 #include <comm/sercomm.h>
+#include <asm/system.h>
 
 #include <layer1/sync.h>
 #include <layer1/async.h>
@@ -82,6 +83,7 @@ const struct tdma_sched_item freq_sched_set[] = {
 void l1a_freq_req(uint32_t fn_sched)
 {
        int32_t diff;
+       unsigned long flags;
 
        /* We must check here, if the time already elapsed.
         * This is required, because we may have an undefined delay between
@@ -103,8 +105,8 @@ void l1a_freq_req(uint32_t fn_sched)
        printf("Scheduling frequency change at fn=%u, currently fn=%u\n",
                fn_sched, l1s.current_time.fn);
 
-       l1a_lock_sync();
+       local_firq_save(flags);
        sched_gsmtime(freq_sched_set, fn_sched, 0);
-       l1a_unlock_sync();
+       local_irq_restore(flags);
 }
 

diff --git a/src/target/firmware/layer1/prim_pm.c b/src/target/firmware/layer1/prim_pm.c
index b5260cb..9bf3921 100644 (file)
--- a/src/target/firmware/layer1/prim_pm.c
+++ b/src/target/firmware/layer1/prim_pm.c
@@ -38,6 +38,7 @@
 #include <calypso/dsp.h>
 #include <calypso/timer.h>
 #include <comm/sercomm.h>
+#include <asm/system.h>
 
 #include <layer1/sync.h>
 #include <layer1/agc.h>
@@ -143,6 +144,11 @@ static const struct tdma_sched_item pm_sched_set[] = {
 /* Schedule a power measurement test */
 void l1s_pm_test(uint8_t base_fn, uint16_t arfcn)
 {
+       unsigned long flags;
+
        printd("l1s_pm_test(%u, %u)\n", base_fn, arfcn);
+
+       local_firq_save(flags);
        tdma_schedule_set(base_fn, pm_sched_set, arfcn);
+       local_irq_restore(flags);
 }

diff --git a/src/target/firmware/layer1/prim_rach.c b/src/target/firmware/layer1/prim_rach.c
index 763ec61..02671af 100644 (file)
--- a/src/target/firmware/layer1/prim_rach.c
+++ b/src/target/firmware/layer1/prim_rach.c
@@ -39,6 +39,7 @@
 #include <calypso/dsp.h>
 #include <calypso/timer.h>
 #include <comm/sercomm.h>
+#include <asm/system.h>
 
 #include <layer1/sync.h>
 #include <layer1/async.h>
@@ -118,15 +119,16 @@ static void l1a_rach_compl(__unused enum l1_compl c)
 void l1a_rach_req(uint8_t fn51, uint8_t mf_off, uint8_t ra)
 {
        uint32_t fn_sched;
+       unsigned long flags;
 
-       l1a_lock_sync();
+       local_firq_save(flags);
        l1s.rach.ra = ra;
        /* TODO: can we wrap here? I don't think so */
        fn_sched = l1s.current_time.fn - l1s.current_time.t3;
        fn_sched += mf_off * 51;
        fn_sched += fn51;
        sched_gsmtime(rach_sched_set_ul, fn_sched, 0);
-       l1a_unlock_sync();
+       local_irq_restore(flags);
 
        memset(&last_rach, 0, sizeof(last_rach));
 }