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.

Sign up for GitHub

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

Jump to bottom

rimage: update submodule to 542924d70c17, back in sync with west.yml (fixes -dirty) #6398

Merged
merged 2 commits into from
Oct 13, 2022
Merged

rimage: update submodule to 542924d70c17, back in sync with west.yml (fixes -dirty) #6398

merged 2 commits into from
Oct 13, 2022

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Oct 11, 2022

Fixes commit a3b3c52 ("west: update to newer rimage baseline")

See commit messages and new warning in west.yml

@marc-hb
rimage: update submodule to 542924d, back in sync with west.yml
ace2414 
Fixes commit a3b3c52 ("west: update to newer rimage baseline")

542924d config: tgl-h-cavs: add probe module
5b076f1 config: tgl-cavs: add probe module
6b4848e mtl: Add UPDWMIX module to the manifest
d50aefc manifest max size for loadable modules
d94cfc9 mtl: add MicSel support
5504179 Add lib_code module type.
12bb327 toml: add tgl-h config file for IPC4
452847d mtl: Add SRC module to the manifest

Signed-off-by: Marc Herbert <[email protected]>
@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 11, 2022

Just for the record the -dirty -ness was found in (unrelated) https://sof-ci.01.org/sofpr/PR6378/build1816/build/tgl_zph.txt

[0/25] cmake -P /srv/home/jenkins/workspace/sof_generic_build/sof/scripts/cmake/version.cmake
-- SOF version.cmake starting at 2022-10-11T14:55:23Z UTC
-- /srv/home/jenkins/workspace/sof_generic_build/sof/zephyr/.. is at git commit with parent(s):
commit 0928231465bcc19c196ed41113c229051df72297 b104f70c3da706bbd76797aaa17ae84940fc1178 32680d7e8f9977a0a59f093fe4f0dc19a49ed001 (HEAD, sof/pull/6378/merge)
Merge: b104f70c3da7 32680d7e8f99
Author: Kai Vehmanen <[email protected]>
Date:   Tue Oct 11 17:49:14 2022 +0300

    Merge 32680d7e8f9977a0a59f093fe4f0dc19a49ed001 into b104f70c3da706bbd76797aaa17ae84940fc1178
-- GIT_TAG / GIT_LOG_HASH : v2.0-rc1-1504-g0928231465bc-dirty / 0928231465bc

@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 11, 2022 •
edited
Loading

-dirty is gone from the Zephyr build logs in https://sof-ci.01.org/sofpr/PR6398/build1818/build/, problem fixed.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 11, 2022

All the compilation failures found by https://github.com/thesofproject/sof/actions/runs/3229592866/ happen ONLY with the bleeding edge zephyr main branch, the manifest revision of Zephyr always builds fine.

@marc-hb marc-hb marked this pull request as ready for review October 11, 2022 19:28
@fredoh9
Copy link
Contributor

fredoh9 commented Oct 11, 2022

The changes in this PR looks good but I don't get '-dirty' problem. '-dirty' post fix is attached when local change is present, isn't it. Can you educate more? Why -dirty problem was there before and how that is fixed by this PR?

@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 11, 2022

The changes in this PR looks good but I don't get '-dirty' problem. '-dirty' post fix is attached when local change is present, isn't it

Try this: git -C rimage checkout HEAD~3 and then:

git status
git describe --dirty
(build)

@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 11, 2022

https://sof-ci.01.org/sofpr/PR6398/build1818/devicetest/?model=TGLU_UP_HDA_ZEPHYR&testcase=verify-kernel-boot-log is lacking a monitor (https://github.com/intel-innersource/drivers.audio.ci.sof-framework/issues/294), everything else is green.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 12, 2022 •
edited
Loading

https://sof-ci.01.org/sof-pr-viewer/#/build/PR6398/build10541812 failed with

License path:  [email protected]:/localdisk/tools/xtensa/RG-2017.8-linux/XtensaTools/Tools/lic/license.dat:
FLEXnet Licensing error:-15,570.  System Error: 115 "Operation now in progress"

@wszypelt port 84300 is deprecated, please switch to 18764

https://sof-ci.01.org/sofpr/PR6398/build1818/devicetest/?model=TGLU_UP_HDA_ZEPHYR&testcase=verify-kernel-boot-log is missing a monitor - harmless

@wszypelt
Copy link

@marc-hb
Done, I switched xtensa to port 18764

@marc-hb
west.yml: add warning to keep git submodules in sync
68ada05 
Trying avoid out-of-sync situations like commit a3b3c52 ("west:
update to newer rimage baseline").

Also explain why sof cannot be cloned as "sof2"

Signed-off-by: Marc Herbert <[email protected]>
@marc-hb marc-hb mentioned this pull request Oct 12, 2022
Merged
kv2019i
kv2019i approved these changes Oct 12, 2022
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Approving but see comment inline.

west.yml
# describe` notice and a "-dirty" suffix appears in the SOF version.
# It is generally very bad practice to git push without looking
# at "git status"; even without submodules.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Given we will need frequent updates to rimage (like #6385 today), the above seems just way too complicated. Can we use a different directory for either the submodule (renaming it) or the west module? This probably needs to be done in steps to remove the hardcodes for rimage path, before the switch, but I don't see we can survive with this duplicated rimage copies setup in the long term.

Not blocking the PR because of this, as long as we use the same directory name, this is needed yes or yes.

Copy link
Collaborator Author

@marc-hb marc-hb Oct 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the best would be to move rimage one level-up in west.yml and leave the rimage submodule in place. west would then stop fooling git submodules. This would help de-hardcoding the sof name of the sof clone too.

BTW west is suspiciously quiet about nesting git repos. It does not explicitly forbid it but it never shows any nested git repo in any example anywhere in the documentation.

HOWEVER, moving rimage in west.yml would require large, disruptive and backwards-incompatible changes in the build system and in CI, which is why it was not done yet. I mean much more complicated and much more time-consuming changes than keeping the submodule in sync. Good request to add the backlog though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Given we will need frequent updates to rimage (like #6385 today),

BTW rimage updates were not really frequent in the past. They are more frequent now because MTL is brand new but I would expect this to ease off soon. As long as the firmware boots who cares about rimage?

@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 12, 2022 •
edited
Loading

Common suspend/resume failure in https://sof-ci.01.org/sofpr/PR6398/build1856/devicetest/?model=TGLU_UP_HDA_ZEPHYR&testcase=check-suspend-resume-5

I think this warning was just fixed in upstream Zephyr by #6400 /zep_workspace/sof/app/boards/intel_adsp_ace15_mtpm.conf:9: warning: attempt to assign the value 'y' to the undefined symbol DW_ICTL_ACE_V1X

-dirty suffix gone in https://sof-ci.01.org/sofpr/PR6398/build1856/devicetest/?model=ADLP_RVP_SDW_ZEPHYR&testcase=check-playback-3times and others.

Everything else green, good to go.

@kv2019i
Copy link
Collaborator

kv2019i commented Oct 13, 2022

Fourth failure in "Internal Intel CI System/merge/build", IPC3 smoke test. FYI @wszypelt

@ranj063 ranj063 merged commit 8fd351e into thesofproject:main Oct 13, 2022
@marc-hb marc-hb deleted the rimage-catchup branch October 13, 2022 17:49
@marc-hb marc-hb mentioned this pull request Jul 5, 2023
Merged
@marc-hb marc-hb mentioned this pull request Jul 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kv2019i kv2019i kv2019i approved these changes

@ujfalusi ujfalusi ujfalusi approved these changes

@lgirdwood lgirdwood Awaiting requested review from lgirdwood lgirdwood is a code owner

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

@kfrydryx kfrydryx Awaiting requested review from kfrydryx

@aborisovich aborisovich Awaiting requested review from aborisovich

@andyross andyross Awaiting requested review from andyross

@aiChaoSONG aiChaoSONG Awaiting requested review from aiChaoSONG

@fredoh9 fredoh9 Awaiting requested review from fredoh9

@greg-intel greg-intel Awaiting requested review from greg-intel

@keqiaozhang keqiaozhang Awaiting requested review from keqiaozhang

@mengdonglin mengdonglin Awaiting requested review from mengdonglin

@abonislawski abonislawski Awaiting requested review from abonislawski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@marc-hb @fredoh9 @wszypelt @kv2019i @ujfalusi @ranj063