From 8b078a9e8f2da84d010c2e66292a75c1767a72f5 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Mon, 19 Sep 2022 17:54:45 +0200 Subject: [PATCH] machine: remove level triggered pin interrupts This removes level-triggered interrupts. While working on https://github.com/tinygo-org/tinygo/pull/3170, I found these level triggered interrupt constants. Apart from them being inconsistent with each other (PinLowLevel vs PinLevelLow) I don't think they are actually used anywhere. In addition, I removed the PinNoInterrupt constant on the esp32c3. This makes the esp32c3 pass the tests in #3170. I looked into level-triggered interrupts and I really couldn't find a good justification for them: - They were added to the esp32c3 and the rp2040 together with other pin interrupt types, meaning they were probably just added because the chip supports the feature and not because they were actually needed. - Level interrupts aren't supported in TinyGo for any other chip, and I haven't seen anybody ask for this feature. - They aren't supported in the nrf series chips _at all_, and with a quick search I found only very little demand for them in general. - I tried to see whether there is any good use case for them, but I couldn't really find one (where an edge triggered interrupt wouldn't work just as well). If there is one where level triggered interrupts are a real advantage over edge triggered interrupts, please let me know. Of course, we shouldn't remove a feature lightly. But in this case, I can't think of an advantage of having this feature. I can think of downsides: more maintenance and having to specify their behavior in the machine package documentation. In general, I would like to keep the machine package clean and only support things that have a proven use case. --- src/machine/machine_esp32c3.go | 7 ++----- src/machine/machine_mimxrt1062.go | 6 ++---- src/machine/machine_rp2040_gpio.go | 6 +----- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/machine/machine_esp32c3.go b/src/machine/machine_esp32c3.go index b57898b8..44c273df 100644 --- a/src/machine/machine_esp32c3.go +++ b/src/machine/machine_esp32c3.go @@ -59,12 +59,9 @@ type PinChange uint8 // Pin change interrupt constants for SetInterrupt. const ( - PinNoInterrupt PinChange = iota - PinRising + PinRising PinChange = iota + 1 PinFalling PinToggle - PinLowLevel - PinHighLevel ) // Configure this pin with the given configuration. @@ -190,7 +187,7 @@ func (p Pin) SetInterrupt(change PinChange, callback func(Pin)) (err error) { return ErrInvalidInputPin } - if callback == nil || change == PinNoInterrupt { + if callback == nil { // Disable this pin interrupt p.pin().ClearBits(esp.GPIO_PIN_PIN_INT_TYPE_Msk | esp.GPIO_PIN_PIN_INT_ENA_Msk) diff --git a/src/machine/machine_mimxrt1062.go b/src/machine/machine_mimxrt1062.go index acac9501..5fd3199f 100644 --- a/src/machine/machine_mimxrt1062.go +++ b/src/machine/machine_mimxrt1062.go @@ -54,9 +54,7 @@ const ( type PinChange uint8 const ( - PinLow PinChange = iota - PinHigh - PinRising + PinRising PinChange = iota + 2 PinFalling PinToggle ) @@ -393,7 +391,7 @@ func (p Pin) SetInterrupt(change PinChange, callback func(Pin)) error { mask := p.getMask() if nil != callback { switch change { - case PinLow, PinHigh, PinRising, PinFalling: + case PinRising, PinFalling: gpio.EDGE_SEL.ClearBits(mask) var reg *volatile.Register32 var pos uint8 diff --git a/src/machine/machine_rp2040_gpio.go b/src/machine/machine_rp2040_gpio.go index b19014b9..e60f2c3a 100644 --- a/src/machine/machine_rp2040_gpio.go +++ b/src/machine/machine_rp2040_gpio.go @@ -226,12 +226,8 @@ type PinChange uint8 // Pin change interrupt constants for SetInterrupt. const ( - // PinLevelLow triggers whenever pin is at a low (around 0V) logic level. - PinLevelLow PinChange = 1 << iota - // PinLevelLow triggers whenever pin is at a high (around 3V) logic level. - PinLevelHigh // Edge falling - PinFalling + PinFalling PinChange = 4 << iota // Edge rising PinRising )