Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PIO functions. #3857

Merged
merged 2 commits into from
Feb 7, 2025
Merged

Add PIO functions. #3857

merged 2 commits into from
Feb 7, 2025

Conversation

swork
Copy link
Contributor

@swork swork commented Feb 7, 2025

Please consider a PR for embassy-rp to add some PIO functions that look to me to be missing:

ConfigPins::set_sideset_pins(), needed like set_set_pins and set_out_pins. (Is there a reason this isn't there that I haven't found yet?)

Several runtime StateMachine manipulations:

  • addr() of running SM, only useful at a stall point but important for that reason
  • tx_threshold() (user code could just save the last-set value I suppose)
  • set_tx_threshold - I've found several PIO programs "out there" that need to manipulate thresholds.
  • rx_threshold()
  • set_rx_threshold()
  • set_thresholds() - both at once, same value

Please review with extreme prejudice!

Add some (I think) needed functions:
ConfigPins::set_sideset_pins (the other pin types are covered, why not this one?)
Several runtime StateMachine manipulations:
 - addr()
 - tx_threshold()
 - set_tx_threshold
 - rx_threshold()
 - set_rx_threshold()
 - set_thresholds() - both at once, same value
@CBJamo
Copy link
Contributor

CBJamo commented Feb 7, 2025

The sideset pins are configured in the use_program function. Is there some reason this would need to be done at runtime?

The addr, tx_threshold, and rx_threshold functions should probably be prefixed with get_ to match the other functions.

…ing.

The tweak arranges that "grep sideset" finds use_program() when grokking source - this spelling is used
elsewhere, as in PinConfig for example, and I managed to miss use_program.
@swork
Copy link
Contributor Author

swork commented Feb 7, 2025

Thanks CBJamo. No, I haven't found a reason to change sideset at runtime, just hadn't found it in use_program() so couldn't sort out how to get it into the config. I removed a dash from one instance of the term "side-set" in use_program's docstring, so the spelling "sideset" occurs there as it does elsewhere, like in PinConfig. Again, I'd managed to miss this, though it's certainly prominent enough otherwise. And I added "get_" to the start of the added getters as you suggested.

@Dirbaio Dirbaio added this pull request to the merge queue Feb 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 7, 2025
@Dirbaio Dirbaio merged commit 3b734a7 into embassy-rs:main Feb 7, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants