| Message ID | 20250324110606.32001-6-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: wcd938x: enable t14s audio headset | expand |
On 24/03/2025 13:06, 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 | 38 ++++++++++++++++++++++++++++++-------- > 2 files changed, 31 insertions(+), 8 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..948b5f6cc45a 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) > > 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); Is it possible to use mux_state for both GPIO and MUX paths? > + } 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"); This can lead to mux being deselected next time even if the mux_control_select returned an error. I think mux_control API needs a way to toggle the state without deselecting it first. Anyway, an error from mux_control_select() must prevent you from calling mux_control_deselect() next time. > + } > > return true; > }
On 24/03/2025 11:20, Dmitry Baryshkov wrote: > On 24/03/2025 13:06, 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 | 38 ++++++++++++++++++++++++++++++-------- >> 2 files changed, 31 insertions(+), 8 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..948b5f6cc45a 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) >> 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); > > Is it possible to use mux_state for both GPIO and MUX paths? Ideally I would like to do that the way that gpio is done, which is clear reflection of hw state, however mux f/w is lacking such api. > >> + } 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"); > > This can lead to mux being deselected next time even if the > mux_control_select returned an error. I think mux_control API needs a > way to toggle the state without deselecting it first. Anyway, an error > from mux_control_select() must prevent you from calling > mux_control_deselect() next time. We can rearrange deselect to be done only on successful select, that should cleanup some of this. --srini > >> + } >> return true; >> } > >
On 24/03/2025 14:04, Srinivas Kandagatla wrote: > > > On 24/03/2025 11:20, Dmitry Baryshkov wrote: >> On 24/03/2025 13:06, 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 | 38 ++++++++++++++++++++++++++++++-------- >>> 2 files changed, 31 insertions(+), 8 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..948b5f6cc45a 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) >>> 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); >> >> Is it possible to use mux_state for both GPIO and MUX paths? > > Ideally I would like to do that the way that gpio is done, which is > clear reflection of hw state, however mux f/w is lacking such api. Anyway, both paths should use the same flow. > > >> >>> + } 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"); >> >> This can lead to mux being deselected next time even if the >> mux_control_select returned an error. I think mux_control API needs a >> way to toggle the state without deselecting it first. Anyway, an error >> from mux_control_select() must prevent you from calling >> mux_control_deselect() next time. > > We can rearrange deselect to be done only on successful select, that > should cleanup some of this. Yes, please. > > --srini >> >>> + } >>> return true; >>> } >> >>
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..948b5f6cc45a 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) 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); }