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

Fix building of native loadable modules #8795

Closed
wants to merge 1 commit into from

Conversation

softwarecki
Copy link
Collaborator

This PR reverts changes in:
src/module/audio/sink_api.c
src/module/audio/source_api.c
made by commit 12d958a. These changes made it impossible to build native loadable modules.

This commit reverts changes in:
    	src/module/audio/sink_api.c
    src/module/audio/source_api.c
made by commit 12d958a. These changes made
it impossible to build native loadable modules.

Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
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.

Where does this fail? I'm good to merge, but I need to understand where the failure is seen. These files are part of the basefw build:

xt-nm ../build-tgl/zephyr/zephyr.elf |grep source_get_

So the dependency to rtos/symbol.h seems correct.

If there is some non-basefw build that also compilers these files, then the offending lines should be #ifdef'ed out but in the base SOF build, these are ok.

@pjdobrowolski
Copy link
Contributor

Native loadable modules are building without Zephyr and they are not finding zephyr headers under define.

Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

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

This breaks LLEXT

@pjdobrowolski
Copy link
Contributor

You are right. That is our fault that building native modules is not in CI to block such changes however it blocks us to add it now.

@kv2019i
Copy link
Collaborator

kv2019i commented Jan 25, 2024

@pjdobrowolski wrote:

Native loadable modules are building without Zephyr and they are not finding zephyr headers under define.

But this is not module code, this is basefw, which makes this confusing. If I build up-down-mixer with lmdk, these files dont' need to be compiled, right?

If this gets compiled with lmdk, toget forward, can you add a ifdef around these lines. ifdef LMDK build, do not export the symbols. Basefw should build and be free to use native Zephyr services.

@lyakh
Copy link
Collaborator

lyakh commented Jan 25, 2024

You are right. That is our fault that building native modules is not in CI to block such changes however it blocks us to add it now.

can you just put

#ifndef __SOF_MODULE_SERVICE_BUILD__

around those things and be done with it

@lgirdwood
Copy link
Member

Let me try and summarize - stand alone module build (without SOF base FW repo or Zephyr repo or maybe both ?) is failing as it has some undefined macros/functions ?
@softwarecki are you able to share the build output and build BKM.

@lyakh
Copy link
Collaborator

lyakh commented Jan 29, 2024

Let me try and summarize - stand alone module build (without SOF base FW repo or Zephyr repo or maybe both ?) is failing as it has some undefined macros/functions ? @softwarecki are you able to share the build output and build BKM.

@lgirdwood I don't think it depends on whether the build is stand-alone or with a base firmware build available. In fact I still think that isolated builds (not using a base FW build environment) aren't a good idea. But in any case, I think the reason here is simply, that "native system-service modules" link with these files, effectively they make a copy of them, in addition to those files being also linked with the base firmware.

@softwarecki
Copy link
Collaborator Author

@lyakh @kv2019i @lgirdwood This files are shared between basefw and lmdk modules. Its contains common helper functions to handle sink/source interface.

@softwarecki
Copy link
Collaborator Author

can you just put

#ifndef __SOF_MODULE_SERVICE_BUILD__

around those things and be done with it

@lyakh can you create PR with it?

@dbaluta
Copy link
Collaborator

dbaluta commented Jan 29, 2024

@softwarecki Can you please add better description to this patch. This needs to explain WHY previous patch needs to be reverted. What is the error and how does this patch fixes it.

@lyakh
Copy link
Collaborator

lyakh commented Jan 29, 2024

can you just put

#ifndef __SOF_MODULE_SERVICE_BUILD__

around those things and be done with it

@lyakh can you create PR with it?

#8812

@softwarecki softwarecki closed this Feb 2, 2024
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.

7 participants