Should we add pio_sm_set_pins_with_mask64 and pio_sm_set_pindirs_with_mask64? #1834

New issue

Closed

opened 2024-08-16 06:58:45 -05:00 by peterharperuk · 9 comments

peterharperuk commented

2024-08-16 06:58:45 -05:00

(Migrated from github.com)

If you try calling pio_sm_set_pins_with_mask or pio_sm_set_pindirs_with_mask with a gpio >= 32 you'll get a build error. You have to know to use pio_get_gpio_base when constructing the mask. Just wondering if it would be more obvious to add a 64bit version of these functions? We'd have to assert it was valid for the gpiobase?

kilograham commented

2024-08-16 19:10:57 -05:00

(Migrated from github.com)

ha, yes, and indeed the existing (non-mask) functions are just plain broken it seems

peterharperuk commented

2024-08-19 04:19:18 -05:00

(Migrated from github.com)

You mean pio_sm_set_pins? I hadn't noticed that. Might have to write some test code

StijnKuipers commented

2024-09-24 09:45:34 -05:00

(Migrated from github.com)

Hmm - I've just been hit with this one on a board I constructed. If any code needs to be tested I can run it here.

peterharperuk commented

2024-09-24 11:12:30 -05:00

(Migrated from github.com)

Which function are you trying to use?

StijnKuipers commented

2024-09-24 19:13:46 -05:00

(Migrated from github.com)

I was trying to get this bit of code to output, ported from a RP2040 product ->

pio_sm_config c = toggleprog_program_get_default_config(offset);
pio_set_gpio_base(pio, 16);
    
pio_gpio_init(pio, LEDCLK);
pio_gpio_init(pio, LEDDAT);
pio_gpio_init(pio, LEDLAT);

#define ADJUST(x) ((x+16)%32) 
sm_config_set_out_pin_base(&c, ADJUST(LEDDAT));
sm_config_set_set_pin_base(&c, ADJUST(LEDDAT));
pio_sm_set_consecutive_pindirs(pio, sm, ADJUST(LEDDAT), 1, true);
pio_sm_set_consecutive_pindirs(pio, sm, ADJUST(LEDCLK), 2, true);

sm_config_set_sideset(&c, 2, false, false);

sm_config_set_sideset_pins(&c, ADJUST(LEDCLK));
sm_config_set_out_pins(&c, ADJUST(LEDDAT), 1);

sm_config_set_fifo_join(&c, PIO_FIFO_JOIN_TX);
sm_config_set_clkdiv(&c, clk_div);
sm_config_set_out_shift(&c, false, true, 32);

 pio_sm_init(pio, sm, offset, &c);
 pio_sm_set_enabled(pio, sm,true);
    ```

I was trying to get this bit of code to output, ported from a RP2040 product -> ``` pio_sm_config c = toggleprog_program_get_default_config(offset); pio_set_gpio_base(pio, 16); pio_gpio_init(pio, LEDCLK); pio_gpio_init(pio, LEDDAT); pio_gpio_init(pio, LEDLAT); #define ADJUST(x) ((x+16)%32) sm_config_set_out_pin_base(&c, ADJUST(LEDDAT)); sm_config_set_set_pin_base(&c, ADJUST(LEDDAT)); pio_sm_set_consecutive_pindirs(pio, sm, ADJUST(LEDDAT), 1, true); pio_sm_set_consecutive_pindirs(pio, sm, ADJUST(LEDCLK), 2, true); sm_config_set_sideset(&c, 2, false, false); sm_config_set_sideset_pins(&c, ADJUST(LEDCLK)); sm_config_set_out_pins(&c, ADJUST(LEDDAT), 1); sm_config_set_fifo_join(&c, PIO_FIFO_JOIN_TX); sm_config_set_clkdiv(&c, clk_div); sm_config_set_out_shift(&c, false, true, 32); pio_sm_init(pio, sm, offset, &c); pio_sm_set_enabled(pio, sm,true); ```

peterharperuk commented

2024-11-19 11:38:36 -06:00

(Migrated from github.com)

@StijnKuipers Just looked at your code again. The code looks correct on develop.

The following functions handle pins >= 32, do not adjust your pin numbers when calling them.
sm_config_set_out_pin_base
pio_sm_set_consecutive_pindirs
sm_config_set_sideset_pins
sm_config_set_out_pins

@StijnKuipers Just looked at your code again. The code looks correct on develop. The following functions handle pins >= 32, do not adjust your pin numbers when calling them. sm_config_set_out_pin_base pio_sm_set_consecutive_pindirs sm_config_set_sideset_pins sm_config_set_out_pins

lurch commented

2024-11-19 19:35:46 -06:00

(Migrated from github.com)

The following functions handle pins >= 32, do not adjust your pin numbers when calling them.
sm_config_set_out_pin_base

Doxygen at https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/hardware_pio/include/hardware/pio.h#L263 says \param out_base 0-31 First pin to set as output so I guess that might need updating to avoid confusion?

Also, would you get a compile-time error if you tried doing both sm_config_set_out_pin_base(&c, 10) and sm_config_set_in_pin_base(&c, 40) ?

> The following functions handle pins >= 32, do not adjust your pin numbers when calling them. sm_config_set_out_pin_base Doxygen at https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/hardware_pio/include/hardware/pio.h#L263 says `\param out_base 0-31 First pin to set as output` so I guess that might need updating to avoid confusion? Also, would you get a compile-time error if you tried doing both `sm_config_set_out_pin_base(&c, 10)` and `sm_config_set_in_pin_base(&c, 40)` ?

kilograham commented

2024-11-21 21:40:29 -06:00

(Migrated from github.com)