thesofproject / sof

Sound Open Firmware
Other
530 stars 307 forks source link

[BUG] Unit tests build error on i.MX #2042

Open paulstelian97 opened 4 years ago

paulstelian97 commented 4 years ago

Describe the bug A clear and concise description of what the bug is. Whenever I'm trying to build the unit tests on i.MX on top of #2023 I'm getting errors other than those of void pointer arithmetic. Specifically, unit tests for Mux/Demux allocate PLATFORM_MAX_CHANNELS in certain constant vectors but on this platform PLATFORM_MAX_CHANNELS is 4.

To Reproduce Steps to reproduce the behavior: (e.g. list commands or actions used to reproduce the bug) Enable unit tests with xcc on i.MX8 (set correct Xtensa toolchain path) Build firmware with default config and unit tests.

Reproduction Rate Build error.

Expected behavior Only build errors related to void pointer arithmetic that weren't fixed by #2023 should exist. Once I fix all errors related to void pointer arithmetic the firmware should build.

Impact What impact does this issue have on your progress (e.g., annoyance, showstopper) Annoyance.

Environment 1) Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).

@dbaluta Why do we even have PLATFORM_MAX_CHANNELS as 4 in the first place? The other platforms have it as 8 and somehow the unit tests seem to depend on that value.

paulstelian97 commented 4 years ago

Additionally, if I fix those somehow when I build unit tests on the i.MX config CAVS seems to be compiled in and I'm getting errors from files which shouldn't even be included.

paulstelian97 commented 4 years ago

@jajanusz It seems like you're the author of at least some of the files (see 21375d45a2c89f10330cdf96adc6fc6aa7c96f29 ) which cause me trouble. I wonder if the best solution would be either to update the unit tests or force PLATFORM_MAX_CHANNELS to be 8 on the i.MX. Still need to discuss.

paulstelian97 commented 4 years ago

Ah, it appears there is issue after issue with the unit tests. Guess that I'll have to build them for a different platform for now.

jajanusz commented 4 years ago

@paulstelian97 Yes, I don't like how they look either, however they are low prio for now and were tested only for APL+ platforms, so I guess you may encounter many problems, maybe I'll revisit/refactor UT stuff when I'll have more time.

jajanusz commented 4 years ago

@paulstelian97 You may disable UTs that don't work for your platform with something like

if(NOT CONFIG_IMX)
    cmocka_test(
        mux_copy
        mux_copy.c
        mock.c
    )
endif()

In appropriate cmake lists.

paulstelian97 commented 4 years ago

Yeah, we'll see if we manage to get Qemu CI by then (I expect a few months for that though).

The thing is, I would have build unit tests precisely to see what builds or fails to build, not actually for running the tests themselves.

jajanusz commented 4 years ago

@paulstelian97 We don't have support for unit tests in qemu yet, just for xt-run

paulstelian97 commented 4 years ago

Ah :)) Anyway, we'll see for it. Should I keep this open for now?

jajanusz commented 4 years ago

You may keep it open, but only people that have commercial xt-xcc toolchain for IMX can fix it - so I guess in this repo ATM these people are you and @dbaluta ;)

paulstelian97 commented 4 years ago

The two of us for now but also the other guys in NXP. But good point.

I think CI in general needs to be done as @dbaluta did mention the idea of actually having the possibility to allow on-device tests to be done internally and to push the results to Github.

dbaluta commented 4 years ago

True, only people inside NXP have access to the license server for using xt-xcc. This most likely will be solved with the on-device tests done internally. Paging @mihaix to put this on the list.