From f101583fa9f8c3f372d4feb61d67da0ccbf4d9a5 Mon Sep 17 00:00:00 2001 From: Sameer Pujar Date: Thu, 7 Sep 2023 20:32:24 +0530 Subject: [PATCH 01/31] ASoC: soc-utils: Export snd_soc_dai_is_dummy() symbol Export symbol snd_soc_dai_is_dummy() for usage outside core driver modules. This is required by Tegra ASoC machine driver. Signed-off-by: Sameer Pujar Link: https://lore.kernel.org/r/1694098945-32760-2-git-send-email-spujar@nvidia.com Signed-off-by: Mark Brown --- sound/soc/soc-utils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 11607c5f5d5a..9c746e4edef7 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -217,6 +217,7 @@ int snd_soc_dai_is_dummy(struct snd_soc_dai *dai) return 1; return 0; } +EXPORT_SYMBOL_GPL(snd_soc_dai_is_dummy); int snd_soc_component_is_dummy(struct snd_soc_component *component) { From e765886249c533e1bb5cbc3cd741bad677417312 Mon Sep 17 00:00:00 2001 From: Sameer Pujar Date: Thu, 7 Sep 2023 20:32:25 +0530 Subject: [PATCH 02/31] ASoC: tegra: Fix redundant PLLA and PLLA_OUT0 updates Tegra audio graph card has many DAI links which connects internal AHUB modules and external audio codecs. Since these are DPCM links, hw_params() call in the machine driver happens for each connected BE link and PLLA is updated every time. This is not really needed for all links as only I/O link DAIs derive respective clocks from PLLA_OUT0 and thus from PLLA. Hence add checks to limit the clock updates to DAIs over I/O links. This found to be fixing a DMIC clock discrepancy which is suspected to happen because of back to back quick PLLA and PLLA_OUT0 rate updates. This was observed on Jetson TX2 platform where DMIC clock ended up with unexpected value. Fixes: 202e2f774543 ("ASoC: tegra: Add audio graph based card driver") Cc: stable@vger.kernel.org Signed-off-by: Sameer Pujar Link: https://lore.kernel.org/r/1694098945-32760-3-git-send-email-spujar@nvidia.com Signed-off-by: Mark Brown --- sound/soc/tegra/tegra_audio_graph_card.c | 30 ++++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/sound/soc/tegra/tegra_audio_graph_card.c b/sound/soc/tegra/tegra_audio_graph_card.c index 1f2c5018bf5a..4737e776d383 100644 --- a/sound/soc/tegra/tegra_audio_graph_card.c +++ b/sound/soc/tegra/tegra_audio_graph_card.c @@ -10,6 +10,7 @@ #include #include #include +#include #define MAX_PLLA_OUT0_DIV 128 @@ -44,6 +45,21 @@ struct tegra_audio_cdata { unsigned int plla_out0_rates[NUM_RATE_TYPE]; }; +static bool need_clk_update(struct snd_soc_dai *dai) +{ + if (snd_soc_dai_is_dummy(dai) || + !dai->driver->ops || + !dai->driver->name) + return false; + + if (strstr(dai->driver->name, "I2S") || + strstr(dai->driver->name, "DMIC") || + strstr(dai->driver->name, "DSPK")) + return true; + + return false; +} + /* Setup PLL clock as per the given sample rate */ static int tegra_audio_graph_update_pll(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) @@ -140,19 +156,7 @@ static int tegra_audio_graph_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0); int err; - /* - * This gets called for each DAI link (FE or BE) when DPCM is used. - * We may not want to update PLLA rate for each call. So PLLA update - * must be restricted to external I/O links (I2S, DMIC or DSPK) since - * they actually depend on it. I/O modules update their clocks in - * hw_param() of their respective component driver and PLLA rate - * update here helps them to derive appropriate rates. - * - * TODO: When more HW accelerators get added (like sample rate - * converter, volume gain controller etc., which don't really - * depend on PLLA) we need a better way to filter here. - */ - if (cpu_dai->driver->ops && rtd->dai_link->no_pcm) { + if (need_clk_update(cpu_dai)) { err = tegra_audio_graph_update_pll(substream, params); if (err) return err; From ec03804552e9a723569e14d2512f36a8e70dc640 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 8 Sep 2023 11:17:16 +0100 Subject: [PATCH 03/31] ASoC: cs35l56: Call pm_runtime_dont_use_autosuspend() Driver remove() must call pm_runtime_dont_use_autosuspend(). Drivers that call pm_runtime_use_autosuspend() must disable it in driver remove(). Unfortunately until recently this was only mentioned in 1 line in a 900+ line document so most people hadn't noticed this. It has only recently been added to the kerneldoc of pm_runtime_use_autosuspend(). THIS WON'T APPLY CLEANLY TO V6.5 AND EARLIER: We will send a separate backported patch to stable. Signed-off-by: Richard Fitzgerald Link: https://lore.kernel.org/r/20230908101716.2658582-1-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs35l56.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c index 600b79c62ec4..f2e7c6d0be46 100644 --- a/sound/soc/codecs/cs35l56.c +++ b/sound/soc/codecs/cs35l56.c @@ -1207,6 +1207,7 @@ void cs35l56_remove(struct cs35l56_private *cs35l56) flush_workqueue(cs35l56->dsp_wq); destroy_workqueue(cs35l56->dsp_wq); + pm_runtime_dont_use_autosuspend(cs35l56->base.dev); pm_runtime_suspend(cs35l56->base.dev); pm_runtime_disable(cs35l56->base.dev); From aedf323b66b2b875137422ecb7d2525179759076 Mon Sep 17 00:00:00 2001 From: Jerome Brunet Date: Thu, 7 Sep 2023 11:05:04 +0200 Subject: [PATCH 04/31] ASoC: meson: spdifin: start hw on dai probe For spdif input to report the locked rate correctly, even when no capture is running, the HW and reference clock must be started as soon as the dai is probed. Fixes: 5ce5658375e6 ("ASoC: meson: add axg spdif input") Signed-off-by: Jerome Brunet Link: https://lore.kernel.org/r/20230907090504.12700-1-jbrunet@baylibre.com Signed-off-by: Mark Brown --- sound/soc/meson/axg-spdifin.c | 49 ++++++++++++----------------------- 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/sound/soc/meson/axg-spdifin.c b/sound/soc/meson/axg-spdifin.c index d86880169075..bc2f2849ecfb 100644 --- a/sound/soc/meson/axg-spdifin.c +++ b/sound/soc/meson/axg-spdifin.c @@ -112,34 +112,6 @@ static int axg_spdifin_prepare(struct snd_pcm_substream *substream, return 0; } -static int axg_spdifin_startup(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct axg_spdifin *priv = snd_soc_dai_get_drvdata(dai); - int ret; - - ret = clk_prepare_enable(priv->refclk); - if (ret) { - dev_err(dai->dev, - "failed to enable spdifin reference clock\n"); - return ret; - } - - regmap_update_bits(priv->map, SPDIFIN_CTRL0, SPDIFIN_CTRL0_EN, - SPDIFIN_CTRL0_EN); - - return 0; -} - -static void axg_spdifin_shutdown(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct axg_spdifin *priv = snd_soc_dai_get_drvdata(dai); - - regmap_update_bits(priv->map, SPDIFIN_CTRL0, SPDIFIN_CTRL0_EN, 0); - clk_disable_unprepare(priv->refclk); -} - static void axg_spdifin_write_mode_param(struct regmap *map, int mode, unsigned int val, unsigned int num_per_reg, @@ -251,17 +223,32 @@ static int axg_spdifin_dai_probe(struct snd_soc_dai *dai) ret = axg_spdifin_sample_mode_config(dai, priv); if (ret) { dev_err(dai->dev, "mode configuration failed\n"); - clk_disable_unprepare(priv->pclk); - return ret; + goto pclk_err; } + ret = clk_prepare_enable(priv->refclk); + if (ret) { + dev_err(dai->dev, + "failed to enable spdifin reference clock\n"); + goto pclk_err; + } + + regmap_update_bits(priv->map, SPDIFIN_CTRL0, SPDIFIN_CTRL0_EN, + SPDIFIN_CTRL0_EN); + return 0; + +pclk_err: + clk_disable_unprepare(priv->pclk); + return ret; } static int axg_spdifin_dai_remove(struct snd_soc_dai *dai) { struct axg_spdifin *priv = snd_soc_dai_get_drvdata(dai); + regmap_update_bits(priv->map, SPDIFIN_CTRL0, SPDIFIN_CTRL0_EN, 0); + clk_disable_unprepare(priv->refclk); clk_disable_unprepare(priv->pclk); return 0; } @@ -270,8 +257,6 @@ static const struct snd_soc_dai_ops axg_spdifin_ops = { .probe = axg_spdifin_dai_probe, .remove = axg_spdifin_dai_remove, .prepare = axg_spdifin_prepare, - .startup = axg_spdifin_startup, - .shutdown = axg_spdifin_shutdown, }; static int axg_spdifin_iec958_info(struct snd_kcontrol *kcontrol, From 28115b1c4f2bb76e786436bf6597c5eb27638a5c Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Thu, 7 Sep 2023 11:55:20 +0200 Subject: [PATCH 05/31] ASoC: rsnd: add missing of_node_put for_each_child_of_node performs an of_node_get on each iteration, so a break out of the loop requires an of_node_put. This was done using the Coccinelle semantic patch iterators/for_each_child.cocci Signed-off-by: Julia Lawall Acked-by: Kuninori Morimoto Link: https://lore.kernel.org/r/20230907095521.14053-11-Julia.Lawall@inria.fr Signed-off-by: Mark Brown --- sound/soc/sh/rcar/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c index e29c2fee9521..1bd7114c472a 100644 --- a/sound/soc/sh/rcar/core.c +++ b/sound/soc/sh/rcar/core.c @@ -1303,6 +1303,7 @@ audio_graph: if (i >= RSND_MAX_COMPONENT) { dev_info(dev, "reach to max component\n"); of_node_put(node); + of_node_put(ports); break; } } From d7e47e32192bb88f5b2dc8e655fa587ecf9d71e0 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Sat, 9 Sep 2023 05:02:37 -0700 Subject: [PATCH 06/31] ASoC: wm8960: Fix error handling in probe Commit 422f10adc3eb ("ASoC: wm8960: Add support for the power supplies") added regulator support to the wm8960 driver, but neglected to update error handling in the probe function. This results in warning backtraces if the probe function fails. Fixes: 422f10adc3eb ("ASoC: wm8960: Add support for the power supplies") Signed-off-by: Guenter Roeck Reviewed-by: Fabio Estevam Link: https://lore.kernel.org/r/20230909120237.2646275-1-linux@roeck-us.net Signed-off-by: Mark Brown --- sound/soc/codecs/wm8960.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c index 0a50180750e8..7689fe3cc86d 100644 --- a/sound/soc/codecs/wm8960.c +++ b/sound/soc/codecs/wm8960.c @@ -1468,8 +1468,10 @@ static int wm8960_i2c_probe(struct i2c_client *i2c) } wm8960->regmap = devm_regmap_init_i2c(i2c, &wm8960_regmap); - if (IS_ERR(wm8960->regmap)) - return PTR_ERR(wm8960->regmap); + if (IS_ERR(wm8960->regmap)) { + ret = PTR_ERR(wm8960->regmap); + goto bulk_disable; + } if (pdata) memcpy(&wm8960->pdata, pdata, sizeof(struct wm8960_data)); @@ -1479,13 +1481,14 @@ static int wm8960_i2c_probe(struct i2c_client *i2c) ret = i2c_master_recv(i2c, &val, sizeof(val)); if (ret >= 0) { dev_err(&i2c->dev, "Not wm8960, wm8960 reg can not read by i2c\n"); - return -EINVAL; + ret = -EINVAL; + goto bulk_disable; } ret = wm8960_reset(wm8960->regmap); if (ret != 0) { dev_err(&i2c->dev, "Failed to issue reset\n"); - return ret; + goto bulk_disable; } if (wm8960->pdata.shared_lrclk) { @@ -1494,7 +1497,7 @@ static int wm8960_i2c_probe(struct i2c_client *i2c) if (ret != 0) { dev_err(&i2c->dev, "Failed to enable LRCM: %d\n", ret); - return ret; + goto bulk_disable; } } @@ -1528,7 +1531,13 @@ static int wm8960_i2c_probe(struct i2c_client *i2c) ret = devm_snd_soc_register_component(&i2c->dev, &soc_component_dev_wm8960, &wm8960_dai, 1); + if (ret) + goto bulk_disable; + return 0; + +bulk_disable: + regulator_bulk_disable(ARRAY_SIZE(wm8960->supplies), wm8960->supplies); return ret; } From 396b907919e028d89bac912e49de014485deb8dc Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Fri, 8 Sep 2023 09:59:20 +0100 Subject: [PATCH 07/31] ASoC: soc-pcm: Shrink stack frame for __soc_pcm_hw_params Commit ac950278b087 ("ASoC: add N cpus to M codecs dai link support") added an additional local params in __soc_pcm_hw_params, for the CPU side of the DAI. The snd_pcm_hw_params struct is pretty large (604 bytes) and keeping two local copies of it can make the stack frame really large. It is worth noting the variables are in separate code blocks so for some optimisation levels in the compiler these will get automatically combined keeping the stack frame reasonable. But better to manually combine them to cover all cases. Add a single local variable for __soc_pcm_hw_params and use in both loops to shrink the stack frame. Signed-off-by: Charles Keepax Link: https://lore.kernel.org/r/20230908085920.2906359-1-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index eb0723876851..54704250c0a2 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -985,6 +985,7 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd, { struct snd_soc_dai *cpu_dai; struct snd_soc_dai *codec_dai; + struct snd_pcm_hw_params tmp_params; int i, ret = 0; snd_soc_dpcm_mutex_assert_held(rtd); @@ -998,7 +999,6 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd, goto out; for_each_rtd_codec_dais(rtd, i, codec_dai) { - struct snd_pcm_hw_params codec_params; unsigned int tdm_mask = snd_soc_dai_tdm_mask_get(codec_dai, substream->stream); /* @@ -1019,23 +1019,22 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd, continue; /* copy params for each codec */ - codec_params = *params; + tmp_params = *params; /* fixup params based on TDM slot masks */ if (tdm_mask) - soc_pcm_codec_params_fixup(&codec_params, tdm_mask); + soc_pcm_codec_params_fixup(&tmp_params, tdm_mask); ret = snd_soc_dai_hw_params(codec_dai, substream, - &codec_params); + &tmp_params); if(ret < 0) goto out; - soc_pcm_set_dai_params(codec_dai, &codec_params); - snd_soc_dapm_update_dai(substream, &codec_params, codec_dai); + soc_pcm_set_dai_params(codec_dai, &tmp_params); + snd_soc_dapm_update_dai(substream, &tmp_params, codec_dai); } for_each_rtd_cpu_dais(rtd, i, cpu_dai) { - struct snd_pcm_hw_params cpu_params; unsigned int ch_mask = 0; int j; @@ -1047,7 +1046,7 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd, continue; /* copy params for each cpu */ - cpu_params = *params; + tmp_params = *params; if (!rtd->dai_link->codec_ch_maps) goto hw_params; @@ -1062,16 +1061,16 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd, /* fixup cpu channel number */ if (ch_mask) - soc_pcm_codec_params_fixup(&cpu_params, ch_mask); + soc_pcm_codec_params_fixup(&tmp_params, ch_mask); hw_params: - ret = snd_soc_dai_hw_params(cpu_dai, substream, &cpu_params); + ret = snd_soc_dai_hw_params(cpu_dai, substream, &tmp_params); if (ret < 0) goto out; /* store the parameters for each DAI */ - soc_pcm_set_dai_params(cpu_dai, &cpu_params); - snd_soc_dapm_update_dai(substream, &cpu_params, cpu_dai); + soc_pcm_set_dai_params(cpu_dai, &tmp_params); + snd_soc_dapm_update_dai(substream, &tmp_params, cpu_dai); } ret = snd_soc_pcm_component_hw_params(substream, params); From e616a916fe8431ebd5eb3cf4ac224d143c57083c Mon Sep 17 00:00:00 2001 From: Walt Holman Date: Sun, 10 Sep 2023 13:54:34 -0500 Subject: [PATCH 08/31] Add DMI ID for MSI Bravo 15 B7ED Signed-off-by: Walt Holman Link: https://lore.kernel.org/r/20230910185433.13677-1-waltholman09@gmail.com Signed-off-by: Mark Brown --- sound/soc/amd/yc/acp6x-mach.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/soc/amd/yc/acp6x-mach.c b/sound/soc/amd/yc/acp6x-mach.c index 3ec15b46fa35..59aa2e9d3a79 100644 --- a/sound/soc/amd/yc/acp6x-mach.c +++ b/sound/soc/amd/yc/acp6x-mach.c @@ -262,6 +262,13 @@ static const struct dmi_system_id yc_acp_quirk_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "M6500RC"), } }, + { + .driver_data = &acp6x_card, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "Micro-Star International Co., Ltd."), + DMI_MATCH(DMI_PRODUCT_NAME, "Bravo 15 B7ED"), + } + }, { .driver_data = &acp6x_card, .matches = { From 2f9426905a63be7ccf8cd10109caf1848aa0993a Mon Sep 17 00:00:00 2001 From: Shengjiu Wang Date: Mon, 11 Sep 2023 14:38:07 +0800 Subject: [PATCH 09/31] ASoC: fsl: imx-pcm-rpmsg: Add SNDRV_PCM_INFO_BATCH flag The rpmsg pcm device is a device which should support double buffering. Found this issue with pipewire. When there is no SNDRV_PCM_INFO_BATCH flag in driver, the pipewire will set headroom to be zero, and because rpmsg pcm device don't support residue report, when the latency setting is small, the "delay" always larger than "target" in alsa-pcm.c, that reading next period data is not scheduled on time. With SNDRV_PCM_INFO_BATCH flag in driver, the pipewire will select a smaller period size for device, then the task of reading next period data will be scheduled on time. Signed-off-by: Shengjiu Wang Link: https://lore.kernel.org/r/1694414287-13291-1-git-send-email-shengjiu.wang@nxp.com Signed-off-by: Mark Brown --- sound/soc/fsl/imx-pcm-rpmsg.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/fsl/imx-pcm-rpmsg.c b/sound/soc/fsl/imx-pcm-rpmsg.c index d63782b8bdef..bb736d45c9e0 100644 --- a/sound/soc/fsl/imx-pcm-rpmsg.c +++ b/sound/soc/fsl/imx-pcm-rpmsg.c @@ -19,6 +19,7 @@ static struct snd_pcm_hardware imx_rpmsg_pcm_hardware = { .info = SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER | + SNDRV_PCM_INFO_BATCH | SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_NO_PERIOD_WAKEUP | From 1263cc0f414d212129c0f1289b49b7df77f92084 Mon Sep 17 00:00:00 2001 From: August Wikerfors Date: Mon, 11 Sep 2023 23:34:09 +0200 Subject: [PATCH 10/31] ASoC: amd: yc: Fix non-functional mic on Lenovo 82QF and 82UG Like the Lenovo 82TL and 82V2, the Lenovo 82QF (Yoga 7 14ARB7) and 82UG (Legion S7 16ARHA7) both need a quirk entry for the internal microphone to function. Commit c008323fe361 ("ASoC: amd: yc: Fix a non-functional mic on Lenovo 82SJ") restricted the quirk that previously matched "82" to "82V2", breaking microphone functionality on these devices. Fix this by adding specific quirks for these models, as was done for the Lenovo 82TL. Fixes: c008323fe361 ("ASoC: amd: yc: Fix a non-functional mic on Lenovo 82SJ") Closes: https://github.com/tomsom/yoga-linux/issues/51 Link: https://bugzilla.kernel.org/show_bug.cgi?id=208555#c780 Cc: stable@vger.kernel.org Signed-off-by: August Wikerfors Link: https://lore.kernel.org/r/20230911213409.6106-1-git@augustwikerfors.se Signed-off-by: Mark Brown --- sound/soc/amd/yc/acp6x-mach.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/sound/soc/amd/yc/acp6x-mach.c b/sound/soc/amd/yc/acp6x-mach.c index 59aa2e9d3a79..94e9eb8e73f2 100644 --- a/sound/soc/amd/yc/acp6x-mach.c +++ b/sound/soc/amd/yc/acp6x-mach.c @@ -213,6 +213,13 @@ static const struct dmi_system_id yc_acp_quirk_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "21J6"), } }, + { + .driver_data = &acp6x_card, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82QF"), + } + }, { .driver_data = &acp6x_card, .matches = { @@ -220,6 +227,13 @@ static const struct dmi_system_id yc_acp_quirk_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "82TL"), } }, + { + .driver_data = &acp6x_card, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82UG"), + } + }, { .driver_data = &acp6x_card, .matches = { From fa6a0c0c1dd53b3949ca56bf7213648dfd6a62ee Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Sep 2023 13:32:40 +0200 Subject: [PATCH 11/31] ASoC: rt5640: Revert "Fix sleep in atomic context" Commit 70a6404ff610 ("ASoC: rt5640: Fix sleep in atomic context") not only switched from request_irq() to request_threaded_irq(), to fix the sleep in atomic context issue, but it also added devm management of the IRQ by actually switching to devm_request_threaded_irq() (without any explanation in the commit message for this change). This is wrong since the IRQ was already explicitly managed by the driver. On unbind the ASoC core will call rt5640_set_jack(NULL) which in turn will call rt5640_disable_jack_detect() which frees the IRQ already. So now we have a double free. Besides the unexplained switch to devm being wrong, the actual fix for the sleep in atomic context issue also is not the best solution. The only thing which rt5640_irq() does is cancel + (re-)queue the jack_work delayed_work. This can be done in a single non sleeping call by replacing queue_delayed_work() with mod_delayed_work(), which does not sleep. Using mod_delayed_work() is a much better fix then adding a thread which does nothing other then queuing a work-item. This patch is a straight revert of the troublesome changes, the switch to mod_delayed_work() is done in a separate follow-up patch. Fixes: 70a6404ff610 ("ASoC: rt5640: Fix sleep in atomic context") Cc: Sameer Pujar Cc: Oder Chiou Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20230912113245.320159-2-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5640.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c index 15e1a62b9e57..05ff8066171b 100644 --- a/sound/soc/codecs/rt5640.c +++ b/sound/soc/codecs/rt5640.c @@ -2565,10 +2565,9 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component, if (jack_data && jack_data->use_platform_clock) rt5640->use_platform_clock = jack_data->use_platform_clock; - ret = devm_request_threaded_irq(component->dev, rt5640->irq, - NULL, rt5640_irq, - IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT, - "rt5640", rt5640); + ret = request_irq(rt5640->irq, rt5640_irq, + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT, + "rt5640", rt5640); if (ret) { dev_warn(component->dev, "Failed to request IRQ %d: %d\n", rt5640->irq, ret); rt5640_disable_jack_detect(component); @@ -2621,9 +2620,8 @@ static void rt5640_enable_hda_jack_detect( rt5640->jack = jack; - ret = devm_request_threaded_irq(component->dev, rt5640->irq, - NULL, rt5640_irq, IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "rt5640", rt5640); + ret = request_irq(rt5640->irq, rt5640_irq, + IRQF_TRIGGER_RISING | IRQF_ONESHOT, "rt5640", rt5640); if (ret) { dev_warn(component->dev, "Failed to request IRQ %d: %d\n", rt5640->irq, ret); rt5640->irq = -ENXIO; From df7d595f6bd9dc96cc275cc4b0f313fcfa423c58 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Sep 2023 13:32:41 +0200 Subject: [PATCH 12/31] ASoC: rt5640: Fix sleep in atomic context Following prints are observed while testing audio on Jetson AGX Orin which has onboard RT5640 audio codec: BUG: sleeping function called from invalid context at kernel/workqueue.c:3027 in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/0 preempt_count: 10001, expected: 0 RCU nest depth: 0, expected: 0 ------------[ cut here ]------------ WARNING: CPU: 0 PID: 0 at kernel/irq/handle.c:159 __handle_irq_event_percpu+0x1e0/0x270 ---[ end trace ad1c64905aac14a6 ]- The IRQ handler rt5640_irq() runs in interrupt context and can sleep during cancel_delayed_work_sync(). The only thing which rt5640_irq() does is cancel + (re-)queue the jack_work delayed_work. This can be done in a single non sleeping call by replacing queue_delayed_work() with mod_delayed_work(), avoiding the sleep in atomic context. Fixes: 051dade34695 ("ASoC: rt5640: Fix the wrong state of JD1 and JD2") Reported-by: Sameer Pujar Closes: https://lore.kernel.org/r/1688015537-31682-4-git-send-email-spujar@nvidia.com Cc: Oder Chiou Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20230912113245.320159-3-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5640.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c index 05ff8066171b..5c34c045d396 100644 --- a/sound/soc/codecs/rt5640.c +++ b/sound/soc/codecs/rt5640.c @@ -2403,13 +2403,11 @@ static irqreturn_t rt5640_irq(int irq, void *data) struct rt5640_priv *rt5640 = data; int delay = 0; - if (rt5640->jd_src == RT5640_JD_SRC_HDA_HEADER) { - cancel_delayed_work_sync(&rt5640->jack_work); + if (rt5640->jd_src == RT5640_JD_SRC_HDA_HEADER) delay = 100; - } if (rt5640->jack) - queue_delayed_work(system_long_wq, &rt5640->jack_work, delay); + mod_delayed_work(system_long_wq, &rt5640->jack_work, delay); return IRQ_HANDLED; } From 786120ebb649b166021f0212250e8627e53d068a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Sep 2023 13:32:42 +0200 Subject: [PATCH 13/31] ASoC: rt5640: Do not disable/enable IRQ twice on suspend/resume When jack-detect was originally added disabling the IRQ during suspend was done by the sound/soc/intel/boards/bytcr_rt5640.c driver calling snd_soc_component_set_jack(NULL) on suspend, which calls rt5640_disable_jack_detect(), which calls free_irq() which also disables it. Commit 5fabcc90e79b ("ASoC: rt5640: Fix Jack work after system suspend") added disable_irq() / enable_irq() calls on suspend/resume for machine drivers which do not call snd_soc_component_set_jack(NULL) on suspend. The new disable_irq() / enable_irq() are made conditional by "if (rt5640->irq)" statements, but this is true for the machine drivers which do call snd_soc_component_set_jack(NULL) on suspend too, causing a disable_irq() call there on the already free-ed IRQ. Change the "if (rt5640->irq)" condition to "if (rt5640->jack)" to fix this, rt5640->jack is only set if the jack-detect IRQ handler is still active when rt5640_suspend() runs. And adjust rt5640_enable_hda_jack_detect()'s request_irq() error handling to set rt5640->jack to NULL to match (note that the old setting of irq to -ENOXIO still resulted in disable_irq(-ENOXIO) calls on suspend). Fixes: 5fabcc90e79b ("ASoC: rt5640: Fix Jack work after system suspend") Cc: Oder Chiou Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20230912113245.320159-4-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5640.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c index 5c34c045d396..1bc281d42ca8 100644 --- a/sound/soc/codecs/rt5640.c +++ b/sound/soc/codecs/rt5640.c @@ -2622,7 +2622,7 @@ static void rt5640_enable_hda_jack_detect( IRQF_TRIGGER_RISING | IRQF_ONESHOT, "rt5640", rt5640); if (ret) { dev_warn(component->dev, "Failed to request IRQ %d: %d\n", rt5640->irq, ret); - rt5640->irq = -ENXIO; + rt5640->jack = NULL; return; } @@ -2797,7 +2797,7 @@ static int rt5640_suspend(struct snd_soc_component *component) { struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component); - if (rt5640->irq) { + if (rt5640->jack) { /* disable jack interrupts during system suspend */ disable_irq(rt5640->irq); } @@ -2825,10 +2825,9 @@ static int rt5640_resume(struct snd_soc_component *component) regcache_cache_only(rt5640->regmap, false); regcache_sync(rt5640->regmap); - if (rt5640->irq) + if (rt5640->jack) { enable_irq(rt5640->irq); - if (rt5640->jack) { if (rt5640->jd_src == RT5640_JD_SRC_HDA_HEADER) { snd_soc_component_update_bits(component, RT5640_DUMMY2, 0x1100, 0x1100); From b5e85e535551bf82242aa5896e14a136ed3c156d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Sep 2023 13:32:43 +0200 Subject: [PATCH 14/31] ASoC: rt5640: Enable the IRQ on resume after configuring jack-detect The jack-detect IRQ should be enabled *after* the jack-detect related configuration registers have been programmed. Move the enable_irq() call for this to after the register setup. Fixes: 5fabcc90e79b ("ASoC: rt5640: Fix Jack work after system suspend") Cc: Oder Chiou Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20230912113245.320159-5-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5640.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c index 1bc281d42ca8..03c866c04c7a 100644 --- a/sound/soc/codecs/rt5640.c +++ b/sound/soc/codecs/rt5640.c @@ -2826,8 +2826,6 @@ static int rt5640_resume(struct snd_soc_component *component) regcache_sync(rt5640->regmap); if (rt5640->jack) { - enable_irq(rt5640->irq); - if (rt5640->jd_src == RT5640_JD_SRC_HDA_HEADER) { snd_soc_component_update_bits(component, RT5640_DUMMY2, 0x1100, 0x1100); @@ -2854,6 +2852,7 @@ static int rt5640_resume(struct snd_soc_component *component) } } + enable_irq(rt5640->irq); queue_delayed_work(system_long_wq, &rt5640->jack_work, 0); } From 8c8bf3df6b7c0ed1c4dd373b23eb0ce13a63f452 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Sep 2023 13:32:44 +0200 Subject: [PATCH 15/31] ASoC: rt5640: Fix IRQ not being free-ed for HDA jack detect mode Set "rt5640->irq_requested = true" after a successful request_irq() in rt5640_enable_hda_jack_detect(), so that rt5640_disable_jack_detect() properly frees the IRQ. This fixes the IRQ not being freed on rmmod / driver unbind. Fixes: 2b9c8d2b3c89 ("ASoC: rt5640: Add the HDA header support") Cc: Oder Chiou Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20230912113245.320159-6-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5640.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c index 03c866c04c7a..a4a11407ab10 100644 --- a/sound/soc/codecs/rt5640.c +++ b/sound/soc/codecs/rt5640.c @@ -2625,6 +2625,7 @@ static void rt5640_enable_hda_jack_detect( rt5640->jack = NULL; return; } + rt5640->irq_requested = true; /* sync initial jack state */ queue_delayed_work(system_long_wq, &rt5640->jack_work, 0); From 8fc7cc507d61fc655172836c74fb7fcc8b7a978b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Sep 2023 13:32:45 +0200 Subject: [PATCH 16/31] ASoC: rt5640: Only cancel jack-detect work on suspend if active If jack-detection is not used; or has already been disabled then there is no need to call rt5640_cancel_work(). Move the rt5640_cancel_work() inside the "if (rt5640->jack) {}" block, grouping it together with the disabling of the IRQ which queues the work in the first place. This also makes suspend() symetrical with resume() which re-queues the work in an "if (rt5640->jack) {}" block. Cc: Oder Chiou Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20230912113245.320159-7-hdegoede@redhat.com Signed-off-by: Mark Brown --- sound/soc/codecs/rt5640.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c index a4a11407ab10..e8cdc166bdaa 100644 --- a/sound/soc/codecs/rt5640.c +++ b/sound/soc/codecs/rt5640.c @@ -2801,9 +2801,9 @@ static int rt5640_suspend(struct snd_soc_component *component) if (rt5640->jack) { /* disable jack interrupts during system suspend */ disable_irq(rt5640->irq); + rt5640_cancel_work(rt5640); } - rt5640_cancel_work(rt5640); snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF); rt5640_reset(component); regcache_cache_only(rt5640->regmap, true); From 18789be8e0d9fbb78b2290dcf93f500726ed19f0 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Tue, 12 Sep 2023 14:38:41 +0100 Subject: [PATCH 17/31] ASoC: cs35l56: Disable low-power hibernation mode Do not allow the CS35L56 to be put into its lowest power "hibernation" mode. This only affects I2C because "hibernation" is already disabled on SPI and SoundWire. Recent firmwares need a different wake-up sequence. Until that sequence has been specified, the chip "hibernation" mode must be disabled otherwise it can intermittently fail to wake. THIS WILL NOT APPLY CLEANLY TO 6.5 AND EARLIER: We will send a separate backport patch to stable. Signed-off-by: Richard Fitzgerald Link: https://lore.kernel.org/r/20230912133841.3480466-1-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs35l56-i2c.c | 1 - 1 file changed, 1 deletion(-) diff --git a/sound/soc/codecs/cs35l56-i2c.c b/sound/soc/codecs/cs35l56-i2c.c index 9f4f2f4f23f5..d10e0e2380e8 100644 --- a/sound/soc/codecs/cs35l56-i2c.c +++ b/sound/soc/codecs/cs35l56-i2c.c @@ -27,7 +27,6 @@ static int cs35l56_i2c_probe(struct i2c_client *client) return -ENOMEM; cs35l56->base.dev = dev; - cs35l56->base.can_hibernate = true; i2c_set_clientdata(client, cs35l56); cs35l56->base.regmap = devm_regmap_init_i2c(client, regmap_config); From cf0ba445f5e4dd74c1e9d7a83ca721ba69204a11 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 13 Sep 2023 11:18:22 +0300 Subject: [PATCH 18/31] ASoC: codecs: aw88395: Fix some error codes These error paths should return -EINVAL instead of success. Fixes: 7f4ec77802aa ("ASoC: codecs: Add code for bin parsing compatible with aw88261") Signed-off-by: Dan Carpenter Link: https://lore.kernel.org/r/81476e78-05c2-4656-b754-f314c7ccdb81@moroto.mountain Signed-off-by: Mark Brown --- sound/soc/codecs/aw88395/aw88395_lib.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/codecs/aw88395/aw88395_lib.c b/sound/soc/codecs/aw88395/aw88395_lib.c index 8ee1baa03269..87dd0ccade4c 100644 --- a/sound/soc/codecs/aw88395/aw88395_lib.c +++ b/sound/soc/codecs/aw88395/aw88395_lib.c @@ -452,11 +452,13 @@ static int aw_dev_parse_reg_bin_with_hdr(struct aw_device *aw_dev, if ((aw_bin->all_bin_parse_num != 1) || (aw_bin->header_info[0].bin_data_type != DATA_TYPE_REGISTER)) { dev_err(aw_dev->dev, "bin num or type error"); + ret = -EINVAL; goto parse_bin_failed; } if (aw_bin->header_info[0].valid_data_len % 4) { dev_err(aw_dev->dev, "bin data len get error!"); + ret = -EINVAL; goto parse_bin_failed; } From 41dac81b56c82c51a6d00fda5f3af7691ffee2d7 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 13 Sep 2023 16:00:10 +0100 Subject: [PATCH 19/31] ASoC: cs42l42: Ensure a reset pulse meets minimum pulse width. The CS42L42 can accept very short reset pulses of a few microseconds but there's no reason to force a very short pulse. Allow a wide range for the usleep_range() so it can be relaxed about the choice of timing source. Signed-off-by: Richard Fitzgerald Signed-off-by: Stefan Binding Link: https://lore.kernel.org/r/20230913150012.604775-2-sbinding@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs42l42.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index a0de0329406a..56d2857a4f01 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -2320,6 +2320,10 @@ int cs42l42_common_probe(struct cs42l42_private *cs42l42, if (cs42l42->reset_gpio) { dev_dbg(cs42l42->dev, "Found reset GPIO\n"); + + /* Ensure minimum reset pulse width */ + usleep_range(10, 500); + gpiod_set_value_cansleep(cs42l42->reset_gpio, 1); } usleep_range(CS42L42_BOOT_TIME_US, CS42L42_BOOT_TIME_US * 2); From a479b44ac0a0ac25cd48e5356200078924d78022 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 13 Sep 2023 16:00:11 +0100 Subject: [PATCH 20/31] ASoC: cs42l42: Don't rely on GPIOD_OUT_LOW to set RESET initially low The ACPI setting for a GPIO default state has higher priority than the flag passed to devm_gpiod_get_optional() so ACPI can override the GPIOD_OUT_LOW. Explicitly set the GPIO low when hard resetting. Although GPIOD_OUT_LOW can't be relied on this doesn't seem like a reason to stop passing it to devm_gpiod_get_optional(). So we still pass it to state our intent, but can deal with it having no effect. Signed-off-by: Richard Fitzgerald Signed-off-by: Stefan Binding Link: https://lore.kernel.org/r/20230913150012.604775-3-sbinding@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs42l42.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 56d2857a4f01..dc93861ddfb0 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -2321,6 +2321,12 @@ int cs42l42_common_probe(struct cs42l42_private *cs42l42, if (cs42l42->reset_gpio) { dev_dbg(cs42l42->dev, "Found reset GPIO\n"); + /* + * ACPI can override the default GPIO state we requested + * so ensure that we start with RESET low. + */ + gpiod_set_value_cansleep(cs42l42->reset_gpio, 0); + /* Ensure minimum reset pulse width */ usleep_range(10, 500); From 2d066c6a78654c179f95c9beda1985d4c6befa4e Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 13 Sep 2023 16:00:12 +0100 Subject: [PATCH 21/31] ASoC: cs42l42: Avoid stale SoundWire ATTACH after hard reset In SoundWire mode leave hard RESET asserted when exiting probe, and wait for an UNATTACHED notification before deasserting RESET. If the boot state of the reset GPIO was deasserted it is possible that the SoundWire core had already enumerated the CS42L42 before cs42l42_sdw_probe() is called. When cs42l42_common_probe() hard resets the CS42L42 it triggers a race condition: 1) After cs42l42_sdw_probe() returns the thread that called it will call cs42l42_sdw_update_status() to report the last status recorded by the SoundWire core. 2) The SoundWire bus master will see a PING with the CS42L42 now reporting as unenumerated and will trigger the core SoundWire code to start enumerating CS42L42. These two threads are racing against each other. If (1) happens before (2) a stale ATTACHED notification will be reported to the cs42l42 driver when in fact the status of cs42l42 is now unattached. To avoid this race condition: - Leave RESET asserted on exit from cs42l42_sdw_probe(). This ensures that an UNATTACHED notification must be sent to the cs42l42 driver. If cs42l42 was already enumerated it will be seen to drop off the bus, causing an UNATTACH notification. If it was never enumerated the status is already UNATTACHED and this will be reported by thread (1). - When the UNATTACH notification is received, release RESET. This will cause CS42L42 to be enumerated and eventually report an ATTACHED notification. - The ATTACHED notification is now valid. Signed-off-by: Richard Fitzgerald Signed-off-by: Stefan Binding Link: https://lore.kernel.org/r/20230913150012.604775-4-sbinding@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs42l42-sdw.c | 20 ++++++++++++++++++++ sound/soc/codecs/cs42l42.c | 11 ++++++++++- sound/soc/codecs/cs42l42.h | 1 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/cs42l42-sdw.c b/sound/soc/codecs/cs42l42-sdw.c index eeab07c850f9..974bae4abfad 100644 --- a/sound/soc/codecs/cs42l42-sdw.c +++ b/sound/soc/codecs/cs42l42-sdw.c @@ -344,6 +344,16 @@ static int cs42l42_sdw_update_status(struct sdw_slave *peripheral, switch (status) { case SDW_SLAVE_ATTACHED: dev_dbg(cs42l42->dev, "ATTACHED\n"); + + /* + * The SoundWire core can report stale ATTACH notifications + * if we hard-reset CS42L42 in probe() but it had already been + * enumerated. Reject the ATTACH if we haven't yet seen an + * UNATTACH report for the device being in reset. + */ + if (cs42l42->sdw_waiting_first_unattach) + break; + /* * Initialise codec, this only needs to be done once. * When resuming from suspend, resume callback will handle re-init of codec, @@ -354,6 +364,16 @@ static int cs42l42_sdw_update_status(struct sdw_slave *peripheral, break; case SDW_SLAVE_UNATTACHED: dev_dbg(cs42l42->dev, "UNATTACHED\n"); + + if (cs42l42->sdw_waiting_first_unattach) { + /* + * SoundWire core has seen that CS42L42 is not on + * the bus so release RESET and wait for ATTACH. + */ + cs42l42->sdw_waiting_first_unattach = false; + gpiod_set_value_cansleep(cs42l42->reset_gpio, 1); + } + break; default: break; diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index dc93861ddfb0..2961340f15e2 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -2330,7 +2330,16 @@ int cs42l42_common_probe(struct cs42l42_private *cs42l42, /* Ensure minimum reset pulse width */ usleep_range(10, 500); - gpiod_set_value_cansleep(cs42l42->reset_gpio, 1); + /* + * On SoundWire keep the chip in reset until we get an UNATTACH + * notification from the SoundWire core. This acts as a + * synchronization point to reject stale ATTACH notifications + * if the chip was already enumerated before we reset it. + */ + if (cs42l42->sdw_peripheral) + cs42l42->sdw_waiting_first_unattach = true; + else + gpiod_set_value_cansleep(cs42l42->reset_gpio, 1); } usleep_range(CS42L42_BOOT_TIME_US, CS42L42_BOOT_TIME_US * 2); diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index 4bd7b85a5747..7785125b73ab 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -53,6 +53,7 @@ struct cs42l42_private { u8 stream_use; bool hp_adc_up_pending; bool suspended; + bool sdw_waiting_first_unattach; bool init_done; }; From 69343ce91435f222052015c5af86b550391bac85 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 13 Sep 2023 17:05:23 +0100 Subject: [PATCH 22/31] firmware: cirrus: cs_dsp: Only log list of algorithms in debug build Change the logging of each algorithm from info level to debug level. On the original devices supported by this code there were typically only one or two algorithms in a firmware and one or two DSPs so this logging only used a small number of log lines. However, for the latest devices there could be 30-40 algorithms in a firmware and 8 DSPs being loaded in parallel, so using 300+ lines of log for information that isn't particularly important to have logged. Signed-off-by: Richard Fitzgerald Link: https://lore.kernel.org/r/20230913160523.3701189-1-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- drivers/firmware/cirrus/cs_dsp.c | 34 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/firmware/cirrus/cs_dsp.c b/drivers/firmware/cirrus/cs_dsp.c index 49b70c70dc69..79d4254d1f9b 100644 --- a/drivers/firmware/cirrus/cs_dsp.c +++ b/drivers/firmware/cirrus/cs_dsp.c @@ -1863,15 +1863,15 @@ static int cs_dsp_adsp2_setup_algs(struct cs_dsp *dsp) return PTR_ERR(adsp2_alg); for (i = 0; i < n_algs; i++) { - cs_dsp_info(dsp, - "%d: ID %x v%d.%d.%d XM@%x YM@%x ZM@%x\n", - i, be32_to_cpu(adsp2_alg[i].alg.id), - (be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff0000) >> 16, - (be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff00) >> 8, - be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff, - be32_to_cpu(adsp2_alg[i].xm), - be32_to_cpu(adsp2_alg[i].ym), - be32_to_cpu(adsp2_alg[i].zm)); + cs_dsp_dbg(dsp, + "%d: ID %x v%d.%d.%d XM@%x YM@%x ZM@%x\n", + i, be32_to_cpu(adsp2_alg[i].alg.id), + (be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff0000) >> 16, + (be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff00) >> 8, + be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff, + be32_to_cpu(adsp2_alg[i].xm), + be32_to_cpu(adsp2_alg[i].ym), + be32_to_cpu(adsp2_alg[i].zm)); alg_region = cs_dsp_create_region(dsp, WMFW_ADSP2_XM, adsp2_alg[i].alg.id, @@ -1996,14 +1996,14 @@ static int cs_dsp_halo_setup_algs(struct cs_dsp *dsp) return PTR_ERR(halo_alg); for (i = 0; i < n_algs; i++) { - cs_dsp_info(dsp, - "%d: ID %x v%d.%d.%d XM@%x YM@%x\n", - i, be32_to_cpu(halo_alg[i].alg.id), - (be32_to_cpu(halo_alg[i].alg.ver) & 0xff0000) >> 16, - (be32_to_cpu(halo_alg[i].alg.ver) & 0xff00) >> 8, - be32_to_cpu(halo_alg[i].alg.ver) & 0xff, - be32_to_cpu(halo_alg[i].xm_base), - be32_to_cpu(halo_alg[i].ym_base)); + cs_dsp_dbg(dsp, + "%d: ID %x v%d.%d.%d XM@%x YM@%x\n", + i, be32_to_cpu(halo_alg[i].alg.id), + (be32_to_cpu(halo_alg[i].alg.ver) & 0xff0000) >> 16, + (be32_to_cpu(halo_alg[i].alg.ver) & 0xff00) >> 8, + be32_to_cpu(halo_alg[i].alg.ver) & 0xff, + be32_to_cpu(halo_alg[i].xm_base), + be32_to_cpu(halo_alg[i].ym_base)); ret = cs_dsp_halo_create_regions(dsp, halo_alg[i].alg.id, halo_alg[i].alg.ver, From 781118bc2fc1026c8285f83ea7ecab07071a09c4 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 13 Sep 2023 17:02:50 +0100 Subject: [PATCH 23/31] ASoC: wm_adsp: Fix missing locking in wm_adsp_[read|write]_ctl() wm_adsp_read_ctl() and wm_adsp_write_ctl() must hold the cs_dsp pwr_lock mutex when calling cs_dsp_coeff_read_ctrl() and cs_dsp_coeff_write_ctrl(). Signed-off-by: Richard Fitzgerald Link: https://lore.kernel.org/r/20230913160250.3700346-1-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/wm_adsp.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 6fc34f41b175..d1b9238d391e 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -687,7 +687,10 @@ int wm_adsp_write_ctl(struct wm_adsp *dsp, const char *name, int type, struct wm_coeff_ctl *ctl; int ret; + mutex_lock(&dsp->cs_dsp.pwr_lock); ret = cs_dsp_coeff_write_ctrl(cs_ctl, 0, buf, len); + mutex_unlock(&dsp->cs_dsp.pwr_lock); + if (ret < 0) return ret; @@ -703,8 +706,14 @@ EXPORT_SYMBOL_GPL(wm_adsp_write_ctl); int wm_adsp_read_ctl(struct wm_adsp *dsp, const char *name, int type, unsigned int alg, void *buf, size_t len) { - return cs_dsp_coeff_read_ctrl(cs_dsp_get_ctl(&dsp->cs_dsp, name, type, alg), - 0, buf, len); + int ret; + + mutex_lock(&dsp->cs_dsp.pwr_lock); + ret = cs_dsp_coeff_read_ctrl(cs_dsp_get_ctl(&dsp->cs_dsp, name, type, alg), + 0, buf, len); + mutex_unlock(&dsp->cs_dsp.pwr_lock); + + return ret; } EXPORT_SYMBOL_GPL(wm_adsp_read_ctl); From fac58baf8fcfcd7481e8f6d60206ce2a47c1476c Mon Sep 17 00:00:00 2001 From: Chancel Liu Date: Wed, 13 Sep 2023 18:26:56 +0800 Subject: [PATCH 24/31] ASoC: imx-rpmsg: Set ignore_pmdown_time for dai_link i.MX rpmsg sound cards work on codec slave mode. MCLK will be disabled by CPU DAI driver in hw_free(). Some codec requires MCLK present at power up/down sequence. So need to set ignore_pmdown_time to power down codec immediately before MCLK is turned off. Take WM8962 as an example, if MCLK is disabled before DAPM power down playback stream, FIFO error will arise in WM8962 which will have bad impact on playback next. Signed-off-by: Chancel Liu Acked-by: Shengjiu Wang Link: https://lore.kernel.org/r/20230913102656.2966757-1-chancel.liu@nxp.com Signed-off-by: Mark Brown --- sound/soc/fsl/imx-rpmsg.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/soc/fsl/imx-rpmsg.c b/sound/soc/fsl/imx-rpmsg.c index 3c7b95db2eac..b578f9a32d7f 100644 --- a/sound/soc/fsl/imx-rpmsg.c +++ b/sound/soc/fsl/imx-rpmsg.c @@ -89,6 +89,14 @@ static int imx_rpmsg_probe(struct platform_device *pdev) SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC; + /* + * i.MX rpmsg sound cards work on codec slave mode. MCLK will be + * disabled by CPU DAI driver in hw_free(). Some codec requires MCLK + * present at power up/down sequence. So need to set ignore_pmdown_time + * to power down codec immediately before MCLK is turned off. + */ + data->dai.ignore_pmdown_time = 1; + /* Optional codec node */ ret = of_parse_phandle_with_fixed_args(np, "audio-codec", 0, 0, &args); if (ret) { From 6ba59c008f08e84b3c87be10f3391c9735e4f833 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Thu, 14 Sep 2023 16:25:04 +0300 Subject: [PATCH 25/31] ASoC: SOF: ipc4-topology: fix wrong sizeof argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit available_fmt is a pointer. Fixes: 4fdef47a44d6 ("ASoC: SOF: ipc4-topology: Add new tokens for input/output pin format count") Signed-off-by: Bard Liao Reviewed-by: Péter Ujfalusi Reviewed-by: Pierre-Louis Bossart Signed-off-by: Peter Ujfalusi Link: https://lore.kernel.org/r/20230914132504.18463-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index f2a30cd31378..7cb63e6b24dc 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -231,7 +231,7 @@ static int sof_ipc4_get_audio_fmt(struct snd_soc_component *scomp, ret = sof_update_ipc_object(scomp, available_fmt, SOF_AUDIO_FMT_NUM_TOKENS, swidget->tuples, - swidget->num_tuples, sizeof(available_fmt), 1); + swidget->num_tuples, sizeof(*available_fmt), 1); if (ret) { dev_err(scomp->dev, "Failed to parse audio format token count\n"); return ret; From bb0216d4db9ecaa51af45d8504757becbe5c050d Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 14 Sep 2023 15:47:25 +0300 Subject: [PATCH 26/31] ASoC: SOF: sof-audio: Fix DSP core put imbalance on widget setup failure In case the widget setup fails we should only decrement the core usage count if the sof_widget_free_unlocked() has not been called as part of the error handling. sof_widget_free_unlocked() calls snd_sof_dsp_core_put() and the additional core_put will cause imbalance in core usage count. Use the existing use_count_decremented to handle this issue. Signed-off-by: Peter Ujfalusi Reviewed-by: Ranjani Sridharan Reviewed-by: Bard Liao Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230914124725.17397-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/sof-audio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index e7ef77012c35..e5405f854a91 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -212,7 +212,8 @@ widget_free: sof_widget_free_unlocked(sdev, swidget); use_count_decremented = true; core_put: - snd_sof_dsp_core_put(sdev, swidget->core); + if (!use_count_decremented) + snd_sof_dsp_core_put(sdev, swidget->core); pipe_widget_free: if (swidget->id != snd_soc_dapm_scheduler) sof_widget_free_unlocked(sdev, swidget->spipe->pipe_widget); From c04efbfd76d23157e64e6d6147518c187ab4233a Mon Sep 17 00:00:00 2001 From: Chen Ni Date: Fri, 15 Sep 2023 02:13:44 +0000 Subject: [PATCH 27/31] ASoC: hdaudio.c: Add missing check for devm_kstrdup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because of the potential failure of the devm_kstrdup(), the dl[i].codecs->name could be NULL. Therefore, we need to check it and return -ENOMEM in order to transfer the error. Fixes: 97030a43371e ("ASoC: Intel: avs: Add HDAudio machine board") Signed-off-by: Chen Ni Reviewed-by: Amadeusz Sławiński Link: https://lore.kernel.org/r/20230915021344.3078-1-nichen@iscas.ac.cn Signed-off-by: Mark Brown --- sound/soc/intel/avs/boards/hdaudio.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/intel/avs/boards/hdaudio.c b/sound/soc/intel/avs/boards/hdaudio.c index cb00bc86ac94..8876558f19a1 100644 --- a/sound/soc/intel/avs/boards/hdaudio.c +++ b/sound/soc/intel/avs/boards/hdaudio.c @@ -55,6 +55,9 @@ static int avs_create_dai_links(struct device *dev, struct hda_codec *codec, int return -ENOMEM; dl[i].codecs->name = devm_kstrdup(dev, cname, GFP_KERNEL); + if (!dl[i].codecs->name) + return -ENOMEM; + dl[i].codecs->dai_name = pcm->name; dl[i].num_codecs = 1; dl[i].num_cpus = 1; From b19a5733de255cabba5feecabf6e900638b582d1 Mon Sep 17 00:00:00 2001 From: Shengjiu Wang Date: Fri, 15 Sep 2023 14:02:11 +0800 Subject: [PATCH 28/31] ASoC: imx-audmix: Fix return error with devm_clk_get() The devm_clk_get() can return -EPROBE_DEFER error, modify the error code to be -EINVAL is not correct, which cause the -EPROBE_DEFER error is not correctly handled. This patch is to fix the return error code. Fixes: b86ef5367761 ("ASoC: fsl: Add Audio Mixer machine driver") Signed-off-by: Shengjiu Wang Reviewed-by: Daniel Baluta Link: https://lore.kernel.org/r/1694757731-18308-1-git-send-email-shengjiu.wang@nxp.com Signed-off-by: Mark Brown --- sound/soc/fsl/imx-audmix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/fsl/imx-audmix.c b/sound/soc/fsl/imx-audmix.c index 0b58df56f4da..aeb81aa61184 100644 --- a/sound/soc/fsl/imx-audmix.c +++ b/sound/soc/fsl/imx-audmix.c @@ -315,7 +315,7 @@ static int imx_audmix_probe(struct platform_device *pdev) if (IS_ERR(priv->cpu_mclk)) { ret = PTR_ERR(priv->cpu_mclk); dev_err(&cpu_pdev->dev, "failed to get DAI mclk1: %d\n", ret); - return -EINVAL; + return ret; } priv->audmix_pdev = audmix_pdev; From c923e7759a29cf67aa4dda77b816263771380f86 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Fri, 15 Sep 2023 15:43:00 +0100 Subject: [PATCH 29/31] ASoC: cs42l43: Add shared IRQ flag for shutters The microphone and speaker shutters on cs42l43 can be configured to trigger from the same GPIO, in this case the current code returns an error as we attempt to request two IRQ handlers for the same IRQ. Fix this by always requesting the shutter IRQs with the IRQF_SHARED flag. Signed-off-by: Charles Keepax Link: https://lore.kernel.org/r/20230915144300.120100-1-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs42l43.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/sound/soc/codecs/cs42l43.c b/sound/soc/codecs/cs42l43.c index 1a95c370fc4c..5643c666d7d0 100644 --- a/sound/soc/codecs/cs42l43.c +++ b/sound/soc/codecs/cs42l43.c @@ -2077,7 +2077,8 @@ static const struct cs42l43_irq cs42l43_irqs[] = { static int cs42l43_request_irq(struct cs42l43_codec *priv, struct irq_domain *dom, const char * const name, - unsigned int irq, irq_handler_t handler) + unsigned int irq, irq_handler_t handler, + unsigned long flags) { int ret; @@ -2087,8 +2088,8 @@ static int cs42l43_request_irq(struct cs42l43_codec *priv, dev_dbg(priv->dev, "Request IRQ %d for %s\n", ret, name); - ret = devm_request_threaded_irq(priv->dev, ret, NULL, handler, IRQF_ONESHOT, - name, priv); + ret = devm_request_threaded_irq(priv->dev, ret, NULL, handler, + IRQF_ONESHOT | flags, name, priv); if (ret) return dev_err_probe(priv->dev, ret, "Failed to request IRQ %s\n", name); @@ -2124,11 +2125,11 @@ static int cs42l43_shutter_irq(struct cs42l43_codec *priv, return 0; } - ret = cs42l43_request_irq(priv, dom, close_name, close_irq, handler); + ret = cs42l43_request_irq(priv, dom, close_name, close_irq, handler, IRQF_SHARED); if (ret) return ret; - return cs42l43_request_irq(priv, dom, open_name, open_irq, handler); + return cs42l43_request_irq(priv, dom, open_name, open_irq, handler, IRQF_SHARED); } static int cs42l43_codec_probe(struct platform_device *pdev) @@ -2178,7 +2179,8 @@ static int cs42l43_codec_probe(struct platform_device *pdev) for (i = 0; i < ARRAY_SIZE(cs42l43_irqs); i++) { ret = cs42l43_request_irq(priv, dom, cs42l43_irqs[i].name, - cs42l43_irqs[i].irq, cs42l43_irqs[i].handler); + cs42l43_irqs[i].irq, + cs42l43_irqs[i].handler, 0); if (ret) goto err_pm; } From e0f96246c4402514acda040be19ee24c1619e01a Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Fri, 15 Sep 2023 16:41:53 +0300 Subject: [PATCH 30/31] ASoC: SOF: Intel: MTL: Reduce the DSP init timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 20s seems unnecessarily large for the DSP init timeout. This coupled with multiple FW boot attempts causes an excessive delay in the error path when booting in recovery mode. Reduce it to 0.5s and use the existing HDA_DSP_INIT_TIMEOUT_US. Link: https://github.com/thesofproject/linux/issues/4565 Signed-off-by: Ranjani Sridharan Reviewed-by: Pierre-Louis Bossart Reviewed-by: Bard Liao Reviewed-by: Péter Ujfalusi Signed-off-by: Peter Ujfalusi Link: https://lore.kernel.org/r/20230915134153.9688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/mtl.c | 2 +- sound/soc/sof/intel/mtl.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c index b84ca58da9d5..f9412517eaf2 100644 --- a/sound/soc/sof/intel/mtl.c +++ b/sound/soc/sof/intel/mtl.c @@ -460,7 +460,7 @@ int mtl_dsp_cl_init(struct snd_sof_dev *sdev, int stream_tag, bool imr_boot) /* step 3: wait for IPC DONE bit from ROM */ ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, chip->ipc_ack, status, ((status & chip->ipc_ack_mask) == chip->ipc_ack_mask), - HDA_DSP_REG_POLL_INTERVAL_US, MTL_DSP_PURGE_TIMEOUT_US); + HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_INIT_TIMEOUT_US); if (ret < 0) { if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS) dev_err(sdev->dev, "timeout waiting for purge IPC done\n"); diff --git a/sound/soc/sof/intel/mtl.h b/sound/soc/sof/intel/mtl.h index 02181490f12a..95696b3d7c4c 100644 --- a/sound/soc/sof/intel/mtl.h +++ b/sound/soc/sof/intel/mtl.h @@ -62,7 +62,6 @@ #define MTL_DSP_IRQSTS_IPC BIT(0) #define MTL_DSP_IRQSTS_SDW BIT(6) -#define MTL_DSP_PURGE_TIMEOUT_US 20000000 /* 20s */ #define MTL_DSP_REG_POLL_INTERVAL_US 10 /* 10 us */ /* Memory windows */ From 31bb7bd9ffee50d09ec931998b823a86132ab807 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 15 Sep 2023 15:40:15 +0300 Subject: [PATCH 31/31] ASoC: SOF: core: Only call sof_ops_free() on remove if the probe was successful All the fail paths during probe will free up the ops, on remove we should only free it if the probe was successful. Fixes: bc433fd76fae ("ASoC: SOF: Add ops_free") Signed-off-by: Peter Ujfalusi Reviewed-by: Bard Liao Reviewed-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Reviewed-by: Rander Wang Link: https://lore.kernel.org/r/20230915124015.19637-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 30db685cc5f4..2d1616b81485 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -486,10 +486,9 @@ int snd_sof_device_remove(struct device *dev) snd_sof_ipc_free(sdev); snd_sof_free_debug(sdev); snd_sof_remove(sdev); + sof_ops_free(sdev); } - sof_ops_free(sdev); - /* release firmware */ snd_sof_fw_unload(sdev);