Skip to content

kmod: sof_remove: add snd_soc_sof_da7219 #1107

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.

Sign up for GitHub

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

Merged
marc-hb merged 4 commits into from
Apr 11, 2024
Merged

kmod: sof_remove: add snd_soc_sof_da7219 #1107

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f93eb56
kmod: sof_remove: add snd_soc_sof_da7219
brentlu Apr 10, 2024
2ab6bd8
kmod: sof_remove: add snd_soc_intel_sof_nuvoton_common
brentlu Apr 10, 2024
689497d
kmod: sof_remove: add snd_soc_intel_sof_cirrus_common
brentlu Apr 10, 2024
9aeccab
kmod: fix acpi-intel-match dependency
brentlu Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion tools/kmod/sof_remove.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ remove_module snd_sof_intel_hda_common
remove_module snd_sof_intel_hda
remove_module snd_sof_intel_ipc
remove_module snd_sof_xtensa_dsp
remove_module snd_soc_acpi_intel_match

#-------------------------------------------
# Helpers
Expand All @@ -152,6 +151,7 @@ remove_module imx_common
#-------------------------------------------
remove_module snd_soc_sof_rt5682
remove_module snd_soc_sof_da7219_max98373
remove_module snd_soc_sof_da7219

@plbossart plbossart Sep 25, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you want to keep the old name for backwards compatibility.
Nothing bad will happen if you remove a nonexistent device. Bad things may happen if you keep a module loaded.

@marc-hb marc-hb Apr 10, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add a very short comment explaining which name changed.

@brentlu brentlu Apr 10, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

to the commit message or to the script?

@plbossart plbossart Apr 10, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

commit message is fine.

@marc-hb marc-hb Apr 10, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When looking at confusing code, few people think about looking at past commit messages (which are sometimes hard to find because of future whitespace changes or other formatting). Even fewer people in a repo with test scripts where commit messages tend to be lower quality.

Hopefully no one will ever have to make sense of these two lines.

@plbossart plbossart Apr 10, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's fine really, there's no need to make the script too detailed to track all the code moves in various parts of the kernel. The updated code supports old and new names, that's already very good.

@marc-hb marc-hb Apr 10, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually, the commit title is still "kmod: sof_remove: add snd_soc_sof_da7219" even when it's not removed anymore!

f93eb56

I don't mind that much if the comments are not in my preferred place. I do mind if the commit title says the opposite of what it does; please fix at least that.

@brentlu brentlu Apr 10, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It seems to me that there is something wrong with Github's GUI.

brentlu@brentlu-desktop:/workspace/sof/my_sof_test_0$ git log -p f93eb56
commit f93eb561a03c58d5055e58b8cd51bec983085a3e
Author: Brent Lu <brent.lu@intel.com>
Date:   Wed Apr 10 17:26:00 2024 +0800

    kmod: sof_remove: add snd_soc_sof_da7219

    Machine driver sof_da7219_max98373 has been renamed as
    snd_soc_sof_da7219. Add the new module to the script and keep the old
    one for backward compatibility.

    Signed-off-by: Brent Lu <brent.lu@intel.com>

diff --git a/tools/kmod/sof_remove.sh b/tools/kmod/sof_remove.sh
index 4907101..b0f4fb6 100755
--- a/tools/kmod/sof_remove.sh
+++ b/tools/kmod/sof_remove.sh
@@ -152,6 +152,7 @@ remove_module imx_common
 #-------------------------------------------
 remove_module snd_soc_sof_rt5682
 remove_module snd_soc_sof_da7219_max98373
+remove_module snd_soc_sof_da7219
 remove_module snd_soc_sst_bdw_rt5677_mach
 remove_module snd_soc_bdw_rt286
 remove_module snd_soc_sst_broadwell

remove_module snd_soc_sst_bdw_rt5677_mach
remove_module snd_soc_bdw_rt286
remove_module snd_soc_sst_broadwell
Expand All @@ -176,8 +176,11 @@ remove_module snd_soc_sof_sdw
remove_module snd_soc_sof_es8336
remove_module snd_soc_ehl_rt5660
remove_module snd_soc_intel_sof_board_helpers
remove_module snd_soc_acpi_intel_match
remove_module snd_soc_intel_hda_dsp_common
remove_module snd_soc_intel_sof_cirrus_common
remove_module snd_soc_intel_sof_maxim_common
remove_module snd_soc_intel_sof_nuvoton_common
remove_module snd_soc_intel_sof_realtek_common

#-------------------------------------------
Expand Down