[PATCH v4 0/6] ASoC: wcd938x: enable t14s audio headset

[PATCH v4 0/6] ASoC: wcd938x: enable t14s audio headset

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

On Lenovo ThinkPad T14s, the headset is connected via a HiFi Switch to
support CTIA and OMTP headsets. This switch is used to minimise pop and
click during headset type switching.

This patchset adds required bindings and changes to codec and dts to   
tnable the regulator required to power this switch along with wiring up
gpio that control the headset switching.

Without this patchset, there will be lots of noise on headset and mic
will not we functional.

Changes since v3:
	- rearranged mux deselct to do better error handling.
	- reused mux_state for gpios as well.
	- removed tested by on bindings

Changes since v2:
	- udpated bindings as suggested by Rob and Krzysztof
	- cleaned up swap_gnd_mic callback
	- updated mux gpio to use dev_err_probe.
	- added Tested-by and reviewed-by tags 

Changes since v1:
	- moved to using mux-controls.
	- fixed typo in regulator naming.

Srinivas Kandagatla (6):
  dt-bindings: mux: add optional regulator binding to gpio mux
  mux: gpio: add optional regulator support
  ASoC: codecs: wcd-mbhc: cleanup swap_gnd_mic api
  ASoC: dt-bindings: wcd93xx: add bindings for audio mux controlling hp
  ASoC: codecs: wcd938x: add mux control support for hp audio mux
  arm64: dts: qcom: x1e78100-t14s: Enable audio headset support

 .../devicetree/bindings/mux/gpio-mux.yaml     |  4 ++
 .../bindings/sound/qcom,wcd938x.yaml          |  6 +++
 .../qcom/x1e78100-lenovo-thinkpad-t14s.dtsi   | 25 ++++++++++
 drivers/mux/gpio.c                            |  5 ++
 sound/soc/codecs/Kconfig                      |  1 +
 sound/soc/codecs/wcd-mbhc-v2.c                |  2 +-
 sound/soc/codecs/wcd-mbhc-v2.h                |  2 +-
 sound/soc/codecs/wcd937x.c                    |  2 +-
 sound/soc/codecs/wcd938x.c                    | 50 +++++++++++++++----
 sound/soc/codecs/wcd939x.c                    |  2 +-
 10 files changed, 84 insertions(+), 15 deletions(-)

-- 
2.39.5

[PATCH v4 1/6] dt-bindings: mux: add optional regulator binding to gpio mux

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

On some platforms to minimise pop and click during switching between
CTIA and OMTP headset an additional HiFi Mux Switch is used. Most common
case is that this switch is switched on by default, but on some
platforms this needs a regulator enable. One such platform is Lenovo
T14s.

This patch adds required bindings in gpio-mux to add such optional regulator.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/mux/gpio-mux.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.yaml b/Documentation/devicetree/bindings/mux/gpio-mux.yaml
index b597c1f2c577..ef7e33ec85d4 100644
--- a/Documentation/devicetree/bindings/mux/gpio-mux.yaml
+++ b/Documentation/devicetree/bindings/mux/gpio-mux.yaml
@@ -25,6 +25,10 @@ properties:
     description:
       List of gpios used to control the multiplexer, least significant bit first.
 
+  mux-supply:
+    description:
+      Regulator to power on the multiplexer.
+
   '#mux-control-cells':
     enum: [ 0, 1 ]
 
-- 
2.39.5

Re: [PATCH v4 1/6] dt-bindings: mux: add optional regulator binding to gpio mux

[Rob Herring (Arm)]

On Mon, 24 Mar 2025 13:00:52 +0000, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> On some platforms to minimise pop and click during switching between
> CTIA and OMTP headset an additional HiFi Mux Switch is used. Most common
> case is that this switch is switched on by default, but on some
> platforms this needs a regulator enable. One such platform is Lenovo
> T14s.
> 
> This patch adds required bindings in gpio-mux to add such optional regulator.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  Documentation/devicetree/bindings/mux/gpio-mux.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH v4 2/6] mux: gpio: add optional regulator support

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Some of the external muxes needs powering up using a regulator.
This is the case with Lenovo T14s laptop which has a external audio mux
to handle US/EURO headsets.

Add support to the driver to handle this optional regulator.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Tested-by: Christopher Obbard <christopher.obbard@linaro.org>
---
 drivers/mux/gpio.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index cc5f2c1861d4..d1607f287755 100644
--- a/drivers/mux/gpio.c
+++ b/drivers/mux/gpio.c
@@ -15,6 +15,7 @@
 #include <linux/mux/driver.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/regulator/consumer.h>
 
 struct mux_gpio {
 	struct gpio_descs *gpios;
@@ -82,6 +83,10 @@ static int mux_gpio_probe(struct platform_device *pdev)
 		mux_chip->mux->idle_state = idle_state;
 	}
 
+	ret = devm_regulator_get_enable_optional(dev, "mux");
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "Couldn't retrieve/enable gpio mux supply\n");
+
 	ret = devm_mux_chip_register(dev, mux_chip);
 	if (ret < 0)
 		return ret;
-- 
2.39.5

[PATCH v4 3/6] ASoC: codecs: wcd-mbhc: cleanup swap_gnd_mic api

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Remove confusing and unused argument in swap_gnd_mic api, the second
argument active is not really used, and always set to true in the mbhc
drivers.

The callback itself is used to toggle the gnd_mic lines when a cross
connection is detected by mbhc circuits, so there is no need of this
argument.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/wcd-mbhc-v2.c | 2 +-
 sound/soc/codecs/wcd-mbhc-v2.h | 2 +-
 sound/soc/codecs/wcd937x.c     | 2 +-
 sound/soc/codecs/wcd938x.c     | 2 +-
 sound/soc/codecs/wcd939x.c     | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/wcd-mbhc-v2.c b/sound/soc/codecs/wcd-mbhc-v2.c
index d589a212b768..4b7c3d6080a1 100644
--- a/sound/soc/codecs/wcd-mbhc-v2.c
+++ b/sound/soc/codecs/wcd-mbhc-v2.c
@@ -1260,7 +1260,7 @@ static void wcd_correct_swch_plug(struct work_struct *work)
 			if (pt_gnd_mic_swap_cnt == mbhc->swap_thr) {
 				/* US_EU gpio present, flip switch */
 				if (mbhc->cfg->swap_gnd_mic) {
-					if (mbhc->cfg->swap_gnd_mic(component, true))
+					if (mbhc->cfg->swap_gnd_mic(component))
 						continue;
 				}
 			}
diff --git a/sound/soc/codecs/wcd-mbhc-v2.h b/sound/soc/codecs/wcd-mbhc-v2.h
index b977e8f87d7c..a5d52b9643f5 100644
--- a/sound/soc/codecs/wcd-mbhc-v2.h
+++ b/sound/soc/codecs/wcd-mbhc-v2.h
@@ -194,7 +194,7 @@ struct wcd_mbhc_config {
 	int num_btn;
 	bool mono_stero_detection;
 	bool typec_analog_mux;
-	bool (*swap_gnd_mic)(struct snd_soc_component *component, bool active);
+	bool (*swap_gnd_mic)(struct snd_soc_component *component);
 	bool hs_ext_micbias;
 	bool gnd_det_en;
 	uint32_t linein_th;
diff --git a/sound/soc/codecs/wcd937x.c b/sound/soc/codecs/wcd937x.c
index c9d5e67bf66e..b2fb8520c823 100644
--- a/sound/soc/codecs/wcd937x.c
+++ b/sound/soc/codecs/wcd937x.c
@@ -2646,7 +2646,7 @@ static void wcd937x_dt_parse_micbias_info(struct device *dev, struct wcd937x_pri
 		dev_warn(dev, "Micbias3 DT property not found\n");
 }
 
-static bool wcd937x_swap_gnd_mic(struct snd_soc_component *component, bool active)
+static bool wcd937x_swap_gnd_mic(struct snd_soc_component *component)
 {
 	int value;
 	struct wcd937x_priv *wcd937x;
diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index f2a4f3262bdb..dfaa3de31164 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -3235,7 +3235,7 @@ static void wcd938x_dt_parse_micbias_info(struct device *dev, struct wcd938x_pri
 		dev_info(dev, "%s: Micbias4 DT property not found\n", __func__);
 }
 
-static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component, bool active)
+static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
 {
 	int value;
 
diff --git a/sound/soc/codecs/wcd939x.c b/sound/soc/codecs/wcd939x.c
index 4a417a92514d..546aa88a398c 100644
--- a/sound/soc/codecs/wcd939x.c
+++ b/sound/soc/codecs/wcd939x.c
@@ -3215,7 +3215,7 @@ static void wcd939x_dt_parse_micbias_info(struct device *dev, struct wcd939x_pri
 }
 
 #if IS_ENABLED(CONFIG_TYPEC)
-static bool wcd939x_swap_gnd_mic(struct snd_soc_component *component, bool active)
+static bool wcd939x_swap_gnd_mic(struct snd_soc_component *component)
 {
 	struct wcd939x_priv *wcd939x = snd_soc_component_get_drvdata(component);
 
-- 
2.39.5

[PATCH v4 4/6] ASoC: dt-bindings: wcd93xx: add bindings for audio mux controlling hp

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

On some platforms to minimise pop and click during switching between
CTIA and OMTP headset an additional HiFi mux is used. Most common
case is that this switch is switched on by default, but on some
platforms this needs a regulator enable.

Move to using mux-controls so that both the gpio and regulators can be
driven correctly, rather than adding regulator handing in the codec.

This patch adds required bindings to add such mux controls.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
index 10531350c336..ab1c6285dbf8 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
@@ -23,9 +23,15 @@ properties:
       - qcom,wcd9380-codec
       - qcom,wcd9385-codec
 
+  mux-controls:
+    description: A reference to the audio mux switch for
+      switching CTIA/OMTP Headset types
+    maxItems: 1
+
   us-euro-gpios:
     description: GPIO spec for swapping gnd and mic segments
     maxItems: 1
+    deprecated: true
 
 required:
   - compatible
-- 
2.39.5

[PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

On some platforms to minimise pop and click during switching between
CTIA and OMTP headset an additional HiFi mux is used. Most common
case is that this switch is switched on by default, but on some
platforms this needs a regulator enable.

move to using mux control to enable both regulator and handle gpios,
deprecate the usage of gpio.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Tested-by: Christopher Obbard <christopher.obbard@linaro.org>
---
 sound/soc/codecs/Kconfig   |  1 +
 sound/soc/codecs/wcd938x.c | 50 +++++++++++++++++++++++++++++---------
 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index ee35f3aa5521..a2829d76e108 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -2226,6 +2226,7 @@ config SND_SOC_WCD938X
 	tristate
 	depends on SOUNDWIRE || !SOUNDWIRE
 	select SND_SOC_WCD_CLASSH
+	select MULTIPLEXER
 
 config SND_SOC_WCD938X_SDW
 	tristate "WCD9380/WCD9385 Codec - SDW"
diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index dfaa3de31164..88c758efe40d 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -19,6 +19,7 @@
 #include <linux/regmap.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
+#include <linux/mux/consumer.h>
 #include <linux/regulator/consumer.h>
 
 #include "wcd-clsh-v2.h"
@@ -178,6 +179,8 @@ struct wcd938x_priv {
 	int variant;
 	int reset_gpio;
 	struct gpio_desc *us_euro_gpio;
+	struct mux_control *us_euro_mux;
+	u32 mux_state;
 	u32 micb1_mv;
 	u32 micb2_mv;
 	u32 micb3_mv;
@@ -3235,17 +3238,31 @@ static void wcd938x_dt_parse_micbias_info(struct device *dev, struct wcd938x_pri
 		dev_info(dev, "%s: Micbias4 DT property not found\n", __func__);
 }
 
-static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
+static int wcd938x_select_mux_state(struct device *dev, struct wcd938x_priv *wcd938x, int state)
 {
-	int value;
+	int ret = mux_control_try_select(wcd938x->us_euro_mux, state);
 
-	struct wcd938x_priv *wcd938x;
+	if (ret) {
+		dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
+		return ret;
+	}
 
-	wcd938x = snd_soc_component_get_drvdata(component);
+	wcd938x->mux_state = state;
+	mux_control_deselect(wcd938x->us_euro_mux);
+
+	return 0;
+}
 
-	value = gpiod_get_value(wcd938x->us_euro_gpio);
+static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
+{
+	struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
 
-	gpiod_set_value(wcd938x->us_euro_gpio, !value);
+	if (wcd938x->us_euro_mux) {
+		if (wcd938x_select_mux_state(component->dev, wcd938x, !wcd938x->mux_state))
+			return false;
+	} else {
+		gpiod_set_value(wcd938x->us_euro_gpio, !wcd938x->mux_state);
+	}
 
 	return true;
 }
@@ -3261,11 +3278,22 @@ 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_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");
+	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;
+
+		/* 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 = wcd938x_select_mux_state(dev, wcd938x, wcd938x->mux_state);
+		if (ret)
+			return ret;
+	}
 
 	cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
 
-- 
2.39.5

Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[Dmitry Baryshkov]

On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@linaro.org> wrote:
>
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> On some platforms to minimise pop and click during switching between
> CTIA and OMTP headset an additional HiFi mux is used. Most common
> case is that this switch is switched on by default, but on some
> platforms this needs a regulator enable.
>
> move to using mux control to enable both regulator and handle gpios,
> deprecate the usage of gpio.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Tested-by: Christopher Obbard <christopher.obbard@linaro.org>
> ---
>  sound/soc/codecs/Kconfig   |  1 +
>  sound/soc/codecs/wcd938x.c | 50 +++++++++++++++++++++++++++++---------
>  2 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index ee35f3aa5521..a2829d76e108 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -2226,6 +2226,7 @@ config SND_SOC_WCD938X
>         tristate
>         depends on SOUNDWIRE || !SOUNDWIRE
>         select SND_SOC_WCD_CLASSH
> +       select MULTIPLEXER
>
>  config SND_SOC_WCD938X_SDW
>         tristate "WCD9380/WCD9385 Codec - SDW"
> diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
> index dfaa3de31164..88c758efe40d 100644
> --- a/sound/soc/codecs/wcd938x.c
> +++ b/sound/soc/codecs/wcd938x.c
> @@ -19,6 +19,7 @@
>  #include <linux/regmap.h>
>  #include <sound/soc.h>
>  #include <sound/soc-dapm.h>
> +#include <linux/mux/consumer.h>
>  #include <linux/regulator/consumer.h>
>
>  #include "wcd-clsh-v2.h"
> @@ -178,6 +179,8 @@ struct wcd938x_priv {
>         int variant;
>         int reset_gpio;
>         struct gpio_desc *us_euro_gpio;
> +       struct mux_control *us_euro_mux;
> +       u32 mux_state;
>         u32 micb1_mv;
>         u32 micb2_mv;
>         u32 micb3_mv;
> @@ -3235,17 +3238,31 @@ static void wcd938x_dt_parse_micbias_info(struct device *dev, struct wcd938x_pri
>                 dev_info(dev, "%s: Micbias4 DT property not found\n", __func__);
>  }
>
> -static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
> +static int wcd938x_select_mux_state(struct device *dev, struct wcd938x_priv *wcd938x, int state)
>  {
> -       int value;
> +       int ret = mux_control_try_select(wcd938x->us_euro_mux, state);

Hmm. Does this really work? You have selected the mux in probe
function, now you are trying to select it again. If I'm reading the
code correctly, you will get -EBUSY here.

>
> -       struct wcd938x_priv *wcd938x;
> +       if (ret) {
> +               dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
> +               return ret;
> +       }
>
> -       wcd938x = snd_soc_component_get_drvdata(component);
> +       wcd938x->mux_state = state;
> +       mux_control_deselect(wcd938x->us_euro_mux);
> +
> +       return 0;
> +}
>
> -       value = gpiod_get_value(wcd938x->us_euro_gpio);
> +static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
> +{
> +       struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
>
> -       gpiod_set_value(wcd938x->us_euro_gpio, !value);
> +       if (wcd938x->us_euro_mux) {
> +               if (wcd938x_select_mux_state(component->dev, wcd938x, !wcd938x->mux_state))
> +                       return false;
> +       } else {
> +               gpiod_set_value(wcd938x->us_euro_gpio, !wcd938x->mux_state);
> +       }
>
>         return true;
>  }
> @@ -3261,11 +3278,22 @@ 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_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");
> +       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;
> +
> +               /* 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 = wcd938x_select_mux_state(dev, wcd938x, wcd938x->mux_state);
> +               if (ret)
> +                       return ret;
> +       }
>
>         cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
>
> --
> 2.39.5
>


-- 
With best wishes
Dmitry

Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[Srinivas Kandagatla]

On 24/03/2025 13:50, Dmitry Baryshkov wrote:
> On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@linaro.org> wrote:
>>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> On some platforms to minimise pop and click during switching between
>> CTIA and OMTP headset an additional HiFi mux is used. Most common
>> case is that this switch is switched on by default, but on some
>> platforms this needs a regulator enable.
>>
>> move to using mux control to enable both regulator and handle gpios,
>> deprecate the usage of gpio.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> Tested-by: Christopher Obbard <christopher.obbard@linaro.org>
>> ---
>>   sound/soc/codecs/Kconfig   |  1 +
>>   sound/soc/codecs/wcd938x.c | 50 +++++++++++++++++++++++++++++---------
>>   2 files changed, 40 insertions(+), 11 deletions(-)
>>
>> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
>> index ee35f3aa5521..a2829d76e108 100644
>> --- a/sound/soc/codecs/Kconfig
>> +++ b/sound/soc/codecs/Kconfig
>> @@ -2226,6 +2226,7 @@ config SND_SOC_WCD938X
>>          tristate
>>          depends on SOUNDWIRE || !SOUNDWIRE
>>          select SND_SOC_WCD_CLASSH
>> +       select MULTIPLEXER
>>
>>   config SND_SOC_WCD938X_SDW
>>          tristate "WCD9380/WCD9385 Codec - SDW"
>> diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
>> index dfaa3de31164..88c758efe40d 100644
>> --- a/sound/soc/codecs/wcd938x.c
>> +++ b/sound/soc/codecs/wcd938x.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/regmap.h>
>>   #include <sound/soc.h>
>>   #include <sound/soc-dapm.h>
>> +#include <linux/mux/consumer.h>
>>   #include <linux/regulator/consumer.h>
>>
>>   #include "wcd-clsh-v2.h"
>> @@ -178,6 +179,8 @@ struct wcd938x_priv {
>>          int variant;
>>          int reset_gpio;
>>          struct gpio_desc *us_euro_gpio;
>> +       struct mux_control *us_euro_mux;
>> +       u32 mux_state;
>>          u32 micb1_mv;
>>          u32 micb2_mv;
>>          u32 micb3_mv;
>> @@ -3235,17 +3238,31 @@ static void wcd938x_dt_parse_micbias_info(struct device *dev, struct wcd938x_pri
>>                  dev_info(dev, "%s: Micbias4 DT property not found\n", __func__);
>>   }
>>
>> -static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
>> +static int wcd938x_select_mux_state(struct device *dev, struct wcd938x_priv *wcd938x, int state)
>>   {
>> -       int value;
>> +       int ret = mux_control_try_select(wcd938x->us_euro_mux, state);
> 
> Hmm. Does this really work? You have selected the mux in probe
> function, now you are trying to select it again. If I'm reading the
> code correctly, you will get -EBUSY here.

On successful selection of mux state, the mux will be kept available 
(mux_control_deselect) for any new callers.

So we will not get EBUSY for the second caller.

--srini
> 
>>
>> -       struct wcd938x_priv *wcd938x;
>> +       if (ret) {
>> +               dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
>> +               return ret;
>> +       }
>>
>> -       wcd938x = snd_soc_component_get_drvdata(component);
>> +       wcd938x->mux_state = state;
>> +       mux_control_deselect(wcd938x->us_euro_mux);
>> +
>> +       return 0;
>> +}
>>
>> -       value = gpiod_get_value(wcd938x->us_euro_gpio);
>> +static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
>> +{
>> +       struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
>>
>> -       gpiod_set_value(wcd938x->us_euro_gpio, !value);
>> +       if (wcd938x->us_euro_mux) {
>> +               if (wcd938x_select_mux_state(component->dev, wcd938x, !wcd938x->mux_state))
>> +                       return false;
>> +       } else {
>> +               gpiod_set_value(wcd938x->us_euro_gpio, !wcd938x->mux_state);
>> +       }
>>
>>          return true;
>>   }
>> @@ -3261,11 +3278,22 @@ 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_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");
>> +       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;
>> +
>> +               /* 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 = wcd938x_select_mux_state(dev, wcd938x, wcd938x->mux_state);
>> +               if (ret)
>> +                       return ret;
>> +       }
>>
>>          cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
>>
>> --
>> 2.39.5
>>
> 
> 

Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[Dmitry Baryshkov]

On Mon, Mar 24, 2025 at 01:58:06PM +0000, Srinivas Kandagatla wrote:
> 
> 
> On 24/03/2025 13:50, Dmitry Baryshkov wrote:
> > On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@linaro.org> wrote:
> > > 
> > > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > > 
> > > On some platforms to minimise pop and click during switching between
> > > CTIA and OMTP headset an additional HiFi mux is used. Most common
> > > case is that this switch is switched on by default, but on some
> > > platforms this needs a regulator enable.
> > > 
> > > move to using mux control to enable both regulator and handle gpios,
> > > deprecate the usage of gpio.
> > > 
> > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > > Tested-by: Christopher Obbard <christopher.obbard@linaro.org>
> > > ---
> > >   sound/soc/codecs/Kconfig   |  1 +
> > >   sound/soc/codecs/wcd938x.c | 50 +++++++++++++++++++++++++++++---------
> > >   2 files changed, 40 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > > index ee35f3aa5521..a2829d76e108 100644
> > > --- a/sound/soc/codecs/Kconfig
> > > +++ b/sound/soc/codecs/Kconfig
> > > @@ -2226,6 +2226,7 @@ config SND_SOC_WCD938X
> > >          tristate
> > >          depends on SOUNDWIRE || !SOUNDWIRE
> > >          select SND_SOC_WCD_CLASSH
> > > +       select MULTIPLEXER
> > > 
> > >   config SND_SOC_WCD938X_SDW
> > >          tristate "WCD9380/WCD9385 Codec - SDW"
> > > diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
> > > index dfaa3de31164..88c758efe40d 100644
> > > --- a/sound/soc/codecs/wcd938x.c
> > > +++ b/sound/soc/codecs/wcd938x.c
> > > @@ -19,6 +19,7 @@
> > >   #include <linux/regmap.h>
> > >   #include <sound/soc.h>
> > >   #include <sound/soc-dapm.h>
> > > +#include <linux/mux/consumer.h>
> > >   #include <linux/regulator/consumer.h>
> > > 
> > >   #include "wcd-clsh-v2.h"
> > > @@ -178,6 +179,8 @@ struct wcd938x_priv {
> > >          int variant;
> > >          int reset_gpio;
> > >          struct gpio_desc *us_euro_gpio;
> > > +       struct mux_control *us_euro_mux;
> > > +       u32 mux_state;
> > >          u32 micb1_mv;
> > >          u32 micb2_mv;
> > >          u32 micb3_mv;
> > > @@ -3235,17 +3238,31 @@ static void wcd938x_dt_parse_micbias_info(struct device *dev, struct wcd938x_pri
> > >                  dev_info(dev, "%s: Micbias4 DT property not found\n", __func__);
> > >   }
> > > 
> > > -static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
> > > +static int wcd938x_select_mux_state(struct device *dev, struct wcd938x_priv *wcd938x, int state)
> > >   {
> > > -       int value;
> > > +       int ret = mux_control_try_select(wcd938x->us_euro_mux, state);
> > 
> > Hmm. Does this really work? You have selected the mux in probe
> > function, now you are trying to select it again. If I'm reading the
> > code correctly, you will get -EBUSY here.
> 
> On successful selection of mux state, the mux will be kept available
> (mux_control_deselect) for any new callers.
> 
> So we will not get EBUSY for the second caller.

No. wcd938x_populate_dt_data() selects the state by calling
wcd938x_select_mux_state(). Then you call mux_control_try_select() here.
As far as I understand, it will return -EBUSY as the sempahore is
already taken. Moreover, this is not how the MUX API is supposed to be
used. The driver is supposed to hold a state while it is still in use.

> 
> --srini
> > 
> > > 
> > > -       struct wcd938x_priv *wcd938x;
> > > +       if (ret) {
> > > +               dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
> > > +               return ret;
> > > +       }
> > > 
> > > -       wcd938x = snd_soc_component_get_drvdata(component);
> > > +       wcd938x->mux_state = state;
> > > +       mux_control_deselect(wcd938x->us_euro_mux);
> > > +
> > > +       return 0;
> > > +}
> > > 
> > > -       value = gpiod_get_value(wcd938x->us_euro_gpio);
> > > +static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
> > > +{
> > > +       struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
> > > 
> > > -       gpiod_set_value(wcd938x->us_euro_gpio, !value);
> > > +       if (wcd938x->us_euro_mux) {
> > > +               if (wcd938x_select_mux_state(component->dev, wcd938x, !wcd938x->mux_state))
> > > +                       return false;
> > > +       } else {
> > > +               gpiod_set_value(wcd938x->us_euro_gpio, !wcd938x->mux_state);
> > > +       }
> > > 
> > >          return true;
> > >   }
> > > @@ -3261,11 +3278,22 @@ 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_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");
> > > +       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;
> > > +
> > > +               /* 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 = wcd938x_select_mux_state(dev, wcd938x, wcd938x->mux_state);
> > > +               if (ret)
> > > +                       return ret;
> > > +       }
> > > 
> > >          cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
> > > 
> > > --
> > > 2.39.5
> > > 
> > 
> > 

-- 
With best wishes
Dmitry

Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[Srinivas Kandagatla]

On 24/03/2025 15:18, Dmitry Baryshkov wrote:
> On Mon, Mar 24, 2025 at 01:58:06PM +0000, Srinivas Kandagatla wrote:
>>
>>
>> On 24/03/2025 13:50, Dmitry Baryshkov wrote:
>>> On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@linaro.org> wrote:
>>>>
>>>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>>
>>>> On some platforms to minimise pop and click during switching between
>>>> CTIA and OMTP headset an additional HiFi mux is used. Most common
>>>> case is that this switch is switched on by default, but on some
>>>> platforms this needs a regulator enable.
>>>>
>>>> move to using mux control to enable both regulator and handle gpios,
>>>> deprecate the usage of gpio.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>> Tested-by: Christopher Obbard <christopher.obbard@linaro.org>
>>>> ---
>>>>    sound/soc/codecs/Kconfig   |  1 +
>>>>    sound/soc/codecs/wcd938x.c | 50 +++++++++++++++++++++++++++++---------
>>>>    2 files changed, 40 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
>>>> index ee35f3aa5521..a2829d76e108 100644
>>>> --- a/sound/soc/codecs/Kconfig
>>>> +++ b/sound/soc/codecs/Kconfig
>>>> @@ -2226,6 +2226,7 @@ config SND_SOC_WCD938X
>>>>           tristate
>>>>           depends on SOUNDWIRE || !SOUNDWIRE
>>>>           select SND_SOC_WCD_CLASSH
>>>> +       select MULTIPLEXER
>>>>
>>>>    config SND_SOC_WCD938X_SDW
>>>>           tristate "WCD9380/WCD9385 Codec - SDW"
>>>> diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
>>>> index dfaa3de31164..88c758efe40d 100644
>>>> --- a/sound/soc/codecs/wcd938x.c
>>>> +++ b/sound/soc/codecs/wcd938x.c
>>>> @@ -19,6 +19,7 @@
>>>>    #include <linux/regmap.h>
>>>>    #include <sound/soc.h>
>>>>    #include <sound/soc-dapm.h>
>>>> +#include <linux/mux/consumer.h>
>>>>    #include <linux/regulator/consumer.h>
>>>>
>>>>    #include "wcd-clsh-v2.h"
>>>> @@ -178,6 +179,8 @@ struct wcd938x_priv {
>>>>           int variant;
>>>>           int reset_gpio;
>>>>           struct gpio_desc *us_euro_gpio;
>>>> +       struct mux_control *us_euro_mux;
>>>> +       u32 mux_state;
>>>>           u32 micb1_mv;
>>>>           u32 micb2_mv;
>>>>           u32 micb3_mv;
>>>> @@ -3235,17 +3238,31 @@ static void wcd938x_dt_parse_micbias_info(struct device *dev, struct wcd938x_pri
>>>>                   dev_info(dev, "%s: Micbias4 DT property not found\n", __func__);
>>>>    }
>>>>
>>>> -static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
>>>> +static int wcd938x_select_mux_state(struct device *dev, struct wcd938x_priv *wcd938x, int state)
>>>>    {
>>>> -       int value;
>>>> +       int ret = mux_control_try_select(wcd938x->us_euro_mux, state);
>>>
>>> Hmm. Does this really work? You have selected the mux in probe
>>> function, now you are trying to select it again. If I'm reading the
>>> code correctly, you will get -EBUSY here.
>>
>> On successful selection of mux state, the mux will be kept available
>> (mux_control_deselect) for any new callers.
>>
>> So we will not get EBUSY for the second caller.
> 
> No. wcd938x_populate_dt_data() selects the state by calling
> wcd938x_select_mux_state().

At this point we also released it (both in success and error case).

This will hold on to the previous state unless we have defined a 
fallback idle-state.


  Then you call mux_control_try_select() here.
> As far as I understand, it will return -EBUSY as the sempahore is > already taken. Moreover, this is not how the MUX API is supposed to be
> used. The driver is supposed to hold a state while it is still in use.
> 
>>
>> --srini
>>>
>>>>
>>>> -       struct wcd938x_priv *wcd938x;
>>>> +       if (ret) {
>>>> +               dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
>>>> +               return ret;
>>>> +       }
>>>>
>>>> -       wcd938x = snd_soc_component_get_drvdata(component);
>>>> +       wcd938x->mux_state = state;
>>>> +       mux_control_deselect(wcd938x->us_euro_mux);
>>>> +
>>>> +       return 0;
>>>> +}
>>>>
>>>> -       value = gpiod_get_value(wcd938x->us_euro_gpio);
>>>> +static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
>>>> +{
>>>> +       struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
>>>>
>>>> -       gpiod_set_value(wcd938x->us_euro_gpio, !value);
>>>> +       if (wcd938x->us_euro_mux) {
>>>> +               if (wcd938x_select_mux_state(component->dev, wcd938x, !wcd938x->mux_state))
>>>> +                       return false;
>>>> +       } else {
>>>> +               gpiod_set_value(wcd938x->us_euro_gpio, !wcd938x->mux_state);
>>>> +       }
>>>>
>>>>           return true;
>>>>    }
>>>> @@ -3261,11 +3278,22 @@ 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_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");
>>>> +       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;
>>>> +
>>>> +               /* 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 = wcd938x_select_mux_state(dev, wcd938x, wcd938x->mux_state);
>>>> +               if (ret)
>>>> +                       return ret;
>>>> +       }
>>>>
>>>>           cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
>>>>
>>>> --
>>>> 2.39.5
>>>>
>>>
>>>
> 

Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[Dmitry Baryshkov]

On Mon, Mar 24, 2025 at 03:58:32PM +0000, Srinivas Kandagatla wrote:
> 
> 
> On 24/03/2025 15:18, Dmitry Baryshkov wrote:
> > On Mon, Mar 24, 2025 at 01:58:06PM +0000, Srinivas Kandagatla wrote:
> > > 
> > > 
> > > On 24/03/2025 13:50, Dmitry Baryshkov wrote:
> > > > On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@linaro.org> wrote:
> > > > > 
> > > > > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > > > > 
> > > > > On some platforms to minimise pop and click during switching between
> > > > > CTIA and OMTP headset an additional HiFi mux is used. Most common
> > > > > case is that this switch is switched on by default, but on some
> > > > > platforms this needs a regulator enable.
> > > > > 
> > > > > move to using mux control to enable both regulator and handle gpios,
> > > > > deprecate the usage of gpio.
> > > > > 
> > > > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > > > > Tested-by: Christopher Obbard <christopher.obbard@linaro.org>
> > > > > ---
> > > > >    sound/soc/codecs/Kconfig   |  1 +
> > > > >    sound/soc/codecs/wcd938x.c | 50 +++++++++++++++++++++++++++++---------
> > > > >    2 files changed, 40 insertions(+), 11 deletions(-)
> > > > > 
> > > > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > > > > index ee35f3aa5521..a2829d76e108 100644
> > > > > --- a/sound/soc/codecs/Kconfig
> > > > > +++ b/sound/soc/codecs/Kconfig
> > > > > @@ -2226,6 +2226,7 @@ config SND_SOC_WCD938X
> > > > >           tristate
> > > > >           depends on SOUNDWIRE || !SOUNDWIRE
> > > > >           select SND_SOC_WCD_CLASSH
> > > > > +       select MULTIPLEXER
> > > > > 
> > > > >    config SND_SOC_WCD938X_SDW
> > > > >           tristate "WCD9380/WCD9385 Codec - SDW"
> > > > > diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
> > > > > index dfaa3de31164..88c758efe40d 100644
> > > > > --- a/sound/soc/codecs/wcd938x.c
> > > > > +++ b/sound/soc/codecs/wcd938x.c
> > > > > @@ -19,6 +19,7 @@
> > > > >    #include <linux/regmap.h>
> > > > >    #include <sound/soc.h>
> > > > >    #include <sound/soc-dapm.h>
> > > > > +#include <linux/mux/consumer.h>
> > > > >    #include <linux/regulator/consumer.h>
> > > > > 
> > > > >    #include "wcd-clsh-v2.h"
> > > > > @@ -178,6 +179,8 @@ struct wcd938x_priv {
> > > > >           int variant;
> > > > >           int reset_gpio;
> > > > >           struct gpio_desc *us_euro_gpio;
> > > > > +       struct mux_control *us_euro_mux;
> > > > > +       u32 mux_state;
> > > > >           u32 micb1_mv;
> > > > >           u32 micb2_mv;
> > > > >           u32 micb3_mv;
> > > > > @@ -3235,17 +3238,31 @@ static void wcd938x_dt_parse_micbias_info(struct device *dev, struct wcd938x_pri
> > > > >                   dev_info(dev, "%s: Micbias4 DT property not found\n", __func__);
> > > > >    }
> > > > > 
> > > > > -static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
> > > > > +static int wcd938x_select_mux_state(struct device *dev, struct wcd938x_priv *wcd938x, int state)
> > > > >    {
> > > > > -       int value;
> > > > > +       int ret = mux_control_try_select(wcd938x->us_euro_mux, state);
> > > > 
> > > > Hmm. Does this really work? You have selected the mux in probe
> > > > function, now you are trying to select it again. If I'm reading the
> > > > code correctly, you will get -EBUSY here.
> > > 
> > > On successful selection of mux state, the mux will be kept available
> > > (mux_control_deselect) for any new callers.
> > > 
> > > So we will not get EBUSY for the second caller.
> > 
> > No. wcd938x_populate_dt_data() selects the state by calling
> > wcd938x_select_mux_state().
> 
> At this point we also released it (both in success and error case).

By which code path? You call this function directly from
wcd938x_swap_gnd_mic() and the control is already selected from the
probe path.

> This will hold on to the previous state unless we have defined a fallback
> idle-state.

As I wrote, this doesn't seem to be a proper use of MUX API. I think
it's much easier to add a boolean which states that MUX has been
selected and use it to deselect the state both in this function and in
shutdown path.

Another option is really to expand the MUX API to add 'switch state'
call which will work if the state is already selected.

> 
> 
>  Then you call mux_control_try_select() here.
> > As far as I understand, it will return -EBUSY as the sempahore is > already taken. Moreover, this is not how the MUX API is supposed to be
> > used. The driver is supposed to hold a state while it is still in use.
> > 
> > > 
> > > --srini
> > > > 
> > > > > 
> > > > > -       struct wcd938x_priv *wcd938x;
> > > > > +       if (ret) {
> > > > > +               dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
> > > > > +               return ret;
> > > > > +       }
> > > > > 
> > > > > -       wcd938x = snd_soc_component_get_drvdata(component);
> > > > > +       wcd938x->mux_state = state;
> > > > > +       mux_control_deselect(wcd938x->us_euro_mux);
> > > > > +
> > > > > +       return 0;
> > > > > +}
> > > > > 
> > > > > -       value = gpiod_get_value(wcd938x->us_euro_gpio);
> > > > > +static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
> > > > > +{
> > > > > +       struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
> > > > > 
> > > > > -       gpiod_set_value(wcd938x->us_euro_gpio, !value);
> > > > > +       if (wcd938x->us_euro_mux) {
> > > > > +               if (wcd938x_select_mux_state(component->dev, wcd938x, !wcd938x->mux_state))
> > > > > +                       return false;
> > > > > +       } else {
> > > > > +               gpiod_set_value(wcd938x->us_euro_gpio, !wcd938x->mux_state);

Which codepath will update mux_state in case of the GPIO?

> > > > > +       }
> > > > > 
> > > > >           return true;
> > > > >    }
> > > > > @@ -3261,11 +3278,22 @@ 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_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");
> > > > > +       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;
> > > > > +
> > > > > +               /* 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 = wcd938x_select_mux_state(dev, wcd938x, wcd938x->mux_state);
> > > > > +               if (ret)
> > > > > +                       return ret;
> > > > > +       }
> > > > > 
> > > > >           cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
> > > > > 
> > > > > --
> > > > > 2.39.5
> > > > > 
> > > > 
> > > > 
> > 

-- 
With best wishes
Dmitry

Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[Peter Rosin]

Hi!

2025-03-24 at 16:58, Srinivas Kandagatla wrote:
> 
> 
> On 24/03/2025 15:18, Dmitry Baryshkov wrote:
>> On Mon, Mar 24, 2025 at 01:58:06PM +0000, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 24/03/2025 13:50, Dmitry Baryshkov wrote:
>>>> On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@linaro.org> wrote:
>>>>>
>>>>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

*snip*

>>>>> +       int ret = mux_control_try_select(wcd938x->us_euro_mux, state);
>>>>
>>>> Hmm. Does this really work? You have selected the mux in probe
>>>> function, now you are trying to select it again. If I'm reading the
>>>> code correctly, you will get -EBUSY here.
>>>
>>> On successful selection of mux state, the mux will be kept available
>>> (mux_control_deselect) for any new callers.
>>>
>>> So we will not get EBUSY for the second caller.
>>
>> No. wcd938x_populate_dt_data() selects the state by calling
>> wcd938x_select_mux_state().
> 
> At this point we also released it (both in success and error case).
> 
> This will hold on to the previous state unless we have defined a fallback idle-state.
> 
> 
>  Then you call mux_control_try_select() here.
>> As far as I understand, it will return -EBUSY as the sempahore is > already taken. Moreover, this is not how the MUX API is supposed to be
>> used. The driver is supposed to hold a state while it is still in use.

Dmitry is correct. A mux consumer is supposed to keep the mux selected
while it needs the mux to remain in a certain state. Relying on details
such as idle as-is and that no other consumer butts in and clobbers the
state is fragile. Mux access is not exclusive, at least not until a
mux state is selected.

Cheers,
Peter

Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[Srinivas Kandagatla]

On 24/03/2025 16:33, Peter Rosin wrote:
> Hi!
> 
> 2025-03-24 at 16:58, Srinivas Kandagatla wrote:
>>
>>
>> On 24/03/2025 15:18, Dmitry Baryshkov wrote:
>>> On Mon, Mar 24, 2025 at 01:58:06PM +0000, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 24/03/2025 13:50, Dmitry Baryshkov wrote:
>>>>> On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@linaro.org> wrote:
>>>>>>
>>>>>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> *snip*
> 
>>>>>> +       int ret = mux_control_try_select(wcd938x->us_euro_mux, state);
>>>>>
>>>>> Hmm. Does this really work? You have selected the mux in probe
>>>>> function, now you are trying to select it again. If I'm reading the
>>>>> code correctly, you will get -EBUSY here.
>>>>
>>>> On successful selection of mux state, the mux will be kept available
>>>> (mux_control_deselect) for any new callers.
>>>>
>>>> So we will not get EBUSY for the second caller.
>>>
>>> No. wcd938x_populate_dt_data() selects the state by calling
>>> wcd938x_select_mux_state().
>>
>> At this point we also released it (both in success and error case).
>>
>> This will hold on to the previous state unless we have defined a fallback idle-state.
>>
>>
>>   Then you call mux_control_try_select() here.
>>> As far as I understand, it will return -EBUSY as the sempahore is > already taken. Moreover, this is not how the MUX API is supposed to be
>>> used. The driver is supposed to hold a state while it is still in use.
> 
> Dmitry is correct. A mux consumer is supposed to keep the mux selected
> while it needs the mux to remain in a certain state. Relying on details
> such as idle as-is and that no other consumer butts in and clobbers the
> state is fragile. Mux access is not exclusive, at least not until a
> mux state is selected.
Thanks Peter,  I agree that its fragile to depend on idle as-is flags.

Will update accordingly.

--srini
> 
> Cheers,
> Peter

[PATCH v4 6/6] arm64: dts: qcom: x1e78100-t14s: Enable audio headset support

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

On Lenovo ThinkPad T14s, the headset is connected via a HiFi mux to
support CTIA and OMTP headsets. This switch is used to minimise pop and
click during headset type switching.

Enable the mux controls required to power this switch along with wiring up
gpio that control the headset switching.

Without this, headset audio will be very noisy and might see headset
detection errors.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Christopher Obbard <christopher.obbard@linaro.org>
---
 .../qcom/x1e78100-lenovo-thinkpad-t14s.dtsi   | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
index afea82616bc4..34d1d8927484 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
@@ -41,6 +41,7 @@ wcd938x: audio-codec {
 		qcom,tx-device = <&wcd_tx>;
 
 		reset-gpios = <&tlmm 191 GPIO_ACTIVE_LOW>;
+		mux-controls = <&us_euro_mux_ctrl>;
 
 		vdd-buck-supply = <&vreg_l15b_1p8>;
 		vdd-rxtx-supply = <&vreg_l15b_1p8>;
@@ -149,6 +150,16 @@ pmic_glink_ss1_con_sbu_in: endpoint {
 		};
 	};
 
+	/* two muxes together support CTIA and OMTP switching */
+	us_euro_mux_ctrl: mux-controller {
+		compatible = "gpio-mux";
+		pinctrl-0 = <&us_euro_hs_sel>;
+		pinctrl-names = "default";
+		mux-supply = <&vreg_l16b_2p5>;
+		#mux-control-cells = <0>;
+		mux-gpios = <&tlmm 68 GPIO_ACTIVE_HIGH>;
+	};
+
 	reserved-memory {
 		linux,cma {
 			compatible = "shared-dma-pool";
@@ -604,6 +615,13 @@ vreg_l15b_1p8: ldo15 {
 			regulator-always-on;
 		};
 
+		vreg_l16b_2p5: ldo16 {
+			regulator-name = "vreg_l16b_2p5";
+			regulator-min-microvolt = <2504000>;
+			regulator-max-microvolt = <2504000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
 		vreg_l17b_2p5: ldo17 {
 			regulator-name = "vreg_l17b_2p5";
 			regulator-min-microvolt = <2504000>;
@@ -1461,6 +1479,13 @@ rtmr1_default: rtmr1-reset-n-active-state {
 		bias-disable;
 	};
 
+	us_euro_hs_sel: us-euro-hs-sel-state {
+		pins = "gpio68";
+		function = "gpio";
+		bias-pull-down;
+		drive-strength = <2>;
+	};
+
 	usb1_pwr_1p15_reg_en: usb1-pwr-1p15-reg-en-state {
 		pins = "gpio188";
 		function = "gpio";
-- 
2.39.5