There are no functions in the sdk to operate QSPI-related io. #1799

New issue

Open

opened 2024-08-11 01:09:59 -05:00 by fan2nd · 4 comments

fan2nd commented

2024-08-11 01:09:59 -05:00

(Migrated from github.com)

I want rp2040 to run in NO_FLASH mode and use qspi-io as normal io, but the relevant functions are not provided in the sdk.

👍 1

lurch commented

2025-02-17 11:25:51 -06:00

(Migrated from github.com)

It was written for RP2350 rather than RP2040, but according to the comments in #2280 , https://github.com/raspberrypi/pico-examples/pull/571 might have some good pointers on how to do GPIO over the QSPI pins?

lifeisafractal commented

2025-08-20 12:20:43 -05:00

(Migrated from github.com)

Statement of problem

The PAD and IO banks for QSPI pins are in different orders.
Mapping QSPI pins to a number is confusing as there is no conical order for SPI pins, unlike the normal GPIO bank which is intrinsically referred to by number.
The QSPI IO bank has entries for the USB DP/DM pins at the front of the register bank which doesn't have a counterpart in the PAD bank.

Issue 3 above is easy enough to handle as the io_qspi_hw struct for the reg map already handles the DP/DM pins individually, and starts the actual gpio[] array at the start of the QSPI pins. It's a bit curious the silicon designers didn't put this at the end of the array, but overall it's a pretty moot and easy to handle issue.

Issues 1 and 2 are pretty related and gets at the crux of how to make this API both clear to use and type safe. The current gpio_set_function function takes a gpio number which is plenty clear for the user GPIOs, but this breaks down with the secondary QSPI bank, and is compounded by the fact that the QSPI pin numbering is anything but clear given the issues outlined above.

Goals in an API

I'm honestly not sure the best way to tackle this, but there are a couple goals I'd want to see to have a nice API here:

Make the calls to the API to setup the QSPI pins clear as to what QSPI is being setup
Encapsulate the confusion about IO and PAD QSPI banks being in different orders as an opaque implementation detail.
Make it (at least somewhat) type safe so users aren't accidentally passing incorrect numbers for the QSPI pin

Possible solution

Here are a couple possible solutions, and I'd love to hear others. I think it's worth bouncing ideas before just going and implementing something. Below is a bit of stream of consciousness on my thoughts...

I see two high level opens:

Reuse and extend the existing gpio_set_function() and other gpio functions
Create a second gpio_qspi_* set of functions

Reuse and extend the existing `gpio_set_function()` and other gpio functions

For this approach we'd need to somehow allow these functions to understand QSPI pins in addition to the current numbered GPIOs in the normal bank. One approach would be to give the QSPI pins 'virtual' numbers. For example:
c #define GPIO_QSPI_NUM_SS 1001 #define GPIO_QSPI_NUM_CLK 1002 #define GPIO_QSPI_NUM_SD1 1003 // ... and so on
We can update check_gpio_param() and other internal functions to flag these offset QSPI gpio numbers as valid. Internally we can then have a function that returns the proper IO or PAD register for a given 'virtual' GPIO number.

Another option in that vein, but more invasive, would be to change these functions to take an enum explicitly rather than a uint to say which GPIO to act on the GPIO. We can keep the same numbering scheme so it falls back and works with normal GPIO, but it would be clear where to look (the enum) for specifying the QSPI pins.

I think I lean this way (reuse the current APIs, and change to taking an enum for the GPIO). The enum approach should be backwards compatible with code as passing number literals or uints will get coerced into the enum value making this an API compatible change.

New `gpio_qspi_*` functions

This gets us out of needing to make larger changes to the existing API, but results in highly duplicated code as the register layout of QSPI and normal GPIO banks are the same. This further has the issue that if these functions take a uint for the QSPI pin, we have to resolve the ambiguity. Even if we have defines for these, taking a unit does nothing for type safety, and doesn't signal to the user where to look for the defines. I'd lean to having an enum still in this case, but even then it's not super type safe and might still lead to confusion.

I'm less positive on this approach due to it not really uniquely solving the ordering issues, and also resulting is mass code duplication.

