ASoC: acpi: fix: continue searching when machine is ignored
PATCH 1/1 ASoC: acpi: fix: continue searching when machine is ignored
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.
--- sound/soc/soc-acpi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
sound/soc/soc-acpi.c +4 -1
@@ -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 ;
Signature · 1 line
2.17.1
Quoted 3 lines · Keyon Jie
The
machine_quirkmay 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...
Quoted 22 lines
Signed-off-by Keyon Jie <[email protected]>--- sound/soc/soc-acpi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)sound/soc/soc-acpi.c +4
-1@@ -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 ;
Hi Keyon,
Quoted 7 lines
Quoted 3 lines
The
machine_quirkmay 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...
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).
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.
Thanks
-Pierre
Quoted 20 lines
Quoted 20 lines
Signed-off-by Keyon Jie <[email protected]>---
sound/soc/soc-acpi.c| 5 ++++-1 file changed, 4 insertions(+), 1 deletion(-)sound/soc/soc-acpi.c +3
-1@@ -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)
Quoted 8 lines
Quoted 4 lines
sound/soc/soc-acpi.c +1
-0+ return mach; } }return NULL;
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Quoted 6 lines · Pierre-Louis Bossart
Hi Keyon,
Quoted 4 lines
Quoted 3 lines
The
machine_quirkmay 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.
Quoted 7 lines
Quoted 3 lines
Nice catch, this was missed in the other patchset since the ACPI ID to ignore was the last in the table...
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:
sound/soc/soc-acpi.c +13 -2
@@ -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; } }
Quoted 2 lines
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
Quoted 33 lines
Thanks
-Pierre
Quoted 20 lines
Quoted 20 lines
Signed-off-by Keyon Jie <[email protected]>---
sound/soc/soc-acpi.c| 5 ++++-1 file changed, 4 insertions(+), 1 deletion(-)sound/soc/soc-acpi.c +3
-1@@ -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)Quoted 8 lines
Quoted 4 lines
sound/soc/soc-acpi.c +1
-0+ return mach; } }return NULL;
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Quoted 4 lines
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Quoted 41 lines · Keyon Jie
Quoted 6 lines · Pierre-Louis Bossart
Hi Keyon,
Quoted 4 lines
Quoted 3 lines
The
machine_quirkmay 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.Quoted 7 lines
Quoted 3 lines
Nice catch, this was missed in the other patchset since the ACPI ID to ignore was the last in the table...
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:
sound/soc/soc-acpi.c +9
-2@@ -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
Quoted 5 lines
machine */
+ returnquirk_mach; + }
+
+ /* return acpi matched machine directly */
Yes, we tried something similar, see https://github.com/plbossart/sound/commit/9acfd674141bfc0a0bdadda1c3f7b16cc61047de
Quoted 6 lines
return mach;
}
}Quoted 1 line
Quoted 44 lines
Quoted 1 line
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,
~KeyonQuoted 37 lines
Thanks
-Pierre
Quoted 20 lines
Quoted 20 lines
Signed-off-by Keyon Jie <[email protected]>---
sound/soc/soc-acpi.c| 5 ++++-1 file changed, 4 insertions(+), 1 deletion(-)sound/soc/soc-acpi.c +3
-1@@ -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)Quoted 8 lines
Quoted 4 lines
sound/soc/soc-acpi.c +1
-0+ return mach; } }return NULL;
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel