Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esp32: ULP programs limited to 512 bytes #8709

Closed
wants to merge 1 commit into from

Conversation

cwalther
Copy link
Contributor

Is there a reason why in 3d49b15 (the move from sdkconfig.h to sdkconfig), CONFIG_ULP_COPROC_RESERVE_MEM = 2040 was removed, reverting to the IDF default value 512?

This causes esp32.ULP.load_binary() to fail with OSError: 260 (ESP_ERR_INVALID_SIZE) on programs larger than 512 bytes, for no reason that I can see. It is possible to work around the limitation by rewriting load_binary() in Python (everything the underlying IDF function ulp_load_binary() does can be replicated using machine.mem32) and the ULP runs the resulting code just fine. (Interestingly enough, ulp_load_binary() only applies the limit to the .text and .data sections of the ULP binary, while the .bss section is allowed to overflow into “non-reserved” memory. I wonder what the point of that is.)

In case that was an oversight, the attached commit restores it. As discussed in #3578, 2040 = 4096 - MICROPY_HW_RTC_USER_MEM_MAX - 8 is the maximum size that fits into RTC slow memory next to rtc_user_mem_{data|len|magic}, higher values cause a linker error.

I have not looked into whether the value could be computed for non-standard values of MICROPY_HW_RTC_USER_MEM_MAX, but for now am just restoring the hardcoded value that was there before. Nor have I looked into how to overlap the ULP and machine.RTC.memory() ranges, as suggested in #3578 (comment).

Tested on a UM_TINYPICO board, built with ESP-IDF v4.2.2.

Allow esp32.ULP.load_binary() to use the maximum amount of memory
available again (lost in 3d49b15), which is 2040 bytes unless
MICROPY_HW_RTC_USER_MEM_MAX is customized.
@dpgeorge
Copy link
Member

That was definitely an unintended regression. Thanks for picking it up and creating a PR. Rebased and merged in cf550ad

@dpgeorge dpgeorge closed this Jun 23, 2022
@cwalther
Copy link
Contributor Author

Thanks!

@cwalther cwalther deleted the ulp-reserve branch June 23, 2022 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants