From ac54becef7b9522781f9c7bd0a60041f6ee367ef Mon Sep 17 00:00:00 2001 From: Philipp Erhardt Date: Thu, 14 May 2026 21:19:05 +0000 Subject: [PATCH 1/4] Add GPIO LED states, mode --- machine/cortex-m/src/native/gpio.rs | 13 +++ machine/cortex-m/src/stub/device_tree.rs | 17 ++++ .../cortex-m/st/stm32l4/interface/export.h | 2 + machine/cortex-m/st/stm32l4/interface/gpio.c | 27 ++++++ machine/cortex-m/st/stm32l4/interface/gpio.h | 2 + src/drivers/led.rs | 27 ++++-- xtasks/crates/dtgen/src/codegen/led.rs | 97 +++++++++++++++++++ 7 files changed, 178 insertions(+), 7 deletions(-) diff --git a/machine/cortex-m/src/native/gpio.rs b/machine/cortex-m/src/native/gpio.rs index 7f4659b..3bbdf6d 100644 --- a/machine/cortex-m/src/native/gpio.rs +++ b/machine/cortex-m/src/native/gpio.rs @@ -82,12 +82,25 @@ pub fn configure_output(pin: Pin, initial: Level) -> Result<()> { ok_or_err(rc, ()) } +pub fn configure_output_od(pin: Pin, initial: Level) -> Result<()> { + let mask = pin_mask(pin)?; + let rc = unsafe { bindings::gpio_configure_output_od(port_ptr(pin), mask, initial as u8) }; + ok_or_err(rc, ()) +} + pub fn write(pin: Pin, level: Level) -> Result<()> { let mask = pin_mask(pin)?; let rc = unsafe { bindings::gpio_write(port_ptr(pin), mask, level as u8) }; ok_or_err(rc, ()) } +/// Ungate the port's clock without touching mode/pull. Needed before +/// `read` on a still-analog pin (e.g. `default-state = "keep"`). +pub fn enable_port_clock(pin: Pin) -> Result<()> { + let rc = unsafe { bindings::gpio_clock_enable(port_ptr(pin)) }; + ok_or_err(rc, ()) +} + pub fn read(pin: Pin) -> Result { let mask = pin_mask(pin)?; let rc = unsafe { bindings::gpio_read(port_ptr(pin), mask) }; diff --git a/machine/cortex-m/src/stub/device_tree.rs b/machine/cortex-m/src/stub/device_tree.rs index 7fbedfb..3faf8b2 100644 --- a/machine/cortex-m/src/stub/device_tree.rs +++ b/machine/cortex-m/src/stub/device_tree.rs @@ -86,6 +86,21 @@ pub fn can_by_compatible(_compatible: &str, _ord: usize) -> Option<&'static CanR None } +#[repr(u8)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LedDefaultState { + Off, + On, + Keep, +} + +#[repr(u8)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LedOutputMode { + PushPull, + OpenDrain, +} + #[derive(Debug, Clone, Copy)] #[repr(C)] pub struct LedRegistryEntry { @@ -94,6 +109,8 @@ pub struct LedRegistryEntry { pub line: u8, pub active_low: u8, pub label: &'static str, + pub default_state: LedDefaultState, + pub output_mode: LedOutputMode, } pub const LED_REGISTRY: &[LedRegistryEntry] = &[]; diff --git a/machine/cortex-m/st/stm32l4/interface/export.h b/machine/cortex-m/st/stm32l4/interface/export.h index 0d97dd2..ceac3fd 100644 --- a/machine/cortex-m/st/stm32l4/interface/export.h +++ b/machine/cortex-m/st/stm32l4/interface/export.h @@ -219,9 +219,11 @@ void can_isr(uint8_t index); #define GPIO_PULL_DOWN 2 int gpio_configure_input(void *port, uint16_t pin_mask, uint8_t pull); int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial); +int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial); int gpio_write(void *port, uint16_t pin_mask, uint8_t level); int gpio_read(void *port, uint16_t pin_mask); int gpio_toggle(void *port, uint16_t pin_mask); +int gpio_clock_enable(void *port); // exti.c // edge_mask bitfield: 0x1 = rising, 0x2 = falling (see exti.h). diff --git a/machine/cortex-m/st/stm32l4/interface/gpio.c b/machine/cortex-m/st/stm32l4/interface/gpio.c index 81a42db..933e241 100644 --- a/machine/cortex-m/st/stm32l4/interface/gpio.c +++ b/machine/cortex-m/st/stm32l4/interface/gpio.c @@ -158,6 +158,24 @@ int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial) return 0; } +int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial) +{ + GPIO_TypeDef *p = (GPIO_TypeDef *)port; + if (!port_is_known(p) || pin_mask == 0) + return -PosixError_EINVAL; + + gpio_enable_clock(p); + /* Pre-drive ODR before switching mode so the line never glitches. */ + HAL_GPIO_WritePin(p, pin_mask, initial ? GPIO_PIN_SET : GPIO_PIN_RESET); + GPIO_InitTypeDef gpio = {0}; + gpio.Mode = GPIO_MODE_OUTPUT_OD; + gpio.Pull = GPIO_NOPULL; + gpio.Speed = GPIO_SPEED_FREQ_VERY_HIGH; + gpio.Pin = pin_mask; + HAL_GPIO_Init(p, &gpio); + return 0; +} + int gpio_write(void *port, uint16_t pin_mask, uint8_t level) { GPIO_TypeDef *p = (GPIO_TypeDef *)port; @@ -183,3 +201,12 @@ int gpio_toggle(void *port, uint16_t pin_mask) HAL_GPIO_TogglePin(p, pin_mask); return 0; } + +int gpio_clock_enable(void *port) +{ + GPIO_TypeDef *p = (GPIO_TypeDef *)port; + if (!port_is_known(p)) + return -PosixError_EINVAL; + gpio_enable_clock(p); + return 0; +} diff --git a/machine/cortex-m/st/stm32l4/interface/gpio.h b/machine/cortex-m/st/stm32l4/interface/gpio.h index 46ab9b0..1c26e7b 100644 --- a/machine/cortex-m/st/stm32l4/interface/gpio.h +++ b/machine/cortex-m/st/stm32l4/interface/gpio.h @@ -20,7 +20,9 @@ void gpio_init_output_od(GPIO_TypeDef *port, uint16_t pin_mask); int gpio_configure_input(void *port, uint16_t pin_mask, uint8_t pull); int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial); +int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial); int gpio_write(void *port, uint16_t pin_mask, uint8_t level); /* gpio_read returns 0 or 1 (level) on success, or negative PosixError. */ int gpio_read(void *port, uint16_t pin_mask); int gpio_toggle(void *port, uint16_t pin_mask); +int gpio_clock_enable(void *port); diff --git a/src/drivers/led.rs b/src/drivers/led.rs index c07508a..08632de 100644 --- a/src/drivers/led.rs +++ b/src/drivers/led.rs @@ -6,7 +6,7 @@ use crate::error::Result; use crate::hal; use crate::sync::once::OnceCell; -use hal::device_tree::LedRegistryEntry; +use hal::device_tree::{LedDefaultState, LedOutputMode, LedRegistryEntry}; use hal::gpio::{Level, Pin}; struct LedState { @@ -98,17 +98,30 @@ pub fn init() { }; let state_ref: &'static LedState = SLOTS[i].set_or_get(state); - // Drive the off level before switching to output so the line - // never glitches the active polarity on boot. - let off = level_for(entry, false); - match hal::gpio::configure_output(pin_of(entry), off) { + let initial = match entry.default_state { + LedDefaultState::Off => level_for(entry, false), + LedDefaultState::On => level_for(entry, true), + LedDefaultState::Keep => { + // Read pre-init level (warm restart). enable_port_clock so + // the read isn't on a gated bus. + let _ = hal::gpio::enable_port_clock(pin_of(entry)); + hal::gpio::read(pin_of(entry)).unwrap_or(level_for(entry, false)) + } + }; + let configure = match entry.output_mode { + LedOutputMode::OpenDrain => hal::gpio::configure_output_od, + LedOutputMode::PushPull => hal::gpio::configure_output, + }; + match configure(pin_of(entry), initial) { Ok(()) => { state_ref.initialized.store(true, Ordering::Release); kprintln!( - " Initialized LED {} on port 0x{:x} line {}", + " Initialized LED {} on port 0x{:x} line {} ({:?}, {:?})", entry.label, entry.port, - entry.line + entry.line, + entry.default_state, + entry.output_mode, ); } Err(e) => kprintln!(" LED {}: configure_output failed: {:?}", entry.label, e), diff --git a/xtasks/crates/dtgen/src/codegen/led.rs b/xtasks/crates/dtgen/src/codegen/led.rs index b044720..ebaf47d 100644 --- a/xtasks/crates/dtgen/src/codegen/led.rs +++ b/xtasks/crates/dtgen/src/codegen/led.rs @@ -4,6 +4,77 @@ use super::*; +#[derive(Clone, Copy)] +enum DefaultState { + Off, + On, + Keep, +} + +impl DefaultState { + fn from_node(node: &crate::ir::Node) -> Self { + match node.extra.get("default-state") { + Some(PropValue::Str(s)) => match s.as_str() { + "on" => DefaultState::On, + "keep" => DefaultState::Keep, + "off" => DefaultState::Off, + other => panic!( + "gpio-leds child {}: unknown `default-state` value {:?} \ + (expected \"on\", \"off\", or \"keep\")", + node.name, other + ), + }, + None => DefaultState::Off, + _ => panic!( + "gpio-leds child {}: `default-state` must be a string", + node.name + ), + } + } + + fn tokens(self) -> TokenStream { + match self { + DefaultState::Off => quote! { LedDefaultState::Off }, + DefaultState::On => quote! { LedDefaultState::On }, + DefaultState::Keep => quote! { LedDefaultState::Keep }, + } + } +} + +#[derive(Clone, Copy)] +enum OutputMode { + PushPull, + OpenDrain, +} + +impl OutputMode { + fn from_node(node: &crate::ir::Node) -> Self { + match node.extra.get("osiris,output-mode") { + Some(PropValue::Str(s)) => match s.as_str() { + "push-pull" => OutputMode::PushPull, + "open-drain" => OutputMode::OpenDrain, + other => panic!( + "gpio-leds child {}: unknown `osiris,output-mode` value {:?} \ + (expected \"push-pull\" or \"open-drain\")", + node.name, other + ), + }, + None => OutputMode::PushPull, + _ => panic!( + "gpio-leds child {}: `osiris,output-mode` must be a string", + node.name + ), + } + } + + fn tokens(self) -> TokenStream { + match self { + OutputMode::PushPull => quote! { LedOutputMode::PushPull }, + OutputMode::OpenDrain => quote! { LedOutputMode::OpenDrain }, + } + } +} + #[derive(Clone)] struct Led { node: usize, @@ -11,6 +82,8 @@ struct Led { line: u8, active_low: u8, label: String, + default_state: DefaultState, + output_mode: OutputMode, } fn collect_leds(dt: &DeviceTree) -> Vec { @@ -22,6 +95,8 @@ fn collect_leds(dt: &DeviceTree) -> Vec { line: c.line, active_low: c.active_low, label: c.label, + default_state: DefaultState::from_node(c.child), + output_mode: OutputMode::from_node(c.child), }) .collect(); @@ -53,6 +128,8 @@ pub fn emit_registry(dt: &DeviceTree) -> TokenStream { let line = l.line; let active_low = l.active_low; let label = l.label.as_str(); + let default_state = l.default_state.tokens(); + let output_mode = l.output_mode.tokens(); quote! { LedRegistryEntry { node: #node, @@ -60,11 +137,29 @@ pub fn emit_registry(dt: &DeviceTree) -> TokenStream { line: #line, active_low: #active_low, label: #label, + default_state: #default_state, + output_mode: #output_mode, }, } }); quote! { + #[repr(u8)] + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub enum LedDefaultState { + Off, + On, + /// Preserve current ODR (warm restart); cold boot reads analog → 0. + Keep, + } + + #[repr(u8)] + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub enum LedOutputMode { + PushPull, + OpenDrain, + } + #[derive(Debug, Clone, Copy)] #[repr(C)] pub struct LedRegistryEntry { @@ -73,6 +168,8 @@ pub fn emit_registry(dt: &DeviceTree) -> TokenStream { pub line: u8, pub active_low: u8, pub label: &'static str, + pub default_state: LedDefaultState, + pub output_mode: LedOutputMode, } pub const LED_REGISTRY: &[LedRegistryEntry] = &[ From dbdd7171b9873124b9fdfb2f3c5c0f76733f7dc2 Mon Sep 17 00:00:00 2001 From: Philipp Erhardt Date: Thu, 14 May 2026 21:56:44 +0000 Subject: [PATCH 2/4] Copilot review --- machine/cortex-m/src/native/gpio.rs | 22 +++++++- machine/cortex-m/src/stub/device_tree.rs | 9 +++ machine/cortex-m/src/stub/gpio.rs | 12 ++++ .../cortex-m/st/stm32l4/interface/export.h | 4 +- machine/cortex-m/st/stm32l4/interface/gpio.c | 22 +++++++- machine/cortex-m/st/stm32l4/interface/gpio.h | 4 +- src/drivers/led.rs | 29 ++++++---- xtasks/crates/dtgen/src/codegen/led.rs | 56 ++++++++++++++++++- 8 files changed, 141 insertions(+), 17 deletions(-) diff --git a/machine/cortex-m/src/native/gpio.rs b/machine/cortex-m/src/native/gpio.rs index 3bbdf6d..dcbd7c9 100644 --- a/machine/cortex-m/src/native/gpio.rs +++ b/machine/cortex-m/src/native/gpio.rs @@ -82,9 +82,11 @@ pub fn configure_output(pin: Pin, initial: Level) -> Result<()> { ok_or_err(rc, ()) } -pub fn configure_output_od(pin: Pin, initial: Level) -> Result<()> { +pub fn configure_output_od(pin: Pin, initial: Level, pull: Pull) -> Result<()> { let mask = pin_mask(pin)?; - let rc = unsafe { bindings::gpio_configure_output_od(port_ptr(pin), mask, initial as u8) }; + let rc = unsafe { + bindings::gpio_configure_output_od(port_ptr(pin), mask, initial as u8, pull as u8) + }; ok_or_err(rc, ()) } @@ -112,6 +114,22 @@ pub fn read(pin: Pin) -> Result { } } +/// Read the latched output (ODR) bit, not the input pad level. Use +/// when the pin may not yet be configured as output: `read` goes via +/// IDR which is forced to 0 in analog mode (RM0432 §8.3.12), so it +/// can't recover the previously commanded level on a freshly-reset +/// pin. Caller must have ungated the port clock. +pub fn read_odr(pin: Pin) -> Result { + let mask = pin_mask(pin)?; + let rc = unsafe { bindings::gpio_read_odr(port_ptr(pin), mask) }; + match rc { + 0 => Ok(Level::Low), + 1 => Ok(Level::High), + other if other < 0 => Err(PosixError::from_errno(-other)), + _ => Err(PosixError::EIO), + } +} + pub fn toggle(pin: Pin) -> Result<()> { let mask = pin_mask(pin)?; let rc = unsafe { bindings::gpio_toggle(port_ptr(pin), mask) }; diff --git a/machine/cortex-m/src/stub/device_tree.rs b/machine/cortex-m/src/stub/device_tree.rs index 3faf8b2..fa54f72 100644 --- a/machine/cortex-m/src/stub/device_tree.rs +++ b/machine/cortex-m/src/stub/device_tree.rs @@ -101,6 +101,14 @@ pub enum LedOutputMode { OpenDrain, } +#[repr(u8)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LedPull { + None, + Up, + Down, +} + #[derive(Debug, Clone, Copy)] #[repr(C)] pub struct LedRegistryEntry { @@ -111,6 +119,7 @@ pub struct LedRegistryEntry { pub label: &'static str, pub default_state: LedDefaultState, pub output_mode: LedOutputMode, + pub pull: LedPull, } pub const LED_REGISTRY: &[LedRegistryEntry] = &[]; diff --git a/machine/cortex-m/src/stub/gpio.rs b/machine/cortex-m/src/stub/gpio.rs index 78809eb..5038346 100644 --- a/machine/cortex-m/src/stub/gpio.rs +++ b/machine/cortex-m/src/stub/gpio.rs @@ -55,14 +55,26 @@ pub fn configure_output(_pin: Pin, _initial: Level) -> Result<()> { Err(PosixError::EOPNOTSUPP) } +pub fn configure_output_od(_pin: Pin, _initial: Level, _pull: Pull) -> Result<()> { + Err(PosixError::EOPNOTSUPP) +} + pub fn write(_pin: Pin, _level: Level) -> Result<()> { Err(PosixError::EOPNOTSUPP) } +pub fn enable_port_clock(_pin: Pin) -> Result<()> { + Err(PosixError::EOPNOTSUPP) +} + pub fn read(_pin: Pin) -> Result { Err(PosixError::EOPNOTSUPP) } +pub fn read_odr(_pin: Pin) -> Result { + Err(PosixError::EOPNOTSUPP) +} + pub fn toggle(_pin: Pin) -> Result<()> { Err(PosixError::EOPNOTSUPP) } diff --git a/machine/cortex-m/st/stm32l4/interface/export.h b/machine/cortex-m/st/stm32l4/interface/export.h index ceac3fd..f54762a 100644 --- a/machine/cortex-m/st/stm32l4/interface/export.h +++ b/machine/cortex-m/st/stm32l4/interface/export.h @@ -219,9 +219,11 @@ void can_isr(uint8_t index); #define GPIO_PULL_DOWN 2 int gpio_configure_input(void *port, uint16_t pin_mask, uint8_t pull); int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial); -int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial); +int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial, + uint8_t pull); int gpio_write(void *port, uint16_t pin_mask, uint8_t level); int gpio_read(void *port, uint16_t pin_mask); +int gpio_read_odr(void *port, uint16_t pin_mask); int gpio_toggle(void *port, uint16_t pin_mask); int gpio_clock_enable(void *port); diff --git a/machine/cortex-m/st/stm32l4/interface/gpio.c b/machine/cortex-m/st/stm32l4/interface/gpio.c index 933e241..2d9e841 100644 --- a/machine/cortex-m/st/stm32l4/interface/gpio.c +++ b/machine/cortex-m/st/stm32l4/interface/gpio.c @@ -158,18 +158,21 @@ int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial) return 0; } -int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial) +int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial, + uint8_t pull) { GPIO_TypeDef *p = (GPIO_TypeDef *)port; if (!port_is_known(p) || pin_mask == 0) return -PosixError_EINVAL; + if (pull > GPIO_PULL_DOWN) + return -PosixError_EINVAL; gpio_enable_clock(p); /* Pre-drive ODR before switching mode so the line never glitches. */ HAL_GPIO_WritePin(p, pin_mask, initial ? GPIO_PIN_SET : GPIO_PIN_RESET); GPIO_InitTypeDef gpio = {0}; gpio.Mode = GPIO_MODE_OUTPUT_OD; - gpio.Pull = GPIO_NOPULL; + gpio.Pull = pull_to_hal(pull); gpio.Speed = GPIO_SPEED_FREQ_VERY_HIGH; gpio.Pin = pin_mask; HAL_GPIO_Init(p, &gpio); @@ -193,6 +196,21 @@ int gpio_read(void *port, uint16_t pin_mask) return HAL_GPIO_ReadPin(p, pin_mask) == GPIO_PIN_SET ? 1 : 0; } +/* Read the output data register directly. Unlike gpio_read (which goes + * through the input data register and returns 0 when the pin is in + * analog mode — see RM0432 §8.3.12), this reflects the latched output + * value regardless of the current pin mode. Use this when you need to + * recover the previously commanded level on a pin that may not yet be + * configured as output (e.g. `default-state = "keep"` at boot). The + * port clock must already be ungated. */ +int gpio_read_odr(void *port, uint16_t pin_mask) +{ + GPIO_TypeDef *p = (GPIO_TypeDef *)port; + if (!port_is_known(p) || pin_mask == 0) + return -PosixError_EINVAL; + return (p->ODR & pin_mask) ? 1 : 0; +} + int gpio_toggle(void *port, uint16_t pin_mask) { GPIO_TypeDef *p = (GPIO_TypeDef *)port; diff --git a/machine/cortex-m/st/stm32l4/interface/gpio.h b/machine/cortex-m/st/stm32l4/interface/gpio.h index 1c26e7b..3eeefbd 100644 --- a/machine/cortex-m/st/stm32l4/interface/gpio.h +++ b/machine/cortex-m/st/stm32l4/interface/gpio.h @@ -20,9 +20,11 @@ void gpio_init_output_od(GPIO_TypeDef *port, uint16_t pin_mask); int gpio_configure_input(void *port, uint16_t pin_mask, uint8_t pull); int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial); -int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial); +int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial, + uint8_t pull); int gpio_write(void *port, uint16_t pin_mask, uint8_t level); /* gpio_read returns 0 or 1 (level) on success, or negative PosixError. */ int gpio_read(void *port, uint16_t pin_mask); +int gpio_read_odr(void *port, uint16_t pin_mask); int gpio_toggle(void *port, uint16_t pin_mask); int gpio_clock_enable(void *port); diff --git a/src/drivers/led.rs b/src/drivers/led.rs index 08632de..d419df8 100644 --- a/src/drivers/led.rs +++ b/src/drivers/led.rs @@ -6,8 +6,8 @@ use crate::error::Result; use crate::hal; use crate::sync::once::OnceCell; -use hal::device_tree::{LedDefaultState, LedOutputMode, LedRegistryEntry}; -use hal::gpio::{Level, Pin}; +use hal::device_tree::{LedDefaultState, LedOutputMode, LedPull, LedRegistryEntry}; +use hal::gpio::{Level, Pin, Pull}; struct LedState { entry: &'static LedRegistryEntry, @@ -102,26 +102,35 @@ pub fn init() { LedDefaultState::Off => level_for(entry, false), LedDefaultState::On => level_for(entry, true), LedDefaultState::Keep => { - // Read pre-init level (warm restart). enable_port_clock so - // the read isn't on a gated bus. + // ODR survives a warm reset that doesn't touch the GPIO + // peripheral; read it directly so we don't rely on the + // input driver (forced to 0 in the analog reset state — + // see RM0432 §8.3.12). Cold boot ODR = 0 → falls + // through to Off. let _ = hal::gpio::enable_port_clock(pin_of(entry)); - hal::gpio::read(pin_of(entry)).unwrap_or(level_for(entry, false)) + hal::gpio::read_odr(pin_of(entry)).unwrap_or(level_for(entry, false)) } }; - let configure = match entry.output_mode { - LedOutputMode::OpenDrain => hal::gpio::configure_output_od, - LedOutputMode::PushPull => hal::gpio::configure_output, + let pull = match entry.pull { + LedPull::None => Pull::None, + LedPull::Up => Pull::Up, + LedPull::Down => Pull::Down, }; - match configure(pin_of(entry), initial) { + let res = match entry.output_mode { + LedOutputMode::OpenDrain => hal::gpio::configure_output_od(pin_of(entry), initial, pull), + LedOutputMode::PushPull => hal::gpio::configure_output(pin_of(entry), initial), + }; + match res { Ok(()) => { state_ref.initialized.store(true, Ordering::Release); kprintln!( - " Initialized LED {} on port 0x{:x} line {} ({:?}, {:?})", + " Initialized LED {} on port 0x{:x} line {} ({:?}, {:?}, {:?})", entry.label, entry.port, entry.line, entry.default_state, entry.output_mode, + entry.pull, ); } Err(e) => kprintln!(" LED {}: configure_output failed: {:?}", entry.label, e), diff --git a/xtasks/crates/dtgen/src/codegen/led.rs b/xtasks/crates/dtgen/src/codegen/led.rs index ebaf47d..6885993 100644 --- a/xtasks/crates/dtgen/src/codegen/led.rs +++ b/xtasks/crates/dtgen/src/codegen/led.rs @@ -75,6 +75,47 @@ impl OutputMode { } } +/// Standard Linux/Zephyr pinctrl `bias-*` triple (empty properties); +/// see Linux's `pinctrl-bindings.txt` and Zephyr's `pincfg-node.yaml`. +/// Absent → no pull (the binding's documented default). +#[derive(Clone, Copy)] +enum Pull { + None, + Up, + Down, +} + +impl Pull { + fn from_node(node: &crate::ir::Node) -> Self { + let up = node.extra.contains_key("bias-pull-up"); + let down = node.extra.contains_key("bias-pull-down"); + let disable = node.extra.contains_key("bias-disable"); + let count = up as u8 + down as u8 + disable as u8; + if count > 1 { + panic!( + "gpio-leds child {}: only one of `bias-pull-up`, `bias-pull-down`, \ + `bias-disable` may be set", + node.name + ); + } + if up { + Pull::Up + } else if down { + Pull::Down + } else { + Pull::None + } + } + + fn tokens(self) -> TokenStream { + match self { + Pull::None => quote! { LedPull::None }, + Pull::Up => quote! { LedPull::Up }, + Pull::Down => quote! { LedPull::Down }, + } + } +} + #[derive(Clone)] struct Led { node: usize, @@ -84,6 +125,7 @@ struct Led { label: String, default_state: DefaultState, output_mode: OutputMode, + pull: Pull, } fn collect_leds(dt: &DeviceTree) -> Vec { @@ -97,6 +139,7 @@ fn collect_leds(dt: &DeviceTree) -> Vec { label: c.label, default_state: DefaultState::from_node(c.child), output_mode: OutputMode::from_node(c.child), + pull: Pull::from_node(c.child), }) .collect(); @@ -130,6 +173,7 @@ pub fn emit_registry(dt: &DeviceTree) -> TokenStream { let label = l.label.as_str(); let default_state = l.default_state.tokens(); let output_mode = l.output_mode.tokens(); + let pull = l.pull.tokens(); quote! { LedRegistryEntry { node: #node, @@ -139,6 +183,7 @@ pub fn emit_registry(dt: &DeviceTree) -> TokenStream { label: #label, default_state: #default_state, output_mode: #output_mode, + pull: #pull, }, } }); @@ -149,7 +194,7 @@ pub fn emit_registry(dt: &DeviceTree) -> TokenStream { pub enum LedDefaultState { Off, On, - /// Preserve current ODR (warm restart); cold boot reads analog → 0. + /// Preserve current ODR (warm restart); cold boot reads ODR → 0. Keep, } @@ -160,6 +205,14 @@ pub fn emit_registry(dt: &DeviceTree) -> TokenStream { OpenDrain, } + #[repr(u8)] + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub enum LedPull { + None, + Up, + Down, + } + #[derive(Debug, Clone, Copy)] #[repr(C)] pub struct LedRegistryEntry { @@ -170,6 +223,7 @@ pub fn emit_registry(dt: &DeviceTree) -> TokenStream { pub label: &'static str, pub default_state: LedDefaultState, pub output_mode: LedOutputMode, + pub pull: LedPull, } pub const LED_REGISTRY: &[LedRegistryEntry] = &[ From ed7c9ea046f700ab1a1e1eac3c1fc1a2126ba164 Mon Sep 17 00:00:00 2001 From: Philipp Erhardt Date: Thu, 14 May 2026 21:58:12 +0000 Subject: [PATCH 3/4] Formatting --- src/drivers/led.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/drivers/led.rs b/src/drivers/led.rs index d419df8..e00730a 100644 --- a/src/drivers/led.rs +++ b/src/drivers/led.rs @@ -117,7 +117,9 @@ pub fn init() { LedPull::Down => Pull::Down, }; let res = match entry.output_mode { - LedOutputMode::OpenDrain => hal::gpio::configure_output_od(pin_of(entry), initial, pull), + LedOutputMode::OpenDrain => { + hal::gpio::configure_output_od(pin_of(entry), initial, pull) + } LedOutputMode::PushPull => hal::gpio::configure_output(pin_of(entry), initial), }; match res { From e13ee6aeea522b2b124149bd51c6644a9d227066 Mon Sep 17 00:00:00 2001 From: Philipp Erhardt Date: Fri, 15 May 2026 10:38:02 +0000 Subject: [PATCH 4/4] Copilot feedback --- machine/cortex-m/src/native/gpio.rs | 18 ++++---- machine/cortex-m/src/stub/gpio.rs | 2 +- .../cortex-m/st/stm32l4/interface/export.h | 3 +- machine/cortex-m/st/stm32l4/interface/gpio.c | 19 +++++---- machine/cortex-m/st/stm32l4/interface/gpio.h | 3 +- src/drivers/led.rs | 42 ++++++++++++++----- xtasks/crates/dtgen/src/codegen/led.rs | 24 ++++++----- 7 files changed, 69 insertions(+), 42 deletions(-) diff --git a/machine/cortex-m/src/native/gpio.rs b/machine/cortex-m/src/native/gpio.rs index dcbd7c9..9609836 100644 --- a/machine/cortex-m/src/native/gpio.rs +++ b/machine/cortex-m/src/native/gpio.rs @@ -76,9 +76,11 @@ pub fn configure_input(pin: Pin, pull: Pull) -> Result<()> { ok_or_err(rc, ()) } -pub fn configure_output(pin: Pin, initial: Level) -> Result<()> { +pub fn configure_output(pin: Pin, initial: Level, pull: Pull) -> Result<()> { let mask = pin_mask(pin)?; - let rc = unsafe { bindings::gpio_configure_output_pp(port_ptr(pin), mask, initial as u8) }; + let rc = unsafe { + bindings::gpio_configure_output_pp(port_ptr(pin), mask, initial as u8, pull as u8) + }; ok_or_err(rc, ()) } @@ -96,8 +98,8 @@ pub fn write(pin: Pin, level: Level) -> Result<()> { ok_or_err(rc, ()) } -/// Ungate the port's clock without touching mode/pull. Needed before -/// `read` on a still-analog pin (e.g. `default-state = "keep"`). +/// Ungate the port's clock without touching mode/pull — needed +/// before `read_odr` on a still-analog pin. pub fn enable_port_clock(pin: Pin) -> Result<()> { let rc = unsafe { bindings::gpio_clock_enable(port_ptr(pin)) }; ok_or_err(rc, ()) @@ -114,11 +116,9 @@ pub fn read(pin: Pin) -> Result { } } -/// Read the latched output (ODR) bit, not the input pad level. Use -/// when the pin may not yet be configured as output: `read` goes via -/// IDR which is forced to 0 in analog mode (RM0432 §8.3.12), so it -/// can't recover the previously commanded level on a freshly-reset -/// pin. Caller must have ungated the port clock. +/// Read ODR (latched output bit). Use instead of `read` on a pin +/// that may still be in analog mode — IDR is forced to 0 there +/// (RM0432 §8.3.12). Clock must be ungated. pub fn read_odr(pin: Pin) -> Result { let mask = pin_mask(pin)?; let rc = unsafe { bindings::gpio_read_odr(port_ptr(pin), mask) }; diff --git a/machine/cortex-m/src/stub/gpio.rs b/machine/cortex-m/src/stub/gpio.rs index 5038346..c83fe6b 100644 --- a/machine/cortex-m/src/stub/gpio.rs +++ b/machine/cortex-m/src/stub/gpio.rs @@ -51,7 +51,7 @@ pub fn configure_input(_pin: Pin, _pull: Pull) -> Result<()> { Err(PosixError::EOPNOTSUPP) } -pub fn configure_output(_pin: Pin, _initial: Level) -> Result<()> { +pub fn configure_output(_pin: Pin, _initial: Level, _pull: Pull) -> Result<()> { Err(PosixError::EOPNOTSUPP) } diff --git a/machine/cortex-m/st/stm32l4/interface/export.h b/machine/cortex-m/st/stm32l4/interface/export.h index f54762a..f8c4102 100644 --- a/machine/cortex-m/st/stm32l4/interface/export.h +++ b/machine/cortex-m/st/stm32l4/interface/export.h @@ -218,7 +218,8 @@ void can_isr(uint8_t index); #define GPIO_PULL_UP 1 #define GPIO_PULL_DOWN 2 int gpio_configure_input(void *port, uint16_t pin_mask, uint8_t pull); -int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial); +int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial, + uint8_t pull); int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial, uint8_t pull); int gpio_write(void *port, uint16_t pin_mask, uint8_t level); diff --git a/machine/cortex-m/st/stm32l4/interface/gpio.c b/machine/cortex-m/st/stm32l4/interface/gpio.c index 2d9e841..af19d48 100644 --- a/machine/cortex-m/st/stm32l4/interface/gpio.c +++ b/machine/cortex-m/st/stm32l4/interface/gpio.c @@ -139,11 +139,14 @@ int gpio_configure_input(void *port, uint16_t pin_mask, uint8_t pull) return 0; } -int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial) +int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial, + uint8_t pull) { GPIO_TypeDef *p = (GPIO_TypeDef *)port; if (!port_is_known(p) || pin_mask == 0) return -PosixError_EINVAL; + if (pull > GPIO_PULL_DOWN) + return -PosixError_EINVAL; gpio_enable_clock(p); /* Drive the initial level before switching the pin to output mode so the @@ -151,7 +154,7 @@ int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial) HAL_GPIO_WritePin(p, pin_mask, initial ? GPIO_PIN_SET : GPIO_PIN_RESET); GPIO_InitTypeDef gpio = {0}; gpio.Mode = GPIO_MODE_OUTPUT_PP; - gpio.Pull = GPIO_NOPULL; + gpio.Pull = pull_to_hal(pull); gpio.Speed = GPIO_SPEED_FREQ_VERY_HIGH; gpio.Pin = pin_mask; HAL_GPIO_Init(p, &gpio); @@ -196,18 +199,16 @@ int gpio_read(void *port, uint16_t pin_mask) return HAL_GPIO_ReadPin(p, pin_mask) == GPIO_PIN_SET ? 1 : 0; } -/* Read the output data register directly. Unlike gpio_read (which goes - * through the input data register and returns 0 when the pin is in - * analog mode — see RM0432 §8.3.12), this reflects the latched output - * value regardless of the current pin mode. Use this when you need to - * recover the previously commanded level on a pin that may not yet be - * configured as output (e.g. `default-state = "keep"` at boot). The - * port clock must already be ungated. */ +/* Read ODR directly; gpio_read goes via IDR which is forced to 0 in + * analog mode (RM0432 §8.3.12). Single-bit mask only — multi-bit + * would OR several ODR bits into one 0/1. Clock must be ungated. */ int gpio_read_odr(void *port, uint16_t pin_mask) { GPIO_TypeDef *p = (GPIO_TypeDef *)port; if (!port_is_known(p) || pin_mask == 0) return -PosixError_EINVAL; + if (pin_mask & (uint16_t)(pin_mask - 1)) + return -PosixError_EINVAL; return (p->ODR & pin_mask) ? 1 : 0; } diff --git a/machine/cortex-m/st/stm32l4/interface/gpio.h b/machine/cortex-m/st/stm32l4/interface/gpio.h index 3eeefbd..31090e7 100644 --- a/machine/cortex-m/st/stm32l4/interface/gpio.h +++ b/machine/cortex-m/st/stm32l4/interface/gpio.h @@ -19,7 +19,8 @@ void gpio_init_output_od(GPIO_TypeDef *port, uint16_t pin_mask); * All return 0 on success or a negative PosixError code on failure. */ int gpio_configure_input(void *port, uint16_t pin_mask, uint8_t pull); -int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial); +int gpio_configure_output_pp(void *port, uint16_t pin_mask, uint8_t initial, + uint8_t pull); int gpio_configure_output_od(void *port, uint16_t pin_mask, uint8_t initial, uint8_t pull); int gpio_write(void *port, uint16_t pin_mask, uint8_t level); diff --git a/src/drivers/led.rs b/src/drivers/led.rs index e00730a..b4bf745 100644 --- a/src/drivers/led.rs +++ b/src/drivers/led.rs @@ -87,6 +87,36 @@ fn level_for(entry: &LedRegistryEntry, on: bool) -> Level { } } +/// `default-state = "keep"`. Reads ODR (set by whatever ran before us, +/// e.g. a bootloader) instead of IDR — IDR is forced to 0 on an +/// analog-mode pin (RM0432 §8.3.12). Only ODR=High is preserved; Low +/// falls back to logical-Off because ODR's reset value is also 0 +/// (RM0432 §8.4.6), so Low is ambiguous and would illuminate +/// active-low LEDs on cold boot. +fn keep_initial_level(entry: &LedRegistryEntry) -> Level { + let pin = pin_of(entry); + if let Err(e) = hal::gpio::enable_port_clock(pin) { + kprintln!( + " LED {}: keep: enable_port_clock failed ({:?}); falling back to Off", + entry.label, + e, + ); + return level_for(entry, false); + } + match hal::gpio::read_odr(pin) { + Ok(Level::High) => Level::High, + Ok(Level::Low) => level_for(entry, false), + Err(e) => { + kprintln!( + " LED {}: keep: read_odr failed ({:?}); falling back to Off", + entry.label, + e, + ); + level_for(entry, false) + } + } +} + pub fn init() { let entries = hal::device_tree::LED_REGISTRY; kprintln!("Found {} gpio-led entries", entries.len()); @@ -101,15 +131,7 @@ pub fn init() { let initial = match entry.default_state { LedDefaultState::Off => level_for(entry, false), LedDefaultState::On => level_for(entry, true), - LedDefaultState::Keep => { - // ODR survives a warm reset that doesn't touch the GPIO - // peripheral; read it directly so we don't rely on the - // input driver (forced to 0 in the analog reset state — - // see RM0432 §8.3.12). Cold boot ODR = 0 → falls - // through to Off. - let _ = hal::gpio::enable_port_clock(pin_of(entry)); - hal::gpio::read_odr(pin_of(entry)).unwrap_or(level_for(entry, false)) - } + LedDefaultState::Keep => keep_initial_level(entry), }; let pull = match entry.pull { LedPull::None => Pull::None, @@ -120,7 +142,7 @@ pub fn init() { LedOutputMode::OpenDrain => { hal::gpio::configure_output_od(pin_of(entry), initial, pull) } - LedOutputMode::PushPull => hal::gpio::configure_output(pin_of(entry), initial), + LedOutputMode::PushPull => hal::gpio::configure_output(pin_of(entry), initial, pull), }; match res { Ok(()) => { diff --git a/xtasks/crates/dtgen/src/codegen/led.rs b/xtasks/crates/dtgen/src/codegen/led.rs index 6885993..8012d8e 100644 --- a/xtasks/crates/dtgen/src/codegen/led.rs +++ b/xtasks/crates/dtgen/src/codegen/led.rs @@ -24,10 +24,12 @@ impl DefaultState { node.name, other ), }, - None => DefaultState::Off, - _ => panic!( - "gpio-leds child {}: `default-state` must be a string", - node.name + // Flag-form (`default-state;`) → Off, matching the Linux binding. + Some(PropValue::Empty) | None => DefaultState::Off, + Some(other) => panic!( + "gpio-leds child {}: `default-state` must be a string \ + (\"on\" / \"off\" / \"keep\"), got {:?}", + node.name, other ), } } @@ -59,10 +61,12 @@ impl OutputMode { node.name, other ), }, - None => OutputMode::PushPull, - _ => panic!( - "gpio-leds child {}: `osiris,output-mode` must be a string", - node.name + // Flag-form / absent → push-pull (the pre-existing default). + Some(PropValue::Empty) | None => OutputMode::PushPull, + Some(other) => panic!( + "gpio-leds child {}: `osiris,output-mode` must be a string \ + (\"push-pull\" / \"open-drain\"), got {:?}", + node.name, other ), } } @@ -75,9 +79,7 @@ impl OutputMode { } } -/// Standard Linux/Zephyr pinctrl `bias-*` triple (empty properties); -/// see Linux's `pinctrl-bindings.txt` and Zephyr's `pincfg-node.yaml`. -/// Absent → no pull (the binding's documented default). +/// Linux/Zephyr `bias-*` triple (empty flag props). Absent → no pull. #[derive(Clone, Copy)] enum Pull { None,