[PATCH 0/3] Enhance imx-wm8962 machine driver

[PATCH 0/3] Enhance imx-wm8962 machine driver

[Nicolin Chen]

Although there's no direct relationship between any of these three patches,
I recommend to apply them in order so as to circumvent merging conflicts.

Nicolin Chen (3):
  ASoC: fsl: imx-wm8962: Don't update bias_level in machine driver
  ASoC: fsl: imx-wm8962: Grant hw_params/free() permission to control
    FLL
  ASoC: fsl: imx-wm8962: Add headphone and microphone jack plugin/out
    detection

 .../devicetree/bindings/sound/imx-audio-wm8962.txt |   8 +
 Documentation/devicetree/bindings/sound/wm8962.txt |   8 +
 sound/soc/fsl/imx-wm8962.c                         | 361 ++++++++++++++++++---
 3 files changed, 326 insertions(+), 51 deletions(-)

-- 
1.8.4

[PATCH 1/3] ASoC: fsl: imx-wm8962: Don't update bias_level in machine driver

[Nicolin Chen]

If we update it here, the set_bias_level() of Codec driver won't be normally
called and we will then miss some essential procedures in set_bias_level() of
the Codec driver. Thus drop it.

Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
 sound/soc/fsl/imx-wm8962.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c
index 61e4885..3fd76bc 100644
--- a/sound/soc/fsl/imx-wm8962.c
+++ b/sound/soc/fsl/imx-wm8962.c
@@ -130,8 +130,6 @@ static int imx_wm8962_set_bias_level(struct snd_soc_card *card,
 		break;
 	}
 
-	dapm->bias_level = level;
-
 	return 0;
 }
 
-- 
1.8.4

[PATCH 2/3] ASoC: fsl: imx-wm8962: Grant hw_params/free() permission to control FLL

[Nicolin Chen]

Previously, we couldn't use hw_params() and hw_free() to open and close FLL
becuase there might be a race between two simmultaneous substreams so the
FLL configuration would be changed and accordingly mulfunction. Also it
wouldn't make sense for bypass path feature of WM8962. So we adopted DAPM
way to control it. However, if we want to playback a different sample rate
file, we need to wait for DAPM to change its bias_level and reconfigure FLL.

But after we introduced full symmetry protection in the soc-pcm, we don't
need to worry about the race any more. And the instance by using hw_params()
and hw_free() to control FLL will allow us to support flexible use cases,
'aplay -Dhw:0 44k16bit.wav 48k24bit.wav 32k16bit.wav' for example.

Thus this patch mainly adds FLL configuration code to hw_params/hw_free()
so as to enchance the sound card's capability. Meanwhile in order not to
break the bypass path feature, we make both set_bias_level() and hw_xxx()
ways coexist.

Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
 sound/soc/fsl/imx-wm8962.c | 136 ++++++++++++++++++++++++++++-----------------
 1 file changed, 86 insertions(+), 50 deletions(-)

diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c
index 3fd76bc..8baf5d4 100644
--- a/sound/soc/fsl/imx-wm8962.c
+++ b/sound/soc/fsl/imx-wm8962.c
@@ -35,6 +35,7 @@ struct imx_wm8962_data {
 	char platform_name[DAI_NAME_SIZE];
 	struct clk *codec_clk;
 	unsigned int clk_frequency;
+	bool fll_lock;
 };
 
 struct imx_priv {
@@ -49,20 +50,93 @@ static const struct snd_soc_dapm_widget imx_wm8962_dapm_widgets[] = {
 	SND_SOC_DAPM_MIC("DMIC", NULL),
 };
 
-static int sample_rate = 44100;
-static snd_pcm_format_t sample_format = SNDRV_PCM_FORMAT_S16_LE;
+static int imx_wm8962_enable_fll(struct snd_soc_dai *codec_dai, u32 sample_rate,
+				snd_pcm_format_t sample_format)
+{
+	struct imx_priv *priv = &card_priv;
+	struct device *dev = &priv->pdev->dev;
+	struct imx_wm8962_data *data = platform_get_drvdata(priv->pdev);
+	u32 freq, ret;
+
+	if (data->fll_lock)
+		return 0;
+
+	data->fll_lock = 1;
+
+	if (sample_format == SNDRV_PCM_FORMAT_S24_LE)
+		freq = sample_rate * 384;
+	else
+		freq = sample_rate * 256;
+
+	ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL, WM8962_FLL_MCLK,
+				  data->clk_frequency, freq);
+	if (ret) {
+		dev_err(dev, "failed to start FLL: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_FLL,
+				     freq, SND_SOC_CLOCK_IN);
+	if (ret)
+		dev_err(dev, "failed to set SYSCLK: %d\n", ret);
+
+	return ret;
+}
+
+static int imx_wm8962_disable_fll(struct snd_soc_dai *codec_dai)
+{
+	struct imx_priv *priv = &card_priv;
+	struct device *dev = &priv->pdev->dev;
+	struct imx_wm8962_data *data = platform_get_drvdata(priv->pdev);
+	int ret;
+
+	if (!data->fll_lock)
+		return 0;
+
+	data->fll_lock = 0;
+
+	/* Switch to MCLK as sysclk once so as to disable FLL */
+	ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_MCLK,
+				     0, SND_SOC_CLOCK_IN);
+	if (ret) {
+		dev_err(dev, "failed to switch away from FLL: %d\n", ret);
+		return ret;
+	}
+
+	/* Disable FLL so that we can reset its output freq later */
+	ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL,
+				  WM8962_FLL_MCLK, 0, 0);
+	if (ret)
+		dev_err(dev, "failed to stop FLL: %d\n", ret);
+
+	return ret;
+}
 
 static int imx_hifi_hw_params(struct snd_pcm_substream *substream,
 		struct snd_pcm_hw_params *params)
 {
-	sample_rate = params_rate(params);
-	sample_format = params_format(params);
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 
-	return 0;
+	return imx_wm8962_enable_fll(codec_dai, params_rate(params),
+				     params_format(params));
+}
+
+static int imx_hifi_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+
+	/* Don't diable FLL if still having multiple substreams running */
+	if (codec_dai->active != 1)
+		return 0;
+
+	return imx_wm8962_disable_fll(codec_dai);
 }
 
 static struct snd_soc_ops imx_hifi_ops = {
 	.hw_params = imx_hifi_hw_params,
+	.hw_free = imx_hifi_hw_free,
 };
 
 static int imx_wm8962_set_bias_level(struct snd_soc_card *card,
@@ -70,60 +144,20 @@ static int imx_wm8962_set_bias_level(struct snd_soc_card *card,
 					enum snd_soc_bias_level level)
 {
 	struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai;
-	struct imx_priv *priv = &card_priv;
-	struct imx_wm8962_data *data = platform_get_drvdata(priv->pdev);
-	struct device *dev = &priv->pdev->dev;
-	unsigned int pll_out;
-	int ret;
 
 	if (dapm->dev != codec_dai->dev)
 		return 0;
 
 	switch (level) {
 	case SND_SOC_BIAS_PREPARE:
-		if (dapm->bias_level == SND_SOC_BIAS_STANDBY) {
-			if (sample_format == SNDRV_PCM_FORMAT_S24_LE)
-				pll_out = sample_rate * 384;
-			else
-				pll_out = sample_rate * 256;
-
-			ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL,
-					WM8962_FLL_MCLK, data->clk_frequency,
-					pll_out);
-			if (ret < 0) {
-				dev_err(dev, "failed to start FLL: %d\n", ret);
-				return ret;
-			}
-
-			ret = snd_soc_dai_set_sysclk(codec_dai,
-					WM8962_SYSCLK_FLL, pll_out,
-					SND_SOC_CLOCK_IN);
-			if (ret < 0) {
-				dev_err(dev, "failed to set SYSCLK: %d\n", ret);
-				return ret;
-			}
-		}
+		if (dapm->bias_level == SND_SOC_BIAS_STANDBY)
+			return imx_wm8962_enable_fll(codec_dai, 44100,
+						SNDRV_PCM_FORMAT_S16_LE);
 		break;
 
 	case SND_SOC_BIAS_STANDBY:
-		if (dapm->bias_level == SND_SOC_BIAS_PREPARE) {
-			ret = snd_soc_dai_set_sysclk(codec_dai,
-					WM8962_SYSCLK_MCLK, data->clk_frequency,
-					SND_SOC_CLOCK_IN);
-			if (ret < 0) {
-				dev_err(dev,
-					"failed to switch away from FLL: %d\n",
-					ret);
-				return ret;
-			}
-
-			ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL,
-					0, 0, 0);
-			if (ret < 0) {
-				dev_err(dev, "failed to stop FLL: %d\n", ret);
-				return ret;
-			}
-		}
+		if (dapm->bias_level == SND_SOC_BIAS_PREPARE)
+			return imx_wm8962_disable_fll(codec_dai);
 		break;
 
 	default:
@@ -141,6 +175,8 @@ static int imx_wm8962_late_probe(struct snd_soc_card *card)
 	struct device *dev = &priv->pdev->dev;
 	int ret;
 
+	data->fll_lock = 0;
+
 	ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_MCLK,
 			data->clk_frequency, SND_SOC_CLOCK_IN);
 	if (ret < 0)
-- 
1.8.4

Re: [PATCH 2/3] ASoC: fsl: imx-wm8962: Grant hw_params/free() permission to control FLL

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 452 bytes --]

On Fri, Dec 06, 2013 at 05:47:43PM +0800, Nicolin Chen wrote:

> +static int imx_wm8962_enable_fll(struct snd_soc_dai *codec_dai, u32 sample_rate,
> +				snd_pcm_format_t sample_format)
> +{
> +	struct imx_priv *priv = &card_priv;
> +	struct device *dev = &priv->pdev->dev;
> +	struct imx_wm8962_data *data = platform_get_drvdata(priv->pdev);
> +	u32 freq, ret;
> +
> +	if (data->fll_lock)
> +		return 0;

Don't we need an actual lock to protect this?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/3] ASoC: fsl: imx-wm8962: Grant hw_params/free() permission to control FLL

[Nicolin Chen]

On Fri, Dec 06, 2013 at 02:34:33PM +0000, Mark Brown wrote:
> On Fri, Dec 06, 2013 at 05:47:43PM +0800, Nicolin Chen wrote:
> 
> > +static int imx_wm8962_enable_fll(struct snd_soc_dai *codec_dai, u32 sample_rate,
> > +				snd_pcm_format_t sample_format)
> > +{
> > +	struct imx_priv *priv = &card_priv;
> > +	struct device *dev = &priv->pdev->dev;
> > +	struct imx_wm8962_data *data = platform_get_drvdata(priv->pdev);
> > +	u32 freq, ret;
> > +
> > +	if (data->fll_lock)
> > +		return 0;
> 
> Don't we need an actual lock to protect this?

Thank you for the advice, I will revise it.

Best regards,
Nicolin Chen

[PATCH 3/3] ASoC: fsl: imx-wm8962: Add headphone and microphone jack plugin/out detection

[Nicolin Chen]

Some platforms of i.MX series place two GPIOs to detect jack pluging events of
headphone and microphone so as to let software switch the output (input) route
between headphone (analog microphone) and external speakers (on-board digital
microphone).

Therefore, this patch mainly adds jack event detection feature and switchs I/O
routes accordingly. Meanwhile, for those monaural input cases, we also turn on
the ADC monomix features of WM8962 if needed.

Signed-off-by: Nicolin Chen <b42378-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 .../devicetree/bindings/sound/imx-audio-wm8962.txt |   8 +
 Documentation/devicetree/bindings/sound/wm8962.txt |   8 +
 sound/soc/fsl/imx-wm8962.c                         | 225 +++++++++++++++++++++
 3 files changed, 241 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/imx-audio-wm8962.txt b/Documentation/devicetree/bindings/sound/imx-audio-wm8962.txt
index f49450a..5d4222e 100644
--- a/Documentation/devicetree/bindings/sound/imx-audio-wm8962.txt
+++ b/Documentation/devicetree/bindings/sound/imx-audio-wm8962.txt
@@ -24,6 +24,12 @@ Required properties:
 Note: The AUDMUX port numbering should start at 1, which is consistent with
 hardware manual.
 
+Optional properties:
+- hp-det-gpios : The gpio pin to detect plug in/out event that happens to
+  Headphone jack.
+- mic-det-gpios: The gpio pin to detect plug in/out event that happens to
+  Microphone jack.
+
 Example:
 
 sound {
@@ -43,4 +49,6 @@ sound {
 		"DMICDAT", "DMIC";
 	mux-int-port = <2>;
 	mux-ext-port = <3>;
+	hp-det-gpios = <&gpio7 8 1>;
+	mic-det-gpios = <&gpio1 9 1>;
 };
diff --git a/Documentation/devicetree/bindings/sound/wm8962.txt b/Documentation/devicetree/bindings/sound/wm8962.txt
index 7f82b59..744d148 100644
--- a/Documentation/devicetree/bindings/sound/wm8962.txt
+++ b/Documentation/devicetree/bindings/sound/wm8962.txt
@@ -13,6 +13,14 @@ Optional properties:
     of R51 (Class D Control 2) gets set, indicating that the speaker is
     in mono mode.
 
+  - amic-mono: This is a boolean property. If present, indicating that the
+    analog micphone is hardware monaural input, the driver would enable the
+    monomix for it.
+
+  - dmic-mono: This is a boolean property. If present, indicating that the
+    digital micphone is hardware monaural input, the driver would enable the
+    monomix for it.
+
   - mic-cfg : Default register value for R48 (Additional Control 4).
     If absent, the default should be the register default.
 
diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c
index 8baf5d4..1f457df 100644
--- a/sound/soc/fsl/imx-wm8962.c
+++ b/sound/soc/fsl/imx-wm8962.c
@@ -14,10 +14,13 @@
  */
 
 #include <linux/module.h>
+#include <linux/of_gpio.h>
 #include <linux/of_platform.h>
+#include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/clk.h>
+#include <sound/jack.h>
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
 #include <sound/soc-dapm.h>
@@ -39,10 +42,113 @@ struct imx_wm8962_data {
 };
 
 struct imx_priv {
+	int hp_gpio;
+	int hp_active_low;
+	int mic_gpio;
+	int mic_active_low;
+	bool amic_mono;
+	bool dmic_mono;
+	struct snd_soc_codec *codec;
 	struct platform_device *pdev;
 };
 static struct imx_priv card_priv;
 
+static struct snd_soc_jack imx_hp_jack;
+static struct snd_soc_jack_pin imx_hp_jack_pins[] = {
+	{
+		.pin = "Headphone Jack",
+		.mask = SND_JACK_HEADPHONE,
+	},
+};
+static struct snd_soc_jack_gpio imx_hp_jack_gpio = {
+	.name = "headphone detect",
+	.report = SND_JACK_HEADPHONE,
+	.debounce_time = 250,
+	.invert = 0,
+};
+
+static struct snd_soc_jack imx_mic_jack;
+static struct snd_soc_jack_pin imx_mic_jack_pins[] = {
+	{
+		.pin = "AMIC",
+		.mask = SND_JACK_MICROPHONE,
+	},
+};
+static struct snd_soc_jack_gpio imx_mic_jack_gpio = {
+	.name = "microphone detect",
+	.report = SND_JACK_MICROPHONE,
+	.debounce_time = 250,
+	.invert = 0,
+};
+
+static int hpjack_status_check(void)
+{
+	struct imx_priv *priv = &card_priv;
+	struct platform_device *pdev = priv->pdev;
+	char *envp[3], buf[32];
+	int hp_status, ret;
+
+	if (!gpio_is_valid(priv->hp_gpio))
+		return 0;
+
+	hp_status = gpio_get_value(priv->hp_gpio) != 0;
+
+	if (hp_status != priv->hp_active_low) {
+		snprintf(buf, 32, "STATE=%d", 2);
+		snd_soc_dapm_disable_pin(&priv->codec->dapm, "Ext Spk");
+		ret = imx_hp_jack_gpio.report;
+	} else {
+		snprintf(buf, 32, "STATE=%d", 0);
+		snd_soc_dapm_enable_pin(&priv->codec->dapm, "Ext Spk");
+		ret = 0;
+	}
+
+	envp[0] = "NAME=headphone";
+	envp[1] = buf;
+	envp[2] = NULL;
+	kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, envp);
+
+	return ret;
+}
+
+static int micjack_status_check(void)
+{
+	struct imx_priv *priv = &card_priv;
+	struct platform_device *pdev = priv->pdev;
+	char *envp[3], buf[32];
+	int mic_status, ret;
+
+	if (!gpio_is_valid(priv->mic_gpio))
+		return 0;
+
+	mic_status = gpio_get_value(priv->mic_gpio) != 0;
+
+	if ((mic_status != priv->mic_active_low && priv->amic_mono) ||
+	    (mic_status == priv->mic_active_low && priv->dmic_mono))
+		snd_soc_update_bits(priv->codec, WM8962_THREED1,
+				WM8962_ADC_MONOMIX_MASK, WM8962_ADC_MONOMIX);
+	else
+		snd_soc_update_bits(priv->codec, WM8962_THREED1,
+				WM8962_ADC_MONOMIX_MASK, 0);
+
+	if (mic_status != priv->mic_active_low) {
+		snprintf(buf, 32, "STATE=%d", 2);
+		snd_soc_dapm_disable_pin(&priv->codec->dapm, "DMIC");
+		ret = imx_mic_jack_gpio.report;
+	} else {
+		snprintf(buf, 32, "STATE=%d", 0);
+		snd_soc_dapm_enable_pin(&priv->codec->dapm, "DMIC");
+		ret = 0;
+	}
+
+	envp[0] = "NAME=microphone";
+	envp[1] = buf;
+	envp[2] = NULL;
+	kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, envp);
+
+	return ret;
+}
+
 static const struct snd_soc_dapm_widget imx_wm8962_dapm_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone Jack", NULL),
 	SND_SOC_DAPM_SPK("Ext Spk", NULL),
@@ -167,6 +273,88 @@ static int imx_wm8962_set_bias_level(struct snd_soc_card *card,
 	return 0;
 }
 
+static int imx_wm8962_gpio_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_codec *codec = rtd->codec;
+	struct imx_priv *priv = &card_priv;
+
+	priv->codec = codec;
+
+	if (gpio_is_valid(priv->hp_gpio)) {
+		imx_hp_jack_gpio.gpio = priv->hp_gpio;
+		imx_hp_jack_gpio.jack_status_check = hpjack_status_check;
+
+		snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, &imx_hp_jack);
+		snd_soc_jack_add_pins(&imx_hp_jack,
+				ARRAY_SIZE(imx_hp_jack_pins), imx_hp_jack_pins);
+		snd_soc_jack_add_gpios(&imx_hp_jack, 1, &imx_hp_jack_gpio);
+	}
+
+	if (gpio_is_valid(priv->mic_gpio)) {
+		imx_mic_jack_gpio.gpio = priv->mic_gpio;
+		imx_mic_jack_gpio.jack_status_check = micjack_status_check;
+
+		snd_soc_jack_new(codec, "AMIC", SND_JACK_MICROPHONE, &imx_mic_jack);
+		snd_soc_jack_add_pins(&imx_mic_jack,
+				ARRAY_SIZE(imx_mic_jack_pins), imx_mic_jack_pins);
+		snd_soc_jack_add_gpios(&imx_mic_jack, 1, &imx_mic_jack_gpio);
+	} else if (priv->amic_mono || priv->dmic_mono) {
+		/* Permanently set monomix bit if only one microphone is
+		 * present on the board while it needs monomix.
+		 */
+		snd_soc_update_bits(priv->codec, WM8962_THREED1,
+				WM8962_ADC_MONOMIX_MASK, WM8962_ADC_MONOMIX);
+	}
+
+	return 0;
+}
+
+static ssize_t show_headphone(struct device_driver *dev, char *buf)
+{
+	struct imx_priv *priv = &card_priv;
+	int hp_status;
+
+	if (!gpio_is_valid(priv->hp_gpio)) {
+		strcpy(buf, "no detect gpio connected\n");
+		return strlen(buf);
+	}
+
+	/* Check if headphone is plugged in */
+	hp_status = gpio_get_value(priv->hp_gpio) != 0;
+
+	if (hp_status != priv->hp_active_low)
+		strcpy(buf, "headphone\n");
+	else
+		strcpy(buf, "speaker\n");
+
+	return strlen(buf);
+}
+
+static DRIVER_ATTR(headphone, S_IRUGO | S_IWUSR, show_headphone, NULL);
+
+static ssize_t show_mic(struct device_driver *dev, char *buf)
+{
+	struct imx_priv *priv = &card_priv;
+	int mic_status;
+
+	if (!gpio_is_valid(priv->mic_gpio)) {
+		strcpy(buf, "no detect gpio connected\n");
+		return strlen(buf);
+	}
+
+	/* Check if analog microphone is plugged in */
+	mic_status = gpio_get_value(priv->mic_gpio) != 0;
+
+	if (mic_status != priv->mic_active_low)
+		strcpy(buf, "amic\n");
+	else
+		strcpy(buf, "dmic\n");
+
+	return strlen(buf);
+}
+
+static DRIVER_ATTR(microphone, S_IRUGO | S_IWUSR, show_mic, NULL);
+
 static int imx_wm8962_late_probe(struct snd_soc_card *card)
 {
 	struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai;
@@ -275,6 +463,14 @@ static int imx_wm8962_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
+	priv->amic_mono = of_property_read_bool(codec_np, "amic-mono");
+	priv->dmic_mono = of_property_read_bool(codec_np, "dmic-mono");
+
+	priv->hp_gpio = of_get_named_gpio_flags(np, "hp-det-gpios", 0,
+				(enum of_gpio_flags *)&priv->hp_active_low);
+	priv->mic_gpio = of_get_named_gpio_flags(np, "mic-det-gpios", 0,
+				(enum of_gpio_flags *)&priv->mic_active_low);
+
 	data->dai.name = "HiFi";
 	data->dai.stream_name = "HiFi";
 	data->dai.codec_dai_name = "wm8962";
@@ -282,6 +478,7 @@ static int imx_wm8962_probe(struct platform_device *pdev)
 	data->dai.cpu_dai_name = dev_name(&ssi_pdev->dev);
 	data->dai.platform_of_node = ssi_np;
 	data->dai.ops = &imx_hifi_ops;
+	data->dai.init = &imx_wm8962_gpio_init;
 	data->dai.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			    SND_SOC_DAIFMT_CBM_CFM;
 
@@ -307,11 +504,33 @@ static int imx_wm8962_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, data);
+
+	if (gpio_is_valid(priv->hp_gpio)) {
+		ret = driver_create_file(pdev->dev.driver, &driver_attr_headphone);
+		if (ret) {
+			dev_err(&pdev->dev, "create hp attr failed (%d)\n", ret);
+			goto fail_hp;
+		}
+	}
+
+	if (gpio_is_valid(priv->mic_gpio)) {
+		ret = driver_create_file(pdev->dev.driver, &driver_attr_microphone);
+		if (ret) {
+			dev_err(&pdev->dev, "create mic attr failed (%d)\n", ret);
+			goto fail_mic;
+		}
+	}
+
 	of_node_put(ssi_np);
 	of_node_put(codec_np);
 
 	return 0;
 
+fail_mic:
+	if (gpio_is_valid(priv->hp_gpio))
+		driver_remove_file(pdev->dev.driver, &driver_attr_headphone);
+fail_hp:
+	snd_soc_unregister_card(&data->card);
 clk_fail:
 	clk_disable_unprepare(data->codec_clk);
 fail:
@@ -326,6 +545,12 @@ fail:
 static int imx_wm8962_remove(struct platform_device *pdev)
 {
 	struct imx_wm8962_data *data = platform_get_drvdata(pdev);
+	struct imx_priv *priv = &card_priv;
+
+	if (gpio_is_valid(priv->mic_gpio))
+		driver_remove_file(pdev->dev.driver, &driver_attr_microphone);
+	if (gpio_is_valid(priv->hp_gpio))
+		driver_remove_file(pdev->dev.driver, &driver_attr_headphone);
 
 	if (!IS_ERR(data->codec_clk))
 		clk_disable_unprepare(data->codec_clk);
-- 
1.8.4


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ASoC: fsl: imx-wm8962: Add headphone and microphone jack plugin/out detection

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1139 bytes --]

On Fri, Dec 06, 2013 at 05:48:23PM +0800, Nicolin Chen wrote:

> Therefore, this patch mainly adds jack event detection feature and switchs I/O
> routes accordingly. Meanwhile, for those monaural input cases, we also turn on
> the ADC monomix features of WM8962 if needed.

No, the kernel shouldn't be doing any route switching or other
configuration - that should be done by userspace.  This is a policy
decision, there are situations where for example audio should go to both
headphones and speaker when headphones are connected in order to make
sure notifications are audible even if headpones have been removed.

> +	if (hp_status != priv->hp_active_low) {
> +		snprintf(buf, 32, "STATE=%d", 2);
> +		snd_soc_dapm_disable_pin(&priv->codec->dapm, "Ext Spk");
> +		ret = imx_hp_jack_gpio.report;
> +	} else {
> +		snprintf(buf, 32, "STATE=%d", 0);
> +		snd_soc_dapm_enable_pin(&priv->codec->dapm, "Ext Spk");
> +		ret = 0;
> +	}
> +
> +	envp[0] = "NAME=headphone";
> +	envp[1] = buf;
> +	envp[2] = NULL;
> +	kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, envp);

What's this uevent trying to do and why is it open coded in the driver?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 3/3] ASoC: fsl: imx-wm8962: Add headphone and microphone jack plugin/out detection

[Nicolin Chen]

On Fri, Dec 06, 2013 at 02:42:56PM +0000, Mark Brown wrote:
> On Fri, Dec 06, 2013 at 05:48:23PM +0800, Nicolin Chen wrote:
> 
> > Therefore, this patch mainly adds jack event detection feature and switchs I/O
> > routes accordingly. Meanwhile, for those monaural input cases, we also turn on
> > the ADC monomix features of WM8962 if needed.
> 
> No, the kernel shouldn't be doing any route switching or other
> configuration - that should be done by userspace.  This is a policy
> decision, there are situations where for example audio should go to both
> headphones and speaker when headphones are connected in order to make
> sure notifications are audible even if headpones have been removed.

I see...It seems my solution is too specific. I will re-think about it.

> > +	if (hp_status != priv->hp_active_low) {
> > +		snprintf(buf, 32, "STATE=%d", 2);
> > +		snd_soc_dapm_disable_pin(&priv->codec->dapm, "Ext Spk");
> > +		ret = imx_hp_jack_gpio.report;
> > +	} else {
> > +		snprintf(buf, 32, "STATE=%d", 0);
> > +		snd_soc_dapm_enable_pin(&priv->codec->dapm, "Ext Spk");
> > +		ret = 0;
> > +	}
> > +
> > +	envp[0] = "NAME=headphone";
> > +	envp[1] = buf;
> > +	envp[2] = NULL;
> > +	kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, envp);
> 
> What's this uevent trying to do and why is it open coded in the driver?

That's a requirement from Android team of Freescale. By checking this
event they can determine whether current output is speaker or headphone
so as to place an icon on the notification area.

Sir, if you think this patch is not common enough or non-community style,
I can try to cut it down to an common one or entirely drop it.

Looking forward to your reply,
Nicolin Chen

Re: [PATCH 3/3] ASoC: fsl: imx-wm8962: Add headphone and microphone jack plugin/out detection

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1066 bytes --]

On Fri, Dec 06, 2013 at 10:38:17PM +0800, Nicolin Chen wrote:
> On Fri, Dec 06, 2013 at 02:42:56PM +0000, Mark Brown wrote:

> > What's this uevent trying to do and why is it open coded in the driver?

> That's a requirement from Android team of Freescale. By checking this
> event they can determine whether current output is speaker or headphone
> so as to place an icon on the notification area.

> Sir, if you think this patch is not common enough or non-community style,
> I can try to cut it down to an common one or entirely drop it.

This isn't mainline stuff, this is the Android standard switch class.
Someone should really contribute something to Android which backs it
onto the back of ALSA jacks.  The mainline equivalent is extcon which
does have switch class emulation but it's a bit broken in its generation
of uevents and is also not yet integrated into the ALSA jack stuff.

The thing to do is ideally integrate extcon with the ALSA jack code and
update the Android headset observer to work with extcon (or write a
separate extcon based observer).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 3/3] ASoC: fsl: imx-wm8962: Add headphone and microphone jack plugin/out detection

[Nicolin Chen]

On Fri, Dec 06, 2013 at 03:03:19PM +0000, Mark Brown wrote:
> On Fri, Dec 06, 2013 at 10:38:17PM +0800, Nicolin Chen wrote:
> > On Fri, Dec 06, 2013 at 02:42:56PM +0000, Mark Brown wrote:
> 
> > > What's this uevent trying to do and why is it open coded in the driver?
> 
> > That's a requirement from Android team of Freescale. By checking this
> > event they can determine whether current output is speaker or headphone
> > so as to place an icon on the notification area.
> 
> > Sir, if you think this patch is not common enough or non-community style,
> > I can try to cut it down to an common one or entirely drop it.
> 
> This isn't mainline stuff, this is the Android standard switch class.
> Someone should really contribute something to Android which backs it
> onto the back of ALSA jacks.  The mainline equivalent is extcon which
> does have switch class emulation but it's a bit broken in its generation
> of uevents and is also not yet integrated into the ALSA jack stuff.
> 
> The thing to do is ideally integrate extcon with the ALSA jack code and
> update the Android headset observer to work with extcon (or write a
> separate extcon based observer).

Thank you for the detailed explain. It looks more complicated to me with
current code base. I think I should just extract the mainline-style part
so it'll make things easy if there's gonna be some plausible integrations
afterward.

Thank you indeed,
Nicolin Chen