Inconsistency on the use of the PICO_RP2350A directive

[gusmanb]

I'm implementing a new design with the RP2350B and have found an inconsistency on the usage of the PICO_RP2350A directive.

After setting the PIO base to channel 16 I found that all the GPIOs were in the incorrect offset. I used GPIO20 as the base pin with sm_config_set_in_pins, then on the PIO I read 32 bits with in pins 32, so I expected to have what is in GPIO20 present at bit 0 of the result, but it was present in bit 16.

Checking the SDK cource code I can see that on pio_set_gpio_base and pio_set_gpio_base_unsafe the only check that is made is to check if the PIO version is higher than 0, what is set to 1 for any version of the RP2350:

static int pio_set_gpio_base_unsafe(PIO pio, uint gpio_base) {
    invalid_params_if_and_return(PIO, gpio_base != 0 && (!PICO_PIO_VERSION || gpio_base != 16), PICO_ERROR_BAD_ALIGNMENT);
#if PICO_PIO_VERSION > 0
    uint32_t used_mask = _used_instruction_space[pio_get_index(pio)];
    invalid_params_if_and_return(PIO, used_mask, PICO_ERROR_INVALID_STATE);
    pio->gpiobase = gpio_base;
#else
    ((void)pio);
    ((void)gpio_base);
#endif
    return PICO_OK;
}

But then, checking functions like pio_sm_set_in_pins or sm_config_set_in_pins we see this:

static inline void pio_sm_set_in_pins(PIO pio, uint sm, uint in_base) {
    check_pio_param(pio);
    check_sm_param(sm);
#if PICO_PIO_USE_GPIO_BASE
    in_base -= pio_get_gpio_base(pio);
#endif
    valid_params_if(HARDWARE_PIO, in_base < 32);
    pio->sm[sm].pinctrl = (pio->sm[sm].pinctrl & ~PIO_SM0_PINCTRL_IN_BASE_BITS) |
                 (in_base << PIO_SM0_PINCTRL_IN_BASE_LSB);
}

Instead of checking if PICO_PIO_VERSION is greater than zero it is being checked if PICO_PIO_USE_GPIO_BASE is set, and this is derived from the definition of PICO_RP2350A.

This leads to inconsistencies, in my case I did not defined a proper board file, is a custom board and didn't knew that defining PICO_RP2350A to zero was a requirement, and as the pio_set_gpio_base in fact did changed the base pin, it made me think that it was an error in my code somewhere else and not that I would need to define something else in order to be able to use the pio base offset properly.

As my point of view, this should be consistent, if pio_set_gpio_base allows to change the base GPIO for any variant of the RP2350 it should also always check the base set when the sm functions are used, or in the other way (and I think is the proper way) if PICO_RP2350A is not set to zero then do not apply the GPIO base offset and leave it to zero, in this way all is consistent using the check in base to the PICO_RP2350A definition.

Also, (maybe this is my fault) I haven't been able to find anythin in the documentation of the SDK about this behavior, in pio_set_gpio_base is documented that setting the value does not change the side effects of this setting, but in the sm functions there is no mention to the effects of having the version defined as RP2350B or not.