-
Notifications
You must be signed in to change notification settings - Fork 318
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
Revert "ipc3: override type field once comp_driver found" #9518
Conversation
This reverts commit b53573a. This commit broke cmocka unit tests. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
@cujomalainey Fixing this needs more time. Fixing in testbench topology parser file component type to expected SOF_COMP_MODULE_ADAPTER causes the ipc3/helper.c comp_specific_builder() to fail to copy file init IPC. Then the garbage init crashes testbench. Fixing for cmocka the type similarly for fir, iir, works but not for mux/demux that operates incorrectly in test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cujomalainey it looks like we need another solution to catch invalid type before its goes to the next stage in processing.
@lyakh @singalsu could we add an extra bool param to get_drv()
that enforces the type check for those when its needed ?
This was why I was considering my other solution. Wasn't as correct but it would intheory allow a lot of previous mistakes to pass.
It's always needed, both the module adapter and the ipc layer parse the enum currently. |
Agreed, we have gotten ourselves into a bit of sticky situation here. |
I think I will have today fix for cmocka unit tests. There's an actual issue in module adapter version of mux that I need to fix as well. The mux mode becomes always demux when comp type is actually module adapter. Just fixing that wasn't enough and it still fails, trying to find out why. |
Unit tests fix is in #9529 |
@cujomalainey @dbaluta I'd need help from you. I'm trying to fix testbench to pass CI without this revert, and got file and process components to pass the added check from #9496. But I still have problem with other module adapter components with other than process type init IPC (version 3). e.g. volume. It in testbench registers as SOF_COMP_MODULE_ADAPTER. But init IPC from tplg_parse sets to the type to SOF_COMP_VOLUME, so the added check fails. It seems that Linux kernel also uses SOF_COMP_VOLUME in function sof_ipc3_widget_setup_comp_pga(). Have you been able to check if IPC3 with current kernel and current git main firmware build can load components like volume, src, mux, demux? |
I get the following error with latest SOF/main:
I have an older kernel - 6.6 |
Thanks! I think that kernel should be similar for IPC3. Have you checked if this revert fixes the issue? |
@lgirdwood @lyakh looks like we have a lot of history here that broke. Lets land the revert and see if we can get more coverage in a draft PR to see better the blast radius of this fix |
Sorry for the late replay, but yes, reverting the patch fixes the issue. |
This reverts commit b53573a. This commit broke cmocka unit tests.