ASoC: codecs: wcd937x: Fix error handling in wcd937x codec driver

[PATCH] ASoC: codecs: wcd937x: Fix error handling in wcd937x codec driver

Posted by Ma Ke 2 months, 3 weeks ago

In wcd937x_bind(), the driver calls of_sdw_find_device_by_node() to
obtain references to RX and TX SoundWire devices, which increment the
device reference counts. However, the corresponding put_device() are
missing in both the error paths and the normal unbind path in
wcd937x_unbind().

Add proper error handling with put_device() calls in all error paths
of wcd937x_bind() and ensure devices are released in wcd937x_unbind().

Found by code review.

Cc: stable@vger.kernel.org
Fixes: 772ed12bd04e ("ASoC: codecs: wcdxxxx: use of_sdw_find_device_by_node helper")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
 sound/soc/codecs/wcd937x.c | 43 ++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/wcd937x.c b/sound/soc/codecs/wcd937x.c
index 421ec7a2d6bd..ed0ff45a8964 100644
--- a/sound/soc/codecs/wcd937x.c
+++ b/sound/soc/codecs/wcd937x.c
@@ -2748,7 +2748,8 @@ static int wcd937x_bind(struct device *dev)
 	wcd937x->rxdev = of_sdw_find_device_by_node(wcd937x->rxnode);
 	if (!wcd937x->rxdev) {
 		dev_err(dev, "could not find slave with matching of node\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_component_unbind;
 	}
 
 	wcd937x->sdw_priv[AIF1_PB] = dev_get_drvdata(wcd937x->rxdev);
@@ -2757,7 +2758,8 @@ static int wcd937x_bind(struct device *dev)
 	wcd937x->txdev = of_sdw_find_device_by_node(wcd937x->txnode);
 	if (!wcd937x->txdev) {
 		dev_err(dev, "could not find txslave with matching of node\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_put_rxdev;
 	}
 
 	wcd937x->sdw_priv[AIF1_CAP] = dev_get_drvdata(wcd937x->txdev);
@@ -2765,7 +2767,8 @@ static int wcd937x_bind(struct device *dev)
 	wcd937x->tx_sdw_dev = dev_to_sdw_dev(wcd937x->txdev);
 	if (!wcd937x->tx_sdw_dev) {
 		dev_err(dev, "could not get txslave with matching of dev\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_put_txdev;
 	}
 
 	/*
@@ -2775,31 +2778,35 @@ static int wcd937x_bind(struct device *dev)
 	if (!device_link_add(wcd937x->rxdev, wcd937x->txdev,
 			     DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME)) {
 		dev_err(dev, "Could not devlink TX and RX\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_put_txdev;
 	}
 
 	if (!device_link_add(dev, wcd937x->txdev,
 			     DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME)) {
 		dev_err(dev, "Could not devlink WCD and TX\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_remove_link1;
 	}
 
 	if (!device_link_add(dev, wcd937x->rxdev,
 			     DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME)) {
 		dev_err(dev, "Could not devlink WCD and RX\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_remove_link2;
 	}
 
 	wcd937x->regmap = wcd937x->sdw_priv[AIF1_CAP]->regmap;
 	if (!wcd937x->regmap) {
 		dev_err(dev, "could not get TX device regmap\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_remove_link3;
 	}
 
 	ret = wcd937x_irq_init(wcd937x, dev);
 	if (ret) {
 		dev_err(dev, "IRQ init failed: %d\n", ret);
-		return ret;
+		goto err_remove_link3;
 	}
 
 	wcd937x->sdw_priv[AIF1_PB]->slave_irq = wcd937x->virq;
@@ -2809,10 +2816,26 @@ static int wcd937x_bind(struct device *dev)
 
 	ret = snd_soc_register_component(dev, &soc_codec_dev_wcd937x,
 					 wcd937x_dais, ARRAY_SIZE(wcd937x_dais));
-	if (ret)
+	if (ret) {
 		dev_err(dev, "Codec registration failed\n");
+		goto err_remove_link3;
+	}
 
 	return ret;
+
+err_remove_link3:
+	device_link_remove(dev, wcd937x->rxdev);
+err_remove_link2:
+	device_link_remove(dev, wcd937x->txdev);
+err_remove_link1:
+	device_link_remove(wcd937x->rxdev, wcd937x->txdev);
+err_put_txdev:
+	put_device(wcd937x->txdev);
+err_put_rxdev:
+	put_device(wcd937x->rxdev);
+err_component_unbind:
+	component_unbind_all(dev, wcd937x);
+	return ret;
 }
 
 static void wcd937x_unbind(struct device *dev)
@@ -2825,6 +2848,8 @@ static void wcd937x_unbind(struct device *dev)
 	device_link_remove(wcd937x->rxdev, wcd937x->txdev);
 	component_unbind_all(dev, wcd937x);
 	mutex_destroy(&wcd937x->micb_lock);
+	put_device(wcd937x->txdev);
+	put_device(wcd937x->rxdev);
 }
 
 static const struct component_master_ops wcd937x_comp_ops = {
-- 
2.17.1

Re: [PATCH] ASoC: codecs: wcd937x: Fix error handling in wcd937x codec driver

Posted by Mark Brown 2 months, 2 weeks ago

On Sun, 16 Nov 2025 14:16:23 +0800, Ma Ke wrote:
> In wcd937x_bind(), the driver calls of_sdw_find_device_by_node() to
> obtain references to RX and TX SoundWire devices, which increment the
> device reference counts. However, the corresponding put_device() are
> missing in both the error paths and the normal unbind path in
> wcd937x_unbind().
> 
> Add proper error handling with put_device() calls in all error paths
> of wcd937x_bind() and ensure devices are released in wcd937x_unbind().
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: codecs: wcd937x: Fix error handling in wcd937x codec driver
      commit: 578ccfe344c5f421c2c6343b872995b397ffd3ff

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH] ASoC: codecs: wcd937x: Fix error handling in wcd937x codec driver

Posted by David Heidelberg 2 months, 3 weeks ago

On 16/11/2025 07:16, Ma Ke wrote:
> In wcd937x_bind(), the driver calls of_sdw_find_device_by_node() to
> obtain references to RX and TX SoundWire devices, which increment the
> device reference counts. However, the corresponding put_device() are
> missing in both the error paths and the normal unbind path in
> wcd937x_unbind().
> 
> Add proper error handling with put_device() calls in all error paths
> of wcd937x_bind() and ensure devices are released in wcd937x_unbind().
> 
> Found by code review.
> 
> Cc: stable@vger.kernel.org
> Fixes: 772ed12bd04e ("ASoC: codecs: wcdxxxx: use of_sdw_find_device_by_node helper")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
>   sound/soc/codecs/wcd937x.c | 43 ++++++++++++++++++++++++++++++--------
>   1 file changed, 34 insertions(+), 9 deletions(-)
> 

[...]

> @@ -2809,10 +2816,26 @@ static int wcd937x_bind(struct device *dev)
>   
>   	ret = snd_soc_register_component(dev, &soc_codec_dev_wcd937x,
>   					 wcd937x_dais, ARRAY_SIZE(wcd937x_dais));
> -	if (ret)
> +	if (ret) {
>   		dev_err(dev, "Codec registration failed\n");
> +		goto err_remove_link3;
> +	}
>   
>   	return ret;
> +
> +err_remove_link3:
> +	device_link_remove(dev, wcd937x->rxdev);
> +err_remove_link2:
> +	device_link_remove(dev, wcd937x->txdev);
> +err_remove_link1:
> +	device_link_remove(wcd937x->rxdev, wcd937x->txdev);
> +err_put_txdev:
> +	put_device(wcd937x->txdev);
> +err_put_rxdev:
> +	put_device(wcd937x->rxdev);
> +err_component_unbind:
> +	component_unbind_all(dev, wcd937x);
> +	return ret;
>   }
>   
>   static void wcd937x_unbind(struct device *dev)
> @@ -2825,6 +2848,8 @@ static void wcd937x_unbind(struct device *dev)
>   	device_link_remove(wcd937x->rxdev, wcd937x->txdev);
>   	component_unbind_all(dev, wcd937x);
>   	mutex_destroy(&wcd937x->micb_lock);
> +	put_device(wcd937x->txdev);
> +	put_device(wcd937x->rxdev);

Shouldn't component_unbind_all be placed here too?

Anyway, current changes (applies with or without the mentioned change):

Reviewed-by: David Heidelberg <david@ixit.cz>

Thank you!

>   }
>   
>   static const struct component_master_ops wcd937x_comp_ops = {

-- 
David Heidelberg