[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.

5 files changed, 23 insertions, 6 deletions

diff --git a/src/target/firmware/include/layer1/async.h b/src/target/firmware/include/layer1/async.h
index 42c32232..16b016e3 100644
--- 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 41f443ac..68fee036 100644
--- 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 4b702ffc..c6e614fc 100644
--- 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 b5260cb9..9bf39212 100644
--- 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 763ec61c..02671af5 100644
--- 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));
 }