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

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

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

--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);
>