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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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);
+       }
+



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux