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

Delay relocation of data/bss init after MEMC init is completed #68888

Closed

Conversation

talih0
Copy link
Contributor

@talih0 talih0 commented Feb 12, 2024

Currently, when relocating to external SRAM regions via MEMC drivers, the relocation/zero initialization is done before the MEMC drivers are initialized.

See issue #68884

This patch delays initialization after PRE_KERNEL_1 level.
The initialization is only delayed on devicetree memory regions with delay-relocation property set.

Note, MEMC drivers (i.e. Atmel SAM SMC) use POST_KERNEL level, so their initialization levels would have to be updated.

Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
@talih0
Copy link
Contributor Author

talih0 commented Feb 12, 2024

@pdgendt, @nandojve, would you mind having a look please, as I believe it also applies to Atmel SAM SMC drivers.

@erwango
Copy link
Member

erwango commented Apr 8, 2024

I guess this would fix #43555

@cfriedt
Copy link
Member

cfriedt commented Apr 9, 2024

I guess this would fix #43555

I had a similar issue #39598 a couple of years ago. That would also be fixed by this (with the addition of a memc driver)

@erwango
Copy link
Member

erwango commented Apr 11, 2024

@talih0 Can you fix compliance issue reported by automated checks ?

Comment on lines +869 to +870
depends on ARCH_HAS_CODE_DATA_RELOCATION
depends on CODE_DATA_RELOCATION
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODE_DATA_RELOCATION already depends on ARCH_HAS_CODE_DATA_RELOCATION, no need to add it again.

Suggested change
depends on ARCH_HAS_CODE_DATA_RELOCATION
depends on CODE_DATA_RELOCATION
depends on CODE_DATA_RELOCATION

@MaureenHelm MaureenHelm requested review from danieldegrasse and removed request for decsny May 16, 2024 15:27
@danieldegrasse
Copy link
Collaborator

I wonder if we should have some form of device_is_ready check done before the relocation calls are made- since the code relocation script now has access to the devicetree, perhaps there is some way we can verify the MEMC device defining the memory region was initialized correctly?

The reason I ask is that if we simply blindly copy data into memory regions that may be set up by MEMC devices, we run the risk of crashing the chip. I have seen this with iMX.RT parts (which use external SDRAM setup by the bootloader). In this case, the chip will simply crash at initialization since the SRAM region is not accessible.

I'm not sure how we would best handle this (perhaps simply triggering a fault). I just would like to avoid the case where an external memory device fails to initialize, and the application simply crashes rather than printing some form of error message

@swift-tk
Copy link
Collaborator

Is cache handled at this point? I see most of cache enables are in the level of PRE_KERNEL_1. The SoCs may have a default cacheable memory map for those regions, so it is best to perform a whole cache clean operation at the end of copy.

@swift-tk
Copy link
Collaborator

There is another issue I just thought about. Since stimer init call is in INIT_LEVEL_PRE_KERNEL_2, if MEMC drivers have to be initialized in INIT_LEVEL_PRE_KERNEL_1, this means that the device init can not use any timer related kernel function like k_sleep, k_busy_wait.

Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jul 17, 2024
@github-actions github-actions bot closed this Aug 1, 2024
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

8 participants