Conclave Assemble!
@tehn @zebra @scanner_darkly @ngwese @bpcmusic
A bit of background. If you run audio rate triggers above a certain frequency into the Teletype, it becomes unresponsive and may crash. If it doesn’t crash and you back off the rate, one of the following will happen:
- everything goes back to normal
- back to normal, but the keyboard needs an unplug and reconnect to start working again
- back to normal, but the CV and triggers get stuck
The last 2 issues are ones that seem to crop up repeatedly and even on other modules (e.g. needing to replug the grid on Ansible).
If I change all the IRQ masking in event.c to mask SYS_IRQ_PRIORITY rather than APP_TC_IRQ_PRIORITY, suddenly all the audio rate crashes go away. The system is still unresponsive with audio rate triggers, but as soon as they slow down everything is fine again. No stuck CV or triggers, no issues with the keyboard.
My strong suspicion is that there is some code somewhere that is calling into the event code at SYS_IRQ_PRIORITY, thus bypassing the current mask (I’ve tried looking…). But that the likelihood of it happening simultaneously with another call is very low. This is why the issue manifests itself reliably with audio rate triggers (high event density), and also why we see it when modules have been powered up for a long time.
I propose that we increase the IRQ masking to SYS_IRQ_PRIORITY, while it would be nice to get to the bottom of it, I haven’t the time (or the JTAG) to do it. And even if we did find the offending piece of code, there is an argument to be made that the event handling code is of such critical importance to the well being of the system that it should be masked at the highest level anyway.
However this change is in libavr32 and so will affect all the modules (possibly fixing some bugs in them too!). So before I make the change, I wouldn’t mind some of your collective time to think if there is anything I’ve missed or if I’m being daft in some way.
The diff is as follows:
diff --git a/src/events.c b/src/events.c
index 34bff5a..b5df442 100644
--- a/src/events.c
+++ b/src/events.c
@@ -52,7 +52,7 @@
// Returns non-zero if an event was available
u8 event_next( event_t *e ) {
u8 status;
- cpu_irq_disable_level(APP_TC_IRQ_PRIORITY);
+ cpu_irq_disable_level(SYS_IRQ_PRIORITY);
// if pointers are equal, the queue is empty... don't allow idx's to wrap!
if ( getIdx != putIdx ) {
@@ -66,7 +66,7 @@ u8 event_next( event_t *e ) {
status = false;
}
- cpu_irq_enable_level(APP_TC_IRQ_PRIORITY);
+ cpu_irq_enable_level(SYS_IRQ_PRIORITY);
return status;
}
@@ -79,7 +79,7 @@ u8 event_post( event_t *e ) {
// print_dbg("\r\n posting event, type: ");
// print_dbg_ulong(e->type);
- cpu_irq_disable_level(APP_TC_IRQ_PRIORITY);
+ cpu_irq_disable_level(SYS_IRQ_PRIORITY);
// increment write idx, posbily wrapping
saveIndex = putIdx;
@@ -93,7 +93,7 @@ u8 event_post( event_t *e ) {
putIdx = saveIndex;
}
- cpu_irq_enable_level(APP_TC_IRQ_PRIORITY);
+ cpu_irq_enable_level(SYS_IRQ_PRIORITY);
if (!status)
print_dbg("\r\n event queue full!");
Well the minutiae of interrupts are indeed mysterious, but oh-so very not cool.