-
Notifications
You must be signed in to change notification settings - Fork 321
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
platform: imx93_a55: handle linker additions at SOF level #9372
platform: imx93_a55: handle linker additions at SOF level #9372
Conversation
OK, this difference in https://github.com/thesofproject/sof/actions/runs/10452157029/job/28940502308?pr=9372 Downloads$ diff -u -b ?in/build-sof-staging/sof-info/imx8/zephyr_version.h
--- lin/build-sof-staging/sof-info/imx8/zephyr_version.h 2024-08-19 11:12:34
+++ win/build-sof-staging/sof-info/imx8/zephyr_version.h 2024-08-19 11:14:22
@@ -19,7 +19,7 @@
#define KERNEL_VERSION_EXTENDED_STRING "3.7.99+0"
#define KERNEL_VERSION_TWEAK_STRING "3.7.99+0"
-#define BUILD_VERSION v3.7.0-1237-g8f33d9dc8498
+#define BUILD_VERSION v3.7.0-1237-g03b829ce71cd
#endif /* _KERNEL_VERSION_H_ */ After a fortunately short time, I found the root cause in zephyrproject-rtos/zephyr#77228
A GitHub race condition between @LaurentiuM1234 and... @LaurentiuM1234 ! I have debugged countless build reproducibility issues but I had never seen any like this. @LaurentiuM1234 , please be nicer on my heart, it's not getting any younger :-D |
@teburd can you comment here ? |
Comments on this change would be very much appreciated. Especially regarding the idea of using linker script snippets on SOF-side to handle any required additions. |
My thinking is that all things linker should really come from Zephyr and we should transition towards that goal, but I dont know yet if this is missing from Zephyr or how much effort would be needed. |
mm, I would think the Zephyr linker stuff should be relatively generic and allow it to be used by as many applications as possible. Adding application-specific linker snippets on Zephyr side is not that big of a deal since you'd encase these in I think Xtensa is a bit special in this regard since there's no architecture generic linker script so each vendor has its own linker script anyways. Instead, ARM and ARM64 have a "generic", architecture linker script also used as the SOC linker script. Anyways, since we do have the |
@nashif who can give advise on Zephyr linker here ? Can we support application linker overlays for certain application sections ? |
any other take on this? if not, I'd prefer handling the SOF-related linker additions in SOF if reasonable to do so/ if Zephyr infrastructure allows us to do so. |
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.
@LaurentiuM1234 we can merge after the Zephyr part is done.
3c5a905
to
0189239
Compare
0189239
to
81e7f6b
Compare
Zephyr dependency merged, updated hash to to that commit's. Dropping !! Note that this commit modifies |
81e7f6b
to
54ad8d1
Compare
Its best practice for bisect-ability to upstream the west update 1st. Can you do 2 patches, one for west 1st and 2nd patch for linker ? |
You mean have them in different PRs? If so, the |
@LaurentiuM1234 @lgirdwood A combine patch is ok (and even required) if Zephyr side has a change that breaks SOF build. With two separate commits, the SOF bisect is broken, so better to have the west.yml update in the same commit (and maintain bisect-capability on SOF side). |
54ad8d1
to
6702131
Compare
Rebased |
SOFCI TEST |
@LaurentiuM1234 linker error:
|
SOF requires some additions to the Zephyr linker script. This should be handled inside SOF instead of Zephyr. Since 3f2790b89ca5 ("soc: imx93: remove custom linker script") removes the custom linker script from Zephyr, this means the custom SOF linker sections will have to be handled on SOF side. This also includes a Zephyr hash bump to 3f2790b89ca5, which brings the following (potentially) relevant patches: dc5f1bfb3f2e west: fix for Python prior to 3.10 3f2790b89ca5 soc: imx9: remove custom linker script d389c95935c5 soc: intel_adsp: ace: Configurable SRAM retention mode and cleanup be041b14fe51 Intel: ADSP: move HPSRAM mask into assembly b3b8360f3993 west: runners: Add `west rtt` command with pyocd implementation Signed-off-by: Laurentiu Mihalcea <[email protected]>
6702131
to
3b12451
Compare
Changed the Zephyr hash bump to dc5f1bfb3f2e ("west: fix for Python prior to 3.10"). Hopefully, that'll take care of the internal CI build failures. |
@LaurentiuM1234 Ah, now you hit this problem zephyrproject-rtos/zephyr#78479 (revert) and zephyrproject-rtos/zephyr#78612 (fix) |
Moving to draft. Submitted #9491 which removes the need to use the extra linker sections. |
SOF requires some additions to the Zephyr linker script. This should be handled inside SOF instead of Zephyr.
Since 8f33d9dc8498 ("soc: imx93: remove custom linker script") removes the custom linker script from Zephyr, this means the custom SOF linker sections will have to be handled on SOF side.
This also includes a Zephyr hash bump to pull/77228/head to pull in the following commits:
8f33d9dc8498 soc: imx93: remove custom linker script
Depends on: zephyrproject-rtos/zephyr#77228