From 8a5fc59a80af2fb2b11c2cd16ecc21a210fe1065 Mon Sep 17 00:00:00 2001 From: Richard Pasek Date: Wed, 8 Jan 2025 21:36:49 -0800 Subject: [PATCH] pw_digital_io_mcuxpresso: Add McuxpressoDigitalInOutInterrupt support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 🤖 Commit-Queue: Richard Pasek Reviewed-by: Austin Foxley Reviewed-by: Jonathon Reinhart Reviewed-by: Eli Lipsitz --- pw_digital_io_mcuxpresso/BUILD.bazel | 2 + pw_digital_io_mcuxpresso/BUILD.gn | 2 + pw_digital_io_mcuxpresso/digital_io.cc | 204 ++++++++++++++++++ pw_digital_io_mcuxpresso/docs.rst | 13 ++ .../pw_digital_io_mcuxpresso/digital_io.h | 33 +++ 5 files changed, 254 insertions(+) diff --git a/pw_digital_io_mcuxpresso/BUILD.bazel b/pw_digital_io_mcuxpresso/BUILD.bazel index d07206e89e..3370a625ef 100644 --- a/pw_digital_io_mcuxpresso/BUILD.bazel +++ b/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", ], ) diff --git a/pw_digital_io_mcuxpresso/BUILD.gn b/pw_digital_io_mcuxpresso/BUILD.gn index 261226e21b..36d9c700cb 100644 --- a/pw_digital_io_mcuxpresso/BUILD.gn +++ b/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" ] diff --git a/pw_digital_io_mcuxpresso/digital_io.cc b/pw_digital_io_mcuxpresso/digital_io.cc index 690ee6cd79..c39ea50cdd 100644 --- a/pw_digital_io_mcuxpresso/digital_io.cc +++ b/pw_digital_io_mcuxpresso/digital_io.cc @@ -15,6 +15,7 @@ #include "pw_digital_io/digital_io.h" #include +#include #include "fsl_clock.h" #include "fsl_gpio.h" @@ -22,12 +23,55 @@ #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, + 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 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 +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(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 diff --git a/pw_digital_io_mcuxpresso/docs.rst b/pw_digital_io_mcuxpresso/docs.rst index ea303d4b40..53653d55e5 100644 --- a/pw_digital_io_mcuxpresso/docs.rst +++ b/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(); diff --git a/pw_digital_io_mcuxpresso/public/pw_digital_io_mcuxpresso/digital_io.h b/pw_digital_io_mcuxpresso/public/pw_digital_io_mcuxpresso/digital_io.h index 258f71875b..20114a93c5 100644 --- a/pw_digital_io_mcuxpresso/public/pw_digital_io_mcuxpresso/digital_io.h +++ b/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::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 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