Re: [PATCH 6/8] ASoC: codecs: wcd-mbhc-v2: fix resource leaks on component remove

[Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>]

On 05/07/2023 13:30, Johan Hovold wrote:
> The MBHC resources must be released on component probe failure and
> removal so can not be tied to the lifetime of the component device.
>
> 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 299. 00002001 (mbhc sw intr) vs. 00002001 (mbhc sw intr)
>      wcd938x_codec audio-codec: Failed to request mbhc interrupts -16
>      wcd938x_codec audio-codec: mbhc initialization failed
>      wcd938x_codec audio-codec: ASoC: error at snd_soc_component_probe on audio-codec: -16
>      snd-sc8280xp sound: ASoC: failed to instantiate card -16
>
> Fixes: 0e5c9e7ff899 ("ASoC: codecs: wcd: add multi button Headset detection support")
> 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>

--srini
>   sound/soc/codecs/wcd-mbhc-v2.c | 57 ++++++++++++++++++++++++----------
>   1 file changed, 41 insertions(+), 16 deletions(-)
>
> diff --git a/sound/soc/codecs/wcd-mbhc-v2.c b/sound/soc/codecs/wcd-mbhc-v2.c
> index 1911750f7445..5da1934527f3 100644
> --- a/sound/soc/codecs/wcd-mbhc-v2.c
> +++ b/sound/soc/codecs/wcd-mbhc-v2.c
> @@ -1454,7 +1454,7 @@ struct wcd_mbhc *wcd_mbhc_init(struct snd_soc_component *component,
>   		return ERR_PTR(-EINVAL);
>   	}
>   
> -	mbhc = devm_kzalloc(dev, sizeof(*mbhc), GFP_KERNEL);
> +	mbhc = kzalloc(sizeof(*mbhc), GFP_KERNEL);
>   	if (!mbhc)
>   		return ERR_PTR(-ENOMEM);
>   
> @@ -1474,61 +1474,76 @@ struct wcd_mbhc *wcd_mbhc_init(struct snd_soc_component *component,
>   
>   	INIT_WORK(&mbhc->correct_plug_swch, wcd_correct_swch_plug);
>   
> -	ret = devm_request_threaded_irq(dev, mbhc->intr_ids->mbhc_sw_intr, NULL,
> +	ret = request_threaded_irq(mbhc->intr_ids->mbhc_sw_intr, NULL,
>   					wcd_mbhc_mech_plug_detect_irq,
>   					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
>   					"mbhc sw intr", mbhc);
>   	if (ret)
> -		goto err;
> +		goto err_free_mbhc;
>   
> -	ret = devm_request_threaded_irq(dev, mbhc->intr_ids->mbhc_btn_press_intr, NULL,
> +	ret = request_threaded_irq(mbhc->intr_ids->mbhc_btn_press_intr, NULL,
>   					wcd_mbhc_btn_press_handler,
>   					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
>   					"Button Press detect", mbhc);
>   	if (ret)
> -		goto err;
> +		goto err_free_sw_intr;
>   
> -	ret = devm_request_threaded_irq(dev, mbhc->intr_ids->mbhc_btn_release_intr, NULL,
> +	ret = request_threaded_irq(mbhc->intr_ids->mbhc_btn_release_intr, NULL,
>   					wcd_mbhc_btn_release_handler,
>   					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
>   					"Button Release detect", mbhc);
>   	if (ret)
> -		goto err;
> +		goto err_free_btn_press_intr;
>   
> -	ret = devm_request_threaded_irq(dev, mbhc->intr_ids->mbhc_hs_ins_intr, NULL,
> +	ret = request_threaded_irq(mbhc->intr_ids->mbhc_hs_ins_intr, NULL,
>   					wcd_mbhc_adc_hs_ins_irq,
>   					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
>   					"Elect Insert", mbhc);
>   	if (ret)
> -		goto err;
> +		goto err_free_btn_release_intr;
>   
>   	disable_irq_nosync(mbhc->intr_ids->mbhc_hs_ins_intr);
>   
> -	ret = devm_request_threaded_irq(dev, mbhc->intr_ids->mbhc_hs_rem_intr, NULL,
> +	ret = request_threaded_irq(mbhc->intr_ids->mbhc_hs_rem_intr, NULL,
>   					wcd_mbhc_adc_hs_rem_irq,
>   					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
>   					"Elect Remove", mbhc);
>   	if (ret)
> -		goto err;
> +		goto err_free_hs_ins_intr;
>   
>   	disable_irq_nosync(mbhc->intr_ids->mbhc_hs_rem_intr);
>   
> -	ret = devm_request_threaded_irq(dev, mbhc->intr_ids->hph_left_ocp, NULL,
> +	ret = request_threaded_irq(mbhc->intr_ids->hph_left_ocp, NULL,
>   					wcd_mbhc_hphl_ocp_irq,
>   					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
>   					"HPH_L OCP detect", mbhc);
>   	if (ret)
> -		goto err;
> +		goto err_free_hs_rem_intr;
>   
> -	ret = devm_request_threaded_irq(dev, mbhc->intr_ids->hph_right_ocp, NULL,
> +	ret = request_threaded_irq(mbhc->intr_ids->hph_right_ocp, NULL,
>   					wcd_mbhc_hphr_ocp_irq,
>   					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
>   					"HPH_R OCP detect", mbhc);
>   	if (ret)
> -		goto err;
> +		goto err_free_hph_left_ocp;
>   
>   	return mbhc;
> -err:
> +
> +err_free_hph_left_ocp:
> +	free_irq(mbhc->intr_ids->hph_left_ocp, mbhc);
> +err_free_hs_rem_intr:
> +	free_irq(mbhc->intr_ids->mbhc_hs_rem_intr, mbhc);
> +err_free_hs_ins_intr:
> +	free_irq(mbhc->intr_ids->mbhc_hs_ins_intr, mbhc);
> +err_free_btn_release_intr:
> +	free_irq(mbhc->intr_ids->mbhc_btn_release_intr, mbhc);
> +err_free_btn_press_intr:
> +	free_irq(mbhc->intr_ids->mbhc_btn_press_intr, mbhc);
> +err_free_sw_intr:
> +	free_irq(mbhc->intr_ids->mbhc_sw_intr, mbhc);
> +err_free_mbhc:
> +	kfree(mbhc);
> +
>   	dev_err(dev, "Failed to request mbhc interrupts %d\n", ret);
>   
>   	return ERR_PTR(ret);
> @@ -1537,9 +1552,19 @@ EXPORT_SYMBOL(wcd_mbhc_init);
>   
>   void wcd_mbhc_deinit(struct wcd_mbhc *mbhc)
>   {
> +	free_irq(mbhc->intr_ids->hph_right_ocp, mbhc);
> +	free_irq(mbhc->intr_ids->hph_left_ocp, mbhc);
> +	free_irq(mbhc->intr_ids->mbhc_hs_rem_intr, mbhc);
> +	free_irq(mbhc->intr_ids->mbhc_hs_ins_intr, mbhc);
> +	free_irq(mbhc->intr_ids->mbhc_btn_release_intr, mbhc);
> +	free_irq(mbhc->intr_ids->mbhc_btn_press_intr, mbhc);
> +	free_irq(mbhc->intr_ids->mbhc_sw_intr, mbhc);
> +
>   	mutex_lock(&mbhc->lock);
>   	wcd_cancel_hs_detect_plug(mbhc,	&mbhc->correct_plug_swch);
>   	mutex_unlock(&mbhc->lock);
> +
> +	kfree(mbhc);
>   }
>   EXPORT_SYMBOL(wcd_mbhc_deinit);
>