Skip to content

Conversation

@iuliana-prodan
Copy link
Contributor

Use the Zephyr sys/printk.h when Zephyr RTOS is used.

Suggested-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
Signed-off-by: Iuliana Prodan iuliana.prodan@nxp.com

Use the Zephyr sys/printk.h when Zephyr RTOS is used.

Suggested-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
@iuliana-prodan
Copy link
Contributor Author

This patch fixes the following warnings when building SOF with Zephyr on i.MX:

60/222] Building C object modules/sof/CMakeFiles/modules_sof.dir sof/sof/src/lib/dai.c.obj
In file included from sof/sof/zephyr/../src/include/sof/lib/dai.h:27,
                 from  sof/sof/src/lib/dai.c:7:
sof/sof/src/lib/dai.c: In function 'dai_group_get':
sof/sof/zephyr/include/sof/trace/trace.h:35:4: warning: implicit declaration of function 'printk'; did you mean 'cbprintf'? [-Wimplicit-function-declaration]
   35 |    printk("%llu: " format "\n", platform_timer_get(NULL), \
      |    ^~~~~~
sof/sof/zephyr/include/sof/trace/../../../../src/include/sof/trace/trace.h:287:2: note: in expansion of macro '_log_message'
  287 |  _log_message(trace_log_filtered, true, LOG_LEVEL_CRITICAL, class, ctx, id_1, \
      |  ^~~~~~~~~~~~
sof/sof/zephyr/include/sof/trace/../../../../src/include/sof/trace/trace.h:290:2: note: in expansion of macro '_trace_error_with_ids'
  290 |  _trace_error_with_ids(class, ctx, id_1, id_2, format, ##__VA_ARGS__)
      |  ^~~~~~~~~~~~~~~~~~~~~
sof/sof/zephyr/include/sof/trace/../../../../src/include/sof/trace/trace.h:374:2: note: in expansion of macro 'trace_error_with_ids'
  374 |  trace_error_with_ids(_TRACE_INV_CLASS, ctx, \
      |  ^~~~~~~~~~~~~~~~~~~~
sof/sof/src/lib/dai.c:85:3: note: in expansion of macro 'tr_err'
   85 |   tr_err(&dai_tr, "dai_group_get(): invalid group_id %u",
      |   ^~~~~~

@lgirdwood lgirdwood merged commit 8357dcf into thesofproject:main May 21, 2021
@marc-hb marc-hb requested a review from andyross May 21, 2021 18:32
@marc-hb
Copy link
Collaborator

marc-hb commented May 21, 2021

The #ifdef __ZEPHYR__ does not make sense to me because this entire file is used only when __ZEPHYR__ anyway. Can you please try to remove it?

The commit message does not make sense to me either, it does not seem to match the code change. Instead it should have explained why this is required with your toolchain but not needed in the configurations already built in CI.

Speaking of toolchains and CI, please add iMX to the list of SUPPORTED_PLATFORMS at the top of ./xtensa-build-zephyr.sh to add it to CI and make sure no regression like this happens again.

@iuliana-prodan
Copy link
Contributor Author

The #ifdef __ZEPHYR__ does not make sense to me because this entire file is used only when __ZEPHYR__ anyway. Can you please try to remove it?

You're right, I shouldn't add the #ifdef __ZEPHYR__. I will remove it.

The commit message does not make sense to me either, it does not seem to match the code change. Instead it should have explained why this is required with your toolchain but not needed in the configurations already built in CI.

I've added the warnings in a comment - see here.
I thought is too long to add it in the commit message.
Why we have the warnings with imx toolchain and not Intel, I don't know :(

Speaking of toolchains and CI, please add iMX to the list of SUPPORTED_PLATFORMS at the top of ./xtensa-build-zephyr.sh to add it to CI and make sure no regression like this happens again.

I'm planning of adding imx in ./xtensa-build-zephyr.sh (I've also answered this here), but maybe we should wait for the Zephyr pull request to be merged - see #35467.

@marc-hb
Copy link
Collaborator

marc-hb commented May 21, 2021

You're right, I shouldn't add the #ifdef ZEPHYR. I will remove it.

Thanks!

I thought is too long to add it in the commit message.

warning: implicit declaration of function 'printk' would have been enough (and accurate)

Why we have the warnings with imx toolchain and not Intel, I don't know :(

You can say "I don't know why" in the commit message and sometimes in the source code even. It happens :-)

"What gets us into trouble is not what we don't know. It's what we know for sure that just ain't so.”

but maybe we should wait for the Zephyr pull request to be merged

Agreed; let's not add to CI something that does not work yet. Otherwise people stop looking at it. Thanks for the links and reminders.

@marc-hb
Copy link
Collaborator

marc-hb commented May 21, 2021

Here are some details about my configuration that does not need this commit for the record and in case anyone would like to get to the bottom of this.

Besides the pre-processor (and doxygen a bit), no one ever understands the maze of #includes and #defines in any C project. So the fastest way to extract #include information from the toolchain is to purposely break the code:

warning: passing argument 1 of 'printk' makes pointer from integer without a cast [-Wint-conversion]
  160 |  printk(42);
      |         ^~
      |         |
      |         int
In file included from zephyr/include/kernel_includes.h:32,
                 from zephyr/include/kernel.h:17,
                 from sof/zephyr/../src/include/sof/debug/panic.h:34,
                 from sof/zephyr/../src/include/sof/audio/audio_stream.h:18,
                 from sof/zephyr/../src/include/sof/audio/buffer.h:11,
                 from sof/src/trace/dma-trace.c:7:
zephyr/include/sys/printk.h:59:52: note: expected 'const char *' but argument is of type 'int'
   59 | extern __printf_like(1, 2) void printk(const char *fmt, ...);
      |                                        ~~~~~~~~~~~~^~~
$ ninja -j1 -C build-apl/ -v

ccache /$HOME/zephyr-sdk-0122/xtensa/intel_apl_adsp/xtensa-zephyr-elf/bin/xtensa-zephyr-elf-gcc 
-DBUILD_VERSION=v2.6.0-rc1-72-g469a10a0f9c9 -DCC_OPTIMIZE_FLAGS=\"-Os\" -DKERNEL 
-DSOF_GIT_TAG=\"\" -DSOF_MAJOR=3 -DSOF_MICRO=1 -DSOF_MINOR=2 -DSOF_SRC_HASH=0x0 -DSOF_TAG=\"7eff1\" 
-DXCC_TOOLS_VERSION=\"zephyr\" -D_FORTIFY_SOURCE=2 -D__ZEPHYR__=1 -DRELATIVE_FILE=\"../../../../build/SOF/sof/src/trace/dma-trace.c\" -I/___/zephyr/include -Izephyr/include/generated 
-I/___/zephyr/soc/xtensa/intel_adsp/cavs_v15 -I/___/zephyr/soc/xtensa/intel_adsp/cavs_v15/include 
-I/___/zephyr/soc/xtensa/intel_adsp/common/include -I/build/SOF/sof/zephyr/include -I/build/SOF/sof/zephyr/../src/platform/intel/cavs/include -I/build/SOF/sof/zephyr/../src/platform/apollolake/include 
-I/___/modules/hal/xtensa/include -I/___/modules/hal/xtensa/zephyr/soc/intel_apl_adsp 
-I/build/SOF/sof/zephyr/../rimage/src/include -I/build/SOF/sof/zephyr/../zephyr/include -I/build/SOF/sof/zephyr/../src/include 
-I/build/SOF/sof/zephyr/../src/arch/xtensa/include -isystem /___/zephyr/lib/libc/minimal/include -isystem /$HOME/zephyr-sdk-0122/xtensa/intel_apl_adsp/xtensa-zephyr-elf/bin/../lib/gcc/xtensa-zephyr-elf/10.2.0/include -isystem /$HOME/zephyr-sdk-0122/xtensa/intel_apl_adsp/xtensa-zephyr-elf/bin/../lib/gcc/xtensa-zephyr-elf/10.2.0/include-fixed 
-Os -imacros /___/build-apl/zephyr/include/generated/autoconf.h -ffreestanding -fno-common -g -fdiagnostics-color=always -imacros /___/zephyr/include/toolchain/zephyr_stdint.h
-Wall -Wformat -Wformat-security -Wno-format-zero-length -Wno-main -Wno-pointer-sign -Wpointer-arith -Wexpansion-to-defined -Wno-address-of-packed-member -Wno-unused-but-set-variable -Werror=implicit-int -fno-asynchronous-unwind-tables -fno-pie -fno-pic -fno-strict-overflow -fno-reorder-functions -fno-defer-pop -fmacro-prefix-map=/___/zephyr/samples/subsys/audio/sof=CMAKE_SOURCE_DIR -fmacro-prefix-map=/___/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/home/mherber2/zephyr-west=WEST_TOPDIR -ffunction-sections -fdata-sections -mlongcalls 
-std=c99 -nostdinc -std=gnu99 -fno-inline-functions -MD -MT modules/sof/CMakeFiles/modules_sof.dir/build/SOF/sof/src/trace/dma-trace.c.obj -MF 
modules/sof/CMakeFiles/modules_sof.dir/build/SOF/sof/src/trace/dma-trace.c.obj.d -o 
modules/sof/CMakeFiles/modules_sof.dir/build/SOF/sof/src/trace/dma-trace.c.obj   -c /build/SOF/sof/src/trace/dma-trace.c
/build/SOF/sof/src/trace/dma-trace.c:160:9: error: 'breakage' undeclared (first use in this function)
  160 |  printk(breakage);
      |         ^~~~~~~~

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.

6 participants