Re: [PATCH 4/8] ASoC: codecs: wcd938x: fix resource leaks on component remove

Srinivas Kandagatla
2023-07-06T11:09:32Z

On 05/07/2023 13:30, Johan Hovold wrote:
> Make sure to release allocated resources on component probe failure and
> on remove.
>
> This is specifically needed to allow probe deferrals of the sound card
> which otherwise fails when reprobing the codec component:
>
> snd-sc8280xp sound: ASoC: failed to instantiate card -517
> genirq: Flags mismatch irq 289. 00002001 (HPHR PDM WD INT) vs. 00002001 (HPHR PDM WD INT)
> wcd938x_codec audio-codec: Failed to request HPHR WD interrupt (-16)
> genirq: Flags mismatch irq 290. 00002001 (HPHL PDM WD INT) vs. 00002001 (HPHL PDM WD INT)
> wcd938x_codec audio-codec: Failed to request HPHL WD interrupt (-16)
> genirq: Flags mismatch irq 291. 00002001 (AUX PDM WD INT) vs. 00002001 (AUX PDM WD INT)
> wcd938x_codec audio-codec: Failed to request Aux WD interrupt (-16)
> genirq: Flags mismatch irq 292. 00002001 (mbhc sw intr) vs. 00002001 (mbhc sw intr)
> wcd938x_codec audio-codec: Failed to request mbhc interrupts -16
>
> Fixes: 8d78602aa87a ("ASoC: codecs: wcd938x: add basic driver")
> Cc: stable@vger.kernel.org # 5.14
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---

Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

> sound/soc/codecs/wcd938x.c | 55 +++++++++++++++++++++++++++++++++-----
> 1 file changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
> index 2e342398d027..be38cad5f354 100644
> --- a/sound/soc/codecs/wcd938x.c
> +++ b/sound/soc/codecs/wcd938x.c
> @@ -2636,6 +2636,14 @@ static int wcd938x_mbhc_init(struct snd_soc_component *component)
>
> return 0;
> }
> +
> +static void wcd938x_mbhc_deinit(struct snd_soc_component *component)
> +{
> + struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
> +
> + wcd_mbhc_deinit(wcd938x->wcd_mbhc);
> +}
> +
> /* END MBHC */
>
> static const struct snd_kcontrol_new wcd938x_snd_controls[] = {
> @@ -3131,20 +3139,26 @@ static int wcd938x_soc_codec_probe(struct snd_soc_component *component)
> ret = request_threaded_irq(wcd938x->hphr_pdm_wd_int, NULL, wcd938x_wd_handle_irq,
> IRQF_ONESHOT | IRQF_TRIGGER_RISING,
> "HPHR PDM WD INT", wcd938x);
> - if (ret)
> + if (ret) {
> dev_err(dev, "Failed to request HPHR WD interrupt (%d)\n", ret);
> + goto err_free_clsh_ctrl;
> + }
>
> ret = request_threaded_irq(wcd938x->hphl_pdm_wd_int, NULL, wcd938x_wd_handle_irq,
> IRQF_ONESHOT | IRQF_TRIGGER_RISING,
> "HPHL PDM WD INT", wcd938x);
> - if (ret)
> + if (ret) {
> dev_err(dev, "Failed to request HPHL WD interrupt (%d)\n", ret);
> + goto err_free_hphr_pdm_wd_int;
> + }
>
> ret = request_threaded_irq(wcd938x->aux_pdm_wd_int, NULL, wcd938x_wd_handle_irq,
> IRQF_ONESHOT | IRQF_TRIGGER_RISING,
> "AUX PDM WD INT", wcd938x);
> - if (ret)
> + if (ret) {
> dev_err(dev, "Failed to request Aux WD interrupt (%d)\n", ret);
> + goto err_free_hphl_pdm_wd_int;
> + }
>
> /* Disable watchdog interrupt for HPH and AUX */
> disable_irq_nosync(wcd938x->hphr_pdm_wd_int);
> @@ -3159,7 +3173,7 @@ static int wcd938x_soc_codec_probe(struct snd_soc_component *component)
> dev_err(component->dev,
> "%s: Failed to add snd ctrls for variant: %d\n",
> __func__, wcd938x->variant);
> - goto err;
> + goto err_free_aux_pdm_wd_int;
> }
> break;
> case WCD9385:
> @@ -3169,7 +3183,7 @@ static int wcd938x_soc_codec_probe(struct snd_soc_component *component)
> dev_err(component->dev,
> "%s: Failed to add snd ctrls for variant: %d\n",
> __func__, wcd938x->variant);
> - goto err;
> + goto err_free_aux_pdm_wd_int;
> }
> break;
> default:
> @@ -3177,12 +3191,38 @@ static int wcd938x_soc_codec_probe(struct snd_soc_component *component)
> }
>
> ret = wcd938x_mbhc_init(component);
> - if (ret)
> + if (ret) {
> dev_err(component->dev, "mbhc initialization failed\n");
> -err:
> + goto err_free_aux_pdm_wd_int;
> + }
> +
> + return 0;
> +
> +err_free_aux_pdm_wd_int:
> + free_irq(wcd938x->aux_pdm_wd_int, wcd938x);
> +err_free_hphl_pdm_wd_int:
> + free_irq(wcd938x->hphl_pdm_wd_int, wcd938x);
> +err_free_hphr_pdm_wd_int:
> + free_irq(wcd938x->hphr_pdm_wd_int, wcd938x);
> +err_free_clsh_ctrl:
> + wcd_clsh_ctrl_free(wcd938x->clsh_info);
> +
> return ret;
> }
>
> +static void wcd938x_soc_codec_remove(struct snd_soc_component *component)
> +{
> + struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
> +
> + wcd938x_mbhc_deinit(component);
> +
> + free_irq(wcd938x->aux_pdm_wd_int, wcd938x);
> + free_irq(wcd938x->hphl_pdm_wd_int, wcd938x);
> + free_irq(wcd938x->hphr_pdm_wd_int, wcd938x);
> +
> + wcd_clsh_ctrl_free(wcd938x->clsh_info);
> +}
> +
> static int wcd938x_codec_set_jack(struct snd_soc_component *comp,
> struct snd_soc_jack *jack, void *data)
> {
> @@ -3199,6 +3239,7 @@ static int wcd938x_codec_set_jack(struct snd_soc_component *comp,
> static const struct snd_soc_component_driver soc_codec_dev_wcd938x = {
> .name = "wcd938x_codec",
> .probe = wcd938x_soc_codec_probe,
> + .remove = wcd938x_soc_codec_remove,
> .controls = wcd938x_snd_controls,
> .num_controls = ARRAY_SIZE(wcd938x_snd_controls),
> .dapm_widgets = wcd938x_dapm_widgets,