[PATCH v2 0/5] ASoC: wcd938x: enable t14s audio headset

[PATCH v2 0/5] 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 v1:
	- moved to using mux-controls.
	- fixed typo in regulator naming.

Srinivas Kandagatla (5):
  dt-bindings: mux: add optional regulator binding to gpio mux
  mux: gpio: add optional regulator support
  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          |  7 +++-
 .../qcom/x1e78100-lenovo-thinkpad-t14s.dts    | 25 ++++++++++++
 drivers/mux/gpio.c                            |  8 ++++
 sound/soc/codecs/Kconfig                      |  2 +
 sound/soc/codecs/wcd938x.c                    | 38 +++++++++++++++----
 6 files changed, 75 insertions(+), 9 deletions(-)

-- 
2.39.5

[PATCH v2 1/5] 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>
---
 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

[PATCH v2 2/5] 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>
---
 drivers/mux/gpio.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index cc5f2c1861d4..12cd9b5c32fb 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,13 @@ 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) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Couldn't retrieve/enable gpio mux supply\n");
+		return ret;
+	}
+
 	ret = devm_mux_chip_register(dev, mux_chip);
 	if (ret < 0)
 		return ret;
-- 
2.39.5

[PATCH v2 3/5] 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>
---
 Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
index 10531350c336..e7aa00a9c59a 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
@@ -23,8 +23,13 @@ properties:
       - qcom,wcd9380-codec
       - qcom,wcd9385-codec
 
+  mux-controls:
+    description: A reference to the audio mux switch for
+      switching CTIA/OMTP Headset types
+
   us-euro-gpios:
-    description: GPIO spec for swapping gnd and mic segments
+    description: GPIO spec for swapping gnd and mic segments.
+      This property is considered obsolete, recommended to use mux-controls.
     maxItems: 1
 
 required:
-- 
2.39.5

[PATCH v2 4/5] 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>
---
 sound/soc/codecs/Kconfig   |  2 ++
 sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index ee35f3aa5521..b04076282c8b 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
 	tristate
 	depends on SOUNDWIRE || !SOUNDWIRE
 	select SND_SOC_WCD_CLASSH
+	select MULTIPLEXER
+	imply MUX_GPIO
 
 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 f2a4f3262bdb..b7a235eef6ba 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;
@@ -3243,9 +3246,16 @@ static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component, bool activ
 
 	wcd938x = snd_soc_component_get_drvdata(component);
 
-	value = gpiod_get_value(wcd938x->us_euro_gpio);
+	if (!wcd938x->us_euro_mux) {
+		value = gpiod_get_value(wcd938x->us_euro_gpio);
 
-	gpiod_set_value(wcd938x->us_euro_gpio, !value);
+		gpiod_set_value(wcd938x->us_euro_gpio, !value);
+	} else {
+		mux_control_deselect(wcd938x->us_euro_mux);
+		wcd938x->mux_state = !wcd938x->mux_state;
+		if (mux_control_select(wcd938x->us_euro_mux, wcd938x->mux_state))
+			dev_err(component->dev, "Unable to select us/euro mux state\n");
+	}
 
 	return true;
 }
@@ -3261,14 +3271,23 @@ 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 {
+		if (mux_control_select(wcd938x->us_euro_mux, wcd938x->mux_state))
+			dev_err(dev, "Unable to select us/euro mux state\n");
+	}
 
 	cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
-
 	wcd938x->supplies[0].supply = "vdd-rxtx";
 	wcd938x->supplies[1].supply = "vdd-io";
 	wcd938x->supplies[2].supply = "vdd-buck";
@@ -3581,6 +3600,9 @@ static void wcd938x_remove(struct platform_device *pdev)
 	pm_runtime_set_suspended(dev);
 	pm_runtime_dont_use_autosuspend(dev);
 
+	if (wcd938x->us_euro_mux)
+		mux_control_deselect(wcd938x->us_euro_mux);
+
 	regulator_bulk_disable(WCD938X_MAX_SUPPLY, wcd938x->supplies);
 	regulator_bulk_free(WCD938X_MAX_SUPPLY, wcd938x->supplies);
 }
-- 
2.39.5

[PATCH v2 5/5] 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>
---
 .../qcom/x1e78100-lenovo-thinkpad-t14s.dts    | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
index b2c2347f54fa..b40775c20493 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
@@ -19,6 +19,16 @@ / {
 	compatible = "lenovo,thinkpad-t14s", "qcom,x1e78100", "qcom,x1e80100";
 	chassis-type = "laptop";
 
+	/* 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>;
+	};
+
 	wcd938x: audio-codec {
 		compatible = "qcom,wcd9385-codec";
 
@@ -36,6 +46,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>;
@@ -367,6 +378,13 @@ vreg_l15b_1p8: ldo15 {
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
+		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>;
@@ -942,6 +960,13 @@ int-n-pins {
 		};
 	};
 
+	us_euro_hs_sel: us-euro-hs-sel-state {
+		pins = "gpio68";
+		function = "gpio";
+		bias-pull-down;
+		drive-strength = <2>;
+	};
+
 	kybd_default: kybd-default-state {
 		pins = "gpio67";
 		function = "gpio";
-- 
2.39.5

Re: [PATCH v2 2/5] mux: gpio: add optional regulator support

[Maud Spierings]

> 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>
> ---
>  drivers/mux/gpio.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
> index cc5f2c1861d4..12cd9b5c32fb 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,13 @@ 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) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "Couldn't retrieve/enable gpio mux supply\n");
> +		return ret;
> +	}

This seems like a good place to use return dev_err_probe()

> +
>  	ret = devm_mux_chip_register(dev, mux_chip);
>  	if (ret < 0)
>  		return ret;
> -- 
> 2.39.5

Kind regards,
Maud

Re: [PATCH v2 5/5] arm64: dts: qcom: x1e78100-t14s: Enable audio headset support

[Dmitry Baryshkov]

On Thu, Mar 20, 2025 at 11:56:33AM +0000, srinivas.kandagatla@linaro.org wrote:
> 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>
> ---
>  .../qcom/x1e78100-lenovo-thinkpad-t14s.dts    | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> index b2c2347f54fa..b40775c20493 100644
> --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> @@ -19,6 +19,16 @@ / {
>  	compatible = "lenovo,thinkpad-t14s", "qcom,x1e78100", "qcom,x1e80100";
>  	chassis-type = "laptop";
>  
> +	/* two muxes together support CTIA and OMTP switching */
> +	us_euro_mux_ctrl: mux-controller {

This node should find its place so that the nodes are sorted
alphabetically.

> +		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>;
> +	};
> +
>  	wcd938x: audio-codec {
>  		compatible = "qcom,wcd9385-codec";
>  

[...]

> @@ -942,6 +960,13 @@ int-n-pins {
>  		};
>  	};
>  
> +	us_euro_hs_sel: us-euro-hs-sel-state {

This one also should be moved to keep them sorted.

LGTM otherwise.

> +		pins = "gpio68";
> +		function = "gpio";
> +		bias-pull-down;
> +		drive-strength = <2>;
> +	};
> +
>  	kybd_default: kybd-default-state {
>  		pins = "gpio67";
>  		function = "gpio";
> -- 
> 2.39.5
> 

-- 
With best wishes
Dmitry

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

[Dmitry Baryshkov]

On Thu, Mar 20, 2025 at 11:56:32AM +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 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>
> ---
>  sound/soc/codecs/Kconfig   |  2 ++
>  sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
>  2 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index ee35f3aa5521..b04076282c8b 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
>  	tristate
>  	depends on SOUNDWIRE || !SOUNDWIRE
>  	select SND_SOC_WCD_CLASSH
> +	select MULTIPLEXER
> +	imply MUX_GPIO

Why? This is true for a particular platform, isn't it?

>  
>  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 f2a4f3262bdb..b7a235eef6ba 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;
> @@ -3243,9 +3246,16 @@ static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component, bool activ
>  
>  	wcd938x = snd_soc_component_get_drvdata(component);
>  
> -	value = gpiod_get_value(wcd938x->us_euro_gpio);
> +	if (!wcd938x->us_euro_mux) {
> +		value = gpiod_get_value(wcd938x->us_euro_gpio);
>  
> -	gpiod_set_value(wcd938x->us_euro_gpio, !value);
> +		gpiod_set_value(wcd938x->us_euro_gpio, !value);

This looks like a separate topic, but why is 'active' being ignored?

> +	} else {
> +		mux_control_deselect(wcd938x->us_euro_mux);
> +		wcd938x->mux_state = !wcd938x->mux_state;
> +		if (mux_control_select(wcd938x->us_euro_mux, wcd938x->mux_state))

Can't it just be 'mux_control_select(wcd938x->us_euro_mux, active)' ?

> +			dev_err(component->dev, "Unable to select us/euro mux state\n");
> +	}
>  
>  	return true;
>  }

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/5] arm64: dts: qcom: x1e78100-t14s: Enable audio headset support

[Christopher Obbard]

Hi Srini,

On Thu, 20 Mar 2025 at 12:02, <srinivas.kandagatla@linaro.org> wrote:
>
> 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>

I tested this series (with
https://github.com/alsa-project/alsa-ucm-conf/pull/522 and latest
audioreach firmware X1E80100-LENOVO-Thinkpad-T14s-tplg.bin) on top of
Johan's kernel (https://github.com/jhovold/linux/tree/wip/x1e80100-6.14-rc7/)
on OLED T14s with Gnome and PipeWire/WirePlumber from Debian unstable
and everything appears to work OK.

The following works:
- internal speakers (no regression)
- internal mic (no regression)
- 3-pin headset, the default output auto-switches when plugged/unplugged
- 4-pin headset with mic, the default output/input auto-switches when
plugged/unplugged

Please let me know if I can test anything else.

Tested-by: Christopher Obbard <christopher.obbard@linaro.org>

> ---
>  .../qcom/x1e78100-lenovo-thinkpad-t14s.dts    | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> index b2c2347f54fa..b40775c20493 100644
> --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> @@ -19,6 +19,16 @@ / {
>         compatible = "lenovo,thinkpad-t14s", "qcom,x1e78100", "qcom,x1e80100";
>         chassis-type = "laptop";
>
> +       /* 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>;
> +       };
> +
>         wcd938x: audio-codec {
>                 compatible = "qcom,wcd9385-codec";
>
> @@ -36,6 +46,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>;
> @@ -367,6 +378,13 @@ vreg_l15b_1p8: ldo15 {
>                         regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>                 };
>
> +               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>;
> @@ -942,6 +960,13 @@ int-n-pins {
>                 };
>         };
>
> +       us_euro_hs_sel: us-euro-hs-sel-state {
> +               pins = "gpio68";
> +               function = "gpio";
> +               bias-pull-down;
> +               drive-strength = <2>;
> +       };
> +
>         kybd_default: kybd-default-state {
>                 pins = "gpio67";
>                 function = "gpio";
> --
> 2.39.5
>
>

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

[Christopher Obbard]

Hi Srini,

On Thu, 20 Mar 2025 at 12:03, <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   |  2 ++
>  sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
>  2 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index ee35f3aa5521..b04076282c8b 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
>         tristate
>         depends on SOUNDWIRE || !SOUNDWIRE
>         select SND_SOC_WCD_CLASSH
> +       select MULTIPLEXER
> +       imply MUX_GPIO
>
>  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 f2a4f3262bdb..b7a235eef6ba 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;
> @@ -3243,9 +3246,16 @@ static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component, bool activ
>
>         wcd938x = snd_soc_component_get_drvdata(component);
>
> -       value = gpiod_get_value(wcd938x->us_euro_gpio);
> +       if (!wcd938x->us_euro_mux) {
> +               value = gpiod_get_value(wcd938x->us_euro_gpio);
>
> -       gpiod_set_value(wcd938x->us_euro_gpio, !value);
> +               gpiod_set_value(wcd938x->us_euro_gpio, !value);
> +       } else {
> +               mux_control_deselect(wcd938x->us_euro_mux);
> +               wcd938x->mux_state = !wcd938x->mux_state;
> +               if (mux_control_select(wcd938x->us_euro_mux, wcd938x->mux_state))
> +                       dev_err(component->dev, "Unable to select us/euro mux state\n");
> +       }
>
>         return true;
>  }
> @@ -3261,14 +3271,23 @@ 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 {
> +               if (mux_control_select(wcd938x->us_euro_mux, wcd938x->mux_state))
> +                       dev_err(dev, "Unable to select us/euro mux state\n");
> +       }
>
>         cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
> -
>         wcd938x->supplies[0].supply = "vdd-rxtx";
>         wcd938x->supplies[1].supply = "vdd-io";
>         wcd938x->supplies[2].supply = "vdd-buck";
> @@ -3581,6 +3600,9 @@ static void wcd938x_remove(struct platform_device *pdev)
>         pm_runtime_set_suspended(dev);
>         pm_runtime_dont_use_autosuspend(dev);
>
> +       if (wcd938x->us_euro_mux)
> +               mux_control_deselect(wcd938x->us_euro_mux);
> +
>         regulator_bulk_disable(WCD938X_MAX_SUPPLY, wcd938x->supplies);
>         regulator_bulk_free(WCD938X_MAX_SUPPLY, wcd938x->supplies);
>  }
> --
> 2.39.5
>
>

Re: [PATCH v2 2/5] mux: gpio: add optional regulator support

[Krzysztof Kozlowski]

On 20/03/2025 12:56, srinivas.kandagatla@linaro.org wrote:
>  struct mux_gpio {
>  	struct gpio_descs *gpios;
> @@ -82,6 +83,13 @@ 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) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "Couldn't retrieve/enable gpio mux supply\n");

return dev_err_probe



Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 20/03/2025 12:56, 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>
> ---
>  Documentation/devicetree/bindings/mux/gpio-mux.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 3/5] ASoC: dt-bindings: wcd93xx: add bindings for audio mux controlling hp

[Krzysztof Kozlowski]

On 20/03/2025 12:56, srinivas.kandagatla@linaro.org wrote:
> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
> index 10531350c336..e7aa00a9c59a 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
> @@ -23,8 +23,13 @@ properties:
>        - qcom,wcd9380-codec
>        - qcom,wcd9385-codec
>  
> +  mux-controls:
> +    description: A reference to the audio mux switch for
> +      switching CTIA/OMTP Headset types
> +
>    us-euro-gpios:
> -    description: GPIO spec for swapping gnd and mic segments
> +    description: GPIO spec for swapping gnd and mic segments.
> +      This property is considered obsolete, recommended to use mux-controls.
>      maxItems: 1


Assuming intention is to really obsolete/deprecate, then please add:

  deprecated: true




Best regards,
Krzysztof

Re: [PATCH v2 5/5] arm64: dts: qcom: x1e78100-t14s: Enable audio headset support

[Krzysztof Kozlowski]

On 20/03/2025 12:56, srinivas.kandagatla@linaro.org wrote:
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> index b2c2347f54fa..b40775c20493 100644
> --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> @@ -19,6 +19,16 @@ / {
>  	compatible = "lenovo,thinkpad-t14s", "qcom,x1e78100", "qcom,x1e80100";
>  	chassis-type = "laptop";
>  
> +	/* two muxes together support CTIA and OMTP switching */
> +	us_euro_mux_ctrl: mux-controller {

This goes somewhere after audio-codec, because nodes are ordered by node
name.

> +		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>;
> +	};
> +
With placement fixed:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 3/5] ASoC: dt-bindings: wcd93xx: add bindings for audio mux controlling hp

[Srinivas Kandagatla]

On 21/03/2025 09:29, Krzysztof Kozlowski wrote:
> On 20/03/2025 12:56, srinivas.kandagatla@linaro.org wrote:
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
>> index 10531350c336..e7aa00a9c59a 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
>> @@ -23,8 +23,13 @@ properties:
>>         - qcom,wcd9380-codec
>>         - qcom,wcd9385-codec
>>   
>> +  mux-controls:
>> +    description: A reference to the audio mux switch for
>> +      switching CTIA/OMTP Headset types
>> +
>>     us-euro-gpios:
>> -    description: GPIO spec for swapping gnd and mic segments
>> +    description: GPIO spec for swapping gnd and mic segments.
>> +      This property is considered obsolete, recommended to use mux-controls.
>>       maxItems: 1
> 
> 
> Assuming intention is to really obsolete/deprecate, then please add:
> 
>    deprecated: true

Thanks, I was looking for this flag..

v3 will fix this.

--srini
> 
> 
> 
> 
> Best regards,
> Krzysztof

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

[Srinivas Kandagatla]

On 20/03/2025 14:03, Dmitry Baryshkov wrote:
> On Thu, Mar 20, 2025 at 11:56:32AM +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 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>
>> ---
>>   sound/soc/codecs/Kconfig   |  2 ++
>>   sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
>>   2 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
>> index ee35f3aa5521..b04076282c8b 100644
>> --- a/sound/soc/codecs/Kconfig
>> +++ b/sound/soc/codecs/Kconfig
>> @@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
>>   	tristate
>>   	depends on SOUNDWIRE || !SOUNDWIRE
>>   	select SND_SOC_WCD_CLASSH
>> +	select MULTIPLEXER
>> +	imply MUX_GPIO
> 
> Why? This is true for a particular platform, isn't it?

We want to move the codec to use gpio mux instead of using gpios directly

So this become codec specific, rather than platform.

> 
>>   
>>   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 f2a4f3262bdb..b7a235eef6ba 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;
>> @@ -3243,9 +3246,16 @@ static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component, bool activ
>>   
>>   	wcd938x = snd_soc_component_get_drvdata(component);
>>   
>> -	value = gpiod_get_value(wcd938x->us_euro_gpio);
>> +	if (!wcd938x->us_euro_mux) {
>> +		value = gpiod_get_value(wcd938x->us_euro_gpio);
>>   
>> -	gpiod_set_value(wcd938x->us_euro_gpio, !value);
>> +		gpiod_set_value(wcd938x->us_euro_gpio, !value);
> 
> This looks like a separate topic, but why is 'active' being ignored?

We should be able to use it.. will fix it in next version for mux usecase.

--srini
> 
>> +	} else {
>> +		mux_control_deselect(wcd938x->us_euro_mux);
>> +		wcd938x->mux_state = !wcd938x->mux_state;
>> +		if (mux_control_select(wcd938x->us_euro_mux, wcd938x->mux_state))
> 
> Can't it just be 'mux_control_select(wcd938x->us_euro_mux, active)' ?
> 
>> +			dev_err(component->dev, "Unable to select us/euro mux state\n");
>> +	}
>>   
>>   	return true;
>>   }
> 

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

[Dmitry Baryshkov]

On Fri, 21 Mar 2025 at 14:35, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
>
>
> On 20/03/2025 14:03, Dmitry Baryshkov wrote:
> > On Thu, Mar 20, 2025 at 11:56:32AM +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 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>
> >> ---
> >>   sound/soc/codecs/Kconfig   |  2 ++
> >>   sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
> >>   2 files changed, 32 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> >> index ee35f3aa5521..b04076282c8b 100644
> >> --- a/sound/soc/codecs/Kconfig
> >> +++ b/sound/soc/codecs/Kconfig
> >> @@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
> >>      tristate
> >>      depends on SOUNDWIRE || !SOUNDWIRE
> >>      select SND_SOC_WCD_CLASSH
> >> +    select MULTIPLEXER
> >> +    imply MUX_GPIO
> >
> > Why? This is true for a particular platform, isn't it?
>
> We want to move the codec to use gpio mux instead of using gpios directly
>
> So this become codec specific, rather than platform.

Not quite. "select MULTIPLEXER" is correct and is not questionable.
I'm asking about the MUX_GPIO. The codec itself has nothing to do with
the board using _GPIO_ to switch 4-pin modes. It is a board-level
decision. A board can use an I2C-controlled MUX instead. I'd say, that
at least you should describe rationale for this `imply` clause in the
commit message.

-- 
With best wishes
Dmitry

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

[Srinivas Kandagatla]

On 21/03/2025 13:16, Dmitry Baryshkov wrote:
> On Fri, 21 Mar 2025 at 14:35, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>>
>>
>>
>> On 20/03/2025 14:03, Dmitry Baryshkov wrote:
>>> On Thu, Mar 20, 2025 at 11:56:32AM +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 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>
>>>> ---
>>>>    sound/soc/codecs/Kconfig   |  2 ++
>>>>    sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
>>>>    2 files changed, 32 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
>>>> index ee35f3aa5521..b04076282c8b 100644
>>>> --- a/sound/soc/codecs/Kconfig
>>>> +++ b/sound/soc/codecs/Kconfig
>>>> @@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
>>>>       tristate
>>>>       depends on SOUNDWIRE || !SOUNDWIRE
>>>>       select SND_SOC_WCD_CLASSH
>>>> +    select MULTIPLEXER
>>>> +    imply MUX_GPIO
>>>
>>> Why? This is true for a particular platform, isn't it?
>>
>> We want to move the codec to use gpio mux instead of using gpios directly
>>
>> So this become codec specific, rather than platform.
> 
> Not quite. "select MULTIPLEXER" is correct and is not questionable.
> I'm asking about the MUX_GPIO. The codec itself has nothing to do with
> the board using _GPIO_ to switch 4-pin modes. It is a board-level
> decision. A board can use an I2C-controlled MUX instead. I'd say, that
> at least you should describe rationale for this `imply` clause in the
> commit message.

I agree to you point, but historically in this case us/euro selection is 
only driven by gpio. But I see no harm in moving the MUX_GPIO dependency 
to machine driver KConfigs.

Will fix this in v3.

thanks,
Srini
> 

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

[Srinivas Kandagatla]

On 20/03/2025 14:03, Dmitry Baryshkov wrote:
> On Thu, Mar 20, 2025 at 11:56:32AM +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 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>
>> ---
>>   sound/soc/codecs/Kconfig   |  2 ++
>>   sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
>>   2 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
>> index ee35f3aa5521..b04076282c8b 100644
>> --- a/sound/soc/codecs/Kconfig
>> +++ b/sound/soc/codecs/Kconfig
>> @@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
>>   	tristate
>>   	depends on SOUNDWIRE || !SOUNDWIRE
>>   	select SND_SOC_WCD_CLASSH
>> +	select MULTIPLEXER
>> +	imply MUX_GPIO
> 
> Why? This is true for a particular platform, isn't it?
> 
>>   
>>   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 f2a4f3262bdb..b7a235eef6ba 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;
>> @@ -3243,9 +3246,16 @@ static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component, bool activ
>>   
>>   	wcd938x = snd_soc_component_get_drvdata(component);
>>   
>> -	value = gpiod_get_value(wcd938x->us_euro_gpio);
>> +	if (!wcd938x->us_euro_mux) {
>> +		value = gpiod_get_value(wcd938x->us_euro_gpio);
>>   
>> -	gpiod_set_value(wcd938x->us_euro_gpio, !value);
>> +		gpiod_set_value(wcd938x->us_euro_gpio, !value);
> 
> This looks like a separate topic, but why is 'active' being ignored?
> 
>> +	} else {
>> +		mux_control_deselect(wcd938x->us_euro_mux);
>> +		wcd938x->mux_state = !wcd938x->mux_state;
>> +		if (mux_control_select(wcd938x->us_euro_mux, wcd938x->mux_state))
> 
> Can't it just be 'mux_control_select(wcd938x->us_euro_mux, active)' ?
> 

No, the way this is supposed to work is that if the codec detects cross 
connection, It will try to switch the mux to other option.

So using active will just work if we try to pulg one type of headset all 
the time. But if we change the headset type the mux will still be 
configured to use the old headset type and not work.

fyi, active is always set to true

I agree the argument to api is poorly labeled. It should be labeled as 
flip or something on those lines?


thanks,
Srini
>> +			dev_err(component->dev, "Unable to select us/euro mux state\n");
>> +	}
>>   
>>   	return true;
>>   }
> 

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

[Dmitry Baryshkov]

On Fri, Mar 21, 2025 at 01:26:44PM +0000, Srinivas Kandagatla wrote:
> 
> 
> On 21/03/2025 13:16, Dmitry Baryshkov wrote:
> > On Fri, 21 Mar 2025 at 14:35, Srinivas Kandagatla
> > <srinivas.kandagatla@linaro.org> wrote:
> > > 
> > > 
> > > 
> > > On 20/03/2025 14:03, Dmitry Baryshkov wrote:
> > > > On Thu, Mar 20, 2025 at 11:56:32AM +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 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>
> > > > > ---
> > > > >    sound/soc/codecs/Kconfig   |  2 ++
> > > > >    sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
> > > > >    2 files changed, 32 insertions(+), 8 deletions(-)
> > > > > 
> > > > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > > > > index ee35f3aa5521..b04076282c8b 100644
> > > > > --- a/sound/soc/codecs/Kconfig
> > > > > +++ b/sound/soc/codecs/Kconfig
> > > > > @@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
> > > > >       tristate
> > > > >       depends on SOUNDWIRE || !SOUNDWIRE
> > > > >       select SND_SOC_WCD_CLASSH
> > > > > +    select MULTIPLEXER
> > > > > +    imply MUX_GPIO
> > > > 
> > > > Why? This is true for a particular platform, isn't it?
> > > 
> > > We want to move the codec to use gpio mux instead of using gpios directly
> > > 
> > > So this become codec specific, rather than platform.
> > 
> > Not quite. "select MULTIPLEXER" is correct and is not questionable.
> > I'm asking about the MUX_GPIO. The codec itself has nothing to do with
> > the board using _GPIO_ to switch 4-pin modes. It is a board-level
> > decision. A board can use an I2C-controlled MUX instead. I'd say, that
> > at least you should describe rationale for this `imply` clause in the
> > commit message.
> 
> I agree to you point, but historically in this case us/euro selection is
> only driven by gpio. But I see no harm in moving the MUX_GPIO dependency to
> machine driver KConfigs.

Machine driver also doesn't depend on it. MUX_GPIO is selectedable item,
so please handle it via the usual way - defconfig.

-- 
With best wishes
Dmitry

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

[Dmitry Baryshkov]

On Fri, Mar 21, 2025 at 02:14:31PM +0000, Srinivas Kandagatla wrote:
> 
> 
> On 20/03/2025 14:03, Dmitry Baryshkov wrote:
> > On Thu, Mar 20, 2025 at 11:56:32AM +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 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>
> > > ---
> > >   sound/soc/codecs/Kconfig   |  2 ++
> > >   sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++--------
> > >   2 files changed, 32 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > > index ee35f3aa5521..b04076282c8b 100644
> > > --- a/sound/soc/codecs/Kconfig
> > > +++ b/sound/soc/codecs/Kconfig
> > > @@ -2226,6 +2226,8 @@ config SND_SOC_WCD938X
> > >   	tristate
> > >   	depends on SOUNDWIRE || !SOUNDWIRE
> > >   	select SND_SOC_WCD_CLASSH
> > > +	select MULTIPLEXER
> > > +	imply MUX_GPIO
> > 
> > Why? This is true for a particular platform, isn't it?
> > 
> > >   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 f2a4f3262bdb..b7a235eef6ba 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;
> > > @@ -3243,9 +3246,16 @@ static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component, bool activ
> > >   	wcd938x = snd_soc_component_get_drvdata(component);
> > > -	value = gpiod_get_value(wcd938x->us_euro_gpio);
> > > +	if (!wcd938x->us_euro_mux) {
> > > +		value = gpiod_get_value(wcd938x->us_euro_gpio);
> > > -	gpiod_set_value(wcd938x->us_euro_gpio, !value);
> > > +		gpiod_set_value(wcd938x->us_euro_gpio, !value);
> > 
> > This looks like a separate topic, but why is 'active' being ignored?
> > 
> > > +	} else {
> > > +		mux_control_deselect(wcd938x->us_euro_mux);
> > > +		wcd938x->mux_state = !wcd938x->mux_state;
> > > +		if (mux_control_select(wcd938x->us_euro_mux, wcd938x->mux_state))
> > 
> > Can't it just be 'mux_control_select(wcd938x->us_euro_mux, active)' ?
> > 
> 
> No, the way this is supposed to work is that if the codec detects cross
> connection, It will try to switch the mux to other option.

I see. It would be nice then to converge GPIO code and mux code in this
area. Invert mux_state, then use it in both paths.
> 
> So using active will just work if we try to pulg one type of headset all the
> time. But if we change the headset type the mux will still be configured to
> use the old headset type and not work.
> 
> fyi, active is always set to true
> 
> I agree the argument to api is poorly labeled. It should be labeled as flip
> or something on those lines?

If it is always true, then it should be dropped instead of renaming it.

> 
> 
> thanks,
> Srini
> > > +			dev_err(component->dev, "Unable to select us/euro mux state\n");
> > > +	}
> > >   	return true;
> > >   }
> > 

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/5] ASoC: dt-bindings: wcd93xx: add bindings for audio mux controlling hp

[Rob Herring]

On Thu, Mar 20, 2025 at 11:56:31AM +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 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>
> ---
>  Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
> index 10531350c336..e7aa00a9c59a 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
> @@ -23,8 +23,13 @@ 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
> +    description: GPIO spec for swapping gnd and mic segments.
> +      This property is considered obsolete, recommended to use mux-controls.
>      maxItems: 1
>  
>  required:
> -- 
> 2.39.5
>