[PATCH 5.16 regression fix 1/2] ASoC: Intel: soc-acpi-byt: Revert shrink tables using compatible IDs
Commit dac7cbd55dca ("ASoC: Intel: soc-acpi-byt: shrink tables using compatible IDs") simplified the match tables in soc-acpi-intel-byt-match.c by merging identical entries using the new .comp_ids snd_soc_acpi_mach field to point a single entry to multiple ACPI HIDs and clearing the previously unique per entry .id field.
But various machine drivers from sound/soc/intel/boards rely on mach->id in one or more ways. For example all of the following machine-drivers for entries combined during the shrinking: sound/soc/intel/boards/bytcr_rt5640.c sound/soc/intel/boards/bytcr_wm5102.c sound/soc/intel/boards/bytcht_da7213.c sound/soc/intel/boards/cht_bsw_rt5645.c
Do: adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1);
Which now no longer works and some of them also do:
pkg_found = snd_soc_acpi_find_package_from_hid(mach->id, ...
Which now also no longer works. All these calls need to be fixed before we can shrink the tables, so revert this change for now.
Fixes: dac7cbd55dca ("ASoC: Intel: soc-acpi-byt: shrink tables using compatible IDs") Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Brent Lu brent.lu@intel.com Signed-off-by: Hans de Goede hdegoede@redhat.com --- .../intel/common/soc-acpi-intel-byt-match.c | 68 ++++++++++++------- 1 file changed, 44 insertions(+), 24 deletions(-)
diff --git a/sound/soc/intel/common/soc-acpi-intel-byt-match.c b/sound/soc/intel/common/soc-acpi-intel-byt-match.c index 142000991813..510a5f38b7f1 100644 --- a/sound/soc/intel/common/soc-acpi-intel-byt-match.c +++ b/sound/soc/intel/common/soc-acpi-intel-byt-match.c @@ -120,29 +120,9 @@ static struct snd_soc_acpi_mach *byt_quirk(void *arg) } }
-static const struct snd_soc_acpi_codecs rt5640_comp_ids = { - .num_codecs = 3, - .codecs = { "10EC5640", "10EC5642", "INTCCFFD"}, -}; - -static const struct snd_soc_acpi_codecs wm5102_comp_ids = { - .num_codecs = 2, - .codecs = { "WM510204", "WM510205"}, -}; - -static const struct snd_soc_acpi_codecs da7213_comp_ids = { - .num_codecs = 2, - .codecs = { "DGLS7212", "DGLS7213"}, -}; - -static const struct snd_soc_acpi_codecs rt5645_comp_ids = { - .num_codecs = 2, - .codecs = { "10EC5645", "10EC5648"}, -}; - struct snd_soc_acpi_mach snd_soc_acpi_intel_baytrail_machines[] = { { - .comp_ids = &rt5640_comp_ids, + .id = "10EC5640", .drv_name = "bytcr_rt5640", .fw_filename = "intel/fw_sst_0f28.bin", .board = "bytcr_rt5640", @@ -150,6 +130,22 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_baytrail_machines[] = { .sof_fw_filename = "sof-byt.ri", .sof_tplg_filename = "sof-byt-rt5640.tplg", }, + { + .id = "10EC5642", + .drv_name = "bytcr_rt5640", + .fw_filename = "intel/fw_sst_0f28.bin", + .board = "bytcr_rt5640", + .sof_fw_filename = "sof-byt.ri", + .sof_tplg_filename = "sof-byt-rt5640.tplg", + }, + { + .id = "INTCCFFD", + .drv_name = "bytcr_rt5640", + .fw_filename = "intel/fw_sst_0f28.bin", + .board = "bytcr_rt5640", + .sof_fw_filename = "sof-byt.ri", + .sof_tplg_filename = "sof-byt-rt5640.tplg", + }, { .id = "10EC5651", .drv_name = "bytcr_rt5651", @@ -159,7 +155,7 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_baytrail_machines[] = { .sof_tplg_filename = "sof-byt-rt5651.tplg", }, { - .comp_ids = &wm5102_comp_ids, + .id = "WM510204", .drv_name = "bytcr_wm5102", .fw_filename = "intel/fw_sst_0f28.bin", .board = "bytcr_wm5102", @@ -167,7 +163,23 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_baytrail_machines[] = { .sof_tplg_filename = "sof-byt-wm5102.tplg", }, { - .comp_ids = &da7213_comp_ids, + .id = "WM510205", + .drv_name = "bytcr_wm5102", + .fw_filename = "intel/fw_sst_0f28.bin", + .board = "bytcr_wm5102", + .sof_fw_filename = "sof-byt.ri", + .sof_tplg_filename = "sof-byt-wm5102.tplg", + }, + { + .id = "DLGS7212", + .drv_name = "bytcht_da7213", + .fw_filename = "intel/fw_sst_0f28.bin", + .board = "bytcht_da7213", + .sof_fw_filename = "sof-byt.ri", + .sof_tplg_filename = "sof-byt-da7213.tplg", + }, + { + .id = "DLGS7213", .drv_name = "bytcht_da7213", .fw_filename = "intel/fw_sst_0f28.bin", .board = "bytcht_da7213", @@ -190,7 +202,15 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_baytrail_machines[] = { }, /* some Baytrail platforms rely on RT5645, use CHT machine driver */ { - .comp_ids = &rt5645_comp_ids, + .id = "10EC5645", + .drv_name = "cht-bsw-rt5645", + .fw_filename = "intel/fw_sst_0f28.bin", + .board = "cht-bsw", + .sof_fw_filename = "sof-byt.ri", + .sof_tplg_filename = "sof-byt-rt5645.tplg", + }, + { + .id = "10EC5648", .drv_name = "cht-bsw-rt5645", .fw_filename = "intel/fw_sst_0f28.bin", .board = "cht-bsw",
Commit 959ae8215a9e ("ASoC: Intel: soc-acpi-cht: shrink tables using compatible IDs") simplified the match tables in soc-acpi-intel-cht-match.c by merging identical entries using the new .comp_ids snd_soc_acpi_mach field to point a single entry to multiple ACPI HIDs and clearing the previously unique per entry .id field.
But various machine drivers from sound/soc/intel/boards rely on mach->id in one or more ways. For example all of the following machine-drivers for entries combined during the shrinking: sound/soc/intel/boards/bytcr_rt5640.c sound/soc/intel/boards/cht_bsw_rt5645.c sound/soc/intel/boards/bytcht_da7213.c
Do: adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1);
Which now no longer works and some of them also do:
pkg_found = snd_soc_acpi_find_package_from_hid(mach->id, ...
if (!strncmp(snd_soc_cards[i].codec_id, mach->id, 8)) { ...
Which now also no longer works. All these calls need to be fixed before we can shrink the tables, so revert this change for now.
Fixes: 959ae8215a9e ("ASoC: Intel: soc-acpi-cht: shrink tables using compatible IDs") Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Brent Lu brent.lu@intel.com Signed-off-by: Hans de Goede hdegoede@redhat.com --- .../intel/common/soc-acpi-intel-cht-match.c | 69 ++++++++++++------- 1 file changed, 44 insertions(+), 25 deletions(-)
diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c index c60a5e8e7bc9..227424236fd5 100644 --- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c +++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c @@ -51,31 +51,18 @@ static struct snd_soc_acpi_mach *cht_quirk(void *arg) return mach; }
-static const struct snd_soc_acpi_codecs rt5640_comp_ids = { - .num_codecs = 2, - .codecs = { "10EC5640", "10EC3276" }, -}; - -static const struct snd_soc_acpi_codecs rt5670_comp_ids = { - .num_codecs = 2, - .codecs = { "10EC5670", "10EC5672" }, -}; - -static const struct snd_soc_acpi_codecs rt5645_comp_ids = { - .num_codecs = 3, - .codecs = { "10EC5645", "10EC5650", "10EC3270" }, -}; - -static const struct snd_soc_acpi_codecs da7213_comp_ids = { - .num_codecs = 2, - .codecs = { "DGLS7212", "DGLS7213"}, - -}; - /* Cherryview-based platforms: CherryTrail and Braswell */ struct snd_soc_acpi_mach snd_soc_acpi_intel_cherrytrail_machines[] = { { - .comp_ids = &rt5670_comp_ids, + .id = "10EC5670", + .drv_name = "cht-bsw-rt5672", + .fw_filename = "intel/fw_sst_22a8.bin", + .board = "cht-bsw", + .sof_fw_filename = "sof-cht.ri", + .sof_tplg_filename = "sof-cht-rt5670.tplg", + }, + { + .id = "10EC5672", .drv_name = "cht-bsw-rt5672", .fw_filename = "intel/fw_sst_22a8.bin", .board = "cht-bsw", @@ -83,7 +70,23 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_cherrytrail_machines[] = { .sof_tplg_filename = "sof-cht-rt5670.tplg", }, { - .comp_ids = &rt5645_comp_ids, + .id = "10EC5645", + .drv_name = "cht-bsw-rt5645", + .fw_filename = "intel/fw_sst_22a8.bin", + .board = "cht-bsw", + .sof_fw_filename = "sof-cht.ri", + .sof_tplg_filename = "sof-cht-rt5645.tplg", + }, + { + .id = "10EC5650", + .drv_name = "cht-bsw-rt5645", + .fw_filename = "intel/fw_sst_22a8.bin", + .board = "cht-bsw", + .sof_fw_filename = "sof-cht.ri", + .sof_tplg_filename = "sof-cht-rt5645.tplg", + }, + { + .id = "10EC3270", .drv_name = "cht-bsw-rt5645", .fw_filename = "intel/fw_sst_22a8.bin", .board = "cht-bsw", @@ -107,7 +110,15 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_cherrytrail_machines[] = { .sof_tplg_filename = "sof-cht-nau8824.tplg", }, { - .comp_ids = &da7213_comp_ids, + .id = "DLGS7212", + .drv_name = "bytcht_da7213", + .fw_filename = "intel/fw_sst_22a8.bin", + .board = "bytcht_da7213", + .sof_fw_filename = "sof-cht.ri", + .sof_tplg_filename = "sof-cht-da7213.tplg", + }, + { + .id = "DLGS7213", .drv_name = "bytcht_da7213", .fw_filename = "intel/fw_sst_22a8.bin", .board = "bytcht_da7213", @@ -124,7 +135,7 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_cherrytrail_machines[] = { }, /* some CHT-T platforms rely on RT5640, use Baytrail machine driver */ { - .comp_ids = &rt5640_comp_ids, + .id = "10EC5640", .drv_name = "bytcr_rt5640", .fw_filename = "intel/fw_sst_22a8.bin", .board = "bytcr_rt5640", @@ -132,6 +143,14 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_cherrytrail_machines[] = { .sof_fw_filename = "sof-cht.ri", .sof_tplg_filename = "sof-cht-rt5640.tplg", }, + { + .id = "10EC3276", + .drv_name = "bytcr_rt5640", + .fw_filename = "intel/fw_sst_22a8.bin", + .board = "bytcr_rt5640", + .sof_fw_filename = "sof-cht.ri", + .sof_tplg_filename = "sof-cht-rt5640.tplg", + }, { .id = "10EC5682", .drv_name = "sof_rt5682",
On 11/17/21 9:10 AM, Hans de Goede wrote:
Commit 959ae8215a9e ("ASoC: Intel: soc-acpi-cht: shrink tables using compatible IDs") simplified the match tables in soc-acpi-intel-cht-match.c by merging identical entries using the new .comp_ids snd_soc_acpi_mach field to point a single entry to multiple ACPI HIDs and clearing the previously unique per entry .id field.
But various machine drivers from sound/soc/intel/boards rely on mach->id in one or more ways. For example all of the following machine-drivers for entries combined during the shrinking: sound/soc/intel/boards/bytcr_rt5640.c sound/soc/intel/boards/cht_bsw_rt5645.c sound/soc/intel/boards/bytcht_da7213.c
Do: adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1);
Which now no longer works and some of them also do:
pkg_found = snd_soc_acpi_find_package_from_hid(mach->id, ...
if (!strncmp(snd_soc_cards[i].codec_id, mach->id, 8)) { ...
Which now also no longer works. All these calls need to be fixed before we can shrink the tables, so revert this change for now.
Fixes: 959ae8215a9e ("ASoC: Intel: soc-acpi-cht: shrink tables using compatible IDs") Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Brent Lu brent.lu@intel.com Signed-off-by: Hans de Goede hdegoede@redhat.com
Thanks for the patch, it's embarrassing. I must have tested the wrong code or the wrong device...
Could we alternatively keep these tables and just copy the information using something like this (compile-tested only)
diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h index 31f4c4f9aeea..ac0893df9c76 100644 --- a/include/sound/soc-acpi.h +++ b/include/sound/soc-acpi.h @@ -147,7 +147,7 @@ struct snd_soc_acpi_link_adr { */ /* Descriptor for SST ASoC machine driver */ struct snd_soc_acpi_mach { - const u8 id[ACPI_ID_LEN]; + u8 id[ACPI_ID_LEN]; const struct snd_soc_acpi_codecs *comp_ids; const u32 link_mask; const struct snd_soc_acpi_link_adr *links; diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c index 2ae99b49d3f5..6b9dfa0608a3 100644 --- a/sound/soc/soc-acpi.c +++ b/sound/soc/soc-acpi.c @@ -20,8 +20,10 @@ static bool snd_soc_acpi_id_present(struct snd_soc_acpi_mach *machine)
if (comp_ids) { for (i = 0; i < comp_ids->num_codecs; i++) { - if (acpi_dev_present(comp_ids->codecs[i], NULL, -1)) + if (acpi_dev_present(comp_ids->codecs[i], NULL, -1)) { + strncpy(machine->id, comp_ids->codecs[i], ACPI_ID_LEN); return true; + } } }
Hi,
On 11/17/21 16:54, Pierre-Louis Bossart wrote:
On 11/17/21 9:10 AM, Hans de Goede wrote:
Commit 959ae8215a9e ("ASoC: Intel: soc-acpi-cht: shrink tables using compatible IDs") simplified the match tables in soc-acpi-intel-cht-match.c by merging identical entries using the new .comp_ids snd_soc_acpi_mach field to point a single entry to multiple ACPI HIDs and clearing the previously unique per entry .id field.
But various machine drivers from sound/soc/intel/boards rely on mach->id in one or more ways. For example all of the following machine-drivers for entries combined during the shrinking: sound/soc/intel/boards/bytcr_rt5640.c sound/soc/intel/boards/cht_bsw_rt5645.c sound/soc/intel/boards/bytcht_da7213.c
Do: adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1);
Which now no longer works and some of them also do:
pkg_found = snd_soc_acpi_find_package_from_hid(mach->id, ...
if (!strncmp(snd_soc_cards[i].codec_id, mach->id, 8)) { ...
Which now also no longer works. All these calls need to be fixed before we can shrink the tables, so revert this change for now.
Fixes: 959ae8215a9e ("ASoC: Intel: soc-acpi-cht: shrink tables using compatible IDs") Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Brent Lu brent.lu@intel.com Signed-off-by: Hans de Goede hdegoede@redhat.com
Thanks for the patch, it's embarrassing. I must have tested the wrong code or the wrong device...
No worries, we all make mistakes.
Could we alternatively keep these tables and just copy the information using something like this (compile-tested only)
Interesting I was thinking along these lines myself as an alternate fix, but I expected all the struct snd_soc_acpi_mach declarations, e.g. the snd_soc_acpi_intel_baytrail_machines array, to be const.
But I see now that these are not const, at least not for byt + cht.
So yes this should work and seems a bit nicer fix then reverting.
I'll give this a test run when I'm done fixing some other 5.16 regressions I'm working on ...
Regards,
Hans
diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h index 31f4c4f9aeea..ac0893df9c76 100644 --- a/include/sound/soc-acpi.h +++ b/include/sound/soc-acpi.h @@ -147,7 +147,7 @@ struct snd_soc_acpi_link_adr { */ /* Descriptor for SST ASoC machine driver */ struct snd_soc_acpi_mach {
const u8 id[ACPI_ID_LEN];
u8 id[ACPI_ID_LEN]; const struct snd_soc_acpi_codecs *comp_ids; const u32 link_mask; const struct snd_soc_acpi_link_adr *links;diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c index 2ae99b49d3f5..6b9dfa0608a3 100644 --- a/sound/soc/soc-acpi.c +++ b/sound/soc/soc-acpi.c @@ -20,8 +20,10 @@ static bool snd_soc_acpi_id_present(struct snd_soc_acpi_mach *machine)
if (comp_ids) { for (i = 0; i < comp_ids->num_codecs; i++) {
if (acpi_dev_present(comp_ids->codecs[i], NULL, -1))
if (acpi_dev_present(comp_ids->codecs[i], NULL,-1)) {
strncpy(machine->id,comp_ids->codecs[i], ACPI_ID_LEN); return true;
} } }
participants (2)
-
Hans de Goede -
Pierre-Louis Bossart