Skip to content

xtensa-build-zephyr.py: pass SOF firmware file version string to rimage #5846

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

Merged
lgirdwood merged 1 commit into from
May 31, 2022
Merged

xtensa-build-zephyr.py: pass SOF firmware file version string to rimage #5846

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions scripts/xtensa-build-zephyr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
west_top = pathlib.Path(SOF_TOP, "zephyrproject")
default_rimage_key = pathlib.Path("modules", "audio", "sof", "keys", "otc_private_key.pem")

sof_version = None

if platform.system() == "Windows":
xtensa_tools_version_postfix = "-win32"
elif platform.system() == "Linux":
Expand Down Expand Up @@ -323,6 +325,28 @@ def west_init_update():
# Do NOT "west update sof"!!
execute_command(["west", "update", "zephyr", "hal_xtensa"], timeout=300, cwd=west_top)

def get_sof_version(abs_build_dir):
"""[summary] Get version string major.minor.micro of SOF firmware
file. When building multiple platforms from the same SOF commit,
all platforms share the same version. So for the 1st platform,
generate the version string from sof_version.h and later platforms
will reuse it.
"""
global sof_version

@aiChaoSONG aiChaoSONG May 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The feature can be done by:

  • use a global sof_version, and modify it with get_sof_version. in signing, use the global one directly.
  • don't use a global sof_version, and in signing, use the one returned from get_sof_version, (function runs for each platform)

Seems you mixed use above two in the code. since the sof_version is already global, the return statement in get_sof_version is redundant. you can modify and use the global sof_version directly.

@marc-hb marc-hb May 27, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with @aiChaoSONG that there is a problem now: the sof_version global is set twice, BOTH inside and outside the function. That's not good.

However I prefer the other option: keep all read/write accesses to the global sof_version and its "maintenance" private and isolated inside the function.

@marc-hb marc-hb May 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh wait: the second sof_version in build_platforms() is local, shadowing the global. That's not good either; shadowing == bad.

pylint scripts/xtensa-build-zephyr.py| sort -t: -k2,3 -n

scripts/xtensa-build-zephyr.py:472:2: W0621: Redefining name 'sof_version' from outer scope (line 27) (redefined-outer-name)

https://pylint.pycqa.org/en/latest/user_guide/messages/warning/redefined-outer-name.html

Same solution.

if sof_version:
return sof_version

versions = {}
with open(pathlib.Path(abs_build_dir,
"zephyr/include/generated/sof_versions.h"), encoding="utf8") as hfile:
for hline in hfile:
words = hline.split()
if words[0] == '#define':
versions[words[1]] = words[2]
sof_version = versions['SOF_MAJOR'] + '.' + versions['SOF_MINOR'] + '.' + \
versions['SOF_MICRO']
return sof_version

def build_platforms():
global west_top, SOF_TOP
print(f"SOF_TOP={SOF_TOP}")
Expand Down Expand Up @@ -447,6 +471,8 @@ def build_platforms():
signing_key = default_rimage_key
sign_cmd += ["--tool-data", str(rimage_config), "--", "-k", str(signing_key)]

sign_cmd += ["-f", get_sof_version(abs_build_dir)]

if args.ipc == "IPC4":
rimage_desc = pathlib.Path(SOF_TOP, "rimage", "config", platform_dict["IPC4_RIMAGE_DESC"])
sign_cmd += ["-c", str(rimage_desc)]
Expand Down