hardware_pio: clarify pio_sm_restart docs (PC, running state)#2945
Open
anoshyn wants to merge 1 commit into
Open
hardware_pio: clarify pio_sm_restart docs (PC, running state)#2945anoshyn wants to merge 1 commit into
anoshyn wants to merge 1 commit into
Conversation
…ts cleared) The previous doxygen claimed pio_sm_restart() clears "the ISR, shift counters, clock divider counter, pin write flags, delay counter, latched EXEC instruction, and IRQ wait condition". That list mixed up a few things and was missing the points users actually trip over (per raspberrypi#2844): - The clock divider counter is NOT cleared by SM_RESTART; that requires CLKDIV_RESTART (pio_sm_clkdiv_restart). - "ISR" was ambiguous (the input shift register vs interrupt service routine); spell it out. - The function name ends in "_restart" but the SM's enable/running state is not affected; SM_RESTART only clears internal bookkeeping. - The program counter is not affected either (the issue's main ask). Rewrite both pio_sm_restart and pio_restart_sm_mask doxygen to track the authoritative wording from the RP2040/RP2350 PIO_CTRL.SM_RESTART field descriptions: list exactly what is cleared (input/output shift counters, input shift register contents, delay counter, waiting-on-IRQ state, stalled SMx_INSTR / OUT|MOV EXEC instruction, OUT_STICKY pin writes) and exactly what is preserved (enable state, PC, output shift register, X/Y scratch). Point readers at pio_sm_clkdiv_restart and pio_sm_exec for the operations they may have expected this function to perform. Also fix the @param description on pio_restart_sm_mask, which previously said "modify the enabled state of" - that function doesn't touch the enable bits. Doxygen build verified locally: no new warnings, cross-references to pio_sm_set_enabled / pio_sm_clkdiv_restart / pio_sm_exec resolve, and the rendered HTML reads cleanly. Fixes raspberrypi#2844.
anoshyn
force-pushed
the
fix-pio-sm-restart-doc
branch
from
2a9a996 to
392cd87
Compare
Author
|
Force-pushed to drop the |
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
2 participants
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.
Summary
Per #2844, the doxygen for
pio_sm_restart()doesn't mention the program counter or the state machine's enable/running state, and includes a couple of inaccuracies.This PR rewrites the doxygen for
pio_sm_restartand its multi-SM siblingpio_restart_sm_maskagainst the authoritative wording from thePIO_CTRL.SM_RESTARTfield description inhardware/regs/pio.h(same text on both RP2040 and RP2350):Fixed inaccuracies:
SM_RESTART— that's a separate bit (CLKDIV_RESTART, exposed viapio_sm_clkdiv_restart). The old doc claimed it was cleared.@param maskdescription onpio_restart_sm_masksaid "modify the enabled state of", but the function does not touchSM_ENABLE.Filled in:
_restartsuffix is misleading without this).pio_sm_execfor the "set PC after restart" use case.PC behavior is documented as "not affected" on RP2040 in the datasheet; the RP2350 datasheet's SM_RESTART description omits the PC from its not-affected list, but the underlying hardware bit is identical and the rest of the description matches. Happy to soften the wording if maintainers know of a behavior difference on RP2350.
Fixes #2844.
Test plan
cmake --build build --target docsproduces no new warnings; specifically nothing on the changed block insrc/rp2_common/hardware_pio/include/hardware/pio.h.pio_sm_set_enabled,pio_sm_clkdiv_restart,pio_sm_execresolve; backtick code spans render as<code>.cmake --build build); no code change.