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

zephyr: update to today's "main" #9455

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Conversation

lyakh
Copy link
Collaborator

@lyakh lyakh commented Sep 10, 2024

Update Zephyr to include f94427deecf0 ("Intel: ADSP: move HPSRAM mask into assembly").

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.

Some "thinking-aloud" what should be written into git commit messages on these inline.

west.yml Outdated
@@ -43,7 +43,7 @@ manifest:

- name: zephyr
repo-path: zephyr
revision: 689d1edee1d57f052b1d4572d67618c0b0e2b8a4
revision: 632d6b9416c1c1813f3304ef29a6eda7954af3c3
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a good question what to write to these commits (and what should be expected). I've been including a short history for what changes this is pulling in from Zephyr, but this has not been a hard requirement. If I look at Zephyr conventions, it seems git commit description culture on west.yml updates is quite varied. Many "update to latest" mixed with commits that have detailed changelogs of pulled in changes.

Oh well, I'll let others chime in, maybe I should stop making the verbose commits as well and just do "update to today's" as well. It's true anyone can check the changes from the source as well, and this can lower the barrier to submit west.yml changes (= you don't need to write a novel to submit one).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kv2019i I like your lists of relevant updates, but since you weren't requiring them in the past... ;-) How do you make them? Something like

git log A..B -- adsp_path_1 adsp_path_2 adsp_path_3...

or by pulling all merged PRs with the "Intel ADSP" label or?.. If it can be scripted well enough, we can do that

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with "update to Zephyr latest to bring in A, B and C for SOF X, Y and Z features". Keep it simple.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lyakh It's mostly manual what I do. I run "git log --pretty=oneline --abbrev-commit 494f88070f6bb909389630264e4f89c71072ae53.. 689d1edee1d57f052b1d4572d67618c0b0e2b8a4 " and then browse through the commit list and pick up commits that seem relevant to highlight. If it's a big update, I just search for keywords (audio/dai/dma/dsp/intel/nxp) and not go through the full list.

But yeah, let's go with a slimmer approach. That seems more aligned with how west.yml is managed in e.g. Zephyr and keeps the barrier-to-entry low.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep in mind these SHA1s can point to literally anything, including unmerged forks. That's just how Github shared storage works.

https://lwn.net/Articles/884105/

Zephyr CI now has an "imposter commit" check:

Short of such a check, I think every SHA1 in the west.yml should have at least a one-line comment with the name of the branch it comes from. Not just for safety and security but also for convenience.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe we should add such a check

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect adding a comment is several orders of magnitude less effort than automating such a check :-)

@marc-hb
Copy link
Collaborator

marc-hb commented Sep 10, 2024

Wow, LNL alsabat 100% PASS (or SKIP) in https://sof-ci.01.org/sofpr/PR9455/build7827/devicetest/index.html

@marc-hb
Copy link
Collaborator

marc-hb commented Sep 10, 2024

@kv2019i
Copy link
Collaborator

kv2019i commented Sep 11, 2024

Failures with D3 entry on Intel PTL with this one, so the "Internal Intel CI System/merge/build" failure is real.

Update Zephyr to include:
1dde70637d2a ("Intel: ACE: move hpsram_mask to a data section")
dc5f1bfb3f2e ("west: fix for Python prior to 3.10")
and other fixes and improvements.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
@lyakh lyakh marked this pull request as ready for review September 18, 2024 13:50
@lgirdwood
Copy link
Member

@LaurentiuM1234 this west update may fix your linker PR too ?

@kv2019i
Copy link
Collaborator

kv2019i commented Sep 18, 2024

I don't fully get how this can work without @LaurentiuM1234 's changes to the linker scripts? I.e. I was expecting we'd need the changes from #9372 to upgrade Zephyr...? Or this is a imx93 case not covered in the github CI flows...?

@LaurentiuM1234
Copy link
Contributor

I don't fully get how this can work without @LaurentiuM1234 's changes to the linker scripts? I.e. I was expecting we'd need the changes from #9372 to upgrade Zephyr...? Or this is a imx93 case not covered in the github CI flows...?

ah, interesting, I guess the resulting warnings from zephyrproject-rtos/zephyr@3f2790b don't break the CI job, but they're still there.

@lgirdwood
Copy link
Member

I can see a column of red on a single MTL DUT that looks like a GFX error cascading to cause all the other tests to fail. Will rerun CI to be sure.

@lgirdwood
Copy link
Member

SOFCI TEST

@lyakh
Copy link
Collaborator Author

lyakh commented Sep 19, 2024

I can see a column of red on a single MTL DUT that looks like a GFX error cascading to cause all the other tests to fail. Will rerun CI to be sure.

for the record: https://sof-ci.01.org/sofpr/PR9455/build8170/devicetest/index.html

@lgirdwood
Copy link
Member

@wszypelt @lrudyX good to merge ?

@lyakh
Copy link
Collaborator Author

lyakh commented Sep 19, 2024

@wszypelt @lrudyX good to merge ?

@lgirdwood quickbuild re-run successful

@lgirdwood lgirdwood merged commit 80d6e5e into thesofproject:main Sep 19, 2024
46 of 47 checks passed
@lyakh lyakh deleted the zephyr branch September 19, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants