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

sof_remove.sh: Remove all ALSA modules #812

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

ujfalusi
Copy link
Contributor

Remove the remaining ALSA modules to have completely fresh start when
inserting the modules.
This way we can also test module parameters for the core.

Signed-off-by: Peter Ujfalusi [email protected]

@marc-hb
Copy link
Collaborator

marc-hb commented Dec 1, 2021

https://sof-ci.01.org/softestpr/PR812/build931/devicetest failed with rmmod: ERROR: Module snd_timer is in use by: snd_seq

@marc-hb
Copy link
Collaborator

marc-hb commented Dec 14, 2021

rmmod: ERROR: Module snd_timer is in use by: snd_seq

Ping @ujfalusi ?

Is this still something you'd like?

@ujfalusi
Copy link
Contributor Author

rmmod: ERROR: Module snd_timer is in use by: snd_seq

Ping @ujfalusi ?

Is this still something you'd like?

@marc-hb, yes, certainly, let me re-spin it.
Thanks for reminding me ;)

@ujfalusi
Copy link
Contributor Author

Changes since v1:

  • added more modules to be removed (snd_seq_midi, snd_seq_midi_event, snd_rawmidi, snd_seq, snd_seq_device, snd_usbmidi_lib)

If we still see failure then we might want to drop this as we can not list all modules we might encounter...

@marc-hb
Copy link
Collaborator

marc-hb commented Dec 15, 2021

https://sof-ci.01.org/softestpr/PR812/build959/devicetest/ is all green now.

If we still see failure then we might want to drop this as we can not list all modules we might encounter...

In that case can you please isolate all these into a new section with some "ALSA modules" title + this comment?

@ujfalusi
Copy link
Contributor Author

https://sof-ci.01.org/softestpr/PR812/build959/devicetest/ is all green now.

If we still see failure then we might want to drop this as we can not list all modules we might encounter...

In that case can you please isolate all these into a new section with some "ALSA modules" title + this comment?

It is passing now in CI so we are covering nicely the common modules, but if you have some new device spanning out new module then it will stop working as we are aiming to take down the whole audio subsystem now.

@greg-intel
Copy link
Contributor

Is there a new device checklist? We'd have to add an entry to this file to prevent failure if the new device brings a unique module with it? Or am I not understanding?

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

Is there a new device checklist? We'd have to add an entry to this file to prevent failure if the new device brings a unique module with it? Or am I not understanding?

Yes I think we just grow this list as needed. For me what's missing now is a very brief summary of this discussion in the code itself.

@marc-hb
Copy link
Collaborator

marc-hb commented Jan 12, 2022

SOFCI TEST

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 30, 2023

Unlikely conflict with unrelated 2ff9c0b, can you please rebase?

Also, please summarize the discussion above in the code.

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 15, 2024

@ujfalusi can you please:

  • rebase and fix the conflict
  • Add some very short comments summarizing the discussion above

Remove the remaining ALSA modules to have completely fresh start when
inserting the modules.
This way we can also test module parameters for the core.

Signed-off-by: Peter Ujfalusi <[email protected]>
@ujfalusi
Copy link
Contributor Author

Changes since v2:

  • rebased on main
  • 2.5 years passed ;)

@ujfalusi ujfalusi force-pushed the pr/kmod_remove_all_sound_01 branch from 6f7ba72 to 32603a8 Compare July 16, 2024 05:49
Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

LGTM but someone familiar with modules must approve. @plbossart , @bardliao maybe?

The LNL overruns in https://sof-ci.01.org/softestpr/PR812/build656/devicetest/index.html are interesting but unrelated. Everything else is either green or not available.

EDIT: overruns filed in new https://github.com/thesofproject/linux/issues/ 5109

@marc-hb marc-hb dismissed their stale review July 16, 2024 18:10

2.5 years old review :-)

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

LGTM

@marc-hb marc-hb merged commit df8e22e into thesofproject:main Jul 16, 2024
5 of 7 checks passed
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.

4 participants