Commit 980d97ef authored by Mark Brown's avatar Mark Brown

ASoC/soundwire/qdsp6/wcd: fix leaks and probe deferral

Merge series from Johan Hovold <johan+linaro@kernel.org>:

I've been hitting a race during boot which breaks probe of the sound
card on the Lenovo ThinkPad X13s as I've previously reported here:

	https://lore.kernel.org/all/ZIHMMFtuDtvdpFAZ@hovoldconsulting.com/

The immediate issue appeared to be a probe deferral that was turned into
a hard failure, but addressing that in itself only made things worse as
it exposed further bugs.

I was hoping someone more familiar with the code in question would look
into this, but as this affects users of the X13s and breaks audio on my
machine every fifth boot or so, I decided to investigate it myself.

As expected, the Qualcomm codec drivers are broken and specifically leak
resources on component remove, which in turn breaks sound card probe
deferrals.

The source of the deferral itself appears to be legitimate and was
simply due to some audio component not yet having been registered due to
random changes in timing during boot.

These issues can most easily be reproduced by simply blacklisting the
q6apm_dai module and loading it manually after boot.

Included are also two patches that suppresses error messages on
component probe deferral to avoid spamming the logs during boot.
parents e231cd83 f09b6e96
......@@ -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);
......
......@@ -3044,6 +3044,17 @@ static int wcd934x_mbhc_init(struct snd_soc_component *component)
return 0;
}
static void wcd934x_mbhc_deinit(struct snd_soc_component *component)
{
struct wcd934x_codec *wcd = snd_soc_component_get_drvdata(component);
if (!wcd->mbhc)
return;
wcd_mbhc_deinit(wcd->mbhc);
}
static int wcd934x_comp_probe(struct snd_soc_component *component)
{
struct wcd934x_codec *wcd = dev_get_drvdata(component->dev);
......@@ -3077,6 +3088,7 @@ static void wcd934x_comp_remove(struct snd_soc_component *comp)
{
struct wcd934x_codec *wcd = dev_get_drvdata(comp->dev);
wcd934x_mbhc_deinit(comp);
wcd_clsh_ctrl_free(wcd->clsh_ctrl);
}
......
......@@ -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[] = {
......@@ -3106,6 +3114,10 @@ static int wcd938x_soc_codec_probe(struct snd_soc_component *component)
WCD938X_ID_MASK);
wcd938x->clsh_info = wcd_clsh_ctrl_alloc(component, WCD938X);
if (IS_ERR(wcd938x->clsh_info)) {
pm_runtime_put(dev);
return PTR_ERR(wcd938x->clsh_info);
}
wcd938x_io_init(wcd938x);
/* Set all interrupts as edge triggered */
......@@ -3127,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);
......@@ -3155,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:
......@@ -3165,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:
......@@ -3173,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)
{
......@@ -3195,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,
......
......@@ -1277,8 +1277,8 @@ int audioreach_tplg_init(struct snd_soc_component *component)
ret = snd_soc_tplg_component_load(component, &audioreach_tplg_ops, fw);
if (ret < 0) {
dev_err(dev, "tplg component load failed%d\n", ret);
ret = -EINVAL;
if (ret != -EPROBE_DEFER)
dev_err(dev, "tplg component load failed: %d\n", ret);
}
release_firmware(fw);
......
......@@ -1988,8 +1988,10 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
/* probe all components used by DAI links on this card */
ret = soc_probe_link_components(card);
if (ret < 0) {
dev_err(card->dev,
"ASoC: failed to instantiate card %d\n", ret);
if (ret != -EPROBE_DEFER) {
dev_err(card->dev,
"ASoC: failed to instantiate card %d\n", ret);
}
goto probe_end;
}
......
......@@ -1732,7 +1732,8 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg,
ret = snd_soc_add_pcm_runtimes(tplg->comp->card, link, 1);
if (ret < 0) {
dev_err(tplg->dev, "ASoC: adding FE link failed\n");
if (ret != -EPROBE_DEFER)
dev_err(tplg->dev, "ASoC: adding FE link failed\n");
goto err;
}
......@@ -2492,8 +2493,11 @@ static int soc_tplg_process_headers(struct soc_tplg *tplg)
/* load the header object */
ret = soc_tplg_load_header(tplg, hdr);
if (ret < 0) {
dev_err(tplg->dev,
"ASoC: topology: could not load header: %d\n", ret);
if (ret != -EPROBE_DEFER) {
dev_err(tplg->dev,
"ASoC: topology: could not load header: %d\n",
ret);
}
return ret;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment