pw_digital_io_mcuxpresso: Add McuxpressoDigitalInOutInterrupt support

This commit adds a new DigitalIo implementation backed by Mcuxpresso,
with input, output, and interrupt support. Compared to the existing
interrupt capable class (McuxpressoDigitalInInterrupt), this class uses
the GPIO interrupt block, rather than the PINT subsystem.

This enables using interrupts on more pins (because PINT does not
support all of the GPIO ports), an unbounded number of GPIOs (because
PINT has a total maximum number of interrupts supported), and allows
proper input and output support, getting and setting the state of the
line as needed.

The current version of the class does not support changing the direction
of the pin (due to the lack of such a method in the DigitalIo interface)

TEST: elipsitz and I tested the input, output and interrupt
functionality in 2 unique implementations

Change-Id: I6fda0d63b0beb9a93508734dfd0d6e0bc9a2c40c
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/247972
Lint: Lint 🤖 <[email protected]>
Commit-Queue: Richard Pasek <[email protected]>
Reviewed-by: Austin Foxley <[email protected]>
Reviewed-by: Jonathon Reinhart <[email protected]>
Reviewed-by: Eli Lipsitz <[email protected]>

pw_digital_io_mcuxpresso/BUILD.bazel

@@ -34,9 +34,11 @@ cc_library(
     ],
     strip_include_prefix = "public",
     deps = [
+        "//pw_containers:intrusive_forward_list",
         "//pw_digital_io",
         "//pw_preprocessor",
         "//pw_sync:borrow",
+        "//pw_sync:interrupt_spin_lock",
         "//targets:mcuxpresso_sdk",
     ],
 )

pw_digital_io_mcuxpresso/BUILD.gn

@@ -33,10 +33,12 @@ if (pw_third_party_mcuxpresso_SDK != "") {
       "public/pw_digital_io_mcuxpresso/interrupt_line.h",
     ]
     public_deps = [
+      "$dir_pw_containers:intrusive_forward_list",
       "$dir_pw_digital_io",
       "$dir_pw_result",
       "$dir_pw_status",
       "$dir_pw_sync:borrow",
+      "$dir_pw_sync:interrupt_spin_lock",
       "$pw_third_party_mcuxpresso_SDK",
     ]
     deps = [ "$dir_pw_assert" ]

pw_digital_io_mcuxpresso/digital_io.cc

@@ -15,19 +15,63 @@
 #include "pw_digital_io/digital_io.h"
 
 #include <array>
+#include <mutex>
 
 #include "fsl_clock.h"
 #include "fsl_gpio.h"
 #include "fsl_reset.h"
 #include "pw_assert/check.h"
 #include "pw_digital_io_mcuxpresso/digital_io.h"
 #include "pw_status/status.h"
+#include "pw_sync/interrupt_spin_lock.h"
 
 namespace pw::digital_io {
 namespace {
 
 constexpr std::array kGpioClocks = GPIO_CLOCKS;
 constexpr std::array kGpioResets = GPIO_RSTS_N;
+constexpr uint32_t kNumGpioPorts = GPIO_INTSTATA_COUNT;
+constexpr gpio_interrupt_index_t kGpioInterruptBankIndex = kGPIO_InterruptA;
+
+// This lock is used to prevent simultaneous access to the list of registered
+// interrupt handlers and the underlying interrupt hardware.
+sync::InterruptSpinLock port_interrupts_lock;
+
+// The HS GPIO block culminates all pin interrupts into single interrupt
+// vectors. Each GPIO port has a corresponding interrupt register and status
+// register.
+//
+// It would be expensive from a memory perspective to statically define a
+// handler pointer for each pin so a linked list used instead. It's added to
+// whenever a handler is registered with DoSetInterruptHandler() and removed
+// whenever a handler is de-registered with DoSetInterruptHandler().
+//
+// To improve handler lookup performance, an array of linked lists is created
+// below, 1 per port. This allows for traversal of smaller linked lists, and
+// only the linked lists that have active interrupts.
+std::array<pw::IntrusiveForwardList<McuxpressoDigitalInOutInterrupt>,
+           kNumGpioPorts>
+    port_interrupts PW_GUARDED_BY(port_interrupts_lock);
+
+uint32_t GPIO_PortGetInterruptEnable(GPIO_Type* base,
+                                     uint32_t port,
+                                     gpio_interrupt_index_t interrupt) {
+  switch (interrupt) {
+    case kGPIO_InterruptA:
+      return base->INTENA[port];
+    case kGPIO_InterruptB:
+      return base->INTENB[port];
+    default:
+      PW_CRASH("Invalid interrupt");
+  }
+}
+
+bool GPIO_PinGetInterruptEnable(GPIO_Type* base,
+                                uint32_t port,
+                                uint32_t pin,
+                                gpio_interrupt_index_t interrupt) {
+  return ((GPIO_PortGetInterruptEnable(base, port, interrupt) >> pin) & 1);
+}
 
 }  // namespace
 
@@ -126,4 +170,164 @@ pw::Result<pw::digital_io::State> McuxpressoDigitalIn::DoGetState() {
                     : pw::digital_io::State::kInactive;
 }
 
+McuxpressoDigitalInOutInterrupt::McuxpressoDigitalInOutInterrupt(
+    GPIO_Type* base, uint32_t port, uint32_t pin, bool output)
+    : base_(base), port_(port), pin_(pin), output_(output) {
+  PW_CHECK(base != nullptr);
+  PW_CHECK(port < kGpioClocks.size());
+  PW_CHECK(port < kGpioResets.size());
+  PW_CHECK(port < port_interrupts.size());
+}
+
+pw::Status McuxpressoDigitalInOutInterrupt::DoEnable(bool enable) {
+  if (!enable) {
+    enabled_ = enable;
+    // Can't disable clock as other users on same port may be active.
+    return pw::OkStatus();
+  }
+
+  if (is_enabled()) {
+    return pw::Status::FailedPrecondition();
+  }
+
+  CLOCK_EnableClock(kGpioClocks[port_]);
+  RESET_ClearPeripheralReset(kGpioResets[port_]);
+
+  gpio_pin_config_t config = {
+      .pinDirection = (output_ ? kGPIO_DigitalOutput : kGPIO_DigitalInput),
+      .outputLogic = 0,
+  };
+  GPIO_PinInit(base_, port_, pin_, &config);
+
+  enabled_ = enable;
+  return pw::OkStatus();
+}
+
+pw::Result<pw::digital_io::State>
+McuxpressoDigitalInOutInterrupt::DoGetState() {
+  if (!is_enabled()) {
+    return pw::Status::FailedPrecondition();
+  }
+
+  uint32_t value = GPIO_PinRead(base_, port_, pin_);
+  return value == 1 ? pw::digital_io::State::kActive
+                    : pw::digital_io::State::kInactive;
+}
+
+pw::Status McuxpressoDigitalInOutInterrupt::DoSetState(
+    pw::digital_io::State state) {
+  if (!is_enabled()) {
+    return pw::Status::FailedPrecondition();
+  }
+  GPIO_PinWrite(
+      base_, port_, pin_, state == pw::digital_io::State::kActive ? 1 : 0);
+
+  return pw::OkStatus();
+}
+
+pw::Status McuxpressoDigitalInOutInterrupt::DoSetInterruptHandler(
+    pw::digital_io::InterruptTrigger trigger,
+    pw::digital_io::InterruptHandler&& handler) {
+  if (handler == nullptr) {
+    std::lock_guard lock(port_interrupts_lock);
+    if (GPIO_PinGetInterruptEnable(
+            base_, port_, pin_, kGpioInterruptBankIndex)) {
+      // Can only clear handler when the interrupt is disabled
+      return pw::Status::FailedPrecondition();
+    }
+
+    unlist();
+
+    interrupt_handler_ = nullptr;
+    return pw::OkStatus();
+  }
+
+  if (interrupt_handler_ != nullptr) {
+    // Can only set a handler when none is set
+    return pw::Status::FailedPrecondition();
+  }
+
+  trigger_ = trigger;
+  gpio_pin_enable_polarity_t polarity;
+
+  switch (trigger_) {
+    case InterruptTrigger::kActivatingEdge:
+      polarity = kGPIO_PinIntEnableHighOrRise;
+      break;
+    case InterruptTrigger::kDeactivatingEdge:
+      polarity = kGPIO_PinIntEnableLowOrFall;
+      break;
+    default:
+      return pw::Status::InvalidArgument();
+  }
+
+  gpio_interrupt_config_t config = {
+      .mode = kGPIO_PinIntEnableEdge,
+      .polarity = static_cast<uint8_t>(polarity),
+  };
+
+  std::lock_guard lock(port_interrupts_lock);
+  GPIO_SetPinInterruptConfig(base_, port_, pin_, &config);
+
+  interrupt_handler_ = std::move(handler);
+
+  if (unlisted()) {
+    auto& list = port_interrupts[port_];
+    list.push_front(*this);
+  }
+
+  return pw::OkStatus();
+}
+
+pw::Status McuxpressoDigitalInOutInterrupt::DoEnableInterruptHandler(
+    bool enable) {
+  uint32_t mask = 1 << pin_;
+
+  std::lock_guard lock(port_interrupts_lock);
+
+  if (enable) {
+    if (interrupt_handler_ == nullptr) {
+      return pw::Status::FailedPrecondition();
+    }
+    GPIO_PortEnableInterrupts(base_, port_, kGpioInterruptBankIndex, mask);
+    NVIC_EnableIRQ(GPIO_INTA_IRQn);
+  } else {
+    GPIO_PortDisableInterrupts(base_, port_, kGpioInterruptBankIndex, mask);
+  }
+
+  return pw::OkStatus();
+}
+
+PW_EXTERN_C void GPIO_INTA_DriverIRQHandler() PW_NO_LOCK_SAFETY_ANALYSIS {
+  auto* base = GPIO;
+
+  // For each port
+  for (uint32_t port = 0; port < kNumGpioPorts; port++) {
+    const auto& list = port_interrupts[port];
+    const uint32_t port_int_flags =
+        GPIO_PortGetInterruptEnable(base, port, kGpioInterruptBankIndex) &
+        GPIO_PortGetInterruptStatus(base, port, kGpioInterruptBankIndex);
+
+    if (port_int_flags == 0) {
+      // If no flags are set, there is nothing to do. Skip traversing the linked
+      // list
+      continue;
+    }
+
+    // For each line registered on that port's interrupt list
+    for (const auto& line : list) {
+      if ((port_int_flags & (1UL << line.pin_)) != 0) {
+        line.interrupt_handler_(line.trigger_ ==
+                                        InterruptTrigger::kActivatingEdge
+                                    ? State::kActive
+                                    : State::kInactive);
+        GPIO_PinClearInterruptFlag(
+            base, port, line.pin_, kGpioInterruptBankIndex);
+      }
+    }
+  }
+
+  SDK_ISR_EXIT_BARRIER;
+}
+
 }  // namespace pw::digital_io

pw_digital_io_mcuxpresso/docs.rst

@@ -51,12 +51,25 @@ Example code to use GPIO pins from an NXP SDK board definition:
 
 .. code-block:: text
 
+   // Using outputs
    McuxpressoDigitalOut out(BOARD_INITPINS_D8_GPIO,
                            BOARD_INITPINS_D8_PORT,
                            BOARD_INITPINS_D8_PIN,
                            pw::digital_io::State::kActive);
    out.SetState(pw::digital_io::State::kInactive);
 
+   // Using inputs
    McuxpressoDigitalIn in(
       BOARD_INITPINS_D9_GPIO, BOARD_INITPINS_D9_PORT, BOARD_INITPINS_D9_PIN);
    auto state = in.GetState();
+
+   // Using inputs with a falling edge interrupt
+   McuxpressoDigitalInOutInterrupt inoutinterrupt(BOARD_INITPINS_D9_GPIO,
+                                                  BOARD_INITPINS_D9_PORT,
+                                                  BOARD_INITPINS_D9_PIN,
+                                                  false);
+   inoutinterrupt.SetInterruptHandler(
+      pw::digital_io::InterruptTrigger::kDeactivatingEdge,
+      [handler]);
+   inoutinterrupt.EnableInterruptHandler();
+   auto state = inoutinterrupt.GetState();

pw_digital_io_mcuxpresso/public/pw_digital_io_mcuxpresso/digital_io.h

@@ -14,10 +14,13 @@
 #pragma once
 
 #include "fsl_gpio.h"
+#include "pw_containers/intrusive_forward_list.h"
 #include "pw_digital_io/digital_io.h"
 
 namespace pw::digital_io {
 
+PW_EXTERN_C void GPIO_INTA_DriverIRQHandler();
+
 class McuxpressoDigitalOut : public pw::digital_io::DigitalOut {
  public:
   McuxpressoDigitalOut(GPIO_Type* base,
@@ -54,4 +57,34 @@ class McuxpressoDigitalIn : public pw::digital_io::DigitalIn {
   bool enabled_ = false;
 };
 
+class McuxpressoDigitalInOutInterrupt
+    : public pw::digital_io::DigitalInOutInterrupt,
+      public pw::IntrusiveForwardList<McuxpressoDigitalInOutInterrupt>::Item {
+ public:
+  McuxpressoDigitalInOutInterrupt(GPIO_Type* base,
+                                  uint32_t port,
+                                  uint32_t pin,
+                                  bool output);
+
+  bool is_enabled() const { return enabled_; }
+
+ private:
+  friend void GPIO_INTA_DriverIRQHandler();
+  pw::Status DoEnable(bool enable) override;
+  pw::Result<pw::digital_io::State> DoGetState() override;
+  pw::Status DoSetState(pw::digital_io::State state) override;
+  pw::Status DoSetInterruptHandler(
+      pw::digital_io::InterruptTrigger trigger,
+      pw::digital_io::InterruptHandler&& handler) override;
+  pw::Status DoEnableInterruptHandler(bool enable) override;
+
+  GPIO_Type* base_;
+  const uint32_t port_;
+  const uint32_t pin_;
+  const bool output_;
+  pw::digital_io::InterruptTrigger trigger_;
+  pw::digital_io::InterruptHandler interrupt_handler_;
+  bool enabled_ = false;
+};
+
 }  // namespace pw::digital_io