[PATCH 0/2] Add Euro Headset support for wcd938x codec

[PATCH 0/2] Add Euro Headset support for wcd938x codec

[Srinivasa Rao Mandadapu]

This patch set is to add switch control for selecting CTIA/OMTP Headset

Srinivasa Rao Mandadapu (2):
  ASoC: codec: wcd938x: Add switch control for selecting CTIA/OMTP
    Headset
  ASoC: dt-bindings: wcd938x: Add gpio property for selecting CTIA/OMTP
    headset

 .../devicetree/bindings/sound/qcom,wcd938x.yaml    |  4 +++
 sound/soc/codecs/wcd938x.c                         | 38 ++++++++++++++++++++++
 2 files changed, 42 insertions(+)

-- 
2.7.4

[PATCH 1/2] ASoC: codec: wcd938x: Add switch control for selecting CTIA/OMTP Headset

[Srinivasa Rao Mandadapu]

Add switch control for selecting CTIA or OMTP Headset by swapping
gnd and mic with the help of GPIO.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 sound/soc/codecs/wcd938x.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index eff200a..08d16a9 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -194,6 +194,7 @@ struct wcd938x_priv {
 	int ear_rx_path;
 	int variant;
 	int reset_gpio;
+	int us_euro_gpio;
 	u32 micb1_mv;
 	u32 micb2_mv;
 	u32 micb3_mv;
@@ -4196,6 +4197,33 @@ 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, bool active)
+{
+	int value;
+
+	struct wcd938x_priv *wcd938x;
+
+	if (!component) {
+		dev_err(component->dev, "%s component is NULL\n", __func__);
+		return false;
+	}
+
+	wcd938x = snd_soc_component_get_drvdata(component);
+	if (!wcd938x) {
+		dev_err(component->dev, "%s private data is NULL\n", __func__);
+		return false;
+	}
+
+	value = gpio_get_value(wcd938x->us_euro_gpio);
+
+	gpio_set_value(wcd938x->us_euro_gpio, !value);
+	/* 20us sleep required after changing the gpio state*/
+	usleep_range(20, 30);
+
+	return true;
+}
+
+
 static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device *dev)
 {
 	struct wcd_mbhc_config *cfg = &wcd938x->mbhc_cfg;
@@ -4208,6 +4236,16 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
 		return wcd938x->reset_gpio;
 	}
 
+	wcd938x->us_euro_gpio = of_get_named_gpio(dev->of_node, "us-euro-gpios", 0);
+	if (wcd938x->us_euro_gpio < 0) {
+		dev_err(dev, "Failed to get us-euro-gpios gpio: err = %d\n", wcd938x->us_euro_gpio);
+	} else {
+		cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
+		gpio_direction_output(wcd938x->us_euro_gpio, 0);
+		/* 20us sleep required after pulling the reset gpio to LOW */
+		usleep_range(20, 30);
+	}
+
 	wcd938x->supplies[0].supply = "vdd-rxtx";
 	wcd938x->supplies[1].supply = "vdd-io";
 	wcd938x->supplies[2].supply = "vdd-buck";
-- 
2.7.4

Re: [PATCH 1/2] ASoC: codec: wcd938x: Add switch control for selecting CTIA/OMTP Headset

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-02-12 04:24:31)
> diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
> index eff200a..08d16a9 100644
> --- a/sound/soc/codecs/wcd938x.c
> +++ b/sound/soc/codecs/wcd938x.c
> @@ -194,6 +194,7 @@ struct wcd938x_priv {
>         int ear_rx_path;
>         int variant;
>         int reset_gpio;
> +       int us_euro_gpio;
>         u32 micb1_mv;
>         u32 micb2_mv;
>         u32 micb3_mv;
> @@ -4196,6 +4197,33 @@ 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, bool active)
> +{
> +       int value;
> +
> +       struct wcd938x_priv *wcd938x;
> +
> +       if (!component) {

So component is NULL

> +               dev_err(component->dev, "%s component is NULL\n", __func__);

And now we deref component. Great NULL pointer exception Batman! Please
test your code and remove useless checks. It makes the code harder to
read and slows things down.

> +               return false;
> +       }
> +
> +       wcd938x = snd_soc_component_get_drvdata(component);
> +       if (!wcd938x) {
> +               dev_err(component->dev, "%s private data is NULL\n", __func__);

Is this possible? I doubt it so can we just remove it?

> +               return false;
> +       }
> +
> +       value = gpio_get_value(wcd938x->us_euro_gpio);
> +
> +       gpio_set_value(wcd938x->us_euro_gpio, !value);
> +       /* 20us sleep required after changing the gpio state*/

Add a space before ending comment with */

> +       usleep_range(20, 30);
> +
> +       return true;
> +}
> +
> +
>  static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device *dev)
>  {
>         struct wcd_mbhc_config *cfg = &wcd938x->mbhc_cfg;
> @@ -4208,6 +4236,16 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
>                 return wcd938x->reset_gpio;
>         }
>
> +       wcd938x->us_euro_gpio = of_get_named_gpio(dev->of_node, "us-euro-gpios", 0);

