Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <31bcbd57-1087-e8a4-6061-0fb89a82aec5@linaro.org>
Date:   Thu, 6 Jul 2023 12:09:47 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH 6/8] ASoC: codecs: wcd-mbhc-v2: fix resource leaks on
 component remove
Content-Language: en-US
To:     Johan Hovold <johan+linaro@kernel.org>,
        Mark Brown <broonie@kernel.org>, Vinod Koul <vkoul@kernel.org>
Cc:     Bard Liao <yung-chuan.liao@linux.intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
        Sanyog Kale <sanyog.r.kale@intel.com>,
        Banajit Goswami <bgoswami@quicinc.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>, alsa-devel@alsa-project.org,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
References: <20230705123018.30903-1-johan+linaro@kernel.org>
 <20230705123018.30903-7-johan+linaro@kernel.org>
From:   Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
In-Reply-To: <20230705123018.30903-7-johan+linaro@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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