| 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: <324d5789-6309-4a64-bbfa-3afa0632e7ff@linaro.org>
Date: Mon, 24 Mar 2025 15:58:32 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
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 24/03/2025 15:18, Dmitry Baryshkov wrote:
> On Mon, Mar 24, 2025 at 01:58:06PM +0000, Srinivas Kandagatla wrote:
>>
>>
>> On 24/03/2025 13:50, Dmitry Baryshkov wrote:
>>> On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@...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().
At this point we also released it (both in success and error case).
This will hold on to the previous state unless we have defined a
fallback idle-state.
Then you call mux_control_try_select() here.
> As far as I understand, it will return -EBUSY as the sempahore is > already taken. Moreover, this is not how the MUX API is supposed to be
> used. The driver is supposed to hold a state while it is still in use.
>
>>
>> --srini
>>>
>>>>
>>>> - struct wcd938x_priv *wcd938x;
>>>> + if (ret) {
>>>> + dev_err(dev, "Error (%d) Unable to select us/euro mux state\n", ret);
>>>> + return ret;
>>>> + }
>>>>
>>>> - wcd938x = snd_soc_component_get_drvdata(component);
>>>> + wcd938x->mux_state = state;
>>>> + mux_control_deselect(wcd938x->us_euro_mux);
>>>> +
>>>> + return 0;
>>>> +}
>>>>
>>>> - value = gpiod_get_value(wcd938x->us_euro_gpio);
>>>> +static bool wcd938x_swap_gnd_mic(struct snd_soc_component *component)
>>>> +{
>>>> + struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
>>>>
>>>> - gpiod_set_value(wcd938x->us_euro_gpio, !value);
>>>> + if (wcd938x->us_euro_mux) {
>>>> + if (wcd938x_select_mux_state(component->dev, wcd938x, !wcd938x->mux_state))
>>>> + return false;
>>>> + } else {
>>>> + gpiod_set_value(wcd938x->us_euro_gpio, !wcd938x->mux_state);
>>>> + }
>>>>
>>>> return true;
>>>> }
>>>> @@ -3261,11 +3278,22 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
>>>> return dev_err_probe(dev, wcd938x->reset_gpio,
>>>> "Failed to get reset gpio\n");
>>>>
>>>> - wcd938x->us_euro_gpio = devm_gpiod_get_optional(dev, "us-euro",
>>>> - GPIOD_OUT_LOW);
>>>> - if (IS_ERR(wcd938x->us_euro_gpio))
>>>> - return dev_err_probe(dev, PTR_ERR(wcd938x->us_euro_gpio),
>>>> - "us-euro swap Control GPIO not found\n");
>>>> + wcd938x->us_euro_mux = devm_mux_control_get(dev, NULL);
>>>> + if (IS_ERR(wcd938x->us_euro_mux)) {
>>>> + if (PTR_ERR(wcd938x->us_euro_mux) == -EPROBE_DEFER)
>>>> + return -EPROBE_DEFER;
>>>> +
>>>> + /* mux is optional and now fallback to using gpio */
>>>> + wcd938x->us_euro_mux = NULL;
>>>> + wcd938x->us_euro_gpio = devm_gpiod_get_optional(dev, "us-euro", GPIOD_OUT_LOW);
>>>> + if (IS_ERR(wcd938x->us_euro_gpio))
>>>> + return dev_err_probe(dev, PTR_ERR(wcd938x->us_euro_gpio),
>>>> + "us-euro swap Control GPIO not found\n");
>>>> + } else {
>>>> + ret = wcd938x_select_mux_state(dev, wcd938x, wcd938x->mux_state);
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>>
>>>> cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
>>>>
>>>> --
>>>> 2.39.5
>>>>
>>>
>>>
>
Powered by blists - more mailing lists