From 2aa0bb89cca0907a9976e5817d164135585677e9 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Tue, 6 Oct 2026 13:35:05 -0400 Subject: [PATCH 1/8] zephyr-cp: add pwmio for nordic boards iobroker allocates PWM channels in period groups (an nRF instance's four outputs share COUNTERTOP): `iobroker_pwm_channel_allocate()` adjusts the frequency to a period the hardware produces (truncating on nRF) and joins a running group with the same period, or takes a free instance. Joining or leaving configures only that output's PSEL, so running channels are not disturbed. `variable_frequency` gets an exclusive group, and `iobroker_pwm_period_cycles()` adjusts its new frequency. common-hal/pwmio uses only Zephyr's PWM API, so other SoCs need just an iobroker implementation (an RP2 slice or an STM32 timer is a group). Enabled where `CONFIG_PWM_NRFX` is set, like neopixel_write. Tested with a logic analyzer on the Feather nRF52840 and nRF54LM20 DK, and on the nRF54L15 DK. Co-Authored-By: Claude Opus 5.5 --- .../autogen_board_info.toml | 2 +- .../autogen_board_info.toml | 2 +- .../autogen_board_info.toml | 2 +- .../nordic/nrf5340dk/autogen_board_info.toml | 2 +- .../nordic/nrf54l15dk/autogen_board_info.toml | 2 +- .../nrf54lm20dk/autogen_board_info.toml | 2 +- ports/zephyr-cp/common-hal/pwmio/PWMOut.c | 135 ++++++++++++ ports/zephyr-cp/common-hal/pwmio/PWMOut.h | 27 +++ ports/zephyr-cp/common-hal/pwmio/__init__.c | 7 + ports/zephyr-cp/cptools/zephyr2cp.py | 5 +- .../internal-modules/iobroker/README.md | 19 +- .../iobroker/include/iobroker/iobroker.h | 60 +++++- .../internal-modules/iobroker/src/iobroker.c | 26 +++ .../iobroker/src/nordic/nrf/iobroker_route.c | 204 +++++++++++++++++- 14 files changed, 481 insertions(+), 14 deletions(-) create mode 100644 ports/zephyr-cp/common-hal/pwmio/PWMOut.c create mode 100644 ports/zephyr-cp/common-hal/pwmio/PWMOut.h create mode 100644 ports/zephyr-cp/common-hal/pwmio/__init__.c diff --git a/ports/zephyr-cp/boards/adafruit/clue_nrf52840_zephyr/autogen_board_info.toml b/ports/zephyr-cp/boards/adafruit/clue_nrf52840_zephyr/autogen_board_info.toml index 52fc4d7531d..94d72997164 100644 --- a/ports/zephyr-cp/boards/adafruit/clue_nrf52840_zephyr/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/adafruit/clue_nrf52840_zephyr/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/boards/adafruit/feather_nrf52840_sense_zephyr/autogen_board_info.toml b/ports/zephyr-cp/boards/adafruit/feather_nrf52840_sense_zephyr/autogen_board_info.toml index edbb32233c9..e6bffe7b239 100644 --- a/ports/zephyr-cp/boards/adafruit/feather_nrf52840_sense_zephyr/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/adafruit/feather_nrf52840_sense_zephyr/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/boards/adafruit/feather_nrf52840_zephyr/autogen_board_info.toml b/ports/zephyr-cp/boards/adafruit/feather_nrf52840_zephyr/autogen_board_info.toml index ca4ba51e5d5..ede7a19d609 100644 --- a/ports/zephyr-cp/boards/adafruit/feather_nrf52840_zephyr/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/adafruit/feather_nrf52840_zephyr/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/boards/nordic/nrf5340dk/autogen_board_info.toml b/ports/zephyr-cp/boards/nordic/nrf5340dk/autogen_board_info.toml index 8df24f2e0b6..e2a038aa69b 100644 --- a/ports/zephyr-cp/boards/nordic/nrf5340dk/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/nordic/nrf5340dk/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/boards/nordic/nrf54l15dk/autogen_board_info.toml b/ports/zephyr-cp/boards/nordic/nrf54l15dk/autogen_board_info.toml index 214981af521..c032ec4946c 100644 --- a/ports/zephyr-cp/boards/nordic/nrf54l15dk/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/nordic/nrf54l15dk/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/boards/nordic/nrf54lm20dk/autogen_board_info.toml b/ports/zephyr-cp/boards/nordic/nrf54lm20dk/autogen_board_info.toml index 59337c53a90..21e64f06bd8 100644 --- a/ports/zephyr-cp/boards/nordic/nrf54lm20dk/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/nordic/nrf54lm20dk/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c new file mode 100644 index 00000000000..5c856c31570 --- /dev/null +++ b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c @@ -0,0 +1,135 @@ +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Dan Halbert for Adafruit Industries LLC +// +// SPDX-License-Identifier: MIT + +// pwmio on Zephyr's PWM API. The iobroker module picks the PWM device and +// channel for the pin and adjusts the frequency to a period the hardware +// can produce; channels that share a period (an nRF instance's outputs) are +// grouped there, so nothing here is SoC-specific. + +#include + +#include +#include +#include + +#include "py/runtime.h" + +#include "shared-bindings/pwmio/PWMOut.h" + +static uint32_t pulse_cycles(uint32_t period_cycles, uint16_t duty) { + return (uint32_t)(((uint64_t)period_cycles * duty) / 0xffff); +} + +// Program the channel's period and the stored duty cycle. +static int set_period_and_duty(pwmio_pwmout_obj_t *self) { + return pwm_set_cycles(self->dev, self->channel, self->period_cycles, + pulse_cycles(self->period_cycles, self->duty_cycle), 0); +} + +pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, + const mcu_pin_obj_t *pin, uint16_t duty, uint32_t frequency, + bool variable_frequency) { + // Deinited until construction succeeds. + self->dev = NULL; + const struct device *dev; + uint32_t channel; + uint32_t period_cycles; + int ret = iobroker_pwm_channel_allocate(pin->package_pin, frequency, + variable_frequency, &dev, &channel, &period_cycles); + switch (ret) { + case 0: + break; + case -ERANGE: + return PWMOUT_INVALID_FREQUENCY; + case -ENODEV: + return PWMOUT_INTERNAL_RESOURCES_IN_USE; + case -EINVAL: + case -EBUSY: + case -ENOSYS: + return PWMOUT_INVALID_PIN; + default: + return PWMOUT_INITIALIZATION_ERROR; + } + + // The first channel of a group initializes the device; an existing + // group reports -EALREADY. + ret = device_init(dev); + if (ret < 0 && ret != -EALREADY) { + (void)iobroker_pwm_channel_release(dev, channel); + return PWMOUT_INITIALIZATION_ERROR; + } + + self->pin = pin; + self->dev = dev; + self->channel = channel; + self->period_cycles = period_cycles; + self->duty_cycle = duty; + self->variable_frequency = variable_frequency; + if (set_period_and_duty(self) < 0) { + (void)iobroker_pwm_channel_release(dev, channel); + self->dev = NULL; + return PWMOUT_INITIALIZATION_ERROR; + } + return PWMOUT_OK; +} + +bool common_hal_pwmio_pwmout_deinited(pwmio_pwmout_obj_t *self) { + return self->dev == NULL; +} + +void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { + if (common_hal_pwmio_pwmout_deinited(self)) { + return; + } + // Stop the output, then hand the channel back. + (void)pwm_set_cycles(self->dev, self->channel, self->period_cycles, 0, 0); + (void)iobroker_pwm_channel_release(self->dev, self->channel); + self->dev = NULL; + self->pin = NULL; +} + +void common_hal_pwmio_pwmout_set_duty_cycle(pwmio_pwmout_obj_t *self, uint16_t duty) { + self->duty_cycle = duty; + (void)set_period_and_duty(self); +} + +uint16_t common_hal_pwmio_pwmout_get_duty_cycle(pwmio_pwmout_obj_t *self) { + return self->duty_cycle; +} + +void common_hal_pwmio_pwmout_set_frequency(pwmio_pwmout_obj_t *self, uint32_t frequency) { + // Only used when variable_frequency=True, so the + // channel's group is exclusive and no other channel shares the period. + uint32_t period_cycles; + if (iobroker_pwm_period_cycles(self->dev, frequency, &period_cycles) < 0) { + common_hal_pwmio_pwmout_raise_error(PWMOUT_INVALID_FREQUENCY); + } + self->period_cycles = period_cycles; + (void)set_period_and_duty(self); +} + +uint32_t common_hal_pwmio_pwmout_get_frequency(pwmio_pwmout_obj_t *self) { + // The frequency the hardware actually produces. + uint64_t cycles_per_sec; + if (pwm_get_cycles_per_sec(self->dev, self->channel, &cycles_per_sec) < 0) { + return 0; + } + return (uint32_t)((cycles_per_sec + self->period_cycles / 2) / self->period_cycles); +} + +bool common_hal_pwmio_pwmout_get_variable_frequency(pwmio_pwmout_obj_t *self) { + return self->variable_frequency; +} + +const mcu_pin_obj_t *common_hal_pwmio_pwmout_get_pin(pwmio_pwmout_obj_t *self) { + return self->pin; +} + +void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { + // CIRCUITPY_BULK_RESET is off on this port: nothing is kept across + // resets, so there is nothing to undo. + (void)self; +} diff --git a/ports/zephyr-cp/common-hal/pwmio/PWMOut.h b/ports/zephyr-cp/common-hal/pwmio/PWMOut.h new file mode 100644 index 00000000000..fd5e1705183 --- /dev/null +++ b/ports/zephyr-cp/common-hal/pwmio/PWMOut.h @@ -0,0 +1,27 @@ +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Dan Halbert for Adafruit Industries LLC +// +// SPDX-License-Identifier: MIT + +#pragma once + +#include "common-hal/microcontroller/Pin.h" + +#include + +#include "py/obj.h" + +typedef struct { + mp_obj_base_t base; + const mcu_pin_obj_t *pin; + // PWM device and channel from iobroker_pwm_channel_allocate(); dev is + // NULL when deinited. + const struct device *dev; + uint32_t channel; + // Period in the device's PWM clock cycles, as iobroker computed it. Every + // pwm_set_cycles() call passes exactly this value. + uint32_t period_cycles; + uint16_t duty_cycle; + bool variable_frequency; +} pwmio_pwmout_obj_t; diff --git a/ports/zephyr-cp/common-hal/pwmio/__init__.c b/ports/zephyr-cp/common-hal/pwmio/__init__.c new file mode 100644 index 00000000000..14e9fd91630 --- /dev/null +++ b/ports/zephyr-cp/common-hal/pwmio/__init__.c @@ -0,0 +1,7 @@ +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Dan Halbert for Adafruit Industries LLC +// +// SPDX-License-Identifier: MIT + +// No pwmio module functions. diff --git a/ports/zephyr-cp/cptools/zephyr2cp.py b/ports/zephyr-cp/cptools/zephyr2cp.py index e0ecd4e0c9f..ed775d606bc 100644 --- a/ports/zephyr-cp/cptools/zephyr2cp.py +++ b/ports/zephyr-cp/cptools/zephyr2cp.py @@ -1404,9 +1404,12 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig # Without it, PWM nodes have no device to reference, and # neopixel_write (the neopixel module's nRF implementation, # which allocates an instance through iobroker) cannot transmit, so - # both the pwm pool and the module follow this symbol. + # both the pwm pool and the module follow this symbol. pwmio + # allocates its channels through iobroker's nRF implementation + # too. pwm_nrfx = line.strip().endswith("=y") board_info["neopixel_write"] = pwm_nrfx + board_info["pwmio"] = pwm_nrfx iobroker_includes = """ #include diff --git a/ports/zephyr-cp/internal-modules/iobroker/README.md b/ports/zephyr-cp/internal-modules/iobroker/README.md index a5eb1de8f1a..7c822bbccb7 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/README.md +++ b/ports/zephyr-cp/internal-modules/iobroker/README.md @@ -162,9 +162,22 @@ allocations the same way bus allocations conflict with each other. and returns both the GPIO controller device and the pin number within it. PWM instances are allocated whole with `iobroker_pwm_allocate()`, for one package pin routed to the instance's first output; the caller initializes the -device and returns it with `iobroker_release()`, as with the buses. Sharing an -instance between pins with the same base frequency (for pwmio) is not -supported yet. +device and returns it with `iobroker_release()`, as with the buses. +Individual PWM channels are allocated with +`iobroker_pwm_channel_allocate()`, which shares an instance between pins. +Channels that share a period form a group (for instance, on nRF, the four +outputs of an instance; on other SoCs this might be a slice or a timer). The +call takes a frequency, adjusts it to a period the hardware can produce +(truncating on nRF), and joins a running group with the same period and a +free channel, or takes a free instance. It returns the device, the channel +number and the group's period, which the caller passes unchanged to +`pwm_set_cycles()`. Joining or leaving a running group connects or +disconnects only that one output, so the group's other outputs are not +disturbed. An exclusive request (variable frequency) gets a group of its +own, and `iobroker_pwm_period_cycles()` adjusts a new frequency for it. +`iobroker_pwm_channel_release()` returns a channel; the group's last channel +releases the instance. A frequency the hardware cannot produce at all fails +with `-ERANGE`. Not every instance can reach every pad. On nRF52 and nRF53 the routing is a full crossbar, but on nRF54L peripherals and GPIO controllers are grouped in diff --git a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h index b611987e41e..369fb3a8f6a 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h +++ b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h @@ -166,6 +166,16 @@ typedef struct { // Used by iobroker_pin_in_use(); empty while the instance is free. package_pin_t pins[IOBROKER_MAX_PINS]; uint8_t pin_count; + // The fields below are used only for a PWM instance shared by channels + // (iobroker_pwm_channel_allocate()). Buses and whole-instance PWM + // allocations (iobroker_pwm_allocate()) leave them 0. + // Period, in PWM clock cycles, of every channel on the instance. + uint32_t period_cycles; + // True when the instance's one channel may change the period (variable + // frequency), so no other channel may join it. + bool exclusive; + // Bit n is set while output n is allocated to a channel. + uint8_t channels_used; } iobroker_state_t; // Generated board tables, emitted by the board's generated board.c whenever @@ -220,10 +230,56 @@ int iobroker_uart_allocate(package_pin_t tx, package_pin_t rx, // disconnected. The instance is not shared, so the caller may program all // of it (neopixel_write plays its own sequence). Like the bus allocate // functions, the caller initializes the device (device_init()) and returns -// it with iobroker_release(). Sharing an instance between pins with the same -// base frequency, as pwmio will want, is not supported yet. +// it with iobroker_release(). To share an instance between pins, use +// iobroker_pwm_channel_allocate() instead. int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out); +// Allocate one PWM output channel for a pin. Channels that share a period +// (all outputs of an nRF instance; on other SoCs a slice or a timer) form a +// group. frequency is in Hz and is adjusted to a period the hardware can +// produce (on nRF by truncation, so the actual frequency is the same or +// slightly higher); a request joins a running group whose period comes out +// the same, unless the group or the request is exclusive +// (variable-frequency use, where the holder changes the period). Otherwise a +// free group is taken. +// +// On success returns 0 and sets *dev_out, *channel_out (the channel number +// pwm_set_cycles() takes) and *period_cycles_out (the group's period in the +// device's PWM clock cycles). The caller must pass exactly that period to +// pwm_set_cycles(): drivers refuse a different period while other channels +// of the group run. The caller then calls device_init(), which returns +// -EALREADY if the group was already running. Joining a running group +// only connects the new pin; the group's other outputs are not disturbed. +// +// Returns a negative errno on failure, with the out parameters untouched: +// -ERANGE: the hardware cannot produce the frequency (including 0) +// -EINVAL: the pin is disconnected, not in the package map, has no GPIO, +// or no PWM group can reach it +// -EBUSY: the pin is already claimed +// -ENODEV: no group with a free channel is compatible with the request +// -ENOSYS: PWM channel allocation is unsupported on this SoC +// Every allocation must be paired with iobroker_pwm_channel_release(). +int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, + bool exclusive, const struct device **dev_out, uint32_t *channel_out, + uint32_t *period_cycles_out); + +// Release a channel allocated with iobroker_pwm_channel_allocate(): its pin +// is disconnected from the group and left quiescent. Releasing the group's +// last channel releases the whole group, de-initializing its device. The +// caller stops the channel's output (pwm_set_cycles() with a zero pulse) +// first. Returns true when the channel was allocated. +bool iobroker_pwm_channel_release(const struct device *dev, uint32_t channel); + +// Adjust frequency (Hz) to a period of an allocated PWM device, the same way +// iobroker_pwm_channel_allocate() does, for a caller holding an exclusive +// group that changes its frequency. Returns 0 and sets *period_cycles_out, +// or a negative errno with *period_cycles_out untouched: +// -ERANGE: the hardware cannot produce the frequency (including 0) +// -EINVAL: dev is not an iobroker PWM device +// -ENOSYS: PWM channel allocation is unsupported on this SoC +int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, + uint32_t *period_cycles_out); + // Allocate a package pin for the ADC. Analog inputs have no runtime routing: // the pad's analog input is fixed by the SoC, so the call resolves and // returns it in *input_out (the input selector the device's channel diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c b/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c index 8871d27f46b..72b1d4915e4 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c +++ b/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c @@ -137,6 +137,32 @@ int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out) { return -ENOSYS; } +int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, + bool exclusive, const struct device **dev_out, uint32_t *channel_out, + uint32_t *period_cycles_out) { + (void)pin; + (void)frequency; + (void)exclusive; + (void)dev_out; + (void)channel_out; + (void)period_cycles_out; + return -ENOSYS; +} + +bool iobroker_pwm_channel_release(const struct device *dev, uint32_t channel) { + (void)dev; + (void)channel; + return false; +} + +int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, + uint32_t *period_cycles_out) { + (void)dev; + (void)frequency; + (void)period_cycles_out; + return -ENOSYS; +} + bool iobroker_release(const struct device *dev) { (void)dev; LOG_DBG("release: no routing support on this SoC, nothing to release"); diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c b/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c index 0c835e7fccc..986d90b71b4 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c +++ b/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c @@ -23,9 +23,12 @@ #include #include +#include #include #include +#include + #include "iobroker_internal.h" LOG_MODULE_DECLARE(iobroker, CONFIG_LOG_DEFAULT_LEVEL); @@ -370,8 +373,6 @@ bool iobroker_release(const struct device *dev) { // De-init so that the device ends up de-initialized (like deferred-init // devices are after boot); the caller initializes it again when it // allocates the instance next. - // TODO: pwm_nrfx has no deinit hook (-ENOTSUP), which matters once pwmio - // initializes PWM instances. (void)device_deinit(dev); state->in_use = false; state->routed = false; @@ -533,4 +534,203 @@ int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out) { iobroker_pwm_bus_states, requested, pins, 4, dev_out); } +// Each PWM instance is one period group: its four outputs share COUNTERTOP +// and PRESCALER. +#define NRF_PWM_OUTPUTS 4 +BUILD_ASSERT(NRF_FUN_PWM_OUT1 == NRF_FUN_PWM_OUT0 + 1 && + NRF_FUN_PWM_OUT2 == NRF_FUN_PWM_OUT0 + 2 && + NRF_FUN_PWM_OUT3 == NRF_FUN_PWM_OUT0 + 3, + "PWM output function codes must be consecutive"); +// Smallest COUNTERTOP the hardware accepts (the nRF52 MDK has no _Min). +#define NRF_PWM_COUNTERTOP_MIN 3U + +// Adjust a frequency to a period of dev's PWM clock that pwm_nrfx programs +// exactly: COUNTERTOP << PRESCALER, with the smallest prescaler whose +// COUNTERTOP fits, as pwm_nrfx's pwm_period_check_and_set() picks it. A +// period of that form maps back to the same prescaler in the driver, so +// every channel of a group passes the identical period_cycles it requires. +// Both divisions truncate, as in ports/nordic, so the period is never +// longer than requested and the actual frequency is the same or slightly +// higher. Assumes the instance counts up only (pwm_nrfx's default, no +// center-aligned mode). +static int nrf_pwm_frequency_period(const struct device *dev, uint32_t frequency, + uint32_t *period_cycles_out) { + uint64_t cycles_per_sec; + if (frequency == 0 || pwm_get_cycles_per_sec(dev, 0, &cycles_per_sec) < 0) { + return -ERANGE; + } + // Period in undivided clock cycles. + uint64_t raw = cycles_per_sec / frequency; + for (uint32_t prescaler = 0; prescaler <= PWM_PRESCALER_PRESCALER_Msk; prescaler++) { + uint64_t countertop = raw >> prescaler; + if (countertop > PWM_COUNTERTOP_COUNTERTOP_Msk) { + continue; + } + if (countertop < NRF_PWM_COUNTERTOP_MIN) { + return -ERANGE; + } + *period_cycles_out = (uint32_t)(countertop << prescaler); + return 0; + } + return -ERANGE; +} + +static iobroker_state_t *nrf_pwm_state_find(const struct device *dev) { + for (size_t i = 0; i < iobroker_pwm_bus_count; i++) { + if (iobroker_pwm_buses[i].dev == dev) { + return &iobroker_pwm_bus_states[i]; + } + } + return NULL; +} + +int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, + uint32_t *period_cycles_out) { + if (nrf_pwm_state_find(dev) == NULL) { + return -EINVAL; + } + return nrf_pwm_frequency_period(dev, frequency, period_cycles_out); +} + +// Connect output `out` of a running instance to a pad, or disconnect it when +// pad is IOBROKER_NO_PIN. Only that output's PSEL (and its pad, when +// connecting) is configured, so the instance's other outputs keep running +// undisturbed. The stored states are updated too, so that the sleep state +// applied at de-init covers exactly the connected pads. +static int nrf_pwm_connect_output(const iobroker_instance_t *inst, + iobroker_state_t *state, uint8_t out, uint16_t pad) { + pinctrl_soc_pin_t psel = nrf_psel_encode(NRF_FUN_PWM_OUT0 + out, pad, false); + int ret = pinctrl_configure_pins(&psel, 1, inst->pcfg->reg); + if (ret < 0) { + LOG_WRN("pwm: connecting %s OUT%u failed: %d", inst->dev->name, + (unsigned)out, ret); + return ret; + } + state->default_pins[out] = psel; + state->sleep_pins[out] = psel | NRF_PSEL_LP; + return 0; +} + +int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, + bool exclusive, const struct device **dev_out, uint32_t *channel_out, + uint32_t *period_cycles_out) { + LOG_INF("pwm channel allocate: pin=%u frequency=%u exclusive=%u", + (unsigned)pin, (unsigned)frequency, (unsigned)exclusive); + // An out-of-range frequency is reported first, ahead of + // pin and busy errors. + bool in_range = false; + for (size_t i = 0; i < iobroker_pwm_bus_count && !in_range; i++) { + uint32_t unused; + in_range = nrf_pwm_frequency_period(iobroker_pwm_buses[i].dev, frequency, &unused) == 0; + } + if (!in_range) { + return -ERANGE; + } + const package_pin_t requested[] = { pin }; + int ret = iobroker_check_request("pwm", requested, 1); + if (ret < 0) { + return ret; + } + uint16_t soc_pad; + uint16_t pad; + if (pin == IOBROKER_NO_PIN || + iobroker_package_pin_soc_pad(pin, &soc_pad) < 0 || + iobroker_pad_gpio(soc_pad, &pad) < 0) { + LOG_WRN("pwm channel allocate: package pin %u is unknown or has no GPIO", + (unsigned)pin); + return -EINVAL; + } + + // Join a running group with the same period and a free output. + for (size_t i = 0; i < iobroker_pwm_bus_count && !exclusive; i++) { + const iobroker_instance_t *inst = &iobroker_pwm_buses[i]; + iobroker_state_t *state = &iobroker_pwm_bus_states[i]; + uint32_t period_cycles; + if (nrf_pwm_frequency_period(inst->dev, frequency, &period_cycles) < 0) { + continue; + } + if (!state->in_use || state->channels_used == 0 || state->exclusive || + state->period_cycles != period_cycles || + !nrf_instance_reaches_pad(inst->reg_addr, pad)) { + continue; + } + for (uint8_t out = 0; out < NRF_PWM_OUTPUTS; out++) { + if (state->channels_used & BIT(out)) { + continue; + } + if (nrf_pwm_connect_output(inst, state, out, pad) < 0) { + break; + } + state->channels_used |= BIT(out); + state->pins[out] = pin; + LOG_INF("pwm channel allocate: joined %s as OUT%u", inst->dev->name, + (unsigned)out); + *dev_out = inst->dev; + *channel_out = out; + *period_cycles_out = period_cycles; + return 0; + } + } + + // Otherwise take a free instance, with the pin on OUT0. The instance's + // clock decides the period, so check the range on the one allocated. + const struct device *dev; + ret = iobroker_pwm_allocate(pin, &dev); + if (ret < 0) { + // No new PWM available. + return ret; + } + uint32_t period_cycles; + if (nrf_pwm_frequency_period(dev, frequency, &period_cycles) < 0) { + // The range check at the top only showed that some instance can + // produce the frequency, not necessarily this one: the period + // depends on the instance's PWM clock. This can only fail on a SoC + // whose instances run from different clocks; on the nRF52, nRF53 + // and nRF54L every instance runs at 16 MHz. + (void)iobroker_release(dev); + iobroker_gpio_pad_quiesce(soc_pad); + return -ERANGE; + } + iobroker_state_t *state = nrf_pwm_state_find(dev); + state->period_cycles = period_cycles; + state->exclusive = exclusive; + state->channels_used = BIT(0); + *dev_out = dev; + *channel_out = 0; + *period_cycles_out = period_cycles; + return 0; +} + +bool iobroker_pwm_channel_release(const struct device *dev, uint32_t channel) { + iobroker_state_t *state = nrf_pwm_state_find(dev); + if (state == NULL || channel >= NRF_PWM_OUTPUTS || + !(state->channels_used & BIT(channel))) { + LOG_WRN("pwm channel release: %s channel %u is not allocated", + dev == NULL ? "(null)" : dev->name, (unsigned)channel); + return false; + } + const iobroker_instance_t *inst = &iobroker_pwm_buses[state - iobroker_pwm_bus_states]; + package_pin_t pin = state->pins[channel]; + uint16_t soc_pad; + bool have_pad = iobroker_package_pin_soc_pad(pin, &soc_pad) == 0; + + state->channels_used &= ~BIT(channel); + if (state->channels_used == 0) { + // Last channel: release the whole group. De-init applies the sleep + // state to the instance's pads. + state->period_cycles = 0; + state->exclusive = false; + (void)iobroker_release(dev); + } else { + (void)nrf_pwm_connect_output(inst, state, (uint8_t)channel, IOBROKER_NO_PIN); + state->pins[channel] = IOBROKER_NO_PIN; + } + if (have_pad) { + iobroker_gpio_pad_quiesce(soc_pad); + } + LOG_INF("pwm channel release: %s OUT%u (%s)", dev->name, (unsigned)channel, + state->in_use ? "group still running" : "group released"); + return true; +} + #endif // IOBROKER_ROUTING From 643e8d4e65e2cf2478a4143b16524b90488fb1da Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Wed, 7 Oct 2026 14:28:10 -0400 Subject: [PATCH 2/8] zephyr-cp: emulated PWM for pwmio on native_sim Add the pwm_emul module: an adafruit,pwm-emul controller that records what `pwm_set_cycles()` receives and, with Perfetto tracing, emits it on per-channel `pwm.NN.period` and `pwm.NN.pulse` counter tracks. Channel N stands for GPIO N. iobroker's emul/iobroker_pwm.c allocates its channels with a pool of period groups that any channel can join, which models only the pwmio rules every port shares: equal frequencies share a group, a variable frequency gets its own, and with no usable group left allocation fails. native_sim gets one 32-channel controller with two groups at 16 MHz, and zephyr2cp enables pwmio when `CONFIG_PWM_ADAFRUIT_EMUL` is set. Co-Authored-By: Claude Opus 5.5 --- ports/zephyr-cp/CMakeLists.txt | 3 + .../native/native_sim/autogen_board_info.toml | 2 +- .../boards/native/native_sim/board.conf | 4 + .../boards/native/native_sim/board.overlay | 13 ++ .../native_sim_asan/autogen_board_info.toml | 2 +- .../boards/native/native_sim_asan/board.conf | 4 + .../native_sim_lfs/autogen_board_info.toml | 2 +- .../boards/native/native_sim_lfs/board.conf | 4 + .../native/native_sim_lfs/board.overlay | 13 ++ ports/zephyr-cp/cptools/zephyr2cp.py | 7 +- .../internal-modules/iobroker/CMakeLists.txt | 6 + .../iobroker/include/iobroker/iobroker.h | 10 ++ .../iobroker/src/emul/iobroker_pwm.c | 166 ++++++++++++++++++ .../internal-modules/iobroker/src/iobroker.c | 21 ++- .../internal-modules/pwm_emul/CMakeLists.txt | 7 + .../internal-modules/pwm_emul/Kconfig | 12 ++ .../dts/bindings/pwm/adafruit,pwm-emul.yaml | 43 +++++ .../internal-modules/pwm_emul/src/pwm_emul.c | 129 ++++++++++++++ .../pwm_emul/zephyr/module.yml | 13 ++ 19 files changed, 450 insertions(+), 11 deletions(-) create mode 100644 ports/zephyr-cp/internal-modules/iobroker/src/emul/iobroker_pwm.c create mode 100644 ports/zephyr-cp/internal-modules/pwm_emul/CMakeLists.txt create mode 100644 ports/zephyr-cp/internal-modules/pwm_emul/Kconfig create mode 100644 ports/zephyr-cp/internal-modules/pwm_emul/dts/bindings/pwm/adafruit,pwm-emul.yaml create mode 100644 ports/zephyr-cp/internal-modules/pwm_emul/src/pwm_emul.c create mode 100644 ports/zephyr-cp/internal-modules/pwm_emul/zephyr/module.yml diff --git a/ports/zephyr-cp/CMakeLists.txt b/ports/zephyr-cp/CMakeLists.txt index 37b5f9e2648..d2e3272e4b6 100644 --- a/ports/zephyr-cp/CMakeLists.txt +++ b/ports/zephyr-cp/CMakeLists.txt @@ -10,6 +10,9 @@ set(ZEPHYR_EXTRA_MODULES # NeoPixel (WS2812-style) transmit on a runtime-chosen pin (see # internal-modules/neopixel/README.md). ${CMAKE_CURRENT_SOURCE_DIR}/internal-modules/neopixel + # Emulated PWM controller for native_sim tests (see + # internal-modules/pwm_emul/src/pwm_emul.c). + ${CMAKE_CURRENT_SOURCE_DIR}/internal-modules/pwm_emul # Kconfig-only module with defaults that must merge before Zephyr's # subsystem Kconfig files (see internal-modules/circuitpython-kconfig/Kconfig). ${CMAKE_CURRENT_SOURCE_DIR}/internal-modules/circuitpython-kconfig diff --git a/ports/zephyr-cp/boards/native/native_sim/autogen_board_info.toml b/ports/zephyr-cp/boards/native/native_sim/autogen_board_info.toml index fb371ac6da4..b541d286830 100644 --- a/ports/zephyr-cp/boards/native/native_sim/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/native/native_sim/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/boards/native/native_sim/board.conf b/ports/zephyr-cp/boards/native/native_sim/board.conf index 0937f18c87e..af7c546765e 100644 --- a/ports/zephyr-cp/boards/native/native_sim/board.conf +++ b/ports/zephyr-cp/boards/native/native_sim/board.conf @@ -40,6 +40,10 @@ CONFIG_ADC_EMUL=y # turns on CONFIG_DAC_TEST) CONFIG_DAC=y +# PWM emulation for pwmio testing (the adafruit,pwm-emul nodes in the board +# overlay turn on CONFIG_PWM_ADAFRUIT_EMUL) +CONFIG_PWM=y + # I2S SDL emulation for audio testing CONFIG_I2S_SDL=y diff --git a/ports/zephyr-cp/boards/native/native_sim/board.overlay b/ports/zephyr-cp/boards/native/native_sim/board.overlay index 23c0c88cb67..334c3dda916 100644 --- a/ports/zephyr-cp/boards/native/native_sim/board.overlay +++ b/ports/zephyr-cp/boards/native/native_sim/board.overlay @@ -52,5 +52,18 @@ }; }; +/* Emulated PWM controller for pwmio tests: one channel per gpio_emul pin + * and two period groups that any channel can join, clocked at 16 MHz. */ +/ { + pwm_emul: pwm-emul { + compatible = "adafruit,pwm-emul"; + status = "okay"; + #pwm-cells = <3>; + channels = <32>; + groups = <2>; + clock-frequency = <16000000>; + }; +}; + #include "../../../app.overlay" diff --git a/ports/zephyr-cp/boards/native/native_sim_asan/autogen_board_info.toml b/ports/zephyr-cp/boards/native/native_sim_asan/autogen_board_info.toml index fb371ac6da4..b541d286830 100644 --- a/ports/zephyr-cp/boards/native/native_sim_asan/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/native/native_sim_asan/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/boards/native/native_sim_asan/board.conf b/ports/zephyr-cp/boards/native/native_sim_asan/board.conf index 72f02a30094..89dfb6deda6 100644 --- a/ports/zephyr-cp/boards/native/native_sim_asan/board.conf +++ b/ports/zephyr-cp/boards/native/native_sim_asan/board.conf @@ -43,6 +43,10 @@ CONFIG_ADC_EMUL=y # turns on CONFIG_DAC_TEST) CONFIG_DAC=y +# PWM emulation for pwmio testing (the adafruit,pwm-emul nodes in the board +# overlay turn on CONFIG_PWM_ADAFRUIT_EMUL) +CONFIG_PWM=y + # I2S SDL emulation for audio testing CONFIG_I2S_SDL=y diff --git a/ports/zephyr-cp/boards/native/native_sim_lfs/autogen_board_info.toml b/ports/zephyr-cp/boards/native/native_sim_lfs/autogen_board_info.toml index 4490613d578..3837c212bbd 100644 --- a/ports/zephyr-cp/boards/native/native_sim_lfs/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/native/native_sim_lfs/autogen_board_info.toml @@ -85,7 +85,7 @@ paralleldisplaybus = false picogame = false ps2io = false pulseio = false -pwmio = false +pwmio = true # Zephyr board has pwmio qrio = false qspibus = false rainbowio = true diff --git a/ports/zephyr-cp/boards/native/native_sim_lfs/board.conf b/ports/zephyr-cp/boards/native/native_sim_lfs/board.conf index c80b9981ff1..c5769b18bef 100644 --- a/ports/zephyr-cp/boards/native/native_sim_lfs/board.conf +++ b/ports/zephyr-cp/boards/native/native_sim_lfs/board.conf @@ -36,6 +36,10 @@ CONFIG_ADC_EMUL=y # turns on CONFIG_DAC_TEST) CONFIG_DAC=y +# PWM emulation for pwmio testing (the adafruit,pwm-emul nodes in the board +# overlay turn on CONFIG_PWM_ADAFRUIT_EMUL) +CONFIG_PWM=y + # I2S SDL emulation for audio testing CONFIG_I2S_SDL=y diff --git a/ports/zephyr-cp/boards/native/native_sim_lfs/board.overlay b/ports/zephyr-cp/boards/native/native_sim_lfs/board.overlay index 495a16de5b7..d91d8c6b309 100644 --- a/ports/zephyr-cp/boards/native/native_sim_lfs/board.overlay +++ b/ports/zephyr-cp/boards/native/native_sim_lfs/board.overlay @@ -83,5 +83,18 @@ }; }; +/* Emulated PWM controller for pwmio tests: one channel per gpio_emul pin + * and two period groups that any channel can join, clocked at 16 MHz. */ +/ { + pwm_emul: pwm-emul { + compatible = "adafruit,pwm-emul"; + status = "okay"; + #pwm-cells = <3>; + channels = <32>; + groups = <2>; + clock-frequency = <16000000>; + }; +}; + #include "../../../app.overlay" diff --git a/ports/zephyr-cp/cptools/zephyr2cp.py b/ports/zephyr-cp/cptools/zephyr2cp.py index ed775d606bc..fee7a604db3 100644 --- a/ports/zephyr-cp/cptools/zephyr2cp.py +++ b/ports/zephyr-cp/cptools/zephyr2cp.py @@ -1395,6 +1395,7 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig # pins match their state. pinctrl_nrf = False pwm_nrfx = False + pwm_emul = False if config_present: for line in config.read_text().splitlines(): if line.startswith("CONFIG_PINCTRL_NRF="): @@ -1408,8 +1409,12 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig # allocates its channels through iobroker's nRF implementation # too. pwm_nrfx = line.strip().endswith("=y") + elif line.startswith("CONFIG_PWM_ADAFRUIT_EMUL="): + # The emulated PWM controllers on native_sim (pwm_emul + # module), which iobroker allocates pwmio channels on too. + pwm_emul = line.strip().endswith("=y") board_info["neopixel_write"] = pwm_nrfx - board_info["pwmio"] = pwm_nrfx + board_info["pwmio"] = pwm_nrfx or pwm_emul iobroker_includes = """ #include diff --git a/ports/zephyr-cp/internal-modules/iobroker/CMakeLists.txt b/ports/zephyr-cp/internal-modules/iobroker/CMakeLists.txt index 0bc5223d010..c4bc0642301 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/CMakeLists.txt +++ b/ports/zephyr-cp/internal-modules/iobroker/CMakeLists.txt @@ -34,6 +34,12 @@ endif() if(CONFIG_ADC_MCUX_LPADC OR CONFIG_DAC_MCUX_LPDAC) zephyr_library_sources(src/nxp/mcxn/iobroker_analog.c) endif() + +# PWM channel allocation for the emulated PWM controllers on native_sim (the +# nRF implementation is part of src/nordic/nrf/iobroker_route.c). +zephyr_library_sources_ifdef(CONFIG_PWM_ADAFRUIT_EMUL + src/emul/iobroker_pwm.c +) # Private headers shared by the core and the vendor implementations. zephyr_library_include_directories(src) diff --git a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h index 369fb3a8f6a..3094501369d 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h +++ b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h @@ -59,6 +59,16 @@ #define IOBROKER_DAC 0 #endif +// PWM channel allocation (iobroker_pwm_channel_allocate()) is available with +// nRF runtime routing (src/nordic/nrf/) and with the emulated PWM on +// native_sim (src/emul/). Without it the PWM channel functions report +// -ENOSYS. +#if IOBROKER_ROUTING || defined(CONFIG_PWM_ADAFRUIT_EMUL) +#define IOBROKER_PWM_CHANNELS 1 +#else +#define IOBROKER_PWM_CHANNELS 0 +#endif + // Signals needed by the widest bus (UART with tx/rx/rts/cts). #define IOBROKER_MAX_PINS 4 diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/emul/iobroker_pwm.c b/ports/zephyr-cp/internal-modules/iobroker/src/emul/iobroker_pwm.c new file mode 100644 index 00000000000..75c145465ab --- /dev/null +++ b/ports/zephyr-cp/internal-modules/iobroker/src/emul/iobroker_pwm.c @@ -0,0 +1,166 @@ +// Emulated PWM channel allocation for the iobroker module (native_sim). +// +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Dan Halbert for Adafruit Industries LLC +// +// SPDX-License-Identifier: MIT + +// The emulated PWM controller (adafruit,pwm-emul, see the pwm_emul module) +// has one channel per GPIO: the channel a pad drives is the pad's global +// GPIO number, as with the emulated ADC and DAC. Channels that share a +// period form a group; the controller has a pool of them (the groups +// property), and any channel can join any group, without a channel limit. +// So this models only the pwmio rules every SoC shares, not any SoC's +// layout: a fixed-frequency request joins a group running at the same +// period, a variable-frequency request takes a group of its own, and with +// no usable group left the request fails. The pad is claimed as a GPIO, +// which makes it busy for every other allocation. + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include "iobroker_internal.h" + +LOG_MODULE_DECLARE(iobroker, CONFIG_LOG_DEFAULT_LEVEL); + +#define EMUL_PWM_NODE DT_COMPAT_GET_ANY_STATUS_OKAY(adafruit_pwm_emul) +#define EMUL_PWM_CHANNELS DT_PROP(EMUL_PWM_NODE, channels) +#define EMUL_PWM_GROUPS DT_PROP(EMUL_PWM_NODE, groups) + +// Shortest period accepted, in clock cycles: a period needs a high and a low +// part. +#define EMUL_PWM_MIN_PERIOD 2U + +// Period groups. A group is free while it has no users. +typedef struct { + uint32_t period_cycles; + // One variable-frequency channel holds the group; nobody may join. + bool exclusive; + uint16_t users; +} emul_pwm_group_t; + +static emul_pwm_group_t groups[EMUL_PWM_GROUPS]; + +// The group each channel belongs to, while the channel is allocated. +static uint8_t channel_group[EMUL_PWM_CHANNELS]; +BUILD_ASSERT(EMUL_PWM_GROUPS <= UINT8_MAX, "group index must fit channel_group[]"); + +// Channels currently allocated, so release only drops claims it made. +static uint32_t allocated[DIV_ROUND_UP(EMUL_PWM_CHANNELS, 32)]; + +// Pick the group for a request: a running group at the same period that +// nobody holds exclusively, unless the request is exclusive itself, or else +// a free group. Returns the group index, or -1 when none is usable. +static int emul_pwm_group_pick(uint32_t period_cycles, bool exclusive) { + if (!exclusive) { + for (int i = 0; i < EMUL_PWM_GROUPS; i++) { + if (groups[i].users > 0 && !groups[i].exclusive && + groups[i].period_cycles == period_cycles) { + return i; + } + } + } + for (int i = 0; i < EMUL_PWM_GROUPS; i++) { + if (groups[i].users == 0) { + return i; + } + } + return -1; +} + +static const struct device *emul_pwm_device(void) { + return DEVICE_DT_GET(EMUL_PWM_NODE); +} + +// Adjust a frequency to a period of the clock by truncation, so the actual +// frequency is the same or slightly higher. +static int emul_pwm_frequency_period(uint32_t frequency, uint32_t *period_cycles_out) { + uint64_t cycles_per_sec; + if (frequency == 0 || pwm_get_cycles_per_sec(emul_pwm_device(), 0, &cycles_per_sec) < 0) { + return -ERANGE; + } + uint64_t period = cycles_per_sec / frequency; + if (period < EMUL_PWM_MIN_PERIOD || period > UINT32_MAX) { + return -ERANGE; + } + *period_cycles_out = (uint32_t)period; + return 0; +} + +int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, + uint32_t *period_cycles_out) { + if (dev != emul_pwm_device()) { + return -EINVAL; + } + return emul_pwm_frequency_period(frequency, period_cycles_out); +} + +int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, + bool exclusive, const struct device **dev_out, uint32_t *channel_out, + uint32_t *period_cycles_out) { + // An out-of-range frequency is reported first, ahead of pin and busy + // errors. + uint32_t period_cycles; + if (emul_pwm_frequency_period(frequency, &period_cycles) < 0) { + return -ERANGE; + } + uint16_t soc_pad; + uint16_t gpio_pad; + if (pin == IOBROKER_NO_PIN || iobroker_package_pin_soc_pad(pin, &soc_pad) < 0 || + iobroker_pad_gpio(soc_pad, &gpio_pad) < 0 || gpio_pad >= EMUL_PWM_CHANNELS) { + LOG_WRN("pwm channel allocate: package pin %u has no emulated PWM channel", + (unsigned)pin); + return -EINVAL; + } + int group = emul_pwm_group_pick(period_cycles, exclusive); + if (group < 0) { + LOG_WRN("pwm channel allocate: no usable period group for package pin %u", + (unsigned)pin); + return -ENODEV; + } + const struct device *port; + gpio_pin_t number; + int ret = iobroker_gpio_allocate(pin, &port, &number); + if (ret < 0) { + return ret; + } + if (groups[group].users == 0) { + groups[group].period_cycles = period_cycles; + groups[group].exclusive = exclusive; + } + groups[group].users++; + channel_group[gpio_pad] = (uint8_t)group; + allocated[gpio_pad / 32] |= BIT(gpio_pad % 32); + *dev_out = emul_pwm_device(); + *channel_out = gpio_pad; + *period_cycles_out = period_cycles; + LOG_INF("pwm channel allocate: package pin %u -> %s channel %u, group %d", + (unsigned)pin, (*dev_out)->name, (unsigned)gpio_pad, group); + return 0; +} + +bool iobroker_pwm_channel_release(const struct device *dev, uint32_t channel) { + if (dev != emul_pwm_device() || channel >= EMUL_PWM_CHANNELS || + !(allocated[channel / 32] & BIT(channel % 32))) { + LOG_WRN("pwm channel release: %s channel %u is not allocated", + dev == NULL ? "(null)" : dev->name, (unsigned)channel); + return false; + } + allocated[channel / 32] &= ~BIT(channel % 32); + groups[channel_group[channel]].users--; + const struct device *port; + gpio_pin_t number; + if (iobroker_gpio_split((uint16_t)channel, &port, &number) == 0) { + (void)iobroker_gpio_release(port, number); + } + return true; +} diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c b/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c index 72b1d4915e4..89ed1703fe0 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c +++ b/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c @@ -137,6 +137,19 @@ int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out) { return -ENOSYS; } +bool iobroker_release(const struct device *dev) { + (void)dev; + LOG_DBG("release: no routing support on this SoC, nothing to release"); + return false; +} + +#endif // !IOBROKER_ROUTING + +#if !IOBROKER_PWM_CHANNELS + +// SoCs without a PWM channel implementation: the API still exists so pwmio +// can call it, but every allocation reports -ENOSYS. The implementations +// live in src/// (nRF) and in src/emul/ (the emulated PWM). int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, bool exclusive, const struct device **dev_out, uint32_t *channel_out, uint32_t *period_cycles_out) { @@ -163,13 +176,7 @@ int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, return -ENOSYS; } -bool iobroker_release(const struct device *dev) { - (void)dev; - LOG_DBG("release: no routing support on this SoC, nothing to release"); - return false; -} - -#endif // !IOBROKER_ROUTING +#endif // !IOBROKER_PWM_CHANNELS #if !IOBROKER_ADC diff --git a/ports/zephyr-cp/internal-modules/pwm_emul/CMakeLists.txt b/ports/zephyr-cp/internal-modules/pwm_emul/CMakeLists.txt new file mode 100644 index 00000000000..b82aa8005d0 --- /dev/null +++ b/ports/zephyr-cp/internal-modules/pwm_emul/CMakeLists.txt @@ -0,0 +1,7 @@ +# pwm_emul Zephyr module build: the emulated PWM driver, built when the +# devicetree enables an adafruit,pwm-emul node (native_sim). + +if(CONFIG_PWM_ADAFRUIT_EMUL) + zephyr_library() + zephyr_library_sources(src/pwm_emul.c) +endif() diff --git a/ports/zephyr-cp/internal-modules/pwm_emul/Kconfig b/ports/zephyr-cp/internal-modules/pwm_emul/Kconfig new file mode 100644 index 00000000000..3fcf9e01c74 --- /dev/null +++ b/ports/zephyr-cp/internal-modules/pwm_emul/Kconfig @@ -0,0 +1,12 @@ +# pwm_emul: emulated PWM controller for native_sim tests. + +config PWM_ADAFRUIT_EMUL + bool "Emulated PWM controller" + default y + depends on DT_HAS_ADAFRUIT_PWM_EMUL_ENABLED + depends on PWM + help + Emulated PWM controller (adafruit,pwm-emul) for native_sim tests. It + produces no output; it records the period and pulse each channel is + set to and, with Perfetto tracing, emits them as counter tracks. + Channel N stands for GPIO N. diff --git a/ports/zephyr-cp/internal-modules/pwm_emul/dts/bindings/pwm/adafruit,pwm-emul.yaml b/ports/zephyr-cp/internal-modules/pwm_emul/dts/bindings/pwm/adafruit,pwm-emul.yaml new file mode 100644 index 00000000000..dcaa878600d --- /dev/null +++ b/ports/zephyr-cp/internal-modules/pwm_emul/dts/bindings/pwm/adafruit,pwm-emul.yaml @@ -0,0 +1,43 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 Dan Halbert for Adafruit Industries LLC +# SPDX-License-Identifier: MIT + +description: | + Emulated PWM controller for native_sim tests. It produces no output: it + records the period and pulse each channel is set to, and with Perfetto + tracing emits them on the counter tracks "pwm.NN.period" and + "pwm.NN.pulse". Channel N stands for GPIO N. Only one controller is + supported. + + Channels that share a period form a group. Any channel can join any + group, and a group has no channel limit; the number of groups is set by + the groups property. The driver itself only records settings: CircuitPython's + iobroker module hands out the groups. + +compatible: "adafruit,pwm-emul" + +include: [pwm-controller.yaml, base.yaml] + +properties: + channels: + required: true + + clock-frequency: + type: int + required: true + description: | + Frequency in Hz that pwm_get_cycles_per_sec() reports; periods and + pulses are counted in cycles of this clock. + + groups: + type: int + required: true + description: | + Number of period groups (sets of channels sharing a period) available. + + "#pwm-cells": + const: 3 + +pwm-cells: + - channel + - period + - flags diff --git a/ports/zephyr-cp/internal-modules/pwm_emul/src/pwm_emul.c b/ports/zephyr-cp/internal-modules/pwm_emul/src/pwm_emul.c new file mode 100644 index 00000000000..6dd982d7d73 --- /dev/null +++ b/ports/zephyr-cp/internal-modules/pwm_emul/src/pwm_emul.c @@ -0,0 +1,129 @@ +// pwm_emul: emulated PWM controller for native_sim tests. +// +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Dan Halbert for Adafruit Industries LLC +// +// SPDX-License-Identifier: MIT + +// The emulated controller (adafruit,pwm-emul) implements Zephyr's PWM API +// but produces no output. Its channels are independent, each with its own +// period, and channel N stands for GPIO N. set_cycles() records the period +// and pulse and, with Perfetto tracing, emits them on the counter tracks +// "pwm.NN.period" and "pwm.NN.pulse" (matching the "gpio_emul.NN" tracks). + +#include +#include +#include +#include + +#include +#include +#include +#include + +#if defined(CONFIG_TRACING_PERFETTO) +#include "perfetto_encoder.h" +#endif + +#define DT_DRV_COMPAT adafruit_pwm_emul + +// Channel numbers are GPIO numbers, so one controller covers every pin and +// the track names need no controller prefix. +BUILD_ASSERT(DT_NUM_INST_STATUS_OKAY(DT_DRV_COMPAT) <= 1, + "only one adafruit,pwm-emul controller is supported"); + +struct pwm_emul_channel { + uint32_t period_cycles; + uint32_t pulse_cycles; + // Whether this channel's tracks have been described. + bool described; +}; + +struct pwm_emul_config { + uint32_t clock_frequency; + struct pwm_emul_channel *channels; + uint32_t channel_count; +}; + +#if defined(CONFIG_TRACING_PERFETTO) + +// Track UUIDs, clear of the ranges the Zephyr encoder and the GPIO tracks +// use: a "PWM" group track, then two counter tracks per channel (period, +// then pulse). +#define PWM_EMUL_GROUP_TRACK_UUID 0x50000000ULL +#define PWM_EMUL_TRACK_UUID(channel, pulse) \ + (PWM_EMUL_GROUP_TRACK_UUID + 1 + (uint64_t)(channel) * 2 + ((pulse) ? 1 : 0)) + +static bool group_described; + +static void trace_channel(uint32_t channel, struct pwm_emul_channel *ch) { + if (!perfetto_start()) { + return; + } + if (!group_described) { + perfetto_emit_track_descriptor(PWM_EMUL_GROUP_TRACK_UUID, + perfetto_get_trace_uuid(), "PWM"); + group_described = true; + } + if (!ch->described) { + char name[24]; + snprintf(name, sizeof(name), "pwm.%02u.period", (unsigned)channel); + perfetto_emit_counter_track_descriptor(PWM_EMUL_TRACK_UUID(channel, false), + PWM_EMUL_GROUP_TRACK_UUID, name, PERFETTO_COUNTER_UNIT_COUNT); + snprintf(name, sizeof(name), "pwm.%02u.pulse", (unsigned)channel); + perfetto_emit_counter_track_descriptor(PWM_EMUL_TRACK_UUID(channel, true), + PWM_EMUL_GROUP_TRACK_UUID, name, PERFETTO_COUNTER_UNIT_COUNT); + ch->described = true; + } + perfetto_emit_counter(PWM_EMUL_TRACK_UUID(channel, false), ch->period_cycles); + perfetto_emit_counter(PWM_EMUL_TRACK_UUID(channel, true), ch->pulse_cycles); +} + +#else + +static void trace_channel(uint32_t channel, struct pwm_emul_channel *ch) { + (void)channel; + (void)ch; +} + +#endif // CONFIG_TRACING_PERFETTO + +static int pwm_emul_set_cycles(const struct device *dev, uint32_t channel, + uint32_t period_cycles, uint32_t pulse_cycles, pwm_flags_t flags) { + (void)flags; + const struct pwm_emul_config *config = dev->config; + if (channel >= config->channel_count) { + return -EINVAL; + } + struct pwm_emul_channel *ch = &config->channels[channel]; + ch->period_cycles = period_cycles; + ch->pulse_cycles = pulse_cycles; + trace_channel(channel, ch); + return 0; +} + +static int pwm_emul_get_cycles_per_sec(const struct device *dev, uint32_t channel, + uint64_t *cycles) { + (void)channel; + const struct pwm_emul_config *config = dev->config; + *cycles = config->clock_frequency; + return 0; +} + +static DEVICE_API(pwm, pwm_emul_api) = { + .set_cycles = pwm_emul_set_cycles, + .get_cycles_per_sec = pwm_emul_get_cycles_per_sec, +}; + +#define PWM_EMUL_DEFINE(n) \ + static struct pwm_emul_channel pwm_emul_channels_##n[DT_INST_PROP(n, channels)]; \ + static const struct pwm_emul_config pwm_emul_config_##n = { \ + .clock_frequency = DT_INST_PROP(n, clock_frequency), \ + .channels = pwm_emul_channels_##n, \ + .channel_count = DT_INST_PROP(n, channels), \ + }; \ + DEVICE_DT_INST_DEFINE(n, NULL, NULL, NULL, &pwm_emul_config_##n, POST_KERNEL, \ + CONFIG_PWM_INIT_PRIORITY, &pwm_emul_api); + +DT_INST_FOREACH_STATUS_OKAY(PWM_EMUL_DEFINE) diff --git a/ports/zephyr-cp/internal-modules/pwm_emul/zephyr/module.yml b/ports/zephyr-cp/internal-modules/pwm_emul/zephyr/module.yml new file mode 100644 index 00000000000..836c05423cb --- /dev/null +++ b/ports/zephyr-cp/internal-modules/pwm_emul/zephyr/module.yml @@ -0,0 +1,13 @@ +# Zephyr module descriptor for the pwm_emul module. +# +# The module is loaded from the CircuitPython tree via ZEPHYR_EXTRA_MODULES in +# ports/zephyr-cp/CMakeLists.txt. To externalize it, move this directory into +# its own repository and either point ZEPHYR_EXTRA_MODULES at the new checkout +# or add it to the west manifest instead. +name: pwm_emul +build: + cmake: . + kconfig: Kconfig + settings: + # dts/bindings/ holds the adafruit,pwm-emul binding. + dts_root: . From 777cbd69b8462d38c392c89d9137124fe0df0fdd Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Wed, 7 Oct 2026 14:28:11 -0400 Subject: [PATCH 3/8] zephyr-cp: add pwmio tests on native_sim Check the period and pulse traced for each pin and pwmio's behavior: frequency and duty conversion, the reported frequency, frequency changes, invalid frequencies, pin claims, group sharing and exhaustion, and variable-frequency groups. Co-Authored-By: Claude Opus 5.5 --- ports/zephyr-cp/tests/test_pwmio.py | 273 ++++++++++++++++++++++++++++ 1 file changed, 273 insertions(+) create mode 100644 ports/zephyr-cp/tests/test_pwmio.py diff --git a/ports/zephyr-cp/tests/test_pwmio.py b/ports/zephyr-cp/tests/test_pwmio.py new file mode 100644 index 00000000000..f0a3309e1cf --- /dev/null +++ b/ports/zephyr-cp/tests/test_pwmio.py @@ -0,0 +1,273 @@ +# SPDX-FileCopyrightText: 2026 Dan Halbert for Adafruit Industries LLC +# SPDX-License-Identifier: MIT + +"""Test pwmio on native_sim. + +The emulated PWM controller (adafruit,pwm-emul) has one channel per gpio_emul +pin and two period groups that any channel can join, clocked at 16 MHz. It +records what pwm_set_cycles() receives on the counter tracks "pwm.NN.period" +and "pwm.NN.pulse". The tests check pwmio's own behavior and the grouping +rules every port shares, not any SoC's layout. +""" + +import pytest +from perfetto.trace_processor import TraceProcessor + +CLOCK = 16_000_000 + + +def _track_values(trace_file, track_name: str) -> list[int]: + tp = TraceProcessor(file_path=str(trace_file)) + result = tp.query( + f''' + SELECT c.value + FROM counter c + JOIN track t ON c.track_id = t.id + WHERE t.name = "{track_name}" + ORDER BY c.ts + ''' + ) + return [int(row.value) for row in result] + + +def _pulse(period: int, duty: int) -> int: + return period * duty // 0xFFFF + + +PWMIO_SETTINGS_CODE = """\ +import microcontroller +import pwmio + +pwm = pwmio.PWMOut(microcontroller.pin.P_05, frequency=1000, duty_cycle=0x4000) +print(f"frequency {pwm.frequency}") +pwm.duty_cycle = 0xC000 +pwm.duty_cycle = 0xFFFF +pwm.deinit() +print("done") +""" + + +@pytest.mark.duration(8.0) +@pytest.mark.circuitpy_drive({"code.py": PWMIO_SETTINGS_CODE}) +def test_pwmio_settings(circuitpython): + """Frequency becomes the period, duty the pulse, and deinit stops the output.""" + circuitpython.wait_until_done() + output = circuitpython.serial.all_output + assert "frequency 1000" in output + assert "done" in output + + period = CLOCK // 1000 + trace = circuitpython.trace_file + assert _track_values(trace, "pwm.05.period") == [period] * 4 + assert _track_values(trace, "pwm.05.pulse") == [ + _pulse(period, 0x4000), + _pulse(period, 0xC000), + period, + 0, + ] + + +PWMIO_FREQUENCY_CODE = """\ +import microcontroller +import pwmio + +pwm = pwmio.PWMOut(microcontroller.pin.P_05, frequency=12345) +print(f"frequency {pwm.frequency}") +pwm.deinit() +print("done") +""" + + +@pytest.mark.duration(8.0) +@pytest.mark.circuitpy_drive({"code.py": PWMIO_FREQUENCY_CODE}) +def test_pwmio_frequency_is_actual(circuitpython): + """frequency reports what the period produces, not what was requested.""" + circuitpython.wait_until_done() + output = circuitpython.serial.all_output + assert "done" in output + + period = _track_values(circuitpython.trace_file, "pwm.05.period")[0] + assert period == CLOCK // 12345 + # 16 MHz / 1296 = 12345.7 Hz, reported to the nearest Hz. + assert f"frequency {round(CLOCK / period)}" in output + + +PWMIO_VARIABLE_FREQUENCY_CODE = """\ +import microcontroller +import pwmio + +pwm = pwmio.PWMOut( + microcontroller.pin.P_05, frequency=440, duty_cycle=0x8000, variable_frequency=True +) +pwm.frequency = 880 +print(f"frequency {pwm.frequency}") +print("done") +""" + + +@pytest.mark.duration(8.0) +@pytest.mark.circuitpy_drive({"code.py": PWMIO_VARIABLE_FREQUENCY_CODE}) +def test_pwmio_variable_frequency_change(circuitpython): + """Changing the frequency writes a new period and keeps the duty cycle.""" + circuitpython.wait_until_done() + output = circuitpython.serial.all_output + assert "frequency 880" in output + assert "done" in output + + trace = circuitpython.trace_file + assert _track_values(trace, "pwm.05.period") == [CLOCK // 440, CLOCK // 880] + assert _track_values(trace, "pwm.05.pulse") == [ + _pulse(CLOCK // 440, 0x8000), + _pulse(CLOCK // 880, 0x8000), + ] + + +PWMIO_INVALID_FREQUENCY_CODE = """\ +import microcontroller +import pwmio + +for frequency in (0, 20_000_000): + try: + pwmio.PWMOut(microcontroller.pin.P_05, frequency=frequency) + print(f"{frequency} ok") + except ValueError: + print(f"{frequency} ValueError") +fastest = pwmio.PWMOut(microcontroller.pin.P_05, frequency=8_000_000) +print(f"fastest {fastest.frequency}") +print("done") +""" + + +@pytest.mark.duration(8.0) +@pytest.mark.circuitpy_drive({"code.py": PWMIO_INVALID_FREQUENCY_CODE}) +def test_pwmio_invalid_frequency(circuitpython): + """Frequencies the clock can't produce raise ValueError; the fastest one works.""" + circuitpython.wait_until_done() + output = circuitpython.serial.all_output + assert "0 ValueError" in output + assert "20000000 ValueError" in output + # 16 MHz / 8 MHz = a 2-cycle period, the shortest allowed. + assert "fastest 8000000" in output + assert "done" in output + + +PWMIO_PIN_CLAIM_CODE = """\ +import digitalio +import microcontroller +import pwmio + +p = microcontroller.pin +dio = digitalio.DigitalInOut(p.P_05) +try: + pwmio.PWMOut(p.P_05) + print("over DigitalInOut ok") +except ValueError: + print("over DigitalInOut ValueError") +dio.deinit() + +pwm = pwmio.PWMOut(p.P_05, frequency=1000) +try: + pwmio.PWMOut(p.P_05, frequency=1000) + print("second PWMOut ok") +except ValueError: + print("second PWMOut ValueError") +pwm.deinit() + +dio = digitalio.DigitalInOut(p.P_05) +print("DigitalInOut after deinit ok") +dio.deinit() +again = pwmio.PWMOut(p.P_05, frequency=2000) +print(f"PWMOut after deinit {again.frequency}") +print("done") +""" + + +@pytest.mark.duration(8.0) +@pytest.mark.circuitpy_drive({"code.py": PWMIO_PIN_CLAIM_CODE}) +def test_pwmio_pin_claims(circuitpython): + """A PWMOut claims its pin while it lives and releases it on deinit.""" + circuitpython.wait_until_done() + output = circuitpython.serial.all_output + assert "over DigitalInOut ValueError" in output + assert "second PWMOut ValueError" in output + assert "DigitalInOut after deinit ok" in output + assert "PWMOut after deinit 2000" in output + assert "done" in output + + +PWMIO_GROUPS_CODE = """\ +import microcontroller +import pwmio + +p = microcontroller.pin +shared = [pwmio.PWMOut(pin, frequency=1000) for pin in (p.P_05, p.P_06, p.P_07)] +other = pwmio.PWMOut(p.P_08, frequency=2000) +print("two groups ok") +try: + pwmio.PWMOut(p.P_09, frequency=3000) + print("third frequency ok") +except RuntimeError: + print("third frequency RuntimeError") +for pwm in shared: + pwm.deinit() +third = pwmio.PWMOut(p.P_09, frequency=3000) +print("third frequency after deinit ok") +print("done") +""" + + +@pytest.mark.duration(8.0) +@pytest.mark.circuitpy_drive({"code.py": PWMIO_GROUPS_CODE}) +def test_pwmio_same_frequency_shares_a_group(circuitpython): + """Equal frequencies share a group; with every group taken a new frequency fails + until a group is freed.""" + circuitpython.wait_until_done() + output = circuitpython.serial.all_output + assert "two groups ok" in output + assert "third frequency RuntimeError" in output + assert "third frequency after deinit ok" in output + assert "done" in output + + trace = circuitpython.trace_file + for pin in (5, 6, 7): + assert _track_values(trace, f"pwm.{pin:02}.period")[0] == CLOCK // 1000 + assert _track_values(trace, "pwm.08.period")[0] == CLOCK // 2000 + assert _track_values(trace, "pwm.09.period") == [CLOCK // 3000] + + +PWMIO_VARIABLE_EXCLUSIVE_CODE = """\ +import microcontroller +import pwmio + +p = microcontroller.pin +var = pwmio.PWMOut(p.P_05, frequency=440, variable_frequency=True) +other = pwmio.PWMOut(p.P_06, frequency=440) +print("other ok") +try: + pwmio.PWMOut(p.P_07, frequency=300) + print("300 ok") +except RuntimeError: + print("300 RuntimeError") +joins = pwmio.PWMOut(p.P_08, frequency=440) +print("joins ok") +var.deinit() +after = pwmio.PWMOut(p.P_09, frequency=300) +print("300 after deinit ok") +print("done") +""" + + +@pytest.mark.duration(8.0) +@pytest.mark.circuitpy_drive({"code.py": PWMIO_VARIABLE_EXCLUSIVE_CODE}) +def test_pwmio_variable_frequency_has_own_group(circuitpython): + """A variable-frequency PWMOut shares its group with nobody.""" + circuitpython.wait_until_done() + output = circuitpython.serial.all_output + # The 440 Hz fixed-frequency PWMOut can't join the variable one's group. + assert "other ok" in output + # Both groups are taken. + assert "300 RuntimeError" in output + # Another fixed 440 Hz PWMOut joins the fixed 440 Hz group. + assert "joins ok" in output + assert "300 after deinit ok" in output + assert "done" in output From ca7429bde219529363a66ff7ff5b82f792f0a393 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Thu, 8 Oct 2026 13:49:58 -0400 Subject: [PATCH 4/8] Remove unused common_hal_pwmio_pwmout_reset_ok() 1acf65ee22 removed its only call and every port's definition but left the declaration in shared-bindings; drop it and the zephyr-cp stub. Co-Authored-By: Claude Opus 5.5 --- ports/zephyr-cp/common-hal/pwmio/PWMOut.c | 6 ------ shared-bindings/pwmio/PWMOut.h | 1 - 2 files changed, 7 deletions(-) diff --git a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c index 5c856c31570..80206c1fc0b 100644 --- a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c +++ b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c @@ -127,9 +127,3 @@ bool common_hal_pwmio_pwmout_get_variable_frequency(pwmio_pwmout_obj_t *self) { const mcu_pin_obj_t *common_hal_pwmio_pwmout_get_pin(pwmio_pwmout_obj_t *self) { return self->pin; } - -void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { - // CIRCUITPY_BULK_RESET is off on this port: nothing is kept across - // resets, so there is nothing to undo. - (void)self; -} diff --git a/shared-bindings/pwmio/PWMOut.h b/shared-bindings/pwmio/PWMOut.h index 9524408b7c5..868a3d31d03 100644 --- a/shared-bindings/pwmio/PWMOut.h +++ b/shared-bindings/pwmio/PWMOut.h @@ -37,6 +37,5 @@ extern const mcu_pin_obj_t *common_hal_pwmio_pwmout_get_pin(pwmio_pwmout_obj_t * #if CIRCUITPY_BULK_RESET extern void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self); #endif -extern void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self); extern void common_hal_pwmio_pwmout_raise_error(pwmout_result_t result); From 87229e59fd40276cda6a74cc0a3fc1e2e828ce73 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 9 Oct 2026 15:36:50 -0400 Subject: [PATCH 5/8] zephyr-cp: compute the PWM period in pwmio, not iobroker iobroker now uses the requested frequency only to decide which channels share a group, and returns just the device and channel. common-hal/pwmio turns the frequency into a period from `pwm_get_cycles_per_sec()` (truncating, as before) and passes it to `pwm_set_cycles()`. `iobroker_pwm_period_cycles()` and the `period_cycles_out` parameter are gone. Co-Authored-By: Claude Opus 5.5 --- ports/zephyr-cp/common-hal/pwmio/PWMOut.c | 64 +++++++++++--- ports/zephyr-cp/common-hal/pwmio/PWMOut.h | 2 +- .../internal-modules/iobroker/README.md | 19 ++-- .../iobroker/include/iobroker/iobroker.h | 47 ++++------ .../iobroker/src/emul/iobroker_pwm.c | 59 +++---------- .../internal-modules/iobroker/src/iobroker.c | 12 +-- .../iobroker/src/nordic/nrf/iobroker_route.c | 87 ++----------------- 7 files changed, 100 insertions(+), 190 deletions(-) diff --git a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c index 80206c1fc0b..de3d95f3d85 100644 --- a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c +++ b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c @@ -5,9 +5,10 @@ // SPDX-License-Identifier: MIT // pwmio on Zephyr's PWM API. The iobroker module picks the PWM device and -// channel for the pin and adjusts the frequency to a period the hardware -// can produce; channels that share a period (an nRF instance's outputs) are -// grouped there, so nothing here is SoC-specific. +// channel for the pin; channels that share a period (an nRF instance's +// outputs) are grouped there, so nothing here is SoC-specific. The period is +// computed here from the device's clock, and the driver rejects a period it +// can't produce. #include @@ -19,6 +20,28 @@ #include "shared-bindings/pwmio/PWMOut.h" +// Shortest period accepted, in clock cycles: a period needs a high and a low +// part. +#define MIN_PERIOD_CYCLES 2U + +// Truncate a frequency to a whole number of clock cycles, so the actual +// frequency is the same or slightly higher. Returns 0, or a negative errno +// when the period would be out of range or the clock is unknown. +static int frequency_to_period(const struct device *dev, uint32_t channel, + uint32_t frequency, uint32_t *period_cycles_out) { + uint64_t cycles_per_sec; + int ret = pwm_get_cycles_per_sec(dev, channel, &cycles_per_sec); + if (ret < 0) { + return ret; + } + uint64_t period = cycles_per_sec / frequency; + if (period < MIN_PERIOD_CYCLES || period > UINT32_MAX) { + return -EINVAL; + } + *period_cycles_out = (uint32_t)period; + return 0; +} + static uint32_t pulse_cycles(uint32_t period_cycles, uint16_t duty) { return (uint32_t)(((uint64_t)period_cycles * duty) / 0xffff); } @@ -34,16 +57,18 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, bool variable_frequency) { // Deinited until construction succeeds. self->dev = NULL; + if (frequency == 0) { + return PWMOUT_INVALID_FREQUENCY; + } const struct device *dev; uint32_t channel; - uint32_t period_cycles; + // A variable frequency needs a group of its own, since changing the + // period would change it for every channel in the group. int ret = iobroker_pwm_channel_allocate(pin->package_pin, frequency, - variable_frequency, &dev, &channel, &period_cycles); + variable_frequency, &dev, &channel); switch (ret) { case 0: break; - case -ERANGE: - return PWMOUT_INVALID_FREQUENCY; case -ENODEV: return PWMOUT_INTERNAL_RESOURCES_IN_USE; case -EINVAL: @@ -61,6 +86,11 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, (void)iobroker_pwm_channel_release(dev, channel); return PWMOUT_INITIALIZATION_ERROR; } + uint32_t period_cycles; + if (frequency_to_period(dev, channel, frequency, &period_cycles) < 0) { + (void)iobroker_pwm_channel_release(dev, channel); + return PWMOUT_INVALID_FREQUENCY; + } self->pin = pin; self->dev = dev; @@ -68,10 +98,11 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, self->period_cycles = period_cycles; self->duty_cycle = duty; self->variable_frequency = variable_frequency; + // The driver rejects a period its hardware can't produce. if (set_period_and_duty(self) < 0) { (void)iobroker_pwm_channel_release(dev, channel); self->dev = NULL; - return PWMOUT_INITIALIZATION_ERROR; + return PWMOUT_INVALID_FREQUENCY; } return PWMOUT_OK; } @@ -104,15 +135,26 @@ void common_hal_pwmio_pwmout_set_frequency(pwmio_pwmout_obj_t *self, uint32_t fr // Only used when variable_frequency=True, so the // channel's group is exclusive and no other channel shares the period. uint32_t period_cycles; - if (iobroker_pwm_period_cycles(self->dev, frequency, &period_cycles) < 0) { + if (frequency == 0 || + frequency_to_period(self->dev, self->channel, frequency, &period_cycles) < 0) { common_hal_pwmio_pwmout_raise_error(PWMOUT_INVALID_FREQUENCY); } + uint32_t old_period_cycles = self->period_cycles; self->period_cycles = period_cycles; - (void)set_period_and_duty(self); + if (set_period_and_duty(self) < 0) { + // The driver rejected the period: keep the old one. + self->period_cycles = old_period_cycles; + (void)set_period_and_duty(self); + common_hal_pwmio_pwmout_raise_error(PWMOUT_INVALID_FREQUENCY); + } } uint32_t common_hal_pwmio_pwmout_get_frequency(pwmio_pwmout_obj_t *self) { - // The frequency the hardware actually produces. + // The frequency the stored period produces. The driver may truncate the + // period further to fit its registers (on nRF, to a multiple of the + // prescaler), which Zephyr's PWM API doesn't report, so at low + // frequencies this can be very slightly below the actual output (under + // 0.01 % on nRF). uint64_t cycles_per_sec; if (pwm_get_cycles_per_sec(self->dev, self->channel, &cycles_per_sec) < 0) { return 0; diff --git a/ports/zephyr-cp/common-hal/pwmio/PWMOut.h b/ports/zephyr-cp/common-hal/pwmio/PWMOut.h index fd5e1705183..15e44f70669 100644 --- a/ports/zephyr-cp/common-hal/pwmio/PWMOut.h +++ b/ports/zephyr-cp/common-hal/pwmio/PWMOut.h @@ -19,7 +19,7 @@ typedef struct { // NULL when deinited. const struct device *dev; uint32_t channel; - // Period in the device's PWM clock cycles, as iobroker computed it. Every + // Period in the device's PWM clock cycles, from the frequency. Every // pwm_set_cycles() call passes exactly this value. uint32_t period_cycles; uint16_t duty_cycle; diff --git a/ports/zephyr-cp/internal-modules/iobroker/README.md b/ports/zephyr-cp/internal-modules/iobroker/README.md index 7c822bbccb7..24bb849f9d1 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/README.md +++ b/ports/zephyr-cp/internal-modules/iobroker/README.md @@ -167,17 +167,16 @@ Individual PWM channels are allocated with `iobroker_pwm_channel_allocate()`, which shares an instance between pins. Channels that share a period form a group (for instance, on nRF, the four outputs of an instance; on other SoCs this might be a slice or a timer). The -call takes a frequency, adjusts it to a period the hardware can produce -(truncating on nRF), and joins a running group with the same period and a -free channel, or takes a free instance. It returns the device, the channel -number and the group's period, which the caller passes unchanged to -`pwm_set_cycles()`. Joining or leaving a running group connects or -disconnects only that one output, so the group's other outputs are not -disturbed. An exclusive request (variable frequency) gets a group of its -own, and `iobroker_pwm_period_cycles()` adjusts a new frequency for it. +call takes the requested frequency only as the sharing key: it joins a +running group requested at exactly the same frequency and with a free +channel, or takes a free instance. It returns the device and the channel +number; the caller turns the frequency into a period itself and passes it to +`pwm_set_cycles()`, and the driver rejects a period its hardware cannot +produce. Joining or leaving a running group connects or disconnects only +that one output, so the group's other outputs are not disturbed. An +exclusive request (variable frequency) gets a group of its own. `iobroker_pwm_channel_release()` returns a channel; the group's last channel -releases the instance. A frequency the hardware cannot produce at all fails -with `-ERANGE`. +releases the instance. Not every instance can reach every pad. On nRF52 and nRF53 the routing is a full crossbar, but on nRF54L peripherals and GPIO controllers are grouped in diff --git a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h index 3094501369d..097c49c0a3b 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h +++ b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h @@ -179,10 +179,11 @@ typedef struct { // The fields below are used only for a PWM instance shared by channels // (iobroker_pwm_channel_allocate()). Buses and whole-instance PWM // allocations (iobroker_pwm_allocate()) leave them 0. - // Period, in PWM clock cycles, of every channel on the instance. - uint32_t period_cycles; - // True when the instance's one channel may change the period (variable - // frequency), so no other channel may join it. + // Frequency in Hz that the instance's channels were requested at; a + // request joins only at exactly this frequency. + uint32_t frequency; + // True when one caller holds the instance alone, so no other channel + // may join it. bool exclusive; // Bit n is set while output n is allocated to a channel. uint8_t channels_used; @@ -246,23 +247,20 @@ int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out); // Allocate one PWM output channel for a pin. Channels that share a period // (all outputs of an nRF instance; on other SoCs a slice or a timer) form a -// group. frequency is in Hz and is adjusted to a period the hardware can -// produce (on nRF by truncation, so the actual frequency is the same or -// slightly higher); a request joins a running group whose period comes out -// the same, unless the group or the request is exclusive -// (variable-frequency use, where the holder changes the period). Otherwise a -// free group is taken. +// group. frequency (Hz) only decides sharing: a request joins a running group +// requested at exactly the same frequency, unless the group or the request +// is exclusive (one caller holds the whole group, e.g. to change the period). +// Otherwise a free group is taken. The caller turns the frequency into a +// period itself (pwm_get_cycles_per_sec()); requests at the same frequency +// thus pass identical periods to pwm_set_cycles(), as drivers require of +// channels in one group. // -// On success returns 0 and sets *dev_out, *channel_out (the channel number -// pwm_set_cycles() takes) and *period_cycles_out (the group's period in the -// device's PWM clock cycles). The caller must pass exactly that period to -// pwm_set_cycles(): drivers refuse a different period while other channels -// of the group run. The caller then calls device_init(), which returns -// -EALREADY if the group was already running. Joining a running group -// only connects the new pin; the group's other outputs are not disturbed. +// On success returns 0 and sets *dev_out and *channel_out (the channel number +// pwm_set_cycles() takes). The caller then calls device_init(), which returns +// -EALREADY if the group was already running. Joining a running group only +// connects the new pin; the group's other outputs are not disturbed. // // Returns a negative errno on failure, with the out parameters untouched: -// -ERANGE: the hardware cannot produce the frequency (including 0) // -EINVAL: the pin is disconnected, not in the package map, has no GPIO, // or no PWM group can reach it // -EBUSY: the pin is already claimed @@ -270,8 +268,7 @@ int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out); // -ENOSYS: PWM channel allocation is unsupported on this SoC // Every allocation must be paired with iobroker_pwm_channel_release(). int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, - bool exclusive, const struct device **dev_out, uint32_t *channel_out, - uint32_t *period_cycles_out); + bool exclusive, const struct device **dev_out, uint32_t *channel_out); // Release a channel allocated with iobroker_pwm_channel_allocate(): its pin // is disconnected from the group and left quiescent. Releasing the group's @@ -280,16 +277,6 @@ int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, // first. Returns true when the channel was allocated. bool iobroker_pwm_channel_release(const struct device *dev, uint32_t channel); -// Adjust frequency (Hz) to a period of an allocated PWM device, the same way -// iobroker_pwm_channel_allocate() does, for a caller holding an exclusive -// group that changes its frequency. Returns 0 and sets *period_cycles_out, -// or a negative errno with *period_cycles_out untouched: -// -ERANGE: the hardware cannot produce the frequency (including 0) -// -EINVAL: dev is not an iobroker PWM device -// -ENOSYS: PWM channel allocation is unsupported on this SoC -int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, - uint32_t *period_cycles_out); - // Allocate a package pin for the ADC. Analog inputs have no runtime routing: // the pad's analog input is fixed by the SoC, so the call resolves and // returns it in *input_out (the input selector the device's channel diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/emul/iobroker_pwm.c b/ports/zephyr-cp/internal-modules/iobroker/src/emul/iobroker_pwm.c index 75c145465ab..8e683e64025 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/src/emul/iobroker_pwm.c +++ b/ports/zephyr-cp/internal-modules/iobroker/src/emul/iobroker_pwm.c @@ -12,9 +12,9 @@ // period form a group; the controller has a pool of them (the groups // property), and any channel can join any group, without a channel limit. // So this models only the pwmio rules every SoC shares, not any SoC's -// layout: a fixed-frequency request joins a group running at the same -// period, a variable-frequency request takes a group of its own, and with -// no usable group left the request fails. The pad is claimed as a GPIO, +// layout: a shared request joins a group requested at the same frequency, +// an exclusive request takes a group of its own, and with no usable group +// left the request fails. The pad is claimed as a GPIO, // which makes it busy for every other allocation. #include @@ -24,7 +24,6 @@ #include #include -#include #include #include @@ -36,14 +35,11 @@ LOG_MODULE_DECLARE(iobroker, CONFIG_LOG_DEFAULT_LEVEL); #define EMUL_PWM_CHANNELS DT_PROP(EMUL_PWM_NODE, channels) #define EMUL_PWM_GROUPS DT_PROP(EMUL_PWM_NODE, groups) -// Shortest period accepted, in clock cycles: a period needs a high and a low -// part. -#define EMUL_PWM_MIN_PERIOD 2U - // Period groups. A group is free while it has no users. typedef struct { - uint32_t period_cycles; - // One variable-frequency channel holds the group; nobody may join. + // Frequency in Hz the group's channels were requested at. + uint32_t frequency; + // One caller holds the group alone; nobody may join. bool exclusive; uint16_t users; } emul_pwm_group_t; @@ -57,14 +53,14 @@ BUILD_ASSERT(EMUL_PWM_GROUPS <= UINT8_MAX, "group index must fit channel_group[] // Channels currently allocated, so release only drops claims it made. static uint32_t allocated[DIV_ROUND_UP(EMUL_PWM_CHANNELS, 32)]; -// Pick the group for a request: a running group at the same period that +// Pick the group for a request: a running group at the same frequency that // nobody holds exclusively, unless the request is exclusive itself, or else // a free group. Returns the group index, or -1 when none is usable. -static int emul_pwm_group_pick(uint32_t period_cycles, bool exclusive) { +static int emul_pwm_group_pick(uint32_t frequency, bool exclusive) { if (!exclusive) { for (int i = 0; i < EMUL_PWM_GROUPS; i++) { if (groups[i].users > 0 && !groups[i].exclusive && - groups[i].period_cycles == period_cycles) { + groups[i].frequency == frequency) { return i; } } @@ -81,38 +77,8 @@ static const struct device *emul_pwm_device(void) { return DEVICE_DT_GET(EMUL_PWM_NODE); } -// Adjust a frequency to a period of the clock by truncation, so the actual -// frequency is the same or slightly higher. -static int emul_pwm_frequency_period(uint32_t frequency, uint32_t *period_cycles_out) { - uint64_t cycles_per_sec; - if (frequency == 0 || pwm_get_cycles_per_sec(emul_pwm_device(), 0, &cycles_per_sec) < 0) { - return -ERANGE; - } - uint64_t period = cycles_per_sec / frequency; - if (period < EMUL_PWM_MIN_PERIOD || period > UINT32_MAX) { - return -ERANGE; - } - *period_cycles_out = (uint32_t)period; - return 0; -} - -int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, - uint32_t *period_cycles_out) { - if (dev != emul_pwm_device()) { - return -EINVAL; - } - return emul_pwm_frequency_period(frequency, period_cycles_out); -} - int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, - bool exclusive, const struct device **dev_out, uint32_t *channel_out, - uint32_t *period_cycles_out) { - // An out-of-range frequency is reported first, ahead of pin and busy - // errors. - uint32_t period_cycles; - if (emul_pwm_frequency_period(frequency, &period_cycles) < 0) { - return -ERANGE; - } + bool exclusive, const struct device **dev_out, uint32_t *channel_out) { uint16_t soc_pad; uint16_t gpio_pad; if (pin == IOBROKER_NO_PIN || iobroker_package_pin_soc_pad(pin, &soc_pad) < 0 || @@ -121,7 +87,7 @@ int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, (unsigned)pin); return -EINVAL; } - int group = emul_pwm_group_pick(period_cycles, exclusive); + int group = emul_pwm_group_pick(frequency, exclusive); if (group < 0) { LOG_WRN("pwm channel allocate: no usable period group for package pin %u", (unsigned)pin); @@ -134,7 +100,7 @@ int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, return ret; } if (groups[group].users == 0) { - groups[group].period_cycles = period_cycles; + groups[group].frequency = frequency; groups[group].exclusive = exclusive; } groups[group].users++; @@ -142,7 +108,6 @@ int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, allocated[gpio_pad / 32] |= BIT(gpio_pad % 32); *dev_out = emul_pwm_device(); *channel_out = gpio_pad; - *period_cycles_out = period_cycles; LOG_INF("pwm channel allocate: package pin %u -> %s channel %u, group %d", (unsigned)pin, (*dev_out)->name, (unsigned)gpio_pad, group); return 0; diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c b/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c index 89ed1703fe0..d395b208203 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c +++ b/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c @@ -151,14 +151,12 @@ bool iobroker_release(const struct device *dev) { // can call it, but every allocation reports -ENOSYS. The implementations // live in src/// (nRF) and in src/emul/ (the emulated PWM). int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, - bool exclusive, const struct device **dev_out, uint32_t *channel_out, - uint32_t *period_cycles_out) { + bool exclusive, const struct device **dev_out, uint32_t *channel_out) { (void)pin; (void)frequency; (void)exclusive; (void)dev_out; (void)channel_out; - (void)period_cycles_out; return -ENOSYS; } @@ -168,14 +166,6 @@ bool iobroker_pwm_channel_release(const struct device *dev, uint32_t channel) { return false; } -int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, - uint32_t *period_cycles_out) { - (void)dev; - (void)frequency; - (void)period_cycles_out; - return -ENOSYS; -} - #endif // !IOBROKER_PWM_CHANNELS #if !IOBROKER_ADC diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c b/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c index 986d90b71b4..12245b482c3 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c +++ b/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c @@ -23,12 +23,9 @@ #include #include -#include #include #include -#include - #include "iobroker_internal.h" LOG_MODULE_DECLARE(iobroker, CONFIG_LOG_DEFAULT_LEVEL); @@ -541,40 +538,6 @@ BUILD_ASSERT(NRF_FUN_PWM_OUT1 == NRF_FUN_PWM_OUT0 + 1 && NRF_FUN_PWM_OUT2 == NRF_FUN_PWM_OUT0 + 2 && NRF_FUN_PWM_OUT3 == NRF_FUN_PWM_OUT0 + 3, "PWM output function codes must be consecutive"); -// Smallest COUNTERTOP the hardware accepts (the nRF52 MDK has no _Min). -#define NRF_PWM_COUNTERTOP_MIN 3U - -// Adjust a frequency to a period of dev's PWM clock that pwm_nrfx programs -// exactly: COUNTERTOP << PRESCALER, with the smallest prescaler whose -// COUNTERTOP fits, as pwm_nrfx's pwm_period_check_and_set() picks it. A -// period of that form maps back to the same prescaler in the driver, so -// every channel of a group passes the identical period_cycles it requires. -// Both divisions truncate, as in ports/nordic, so the period is never -// longer than requested and the actual frequency is the same or slightly -// higher. Assumes the instance counts up only (pwm_nrfx's default, no -// center-aligned mode). -static int nrf_pwm_frequency_period(const struct device *dev, uint32_t frequency, - uint32_t *period_cycles_out) { - uint64_t cycles_per_sec; - if (frequency == 0 || pwm_get_cycles_per_sec(dev, 0, &cycles_per_sec) < 0) { - return -ERANGE; - } - // Period in undivided clock cycles. - uint64_t raw = cycles_per_sec / frequency; - for (uint32_t prescaler = 0; prescaler <= PWM_PRESCALER_PRESCALER_Msk; prescaler++) { - uint64_t countertop = raw >> prescaler; - if (countertop > PWM_COUNTERTOP_COUNTERTOP_Msk) { - continue; - } - if (countertop < NRF_PWM_COUNTERTOP_MIN) { - return -ERANGE; - } - *period_cycles_out = (uint32_t)(countertop << prescaler); - return 0; - } - return -ERANGE; -} - static iobroker_state_t *nrf_pwm_state_find(const struct device *dev) { for (size_t i = 0; i < iobroker_pwm_bus_count; i++) { if (iobroker_pwm_buses[i].dev == dev) { @@ -584,14 +547,6 @@ static iobroker_state_t *nrf_pwm_state_find(const struct device *dev) { return NULL; } -int iobroker_pwm_period_cycles(const struct device *dev, uint32_t frequency, - uint32_t *period_cycles_out) { - if (nrf_pwm_state_find(dev) == NULL) { - return -EINVAL; - } - return nrf_pwm_frequency_period(dev, frequency, period_cycles_out); -} - // Connect output `out` of a running instance to a pad, or disconnect it when // pad is IOBROKER_NO_PIN. Only that output's PSEL (and its pad, when // connecting) is configured, so the instance's other outputs keep running @@ -612,20 +567,9 @@ static int nrf_pwm_connect_output(const iobroker_instance_t *inst, } int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, - bool exclusive, const struct device **dev_out, uint32_t *channel_out, - uint32_t *period_cycles_out) { + bool exclusive, const struct device **dev_out, uint32_t *channel_out) { LOG_INF("pwm channel allocate: pin=%u frequency=%u exclusive=%u", (unsigned)pin, (unsigned)frequency, (unsigned)exclusive); - // An out-of-range frequency is reported first, ahead of - // pin and busy errors. - bool in_range = false; - for (size_t i = 0; i < iobroker_pwm_bus_count && !in_range; i++) { - uint32_t unused; - in_range = nrf_pwm_frequency_period(iobroker_pwm_buses[i].dev, frequency, &unused) == 0; - } - if (!in_range) { - return -ERANGE; - } const package_pin_t requested[] = { pin }; int ret = iobroker_check_request("pwm", requested, 1); if (ret < 0) { @@ -641,16 +585,13 @@ int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, return -EINVAL; } - // Join a running group with the same period and a free output. + // Join a running group requested at the same frequency, with a free + // output. for (size_t i = 0; i < iobroker_pwm_bus_count && !exclusive; i++) { const iobroker_instance_t *inst = &iobroker_pwm_buses[i]; iobroker_state_t *state = &iobroker_pwm_bus_states[i]; - uint32_t period_cycles; - if (nrf_pwm_frequency_period(inst->dev, frequency, &period_cycles) < 0) { - continue; - } if (!state->in_use || state->channels_used == 0 || state->exclusive || - state->period_cycles != period_cycles || + state->frequency != frequency || !nrf_instance_reaches_pad(inst->reg_addr, pad)) { continue; } @@ -667,37 +608,23 @@ int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, (unsigned)out); *dev_out = inst->dev; *channel_out = out; - *period_cycles_out = period_cycles; return 0; } } - // Otherwise take a free instance, with the pin on OUT0. The instance's - // clock decides the period, so check the range on the one allocated. + // Otherwise take a free instance, with the pin on OUT0. const struct device *dev; ret = iobroker_pwm_allocate(pin, &dev); if (ret < 0) { // No new PWM available. return ret; } - uint32_t period_cycles; - if (nrf_pwm_frequency_period(dev, frequency, &period_cycles) < 0) { - // The range check at the top only showed that some instance can - // produce the frequency, not necessarily this one: the period - // depends on the instance's PWM clock. This can only fail on a SoC - // whose instances run from different clocks; on the nRF52, nRF53 - // and nRF54L every instance runs at 16 MHz. - (void)iobroker_release(dev); - iobroker_gpio_pad_quiesce(soc_pad); - return -ERANGE; - } iobroker_state_t *state = nrf_pwm_state_find(dev); - state->period_cycles = period_cycles; + state->frequency = frequency; state->exclusive = exclusive; state->channels_used = BIT(0); *dev_out = dev; *channel_out = 0; - *period_cycles_out = period_cycles; return 0; } @@ -718,7 +645,7 @@ bool iobroker_pwm_channel_release(const struct device *dev, uint32_t channel) { if (state->channels_used == 0) { // Last channel: release the whole group. De-init applies the sleep // state to the instance's pads. - state->period_cycles = 0; + state->frequency = 0; state->exclusive = false; (void)iobroker_release(dev); } else { From ad2e50d940400aa056465c6ca8f75e6129bd56b2 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 9 Oct 2026 15:40:31 -0400 Subject: [PATCH 6/8] zephyr-cp: allocate PWM only by channel Remove the public whole-instance `iobroker_pwm_allocate()`. A PWM allocation is always a channel, identified by its device and channel number, like the GPIO, ADC and DAC allocations. A caller that needs a group to itself asks for an exclusive channel: pwmio for a variable frequency, and neopixel_write, which programs a whole nRF instance. Co-Authored-By: Claude Opus 5.5 --- .../internal-modules/iobroker/README.md | 31 +++++++++---------- .../iobroker/include/iobroker/iobroker.h | 20 ++++-------- .../internal-modules/iobroker/src/iobroker.c | 6 ---- .../iobroker/src/nordic/nrf/iobroker_route.c | 7 +++-- .../neopixel/src/nordic/nrf/neopixel_nrf.c | 10 ++++-- 5 files changed, 33 insertions(+), 41 deletions(-) diff --git a/ports/zephyr-cp/internal-modules/iobroker/README.md b/ports/zephyr-cp/internal-modules/iobroker/README.md index 24bb849f9d1..6c4568924f1 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/README.md +++ b/ports/zephyr-cp/internal-modules/iobroker/README.md @@ -160,23 +160,20 @@ quiescent state (disconnected) on release, and GPIO claims conflict with bus allocations the same way bus allocations conflict with each other. `iobroker_gpio_allocate()` resolves the package pin through the map and returns both the GPIO controller device and the pin number within it. -PWM instances are allocated whole with `iobroker_pwm_allocate()`, for one -package pin routed to the instance's first output; the caller initializes the -device and returns it with `iobroker_release()`, as with the buses. -Individual PWM channels are allocated with -`iobroker_pwm_channel_allocate()`, which shares an instance between pins. -Channels that share a period form a group (for instance, on nRF, the four -outputs of an instance; on other SoCs this might be a slice or a timer). The -call takes the requested frequency only as the sharing key: it joins a -running group requested at exactly the same frequency and with a free -channel, or takes a free instance. It returns the device and the channel -number; the caller turns the frequency into a period itself and passes it to -`pwm_set_cycles()`, and the driver rejects a period its hardware cannot -produce. Joining or leaving a running group connects or disconnects only -that one output, so the group's other outputs are not disturbed. An -exclusive request (variable frequency) gets a group of its own. -`iobroker_pwm_channel_release()` returns a channel; the group's last channel -releases the instance. +PWM channels are allocated with `iobroker_pwm_channel_allocate()`, which +shares an instance between pins. Channels that share a period form a group +(for instance, on nRF, the four outputs of an instance; on other SoCs this +might be a slice or a timer). The call takes the requested frequency only as +the sharing key: it joins a running group requested at exactly the same +frequency and with a free channel, or takes a free instance. It returns the +device and the channel number; the caller turns the frequency into a period +itself and passes it to `pwm_set_cycles()`, and the driver rejects a period +its hardware cannot produce. Joining or leaving a running group connects or +disconnects only that one output, so the group's other outputs are not +disturbed. An exclusive request gets a group of its own that nobody joins: +pwmio uses it for a variable frequency, and neopixel_write to program a +whole nRF instance itself. `iobroker_pwm_channel_release()` returns a +channel; the group's last channel releases the instance. Not every instance can reach every pad. On nRF52 and nRF53 the routing is a full crossbar, but on nRF54L peripherals and GPIO controllers are grouped in diff --git a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h index 097c49c0a3b..41a40badc01 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h +++ b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h @@ -176,9 +176,8 @@ typedef struct { // Used by iobroker_pin_in_use(); empty while the instance is free. package_pin_t pins[IOBROKER_MAX_PINS]; uint8_t pin_count; - // The fields below are used only for a PWM instance shared by channels - // (iobroker_pwm_channel_allocate()). Buses and whole-instance PWM - // allocations (iobroker_pwm_allocate()) leave them 0. + // The fields below are used only for PWM instances allocated by channel + // (iobroker_pwm_channel_allocate()); buses leave them 0. // Frequency in Hz that the instance's channels were requested at; a // request joins only at exactly this frequency. uint32_t frequency; @@ -236,21 +235,14 @@ int iobroker_spi_allocate(package_pin_t clock, package_pin_t mosi, package_pin_t miso, const struct device **dev_out); int iobroker_uart_allocate(package_pin_t tx, package_pin_t rx, package_pin_t rts, package_pin_t cts, const struct device **dev_out); -// Allocate a whole PWM instance for one output pin, routed to the -// instance's first output (OUT0 on nRF); its other outputs stay -// disconnected. The instance is not shared, so the caller may program all -// of it (neopixel_write plays its own sequence). Like the bus allocate -// functions, the caller initializes the device (device_init()) and returns -// it with iobroker_release(). To share an instance between pins, use -// iobroker_pwm_channel_allocate() instead. -int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out); - // Allocate one PWM output channel for a pin. Channels that share a period // (all outputs of an nRF instance; on other SoCs a slice or a timer) form a // group. frequency (Hz) only decides sharing: a request joins a running group // requested at exactly the same frequency, unless the group or the request -// is exclusive (one caller holds the whole group, e.g. to change the period). -// Otherwise a free group is taken. The caller turns the frequency into a +// is exclusive. Otherwise a free group is taken. An exclusive request gets a +// group of its own that nobody joins, for a caller that changes the period +// or programs the group's hardware itself. What a group is depends on the +// SoC: on nRF it is a whole PWM instance. The caller turns the frequency into a // period itself (pwm_get_cycles_per_sec()); requests at the same frequency // thus pass identical periods to pwm_set_cycles(), as drivers require of // channels in one group. diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c b/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c index d395b208203..3aa1fa14f93 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c +++ b/ports/zephyr-cp/internal-modules/iobroker/src/iobroker.c @@ -131,12 +131,6 @@ int iobroker_uart_allocate(package_pin_t tx, package_pin_t rx, return -ENOSYS; } -int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out) { - (void)pin; - (void)dev_out; - return -ENOSYS; -} - bool iobroker_release(const struct device *dev) { (void)dev; LOG_DBG("release: no routing support on this SoC, nothing to release"); diff --git a/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c b/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c index 12245b482c3..1044409cf6c 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c +++ b/ports/zephyr-cp/internal-modules/iobroker/src/nordic/nrf/iobroker_route.c @@ -501,7 +501,10 @@ int iobroker_uart_allocate(package_pin_t tx, package_pin_t rx, iobroker_uart_bus_states, requested, pins, 4, dev_out); } -int iobroker_pwm_allocate(package_pin_t pin, const struct device **dev_out) { +// Allocate a whole PWM instance for one output pin, routed to its first +// output (OUT0); the other outputs stay disconnected. Used for a new +// channel group. +static int nrf_pwm_instance_allocate(package_pin_t pin, const struct device **dev_out) { LOG_INF("pwm allocate: pin=%u", (unsigned)pin); // The pin goes to OUT0; OUT1..OUT3 stay disconnected, and are recorded as // disconnected so the instance's pin list is complete. @@ -614,7 +617,7 @@ int iobroker_pwm_channel_allocate(package_pin_t pin, uint32_t frequency, // Otherwise take a free instance, with the pin on OUT0. const struct device *dev; - ret = iobroker_pwm_allocate(pin, &dev); + ret = nrf_pwm_instance_allocate(pin, &dev); if (ret < 0) { // No new PWM available. return ret; diff --git a/ports/zephyr-cp/internal-modules/neopixel/src/nordic/nrf/neopixel_nrf.c b/ports/zephyr-cp/internal-modules/neopixel/src/nordic/nrf/neopixel_nrf.c index efdbc607e14..9610ac0c050 100644 --- a/ports/zephyr-cp/internal-modules/neopixel/src/nordic/nrf/neopixel_nrf.c +++ b/ports/zephyr-cp/internal-modules/neopixel/src/nordic/nrf/neopixel_nrf.c @@ -247,11 +247,17 @@ int neopixel_send(package_pin_t pin, const uint8_t *pixels, size_t num_bytes, return -EINVAL; } + // An exclusive channel gets a group of its own, which on nRF is a whole + // PWM instance: the sequence drives all of the instance's outputs + // (NRF_PWM_LOAD_COMMON), and only this pin's output is connected. The + // frequency, the 800 kHz bit rate, only matters for sharing, which an + // exclusive channel never does. const struct device *dev = NULL; - int ret = iobroker_pwm_allocate(pin, &dev); + uint32_t channel; + int ret = iobroker_pwm_channel_allocate(pin, 800000, true, &dev, &channel); if (ret == 0) { ret = pwm_send_on(dev, pixels, num_bytes, pattern_buffer); - (void)iobroker_release(dev); + (void)iobroker_pwm_channel_release(dev, channel); return ret; } if (ret == -ENODEV || ret == -EINVAL) { From 8026199653aedaf4b10d02ccba4af26ddc46a8ca Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 9 Oct 2026 15:49:16 -0400 Subject: [PATCH 7/8] zephyr-cp: don't claim PWM drivers reject unproducible periods `pwm_nrfx` skips the period check at 0 % and 100 % duty and never checks the minimum COUNTERTOP, so out-of-range frequencies can be accepted. Say so in the iobroker README and pwmio comments; validation will be handled separately. Co-Authored-By: Claude Opus 5.5 --- ports/zephyr-cp/common-hal/pwmio/PWMOut.c | 7 ++++--- .../zephyr-cp/internal-modules/iobroker/README.md | 15 ++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c index de3d95f3d85..9fe6fb078b1 100644 --- a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c +++ b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c @@ -7,8 +7,9 @@ // pwmio on Zephyr's PWM API. The iobroker module picks the PWM device and // channel for the pin; channels that share a period (an nRF instance's // outputs) are grouped there, so nothing here is SoC-specific. The period is -// computed here from the device's clock, and the driver rejects a period it -// can't produce. +// computed here from the device's clock. Checking that the hardware can +// produce it is left to the driver, which doesn't reliably do so yet (pwm_nrfx +// skips the check at 0 % and 100 % duty). #include @@ -98,7 +99,7 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, self->period_cycles = period_cycles; self->duty_cycle = duty; self->variable_frequency = variable_frequency; - // The driver rejects a period its hardware can't produce. + // The driver may reject a period its hardware can't produce. if (set_period_and_duty(self) < 0) { (void)iobroker_pwm_channel_release(dev, channel); self->dev = NULL; diff --git a/ports/zephyr-cp/internal-modules/iobroker/README.md b/ports/zephyr-cp/internal-modules/iobroker/README.md index 6c4568924f1..6ddacaaee7e 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/README.md +++ b/ports/zephyr-cp/internal-modules/iobroker/README.md @@ -167,13 +167,14 @@ might be a slice or a timer). The call takes the requested frequency only as the sharing key: it joins a running group requested at exactly the same frequency and with a free channel, or takes a free instance. It returns the device and the channel number; the caller turns the frequency into a period -itself and passes it to `pwm_set_cycles()`, and the driver rejects a period -its hardware cannot produce. Joining or leaving a running group connects or -disconnects only that one output, so the group's other outputs are not -disturbed. An exclusive request gets a group of its own that nobody joins: -pwmio uses it for a variable frequency, and neopixel_write to program a -whole nRF instance itself. `iobroker_pwm_channel_release()` returns a -channel; the group's last channel releases the instance. +itself and passes it to `pwm_set_cycles()`. Checking that the hardware can +produce that period is left to the driver, which doesn't reliably do so yet. +Joining or leaving a running group connects or disconnects only that one +output, so the group's other outputs are not disturbed. An exclusive request +gets a group of its own that nobody joins: pwmio uses it for a variable +frequency, and neopixel_write to program a whole nRF instance itself. +`iobroker_pwm_channel_release()` returns a channel; the group's last channel +releases the instance. Not every instance can reach every pad. On nRF52 and nRF53 the routing is a full crossbar, but on nRF54L peripherals and GPIO controllers are grouped in From 58e80e74a26ac8e1408ed2e24485d3952581770f Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 9 Oct 2026 16:18:29 -0400 Subject: [PATCH 8/8] zephyr-cp: clarify PWM allocation comments, drop unreachable 0 Hz check - PWMOut frequency setter: say what shared-bindings validates, and remove the frequency == 0 check that validation makes unreachable. - iobroker.h: say iobroker_pwm_channel_allocate() doesn't set the frequency, list what a group is on each SoC, and split the comment into paragraphs. Co-Authored-By: Claude Opus 5.5 --- ports/zephyr-cp/common-hal/pwmio/PWMOut.c | 8 ++--- .../iobroker/include/iobroker/iobroker.h | 31 ++++++++++++------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c index 9fe6fb078b1..7ed25b31fd7 100644 --- a/ports/zephyr-cp/common-hal/pwmio/PWMOut.c +++ b/ports/zephyr-cp/common-hal/pwmio/PWMOut.c @@ -133,11 +133,11 @@ uint16_t common_hal_pwmio_pwmout_get_duty_cycle(pwmio_pwmout_obj_t *self) { } void common_hal_pwmio_pwmout_set_frequency(pwmio_pwmout_obj_t *self, uint32_t frequency) { - // Only used when variable_frequency=True, so the - // channel's group is exclusive and no other channel shares the period. + // shared-bindings raises AttributeError unless variable_frequency=True, + // so the channel's group is exclusive and no other channel shares the + // period. It also rejects 0 Hz, so frequency_to_period() can divide. uint32_t period_cycles; - if (frequency == 0 || - frequency_to_period(self->dev, self->channel, frequency, &period_cycles) < 0) { + if (frequency_to_period(self->dev, self->channel, frequency, &period_cycles) < 0) { common_hal_pwmio_pwmout_raise_error(PWMOUT_INVALID_FREQUENCY); } uint32_t old_period_cycles = self->period_cycles; diff --git a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h index 41a40badc01..1609826fefe 100644 --- a/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h +++ b/ports/zephyr-cp/internal-modules/iobroker/include/iobroker/iobroker.h @@ -235,17 +235,26 @@ int iobroker_spi_allocate(package_pin_t clock, package_pin_t mosi, package_pin_t miso, const struct device **dev_out); int iobroker_uart_allocate(package_pin_t tx, package_pin_t rx, package_pin_t rts, package_pin_t cts, const struct device **dev_out); -// Allocate one PWM output channel for a pin. Channels that share a period -// (all outputs of an nRF instance; on other SoCs a slice or a timer) form a -// group. frequency (Hz) only decides sharing: a request joins a running group -// requested at exactly the same frequency, unless the group or the request -// is exclusive. Otherwise a free group is taken. An exclusive request gets a -// group of its own that nobody joins, for a caller that changes the period -// or programs the group's hardware itself. What a group is depends on the -// SoC: on nRF it is a whole PWM instance. The caller turns the frequency into a -// period itself (pwm_get_cycles_per_sec()); requests at the same frequency -// thus pass identical periods to pwm_set_cycles(), as drivers require of -// channels in one group. + +// Allocate one PWM output channel for a pin. +// +// Channels that share a period form a group. What a group is depends on +// the SoC: +// nRF: a whole PWM instance (four channels) +// RP2: a slice (channels A and B) +// ESP32: an LEDC timer and the channels bound to it +// STM32: a timer (up to four channels sharing its auto-reload period) +// +// frequency (Hz) is used only to decide sharing; this call does not set the +// frequency. The caller converts it to a period itself (with +// pwm_get_cycles_per_sec()) and sets it with pwm_set_cycles(). Requests at +// the same frequency thus pass identical periods to pwm_set_cycles(), as +// drivers require of channels in one group. +// +// A request joins a running group requested at exactly the same frequency, +// unless the group or the request is exclusive. Otherwise a free group is +// taken. An exclusive request gets a group of its own that nobody joins, for +// a caller that changes the period or programs the group's hardware itself. // // On success returns 0 and sets *dev_out and *channel_out (the channel number // pwm_set_cycles() takes). The caller then calls device_init(), which returns