[alsa-devel] [PATCH] ASoC: acpi: fix: continue searching when machine is ignored

[Keyon Jie]

On 2018/11/16 上午7:01, Pierre-Louis Bossart wrote:
> Hi Keyon,
> 
>>> The machine_quirk may return NULL which means the acpi entries should be
>>> skipped and search for next matched entry is needed, here add return
>>> check here and continue for NULL case.
>>
>> Nice catch, this was missed in the other patchset since the ACPI ID to 
>> ignore was the last in the table...
>>
>> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> 
> I was wondering if this patch was merged, but actually it's good Mark 
> wasn't copied on this one: the code doesn't quite work as intended and 
> shouldn't be merged as is.
> 
> Chintan and I see an oops and reboot on KBL Chromebooks, when the quirk 
> returns NULL, the mach variable becomes NULL and cannot be modified with 
> pointer arithmetic (which doesn't show with the diff format).

Ah, yeap, I omitted this, so we may need introduce a new pointer to fix 
that, something like this:

diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c
index b8e72b52db30..0d967d0ca222 100644
--- a/sound/soc/soc-acpi.c
+++ b/sound/soc/soc-acpi.c
@@ -10,11 +10,22 @@ struct snd_soc_acpi_mach *
  snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
  {
         struct snd_soc_acpi_mach *mach;
+       struct snd_soc_acpi_mach *quirk_mach = NULL;

         for (mach = machines; mach->id[0]; mach++) {
                 if (acpi_dev_present(mach->id, NULL, -1)) {
-                       if (mach->machine_quirk)
-                               mach = mach->machine_quirk(mach);
+                       if (mach->machine_quirk) {
+                               quirk_mach = mach->machine_quirk(mach);
+
+                               if (!quirk_mach)
+                                       /* skip if no quirk matched */
+                                       continue;
+                               else
+                                       /* return quirk matched machine */
+                                       return quirk_mach;
+                       }
+
+                       /* return acpi matched machine directly */
                         return mach;
                 }
         }


> 
> It's very likely btw that anyone trying a recent kernel on these devices 
> would not see the audio driver probe at all, so we do want to try other 
> entries - but of course without this nasty side effect. I will submit a 
> v2 when I've sorted out other problems on Chrome.

That's fine.

Thanks,
~Keyon

> 
> Thanks
> 
> -Pierre
> 
>>
>>>
>>> Signed-off-by: Keyon Jie <yang.jie at linux.intel.com>
>>> ---
>>>   sound/soc/soc-acpi.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c
>>> index b8e72b52db30..faf8941363e4 100644
>>> --- a/sound/soc/soc-acpi.c
>>> +++ b/sound/soc/soc-acpi.c
>>> @@ -15,7 +15,10 @@ snd_soc_acpi_find_machine(struct snd_soc_acpi_mach 
>>> *machines)
>>>           if (acpi_dev_present(mach->id, NULL, -1)) {
>>>               if (mach->machine_quirk)
>>>                   mach = mach->machine_quirk(mach);
>>> -            return mach;
>>> +
>>> +            /* return matched machine, continue otherwise */
>>> +            if (mach)
>>> +                return mach;
>>>           }
>>>       }
>>>       return NULL;
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel at alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel