[alsa-devel] [PATCH resend 00/11] ASoC: soc-core cleanup repost
Hi Mark
I posted soc-core cleanup patches, almost all are already accepted, and I got response/review for some patches.
I adjusted to not-yet-accepted patches
Kuninori Morimoto (11): ASoC: soc-core: use device_register() ASoC: soc-core: merge snd_soc_initialize_card_lists() ASoC: soc-core: remove unneeded dai_link check from snd_soc_remove_dai_link() ASoC: soc-core: add NOTE to snd_soc_rtdcom_lookup() ASoC: soc-core: don't alloc memory carelessly when creating debugfs ASoC: soc-core: soc_cleanup_card_resources() become void ASoC: soc-core: initialize component list ASoC: soc-core: initialize list at one place ASoC: soc-dai: use bit field for bus_control ASoC: soc-topology: use for_each_component_dais() at remove_dai() ASoC: soc-core: add soc_unbind_aux_dev()
include/sound/soc-dai.h | 3 +- include/sound/soc.h | 10 ------- sound/soc/soc-core.c | 77 +++++++++++++++++++++++++----------------------- sound/soc/soc-topology.c | 2 +- 4 files changed, 42 insertions(+), 50 deletions(-)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
It is easy to read code if it is cleanly using paired function/naming, like start <-> stop, register <-> unregister, etc, etc. But, current ALSA SoC code is very random, unbalance, not paired, etc.
soc-core.c is using device_unregiser(), but there is no its paired device_regiser(). We can find its code at soc_post_component_init() which is using device_initialize() and device_add(). Here, device_initialize() + device_add() = device_register().
-- linux/drivers/base/core.c -- int device_register(struct device *dev) { device_initialize(dev); return device_add(dev); }
device_initialize() is doing each dev member's initialization only, not related to device parent/release/groups. Thus, we can postpone it. let's use device_register() instead of device_initialize()/device_add().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
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 b3f820f..b3d02e7 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1354,7 +1354,6 @@ static int soc_post_component_init(struct snd_soc_pcm_runtime *rtd, rtd->dev = kzalloc(sizeof(struct device), GFP_KERNEL); if (!rtd->dev) return -ENOMEM; - device_initialize(rtd->dev); rtd->dev->parent = rtd->card->dev; rtd->dev->release = rtd_release; rtd->dev->groups = soc_dev_attr_groups; @@ -1364,7 +1363,7 @@ static int soc_post_component_init(struct snd_soc_pcm_runtime *rtd, INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].be_clients); INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_PLAYBACK].fe_clients); INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].fe_clients); - ret = device_add(rtd->dev); + ret = device_register(rtd->dev); if (ret < 0) { /* calling put_device() here to free the rtd->dev */ put_device(rtd->dev);
The patch
ASoC: soc-core: use device_register()
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 4168ddabb480bef818c93f378428632fb681b500 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:04:49 +0900 Subject: [PATCH] ASoC: soc-core: use device_register()
It is easy to read code if it is cleanly using paired function/naming, like start <-> stop, register <-> unregister, etc, etc. But, current ALSA SoC code is very random, unbalance, not paired, etc.
soc-core.c is using device_unregiser(), but there is no its paired device_regiser(). We can find its code at soc_post_component_init() which is using device_initialize() and device_add(). Here, device_initialize() + device_add() = device_register().
-- linux/drivers/base/core.c -- int device_register(struct device *dev) { device_initialize(dev); return device_add(dev); }
device_initialize() is doing each dev member's initialization only, not related to device parent/release/groups. Thus, we can postpone it. let's use device_register() instead of device_initialize()/device_add().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Link: https://lore.kernel.org/r/878sro1ldw.wl-kuninori.morimoto.gx@renesas.com 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 6df880be1622..3860d8521734 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1347,7 +1347,6 @@ static int soc_post_component_init(struct snd_soc_pcm_runtime *rtd, rtd->dev = kzalloc(sizeof(struct device), GFP_KERNEL); if (!rtd->dev) return -ENOMEM; - device_initialize(rtd->dev); rtd->dev->parent = rtd->card->dev; rtd->dev->release = rtd_release; rtd->dev->groups = soc_dev_attr_groups; @@ -1357,7 +1356,7 @@ static int soc_post_component_init(struct snd_soc_pcm_runtime *rtd, INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].be_clients); INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_PLAYBACK].fe_clients); INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].fe_clients); - ret = device_add(rtd->dev); + ret = device_register(rtd->dev); if (ret < 0) { /* calling put_device() here to free the rtd->dev */ put_device(rtd->dev);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
snd_soc_initialize_card_lists() is doing card related INIT_LIST_HEAD(), but, it is already doing at snd_soc_register_card(). We don't need to do it separately. This patch merges these.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
include/sound/soc.h | 10 ---------- sound/soc/soc-core.c | 13 ++++++++----- 2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 5c841c2..f264c65 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1220,16 +1220,6 @@ static inline void *snd_soc_card_get_drvdata(struct snd_soc_card *card) return card->drvdata; }
-static inline void snd_soc_initialize_card_lists(struct snd_soc_card *card) -{ - INIT_LIST_HEAD(&card->widgets); - INIT_LIST_HEAD(&card->paths); - INIT_LIST_HEAD(&card->dapm_list); - INIT_LIST_HEAD(&card->aux_comp_list); - INIT_LIST_HEAD(&card->component_dev_list); - INIT_LIST_HEAD(&card->list); -} - static inline bool snd_soc_volsw_is_stereo(struct soc_mixer_control *mc) { if (mc->reg == mc->rreg && mc->shift == mc->rshift) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index b3d02e7..0293383 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2369,15 +2369,18 @@ int snd_soc_register_card(struct snd_soc_card *card)
dev_set_drvdata(card->dev, card);
- snd_soc_initialize_card_lists(card); - + INIT_LIST_HEAD(&card->widgets); + INIT_LIST_HEAD(&card->paths); + INIT_LIST_HEAD(&card->dapm_list); + INIT_LIST_HEAD(&card->aux_comp_list); + INIT_LIST_HEAD(&card->component_dev_list); + INIT_LIST_HEAD(&card->list); INIT_LIST_HEAD(&card->dai_link_list); - INIT_LIST_HEAD(&card->rtd_list); - card->num_rtd = 0; - INIT_LIST_HEAD(&card->dapm_dirty); INIT_LIST_HEAD(&card->dobj_list); + + card->num_rtd = 0; card->instantiated = 0; mutex_init(&card->mutex); mutex_init(&card->dapm_mutex);
The patch
ASoC: soc-core: merge snd_soc_initialize_card_lists()
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 b03bfaec1d52123d5d941488f71e06964535e471 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:04:54 +0900 Subject: [PATCH] ASoC: soc-core: merge snd_soc_initialize_card_lists()
snd_soc_initialize_card_lists() is doing card related INIT_LIST_HEAD(), but, it is already doing at snd_soc_register_card(). We don't need to do it separately. This patch merges these.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Link: https://lore.kernel.org/r/877e781ldq.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc.h | 10 ---------- sound/soc/soc-core.c | 13 ++++++++----- 2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 5c841c2ee814..f264c6509f00 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1220,16 +1220,6 @@ static inline void *snd_soc_card_get_drvdata(struct snd_soc_card *card) return card->drvdata; }
-static inline void snd_soc_initialize_card_lists(struct snd_soc_card *card) -{ - INIT_LIST_HEAD(&card->widgets); - INIT_LIST_HEAD(&card->paths); - INIT_LIST_HEAD(&card->dapm_list); - INIT_LIST_HEAD(&card->aux_comp_list); - INIT_LIST_HEAD(&card->component_dev_list); - INIT_LIST_HEAD(&card->list); -} - static inline bool snd_soc_volsw_is_stereo(struct soc_mixer_control *mc) { if (mc->reg == mc->rreg && mc->shift == mc->rshift) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index b3f820fb53e6..d428491d51a7 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2370,15 +2370,18 @@ int snd_soc_register_card(struct snd_soc_card *card)
dev_set_drvdata(card->dev, card);
- snd_soc_initialize_card_lists(card); - + INIT_LIST_HEAD(&card->widgets); + INIT_LIST_HEAD(&card->paths); + INIT_LIST_HEAD(&card->dapm_list); + INIT_LIST_HEAD(&card->aux_comp_list); + INIT_LIST_HEAD(&card->component_dev_list); + INIT_LIST_HEAD(&card->list); INIT_LIST_HEAD(&card->dai_link_list); - INIT_LIST_HEAD(&card->rtd_list); - card->num_rtd = 0; - INIT_LIST_HEAD(&card->dapm_dirty); INIT_LIST_HEAD(&card->dobj_list); + + card->num_rtd = 0; card->instantiated = 0; mutex_init(&card->mutex); mutex_init(&card->dapm_mutex);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
snd_soc_remove_dai_link() has card connected dai_link check. but 1) we need to call list_del() anyway, because it is "remove" function, 2) It is doing many thing for this card / dai_link already before checking dai_link.
This patch removes poinless dai_link check
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
sound/soc/soc-core.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 0293383..3860d85 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1182,8 +1182,6 @@ EXPORT_SYMBOL_GPL(snd_soc_add_dai_link); void snd_soc_remove_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link) { - struct snd_soc_dai_link *link, *_link; - if (dai_link->dobj.type && dai_link->dobj.type != SND_SOC_DOBJ_DAI_LINK) { dev_err(card->dev, "Invalid dai link type %d\n", @@ -1199,12 +1197,7 @@ void snd_soc_remove_dai_link(struct snd_soc_card *card, if (dai_link->dobj.type && card->remove_dai_link) card->remove_dai_link(card, dai_link);
- for_each_card_links_safe(card, link, _link) { - if (link == dai_link) { - list_del(&link->list); - return; - } - } + list_del(&dai_link->list); } EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
The patch
ASoC: soc-core: remove unneeded dai_link check from snd_soc_remove_dai_link()
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 c26a8841119826badc8d358a4266880f83359f26 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:04:58 +0900 Subject: [PATCH] ASoC: soc-core: remove unneeded dai_link check from snd_soc_remove_dai_link()
snd_soc_remove_dai_link() has card connected dai_link check. but 1) we need to call list_del() anyway, because it is "remove" function, 2) It is doing many thing for this card / dai_link already before checking dai_link.
This patch removes poinless dai_link check
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Link: https://lore.kernel.org/r/875zms1ldm.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index d428491d51a7..6df880be1622 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1182,8 +1182,6 @@ EXPORT_SYMBOL_GPL(snd_soc_add_dai_link); void snd_soc_remove_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link) { - struct snd_soc_dai_link *link, *_link; - if (dai_link->dobj.type && dai_link->dobj.type != SND_SOC_DOBJ_DAI_LINK) { dev_err(card->dev, "Invalid dai link type %d\n", @@ -1199,12 +1197,7 @@ void snd_soc_remove_dai_link(struct snd_soc_card *card, if (dai_link->dobj.type && card->remove_dai_link) card->remove_dai_link(card, dai_link);
- for_each_card_links_safe(card, link, _link) { - if (link == dai_link) { - list_del(&link->list); - return; - } - } + list_del(&dai_link->list); } EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
We can find specified name component via snd_soc_rtdcom_lookup(). But, it is not enough under multi CPU/Codec/Platform, because many components which have same driver name might be connected to same rtd.
Not using this function as much as possible is best solution, but some drivers are already deeply depended to it.
We can expand this function, for example having "num" which specifies found order at parameter, etc (In such case, it need to have fixed probing order). Or, use different driver name in such component, etc.
We will have such issue if multi CPU/Codec/Platform were supported. To indicate it, this patch adds NOTE to this function.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
sound/soc/soc-core.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 3860d85..0ed6576 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -315,6 +315,14 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd, if (!driver_name) return NULL;
+ /* + * NOTE + * + * snd_soc_rtdcom_lookup() will find component from rtd by using + * specified driver name. + * But, if many components which have same driver name are connected + * to 1 rtd, this function will return 1st found component. + */ for_each_rtdcom(rtd, rtdcom) { const char *component_name = rtdcom->component->driver->name;
The patch
ASoC: soc-core: add NOTE to snd_soc_rtdcom_lookup()
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 a33c0d166cc5bcb3b9718649b84974216709acb1 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:05:02 +0900 Subject: [PATCH] ASoC: soc-core: add NOTE to snd_soc_rtdcom_lookup()
We can find specified name component via snd_soc_rtdcom_lookup(). But, it is not enough under multi CPU/Codec/Platform, because many components which have same driver name might be connected to same rtd.
Not using this function as much as possible is best solution, but some drivers are already deeply depended to it.
We can expand this function, for example having "num" which specifies found order at parameter, etc (In such case, it need to have fixed probing order). Or, use different driver name in such component, etc.
We will have such issue if multi CPU/Codec/Platform were supported. To indicate it, this patch adds NOTE to this function.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Link: https://lore.kernel.org/r/874l2c1ldi.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 3860d8521734..0ed6576bfef4 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -315,6 +315,14 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd, if (!driver_name) return NULL;
+ /* + * NOTE + * + * snd_soc_rtdcom_lookup() will find component from rtd by using + * specified driver name. + * But, if many components which have same driver name are connected + * to 1 rtd, this function will return 1st found component. + */ for_each_rtdcom(rtd, rtdcom) { const char *component_name = rtdcom->component->driver->name;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current ALSA SoC might allocate debugfs name memory via kasprintf(), but, it is unnecessary and the code becoming very complex. Using local variable with appropriate size is enough.
This patch uses 64byte local variable for it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
sound/soc/soc-core.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 0ed6576..6862abd 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -149,23 +149,19 @@ static const struct attribute_group *soc_dev_attr_groups[] = { #ifdef CONFIG_DEBUG_FS static void soc_init_component_debugfs(struct snd_soc_component *component) { + char name[64]; + if (!component->card->debugfs_card_root) return;
- if (component->debugfs_prefix) { - char *name; - - name = kasprintf(GFP_KERNEL, "%s:%s", + if (component->debugfs_prefix) + snprintf(name, ARRAY_SIZE(name), "%s:%s", component->debugfs_prefix, component->name); - if (name) { - component->debugfs_root = debugfs_create_dir(name, - component->card->debugfs_card_root); - kfree(name); - } - } else { - component->debugfs_root = debugfs_create_dir(component->name, - component->card->debugfs_card_root); - } + else + snprintf(name, ARRAY_SIZE(name), "%s", component->name); + + component->debugfs_root = debugfs_create_dir(name, + component->card->debugfs_card_root);
snd_soc_dapm_debugfs_init(snd_soc_component_get_dapm(component), component->debugfs_root);
On Tue, Aug 20, 2019 at 02:05:06PM +0900, Kuninori Morimoto wrote:
Current ALSA SoC might allocate debugfs name memory via kasprintf(), but, it is unnecessary and the code becoming very complex. Using local variable with appropriate size is enough.
This patch uses 64byte local variable for it.
Is it enough though? It'd be a very long name but as soon as you start putting a fixed limit in you're tempting fate a bit.
The other potential issue is that it's a relatively large allocation to do on the stack and while we're probably fine it still doesn't feel great.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
There is no need to check return value for soc_cleanup_card_resources(). Let't makes it as void.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
sound/soc/soc-core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 6862abd..6d2b744 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1881,7 +1881,7 @@ static void soc_check_tplg_fes(struct snd_soc_card *card) } }
-static int soc_cleanup_card_resources(struct snd_soc_card *card) +static void soc_cleanup_card_resources(struct snd_soc_card *card) { /* free the ALSA card at first; this syncs with pending operations */ if (card->snd_card) { @@ -1902,8 +1902,6 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card) /* remove the card */ if (card->remove) card->remove(card); - - return 0; }
static int snd_soc_instantiate_card(struct snd_soc_card *card)
The patch
ASoC: soc-core: soc_cleanup_card_resources() become void
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 a4de83a385670c22c31e9bbb726595a447b32ba4 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:05:10 +0900 Subject: [PATCH] ASoC: soc-core: soc_cleanup_card_resources() become void
There is no need to check return value for soc_cleanup_card_resources(). Let't makes it as void.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Link: https://lore.kernel.org/r/871rxg1lda.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 0ed6576bfef4..1be069c2ac8d 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1885,7 +1885,7 @@ static void soc_check_tplg_fes(struct snd_soc_card *card) } }
-static int soc_cleanup_card_resources(struct snd_soc_card *card) +static void soc_cleanup_card_resources(struct snd_soc_card *card) { /* free the ALSA card at first; this syncs with pending operations */ if (card->snd_card) { @@ -1906,8 +1906,6 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card) /* remove the card */ if (card->remove) card->remove(card); - - return 0; }
static int snd_soc_instantiate_card(struct snd_soc_card *card)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
It might return without initializing in error case. In such case, uninitialized variable might be used at error handler. This patch initializes all necessary variable before return.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
sound/soc/soc-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 6d2b744..f7fb114 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2643,6 +2643,9 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, { struct snd_soc_dapm_context *dapm;
+ INIT_LIST_HEAD(&component->dai_list); + mutex_init(&component->io_mutex); + component->name = fmt_single_name(dev, &component->id); if (!component->name) { dev_err(dev, "ASoC: Failed to allocate name\n"); @@ -2659,9 +2662,6 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, dapm->idle_bias_off = !driver->idle_bias_on; dapm->suspend_bias_off = driver->suspend_bias_off;
- INIT_LIST_HEAD(&component->dai_list); - mutex_init(&component->io_mutex); - return 0; }
The patch
ASoC: soc-core: initialize component list
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 8d92bb516831e80fac916701447ee6e9f0a6f0f2 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:05:16 +0900 Subject: [PATCH] ASoC: soc-core: initialize component list
It might return without initializing in error case. In such case, uninitialized variable might be used at error handler. This patch initializes all necessary variable before return.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Link: https://lore.kernel.org/r/87zhk4zazt.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 1be069c2ac8d..1a17cb1bc03b 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2647,6 +2647,9 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, { struct snd_soc_dapm_context *dapm;
+ INIT_LIST_HEAD(&component->dai_list); + mutex_init(&component->io_mutex); + component->name = fmt_single_name(dev, &component->id); if (!component->name) { dev_err(dev, "ASoC: Failed to allocate name\n"); @@ -2663,9 +2666,6 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, dapm->idle_bias_off = !driver->idle_bias_on; dapm->suspend_bias_off = driver->suspend_bias_off;
- INIT_LIST_HEAD(&component->dai_list); - mutex_init(&component->io_mutex); - return 0; }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Initialize component related list at random place is very difficult to read. This patch initialize it at snd_soc_component_initialize().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index f7fb114..39248e2 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1268,7 +1268,6 @@ static int soc_probe_component(struct snd_soc_card *card,
component->card = card; dapm->card = card; - INIT_LIST_HEAD(&component->card_list); INIT_LIST_HEAD(&dapm->list); soc_set_name_prefix(card, component);
@@ -2644,6 +2643,8 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, struct snd_soc_dapm_context *dapm;
INIT_LIST_HEAD(&component->dai_list); + INIT_LIST_HEAD(&component->dobj_list); + INIT_LIST_HEAD(&component->card_list); mutex_init(&component->io_mutex);
component->name = fmt_single_name(dev, &component->id); @@ -2729,7 +2730,6 @@ static void snd_soc_component_add(struct snd_soc_component *component)
/* see for_each_component */ list_add(&component->list, &component_list); - INIT_LIST_HEAD(&component->dobj_list);
mutex_unlock(&client_mutex); }
The patch
ASoC: soc-core: initialize list at one place
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 495efdb01f89a5fc53f9b2e61f5726d804d4a15d Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:05:20 +0900 Subject: [PATCH] ASoC: soc-core: initialize list at one place
Initialize component related list at random place is very difficult to read. This patch initialize it at snd_soc_component_initialize().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Link: https://lore.kernel.org/r/87y2zozazp.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 1a17cb1bc03b..0af83963289f 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1272,7 +1272,6 @@ static int soc_probe_component(struct snd_soc_card *card,
component->card = card; dapm->card = card; - INIT_LIST_HEAD(&component->card_list); INIT_LIST_HEAD(&dapm->list); soc_set_name_prefix(card, component);
@@ -2648,6 +2647,8 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, struct snd_soc_dapm_context *dapm;
INIT_LIST_HEAD(&component->dai_list); + INIT_LIST_HEAD(&component->dobj_list); + INIT_LIST_HEAD(&component->card_list); mutex_init(&component->io_mutex);
component->name = fmt_single_name(dev, &component->id); @@ -2733,7 +2734,6 @@ static void snd_soc_component_add(struct snd_soc_component *component)
/* see for_each_component */ list_add(&component->list, &component_list); - INIT_LIST_HEAD(&component->dobj_list);
mutex_unlock(&client_mutex); }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Initialize component related list at random place is very difficult to read. This patch initialize it at snd_soc_component_initialize().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index f7fb114..39248e2 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1268,7 +1268,6 @@ static int soc_probe_component(struct snd_soc_card *card,
component->card = card; dapm->card = card; - INIT_LIST_HEAD(&component->card_list); INIT_LIST_HEAD(&dapm->list); soc_set_name_prefix(card, component);
@@ -2644,6 +2643,8 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, struct snd_soc_dapm_context *dapm;
INIT_LIST_HEAD(&component->dai_list); + INIT_LIST_HEAD(&component->dobj_list); + INIT_LIST_HEAD(&component->card_list); mutex_init(&component->io_mutex);
component->name = fmt_single_name(dev, &component->id); @@ -2729,7 +2730,6 @@ static void snd_soc_component_add(struct snd_soc_component *component)
/* see for_each_component */ list_add(&component->list, &component_list); - INIT_LIST_HEAD(&component->dobj_list);
mutex_unlock(&client_mutex); }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
.bus_control can be bit field. this patch do it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
include/sound/soc-dai.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index dc48fe0..939c73d 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -293,8 +293,6 @@ struct snd_soc_dai_driver { /* Optional Callback used at pcm creation*/ int (*pcm_new)(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai *dai); - /* DAI is also used for the control bus */ - bool bus_control;
/* ops */ const struct snd_soc_dai_ops *ops; @@ -306,6 +304,7 @@ struct snd_soc_dai_driver { unsigned int symmetric_rates:1; unsigned int symmetric_channels:1; unsigned int symmetric_samplebits:1; + unsigned int bus_control:1; /* DAI is also used for the control bus */
/* probe ordering - for components with runtime dependencies */ int probe_order;
The patch
ASoC: soc-dai: use bit field for bus_control
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 6b8ac43c33b912bc5435192ce3b3f051f6f2dc9d Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:05:28 +0900 Subject: [PATCH] ASoC: soc-dai: use bit field for bus_control
.bus_control can be bit field. this patch do it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Link: https://lore.kernel.org/r/87v9uszazh.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc-dai.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index dc48fe081a20..939c73db6a03 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -293,8 +293,6 @@ struct snd_soc_dai_driver { /* Optional Callback used at pcm creation*/ int (*pcm_new)(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai *dai); - /* DAI is also used for the control bus */ - bool bus_control;
/* ops */ const struct snd_soc_dai_ops *ops; @@ -306,6 +304,7 @@ struct snd_soc_dai_driver { unsigned int symmetric_rates:1; unsigned int symmetric_channels:1; unsigned int symmetric_samplebits:1; + unsigned int bus_control:1; /* DAI is also used for the control bus */
/* probe ordering - for components with runtime dependencies */ int probe_order;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
commit 52abe6cc1866a ("ASoC: topology: fix oops/use-after-free case with dai driver") fixups remove_dai() error, but it is using list_for_each_entry() for component->dai_list.
We already have for_each_component_dais() macro for it. Let's use exising method.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index dc463f1..b869071 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -530,7 +530,7 @@ static void remove_dai(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->dai_unload) dobj->ops->dai_unload(comp, dobj);
- list_for_each_entry(dai, &comp->dai_list, list) + for_each_component_dais(comp, dai) if (dai->driver == dai_drv) dai->driver = NULL;
The patch
ASoC: soc-topology: use for_each_component_dais() at remove_dai()
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 43ca5dab978294eae26a36f8989b6f0769da4256 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Tue, 20 Aug 2019 14:05:32 +0900 Subject: [PATCH] ASoC: soc-topology: use for_each_component_dais() at remove_dai()
commit 52abe6cc1866a ("ASoC: topology: fix oops/use-after-free case with dai driver") fixups remove_dai() error, but it is using list_for_each_entry() for component->dai_list.
We already have for_each_component_dais() macro for it. Let's use exising method.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87tvaczazd.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index dc463f1a9e24..b8690715abb5 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -530,7 +530,7 @@ static void remove_dai(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->dai_unload) dobj->ops->dai_unload(comp, dobj);
- list_for_each_entry(dai, &comp->dai_list, list) + for_each_component_dais(comp, dai) if (dai->driver == dai_drv) dai->driver = NULL;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
commit 52abe6cc1866a ("ASoC: topology: fix oops/use-after-free case with dai driver") fixups remove_dai() error, but it is using list_for_each_entry() for component->dai_list.
We already have for_each_component_dais() macro for it. Let's use exising method.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- v2 -> v3
- add Ranjani's Reviewed-by
sound/soc/soc-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index dc463f1..b869071 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -530,7 +530,7 @@ static void remove_dai(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->dai_unload) dobj->ops->dai_unload(comp, dobj);
- list_for_each_entry(dai, &comp->dai_list, list) + for_each_component_dais(comp, dai) if (dai->driver == dai_drv) dai->driver = NULL;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
It is easy to read code if it is cleanly using paired function/naming, like start <-> stop, register <-> unregister, etc, etc. But, current ALSA SoC code is very random, unbalance, not paired, etc. It is easy to create bug at the such code, and it will be difficult to debug.
soc-core.c has soc_bind_aux_dev(), but, there is no its paired soc_unbind_aux_dev(). This patch adds soc_unbind_aux_dev().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- care component->init
sound/soc/soc-core.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 39248e2..c433bdb 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1529,6 +1529,12 @@ static int soc_probe_link_dais(struct snd_soc_card *card, return ret; }
+static void soc_unbind_aux_dev(struct snd_soc_component *component) +{ + component->init = NULL; + list_del(&component->card_aux_list); +} + static int soc_bind_aux_dev(struct snd_soc_card *card, struct snd_soc_aux_dev *aux_dev) { @@ -1580,7 +1586,7 @@ static void soc_remove_aux_devices(struct snd_soc_card *card) if (comp->driver->remove_order == order) { soc_remove_component(comp); /* remove it from the card's aux_comp_list */ - list_del(&comp->card_aux_list); + soc_unbind_aux_dev(comp); } } }
participants (2)
-
Kuninori Morimoto -
Mark Brown