From 9e421ae7bc9b1a8195482458a7164b853586b42b Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Wed, 8 Sep 2021 18:37:45 +0000 Subject: [PATCH 1/4] xtensa-build-zephyr: extract new parse_args() function from main() main() was growing too big. Zero functional change. Also rename the too generic "build()" to build_all() Signed-off-by: Marc Herbert --- scripts/xtensa-build-zephyr.sh | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/scripts/xtensa-build-zephyr.sh b/scripts/xtensa-build-zephyr.sh index b9347cb722cc..dcd21209dbe5 100755 --- a/scripts/xtensa-build-zephyr.sh +++ b/scripts/xtensa-build-zephyr.sh @@ -39,8 +39,8 @@ the _defconfig file for that platform. usage: $0 [options] platform(s) -a Build all platforms. - -j n Set number of make build jobs. Jobs=#cores by default. - Infinite when not specified. + -j n Set number of make build jobs for rimage. Jobs=#cores by default. + Ignored by "west build". -k Path to a non-default rimage signing key. -c recursively clones Zephyr inside sof before building. Incompatible with -p. @@ -103,7 +103,7 @@ install_opts() install -D -p "$@" } -build() +build_all() { cd "$WEST_TOP" west topdir || { @@ -117,6 +117,8 @@ build() mkdir -p ${STAGING}/sof/ # smex does not use 'install -D' for platform in "${PLATFORMS[@]}"; do + # TODO: extract a new build_platform() function for + # cleaner error handling and saving a lot of tabs. # default value local RIMAGE_KEY=modules/audio/sof/keys/otc_private_key.pem @@ -212,7 +214,7 @@ build() tree "$STAGING" } -main() +parse_args() { local zeproj @@ -270,6 +272,11 @@ main() print_usage exit 1 fi +} + +main() +{ + parse_args "$@" if [ "x$DO_CLONE" == "xyes" ]; then clone @@ -289,7 +296,7 @@ main() ) fi - build + build_all } main "$@" From c2e8837cf5b5f18f6cd42ae10b2fee9df79fbb15 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Wed, 8 Sep 2021 19:47:11 +0000 Subject: [PATCH 2/4] xtensa-build-zephyr: support passing through CMake arguments Like this: xtensa-build-zephyr.sh -a -- -DEXTRA_CFLAGS='-Werror -Wextra' Signed-off-by: Marc Herbert --- scripts/xtensa-build-zephyr.sh | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/scripts/xtensa-build-zephyr.sh b/scripts/xtensa-build-zephyr.sh index dcd21209dbe5..0c7fc7e3a69f 100755 --- a/scripts/xtensa-build-zephyr.sh +++ b/scripts/xtensa-build-zephyr.sh @@ -36,7 +36,7 @@ print_usage() Re-configures and re-builds SOF with Zephyr using a pre-installed Zephyr toolchain and the _defconfig file for that platform. -usage: $0 [options] platform(s) +usage: $0 [options] platform(s) [ -- cmake arguments ] -a Build all platforms. -j n Set number of make build jobs for rimage. Jobs=#cores by default. @@ -181,7 +181,8 @@ build_all() /bin/pwd set -x west build --build-dir "$bdir" --board "$PLAT_CONFIG" \ - zephyr/samples/subsys/audio/sof + zephyr/samples/subsys/audio/sof \ + -- "${CMAKE_ARGS[@]}" # This should ideally be part of # sof/zephyr/CMakeLists.txt but due to the way @@ -218,7 +219,9 @@ parse_args() { local zeproj - # parse the args + local OPTIND=1 + + # Parse -options while getopts "acj:k:p:" OPTION; do case "$OPTION" in a) PLATFORMS=("${SUPPORTED_PLATFORMS[@]}") ;; @@ -229,6 +232,10 @@ parse_args() *) print_usage; exit 1 ;; esac done + # This also drops any _leading_ '--'. Note this parser is + # compatible with using '--' twice (undocumented feature), as + # in: + # build-zephyr.sh -c -k somekey -- apl imx -- -DEXTRA_CFLAGS='-Werror' shift $((OPTIND-1)) if [ -n "$zeproj" ] && [ x"$DO_CLONE" = xyes ]; then @@ -247,8 +254,15 @@ parse_args() WEST_TOP="$zeproj" fi - # parse platform args - for arg in "$@"; do + # Find platform arguments if '-a' was not used + test "${#PLATFORMS[@]}" -ne 0 || while test -n "$1"; do + local arg="$1" + + # '--' ends platforms + if [ x"$arg" = x-- ]; then + shift || true; break + fi + platform=none for i in "${SUPPORTED_PLATFORMS[@]}"; do if [ x"$i" = x"$arg" ]; then @@ -265,13 +279,21 @@ parse_args() fi done - # check target platform(s) have been passed in + # Check some target platform(s) have been passed in one way or + # the other if [ "${#PLATFORMS[@]}" -eq 0 ]; then echo "Error: No platforms specified. Supported are: " \ "${SUPPORTED_PLATFORMS[*]}" print_usage exit 1 fi + + printf 'Building platforms:' + printf ' %s' "${PLATFORMS[@]}"; printf '\n' + + CMAKE_ARGS=("$@") + # For debugging quoting and whitespace + #declare -p CMAKE_ARGS } main() From 77f111c64ad8a98ce36c5753f7b7f5cdd3db990d Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Wed, 8 Sep 2021 19:33:23 +0000 Subject: [PATCH 3/4] zephyr/docker-build.sh: pass arguments through to xtensa-build-zephyr.sh Allows passing compilation flags and any other argument. Signed-off-by: Marc Herbert --- zephyr/docker-build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zephyr/docker-build.sh b/zephyr/docker-build.sh index 87c3cbc91811..757c66e7724f 100755 --- a/zephyr/docker-build.sh +++ b/zephyr/docker-build.sh @@ -19,12 +19,12 @@ sudo apt-get update sudo apt-get -y install tree if test -e zephyrproject; then - ./scripts/xtensa-build-zephyr.sh -a + ./scripts/xtensa-build-zephyr.sh -a "$@" else # Matches docker.io/zephyrprojectrtos/zephyr-build:latest gid ls -ln | head stat . sudo chgrp -R 1000 . sudo chmod -R g+rwX . - ./scripts/xtensa-build-zephyr.sh -a -c + ./scripts/xtensa-build-zephyr.sh -a -c "$@" fi From d1e612bce055ce928cedf437bb2092a823ad28d4 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 7 Sep 2021 17:38:31 +0000 Subject: [PATCH 4/4] .github/zephyr: add -Werror Because we can. This would have caught this regression from commit 287a5f9a2b1e ("ssp: mclk/bclk turned off unexpectedly") -DEXTRA_CFLAGS is not very well documented but it is what Zephyr uses, try `git -C zephyr grep -C 5 EXTRA_CFLAGS and see. ``` sof/src/drivers/intel/ssp/ssp.c: In function 'ssp_set_config_tplg': sof/zephyr/include/sof/trace/trace.h:44:11: warning: too many arguments for format [-Wformat-extra-args] 44 | printk("%llu: " format "\n", platform_timer_get(NULL), \ | ^~~~~~~~ ... sof/src/drivers/intel/ssp/ssp.c:763:4: note: in expansion of macro 'dai_info' 763 | dai_info(dai, "ssp_set_config(): hw_free stage: ignore since there is still user", dai->index); ``` Using -Werror only in CI avoids slowing down developers with temporary warnings they intend on fixing later (but before submission) Signed-off-by: Marc Herbert --- .github/workflows/zephyr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/zephyr.yml b/.github/workflows/zephyr.yml index 68f80dddd98d..f30c05d256e3 100644 --- a/.github/workflows/zephyr.yml +++ b/.github/workflows/zephyr.yml @@ -19,4 +19,4 @@ jobs: run: docker run -v "$(pwd)":/workdir -e ZEPHYR_SDK_INSTALL_DIR='/opt/toolchains/zephyr-sdk-0.13.0' docker.io/zephyrprojectrtos/zephyr-build:v0.18.3 - ./zephyr/docker-build.sh + ./zephyr/docker-build.sh -- -DEXTRA_CFLAGS='-Werror'