Skip to content
David Brown edited this page Apr 9, 2026 · 1 revision

PR #146 — Add raw DT properties, GPIO detection, and byte support

Author: d3zd3z (David Brown) | Base: main | Size: 7 files, +184/−6 lines CI: passing ✓ (build + docs) Review-time: 2026-04-08


What this changes

This PR makes the full devicetree visible to Rust application code at runtime by generating pub static RAW_<PROP> constants for every property on every DT node, using a new Value/Word enum representation. Within that representation, GPIO phandles are detected at build time (via gpio-controller and #gpio-cells) and emitted as a typed Word::Gpio variant instead of a bare phandle reference. To make the generated data usable, several internal types (NoStatic, GpioStatic) are widened from pub(crate) to pub, and a new unsafe fn raw_new() constructor is added on GpioPin. A separate bug fix prevents compile errors when a Parent augment references a disabled ancestor node.

Map of changes

Area Files Purpose
Config dt-rust.yaml New raw_properties augment rule (empty rules = all nodes)
Build/codegen zephyr-build/src/devicetree.rs Value::to_tokens() — core serializer with GPIO detection logic
Build/codegen zephyr-build/src/devicetree/augment.rs RawProperties action + Parent disabled-ancestor fix
Runtime types zephyr/src/lib.rs Value and Word enums in devicetree module
Runtime API zephyr/src/device.rs NoStatic visibility: pub(crate)pub
Runtime API zephyr/src/device/gpio.rs GpioStatic visibility × 2, Debug for GpioPin, raw_new()
C interop zephyr-sys/wrapper.h GPIO_OUTPUT_INACTIVE constant

Notable: The Parent disabled-ancestor fix is a separate bug fix bundled into a feature PR. The Debug impl on GpioPin is also orthogonal. Both would benefit from separate commits for bisectability.

Where to focus

  1. to_tokens() GPIO iterator logic (zephyr-build/src/devicetree.rs): The lookahead-then-advance pattern (clone iterator, peek #gpio-cells words, then advance real iterator) is the most complex new code. The logic is correct on inspection — the two loops stay in sync because the slice iterator length is known — but it is fragile and has no automated test coverage. Silent fallback paths (unresolved phandle, missing #gpio-cells, non-number trailing words) mean bugs would produce subtly wrong output rather than errors.

  2. unsafe fn raw_new() (zephyr/src/device/gpio.rs:321): New public unsafe constructor that bypasses the Unique single-instance guard. Missing a # Safety doc section documenting preconditions. The pin as gpio_pin_t cast silently truncates u32u8; if pin >= 32 the async GpioStatic path would index out-of-bounds on the wakers array.

  3. Parent ancestor walk (zephyr-build/src/devicetree/augment.rs): If the walk overshoots the root (level > tree depth), ancestor becomes None, the status check is skipped, and the generated code will reference a non-existent super:: path — producing a confusing compile error. A descriptive panic! would be friendlier.

  4. RawProperties applies to every node (dt-rust.yaml): rules: [] means every DT node gets RAW_* statics. This will increase generated file size and compile time. Dead-code elimination should prevent binary bloat, but the compile-time cost deserves consideration.

  5. Visibility expansion: NoStatic, GpioStatic, and get_static_raw() are now part of the public API surface. Combined with raw_new(), this creates a complete external path to construct GpioPin without the Unique guard.

Things to consider

  • [concern] raw_new() needs a # Safety section: preconditions should document (a) device must be valid non-null initialized Zephyr device, (b) pin < 32 for async builds, (c) caller must manage concurrent access. Standard practice for unsafe fn.

  • [concern] raw_new() is dead code — no caller exists in the tree. Its intended consumer (code that reads RAW_* properties and constructs GpioPin) isn't in this PR. Shipping an untested, undocumented pub unsafe fn without a caller is a latent risk.

  • [question] Should the GPIO detection logic live inside the general Value::to_tokens(), or in a GPIO-specific augment action? Embedding peripheral-specific logic in the general value serializer couples layers that could be independent.

  • [question] The build-time Value enum (in zephyr-build) and the runtime Value enum (in zephyr/src/lib.rs) share a name but are different types. Will this cause confusion in docs or maintenance?

  • [nit] GPIO_OUTPUT_INACTIVE in wrapper.h is not mentioned in the PR description.

  • [nit] The Debug impl for GpioPin is orthogonal to the feature and could be a separate commit.

  • [question] The manual test plan says "Build hello_world and verify DT property access" but hello_world/src/lib.rs does not reference any RAW_* constant. What actually validates that the generated data is correct?

  • [concern] No automated test coverage for any of the new code. The to_tokens() GPIO lookahead is precisely the kind of host-side logic that benefits from unit tests with synthetic DTS data, since zephyr-build runs on the host with no hardware dependency.

Overall impression

This is a well-structured foundational PR that opens up general-purpose DT property access for Rust applications — a meaningful capability gap being filled. The core logic (GPIO detection, parent walk) is correct on close inspection, and the generated-static-data approach is sound. The main concerns are operational: the unsafe raw_new() constructor lacks safety documentation and has a latent pin-bounds issue, the GPIO detection logic is complex enough to warrant unit tests, and the PR bundles an unrelated bug fix (disabled ancestors) that should ideally be a separate commit. None of these are blockers, but the safety docs and the ancestor-walk edge case are worth addressing before merge.

Clone this wiki locally