| Message ID | 20250320115633.4248-5-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: wcd938x: enable t14s audio headset | expand |
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; > }
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 > >
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; >> } >
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.
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 >
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; >> } >
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.
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; > > > } > >
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); }