This post continues after Keyboard from scratch. or Back to Blinky for a debugging session.

From a hardware perspective, there’s only a minor change. The interrupt line from the mcp23017 is connected to PA3 on the longan nano. I picked that one arbitrarily, since I was testing on this one before. This time, no picture. But here’s a circuit diagram. It’s .svg, so you can open it in a new tab and zoom as much as you want.

Going down the interrupt route

While the current plan for the firmware is not to have very piece of code in an interrupt context, the main loop should block in wfi as much as possible. I.e. we save on power and only do work after an interrupt came in.

In the end, I might rewrite it to be actually a huge statemachine which only advances in interrupts, but that’s harder to debug and work with than a main loop nudged by interrupts, so I’ll push that towards the end.

After a bit of refactoring, our code looks like this. Main difference is that main.c isn’t overburdened anymore and it’s easier to find stuff.

Enable incomming interrupts

The firmware already contains an interrupt handler and the surrounding enablement code for the A3 pad. This is mostly a relic from playing around with the device, but we’ll continue using it for good measure.

The main loop is also already prepared for interrupt pushing. Start of the loop goes into pmu_to_sleepmode(WFI_CMD); which puts us to sleep until we get an interrupt \o/.

Thus, as first step, we’ll connect the interrupt trigger pin from our GPIO expander to the pad on the MCU. I decided to run with GPIO_MODE_IPU. To properly support this, we need to set the interrupt trigger to open drain and should be active low.

Since I don’t know how to make pictures not oversized in text, I don’t think the extra cable is worth an image. Maybe connection plan later on? The code on the other hand, is online again.

First, let’s take a look at the code that sets the general configuration register of the mcp23017.

-    i2c_data_transmit(bus, 0x20);
+    i2c_data_transmit(bus, 0x64);

Previously it only set a single bit. This put the device into sequentiell mode, allowing us to make some configuration easier since writing twice writes to both port A and port B configuration registers. Now we added 2 more bits. They connect the interrupts, so we only have one logical interrupt pin for both ports, and sets the interrupt pin open drain. It’s already active low by default.

The new function set_mcp23017_interrupt_enabled is very boilerplate-y. It simply sets the interrupt enable registers for both ports to all 1. I.e. enables interrupts on all pins of the extender.

Further down, there’s some i2c_ack_config(bus, I2C_ACK_ENABLE); moving around. That’s a bugfix ;)

Reducing the race

The careful reader might have noticed, but right now we have a not too short race condition. When the interrupt fires while we aren’t in our sleep function, we will never wake up again. If you test this at home, make sure the USB is not connected here. In the current configuration it interrupts us around a thousand times a second and hides the effect.

To make this more obvious and easy to test, we’ll simply add in a delay. And suround it with led on/off to provide visual feedback. Code then looks like this.

The addition is really only usb_mdelay(500) and visualization around it. Now we can easily trigger the race. Just press a button and release it while the blue led is on. At this point the extender will try to interrupt us while we are not in our sleep function and we will go to sleep without handling the interrupt.

Well, we did handle the interrupt before, but we never read from the device, so it’s not cleared on the device side. Thus it can’t send a new interrupt for further IO changes.

Adding a memory

To fix this issue, we’ll just add some memory to our loop and interrupt combination and avoid going to sleep here, easy enough right?

The updated code does this.

There’s a new static volatile variable. We set it static since we only want to modify it from main and the interrupt handler for now. It’s volatile to ensure the compiler doesn’t optimize it out due to exection model mismatch of C and interrupts.

static volatile int interrupts = 0;

Then interrupt handler just increments it with ++interrupts;. Now everytime the handler ran, we will know and can avoid going to sleep, but re-enter the loop.

    if (!interrupts) {
        pmu_to_sleepmode(WFI_CMD);
    }
    interrupts = 0;

Looks easy enough, we’ll check if we should continue with the loop. If we don’t, we’ll go to sleep. Otherwise we clear the marker and loop on. Since we clear the marker before we read from the mcp23017, we might read newer state than the interrupt indicated, but we are sure we don’t miss a wakeup. And I’m not convinced that there’s a real chance for meaningful input we can miss here.

Non preempt section

In the previous section we added code to avoid a race condition.

Sadly, so far we’ve only reduced the critical section that can be interrupted. It’s a classical TOCTOU issue. If we move the delay it becomes obvious again. We still have a delay between checking our interrupts counter and the sleep. It’s minimal but a race none the less.

To fix this, we’ll have to prevent the interrupt from actually happening. If we disable interrupt handling before we check if an interrupt ran and re-enable it after we slept and reset the counter things work out. Even if we switch the order of reset and enable, things work out.

Noteworthy is probably, that we need to switch out our delay function. The usb_mdelay function from the USBFS example uses an interrupt to trigger itself. The delay_1ms function from the blinky example does not rely on interrupts and can run in the critical section.

In the end though, we don’t want it either way, so the final code removes it.

At this point we have a good state to base further work on again. But now with reliable interrupt triggered main loop operation. The series continues in Interrupts squared.