SDK 2.3.0 - pio_encoding still broken #3068

Closed
opened 2026-07-09 16:06:59 -05:00 by gmx-pico · 4 comments
gmx-pico commented 2026-07-09 16:06:59 -05:00 (Migrated from github.com)

Following this PR #2958 pio_encoding fixes + added PIO_VERSION to platform_defs.h
it fixes #2946 hardware_pio: fix pio_encode_mov(pio_pindirs, ...) on RP2350
but it breaks other opcode encodings.

A simple test:

  printf( "mov osr,x          0x%4X (should be 0xA0E1) pio_osr=0x%1X\n", pio_encode_mov(pio_osr, pio_x), pio_osr );
  printf( "mov pindirs,x      0x%4X (should be 0xA061) pio_pindirs=0x%1X\n", pio_encode_mov(pio_pindirs, pio_x), pio_pindirs );
  printf( "mov pindirs_mov,x  0x%4X (should be 0xA061) pio_pindirs_mov=0x%1X\n", pio_encode_mov(pio_pindirs_mov, pio_x), pio_pindirs_mov );
  printf( "mov exec,x         0x%4X (should be 0xA081) pio_exec=0x%1X\n", pio_encode_mov(pio_exec, pio_x), pio_exec );
  printf( "mov exec_mov,x     0x%4X (should be 0xA081) pio_exec_mov=0x%1X\n", pio_encode_mov(pio_exec_mov, pio_x), pio_exec_mov );
  printf( "mov y,x            0x%4X (should be 0xA041) pio_y=0x%1X\n", pio_encode_mov(pio_y, pio_x), pio_y );

RP2350 SDK 2.3.0 results:

mov osr,x          0xA061 (should be 0xA0E1) pio_osr=0x7          << WRONG
mov pindirs,x      0xA061 (should be 0xA061) pio_pindirs=0x4      << OK
mov pindirs_mov,x  0xA061 (should be 0xA061) pio_pindirs_mov=0x3  << OK
mov exec,x         0xA061 (should be 0xA081) pio_exec=0x7         << WRONG
mov exec_mov,x     0xA061 (should be 0xA081) pio_exec_mov=0x4     << WRONG
mov y,x            0xA041 (should be 0xA041) pio_y=0x2            << OK

For comparison SDK 2.2.0

mov osr,x          0xA0E1 (should be 0xA0E1) pio_osr=0x7          << OK
mov pindirs,x      0xA081 (should be 0xA061) pio_pindirs=0x4      << WRONG
mov exec_mov,x     0xA081 (should be 0xA081) pio_exec_mov=0x4     << OK
mov y,x            0xA041 (should be 0xA041) pio_y=0x2            << OK

There are reports on forum that RP2040 is affected too: https://forums.raspberrypi.com/viewtopic.php?t=399120#p2380376
pio_encode_mov(pio_osr, pio_x) is actually generating mov exec,x

I think the problem arises from overlapping enum fields across different instructions, maybe separate enums (per instruction type) would be easier to manage.

Following this PR #2958 **pio_encoding fixes + added PIO_VERSION to platform_defs.h** it fixes #2946 **hardware_pio: fix pio_encode_mov(pio_pindirs, ...) on RP2350** but it breaks other opcode encodings. A simple test: ``` c printf( "mov osr,x 0x%4X (should be 0xA0E1) pio_osr=0x%1X\n", pio_encode_mov(pio_osr, pio_x), pio_osr ); printf( "mov pindirs,x 0x%4X (should be 0xA061) pio_pindirs=0x%1X\n", pio_encode_mov(pio_pindirs, pio_x), pio_pindirs ); printf( "mov pindirs_mov,x 0x%4X (should be 0xA061) pio_pindirs_mov=0x%1X\n", pio_encode_mov(pio_pindirs_mov, pio_x), pio_pindirs_mov ); printf( "mov exec,x 0x%4X (should be 0xA081) pio_exec=0x%1X\n", pio_encode_mov(pio_exec, pio_x), pio_exec ); printf( "mov exec_mov,x 0x%4X (should be 0xA081) pio_exec_mov=0x%1X\n", pio_encode_mov(pio_exec_mov, pio_x), pio_exec_mov ); printf( "mov y,x 0x%4X (should be 0xA041) pio_y=0x%1X\n", pio_encode_mov(pio_y, pio_x), pio_y ); ``` RP2350 SDK 2.3.0 results: ``` mov osr,x 0xA061 (should be 0xA0E1) pio_osr=0x7 << WRONG mov pindirs,x 0xA061 (should be 0xA061) pio_pindirs=0x4 << OK mov pindirs_mov,x 0xA061 (should be 0xA061) pio_pindirs_mov=0x3 << OK mov exec,x 0xA061 (should be 0xA081) pio_exec=0x7 << WRONG mov exec_mov,x 0xA061 (should be 0xA081) pio_exec_mov=0x4 << WRONG mov y,x 0xA041 (should be 0xA041) pio_y=0x2 << OK ``` For comparison SDK 2.2.0 ``` mov osr,x 0xA0E1 (should be 0xA0E1) pio_osr=0x7 << OK mov pindirs,x 0xA081 (should be 0xA061) pio_pindirs=0x4 << WRONG mov exec_mov,x 0xA081 (should be 0xA081) pio_exec_mov=0x4 << OK mov y,x 0xA041 (should be 0xA041) pio_y=0x2 << OK ``` There are reports on forum that RP2040 is affected too: https://forums.raspberrypi.com/viewtopic.php?t=399120#p2380376 _pio_encode_mov(pio_osr, pio_x)_ is actually generating _mov exec,x_ I think the problem arises from overlapping enum fields across different instructions, maybe separate enums (per instruction type) would be easier to manage.
lurch commented 2026-07-12 06:14:14 -05:00 (Migrated from github.com)

