-
Notifications
You must be signed in to change notification settings - Fork 226
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
Add linker script for external ram #1090
Conversation
Added linker scripts and bss reset for S3. I don’t know how correctly I did it, but all the addresses matched and it started for me @bjoernQ Please take a look and tell me your opinion. I will be glad for any help |
|
||
/* RTC slow memory (data accessible). Persists over deep sleep. */ | ||
rtc_slow_seg(RW) : ORIGIN = 0x50000000, len = 8k | ||
|
||
/* external memory, including data and text */ | ||
psram_seg(RWX) : ORIGIN = 0x3C000020, len = 32M - 0x20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should have that +0x20 here - it's used for mapped flash
/**
* (0x20 offset above is a convenience for the app binary image generation.
* Flash cache has 64KB pages. The .bin file which is flashed to the chip
* has a 0x18 byte file header, and each segment has a 0x08 byte segment
* header. Setting this offset makes it simple to meet the flash cache MMU's
* constraint that (paddr % 64KB == vaddr % 64KB).)
*/
@@ -0,0 +1,17 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this dummy section
|
||
|
||
SECTIONS { | ||
.external.data : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To support .data
in ext-ram we would need a way to copy the initial data here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know. I just want to first understand if I took everything into account for bss, at least 😅
Probably having a way to place uninitialized and zeroed data in PSRAM would require us to initialize PSRAM before calling user's main function If a user would (by accident) try to use data placed there before initializing PSRAM things will fail in pretty bad ways. Would probably get even worse when we add a way to place initialized data into PSRAM Maybe it would be even slightly more convenient if the HAL would initialize PSRAM automatically |
In general I agree with this. I suggest doing this either at the end of ESP32Reset or in _post_init. I think the first option is better.
|
Well, after a little research I determined that dummy and offset were needed. Without them, we cannot calculate the correct offset for the psram and, therefore, the placement of data in its segments |
Ah ok ... maybe I misread the esp-idf linker scripts |
I'd say
Probably not. It's just a virtual peripheral and of no use if we initialize the PSRAM before
True - while not ideal we could just remove the logs and keep the information somewhere for the user to query and log themselves - if something goes terribly wrong during initialization, we'll panic with some (hopefully) helpful information anyway |
How about giving the user a pre_main function. Call post_init at the end. |
Not really sure if it's worth the effort. Since we basically would need to "remember" all the things from the initialization in both scenarios we can also give the user a way to log things if they want later. I guess it's pretty similar but the "pre_main" thing would be something new to learn for the user |
3c43841
to
7d7e432
Compare
7d7e432
to
92ec17d
Compare
Anything we can do to move this PR along? It would be nice to "statically" load some things into PSRAM at boot time. |
Given the lack of activity here I'm going to go ahead and close this PR for the time being. Thank you for your contribution regardless, I hope somebody can pick this up some time and wrap it up. If you plan to continue work on this, please feel free to reopen the PR or create a new one. |
Thank you!
Thank you for your contribution.
Please make sure that your submission includes the following:
Must
errors
orwarnings
.cargo fmt
was run.CHANGELOG.md
in the proper section.Nice to have