Standardise usage of rom_get_sys_info function #3094

2026-07-22T10:28:36-05:00

lurch commented

2026-07-22 10:28:36 -05:00

(Migrated from github.com)

I copied the rom_get_sys_info usage-pattern from bootrom.h.
RP2350 Datasheet says "You should always check (the first word in the returned buffer) before interpreting the buffer.", so do that too.
Also some small unique_id tidy-ups.

I copied the `rom_get_sys_info` usage-pattern from [`bootrom.h`](https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_bootrom/include/pico/bootrom.h#L1122). RP2350 Datasheet says "You should always check (the first word in the returned buffer) before interpreting the buffer.", so do that too. Also some small unique_id tidy-ups.

peterharperuk (Migrated from github.com) reviewed

2026-07-22 10:28:36 -05:00

will-v-pi (Migrated from github.com) reviewed

2026-07-23 11:16:27 -05:00

src/rp2_common/pico_bootrom/bootrom.c

					
				@ -85,9 +85,9 @@ void __attribute__((noreturn)) rom_reset_usb_boot_extra(int usb_activity_gpio_pi

				#if !PICO_RP2040

I think using count_of might be clearer - and then the uses in bootrom.h could be updated to count_of too

    int words_returned = rom_get_sys_info(result, count_of(result), SYS_INFO_BOOT_RANDOM);

I think using `count_of` might be clearer - and then the uses in `bootrom.h` could be updated to `count_of` too ```suggestion int words_returned = rom_get_sys_info(result, count_of(result), SYS_INFO_BOOT_RANDOM); ```

src/rp2_common/pico_unique_id/unique_id.c

					
				@ -38,16 +43,22 @@ static void __attribute__((PICO_UNIQUE_BOARD_ID_INIT_ATTRIBUTES)) _retrieve_uniq

				        #error unique board ID size is greater than flash unique ID size

Ditto on count_of

        int words_returned = rom_get_sys_info(out.words, count_of(out.words), SYS_INFO_CHIP_INFO);

Ditto on `count_of` ```suggestion int words_returned = rom_get_sys_info(out.words, count_of(out.words), SYS_INFO_CHIP_INFO); ```

lurch (Migrated from github.com) reviewed

2026-07-23 11:54:18 -05:00

src/rp2_common/pico_bootrom/bootrom.c

					
				@ -85,9 +85,9 @@ void __attribute__((noreturn)) rom_reset_usb_boot_extra(int usb_activity_gpio_pi

				#if !PICO_RP2040

Okay, I can change all of these. Is it still appropriate for count_of to continue being used in the words_returned == count_of(result) check on the next line too? This would mean that the number 5 only appears in the uint32_t result[5] line. (Which would be fine with me, I'm always happy to see the elimination of duplicated magic-numbers; I'm just double-checking that this is what's desired.)

And in that case I guess it might make sense to fold #3089 into this PR too.

Okay, I can change all of these. Is it still appropriate for `count_of` to continue being used in the `words_returned == count_of(result)` check on the next line too? This would mean that the number `5` _only_ appears in the `uint32_t result[5]` line. (Which would be fine with me, I'm always happy to see the elimination of duplicated magic-numbers; I'm just double-checking that this is what's desired.) And in that case I guess it might make sense to fold #3089 into this PR too.

will-v-pi (Migrated from github.com) reviewed

2026-07-23 12:00:11 -05:00

src/rp2_common/pico_bootrom/bootrom.c

					
				@ -85,9 +85,9 @@ void __attribute__((noreturn)) rom_reset_usb_boot_extra(int usb_activity_gpio_pi

				#if !PICO_RP2040

I think it's still appropriate - I also agree it's nice to remove the magic numbers.

Re folding the PRs, I don't mind - you could make the changes to use count_of in the existing bootrom.h functions in that PR, or fold it into this one and do it here

I think it's still appropriate - I also agree it's nice to remove the magic numbers. Re folding the PRs, I don't mind - you could make the changes to use `count_of` in the existing `bootrom.h` functions in that PR, or fold it into this one and do it here

lurch (Migrated from github.com) reviewed

2026-07-23 12:02:03 -05:00

src/rp2_common/pico_bootrom/bootrom.c

					
				@ -85,9 +85,9 @@ void __attribute__((noreturn)) rom_reset_usb_boot_extra(int usb_activity_gpio_pi

				#if !PICO_RP2040

Taking that idea of removing magic-numbers even further, perhaps it would also make sense to add e.g.

#define SYS_INFO_BOOT_RANDOM_WORDS_RETURNED     _u(4)

to bootrom_constants.h, and then we could do

uint32_t result[SYS_INFO_BOOT_RANDOM_WORDS_RETURNED + 1];

?

Taking that idea of removing magic-numbers even further, _perhaps_ it would also make sense to add e.g. ```c #define SYS_INFO_BOOT_RANDOM_WORDS_RETURNED _u(4) ``` to `bootrom_constants.h`, and then we could do ```c uint32_t result[SYS_INFO_BOOT_RANDOM_WORDS_RETURNED + 1]; ``` ?

will-v-pi (Migrated from github.com) reviewed

2026-07-23 12:06:54 -05:00

src/rp2_common/pico_bootrom/bootrom.c

					
				@ -85,9 +85,9 @@ void __attribute__((noreturn)) rom_reset_usb_boot_extra(int usb_activity_gpio_pi

				#if !PICO_RP2040

Yeah, that sounds good too - for all the SYS_INFO_XXX defines having a corresponding SYS_INFO_XXX_WORDS_RETURNED rather than just having it in the comment above.

Yeah, that sounds good too - for all the `SYS_INFO_XXX` defines having a corresponding `SYS_INFO_XXX_WORDS_RETURNED` rather than just having it in the comment above.

lurch (Migrated from github.com) reviewed

2026-07-30 10:37:35 -05:00

src/rp2_common/pico_bootrom/bootrom.c

					
				@ -85,9 +85,9 @@ void __attribute__((noreturn)) rom_reset_usb_boot_extra(int usb_activity_gpio_pi

				#if !PICO_RP2040

Finally got round to making and pushing this change 🙂

Finally got round to making and pushing this change :slightly_smiling_face:

kilograham (Migrated from github.com) reviewed

2026-07-31 10:53:23 -05:00

src/rp2_common/boot_bootrom_headers/include/boot/bootrom_constants.h

					
				@ -239,18 +239,25 @@ typedef int (*bootrom_api_callback_generic_t)(uint32_t r0, uint32_t r1, uint32_t

				// items are returned in order

I think _WORD_COUNT would be better, as WORDS_RETURNED is confusing below, and who says they are returned

I think `_WORD_COUNT` would be better, as WORDS_RETURNED is confusing below, and who says they are returned

kilograham (Migrated from github.com) reviewed

2026-07-31 10:53:43 -05:00

src/rp2_common/pico_bootrom/include/pico/bootrom.h

					
				@ -1109,9 +1109,9 @@ typedef struct {

				} boot_info_t;

I think i prefer wordss_returned == SYS_INFO... + 1 as per below

I think i prefer `wordss_returned == SYS_INFO... + 1` as per below

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'bootrom_sysinfo_tweaks:bootrom_sysinfo_tweaks'

git switch 'bootrom_sysinfo_tweaks'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'develop'

git merge --no-ff 'bootrom_sysinfo_tweaks'

git switch 'bootrom_sysinfo_tweaks'

git rebase 'develop'

git switch 'develop'

git merge --ff-only 'bootrom_sysinfo_tweaks'

git switch 'bootrom_sysinfo_tweaks'

git rebase 'develop'

git switch 'develop'

git merge --no-ff 'bootrom_sysinfo_tweaks'

git switch 'develop'

git merge --squash 'bootrom_sysinfo_tweaks'

git switch 'develop'

git merge --ff-only 'bootrom_sysinfo_tweaks'

git switch 'develop'

git merge 'bootrom_sysinfo_tweaks'

git push origin 'develop'

Sign in to join this conversation.

No reviewers

peterharperuk

No labels

compiler version issues

needs reporter detail

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!3094

				`@ -85,9 +85,9 @@ void __attribute__((noreturn)) rom_reset_usb_boot_extra(int usb_activity_gpio_pi`

				`#if !PICO_RP2040`

				`@ -38,16 +43,22 @@ static void __attribute__((PICO_UNIQUE_BOARD_ID_INIT_ATTRIBUTES)) _retrieve_uniq`
				`#error unique board ID size is greater than flash unique ID size`

				`@ -239,18 +239,25 @@ typedef int (*bootrom_api_callback_generic_t)(uint32_t r0, uint32_t r1, uint32_t`
				`// items are returned in order`

Rows
Columns