[alsa-devel] [PATCH] ASoC: core: delete component->card_list in soc_remove_component only
From: Bard Liao yung-chuan.liao@linux.intel.com
We add component->card_list in the end of soc_probe_component(). In other words, component->card_list will not be added if there is an error in the soc_probe_component() function. So we can't delete component->card_list in the error handling of soc_probe_component().
Fixes: 22d1423187e5 ("ASoC: soc-core: add soc_cleanup_component()") Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- sound/soc/soc-core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 35f48e9c5ead..aff4b4bf4d07 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -978,7 +978,6 @@ static void soc_cleanup_component(struct snd_soc_component *component) /* For framework level robustness */ snd_soc_component_set_jack(component, NULL, NULL);
- list_del(&component->card_list); snd_soc_dapm_free(snd_soc_component_get_dapm(component)); soc_cleanup_component_debugfs(component); component->card = NULL; @@ -991,7 +990,7 @@ static void soc_remove_component(struct snd_soc_component *component) return;
snd_soc_component_remove(component); - + list_del(&component->card_list); soc_cleanup_component(component); }
On 9/16/19 4:03 PM, Bard liao wrote:
From: Bard Liao yung-chuan.liao@linux.intel.com
We add component->card_list in the end of soc_probe_component(). In other words, component->card_list will not be added if there is an error in the soc_probe_component() function. So we can't delete component->card_list in the error handling of soc_probe_component().
Fixes: 22d1423187e5 ("ASoC: soc-core: add soc_cleanup_component()") Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
I wish we had a way to do this on the SOF GitHub, it's painful that prior reviews and approvals are not tracked automagically.
sound/soc/soc-core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 35f48e9c5ead..aff4b4bf4d07 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -978,7 +978,6 @@ static void soc_cleanup_component(struct snd_soc_component *component) /* For framework level robustness */ snd_soc_component_set_jack(component, NULL, NULL);
- list_del(&component->card_list); snd_soc_dapm_free(snd_soc_component_get_dapm(component)); soc_cleanup_component_debugfs(component); component->card = NULL;
@@ -991,7 +990,7 @@ static void soc_remove_component(struct snd_soc_component *component) return;
snd_soc_component_remove(component);
- list_del(&component->card_list); soc_cleanup_component(component); }
The patch
ASoC: core: delete component->card_list in soc_remove_component only
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From a0a4bf57a977ed37bcbdfc8027a44485fe086a3d Mon Sep 17 00:00:00 2001
From: Bard Liao yung-chuan.liao@linux.intel.com Date: Tue, 17 Sep 2019 05:03:53 +0800 Subject: [PATCH] ASoC: core: delete component->card_list in soc_remove_component only
We add component->card_list in the end of soc_probe_component(). In other words, component->card_list will not be added if there is an error in the soc_probe_component() function. So we can't delete component->card_list in the error handling of soc_probe_component().
Fixes: 22d1423187e5 ("ASoC: soc-core: add soc_cleanup_component()") Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20190916210353.6318-1-yung-chuan.liao@linux.intel.... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 35f48e9c5ead..aff4b4bf4d07 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -978,7 +978,6 @@ static void soc_cleanup_component(struct snd_soc_component *component) /* For framework level robustness */ snd_soc_component_set_jack(component, NULL, NULL);
- list_del(&component->card_list); snd_soc_dapm_free(snd_soc_component_get_dapm(component)); soc_cleanup_component_debugfs(component); component->card = NULL; @@ -991,7 +990,7 @@ static void soc_remove_component(struct snd_soc_component *component) return;
snd_soc_component_remove(component); - + list_del(&component->card_list); soc_cleanup_component(component); }
Hi Bard
Thank you for your patch
From: Bard Liao yung-chuan.liao@linux.intel.com
We add component->card_list in the end of soc_probe_component(). In other words, component->card_list will not be added if there is an error in the soc_probe_component() function. So we can't delete component->card_list in the error handling of soc_probe_component().
Fixes: 22d1423187e5 ("ASoC: soc-core: add soc_cleanup_component()") Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com
Hmm... what happen if you get error at soc_probe_component() ? What does your "can't delete component->card_list" mean ? Kernel Oops or warning ?
I tried to create an error on purpose at soc_probe_component(), but, there was no kernel oops, no warning, etc. It just can't create sound card. It is very normal for me. Or, which kernel are you using ?
Thank you for your help !! Best regards --- Kuninori Morimoto
-----Original Message----- From: Kuninori Morimoto [mailto:kuninori.morimoto.gx@renesas.com] Sent: Wednesday, September 18, 2019 8:52 AM To: Bard liao yung-chuan.liao@linux.intel.com Cc: broonie@kernel.org; tiwai@suse.de; alsa-devel@alsa-project.org; pierre- louis.bossart@linux.intel.com; Liao, Bard bard.liao@intel.com Subject: Re: [PATCH] ASoC: core: delete component->card_list in soc_remove_component only
Hi Bard
Thank you for your patch
From: Bard Liao yung-chuan.liao@linux.intel.com
We add component->card_list in the end of soc_probe_component(). In other words, component->card_list will not be added if there is an error in the soc_probe_component() function. So we can't delete component->card_list in the error handling of soc_probe_component().
Fixes: 22d1423187e5 ("ASoC: soc-core: add soc_cleanup_component()") Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com
Hmm... what happen if you get error at soc_probe_component() ? What does your "can't delete component->card_list" mean ? Kernel Oops or warning ?
That is Kernel Oops, see the dmesg below. [ 84.180608] rt711 sdw:0:25d:711:0:1: ASoC: failed to probe component -517 [ 84.180653] sdw_rt711_rt1308_rt715 sdw_rt711_rt1308_rt715: ASoC: failed to instantiate card -517 [ 84.180701] sdw_rt711_rt1308_rt715 sdw_rt711_rt1308_rt715: snd_soc_register_card failed -517 ... [ 94.419962] rt711 sdw:0:25d:711:0:1: ASoC: failed to probe component -517 [ 94.419983] general protection fault: 0000 [#1] SMP PTI [ 94.419986] CPU: 6 PID: 119 Comm: kworker/6:1 Not tainted 5.3.0-rc7+ #821 [ 94.419988] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3042.A02.1901222005 01/22/2019 [ 94.419994] Workqueue: events deferred_probe_work_func [ 94.420012] RIP: 0010:soc_cleanup_component+0x1c/0x70 [snd_soc_core] [ 94.420015] Code: c3 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 53 31 d2 48 89 fb 31 f6 e8 03 bc 00 00 48 8b 43 58 48 8b 53 50 48 8d bb c0 00 00 00 <48> 89 42 08 48 89 10 48 b8 00 01 00 00 00 00 ad de 48 89 43 50 48 [ 94.420017] RSP: 0000:ffffa40340317c38 EFLAGS: 00010246 [ 94.420019] RAX: dead000000000122 RBX: ffff8fe22576ac28 RCX: 0000000000000006 [ 94.420023] RDX: dead000000000100 RSI: 0000000000000000 RDI: ffff8fe22576ace8 [ 94.420024] RBP: ffffffffc07430c0 R08: 0000000000000586 R09: 000000000000004c [ 94.420026] R10: ffffa40340317a68 R11: ffffa403403179a0 R12: ffff8fe22576ac20 [ 94.420027] R13: ffff8fe22576ace8 R14: ffff8fe22576ac90 R15: 0000000000000000 [ 94.420028] FS: 0000000000000000(0000) GS:ffff8fe227f80000(0000) knlGS:0000000000000000 [ 94.420030] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 94.420034] CR2: 00007ffdf5555c40 CR3: 000000028cd5c002 CR4: 0000000000760ee0 [ 94.420036] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 94.420037] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 94.420038] PKRU: 55555554 [ 94.420039] Call Trace: [ 94.420050] soc_probe_component+0x218/0x370 [snd_soc_core] [ 94.420059] snd_soc_instantiate_card+0x4da/0xd10 [snd_soc_core] [ 94.420064] ? devm_snd_soc_register_card+0x2e/0x80 [snd_soc_core] [ 94.420071] snd_soc_register_card+0x169/0x190 [snd_soc_core] [ 94.420076] devm_snd_soc_register_card+0x3e/0x80 [snd_soc_core] [ 94.420082] mc_probe+0x152/0x1ac [snd_soc_sdw_rt711_rt1308_rt715] [ 94.420087] ? acpi_dev_pm_attach+0x1b/0xa0 [ 94.420090] platform_drv_probe+0x32/0x90 [ 94.420095] really_probe+0xea/0x3d0 [ 94.420097] ? driver_allows_async_probing+0x50/0x50 [ 94.420100] driver_probe_device+0x10b/0x120 [ 94.420102] ? driver_allows_async_probing+0x50/0x50 [ 94.420107] bus_for_each_drv+0x61/0xa0 [ 94.420110] __device_attach+0xcf/0x150 [ 94.420113] bus_probe_device+0x82/0x90 [ 94.420115] deferred_probe_work_func+0x6f/0xc0 [ 94.420121] process_one_work+0x1e3/0x3d0 [ 94.420124] worker_thread+0x28/0x3c0 [ 94.420126] ? cancel_delayed_work+0x80/0x80 [ 94.420128] kthread+0x10e/0x130 [ 94.420132] ? kthread_park+0xa0/0xa0 [ 94.420135] ret_from_fork+0x35/0x40 [ 94.420138] Modules linked in: snd_soc_rt715 snd_soc_rt1308_sdw snd_soc_rt711 regmap_sdw soundwire_intel snd_soc_sdw_rt711_rt1308_rt715 soundwire_cadence snd_soc_hdac_hdmi snd_soc_dmic snd_sof_pci snd_sof_intel_hda_common snd_soc_hdac_hda snd_hda_codec soundwire_intel_init snd_intel_nhlt snd_sof_intel_hda snd_sof_intel_byt snd_soc_acpi_intel_match snd_soc_acpi snd_sof_intel_ipc snd_sof snd_sof_xtensa_dsp snd_hda_ext_core snd_hda_core snd_soc_core snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer snd soundcore ax88179_178a usbnet x86_pkg_temp_thermal intel_powerclamp intel_lpss_pci mei_me intel_lpss mfd_core mei efivarfs i915 i2c_algo_bit drm_kms_helper syscopyarea xhci_pci sysfillrect sysimgblt sdhci_pci fb_sys_fops cqhci sdhci xhci_hcd drm [ 94.420169] ---[ end trace 32434ffe9a1d6bab ]---
I tried to create an error on purpose at soc_probe_component(), but, there was no kernel oops, no warning, etc. It just can't create sound card. It is very normal for me. Or, which kernel are you using ?
Somehow I can only see the second attempt of component probe when it return -517 (EPROBE_DEFER) in the first attempt by using below kernel. https://github.com/plbossart/sound/commits/fix/soundwire-split-lookup-init
To me, the easiest way to see the issue is force return - EPROBE_DEFER on codec driver's probe function and call list_del(&component->card_list); before calling soc_cleanup_component(component); So list_del() will be called twice and you will see the issue.
Thank you for your help !! Best regards
Kuninori Morimoto
Hi Liao
Thank you for your feedback
I tried to create an error on purpose at soc_probe_component(), but, there was no kernel oops, no warning, etc. It just can't create sound card. It is very normal for me. Or, which kernel are you using ?
Somehow I can only see the second attempt of component probe when it return -517 (EPROBE_DEFER) in the first attempt by using below kernel. https://github.com/plbossart/sound/commits/fix/soundwire-split-lookup-init
To me, the easiest way to see the issue is force return - EPROBE_DEFER on codec driver's probe function and call list_del(&component->card_list); before calling soc_cleanup_component(component); So list_del() will be called twice and you will see the issue.
OK, I could reproduce your issue. And I think it will be solved if you can use list_del_init() instead of list_del() at soc_cleanup_component() ? (= without your patch)
- list_del() + list_del_init()
If possible, I want to cleanup all component related resource at soc_cleanup_component(). Because it is easy to read / understand.
Thank you for your help !! Best regards --- Kuninori Morimoto
-----Original Message----- From: Kuninori Morimoto [mailto:kuninori.morimoto.gx@renesas.com] Sent: Wednesday, September 18, 2019 12:06 PM To: Liao, Bard bard.liao@intel.com Cc: Bard liao yung-chuan.liao@linux.intel.com; broonie@kernel.org; tiwai@suse.de; alsa-devel@alsa-project.org; pierre- louis.bossart@linux.intel.com Subject: Re: [PATCH] ASoC: core: delete component->card_list in soc_remove_component only
Hi Liao
Thank you for your feedback
I tried to create an error on purpose at soc_probe_component(), but, there was no kernel oops, no warning, etc. It just can't create sound card. It is very normal for me. Or, which kernel are you using ?
Somehow I can only see the second attempt of component probe when it return -517 (EPROBE_DEFER) in the first attempt by using below kernel. https://github.com/plbossart/sound/commits/fix/soundwire-split-lookup- init
To me, the easiest way to see the issue is force return - EPROBE_DEFER on codec driver's probe function and call list_del(&component->card_list); before calling soc_cleanup_component(component); So list_del() will be called twice and you
will see the issue.
OK, I could reproduce your issue. And I think it will be solved if you can use list_del_init() instead of list_del() at soc_cleanup_component() ? (= without your patch)
- list_del()
- list_del_init()
If possible, I want to cleanup all component related resource at soc_cleanup_component(). Because it is easy to read / understand.
Thanks Morimoto san, I verified the solution works and agree with you. Will you send a patch to upstream?
Thank you for your help !! Best regards
Kuninori Morimoto
Hi Liao
Thank you for your feedback
To me, the easiest way to see the issue is force return - EPROBE_DEFER on codec driver's probe function and call list_del(&component->card_list); before calling soc_cleanup_component(component); So list_del() will be called twice and you
will see the issue.
OK, I could reproduce your issue. And I think it will be solved if you can use list_del_init() instead of list_del() at soc_cleanup_component() ? (= without your patch)
- list_del()
- list_del_init()
If possible, I want to cleanup all component related resource at soc_cleanup_component(). Because it is easy to read / understand.
Thanks Morimoto san, I verified the solution works and agree with you. Will you send a patch to upstream?
Good to know !! I'm happy if you can update it :)
Thank you for your help !! Best regards --- Kuninori Morimoto
-----Original Message----- From: Kuninori Morimoto [mailto:kuninori.morimoto.gx@renesas.com] Sent: Wednesday, September 18, 2019 1:46 PM To: Liao, Bard bard.liao@intel.com Cc: Bard liao yung-chuan.liao@linux.intel.com; broonie@kernel.org; tiwai@suse.de; alsa-devel@alsa-project.org; pierre- louis.bossart@linux.intel.com Subject: Re: [PATCH] ASoC: core: delete component->card_list in soc_remove_component only
Hi Liao
Thank you for your feedback
To me, the easiest way to see the issue is force return - EPROBE_DEFER on codec driver's probe function and call list_del(&component->card_list); before calling soc_cleanup_component(component); So list_del() will be called twice and you
will see the issue.
OK, I could reproduce your issue. And I think it will be solved if you can use list_del_init() instead of list_del() at soc_cleanup_component() ? (= without your patch)
- list_del()
- list_del_init()
If possible, I want to cleanup all component related resource at soc_cleanup_component(). Because it is easy to read / understand.
Thanks Morimoto san, I verified the solution works and agree with you. Will you send a patch to upstream?
Good to know !! I'm happy if you can update it :)
The original patch has been applied by Mark. Should I send a patch on top of the original patch or overwrite the original one?
Thank you for your help !! Best regards
Kuninori Morimoto
On Wed, Sep 18, 2019 at 06:13:54AM +0000, Liao, Bard wrote:
The original patch has been applied by Mark. Should I send a patch on top of the original patch or overwrite the original one?
Send an incremental patch on top of the one that's already applied please.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Wednesday, September 18, 2019 8:08 PM To: Liao, Bard bard.liao@intel.com Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com; Bard liao <yung- chuan.liao@linux.intel.com>; tiwai@suse.de; alsa-devel@alsa-project.org; pierre-louis.bossart@linux.intel.com Subject: Re: [PATCH] ASoC: core: delete component->card_list in soc_remove_component only
On Wed, Sep 18, 2019 at 06:13:54AM +0000, Liao, Bard wrote:
The original patch has been applied by Mark. Should I send a patch on top of the original patch or overwrite the original one?
Send an incremental patch on top of the one that's already applied please.
Thanks Mark for the instruction.
Hi Morimoto-san,
May I use your signed-off as first author since that is your idea? :)
On Wed, Sep 18, 2019 at 12:31:10PM +0000, Liao, Bard wrote:
May I use your signed-off as first author since that is your idea? :)
Suggested-by also works for situations like this (and is a bit better if you didn't get sent an actual patch).
Hi Liao, Mark
May I use your signed-off as first author since that is your idea? :)
Suggested-by also works for situations like this (and is a bit better if you didn't get sent an actual patch).
Everything is OK for me.
Thank you for your help !! Best regards --- Kuninori Morimoto
participants (5)
-
Bard liao -
Kuninori Morimoto -
Liao, Bard -
Mark Brown -
Pierre-Louis Bossart