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

use prep_c for xtensa to align with other architectures #75665

Merged
merged 5 commits into from
Aug 7, 2024

Conversation

nashif
Copy link
Member

@nashif nashif commented Jul 9, 2024

  • xtensa: remove mention of crt0-app.S
  • xtensa: introduce prep_c for xtensa
  • xtensa: adapt soc code to use prep_c
  • xtensa: move arch_kernel_init code into prep_c

@nashif nashif force-pushed the topic/arch/xtensa_prep_c branch 2 times, most recently from f5c33a3 to 63cd77b Compare July 9, 2024 21:38
@nashif nashif added this to the v4.0.0 milestone Jul 9, 2024
@nashif nashif marked this pull request as ready for review August 1, 2024 14:33
@zephyrbot zephyrbot added platform: Intel ADSP Intel Audio platforms platform: ESP32 Espressif ESP32 area: Xtensa Xtensa Architecture labels Aug 1, 2024
dcpleung
dcpleung previously approved these changes Aug 1, 2024
uLipe
uLipe previously approved these changes Aug 1, 2024
andyross
andyross previously approved these changes Aug 1, 2024
sylvioalves
sylvioalves previously approved these changes Aug 1, 2024
Copy link
Collaborator

@tmleman tmleman left a comment

Choose a reason for hiding this comment

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

SOF CI reports a FW exception in tests with this PR. From what I see, the tests on power flow from D3 are failing.

thesofproject/sof#9345

@marc-hb
Copy link
Collaborator

marc-hb commented Aug 2, 2024

Thanks @tmleman for testing this, great catch.

I just got confirmation from @ranj063 that her recent "DSP Ops tester" work could be useful to debug this panic. It requires this Linux kernel patch:

The corresponding test script is here:

crt0-app.S does not exist, remove it from comments to avoid confusion.

Signed-off-by: Anas Nashif <[email protected]>
xtensa is the only architecutre doing thing differently and introduces
inconsistency in the init process and dependencies as we attemp to
cleanup init levels and remove misused of SYS_INIT.

Introduce prep_c for this architecture and align with other
architectures.

Signed-off-by: Anas Nashif <[email protected]>
Many xtensa target jump from soc code directly into cstart and depend on
architecture code being initialized in arch_kernel_init(). Instead of
jumping to cstart, jump to newly introduced prep_c similar to all other
architectures, where common platfotm initialization will happen.

Signed-off-by: Anas Nashif <[email protected]>
arch_kernel_init() was misused for all architecture initialization code
that is done in prep_c and prior to cstart on other architectures.
arch_kernel_init() is late in the init process and comes after EARLY
init level, making xtensa have a very special boot path.

Signed-off-by: Anas Nashif <[email protected]>
Restore order of execution. Code that was run in EARLY init level is now
too late.

Signed-off-by: Anas Nashif <[email protected]>
@nashif nashif dismissed stale reviews from sylvioalves, andyross, uLipe, and dcpleung via dbdcb4e August 5, 2024 22:55
@nashif nashif requested a review from tmleman August 6, 2024 01:10
@nashif
Copy link
Member Author

nashif commented Aug 6, 2024

SOF CI reports a FW exception in tests with this PR. From what I see, the tests on power flow from D3 are failing.

should be fixed now.

@nashif
Copy link
Member Author

nashif commented Aug 6, 2024

@tmleman please have another look.

@carlescufi carlescufi merged commit d590c18 into zephyrproject-rtos:main Aug 7, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Xtensa Xtensa Architecture platform: ESP32 Espressif ESP32 platform: Intel ADSP Intel Audio platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants