| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ctcqkdbv6zh2rabkkr7tlhxlcfsn5nazjfbsnbbu4l4blyakft@pejdsvnazfh6>
Date: Mon, 24 Mar 2025 17:18:12 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc: peda@...ntia.se, broonie@...nel.org, andersson@...nel.org,
krzk+dt@...nel.org, ivprusov@...utedevices.com,
luca.ceresoli@...tlin.com, zhoubinbin@...ngson.cn,
paulha@...nsource.cirrus.com, lgirdwood@...il.com, robh@...nel.org,
conor+dt@...nel.org, konradybcio@...nel.org, perex@...ex.cz,
tiwai@...e.com, linux-sound@...r.kernel.org,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, johan+linaro@...nel.org,
Christopher Obbard <christopher.obbard@...aro.org>
Subject: Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support
for hp audio mux
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@...aro.org> wrote:
> > >
> > > From: Srinivas Kandagatla <srinivas.kandagatla@...aro.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@...aro.org>
> > > Tested-by: Christopher Obbard <christopher.obbard@...aro.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
Powered by blists - more mailing lists