Tweak check_board_header.py to also validate the PSRAM settings #2924

Merged
lurch merged 1 commit from check_psram_settings into pico-psram 2026-05-08 07:55:35 -05:00
lurch commented 2026-05-06 04:31:44 -05:00 (Migrated from github.com)

This relates to the changes in #2919 and is effectively a "sub-PR" of that one.

This relates to the changes in #2919 and is effectively a "sub-PR" of that one.
will-v-pi (Migrated from github.com) reviewed 2026-05-06 04:55:12 -05:00
@ -37,10 +37,24 @@ chip_interfaces = {
'RP2350B': "src/rp2350/rp2350b_interface_pins.json",
will-v-pi (Migrated from github.com) commented 2026-05-06 04:55:13 -05:00

There is a scenario where you could have PICO_PSRAM_CS_PIN defined but not PICO_PSRAM_SIZE_BYTES, for boards with a place to solder on PSRAM but none fitted from the factory. Can this check be made uni-directional instead?

There is a scenario where you could have `PICO_PSRAM_CS_PIN` defined but not `PICO_PSRAM_SIZE_BYTES`, for boards with a place to solder on PSRAM but none fitted from the factory. Can this check be made uni-directional instead?
will-v-pi (Migrated from github.com) reviewed 2026-05-06 05:00:25 -05:00
will-v-pi (Migrated from github.com) commented 2026-05-06 05:00:25 -05:00

I think the interface should be QSPI (it's like SPI, but Quad). QMI stands for "QSPI Memory Interface".

I think the interface should be QSPI (it's like SPI, but Quad). QMI stands for "QSPI Memory Interface".
lurch (Migrated from github.com) reviewed 2026-05-06 05:34:54 -05:00
@ -37,10 +37,24 @@ chip_interfaces = {
'RP2350B': "src/rp2350/rp2350b_interface_pins.json",
lurch (Migrated from github.com) commented 2026-05-06 05:34:54 -05:00

Ahh, I hadn't considered that. I'll update...

Ahh, I hadn't considered that. I'll update...
lurch (Migrated from github.com) reviewed 2026-05-06 05:40:52 -05:00
lurch (Migrated from github.com) commented 2026-05-06 05:40:52 -05:00

I was going along with what Table 3 (on pages 18-20) of the RP2350 datasheet says. But I now realise that this is actually the CS1N pin of the (only) QMI interface, and not the CSN pin of the second QMI interface 🤦 so I'll fix that.

I was going along with what Table 3 (on pages 18-20) of the RP2350 datasheet says. But I now realise that this is actually the `CS1N` pin of the (only) QMI interface, and not the `CSN` pin of the second QMI interface :facepalm: so I'll fix that.
lurch (Migrated from github.com) reviewed 2026-05-06 06:50:58 -05:00
@ -37,10 +37,24 @@ chip_interfaces = {
'RP2350B': "src/rp2350/rp2350b_interface_pins.json",
lurch (Migrated from github.com) commented 2026-05-06 06:50:57 -05:00

Fixed; you can now define PICO_PSRAM_CS_PIN without defining PICO_PSRAM_SIZE_BYTES, but if you define PICO_PSRAM_SIZE_BYTES then you have to also define PICO_PSRAM_CS_PIN.

Fixed; you can now define `PICO_PSRAM_CS_PIN` without defining `PICO_PSRAM_SIZE_BYTES`, but if you define `PICO_PSRAM_SIZE_BYTES` then you have to also define `PICO_PSRAM_CS_PIN`.
lurch commented 2026-05-06 06:51:34 -05:00 (Migrated from github.com)

PR updated, rebased and force-pushed.

PR updated, rebased and force-pushed.
lurch commented 2026-05-06 07:39:01 -05:00 (Migrated from github.com)

Looking at https://github.com/raspberrypi/pico-sdk/pull/2919/commits/2afd108fc8b44090e1edd38f25f5f43ef1dbcc8f , should I also update this to error if PICO_AUTO_DETECT_PSRAM_SIZE is set and PICO_PSRAM_CS_PIN is missing?

Looking at https://github.com/raspberrypi/pico-sdk/pull/2919/commits/2afd108fc8b44090e1edd38f25f5f43ef1dbcc8f , should I also update this to error if `PICO_AUTO_DETECT_PSRAM_SIZE` is set and `PICO_PSRAM_CS_PIN` is missing?
lurch commented 2026-05-06 07:43:05 -05:00 (Migrated from github.com)

...although saying that, perhaps it makes sense to check for exists(PICO_PSRAM_CS_PIN) or exists(PICO_AUTO_DETECT_PSRAM_CS) rather than just exists(PICO_PSRAM_CS_PIN)? 🤔

...although saying that, _perhaps_ it makes sense to check for `exists(PICO_PSRAM_CS_PIN) or exists(PICO_AUTO_DETECT_PSRAM_CS)` rather than just `exists(PICO_PSRAM_CS_PIN)`? :thinking:
will-v-pi commented 2026-05-06 07:56:21 -05:00 (Migrated from github.com)

I don't think there's a use case for PICO_AUTO_DETECT_PSRAM_CS in a board header, as if you know what board you're building for, you should know what pin CS is wired to - so I think just adding an error when PICO_AUTO_DETECT_PSRAM_SIZE is set and PICO_PSRAM_CS_PIN is missing is fine

I don't think there's a use case for `PICO_AUTO_DETECT_PSRAM_CS` in a board header, as if you know what board you're building for, you should know what pin CS is wired to - so I think just adding an error when `PICO_AUTO_DETECT_PSRAM_SIZE` is set and `PICO_PSRAM_CS_PIN` is missing is fine
lurch commented 2026-05-06 08:15:34 -05:00 (Migrated from github.com)

so I think just adding an error when PICO_AUTO_DETECT_PSRAM_SIZE is set and PICO_PSRAM_CS_PIN is missing is fine

Done 👍

> so I think just adding an error when `PICO_AUTO_DETECT_PSRAM_SIZE` is set and `PICO_PSRAM_CS_PIN` is missing is fine Done :+1:
will-v-pi (Migrated from github.com) approved these changes 2026-05-08 07:55:00 -05:00
Sign in to join this conversation.
No reviewers
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!2924
No description provided.