Why do we need to use of GPIO APIs here? Can this driver be converted to
use GPIO descriptors via the gpiod APIs?

> +       if (wcd938x->us_euro_gpio < 0) {
> +               dev_err(dev, "Failed to get us-euro-gpios gpio: err = %d\n", wcd938x->us_euro_gpio);
> +       } else {
> +               cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
> +               gpio_direction_output(wcd938x->us_euro_gpio, 0);
> +               /* 20us sleep required after pulling the reset gpio to LOW */
> +               usleep_range(20, 30);
> +       }
> +

Re: [PATCH 1/2] ASoC: codec: wcd938x: Add switch control for selecting CTIA/OMTP Headset

[Srinivasa Rao Mandadapu]

On 2/15/2022 3:17 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-12 04:24:31)
>> diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
>> index eff200a..08d16a9 100644
>> --- a/sound/soc/codecs/wcd938x.c
>> +++ b/sound/soc/codecs/wcd938x.c
>> @@ -194,6 +194,7 @@ struct wcd938x_priv {
>>          int ear_rx_path;
>>          int variant;
>>          int reset_gpio;
>> +       int us_euro_gpio;
>>          u32 micb1_mv;
>>          u32 micb2_mv;
>>          u32 micb3_mv;
>> @@ -4196,6 +4197,33 @@ 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, bool active)
>> +{
>> +       int value;
>> +
>> +       struct wcd938x_priv *wcd938x;
>> +
>> +       if (!component) {
> So component is NULL
>
>> +               dev_err(component->dev, "%s component is NULL\n", __func__);
> And now we deref component. Great NULL pointer exception Batman! Please
> test your code and remove useless checks. It makes the code harder to
> read and slows things down.
Okay. In last minute, changed from pr_err to dev_err and overlooked this 
mistake. Will remove it.
>
>> +               return false;
>> +       }
>> +
>> +       wcd938x = snd_soc_component_get_drvdata(component);
>> +       if (!wcd938x) {
>> +               dev_err(component->dev, "%s private data is NULL\n", __func__);
> Is this possible? I doubt it so can we just remove it?
Okay. Will remove it.
>
>> +               return false;
>> +       }
>> +
>> +       value = gpio_get_value(wcd938x->us_euro_gpio);
>> +
>> +       gpio_set_value(wcd938x->us_euro_gpio, !value);
>> +       /* 20us sleep required after changing the gpio state*/
> Add a space before ending comment with */
Okay.
>
>> +       usleep_range(20, 30);
>> +
>> +       return true;
>> +}
>> +
>> +
>>   static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device *dev)
>>   {
>>          struct wcd_mbhc_config *cfg = &wcd938x->mbhc_cfg;
>> @@ -4208,6 +4236,16 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
>>                  return wcd938x->reset_gpio;
>>          }
>>
>> +       wcd938x->us_euro_gpio = of_get_named_gpio(dev->of_node, "us-euro-gpios", 0);
> Why do we need to use of GPIO APIs here? Can this driver be converted to
> use GPIO descriptors via the gpiod APIs?
Okay.  will look into it and proceed accordingly!!!
>
>> +       if (wcd938x->us_euro_gpio < 0) {
>> +               dev_err(dev, "Failed to get us-euro-gpios gpio: err = %d\n", wcd938x->us_euro_gpio);
>> +       } else {
>> +               cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
>> +               gpio_direction_output(wcd938x->us_euro_gpio, 0);
>> +               /* 20us sleep required after pulling the reset gpio to LOW */
>> +               usleep_range(20, 30);
>> +       }
>> +

[PATCH 2/2] ASoC: dt-bindings: wcd938x: Add gpio property for selecting CTIA/OMTP headset

[Srinivasa Rao Mandadapu]

Add gpio property used for selecting CTIA/OMTP headset connected
to wcd codec.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
index cb74ce4..7bf1a5f 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
@@ -23,6 +23,10 @@ properties:
     description: GPIO spec for reset line to use
     maxItems: 1
 
+  us-euro-gpios:
+    description: GPIO spec for swapping gnd and mic segments
+    maxItems: 1
+
   vdd-buck-supply:
     description: A reference to the 1.8V buck supply
 
-- 
2.7.4

Re: [PATCH 0/2] Add Euro Headset support for wcd938x codec

[Mark Brown]

On Sat, 12 Feb 2022 17:54:30 +0530, Srinivasa Rao Mandadapu wrote:
> This patch set is to add switch control for selecting CTIA/OMTP Headset
> 
> Srinivasa Rao Mandadapu (2):
>   ASoC: codec: wcd938x: Add switch control for selecting CTIA/OMTP
>     Headset
>   ASoC: dt-bindings: wcd938x: Add gpio property for selecting CTIA/OMTP
>     headset
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: codec: wcd938x: Add switch control for selecting CTIA/OMTP Headset
      commit: 013cc2aea0f62f864c8497b8497299bed4a248fb
[2/2] ASoC: dt-bindings: wcd938x: Add gpio property for selecting CTIA/OMTP headset
      commit: 20ea94bc5317475af70f003075e7988715457d66

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark