[PATCH 0/2] ASoC: codecs: wcd938x: fix mux error handling

[PATCH 0/2] ASoC: codecs: wcd938x: fix mux error handling

[Johan Hovold]

A recent change added support for looking up an optional mux control
before falling back to gpio control for us-euro plug selection.

The mux framework does however not yet support optional muxes and an
error message is now incorrectly logged on machines like the Lenovo
ThinkPad X13s which do not have one:

    wcd938x_codec audio-codec: /audio-codec: failed to get mux-control (0)

Suppress the bogus error and add the missing mux error handling by
making sure that the 'mux-controls' DT property is present before
looking up the mux control.

Included is also a related cleanup.

Johan


Johan Hovold (2):
  ASoC: codecs: wcd938x: fix mux error handling
  ASoC: codecs: wcd938x: drop unnecessary mux flag assignment

 sound/soc/codecs/wcd938x.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

-- 
2.49.0

[PATCH 1/2] ASoC: codecs: wcd938x: fix mux error handling

[Johan Hovold]

A recent change added support for looking up an optional mux control
before falling back to gpio control for us-euro plug selection.

The mux framework does however not yet support optional muxes and an
error message is now incorrectly logged on machines like the Lenovo
ThinkPad X13s which do not have one:

    wcd938x_codec audio-codec: /audio-codec: failed to get mux-control (0)

Suppress the bogus error and add the missing mux error handling by
making sure that the 'mux-controls' DT property is present before
looking up the mux control.

Fixes: eec611d26f84 ("ASoC: codecs: wcd938x: add mux control support for hp audio mux")
Link: https://lore.kernel.org/lkml/Z-z_ZAyVBK5ui50k@hovoldconsulting.com/
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 sound/soc/codecs/wcd938x.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index 585a92772c2a..55031c16e4e1 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -3271,18 +3271,13 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
 		return dev_err_probe(dev, wcd938x->reset_gpio,
 				     "Failed to get reset gpio\n");
 
-	wcd938x->us_euro_mux = devm_mux_control_get(dev, NULL);
-	if (IS_ERR(wcd938x->us_euro_mux)) {
-		if (PTR_ERR(wcd938x->us_euro_mux) == -EPROBE_DEFER)
-			return -EPROBE_DEFER;
+	if (of_property_present(dev->of_node, "mux-controls")) {
+		wcd938x->us_euro_mux = devm_mux_control_get(dev, NULL);
+		if (IS_ERR(wcd938x->us_euro_mux)) {
+			ret = PTR_ERR(wcd938x->us_euro_mux);
+			return dev_err_probe(dev, ret, "failed to get mux control\n");
+		}
 
-		/* mux is optional and now fallback to using gpio */
-		wcd938x->us_euro_mux = NULL;
-		wcd938x->us_euro_gpio = devm_gpiod_get_optional(dev, "us-euro", GPIOD_OUT_LOW);
-		if (IS_ERR(wcd938x->us_euro_gpio))
-			return dev_err_probe(dev, PTR_ERR(wcd938x->us_euro_gpio),
-					     "us-euro swap Control GPIO not found\n");
-	} else {
 		ret = mux_control_try_select(wcd938x->us_euro_mux, wcd938x->mux_state);
 		if (ret) {
 			dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
@@ -3290,6 +3285,11 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
 			return ret;
 		}
 		wcd938x->mux_setup_done = true;
+	} else {
+		wcd938x->us_euro_gpio = devm_gpiod_get_optional(dev, "us-euro", GPIOD_OUT_LOW);
+		if (IS_ERR(wcd938x->us_euro_gpio))
+			return dev_err_probe(dev, PTR_ERR(wcd938x->us_euro_gpio),
+					     "us-euro swap Control GPIO not found\n");
 	}
 
 	cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
-- 
2.49.0

Re: [PATCH 1/2] ASoC: codecs: wcd938x: fix mux error handling

[Markus Elfring]

…
> +++ b/sound/soc/codecs/wcd938x.c
> @@ -3271,18 +3271,13 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
>  		return dev_err_probe(dev, wcd938x->reset_gpio,
>  				     "Failed to get reset gpio\n");
>  
> -	wcd938x->us_euro_mux = devm_mux_control_get(dev, NULL);
> -	if (IS_ERR(wcd938x->us_euro_mux)) {
> -		if (PTR_ERR(wcd938x->us_euro_mux) == -EPROBE_DEFER)
> -			return -EPROBE_DEFER;
> +	if (of_property_present(dev->of_node, "mux-controls")) {
> +		wcd938x->us_euro_mux = devm_mux_control_get(dev, NULL);
> +		if (IS_ERR(wcd938x->us_euro_mux)) {
> +			ret = PTR_ERR(wcd938x->us_euro_mux);
> +			return dev_err_probe(dev, ret, "failed to get mux control\n");
> +		}

May the error code assignment statement be omitted here together with extra curly brackets
(as such a case is demonstrated in a subsequent if branch)?

+			return dev_err_probe(dev, PTR_ERR(wcd938x->us_euro_mux),
+					     "failed to get mux control\n");


Regards,
Markus

Re: [PATCH 1/2] ASoC: codecs: wcd938x: fix mux error handling

[Johan Hovold]

On Tue, Apr 15, 2025 at 12:00:33PM +0200, Markus Elfring wrote:
> …
> > +++ b/sound/soc/codecs/wcd938x.c
> > @@ -3271,18 +3271,13 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
> >  		return dev_err_probe(dev, wcd938x->reset_gpio,
> >  				     "Failed to get reset gpio\n");
> >  
> > -	wcd938x->us_euro_mux = devm_mux_control_get(dev, NULL);
> > -	if (IS_ERR(wcd938x->us_euro_mux)) {
> > -		if (PTR_ERR(wcd938x->us_euro_mux) == -EPROBE_DEFER)
> > -			return -EPROBE_DEFER;
> > +	if (of_property_present(dev->of_node, "mux-controls")) {
> > +		wcd938x->us_euro_mux = devm_mux_control_get(dev, NULL);
> > +		if (IS_ERR(wcd938x->us_euro_mux)) {
> > +			ret = PTR_ERR(wcd938x->us_euro_mux);
> > +			return dev_err_probe(dev, ret, "failed to get mux control\n");
> > +		}
> 
> May the error code assignment statement be omitted here together with extra curly brackets
> (as such a case is demonstrated in a subsequent if branch)?

I considered that, but I prefer using a temporary variable as it is more
readable.

> 
> +			return dev_err_probe(dev, PTR_ERR(wcd938x->us_euro_mux),
> +					     "failed to get mux control\n");

Johan

Re: [1/2] ASoC: codecs: wcd938x: fix mux error handling

[Markus Elfring]

>> May the error code assignment statement be omitted here together with extra curly brackets
>> (as such a case is demonstrated in a subsequent if branch)?
> 
> I considered that, but I prefer using a temporary variable as it is more readable.
Would your coding style preferences be inconsistent here?

Regards,
Markus

[PATCH 2/2] ASoC: codecs: wcd938x: drop unnecessary mux flag assignment

[Johan Hovold]

The codec driver data is allocated using kzalloc() so there's no need to
clear the mux setup flag when mux selection fails during probe.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 sound/soc/codecs/wcd938x.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index 55031c16e4e1..8c9f67dedb83 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -3281,7 +3281,6 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
 		ret = mux_control_try_select(wcd938x->us_euro_mux, wcd938x->mux_state);
 		if (ret) {
 			dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
-			wcd938x->mux_setup_done = false;
 			return ret;
 		}
 		wcd938x->mux_setup_done = true;
-- 
2.49.0

Re: [PATCH 0/2] ASoC: codecs: wcd938x: fix mux error handling

[Mark Brown]

On Tue, 15 Apr 2025 09:41:43 +0200, Johan Hovold wrote:
> A recent change added support for looking up an optional mux control
> before falling back to gpio control for us-euro plug selection.
> 
> The mux framework does however not yet support optional muxes and an
> error message is now incorrectly logged on machines like the Lenovo
> ThinkPad X13s which do not have one:
> 
> [...]

Applied to

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

Thanks!

[1/2] ASoC: codecs: wcd938x: fix mux error handling
      commit: b0090115001a3602d0d18828960e8eb1ae129b41
[2/2] ASoC: codecs: wcd938x: drop unnecessary mux flag assignment
      commit: e358e012a69a3d553803cbe62d9f6eeea57726fc

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