I opened a PR in pico-example (raspberrypi/pico-examples#697) to fix the UART bootloader example when it is setting the IO pins up for UART. I'd love to see an API in the SDK for this, but there are some confusing issues to tackle before making a clean and safe API for this. # Statement of problem 1. The PAD and IO banks for QSPI pins are in different orders. 2. Mapping QSPI pins to a number is confusing as there is no conical order for SPI pins, unlike the normal GPIO bank which is intrinsically referred to by number. 3. The QSPI IO bank has entries for the USB DP/DM pins at the front of the register bank which doesn't have a counterpart in the PAD bank. Issue 3 above is easy enough to handle as the `io_qspi_hw` struct for the reg map already handles the DP/DM pins individually, and starts the actual `gpio[]` array at the start of the QSPI pins. It's a bit curious the silicon designers didn't put this at the end of the array, but overall it's a pretty moot and easy to handle issue. Issues 1 and 2 are pretty related and gets at the crux of how to make this API both clear to use and type safe. The current [`gpio_set_function`](https://github.com/raspberrypi/pico-sdk/blob/a1438dff1d38bd9c65dbd693f0e5db4b9ae91779/src/rp2_common/hardware_gpio/include/hardware/gpio.h#L248-L259) function takes a gpio number which is plenty clear for the user GPIOs, but this breaks down with the secondary QSPI bank, and is compounded by the fact that the QSPI pin numbering is anything but clear given the issues outlined above. # Goals in an API I'm honestly not sure the best way to tackle this, but there are a couple goals I'd want to see to have a nice API here: 1. Make the calls to the API to setup the QSPI pins clear as to what QSPI is being setup 2. Encapsulate the confusion about IO and PAD QSPI banks being in different orders as an opaque implementation detail. 3. Make it (at least somewhat) type safe so users aren't accidentally passing incorrect numbers for the QSPI pin # Possible solution Here are a couple possible solutions, and I'd love to hear others. I think it's worth bouncing ideas before just going and implementing something. Below is a bit of stream of consciousness on my thoughts... I see two high level opens: 1. Reuse and extend the existing `gpio_set_function()` and other gpio functions 2. Create a second `gpio_qspi_*` set of functions ## Reuse and extend the existing `gpio_set_function()` and other gpio functions For this approach we'd need to somehow allow these functions to understand QSPI pins in addition to the current numbered GPIOs in the normal bank. One approach would be to give the QSPI pins 'virtual' numbers. For example: ```c #define GPIO_QSPI_NUM_SS 1001 #define GPIO_QSPI_NUM_CLK 1002 #define GPIO_QSPI_NUM_SD1 1003 // ... and so on ``` We can update `check_gpio_param()` and other internal functions to flag these offset QSPI gpio numbers as valid. Internally we can then have a function that returns the proper IO or PAD register for a given 'virtual' GPIO number. Another option in that vein, but more invasive, would be to change these functions to take an enum explicitly rather than a uint to say which GPIO to act on the GPIO. We can keep the same numbering scheme so it falls back and works with normal GPIO, but it would be clear where to look (the enum) for specifying the QSPI pins. I think I lean this way (reuse the current APIs, and change to taking an enum for the GPIO). The enum approach should be backwards compatible with code as passing number literals or uints will get coerced into the enum value making this an API compatible change. ## New `gpio_qspi_*` functions This gets us out of needing to make larger changes to the existing API, but results in highly duplicated code as the register layout of QSPI and normal GPIO banks are the same. This further has the issue that if these functions take a uint for the QSPI pin, we have to resolve the ambiguity. Even if we have defines for these, taking a unit does nothing for type safety, and doesn't signal to the user where to look for the defines. I'd lean to having an enum still in this case, but even then it's not super type safe and might still lead to confusion. I'm less positive on this approach due to it not really uniquely solving the ordering issues, and also resulting is mass code duplication.

lurch commented

2025-08-20 13:24:19 -05:00

(Migrated from github.com)

Both approaches sound equally plausible to me, but this'll be a decision for @kilograham to make.

Note that there are many places in pico-sdk where "mass code duplication" has been avoided, by having multiple wrapper functions (pico_foo_xyz(), pico_bar_xyz()) which all call a shared lower-level function (pico_xyz_internal()).

Both approaches sound equally plausible to me, but this'll be a decision for @kilograham to make. Note that there are many places in pico-sdk where "mass code duplication" has been avoided, by having multiple wrapper functions (`pico_foo_xyz()`, `pico_bar_xyz()`) which all call a shared lower-level function (`pico_xyz_internal()`).

lifeisafractal commented

2025-08-20 13:57:03 -05:00

(Migrated from github.com)

Great, thanks @lurch for the input and for summing @kilograham. Looking forward to getting agreement on an approach that fits well with the style of the current SDK. Once we get that, I think I have time in the coming weeks to take a stab at this if that helps.