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

[Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxx	# 5.14
> Cc: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> Signed-off-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
> ---

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


>   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,