Repository navigation
Feature/first implementation - #1
Closed
JohnAmadis wants to merge 4 commits into
Closed
JohnAmadis wants to merge 4 commits into
JohnAmadis wants to merge 4 commits into
Conversation
Testing on real STM32F746G-DISCO hardware found two bugs: - The cs_path device path format was wrong: dmdevfs mounts alt-named devices under /dev/dmgpio<port_index>/<name>, not a flat /dev/<name>. Fixed in all 8 board configs. - More fundamentally, dmspi_dmdrvi_create() was opening cs_path eagerly, which fails whenever dmspi's own config is part of dmdevfs's initial boot-time scan (the normal case): dmdevfs's /dev mount isn't marked "ready" until that whole scan finishes, so Dmod_FileOpen() on any other /dev path fails during that window even when the path is correct. Fix the path alone wasn't enough - confirmed the same path opens fine from an interactive shell after boot, but not from within the boot scan itself. Fixed by opening cs_path lazily on first transfer instead of at create time, which sidesteps the ordering problem entirely. Verified end-to-end on hardware: dmspi now creates successfully, the device appears under /dev, spitest opens it and queries its role, and a transfer correctly asserts/deasserts the CS pin (confirmed via readback) before timing out cleanly (bounded, no hang) with nothing wired to MISO. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The cs_path design - Dmod_FileOpen() on a dmdevfs /dev path, opened
lazily, driven via hardcoded dmgpio ioctl constants - had three real
problems: the /dev/dmgpio<port_index>/<name> path format is an
implementation detail the caller had to compute by hand (got it wrong
twice while writing the board configs); the ioctl constants were a
manually-copied guess at dmgpio's ABI; and the lazy-open only existed
to work around dmdevfs's /dev mount not being "ready" during its own
boot-time config scan, so a broken CS pin only surfaced on first
transfer, not at config time.
dmgpio now exposes a pin-lease API (dmgpio_lease.h) built for exactly
this: dmgpio_pin_acquire/_release/_write, a direct Built-in API
dependency resolved by the loader at module-load time, not a runtime
/dev lookup, with real collision detection (dmgpio_port_claim_pins).
cs_pin names the pin directly ("PE3", the same syntax dmgpio's own
pin= already uses) and dmspi claims it eagerly in _create() - a bad or
already-claimed pin now fails at config time instead of silently on
first transfer. The lease also configures the pin itself, so the
separate [xxx_cs] dmgpio board-config section is gone entirely.
Requires a dmgpio release with the pin-lease API and the packaging fix
in choco-technologies/dmgpio (pushed to master there); dmod_link_modules
here will not resolve dmgpio's headers until that release exists.
Verified locally in the meantime by building against the fixed dmgpio
checkout directly (stm32f4, stm32f7, and the x86_64 "core" build all
compile and link cleanly).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Other drivers (flash, sensor, display, ...) will typically be created during the same dmdevfs boot-time config scan dmspi itself runs in, so opening /dev/dmspiN from inside their own _create() hits the exact ordering problem cs_pin used to have before it moved off /dev: the /dev mount isn't marked ready yet during that scan. dmspi_acquire()/_release()/_transfer() are a direct Built-in API (dmod_link_modules(<you> dmspi)), resolved by the loader at module-load time rather than a /dev lookup, so they work regardless of mount readiness - the same fix dmgpio_lease.h already gives dmspi itself for cs_pin. dmspi_acquire() looks up an already-created instance by number via a small static table populated in _create()/cleared in _free(); dmspi_transfer() applies the same cs_pin handling _write/_read/ dmspi_ioctl_cmd_transfer already do via /dev. Since more than one driver can now reach the same instance concurrently (leased or via /dev), added a dmosi mutex around the actual bus access in all three paths (_read, _write, and a new locked_transfer() shared by _ioctl's transfer command and dmspi_transfer()) - this was an unguarded race before, just a narrower one since /dev was the only access path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.