Commit e1822121 authored by Mark Brown's avatar Mark Brown

ASoC: codecs: wcd938x: fix probe and bind error

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

The wcd938x codec driver happily ignores error handling, something which
has bitten us in the past when we hit a probe deferral:

	https://lore.kernel.org/lkml/20230705123018.30903-1-johan+linaro@kernel.org/

Fix up the remaining probe and component bind paths that left resources
allocated and registered after errors to avoid similar future issues.
parents aa6464ed c5c03830
...@@ -1278,7 +1278,31 @@ static int wcd9380_probe(struct sdw_slave *pdev, ...@@ -1278,7 +1278,31 @@ static int wcd9380_probe(struct sdw_slave *pdev,
pm_runtime_set_active(dev); pm_runtime_set_active(dev);
pm_runtime_enable(dev); pm_runtime_enable(dev);
return component_add(dev, &wcd938x_sdw_component_ops); ret = component_add(dev, &wcd938x_sdw_component_ops);
if (ret)
goto err_disable_rpm;
return 0;
err_disable_rpm:
pm_runtime_disable(dev);
pm_runtime_set_suspended(dev);
pm_runtime_dont_use_autosuspend(dev);
return ret;
}
static int wcd9380_remove(struct sdw_slave *pdev)
{
struct device *dev = &pdev->dev;
component_del(dev, &wcd938x_sdw_component_ops);
pm_runtime_disable(dev);
pm_runtime_set_suspended(dev);
pm_runtime_dont_use_autosuspend(dev);
return 0;
} }
static const struct sdw_device_id wcd9380_slave_id[] = { static const struct sdw_device_id wcd9380_slave_id[] = {
...@@ -1320,6 +1344,7 @@ static const struct dev_pm_ops wcd938x_sdw_pm_ops = { ...@@ -1320,6 +1344,7 @@ static const struct dev_pm_ops wcd938x_sdw_pm_ops = {
static struct sdw_driver wcd9380_codec_driver = { static struct sdw_driver wcd9380_codec_driver = {
.probe = wcd9380_probe, .probe = wcd9380_probe,
.remove = wcd9380_remove,
.ops = &wcd9380_slave_ops, .ops = &wcd9380_slave_ops,
.id_table = wcd9380_slave_id, .id_table = wcd9380_slave_id,
.driver = { .driver = {
......
...@@ -3325,8 +3325,10 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device ...@@ -3325,8 +3325,10 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
return dev_err_probe(dev, ret, "Failed to get supplies\n"); return dev_err_probe(dev, ret, "Failed to get supplies\n");
ret = regulator_bulk_enable(WCD938X_MAX_SUPPLY, wcd938x->supplies); ret = regulator_bulk_enable(WCD938X_MAX_SUPPLY, wcd938x->supplies);
if (ret) if (ret) {
regulator_bulk_free(WCD938X_MAX_SUPPLY, wcd938x->supplies);
return dev_err_probe(dev, ret, "Failed to enable supplies\n"); return dev_err_probe(dev, ret, "Failed to enable supplies\n");
}
wcd938x_dt_parse_micbias_info(dev, wcd938x); wcd938x_dt_parse_micbias_info(dev, wcd938x);
...@@ -3435,7 +3437,8 @@ static int wcd938x_bind(struct device *dev) ...@@ -3435,7 +3437,8 @@ static int wcd938x_bind(struct device *dev)
wcd938x->rxdev = wcd938x_sdw_device_get(wcd938x->rxnode); wcd938x->rxdev = wcd938x_sdw_device_get(wcd938x->rxnode);
if (!wcd938x->rxdev) { if (!wcd938x->rxdev) {
dev_err(dev, "could not find slave with matching of node\n"); dev_err(dev, "could not find slave with matching of node\n");
return -EINVAL; ret = -EINVAL;
goto err_unbind;
} }
wcd938x->sdw_priv[AIF1_PB] = dev_get_drvdata(wcd938x->rxdev); wcd938x->sdw_priv[AIF1_PB] = dev_get_drvdata(wcd938x->rxdev);
wcd938x->sdw_priv[AIF1_PB]->wcd938x = wcd938x; wcd938x->sdw_priv[AIF1_PB]->wcd938x = wcd938x;
...@@ -3443,46 +3446,47 @@ static int wcd938x_bind(struct device *dev) ...@@ -3443,46 +3446,47 @@ static int wcd938x_bind(struct device *dev)
wcd938x->txdev = wcd938x_sdw_device_get(wcd938x->txnode); wcd938x->txdev = wcd938x_sdw_device_get(wcd938x->txnode);
if (!wcd938x->txdev) { if (!wcd938x->txdev) {
dev_err(dev, "could not find txslave with matching of node\n"); dev_err(dev, "could not find txslave with matching of node\n");
return -EINVAL; ret = -EINVAL;
goto err_put_rxdev;
} }
wcd938x->sdw_priv[AIF1_CAP] = dev_get_drvdata(wcd938x->txdev); wcd938x->sdw_priv[AIF1_CAP] = dev_get_drvdata(wcd938x->txdev);
wcd938x->sdw_priv[AIF1_CAP]->wcd938x = wcd938x; wcd938x->sdw_priv[AIF1_CAP]->wcd938x = wcd938x;
wcd938x->tx_sdw_dev = dev_to_sdw_dev(wcd938x->txdev); wcd938x->tx_sdw_dev = dev_to_sdw_dev(wcd938x->txdev);
if (!wcd938x->tx_sdw_dev) {
dev_err(dev, "could not get txslave with matching of dev\n");
return -EINVAL;
}
/* As TX is main CSR reg interface, which should not be suspended first. /* As TX is main CSR reg interface, which should not be suspended first.
* expicilty add the dependency link */ * expicilty add the dependency link */
if (!device_link_add(wcd938x->rxdev, wcd938x->txdev, DL_FLAG_STATELESS | if (!device_link_add(wcd938x->rxdev, wcd938x->txdev, DL_FLAG_STATELESS |
DL_FLAG_PM_RUNTIME)) { DL_FLAG_PM_RUNTIME)) {
dev_err(dev, "could not devlink tx and rx\n"); dev_err(dev, "could not devlink tx and rx\n");
return -EINVAL; ret = -EINVAL;
goto err_put_txdev;
} }
if (!device_link_add(dev, wcd938x->txdev, DL_FLAG_STATELESS | if (!device_link_add(dev, wcd938x->txdev, DL_FLAG_STATELESS |
DL_FLAG_PM_RUNTIME)) { DL_FLAG_PM_RUNTIME)) {
dev_err(dev, "could not devlink wcd and tx\n"); dev_err(dev, "could not devlink wcd and tx\n");
return -EINVAL; ret = -EINVAL;
goto err_remove_rxtx_link;
} }
if (!device_link_add(dev, wcd938x->rxdev, DL_FLAG_STATELESS | if (!device_link_add(dev, wcd938x->rxdev, DL_FLAG_STATELESS |
DL_FLAG_PM_RUNTIME)) { DL_FLAG_PM_RUNTIME)) {
dev_err(dev, "could not devlink wcd and rx\n"); dev_err(dev, "could not devlink wcd and rx\n");
return -EINVAL; ret = -EINVAL;
goto err_remove_tx_link;
} }
wcd938x->regmap = dev_get_regmap(&wcd938x->tx_sdw_dev->dev, NULL); wcd938x->regmap = dev_get_regmap(&wcd938x->tx_sdw_dev->dev, NULL);
if (!wcd938x->regmap) { if (!wcd938x->regmap) {
dev_err(dev, "could not get TX device regmap\n"); dev_err(dev, "could not get TX device regmap\n");
return -EINVAL; ret = -EINVAL;
goto err_remove_rx_link;
} }
ret = wcd938x_irq_init(wcd938x, dev); ret = wcd938x_irq_init(wcd938x, dev);
if (ret) { if (ret) {
dev_err(dev, "%s: IRQ init failed: %d\n", __func__, ret); dev_err(dev, "%s: IRQ init failed: %d\n", __func__, ret);
return ret; goto err_remove_rx_link;
} }
wcd938x->sdw_priv[AIF1_PB]->slave_irq = wcd938x->virq; wcd938x->sdw_priv[AIF1_PB]->slave_irq = wcd938x->virq;
...@@ -3491,27 +3495,45 @@ static int wcd938x_bind(struct device *dev) ...@@ -3491,27 +3495,45 @@ static int wcd938x_bind(struct device *dev)
ret = wcd938x_set_micbias_data(wcd938x); ret = wcd938x_set_micbias_data(wcd938x);
if (ret < 0) { if (ret < 0) {
dev_err(dev, "%s: bad micbias pdata\n", __func__); dev_err(dev, "%s: bad micbias pdata\n", __func__);
return ret; goto err_remove_rx_link;
} }
ret = snd_soc_register_component(dev, &soc_codec_dev_wcd938x, ret = snd_soc_register_component(dev, &soc_codec_dev_wcd938x,
wcd938x_dais, ARRAY_SIZE(wcd938x_dais)); wcd938x_dais, ARRAY_SIZE(wcd938x_dais));
if (ret) if (ret) {
dev_err(dev, "%s: Codec registration failed\n", dev_err(dev, "%s: Codec registration failed\n",
__func__); __func__);
goto err_remove_rx_link;
}
return ret; return 0;
err_remove_rx_link:
device_link_remove(dev, wcd938x->rxdev);
err_remove_tx_link:
device_link_remove(dev, wcd938x->txdev);
err_remove_rxtx_link:
device_link_remove(wcd938x->rxdev, wcd938x->txdev);
err_put_txdev:
put_device(wcd938x->txdev);
err_put_rxdev:
put_device(wcd938x->rxdev);
err_unbind:
component_unbind_all(dev, wcd938x);
return ret;
} }
static void wcd938x_unbind(struct device *dev) static void wcd938x_unbind(struct device *dev)
{ {
struct wcd938x_priv *wcd938x = dev_get_drvdata(dev); struct wcd938x_priv *wcd938x = dev_get_drvdata(dev);
snd_soc_unregister_component(dev);
device_link_remove(dev, wcd938x->txdev); device_link_remove(dev, wcd938x->txdev);
device_link_remove(dev, wcd938x->rxdev); device_link_remove(dev, wcd938x->rxdev);
device_link_remove(wcd938x->rxdev, wcd938x->txdev); device_link_remove(wcd938x->rxdev, wcd938x->txdev);
snd_soc_unregister_component(dev); put_device(wcd938x->txdev);
put_device(wcd938x->rxdev);
component_unbind_all(dev, wcd938x); component_unbind_all(dev, wcd938x);
} }
...@@ -3572,13 +3594,13 @@ static int wcd938x_probe(struct platform_device *pdev) ...@@ -3572,13 +3594,13 @@ static int wcd938x_probe(struct platform_device *pdev)
ret = wcd938x_add_slave_components(wcd938x, dev, &match); ret = wcd938x_add_slave_components(wcd938x, dev, &match);
if (ret) if (ret)
return ret; goto err_disable_regulators;
wcd938x_reset(wcd938x); wcd938x_reset(wcd938x);
ret = component_master_add_with_match(dev, &wcd938x_comp_ops, match); ret = component_master_add_with_match(dev, &wcd938x_comp_ops, match);
if (ret) if (ret)
return ret; goto err_disable_regulators;
pm_runtime_set_autosuspend_delay(dev, 1000); pm_runtime_set_autosuspend_delay(dev, 1000);
pm_runtime_use_autosuspend(dev); pm_runtime_use_autosuspend(dev);
...@@ -3588,11 +3610,27 @@ static int wcd938x_probe(struct platform_device *pdev) ...@@ -3588,11 +3610,27 @@ static int wcd938x_probe(struct platform_device *pdev)
pm_runtime_idle(dev); pm_runtime_idle(dev);
return 0; return 0;
err_disable_regulators:
regulator_bulk_disable(WCD938X_MAX_SUPPLY, wcd938x->supplies);
regulator_bulk_free(WCD938X_MAX_SUPPLY, wcd938x->supplies);
return ret;
} }
static void wcd938x_remove(struct platform_device *pdev) static void wcd938x_remove(struct platform_device *pdev)
{ {
component_master_del(&pdev->dev, &wcd938x_comp_ops); struct device *dev = &pdev->dev;
struct wcd938x_priv *wcd938x = dev_get_drvdata(dev);
component_master_del(dev, &wcd938x_comp_ops);
pm_runtime_disable(dev);
pm_runtime_set_suspended(dev);
pm_runtime_dont_use_autosuspend(dev);
regulator_bulk_disable(WCD938X_MAX_SUPPLY, wcd938x->supplies);
regulator_bulk_free(WCD938X_MAX_SUPPLY, wcd938x->supplies);
} }
#if defined(CONFIG_OF) #if defined(CONFIG_OF)
......
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