[PATCH 0/2] ASoC: soc-dapm.c: random cleanup retry
Hi Mark
These are remains of my previous cleanup patch-set. [1/2] adds comment why kzalloc()/kfree() are needed on snd_soc_dai_link_event_pre_pmu(). [2/2] is adjusted to [1/1] patch.
Link: https://lore.kernel.org/all/8735d59zt9.wl-kuninori.morimoto.gx@renesas.com/
Kuninori Morimoto (2): ASoC: soc-dapm.c: add comment for kzalloc()/kfree() on snd_soc_dai_link_event_pre_pmu() ASoC: soc-dapm.c: tidyup snd_soc_dai_link_event_pre_pmu()
sound/soc/soc-dapm.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
snd_soc_dai_link_event_pre_pmu() is using kzalloc()/kfree() for params. It looks we don't need to use these, but are necessary. But, it is difficult to know why it is necessary without any comments. This patch adds the reasons via comment.
Link: https://lore.kernel.org/all/Yxc2wzbZsSVZNf8Y@sirena.org.uk/ Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-dapm.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 76615d59e2b6..fc2f75ed190d 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3842,6 +3842,15 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, unsigned int fmt; int ret = 0;
+ /* + * NOTE + * + * snd_pcm_hw_params is quite large (608 bytes on arm64) and is + * starting to get a bit excessive for allocation on the stack, + * especially when you're building with some of the KASAN type + * stuff that increases stack usage. + * So, we use kzalloc()/kfree() for params in this function. + */ params = kzalloc(sizeof(*params), GFP_KERNEL); if (!params) return -ENOMEM; @@ -3939,7 +3948,9 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, runtime->rate = params_rate(params);
out: + /* see above NOTE */ kfree(params); + return ret; }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
snd_soc_dai_link_event_pre_pmu() is using if/else for config->formats check, but "else" case is for just error. Unnecessary if/else is not good for readable code. this patch checks if config->formats was zero as error case.
Moreover, we don't need to indicate config->formats value in error message, because it is zero. This patch tidyup it, too.
=> if (config->formats) { ... } else { dev_warn(w->dapm->dev, "ASoC: Invalid format %llx specified\n", => config->formats); ... }
Link: https://lore.kernel.org/all/YxiDkDOwRsbXeZ17@sirena.org.uk/ Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-dapm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index fc2f75ed190d..47a7bf99472e 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3900,16 +3900,15 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, }
/* Be a little careful as we don't want to overflow the mask array */ - if (config->formats) { - fmt = ffs(config->formats) - 1; - } else { - dev_warn(w->dapm->dev, "ASoC: Invalid format %llx specified\n", - config->formats); + if (!config->formats) { + dev_warn(w->dapm->dev, "ASoC: Invalid format was specified\n");
ret = -EINVAL; goto out; }
+ fmt = ffs(config->formats) - 1; + snd_mask_set(hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT), fmt); hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE)->min = config->rate_min;
On Thu, 8 Sep 2022 02:44:39 +0000, Kuninori Morimoto wrote:
These are remains of my previous cleanup patch-set. [1/2] adds comment why kzalloc()/kfree() are needed on snd_soc_dai_link_event_pre_pmu(). [2/2] is adjusted to [1/1] patch.
Link: https://lore.kernel.org/all/8735d59zt9.wl-kuninori.morimoto.gx@renesas.com/
Kuninori Morimoto (2): ASoC: soc-dapm.c: add comment for kzalloc()/kfree() on snd_soc_dai_link_event_pre_pmu() ASoC: soc-dapm.c: tidyup snd_soc_dai_link_event_pre_pmu()
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: soc-dapm.c: add comment for kzalloc()/kfree() on snd_soc_dai_link_event_pre_pmu() commit: 6ef8443fb1ced148417d830894240a097ba79a03 [2/2] ASoC: soc-dapm.c: tidyup snd_soc_dai_link_event_pre_pmu() commit: 59a1063dcaa5c9450dc1d221679418747bf086fc
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
participants (2)
-
Kuninori Morimoto -
Mark Brown