georgerobotics / cyw43-driver

Other
79 stars 42 forks source link

Make cyw43_async_event_name_table const so it goes to flash #52

Closed bruelltuete closed 1 year ago

bruelltuete commented 1 year ago

Checking the map file, cyw43_async_event_name_table was in RAM instead of flash, despite being "read only". Needs an additional const.

dpgeorge commented 1 year ago

I checked a few builds and this table does actually go in ROM (text/rodata sections) because the compiler sees it's static and sees that it's not written to, so optimises it into ROM.

But you're right that it should have a const, to make the intention clear and make sure it does always go into the text or rodata section.

dpgeorge commented 1 year ago

Thanks for the contribution!

bruelltuete commented 1 year ago

I checked a few builds and this table does actually go in ROM (text/rodata sections) because the compiler sees it's static and sees that it's not written to, so optimises it into ROM.

For debug builds it doesn't automatically go to read-only. The linker map says:

 .data.cyw43_async_event_name_table
                0x0000000020001628      0x164 CMakeFiles/farty.dir/pico-sdk/lib/cyw43-driver/src/cyw43_ctrl.c.obj

And from the release build:

 .rodata.cyw43_async_event_name_table
                0x000000001001f374      0x164 CMakeFiles/farty.dir/pico-sdk/lib/cyw43-driver/src/cyw43_ctrl.c.obj

With the extra const it goes to .rodata in both cases. this is for a raspi pico project... i don't know if that's different for other targets...

dpgeorge commented 1 year ago

Ah, OK, that makes sense that it wouldn't optimise it into ROM for debug builds.

(Note that this fix was merged.)