chips/nrf5x: disallow registering multiple interrupts for a single GPIO

[lschuermann]

Pull Request Overview

This pull request ensures that only a single interrupt can be configured for a GPIO pin on the nRF5x at any given time.

The current nRF5x GPIO implementation allows registering multiple interrupts into the interrupt channels even for a single GPIO. However, all of those interrupts will still be routed to the same GPIOPin's client. Also, a second enabled interrupt would not be disabled by a single call to disable_interrupt.

Apart from these issues, other Tock code is not written under the assumption that there can be multiple interrupts fired by a single Pin event. For instance, when restarting an app, this will cause the button driver to issue two callbacks for every actual physical event to the restarted app.

This PR extends GPIOPin with an additional allocated_channel field that keeps track of the channel allocated. Only one interrupt channel can be allocated per GPIO. Using allocated_channel also avoids scanning over all channels in some cases.

Testing Strategy

This pull request was tested by restarting an app and ensuring that only a single button interrupt is delivered.

TODO or Help Wanted

N/A

Documentation Updated

• Updated the relevant files in /docs, or no updates are required.

Formatting

• Ran make prepush.

[brghena]

Small change and a question, but I do approve

[brghena]

Change to .clear() to make the intent more obvious?

[lschuermann]

Changed and fixed formatting.

[brghena]

This change seems reasonable, but isn't the bigger issue that an app 1) made a request that allocated some resource and then 2) stopped (or restarted in this case) but didn't free the resource. So if an app stops you'll keep getting GPIO interrupts for it, even though there's no client?

I wonder if we handle this resource dropped issue anywhere in Tock.

[lschuermann]

Yes, this is an issue and I believe it to be an issue at large in the Tock ecosystem. We don't currently have a good way of testing or exercising these cases, and the fact that an app restart can trigger this relatively simple bug seems to be a symptom of that.

However, for this particular case, it's actually quite clear what we're ought to do:

tock/capsules/core/src/button.rs

Lines 245 to 250 in ae86c83

	// It's possible we got an interrupt for a process that has since died
	// (and didn't unregister the interrupt). Lazily disable interrupts for
	// this button if so.
	if interrupt_count.get() == 0 {
	self.pins[pin_num as usize].0.disable_interrupts();
	}

The driver itself is managing these subscriptions and multiplexing multiple applications onto interrupts. As such, when an application dies, there might actually be other apps that are still interested in this subscription. The driver does then exercise the above logic to deregister clients when they're no longer needed, lazily upon getting a callback.

The issue here is really just that we assume enable_interrupt to be an idempotent function, when it is not for the nRF. And that actually seems like a reasonable design to me, if only for this subsystem.