I did a bit of investigation, and surprisingly it works fine in Debug mode (i.e. when #ifndef NDEBUG).

I did a bit more investigation, and tracked down that in Debug mode pio_osr and pio_exec have different integer values, because of the invalid parameter-checking stuff; and so everything works fine. But in Release mode (i.e. when NDEBUG is defined) pio_osr and pio_exec both have the integer value 0x7, and this means that the if (dest == pio_exec) dest = pio_exec_mov; line in pio_encode_mov() is incorrectly translating a pio_osr into a pio_exec_mov.

EDIT: Similarly (when NDEBUG is defined), pio_pindirs and pio_exec_mov both have the integer value 0x4 which means that the if (dest == pio_pindirs) dest = pio_pindirs_mov; line in pio_encode_mov() is incorrectly translating a pio_exec_mov into a pio_pindirs_mov.
And the combination of

    if (dest == pio_exec) dest = pio_exec_mov;
    if (dest == pio_pindirs) dest = pio_pindirs_mov;

means that a pio_exec gets translated intto a pio_exec_mov, and this then gets incorrectly translated into into a pio_pindirs_mov.
(Which in turn also means that pio_osr is getting translated to pio_pindirs_mov, which explains why you're getting three identical "WRONG" encodings in your report above.)

I did a bit of investigation, and surprisingly it works fine in Debug mode (i.e. when [`#ifndef NDEBUG`](https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/hardware_pio/include/hardware/pio_instructions.h#L44)). I did a bit more investigation, and tracked down that in Debug mode `pio_osr` and `pio_exec` have _different_ integer values, because of the [invalid parameter-checking stuff](https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/hardware_pio/include/hardware/pio_instructions.h#L64); and so everything works fine. But in Release mode (i.e. when `NDEBUG` is defined) `pio_osr` and `pio_exec` **both** have the integer value `0x7`, and this means that the `if (dest == pio_exec) dest = pio_exec_mov;` line in [`pio_encode_mov()`](https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/hardware_pio/include/hardware/pio_instructions.h#L407) is incorrectly translating a `pio_osr` into a `pio_exec_mov`. EDIT: Similarly (when `NDEBUG` is defined), `pio_pindirs` and `pio_exec_mov` **both** have the integer value `0x4` which means that the `if (dest == pio_pindirs) dest = pio_pindirs_mov;` line in `pio_encode_mov()` is incorrectly translating a `pio_exec_mov` into a `pio_pindirs_mov`. And the combination of ```c if (dest == pio_exec) dest = pio_exec_mov; if (dest == pio_pindirs) dest = pio_pindirs_mov; ``` means that a `pio_exec` gets translated intto a `pio_exec_mov`, and this then gets incorrectly translated into into a `pio_pindirs_mov`. (Which in turn also means that `pio_osr` is getting translated to `pio_pindirs_mov`, which explains why you're getting three identical "WRONG" encodings in your report above.)
kilograham commented 2026-07-14 17:30:40 -05:00 (Migrated from github.com)

ack - i guess a testcase is in order!

ack - i guess a testcase is in order!
lurch commented 2026-08-12 03:25:16 -05:00 (Migrated from github.com)

Fix merged into develop

Fix merged into `develop`
gmx-pico commented 2026-08-12 18:07:38 -05:00 (Migrated from github.com)

Great, many thanks!

Great, many thanks!
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk#3068
No description provided.