Skip to content

pio_encoding fixes + added PIO_VERSION to platform_defs.h#2958

Open
kilograham wants to merge 3 commits into
developfrom
pio-encode
Open

pio_encoding fixes + added PIO_VERSION to platform_defs.h#2958
kilograham wants to merge 3 commits into
developfrom
pio-encode

Conversation

@kilograham
Copy link
Copy Markdown
Contributor

@kilograham kilograham commented May 19, 2026

  • pio_encode_mov(pio_pindirs) was wrong because the mov destination for pindirs is a different value
  • Noticed that pio_instructions.h is included before PICO_PIO_VERSION is set; didn't want to make a circular include, and previously pio_instructions.h is somewhat standalone include-able, so decided to add PIO_VERSION to platform_defs.h... TBH it should have been there already as the init of PICO_PIO_VERSION was a bit of a hack. I doubt anyone wants to override PICO_PIO_VERSION anyway
  • I saw that we have pio_exec_mov and pio_exec_out - kept these for backwards compatibility, but decided to just allow pio_exec and have pio_encode_mov take care of it. as i imagine most of uses of pio_encode_mov take a constant arg, this isn't much of an overhead (the alternative would have been to encode the two different values in the enum value, which is fine, but likely going to lead to more code if done at runtime vs just the ifs)

@kilograham
Copy link
Copy Markdown
Contributor Author

fixes #2754

@lurch
Copy link
Copy Markdown
Contributor

lurch commented May 20, 2026

I doubt anyone wants to override PICO_PIO_VERSION anyway

Maybe it's worth removing the PICO_CONFIG: PICO_PIO_VERSION line? 🤔

Comment thread src/rp2_common/hardware_pio/include/hardware/pio_instructions.h Outdated
Comment thread src/rp2_common/hardware_pio/include/hardware/pio_instructions.h
Comment thread src/rp2_common/hardware_pio/include/hardware/pio_instructions.h Outdated
Co-authored-by: Andrew Scheller <lurch@durge.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants