Skip to content

Commit e182212

Browse files
committed
ASoC: codecs: wcd938x: fix probe and bind error
Merge series from Johan Hovold <[email protected]>: 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/[email protected]/ Fix up the remaining probe and component bind paths that left resources allocated and registered after errors to avoid similar future issues.
2 parents aa6464e + c5c0383 commit e182212

File tree

2 files changed

+83
-20
lines changed

2 files changed

+83
-20
lines changed

sound/soc/codecs/wcd938x-sdw.c

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1278,7 +1278,31 @@ static int wcd9380_probe(struct sdw_slave *pdev,
12781278
pm_runtime_set_active(dev);
12791279
pm_runtime_enable(dev);
12801280

1281-
return component_add(dev, &wcd938x_sdw_component_ops);
1281+
ret = component_add(dev, &wcd938x_sdw_component_ops);
1282+
if (ret)
1283+
goto err_disable_rpm;
1284+
1285+
return 0;
1286+
1287+
err_disable_rpm:
1288+
pm_runtime_disable(dev);
1289+
pm_runtime_set_suspended(dev);
1290+
pm_runtime_dont_use_autosuspend(dev);
1291+
1292+
return ret;
1293+
}
1294+
1295+
static int wcd9380_remove(struct sdw_slave *pdev)
1296+
{
1297+
struct device *dev = &pdev->dev;
1298+
1299+
component_del(dev, &wcd938x_sdw_component_ops);
1300+
1301+
pm_runtime_disable(dev);
1302+
pm_runtime_set_suspended(dev);
1303+
pm_runtime_dont_use_autosuspend(dev);
1304+
1305+
return 0;
12821306
}
12831307

12841308
static const struct sdw_device_id wcd9380_slave_id[] = {
@@ -1320,6 +1344,7 @@ static const struct dev_pm_ops wcd938x_sdw_pm_ops = {
13201344

13211345
static struct sdw_driver wcd9380_codec_driver = {
13221346
.probe = wcd9380_probe,
1347+
.remove = wcd9380_remove,
13231348
.ops = &wcd9380_slave_ops,
13241349
.id_table = wcd9380_slave_id,
13251350
.driver = {

sound/soc/codecs/wcd938x.c

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3325,8 +3325,10 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
33253325
return dev_err_probe(dev, ret, "Failed to get supplies\n");
33263326

33273327
ret = regulator_bulk_enable(WCD938X_MAX_SUPPLY, wcd938x->supplies);
3328-
if (ret)
3328+
if (ret) {
3329+
regulator_bulk_free(WCD938X_MAX_SUPPLY, wcd938x->supplies);
33293330
return dev_err_probe(dev, ret, "Failed to enable supplies\n");
3331+
}
33303332

33313333
wcd938x_dt_parse_micbias_info(dev, wcd938x);
33323334

@@ -3435,54 +3437,56 @@ static int wcd938x_bind(struct device *dev)
34353437
wcd938x->rxdev = wcd938x_sdw_device_get(wcd938x->rxnode);
34363438
if (!wcd938x->rxdev) {
34373439
dev_err(dev, "could not find slave with matching of node\n");
3438-
return -EINVAL;
3440+
ret = -EINVAL;
3441+
goto err_unbind;
34393442
}
34403443
wcd938x->sdw_priv[AIF1_PB] = dev_get_drvdata(wcd938x->rxdev);
34413444
wcd938x->sdw_priv[AIF1_PB]->wcd938x = wcd938x;
34423445

34433446
wcd938x->txdev = wcd938x_sdw_device_get(wcd938x->txnode);
34443447
if (!wcd938x->txdev) {
34453448
dev_err(dev, "could not find txslave with matching of node\n");
3446-
return -EINVAL;
3449+
ret = -EINVAL;
3450+
goto err_put_rxdev;
34473451
}
34483452
wcd938x->sdw_priv[AIF1_CAP] = dev_get_drvdata(wcd938x->txdev);
34493453
wcd938x->sdw_priv[AIF1_CAP]->wcd938x = wcd938x;
34503454
wcd938x->tx_sdw_dev = dev_to_sdw_dev(wcd938x->txdev);
3451-
if (!wcd938x->tx_sdw_dev) {
3452-
dev_err(dev, "could not get txslave with matching of dev\n");
3453-
return -EINVAL;
3454-
}
34553455

34563456
/* As TX is main CSR reg interface, which should not be suspended first.
34573457
* expicilty add the dependency link */
34583458
if (!device_link_add(wcd938x->rxdev, wcd938x->txdev, DL_FLAG_STATELESS |
34593459
DL_FLAG_PM_RUNTIME)) {
34603460
dev_err(dev, "could not devlink tx and rx\n");
3461-
return -EINVAL;
3461+
ret = -EINVAL;
3462+
goto err_put_txdev;
34623463
}
34633464

34643465
if (!device_link_add(dev, wcd938x->txdev, DL_FLAG_STATELESS |
34653466
DL_FLAG_PM_RUNTIME)) {
34663467
dev_err(dev, "could not devlink wcd and tx\n");
3467-
return -EINVAL;
3468+
ret = -EINVAL;
3469+
goto err_remove_rxtx_link;
34683470
}
34693471

34703472
if (!device_link_add(dev, wcd938x->rxdev, DL_FLAG_STATELESS |
34713473
DL_FLAG_PM_RUNTIME)) {
34723474
dev_err(dev, "could not devlink wcd and rx\n");
3473-
return -EINVAL;
3475+
ret = -EINVAL;
3476+
goto err_remove_tx_link;
34743477
}
34753478

34763479
wcd938x->regmap = dev_get_regmap(&wcd938x->tx_sdw_dev->dev, NULL);
34773480
if (!wcd938x->regmap) {
34783481
dev_err(dev, "could not get TX device regmap\n");
3479-
return -EINVAL;
3482+
ret = -EINVAL;
3483+
goto err_remove_rx_link;
34803484
}
34813485

34823486
ret = wcd938x_irq_init(wcd938x, dev);
34833487
if (ret) {
34843488
dev_err(dev, "%s: IRQ init failed: %d\n", __func__, ret);
3485-
return ret;
3489+
goto err_remove_rx_link;
34863490
}
34873491

34883492
wcd938x->sdw_priv[AIF1_PB]->slave_irq = wcd938x->virq;
@@ -3491,27 +3495,45 @@ static int wcd938x_bind(struct device *dev)
34913495
ret = wcd938x_set_micbias_data(wcd938x);
34923496
if (ret < 0) {
34933497
dev_err(dev, "%s: bad micbias pdata\n", __func__);
3494-
return ret;
3498+
goto err_remove_rx_link;
34953499
}
34963500

34973501
ret = snd_soc_register_component(dev, &soc_codec_dev_wcd938x,
34983502
wcd938x_dais, ARRAY_SIZE(wcd938x_dais));
3499-
if (ret)
3503+
if (ret) {
35003504
dev_err(dev, "%s: Codec registration failed\n",
35013505
__func__);
3506+
goto err_remove_rx_link;
3507+
}
35023508

3503-
return ret;
3509+
return 0;
35043510

3511+
err_remove_rx_link:
3512+
device_link_remove(dev, wcd938x->rxdev);
3513+
err_remove_tx_link:
3514+
device_link_remove(dev, wcd938x->txdev);
3515+
err_remove_rxtx_link:
3516+
device_link_remove(wcd938x->rxdev, wcd938x->txdev);
3517+
err_put_txdev:
3518+
put_device(wcd938x->txdev);
3519+
err_put_rxdev:
3520+
put_device(wcd938x->rxdev);
3521+
err_unbind:
3522+
component_unbind_all(dev, wcd938x);
3523+
3524+
return ret;
35053525
}
35063526

35073527
static void wcd938x_unbind(struct device *dev)
35083528
{
35093529
struct wcd938x_priv *wcd938x = dev_get_drvdata(dev);
35103530

3531+
snd_soc_unregister_component(dev);
35113532
device_link_remove(dev, wcd938x->txdev);
35123533
device_link_remove(dev, wcd938x->rxdev);
35133534
device_link_remove(wcd938x->rxdev, wcd938x->txdev);
3514-
snd_soc_unregister_component(dev);
3535+
put_device(wcd938x->txdev);
3536+
put_device(wcd938x->rxdev);
35153537
component_unbind_all(dev, wcd938x);
35163538
}
35173539

@@ -3572,13 +3594,13 @@ static int wcd938x_probe(struct platform_device *pdev)
35723594

35733595
ret = wcd938x_add_slave_components(wcd938x, dev, &match);
35743596
if (ret)
3575-
return ret;
3597+
goto err_disable_regulators;
35763598

35773599
wcd938x_reset(wcd938x);
35783600

35793601
ret = component_master_add_with_match(dev, &wcd938x_comp_ops, match);
35803602
if (ret)
3581-
return ret;
3603+
goto err_disable_regulators;
35823604

35833605
pm_runtime_set_autosuspend_delay(dev, 1000);
35843606
pm_runtime_use_autosuspend(dev);
@@ -3588,11 +3610,27 @@ static int wcd938x_probe(struct platform_device *pdev)
35883610
pm_runtime_idle(dev);
35893611

35903612
return 0;
3613+
3614+
err_disable_regulators:
3615+
regulator_bulk_disable(WCD938X_MAX_SUPPLY, wcd938x->supplies);
3616+
regulator_bulk_free(WCD938X_MAX_SUPPLY, wcd938x->supplies);
3617+
3618+
return ret;
35913619
}
35923620

35933621
static void wcd938x_remove(struct platform_device *pdev)
35943622
{
3595-
component_master_del(&pdev->dev, &wcd938x_comp_ops);
3623+
struct device *dev = &pdev->dev;
3624+
struct wcd938x_priv *wcd938x = dev_get_drvdata(dev);
3625+
3626+
component_master_del(dev, &wcd938x_comp_ops);
3627+
3628+
pm_runtime_disable(dev);
3629+
pm_runtime_set_suspended(dev);
3630+
pm_runtime_dont_use_autosuspend(dev);
3631+
3632+
regulator_bulk_disable(WCD938X_MAX_SUPPLY, wcd938x->supplies);
3633+
regulator_bulk_free(WCD938X_MAX_SUPPLY, wcd938x->supplies);
35963634
}
35973635

35983636
#if defined(CONFIG_OF)

0 commit comments

Comments
 (0)