[PATCH 5.16 regression fix 2/2] ASoC: Intel: soc-acpi-cht: Revert shrink tables using compatible IDs

[Hans de Goede]

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 at linux.intel.com>
>> Cc: Brent Lu <brent.lu at intel.com>
>> Signed-off-by: Hans de Goede <hdegoede at 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;
> +                       }
>                 }
>         }
>