All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 1/9] Clean WM8971 through checkpatch
@ 2014-09-02  3:27 Xavier Hsu
  2014-09-02  3:27 ` [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE Xavier Hsu
                   ` (7 more replies)
  0 siblings, 8 replies; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch improves WM8971.
We clean the file through checkpatch.

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |  107 ++++++++++++++++++++++++---------------------
 1 file changed, 56 insertions(+), 51 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 0499cd4..064278f 100644
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -31,7 +31,7 @@
 
 #define	WM8971_REG_COUNT		43
 
-static struct workqueue_struct *wm8971_workq = NULL;
+static struct workqueue_struct *wm8971_workq;
 
 /* codec private data */
 struct wm8971_priv {
@@ -92,25 +92,28 @@ static const struct reg_default wm8971_reg_defaults[] = {
 #define wm8971_reset(c)	snd_soc_write(c, WM8971_RESET, 0)
 
 /* WM8971 Controls */
-static const char *wm8971_bass[] = { "Linear Control", "Adaptive Boost" };
-static const char *wm8971_bass_filter[] = { "130Hz @ 48kHz",
-	"200Hz @ 48kHz" };
-static const char *wm8971_treble[] = { "8kHz", "4kHz" };
-static const char *wm8971_alc_func[] = { "Off", "Right", "Left", "Stereo" };
-static const char *wm8971_ng_type[] = { "Constant PGA Gain",
-	"Mute ADC Output" };
-static const char *wm8971_deemp[] = { "None", "32kHz", "44.1kHz", "48kHz" };
-static const char *wm8971_mono_mux[] = {"Stereo", "Mono (Left)",
-	"Mono (Right)", "Digital Mono"};
-static const char *wm8971_dac_phase[] = { "Non Inverted", "Inverted" };
-static const char *wm8971_lline_mux[] = {"Line", "NC", "NC", "PGA",
-	"Differential"};
-static const char *wm8971_rline_mux[] = {"Line", "Mic", "NC", "PGA",
-	"Differential"};
-static const char *wm8971_lpga_sel[] = {"Line", "NC", "NC", "Differential"};
-static const char *wm8971_rpga_sel[] = {"Line", "Mic", "NC", "Differential"};
-static const char *wm8971_adcpol[] = {"Normal", "L Invert", "R Invert",
-	"L + R Invert"};
+static const char const *wm8971_bass[] = {"Linear Control", "Adaptive Boost"};
+static const char const *wm8971_bass_filter[] = {"130Hz @ 48kHz",
+						 "200Hz @ 48kHz"};
+static const char const *wm8971_treble[] = {"8kHz", "4kHz"};
+static const char const *wm8971_alc_func[] = {"Off", "Right",
+					      "Left", "Stereo"};
+static const char const *wm8971_ng_type[] = {"Constant PGA Gain",
+					     "Mute ADC Output"};
+static const char const *wm8971_deemp[] = {"None", "32kHz", "44.1kHz", "48kHz"};
+static const char const *wm8971_mono_mux[] = {"Stereo", "Mono (Left)",
+					      "Mono (Right)", "Digital Mono"};
+static const char const *wm8971_dac_phase[] = {"Non Inverted", "Inverted"};
+static const char const *wm8971_lline_mux[] = {"Line", "NC", "NC",
+					       "PGA", "Differential"};
+static const char const *wm8971_rline_mux[] = {"Line", "Mic", "NC",
+					       "PGA", "Differential"};
+static const char const *wm8971_lpga_sel[] = {"Line", "NC", "NC",
+					      "Differential"};
+static const char const *wm8971_rpga_sel[] = {"Line", "Mic", "NC",
+					      "Differential"};
+static const char const *wm8971_adcpol[] = {"Normal", "L Invert",
+					    "R Invert", "L + R Invert"};
 
 static const struct soc_enum wm8971_enum[] = {
 	SOC_ENUM_SINGLE(WM8971_BASS, 7, 2, wm8971_bass),	/* 0 */
@@ -136,24 +139,24 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 	SOC_DOUBLE_R("Capture Switch", WM8971_LINVOL, WM8971_RINVOL, 7, 1, 1),
 
 	SOC_DOUBLE_R("Headphone Playback ZC Switch", WM8971_LOUT1V,
-		WM8971_ROUT1V, 7, 1, 0),
+		     WM8971_ROUT1V, 7, 1, 0),
 	SOC_DOUBLE_R("Speaker Playback ZC Switch", WM8971_LOUT2V,
-		WM8971_ROUT2V, 7, 1, 0),
+		     WM8971_ROUT2V, 7, 1, 0),
 	SOC_SINGLE("Mono Playback ZC Switch", WM8971_MOUTV, 7, 1, 0),
 
 	SOC_DOUBLE_R("PCM Volume", WM8971_LDAC, WM8971_RDAC, 0, 255, 0),
 
 	SOC_DOUBLE_R("Bypass Left Playback Volume", WM8971_LOUTM1,
-		WM8971_LOUTM2, 4, 7, 1),
+		     WM8971_LOUTM2, 4, 7, 1),
 	SOC_DOUBLE_R("Bypass Right Playback Volume", WM8971_ROUTM1,
-		WM8971_ROUTM2, 4, 7, 1),
+		     WM8971_ROUTM2, 4, 7, 1),
 	SOC_DOUBLE_R("Bypass Mono Playback Volume", WM8971_MOUTM1,
-		WM8971_MOUTM2, 4, 7, 1),
+		     WM8971_MOUTM2, 4, 7, 1),
 
 	SOC_DOUBLE_R("Headphone Playback Volume", WM8971_LOUT1V,
-		WM8971_ROUT1V, 0, 127, 0),
+		     WM8971_ROUT1V, 0, 127, 0),
 	SOC_DOUBLE_R("Speaker Playback Volume", WM8971_LOUT2V,
-		WM8971_ROUT2V, 0, 127, 0),
+		     WM8971_ROUT2V, 0, 127, 0),
 
 	SOC_ENUM("Bass Boost", wm8971_enum[0]),
 	SOC_ENUM("Bass Filter", wm8971_enum[1]),
@@ -238,14 +241,14 @@ SOC_DAPM_ENUM("Route", wm8971_enum[13]);
 
 static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
 	SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
-		&wm8971_left_mixer_controls[0],
-		ARRAY_SIZE(wm8971_left_mixer_controls)),
+			   &wm8971_left_mixer_controls[0],
+			   ARRAY_SIZE(wm8971_left_mixer_controls)),
 	SND_SOC_DAPM_MIXER("Right Mixer", SND_SOC_NOPM, 0, 0,
-		&wm8971_right_mixer_controls[0],
-		ARRAY_SIZE(wm8971_right_mixer_controls)),
+			   &wm8971_right_mixer_controls[0],
+			   ARRAY_SIZE(wm8971_right_mixer_controls)),
 	SND_SOC_DAPM_MIXER("Mono Mixer", WM8971_PWR2, 2, 0,
-		&wm8971_mono_mixer_controls[0],
-		ARRAY_SIZE(wm8971_mono_mixer_controls)),
+			   &wm8971_mono_mixer_controls[0],
+			   ARRAY_SIZE(wm8971_mono_mixer_controls)),
 
 	SND_SOC_DAPM_PGA("Right Out 2", WM8971_PWR2, 3, 0, NULL, 0),
 	SND_SOC_DAPM_PGA("Left Out 2", WM8971_PWR2, 4, 0, NULL, 0),
@@ -260,18 +263,18 @@ static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
 	SND_SOC_DAPM_ADC("Left ADC", "Left Capture", WM8971_PWR1, 3, 0),
 
 	SND_SOC_DAPM_MUX("Left PGA Mux", WM8971_PWR1, 5, 0,
-		&wm8971_left_pga_controls),
+			 &wm8971_left_pga_controls),
 	SND_SOC_DAPM_MUX("Right PGA Mux", WM8971_PWR1, 4, 0,
-		&wm8971_right_pga_controls),
+			 &wm8971_right_pga_controls),
 	SND_SOC_DAPM_MUX("Left Line Mux", SND_SOC_NOPM, 0, 0,
-		&wm8971_left_line_controls),
+			 &wm8971_left_line_controls),
 	SND_SOC_DAPM_MUX("Right Line Mux", SND_SOC_NOPM, 0, 0,
-		&wm8971_right_line_controls),
+			 &wm8971_right_line_controls),
 
 	SND_SOC_DAPM_MUX("Left ADC Mux", SND_SOC_NOPM, 0, 0,
-		&wm8971_monomux_controls),
+			 &wm8971_monomux_controls),
 	SND_SOC_DAPM_MUX("Right ADC Mux", SND_SOC_NOPM, 0, 0,
-		&wm8971_monomux_controls),
+			 &wm8971_monomux_controls),
 
 	SND_SOC_DAPM_OUTPUT("LOUT1"),
 	SND_SOC_DAPM_OUTPUT("ROUT1"),
@@ -431,7 +434,7 @@ static int get_coeff(int mclk, int rate)
 }
 
 static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
-		int clk_id, unsigned int freq, int dir)
+				 int clk_id, unsigned int freq, int dir)
 {
 	struct snd_soc_codec *codec = codec_dai->codec;
 	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
@@ -449,7 +452,7 @@ static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 }
 
 static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
-		unsigned int fmt)
+			      unsigned int fmt)
 {
 	struct snd_soc_codec *codec = codec_dai->codec;
 	u16 iface = 0;
@@ -507,8 +510,8 @@ static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
 }
 
 static int wm8971_pcm_hw_params(struct snd_pcm_substream *substream,
-	struct snd_pcm_hw_params *params,
-	struct snd_soc_dai *dai)
+				struct snd_pcm_hw_params *params,
+				struct snd_soc_dai *dai)
 {
 	struct snd_soc_codec *codec = dai->codec;
 	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
@@ -553,7 +556,7 @@ static int wm8971_mute(struct snd_soc_dai *dai, int mute)
 }
 
 static int wm8971_set_bias_level(struct snd_soc_codec *codec,
-	enum snd_soc_bias_level level)
+				 enum snd_soc_bias_level level)
 {
 	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
 
@@ -580,11 +583,12 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
 }
 
 #define WM8971_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 |\
-		SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_44100 | \
-		SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)
+		      SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 |\
+		      SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |\
+		      SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)
 
 #define WM8971_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
-	SNDRV_PCM_FMTBIT_S24_LE)
+			SNDRV_PCM_FMTBIT_S24_LE)
 
 static const struct snd_soc_dai_ops wm8971_dai_ops = {
 	.hw_params	= wm8971_pcm_hw_params,
@@ -616,6 +620,7 @@ static void wm8971_work(struct work_struct *work)
 		container_of(work, struct snd_soc_dapm_context,
 			     delayed_work.work);
 	struct snd_soc_codec *codec = dapm->codec;
+
 	wm8971_set_bias_level(codec, codec->dapm.bias_level);
 }
 
@@ -637,7 +642,7 @@ static int wm8971_resume(struct snd_soc_codec *codec)
 		snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
 		codec->dapm.bias_level = SND_SOC_BIAS_ON;
 		queue_delayed_work(wm8971_workq, &codec->dapm.delayed_work,
-			msecs_to_jiffies(1000));
+				   msecs_to_jiffies(1000));
 	}
 
 	return 0;
@@ -660,7 +665,7 @@ static int wm8971_probe(struct snd_soc_codec *codec)
 	snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
 	codec->dapm.bias_level = SND_SOC_BIAS_STANDBY;
 	queue_delayed_work(wm8971_workq, &codec->dapm.delayed_work,
-		msecs_to_jiffies(1000));
+			   msecs_to_jiffies(1000));
 
 	/* set the update bits */
 	snd_soc_update_bits(codec, WM8971_LDAC, 0x0100, 0x0100);
@@ -729,8 +734,8 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, wm8971);
 
-	ret = snd_soc_register_codec(&i2c->dev,
-			&soc_codec_dev_wm8971, &wm8971_dai, 1);
+	ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_wm8971,
+				     &wm8971_dai, 1);
 
 	return ret;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE
  2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
@ 2014-09-02  3:27 ` Xavier Hsu
  2014-09-02  9:33   ` Charles Keepax
  2014-09-02 14:56   ` Lars-Peter Clausen
  2014-09-02  3:27 ` [PATCHv3 3/9] WM8971 uses TLV information Xavier Hsu
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch improves WM8971.
We uses SOC_ENUM_SINGLE_DECL macro to
replace SOC_ENUM_SINGLE macro.

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |  136 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 106 insertions(+), 30 deletions(-)
 mode change 100644 => 100755 sound/soc/codecs/wm8971.c

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
old mode 100644
new mode 100755
index 064278f..59c4828
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -7,6 +7,10 @@
  *
  * Based on wm8753.c by Liam Girdwood
  *
+ * WM8971 Improve Copyright (C) 2014 Linaro, Ltd
+ * Author: Xavier Hsu <xavier.hsu@linaro.org>
+ *         Andy Green <andy.green@linaro.org>
+ *
  *  This program is free software; you can redistribute  it and/or modify it
  *  under  the terms of  the GNU General  Public License as published by the
  *  Free Software Foundation;  either version 2 of the  License, or (at your
@@ -36,6 +40,8 @@ static struct workqueue_struct *wm8971_workq;
 /* codec private data */
 struct wm8971_priv {
 	unsigned int sysclk;
+	int playback_fs;
+	bool deemph;
 };
 
 /*
@@ -100,7 +106,6 @@ static const char const *wm8971_alc_func[] = {"Off", "Right",
 					      "Left", "Stereo"};
 static const char const *wm8971_ng_type[] = {"Constant PGA Gain",
 					     "Mute ADC Output"};
-static const char const *wm8971_deemp[] = {"None", "32kHz", "44.1kHz", "48kHz"};
 static const char const *wm8971_mono_mux[] = {"Stereo", "Mono (Left)",
 					      "Mono (Right)", "Digital Mono"};
 static const char const *wm8971_dac_phase[] = {"Non Inverted", "Inverted"};
@@ -114,23 +119,91 @@ static const char const *wm8971_rpga_sel[] = {"Line", "Mic", "NC",
 					      "Differential"};
 static const char const *wm8971_adcpol[] = {"Normal", "L Invert",
 					    "R Invert", "L + R Invert"};
+static const char const *wm8971_deemp[] = {"None", "32kHz",
+					   "44.1kHz", "48kHz"};
 
-static const struct soc_enum wm8971_enum[] = {
-	SOC_ENUM_SINGLE(WM8971_BASS, 7, 2, wm8971_bass),	/* 0 */
-	SOC_ENUM_SINGLE(WM8971_BASS, 6, 2, wm8971_bass_filter),
-	SOC_ENUM_SINGLE(WM8971_TREBLE, 6, 2, wm8971_treble),
-	SOC_ENUM_SINGLE(WM8971_ALC1, 7, 4, wm8971_alc_func),
-	SOC_ENUM_SINGLE(WM8971_NGATE, 1, 2, wm8971_ng_type),    /* 4 */
-	SOC_ENUM_SINGLE(WM8971_ADCDAC, 1, 4, wm8971_deemp),
-	SOC_ENUM_SINGLE(WM8971_ADCTL1, 4, 4, wm8971_mono_mux),
-	SOC_ENUM_SINGLE(WM8971_ADCTL1, 1, 2, wm8971_dac_phase),
-	SOC_ENUM_SINGLE(WM8971_LOUTM1, 0, 5, wm8971_lline_mux), /* 8 */
-	SOC_ENUM_SINGLE(WM8971_ROUTM1, 0, 5, wm8971_rline_mux),
-	SOC_ENUM_SINGLE(WM8971_LADCIN, 6, 4, wm8971_lpga_sel),
-	SOC_ENUM_SINGLE(WM8971_RADCIN, 6, 4, wm8971_rpga_sel),
-	SOC_ENUM_SINGLE(WM8971_ADCDAC, 5, 4, wm8971_adcpol),    /* 12 */
-	SOC_ENUM_SINGLE(WM8971_ADCIN, 6, 4, wm8971_mono_mux),
-};
+static int wm8971_set_deemph(struct snd_soc_codec *codec)
+{
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
+	int val = 0, i, best = 0;
+
+	/* If we're using deemphasis select the nearest available sample
+	 * rate.
+	*/
+	if (wm8971->deemph) {
+		best = 1;
+		for (i = 2; i < ARRAY_SIZE(wm8971_deemp); i++) {
+			if (abs(wm8971_deemp[i] - wm8971->playback_fs) <
+				abs(wm8971_deemp[best] -
+					wm8971->playback_fs))
+				best = i;
+		}
+		val = best << 1;
+	}
+
+	dev_dbg(codec->dev, "Set deemphasis %d (%dHz)\n",
+		best, wm8971_deemp[best]);
+
+	return snd_soc_update_bits(codec, WM8971_ADCDAC, 0x6, val);
+}
+
+static int wm8971_get_deemph(struct snd_kcontrol *kcontrol,
+			     struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
+
+	ucontrol->value.enumerated.item[0] = wm8971->deemph;
+
+	return 0;
+}
+
+static int wm8971_put_deemph(struct snd_kcontrol *kcontrol,
+			     struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
+	int deemph = ucontrol->value.enumerated.item[0];
+	int ret = 0;
+
+	if (deemph > 1)
+		return -EINVAL;
+
+	mutex_lock(&codec->mutex);
+	if (wm8971->deemph != deemph) {
+		wm8971->deemph = deemph;
+		wm8971_set_deemph(codec);
+
+		ret = 1;
+	}
+	mutex_unlock(&codec->mutex);
+
+	return ret;
+}
+
+static const SOC_ENUM_SINGLE_DECL(bass_boost, WM8971_BASS, 7, wm8971_bass);
+static const SOC_ENUM_SINGLE_DECL(bass_filter, WM8971_BASS,
+				  6, wm8971_bass_filter);
+static const SOC_ENUM_SINGLE_DECL(treble_cutoff, WM8971_TREBLE,
+				  6, wm8971_treble);
+static const SOC_ENUM_SINGLE_DECL(alc_capture_func, WM8971_ALC1,
+				  7, wm8971_alc_func);
+static const SOC_ENUM_SINGLE_DECL(alc_capture_ngtype, WM8971_NGATE,
+				  1, wm8971_ng_type);
+static const SOC_ENUM_SINGLE_DECL(dac_mono_mix, WM8971_ADCTL1,
+				  4, wm8971_mono_mux);
+static const SOC_ENUM_SINGLE_DECL(dac_phase_inv, WM8971_ADCTL1,
+				  1, wm8971_dac_phase);
+static const SOC_ENUM_SINGLE_DECL(left_line, WM8971_LOUTM1,
+				  0, wm8971_lline_mux);
+static const SOC_ENUM_SINGLE_DECL(right_line, WM8971_ROUTM1,
+				  0, wm8971_rline_mux);
+static const SOC_ENUM_SINGLE_DECL(left_pga, WM8971_LADCIN, 6, wm8971_lpga_sel);
+static const SOC_ENUM_SINGLE_DECL(right_pga, WM8971_RADCIN,
+				  6, wm8971_rpga_sel);
+static const SOC_ENUM_SINGLE_DECL(capture_polarity, WM8971_ADCDAC,
+				  5, wm8971_adcpol);
+static const SOC_ENUM_SINGLE_DECL(monomux, WM8971_ADCIN, 6, wm8971_mono_mux);
 
 static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 	SOC_DOUBLE_R("Capture Volume", WM8971_LINVOL, WM8971_RINVOL, 0, 63, 0),
@@ -158,12 +231,12 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 	SOC_DOUBLE_R("Speaker Playback Volume", WM8971_LOUT2V,
 		     WM8971_ROUT2V, 0, 127, 0),
 
-	SOC_ENUM("Bass Boost", wm8971_enum[0]),
-	SOC_ENUM("Bass Filter", wm8971_enum[1]),
+	SOC_ENUM("Bass Boost", bass_boost),
+	SOC_ENUM("Bass Filter", bass_filter),
 	SOC_SINGLE("Bass Volume", WM8971_BASS, 0, 7, 1),
 
 	SOC_SINGLE("Treble Volume", WM8971_TREBLE, 0, 7, 0),
-	SOC_ENUM("Treble Cut-off", wm8971_enum[2]),
+	SOC_ENUM("Treble Cut-off", treble_cutoff),
 
 	SOC_SINGLE("Capture Filter Switch", WM8971_ADCDAC, 0, 1, 1),
 
@@ -172,21 +245,22 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 
 	SOC_SINGLE("ALC Capture Target Volume", WM8971_ALC1, 0, 7, 0),
 	SOC_SINGLE("ALC Capture Max Volume", WM8971_ALC1, 4, 7, 0),
-	SOC_ENUM("ALC Capture Function", wm8971_enum[3]),
+	SOC_ENUM("ALC Capture Function", alc_capture_func),
 	SOC_SINGLE("ALC Capture ZC Switch", WM8971_ALC2, 7, 1, 0),
 	SOC_SINGLE("ALC Capture Hold Time", WM8971_ALC2, 0, 15, 0),
 	SOC_SINGLE("ALC Capture Decay Time", WM8971_ALC3, 4, 15, 0),
 	SOC_SINGLE("ALC Capture Attack Time", WM8971_ALC3, 0, 15, 0),
 	SOC_SINGLE("ALC Capture NG Threshold", WM8971_NGATE, 3, 31, 0),
-	SOC_ENUM("ALC Capture NG Type", wm8971_enum[4]),
+	SOC_ENUM("ALC Capture NG Type", alc_capture_ngtype),
 	SOC_SINGLE("ALC Capture NG Switch", WM8971_NGATE, 0, 1, 0),
 
 	SOC_SINGLE("Capture 6dB Attenuate", WM8971_ADCDAC, 8, 1, 0),
 	SOC_SINGLE("Playback 6dB Attenuate", WM8971_ADCDAC, 7, 1, 0),
 
-	SOC_ENUM("Playback De-emphasis", wm8971_enum[5]),
-	SOC_ENUM("Playback Function", wm8971_enum[6]),
-	SOC_ENUM("Playback Phase", wm8971_enum[7]),
+	SOC_SINGLE_BOOL_EXT("Playback De-emphasis Switch", 0,
+			    wm8971_get_deemph, wm8971_put_deemph),
+	SOC_ENUM("Playback Function", dac_mono_mix),
+	SOC_ENUM("Playback Phase", dac_phase_inv),
 
 	SOC_DOUBLE_R("Mic Boost", WM8971_LADCIN, WM8971_RADCIN, 4, 3, 0),
 };
@@ -221,23 +295,23 @@ SOC_DAPM_SINGLE("Right Bypass Switch", WM8971_MOUTM2, 7, 1, 0),
 
 /* Left Line Mux */
 static const struct snd_kcontrol_new wm8971_left_line_controls =
-SOC_DAPM_ENUM("Route", wm8971_enum[8]);
+SOC_DAPM_ENUM("Route", left_line);
 
 /* Right Line Mux */
 static const struct snd_kcontrol_new wm8971_right_line_controls =
-SOC_DAPM_ENUM("Route", wm8971_enum[9]);
+SOC_DAPM_ENUM("Route", right_line);
 
 /* Left PGA Mux */
 static const struct snd_kcontrol_new wm8971_left_pga_controls =
-SOC_DAPM_ENUM("Route", wm8971_enum[10]);
+SOC_DAPM_ENUM("Route", left_pga);
 
 /* Right PGA Mux */
 static const struct snd_kcontrol_new wm8971_right_pga_controls =
-SOC_DAPM_ENUM("Route", wm8971_enum[11]);
+SOC_DAPM_ENUM("Route", right_pga);
 
 /* Mono ADC Mux */
 static const struct snd_kcontrol_new wm8971_monomux_controls =
-SOC_DAPM_ENUM("Route", wm8971_enum[13]);
+SOC_DAPM_ENUM("Route", monomux);
 
 static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
 	SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
@@ -534,6 +608,8 @@ static int wm8971_pcm_hw_params(struct snd_pcm_substream *substream,
 		break;
 	}
 
+	wm8971_set_deemph(codec);
+
 	/* set iface & srate */
 	snd_soc_write(codec, WM8971_IFACE, iface);
 	if (coeff >= 0)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 3/9] WM8971 uses TLV information
  2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
  2014-09-02  3:27 ` [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE Xavier Hsu
@ 2014-09-02  3:27 ` Xavier Hsu
  2014-09-02  9:47   ` Charles Keepax
  2014-09-02  3:27 ` [PATCHv3 4/9] Improve wm8971_set_dai_fmt Xavier Hsu
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch improves WM8971.
We use TLV information.

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |   59 ++++++++++++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 59c4828..7c2b175 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -30,10 +30,11 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/initval.h>
+#include <sound/tlv.h>
 
 #include "wm8971.h"
 
-#define	WM8971_REG_COUNT		43
+#define	WM8971_REG_COUNT	43
 
 static struct workqueue_struct *wm8971_workq;
 
@@ -205,8 +206,19 @@ static const SOC_ENUM_SINGLE_DECL(capture_polarity, WM8971_ADCDAC,
 				  5, wm8971_adcpol);
 static const SOC_ENUM_SINGLE_DECL(monomux, WM8971_ADCIN, 6, wm8971_mono_mux);
 
+static const DECLARE_TLV_DB_SCALE(in_vol, -1725, 75, 0);
+static const DECLARE_TLV_DB_SCALE(out_vol, -6700, 91, 0);
+static const DECLARE_TLV_DB_SCALE(attenuate_6db, -600, 600, 0);
+static const DECLARE_TLV_DB_SCALE(dac_vol, -12700, 50, 0);
+static const DECLARE_TLV_DB_SCALE(tone_vol, -600, 150, 0);
+static const DECLARE_TLV_DB_SCALE(alc_tar_vol, -2850, 150, 0);
+static const DECLARE_TLV_DB_SCALE(alc_max_vol, -1200, 600, 0);
+static const DECLARE_TLV_DB_SCALE(adc_vol, -9700, 50, 0);
+static const DECLARE_TLV_DB_SCALE(bypass_out_vol, -1500, 300, 0);
+
 static const struct snd_kcontrol_new wm8971_snd_controls[] = {
-	SOC_DOUBLE_R("Capture Volume", WM8971_LINVOL, WM8971_RINVOL, 0, 63, 0),
+	SOC_DOUBLE_R_TLV("Capture Volume", WM8971_LINVOL, WM8971_RINVOL,
+			 0, 63, 0, in_vol),
 	SOC_DOUBLE_R("Capture ZC Switch", WM8971_LINVOL, WM8971_RINVOL,
 		     6, 1, 0),
 	SOC_DOUBLE_R("Capture Switch", WM8971_LINVOL, WM8971_RINVOL, 7, 1, 1),
@@ -217,31 +229,35 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 		     WM8971_ROUT2V, 7, 1, 0),
 	SOC_SINGLE("Mono Playback ZC Switch", WM8971_MOUTV, 7, 1, 0),
 
-	SOC_DOUBLE_R("PCM Volume", WM8971_LDAC, WM8971_RDAC, 0, 255, 0),
+	SOC_DOUBLE_R_TLV("PCM Volume", WM8971_LDAC, WM8971_RDAC,
+			 0, 255, 0, dac_vol),
 
-	SOC_DOUBLE_R("Bypass Left Playback Volume", WM8971_LOUTM1,
-		     WM8971_LOUTM2, 4, 7, 1),
-	SOC_DOUBLE_R("Bypass Right Playback Volume", WM8971_ROUTM1,
-		     WM8971_ROUTM2, 4, 7, 1),
-	SOC_DOUBLE_R("Bypass Mono Playback Volume", WM8971_MOUTM1,
-		     WM8971_MOUTM2, 4, 7, 1),
+	SOC_DOUBLE_R_TLV("Bypass Left Playback Volume", WM8971_LOUTM1,
+			 WM8971_LOUTM2, 4, 7, 1, bypass_out_vol),
+	SOC_DOUBLE_R_TLV("Bypass Right Playback Volume", WM8971_ROUTM1,
+			 WM8971_ROUTM2, 4, 7, 1, bypass_out_vol),
+	SOC_DOUBLE_R_TLV("Bypass Mono Playback Volume", WM8971_MOUTM1,
+			 WM8971_MOUTM2, 4, 7, 1, bypass_out_vol),
 
-	SOC_DOUBLE_R("Headphone Playback Volume", WM8971_LOUT1V,
-		     WM8971_ROUT1V, 0, 127, 0),
-	SOC_DOUBLE_R("Speaker Playback Volume", WM8971_LOUT2V,
-		     WM8971_ROUT2V, 0, 127, 0),
+	SOC_DOUBLE_R_TLV("Headphone Playback Volume", WM8971_LOUT1V,
+			 WM8971_ROUT1V, 0, 127, 0, out_vol),
+	SOC_DOUBLE_R_TLV("Speaker Playback Volume", WM8971_LOUT2V,
+			 WM8971_ROUT2V, 0, 127, 0, out_vol),
+	SOC_SINGLE_TLV("Mono Playback Volume", WM8971_MOUTV,
+		       0, 127, 0, out_vol),
 
 	SOC_ENUM("Bass Boost", bass_boost),
 	SOC_ENUM("Bass Filter", bass_filter),
-	SOC_SINGLE("Bass Volume", WM8971_BASS, 0, 7, 1),
+	SOC_SINGLE_TLV("Bass Volume", WM8971_BASS, 0, 15, 1, tone_vol),
 
-	SOC_SINGLE("Treble Volume", WM8971_TREBLE, 0, 7, 0),
+	SOC_SINGLE_TLV("Treble Volume", WM8971_TREBLE, 0, 15, 0, tone_vol),
 	SOC_ENUM("Treble Cut-off", treble_cutoff),
 
 	SOC_SINGLE("Capture Filter Switch", WM8971_ADCDAC, 0, 1, 1),
 
-	SOC_SINGLE("ALC Target Volume", WM8971_ALC1, 0, 7, 0),
-	SOC_SINGLE("ALC Max Volume", WM8971_ALC1, 4, 7, 0),
+	SOC_SINGLE_TLV("ALC Target Volume", WM8971_ALC1,
+		       0, 15, 0, alc_tar_vol),
+	SOC_SINGLE_TLV("ALC Max Volume", WM8971_ALC1, 4, 7, 0, alc_max_vol),
 
 	SOC_SINGLE("ALC Capture Target Volume", WM8971_ALC1, 0, 7, 0),
 	SOC_SINGLE("ALC Capture Max Volume", WM8971_ALC1, 4, 7, 0),
@@ -254,8 +270,10 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 	SOC_ENUM("ALC Capture NG Type", alc_capture_ngtype),
 	SOC_SINGLE("ALC Capture NG Switch", WM8971_NGATE, 0, 1, 0),
 
-	SOC_SINGLE("Capture 6dB Attenuate", WM8971_ADCDAC, 8, 1, 0),
-	SOC_SINGLE("Playback 6dB Attenuate", WM8971_ADCDAC, 7, 1, 0),
+	SOC_SINGLE_TLV("Capture 6dB Attenuate", WM8971_ADCDAC,
+		       8, 1, 0, attenuate_6db),
+	SOC_SINGLE_TLV("Playback 6dB Attenuate", WM8971_ADCDAC,
+		       7, 1, 0, attenuate_6db),
 
 	SOC_SINGLE_BOOL_EXT("Playback De-emphasis Switch", 0,
 			    wm8971_get_deemph, wm8971_put_deemph),
@@ -263,6 +281,9 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 	SOC_ENUM("Playback Phase", dac_phase_inv),
 
 	SOC_DOUBLE_R("Mic Boost", WM8971_LADCIN, WM8971_RADCIN, 4, 3, 0),
+
+	SOC_DOUBLE_R_TLV("ADC Volume", WM8971_LADC, WM8971_RADC,
+			 0, 255, 0, adc_vol),
 };
 
 /*
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 4/9] Improve wm8971_set_dai_fmt
  2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
  2014-09-02  3:27 ` [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE Xavier Hsu
  2014-09-02  3:27 ` [PATCHv3 3/9] WM8971 uses TLV information Xavier Hsu
@ 2014-09-02  3:27 ` Xavier Hsu
  2014-09-02  9:48   ` Charles Keepax
  2014-09-02  3:27 ` [PATCHv3 5/9] Using the constraint based on wm8971_set_dai_sysclk Xavier Hsu
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch improves WM8971.
We modify the function of wm8971_set_dai_fmt().

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |   18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 7c2b175..64ed226 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -550,12 +550,11 @@ static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
 			      unsigned int fmt)
 {
 	struct snd_soc_codec *codec = codec_dai->codec;
-	u16 iface = 0;
 
 	/* set master/slave audio interface */
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBM_CFM:
-		iface = 0x0040;
+		snd_soc_update_bits(codec, WM8971_IFACE, 0x0040, 0x0040);
 		break;
 	case SND_SOC_DAIFMT_CBS_CFS:
 		break;
@@ -566,18 +565,18 @@ static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	/* interface format */
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
-		iface |= 0x0002;
+		snd_soc_update_bits(codec, WM8971_IFACE, 0x0002, 0x0002);
 		break;
 	case SND_SOC_DAIFMT_RIGHT_J:
 		break;
 	case SND_SOC_DAIFMT_LEFT_J:
-		iface |= 0x0001;
+		snd_soc_update_bits(codec, WM8971_IFACE, 0x0001, 0x0001);
 		break;
 	case SND_SOC_DAIFMT_DSP_A:
-		iface |= 0x0003;
+		snd_soc_update_bits(codec, WM8971_IFACE, 0x0003, 0x0003);
 		break;
 	case SND_SOC_DAIFMT_DSP_B:
-		iface |= 0x0013;
+		snd_soc_update_bits(codec, WM8971_IFACE, 0x0013, 0x0013);
 		break;
 	default:
 		return -EINVAL;
@@ -588,19 +587,18 @@ static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	case SND_SOC_DAIFMT_NB_NF:
 		break;
 	case SND_SOC_DAIFMT_IB_IF:
-		iface |= 0x0090;
+		snd_soc_update_bits(codec, WM8971_IFACE, 0x0090, 0x0090);
 		break;
 	case SND_SOC_DAIFMT_IB_NF:
-		iface |= 0x0080;
+		snd_soc_update_bits(codec, WM8971_IFACE, 0x0080, 0x0080);
 		break;
 	case SND_SOC_DAIFMT_NB_IF:
-		iface |= 0x0010;
+		snd_soc_update_bits(codec, WM8971_IFACE, 0x0010, 0x0010);
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	snd_soc_write(codec, WM8971_IFACE, iface);
 	return 0;
 }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 5/9] Using the constraint based on wm8971_set_dai_sysclk
  2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
                   ` (2 preceding siblings ...)
  2014-09-02  3:27 ` [PATCHv3 4/9] Improve wm8971_set_dai_fmt Xavier Hsu
@ 2014-09-02  3:27 ` Xavier Hsu
  2014-09-02  9:28   ` Charles Keepax
  2014-09-02  3:27 ` [PATCHv3 6/9] WM8971 uses msleep to replace work queue Xavier Hsu
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch improves WM8971.
We use the constraint based on the function of
wm8971_set_dai_sysclk().

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |   77 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 71 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 64ed226..20cfdd3 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -41,6 +41,7 @@ static struct workqueue_struct *wm8971_workq;
 /* codec private data */
 struct wm8971_priv {
 	unsigned int sysclk;
+	struct snd_pcm_hw_constraint_list *sysclk_constraints;
 	int playback_fs;
 	bool deemph;
 };
@@ -528,6 +529,53 @@ static int get_coeff(int mclk, int rate)
 	return -EINVAL;
 }
 
+/* The set of rates we can generate from the above for each SYSCLK */
+static const unsigned int rates_12288[] = {
+	8000, 12000, 16000, 24000, 32000, 48000, 96000
+};
+
+static struct snd_pcm_hw_constraint_list constraints_12288 = {
+	.count  = ARRAY_SIZE(rates_12288),
+	.list   = rates_12288,
+};
+
+static const unsigned int rates_112896[] = {
+	8000, 11025, 22050, 44100, 88200
+};
+
+static struct snd_pcm_hw_constraint_list constraints_112896 = {
+	.count  = ARRAY_SIZE(rates_112896),
+	.list   = rates_112896,
+};
+
+static const unsigned int rates_18432[] = {
+	8000, 12000, 16000, 24000, 32000, 48000, 96000
+};
+
+static struct snd_pcm_hw_constraint_list constraints_18432 = {
+	.count  = ARRAY_SIZE(rates_18432),
+	.list   = rates_18432,
+};
+
+static const unsigned int rates_169344[] = {
+	8000, 11025, 22050, 44100, 88200
+};
+
+static struct snd_pcm_hw_constraint_list constraints_169344 = {
+	.count  = ARRAY_SIZE(rates_169344),
+	.list   = rates_169344,
+};
+
+static const unsigned int rates_12[] = {
+	8000, 11025, 12000, 16000, 22050, 2400, 32000, 41100, 48000,
+	48000, 88235, 96000,
+};
+
+static struct snd_pcm_hw_constraint_list constraints_12 = {
+	.count  = ARRAY_SIZE(rates_12),
+	.list   = rates_12,
+};
+
 static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 				 int clk_id, unsigned int freq, int dir)
 {
@@ -535,15 +583,32 @@ static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
 
 	switch (freq) {
-	case 11289600:
-	case 12000000:
 	case 12288000:
-	case 16934400:
+	case 24576000:
+		wm8971->sysclk_constraints = &constraints_12288;
+		break;
+	case 11289600:
+	case 22579200:
+		wm8971->sysclk_constraints = &constraints_112896;
+		break;
 	case 18432000:
-		wm8971->sysclk = freq;
-		return 0;
+		wm8971->sysclk_constraints = &constraints_18432;
+		break;
+	case 16934400:
+	case 33868800:
+		wm8971->sysclk_constraints = &constraints_169344;
+		break;
+	case 12000000:
+	case 24000000:
+		wm8971->sysclk_constraints = &constraints_12;
+		break;
+	default:
+		return -EINVAL;
 	}
-	return -EINVAL;
+
+	wm8971->sysclk = freq;
+
+	return 0;
 }
 
 static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 6/9] WM8971 uses msleep to replace work queue
  2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
                   ` (3 preceding siblings ...)
  2014-09-02  3:27 ` [PATCHv3 5/9] Using the constraint based on wm8971_set_dai_sysclk Xavier Hsu
@ 2014-09-02  3:27 ` Xavier Hsu
  2014-09-02 10:00   ` Charles Keepax
  2014-09-02  3:27 ` [PATCHv3 7/9] WM8971 improves the function of regmap Xavier Hsu
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch improves WM8971.
We use msleep to replace work queue.
"As suggested by Mark Brown"

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |   25 ++-----------------------
 1 file changed, 2 insertions(+), 23 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 20cfdd3..68b4f52 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -36,8 +36,6 @@
 
 #define	WM8971_REG_COUNT	43
 
-static struct workqueue_struct *wm8971_workq;
-
 /* codec private data */
 struct wm8971_priv {
 	unsigned int sysclk;
@@ -774,16 +772,6 @@ static struct snd_soc_dai_driver wm8971_dai = {
 	.ops = &wm8971_dai_ops,
 };
 
-static void wm8971_work(struct work_struct *work)
-{
-	struct snd_soc_dapm_context *dapm =
-		container_of(work, struct snd_soc_dapm_context,
-			     delayed_work.work);
-	struct snd_soc_codec *codec = dapm->codec;
-
-	wm8971_set_bias_level(codec, codec->dapm.bias_level);
-}
-
 static int wm8971_suspend(struct snd_soc_codec *codec)
 {
 	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
@@ -801,8 +789,7 @@ static int wm8971_resume(struct snd_soc_codec *codec)
 		reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
 		snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
 		codec->dapm.bias_level = SND_SOC_BIAS_ON;
-		queue_delayed_work(wm8971_workq, &codec->dapm.delayed_work,
-				   msecs_to_jiffies(1000));
+		msleep(100);
 	}
 
 	return 0;
@@ -813,19 +800,13 @@ static int wm8971_probe(struct snd_soc_codec *codec)
 	int ret = 0;
 	u16 reg;
 
-	INIT_DELAYED_WORK(&codec->dapm.delayed_work, wm8971_work);
-	wm8971_workq = create_workqueue("wm8971");
-	if (wm8971_workq == NULL)
-		return -ENOMEM;
-
 	wm8971_reset(codec);
 
 	/* charge output caps - set vmid to 5k for quick power up */
 	reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
 	snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
 	codec->dapm.bias_level = SND_SOC_BIAS_STANDBY;
-	queue_delayed_work(wm8971_workq, &codec->dapm.delayed_work,
-			   msecs_to_jiffies(1000));
+	msleep(100);
 
 	/* set the update bits */
 	snd_soc_update_bits(codec, WM8971_LDAC, 0x0100, 0x0100);
@@ -846,8 +827,6 @@ static int wm8971_remove(struct snd_soc_codec *codec)
 {
 	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
 
-	if (wm8971_workq)
-		destroy_workqueue(wm8971_workq);
 	return 0;
 }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 7/9] WM8971 improves the function of regmap
  2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
                   ` (4 preceding siblings ...)
  2014-09-02  3:27 ` [PATCHv3 6/9] WM8971 uses msleep to replace work queue Xavier Hsu
@ 2014-09-02  3:27 ` Xavier Hsu
  2014-09-02 11:03   ` Charles Keepax
  2014-09-03 19:19   ` Lars-Peter Clausen
  2014-09-02  3:27 ` [PATCHv3 8/9] WM8971 adds kcontrol functions Xavier Hsu
  2014-09-02  3:27 ` [PATCHv3 9/9] ASOC add WM8973 support to WM8971 Xavier Hsu
  7 siblings, 2 replies; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch improves WM8971.
We modify the function of regmap.

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |   26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 68b4f52..95e7c39 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -38,6 +38,7 @@
 
 /* codec private data */
 struct wm8971_priv {
+	struct regmap *regmap;
 	unsigned int sysclk;
 	struct snd_pcm_hw_constraint_list *sysclk_constraints;
 	int playback_fs;
@@ -716,6 +717,7 @@ static int wm8971_mute(struct snd_soc_dai *dai, int mute)
 static int wm8971_set_bias_level(struct snd_soc_codec *codec,
 				 enum snd_soc_bias_level level)
 {
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
 	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
 
 	switch (level) {
@@ -727,7 +729,7 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
 		break;
 	case SND_SOC_BIAS_STANDBY:
 		if (codec->dapm.bias_level == SND_SOC_BIAS_OFF)
-			snd_soc_cache_sync(codec);
+			regcache_sync(wm8971->regmap);
 
 		/* mute dac and set vmid to 500k, enable VREF */
 		snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140);
@@ -774,7 +776,11 @@ static struct snd_soc_dai_driver wm8971_dai = {
 
 static int wm8971_suspend(struct snd_soc_codec *codec)
 {
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
+
 	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
+	regcache_mark_dirty(wm8971->regmap);
+
 	return 0;
 }
 
@@ -797,9 +803,12 @@ static int wm8971_resume(struct snd_soc_codec *codec)
 
 static int wm8971_probe(struct snd_soc_codec *codec)
 {
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
 	int ret = 0;
 	u16 reg;
 
+	codec->control_data = wm8971->regmap;
+
 	wm8971_reset(codec);
 
 	/* charge output caps - set vmid to 5k for quick power up */
@@ -830,12 +839,20 @@ static int wm8971_remove(struct snd_soc_codec *codec)
 	return 0;
 }
 
+struct regmap *wm8971_get_regmap(struct device *dev)
+{
+	struct wm8971_priv *priv = dev_get_drvdata(dev);
+
+	return priv->regmap;
+}
+
 static struct snd_soc_codec_driver soc_codec_dev_wm8971 = {
 	.probe =	wm8971_probe,
 	.remove =	wm8971_remove,
 	.suspend =	wm8971_suspend,
 	.resume =	wm8971_resume,
 	.set_bias_level = wm8971_set_bias_level,
+	.get_regmap =	wm8971_get_regmap,
 
 	.controls = wm8971_snd_controls,
 	.num_controls = ARRAY_SIZE(wm8971_snd_controls),
@@ -859,7 +876,6 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
 	struct wm8971_priv *wm8971;
-	struct regmap *regmap;
 	int ret;
 
 	wm8971 = devm_kzalloc(&i2c->dev, sizeof(struct wm8971_priv),
@@ -867,9 +883,9 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
 	if (wm8971 == NULL)
 		return -ENOMEM;
 
-	regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
+	wm8971->regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap);
+	if (IS_ERR(wm8971->regmap))
+		return PTR_ERR(wm8971->regmap);
 
 	i2c_set_clientdata(i2c, wm8971);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 8/9] WM8971 adds kcontrol functions
  2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
                   ` (5 preceding siblings ...)
  2014-09-02  3:27 ` [PATCHv3 7/9] WM8971 improves the function of regmap Xavier Hsu
@ 2014-09-02  3:27 ` Xavier Hsu
  2014-09-02 12:41   ` Charles Keepax
  2014-09-02  3:27 ` [PATCHv3 9/9] ASOC add WM8973 support to WM8971 Xavier Hsu
  7 siblings, 1 reply; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch improves WM8971.
We add kcontrol functions on WM8971.

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 95e7c39..6d34565 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -284,6 +284,12 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 
 	SOC_DOUBLE_R_TLV("ADC Volume", WM8971_LADC, WM8971_RADC,
 			 0, 255, 0, adc_vol),
+
+	SOC_SINGLE("Analogue Bias", WM8971_ADCTL1, 6, 3, 0),
+	SOC_SINGLE("Right Speaker Playback Invert Switch", WM8971_ADCTL2,
+		   4, 1, 0),
+	SOC_SINGLE("Headphone Switch POL", WM8971_ADCTL2, 5, 1, 0),
+	SOC_SINGLE("Headphone Switch EN", WM8971_ADCTL2, 6, 1, 0),
 };
 
 /*
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 9/9] ASOC add WM8973 support to WM8971
  2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
                   ` (6 preceding siblings ...)
  2014-09-02  3:27 ` [PATCHv3 8/9] WM8971 adds kcontrol functions Xavier Hsu
@ 2014-09-02  3:27 ` Xavier Hsu
  2014-09-02 12:38   ` Charles Keepax
  7 siblings, 1 reply; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:27 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, Andy Green

This patch adds support for the wm8973 codec.

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 Documentation/devicetree/bindings/sound/wm8971.txt |   26 ++
 sound/soc/codecs/wm8971.c                          |  326 +++++++++++++++-----
 sound/soc/codecs/wm8971.h                          |   21 +-
 3 files changed, 294 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/wm8971.txt

diff --git a/Documentation/devicetree/bindings/sound/wm8971.txt b/Documentation/devicetree/bindings/sound/wm8971.txt
new file mode 100644
index 0000000..7313dd1
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/wm8971.txt
@@ -0,0 +1,26 @@
+WM8971 and WM8973 audio CODEC
+
+These devices support both I2C and SPI (configured with pin strapping
+on the board).
+
+Required properties:
+
+  - compatible : "wlf,wm8971".
+		 "wlf,wm8973".
+
+  - reg : the I2C address of the device for I2C, the chip select
+	  number for SPI.
+
+Optional properties:
+
+  - mclk-div : Setting the CLKDIV2 bit for dividing MCLK.
+    mclk-div = <0> (Default & not divide).
+    mclk-div = <1> (Divide by 2).
+
+Example:
+
+codec: wm8973@1a {
+	compatible = "wlf,wm8973";
+	reg = <0x1a>;
+	mclk-div = <1>;
+};
diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 6d34565..2893fc6 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -7,7 +7,7 @@
  *
  * Based on wm8753.c by Liam Girdwood
  *
- * WM8971 Improve Copyright (C) 2014 Linaro, Ltd
+ * WM8971 Improve & Wm8973 Support Copyright (C) 2014 Linaro, Ltd
  * Author: Xavier Hsu <xavier.hsu@linaro.org>
  *         Andy Green <andy.green@linaro.org>
  *
@@ -34,15 +34,27 @@
 
 #include "wm8971.h"
 
+enum variant {
+	VARIANT_WM8971,
+	VARIANT_WM8973
+};
+
 #define	WM8971_REG_COUNT	43
 
 /* codec private data */
 struct wm8971_priv {
 	struct regmap *regmap;
+	enum variant variant;
 	unsigned int sysclk;
 	struct snd_pcm_hw_constraint_list *sysclk_constraints;
 	int playback_fs;
 	bool deemph;
+	int mclk_div;
+};
+
+static const u16 charge[][2] = {
+	[VARIANT_WM8971] = { 0xfe3e, 0x01c0 },
+	[VARIANT_WM8973] = { 0x003e, 0x01f0 },
 };
 
 /*
@@ -98,7 +110,7 @@ static const struct reg_default wm8971_reg_defaults[] = {
 
 #define wm8971_reset(c)	snd_soc_write(c, WM8971_RESET, 0)
 
-/* WM8971 Controls */
+/* WM8971 / 3 Controls */
 static const char const *wm8971_bass[] = {"Linear Control", "Adaptive Boost"};
 static const char const *wm8971_bass_filter[] = {"130Hz @ 48kHz",
 						 "200Hz @ 48kHz"};
@@ -110,19 +122,33 @@ static const char const *wm8971_ng_type[] = {"Constant PGA Gain",
 static const char const *wm8971_mono_mux[] = {"Stereo", "Mono (Left)",
 					      "Mono (Right)", "Digital Mono"};
 static const char const *wm8971_dac_phase[] = {"Non Inverted", "Inverted"};
-static const char const *wm8971_lline_mux[] = {"Line", "NC", "NC",
-					       "PGA", "Differential"};
-static const char const *wm8971_rline_mux[] = {"Line", "Mic", "NC",
-					       "PGA", "Differential"};
-static const char const *wm8971_lpga_sel[] = {"Line", "NC", "NC",
-					      "Differential"};
-static const char const *wm8971_rpga_sel[] = {"Line", "Mic", "NC",
-					      "Differential"};
 static const char const *wm8971_adcpol[] = {"Normal", "L Invert",
 					    "R Invert", "L + R Invert"};
 static const char const *wm8971_deemp[] = {"None", "32kHz",
 					   "44.1kHz", "48kHz"};
 
+/* WM8971-only*/
+static const char const *wm8971_lline_mux[] = {"Line 1", "NC", "NC",
+					       "PGA", "Differential"};
+static const char const *wm8971_rline_mux[] = {"Line 1", "Mic", "NC",
+					       "PGA", "Differential"};
+static const char const *wm8971_lpga_sel[] = {"Line 1", "NC", "NC",
+					      "Differential"};
+static const char const *wm8971_rpga_sel[] = {"Line 1", "Mic", "NC",
+					      "Differential"};
+
+/* WM8973-only*/
+static const char const *wm8973_line_mux[] = {"Line 1", "Line 2", "Line 3",
+					      "PGA", "Differential"};
+static const char const *wm8973_pga_sel[] = {"Line 1", "Line 2", "Line 3",
+					     "Differential"};
+static const char const *wm8973_out3[] = {"VREF", "ROUT1 + Vol", "MonoOut",
+					  "ROUT1"};
+static const char const *wm8973_diff_sel[] = {"Line 1", "Line 2"};
+static const char const *wm8973_3d_lc[] = { "200Hz", "500Hz" };
+static const char const *wm8973_3d_uc[] = { "2.2kHz", "1.5kHz" };
+static const char const *wm8973_3d_func[] = {"Capture", "Playback"};
+
 static int wm8971_set_deemph(struct snd_soc_codec *codec)
 {
 	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
@@ -182,6 +208,7 @@ static int wm8971_put_deemph(struct snd_kcontrol *kcontrol,
 	return ret;
 }
 
+/* WM8971 / 3 Controls */
 static const SOC_ENUM_SINGLE_DECL(bass_boost, WM8971_BASS, 7, wm8971_bass);
 static const SOC_ENUM_SINGLE_DECL(bass_filter, WM8971_BASS,
 				  6, wm8971_bass_filter);
@@ -195,17 +222,39 @@ static const SOC_ENUM_SINGLE_DECL(dac_mono_mix, WM8971_ADCTL1,
 				  4, wm8971_mono_mux);
 static const SOC_ENUM_SINGLE_DECL(dac_phase_inv, WM8971_ADCTL1,
 				  1, wm8971_dac_phase);
-static const SOC_ENUM_SINGLE_DECL(left_line, WM8971_LOUTM1,
-				  0, wm8971_lline_mux);
-static const SOC_ENUM_SINGLE_DECL(right_line, WM8971_ROUTM1,
-				  0, wm8971_rline_mux);
-static const SOC_ENUM_SINGLE_DECL(left_pga, WM8971_LADCIN, 6, wm8971_lpga_sel);
-static const SOC_ENUM_SINGLE_DECL(right_pga, WM8971_RADCIN,
-				  6, wm8971_rpga_sel);
 static const SOC_ENUM_SINGLE_DECL(capture_polarity, WM8971_ADCDAC,
 				  5, wm8971_adcpol);
 static const SOC_ENUM_SINGLE_DECL(monomux, WM8971_ADCIN, 6, wm8971_mono_mux);
 
+/* WM8971-only */
+static const SOC_ENUM_SINGLE_DECL(wm8971_left_line, WM8971_LOUTM1,
+				  0, wm8971_lline_mux);
+static const SOC_ENUM_SINGLE_DECL(wm8971_right_line, WM8971_ROUTM1,
+				  0, wm8971_rline_mux);
+static const SOC_ENUM_SINGLE_DECL(wm8971_left_pga, WM8971_LADCIN,
+				  6, wm8971_lpga_sel);
+static const SOC_ENUM_SINGLE_DECL(wm8971_right_pga, WM8971_RADCIN,
+				  6, wm8971_rpga_sel);
+
+/* WM8973-only */
+static const SOC_ENUM_SINGLE_DECL(wm8973_left_line, WM8971_LOUTM1,
+				  0, wm8973_line_mux);
+static const SOC_ENUM_SINGLE_DECL(wm8973_right_line, WM8971_ROUTM1,
+				  0, wm8973_line_mux);
+static const SOC_ENUM_SINGLE_DECL(wm8973_left_pga, WM8971_LADCIN,
+				  6, wm8973_pga_sel);
+static const SOC_ENUM_SINGLE_DECL(wm8973_right_pga, WM8971_RADCIN,
+				  6, wm8973_pga_sel);
+static const SOC_ENUM_SINGLE_DECL(wm8973_out3_control, WM8971_ADCTL2,
+				  7, wm8973_out3);
+static const SOC_ENUM_SINGLE_DECL(wm8973_diffmux, WM8971_ADCIN,
+				  8, wm8973_diff_sel);
+static const SOC_ENUM_SINGLE_DECL(wm8973_lower_cutoff, WM8973_3D,
+				  5, wm8973_3d_lc);
+static const SOC_ENUM_SINGLE_DECL(wm8973_upper_cutoff, WM8973_3D,
+				  6, wm8973_3d_uc);
+static const SOC_ENUM_SINGLE_DECL(wm8973_mode, WM8973_3D, 7, wm8973_3d_func);
+
 static const DECLARE_TLV_DB_SCALE(in_vol, -1725, 75, 0);
 static const DECLARE_TLV_DB_SCALE(out_vol, -6700, 91, 0);
 static const DECLARE_TLV_DB_SCALE(attenuate_6db, -600, 600, 0);
@@ -292,6 +341,15 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
 	SOC_SINGLE("Headphone Switch EN", WM8971_ADCTL2, 6, 1, 0),
 };
 
+static const struct snd_kcontrol_new wm8973_snd_controls[] = {
+	/* 3D Control */
+	SOC_SINGLE("3D Switch", WM8973_3D, 0, 1, 0),
+	SOC_SINGLE("3D Volume", WM8973_3D, 1, 15, 0),
+	SOC_ENUM("3D Lower Cut-off", wm8973_lower_cutoff),
+	SOC_ENUM("3D Upper Cut-off", wm8973_upper_cutoff),
+	SOC_ENUM("3D Mode", wm8973_mode),
+};
+
 /*
  * DAPM Controls
  */
@@ -322,25 +380,41 @@ SOC_DAPM_SINGLE("Right Bypass Switch", WM8971_MOUTM2, 7, 1, 0),
 
 /* Left Line Mux */
 static const struct snd_kcontrol_new wm8971_left_line_controls =
-SOC_DAPM_ENUM("Route", left_line);
+SOC_DAPM_ENUM("Route", wm8971_left_line);
+static const struct snd_kcontrol_new wm8973_left_line_controls =
+SOC_DAPM_ENUM("Route", wm8973_left_line);
 
 /* Right Line Mux */
 static const struct snd_kcontrol_new wm8971_right_line_controls =
-SOC_DAPM_ENUM("Route", right_line);
+SOC_DAPM_ENUM("Route", wm8971_right_line);
+static const struct snd_kcontrol_new wm8973_right_line_controls =
+SOC_DAPM_ENUM("Route", wm8973_right_line);
 
 /* Left PGA Mux */
 static const struct snd_kcontrol_new wm8971_left_pga_controls =
-SOC_DAPM_ENUM("Route", left_pga);
+SOC_DAPM_ENUM("Route", wm8971_left_pga);
+static const struct snd_kcontrol_new wm8973_left_pga_controls =
+SOC_DAPM_ENUM("Route", wm8973_left_pga);
 
 /* Right PGA Mux */
 static const struct snd_kcontrol_new wm8971_right_pga_controls =
-SOC_DAPM_ENUM("Route", right_pga);
+SOC_DAPM_ENUM("Route", wm8971_right_pga);
+static const struct snd_kcontrol_new wm8973_right_pga_controls =
+SOC_DAPM_ENUM("Route", wm8973_right_pga);
+
+/* Out 3 Mux */
+static const struct snd_kcontrol_new wm8973_out3_controls =
+SOC_DAPM_ENUM("Route", wm8973_out3_control);
+
+/* Differential Mux */
+static const struct snd_kcontrol_new wm8973_diffmux_controls =
+SOC_DAPM_ENUM("Route", wm8973_diffmux);
 
 /* Mono ADC Mux */
 static const struct snd_kcontrol_new wm8971_monomux_controls =
 SOC_DAPM_ENUM("Route", monomux);
 
-static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
+static const struct snd_soc_dapm_widget wm897x_dapm_widgets[] = {
 	SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
 			   &wm8971_left_mixer_controls[0],
 			   ARRAY_SIZE(wm8971_left_mixer_controls)),
@@ -363,15 +437,6 @@ static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
 	SND_SOC_DAPM_ADC("Right ADC", "Right Capture", WM8971_PWR1, 2, 0),
 	SND_SOC_DAPM_ADC("Left ADC", "Left Capture", WM8971_PWR1, 3, 0),
 
-	SND_SOC_DAPM_MUX("Left PGA Mux", WM8971_PWR1, 5, 0,
-			 &wm8971_left_pga_controls),
-	SND_SOC_DAPM_MUX("Right PGA Mux", WM8971_PWR1, 4, 0,
-			 &wm8971_right_pga_controls),
-	SND_SOC_DAPM_MUX("Left Line Mux", SND_SOC_NOPM, 0, 0,
-			 &wm8971_left_line_controls),
-	SND_SOC_DAPM_MUX("Right Line Mux", SND_SOC_NOPM, 0, 0,
-			 &wm8971_right_line_controls),
-
 	SND_SOC_DAPM_MUX("Left ADC Mux", SND_SOC_NOPM, 0, 0,
 			 &wm8971_monomux_controls),
 	SND_SOC_DAPM_MUX("Right ADC Mux", SND_SOC_NOPM, 0, 0,
@@ -388,6 +453,41 @@ static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
 	SND_SOC_DAPM_INPUT("MIC"),
 };
 
+static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
+	SND_SOC_DAPM_MUX("Left Line Mux", SND_SOC_NOPM, 0, 0,
+			 &wm8971_left_line_controls),
+	SND_SOC_DAPM_MUX("Right Line Mux", SND_SOC_NOPM, 0, 0,
+			 &wm8971_right_line_controls),
+	SND_SOC_DAPM_MUX("Left PGA Mux", WM8971_PWR1, 5, 0,
+			 &wm8971_left_pga_controls),
+	SND_SOC_DAPM_MUX("Right PGA Mux", WM8971_PWR1, 4, 0,
+			 &wm8971_right_pga_controls),
+};
+
+static const struct snd_soc_dapm_widget wm8973_dapm_widgets[] = {
+	SND_SOC_DAPM_MUX("Left Line Mux", SND_SOC_NOPM, 0, 0,
+			 &wm8973_left_line_controls),
+	SND_SOC_DAPM_MUX("Right Line Mux", SND_SOC_NOPM, 0, 0,
+			 &wm8973_right_line_controls),
+	SND_SOC_DAPM_MUX("Right PGA Mux", WM8971_PWR1, 4, 0,
+			 &wm8973_right_pga_controls),
+	SND_SOC_DAPM_MUX("Left PGA Mux", WM8971_PWR1, 5, 0,
+			 &wm8973_left_pga_controls),
+	SND_SOC_DAPM_MUX("Out3 Mux", SND_SOC_NOPM, 0, 0,
+			 &wm8973_out3_controls),
+	SND_SOC_DAPM_PGA("Out 3", WM8971_PWR2, 1, 0, NULL, 0),
+	SND_SOC_DAPM_MUX("Differential Mux", SND_SOC_NOPM, 0, 0,
+			 &wm8973_diffmux_controls),
+
+	SND_SOC_DAPM_OUTPUT("OUT3"),
+	SND_SOC_DAPM_OUTPUT("VREF"),
+
+	SND_SOC_DAPM_INPUT("LINPUT2"),
+	SND_SOC_DAPM_INPUT("LINPUT3"),
+	SND_SOC_DAPM_INPUT("RINPUT2"),
+	SND_SOC_DAPM_INPUT("RINPUT3"),
+};
+
 static const struct snd_soc_dapm_route wm8971_dapm_routes[] = {
 	/* left mixer */
 	{"Left Mixer", "Playback Switch", "Left DAC"},
@@ -428,27 +528,27 @@ static const struct snd_soc_dapm_route wm8971_dapm_routes[] = {
 	{"MONO1", NULL, "Mono Out"},
 
 	/* Left Line Mux */
-	{"Left Line Mux", "Line", "LINPUT1"},
+	{"Left Line Mux", "Line 1", "LINPUT1"},
 	{"Left Line Mux", "PGA", "Left PGA Mux"},
 	{"Left Line Mux", "Differential", "Differential Mux"},
 
 	/* Right Line Mux */
-	{"Right Line Mux", "Line", "RINPUT1"},
+	{"Right Line Mux", "Line 1", "RINPUT1"},
 	{"Right Line Mux", "Mic", "MIC"},
 	{"Right Line Mux", "PGA", "Right PGA Mux"},
 	{"Right Line Mux", "Differential", "Differential Mux"},
 
 	/* Left PGA Mux */
-	{"Left PGA Mux", "Line", "LINPUT1"},
+	{"Left PGA Mux", "Line 1", "LINPUT1"},
 	{"Left PGA Mux", "Differential", "Differential Mux"},
 
 	/* Right PGA Mux */
-	{"Right PGA Mux", "Line", "RINPUT1"},
+	{"Right PGA Mux", "Line 1", "RINPUT1"},
 	{"Right PGA Mux", "Differential", "Differential Mux"},
 
 	/* Differential Mux */
-	{"Differential Mux", "Line", "LINPUT1"},
-	{"Differential Mux", "Line", "RINPUT1"},
+	{"Differential Mux", "Line 1", "LINPUT1"},
+	{"Differential Mux", "Line 1", "RINPUT1"},
 
 	/* Left ADC Mux */
 	{"Left ADC Mux", "Stereo", "Left PGA Mux"},
@@ -465,6 +565,62 @@ static const struct snd_soc_dapm_route wm8971_dapm_routes[] = {
 	{"Right ADC", NULL, "Right ADC Mux"},
 };
 
+static const struct snd_soc_dapm_route wm8973_dapm_routes[] = {
+	{"Out3 Mux", "VREF", "VREF"},
+	{"Out3 Mux", "ROUT1 + Vol", "ROUT1"},
+	{"Out3 Mux", "ROUT1", "Right Mixer"},
+	{"Out3 Mux", "MonoOut", "MONO1"},
+	{"Out 3", NULL, "Out3 Mux"},
+	{"OUT3", NULL, "Out 3"},
+
+	{"Left Line Mux", "Line 2", "LINPUT2"},
+	{"Left Line Mux", "Line 3", "LINPUT3"},
+
+	{"Right Line Mux", "Line 2", "RINPUT2"},
+	{"Right Line Mux", "Line 3", "RINPUT3"},
+
+	{"Left PGA Mux", "Line 2", "LINPUT2"},
+	{"Left PGA Mux", "Line 3", "LINPUT3"},
+
+	{"Right PGA Mux", "Line 2", "RINPUT2"},
+	{"Right PGA Mux", "Line 3", "RINPUT3"},
+
+	{"Differential Mux", "Line 2", "LINPUT2"},
+	{"Differential Mux", "Line 2", "RINPUT2"},
+};
+
+static int wm897x_add_controls(struct snd_soc_codec *codec)
+{
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
+	struct snd_soc_dapm_context *dapm = &codec->dapm;
+
+	snd_soc_add_codec_controls(codec, wm8971_snd_controls,
+				   ARRAY_SIZE(wm8971_snd_controls));
+
+	if (wm8971->variant)
+		snd_soc_add_codec_controls(codec, wm8973_snd_controls,
+					   ARRAY_SIZE(wm8973_snd_controls));
+
+	snd_soc_dapm_new_controls(dapm, wm897x_dapm_widgets,
+				  ARRAY_SIZE(wm897x_dapm_widgets));
+
+	if (wm8971->variant)
+		snd_soc_dapm_new_controls(dapm, wm8973_dapm_widgets,
+					  ARRAY_SIZE(wm8973_dapm_widgets));
+	else
+		snd_soc_dapm_new_controls(dapm, wm8971_dapm_widgets,
+					  ARRAY_SIZE(wm8971_dapm_widgets));
+
+	snd_soc_dapm_add_routes(dapm, wm8971_dapm_routes,
+				ARRAY_SIZE(wm8971_dapm_routes));
+
+	if (wm8971->variant)
+		snd_soc_dapm_add_routes(dapm, wm8973_dapm_routes,
+					ARRAY_SIZE(wm8973_dapm_routes));
+
+	return 0;
+}
+
 struct _coeff_div {
 	u32 mclk;
 	u32 rate;
@@ -724,12 +880,18 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
 				 enum snd_soc_bias_level level)
 {
 	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
-	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
+	static const u16 mask[][2] = {
+		[VARIANT_WM8971] = { 0x00c1, 0x0140 },
+		[VARIANT_WM8973] = { 0x00c2, 0x0141 },
+	};
+	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1)
+				   & charge[wm8971->variant][0];
 
 	switch (level) {
 	case SND_SOC_BIAS_ON:
 		/* set vmid to 50k and unmute dac */
-		snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x00c1);
+		snd_soc_write(codec, WM8971_PWR1,
+			      pwr_reg | mask[wm8971->variant][0]);
 		break;
 	case SND_SOC_BIAS_PREPARE:
 		break;
@@ -738,7 +900,8 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
 			regcache_sync(wm8971->regmap);
 
 		/* mute dac and set vmid to 500k, enable VREF */
-		snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140);
+		snd_soc_write(codec, WM8971_PWR1,
+			      pwr_reg | mask[wm8971->variant][1]);
 		break;
 	case SND_SOC_BIAS_OFF:
 		snd_soc_write(codec, WM8971_PWR1, 0x0001);
@@ -763,21 +926,28 @@ static const struct snd_soc_dai_ops wm8971_dai_ops = {
 	.set_sysclk	= wm8971_set_dai_sysclk,
 };
 
-static struct snd_soc_dai_driver wm8971_dai = {
-	.name = "wm8971-hifi",
-	.playback = {
-		.stream_name = "Playback",
-		.channels_min = 1,
-		.channels_max = 2,
-		.rates = WM8971_RATES,
-		.formats = WM8971_FORMATS,},
-	.capture = {
-		.stream_name = "Capture",
-		.channels_min = 1,
-		.channels_max = 2,
-		.rates = WM8971_RATES,
-		.formats = WM8971_FORMATS,},
-	.ops = &wm8971_dai_ops,
+static struct snd_soc_dai_driver wm8971_dai[] = {
+	{
+		.name = "wm8971-hifi-playback",
+		.playback = {
+			.stream_name = "Playback",
+			.channels_min = 1,
+			.channels_max = 2,
+			.rates = WM8971_RATES,
+			.formats = WM8971_FORMATS,
+		},
+		.ops = &wm8971_dai_ops,
+	}, {
+		.name = "wm8971-hifi-capture",
+		.capture = {
+			.stream_name = "Capture",
+			.channels_min = 1,
+			.channels_max = 2,
+			.rates = WM8971_RATES,
+			.formats = WM8971_FORMATS,
+		},
+		.ops = &wm8971_dai_ops,
+	},
 };
 
 static int wm8971_suspend(struct snd_soc_codec *codec)
@@ -792,14 +962,17 @@ static int wm8971_suspend(struct snd_soc_codec *codec)
 
 static int wm8971_resume(struct snd_soc_codec *codec)
 {
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
 	u16 reg;
 
 	wm8971_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 
 	/* charge wm8971 caps */
 	if (codec->dapm.suspend_bias_level == SND_SOC_BIAS_ON) {
-		reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
-		snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
+		reg = snd_soc_read(codec, WM8971_PWR1)
+				   & charge[wm8971->variant][0];
+		snd_soc_write(codec, WM8971_PWR1,
+			      reg | charge[wm8971->variant][1]);
 		codec->dapm.bias_level = SND_SOC_BIAS_ON;
 		msleep(100);
 	}
@@ -810,16 +983,25 @@ static int wm8971_resume(struct snd_soc_codec *codec)
 static int wm8971_probe(struct snd_soc_codec *codec)
 {
 	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
+	const u32 *p;
 	int ret = 0;
 	u16 reg;
 
 	codec->control_data = wm8971->regmap;
 
 	wm8971_reset(codec);
+	if (codec->dev->of_node) {
+		p = of_get_property(codec->dev->of_node, "mclk-div", NULL);
+		if (p)
+			wm8971->mclk_div = be32_to_cpu(*p);
+	}
+	/* Master Clock Divide by 2 (0 = not div, 1 = div by 2) */
+	if (wm8971->mclk_div)
+		snd_soc_update_bits(codec, WM8971_SRATE, 0x0040, 0x0040);
 
 	/* charge output caps - set vmid to 5k for quick power up */
-	reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
-	snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
+	reg = snd_soc_read(codec, WM8971_PWR1) & charge[wm8971->variant][0];
+	snd_soc_write(codec, WM8971_PWR1, reg | charge[wm8971->variant][1]);
 	codec->dapm.bias_level = SND_SOC_BIAS_STANDBY;
 	msleep(100);
 
@@ -833,6 +1015,8 @@ static int wm8971_probe(struct snd_soc_codec *codec)
 	snd_soc_update_bits(codec, WM8971_LINVOL, 0x0100, 0x0100);
 	snd_soc_update_bits(codec, WM8971_RINVOL, 0x0100, 0x0100);
 
+	wm897x_add_controls(codec);
+
 	return ret;
 }
 
@@ -841,7 +1025,6 @@ static int wm8971_probe(struct snd_soc_codec *codec)
 static int wm8971_remove(struct snd_soc_codec *codec)
 {
 	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
-
 	return 0;
 }
 
@@ -859,13 +1042,6 @@ static struct snd_soc_codec_driver soc_codec_dev_wm8971 = {
 	.resume =	wm8971_resume,
 	.set_bias_level = wm8971_set_bias_level,
 	.get_regmap =	wm8971_get_regmap,
-
-	.controls = wm8971_snd_controls,
-	.num_controls = ARRAY_SIZE(wm8971_snd_controls),
-	.dapm_widgets = wm8971_dapm_widgets,
-	.num_dapm_widgets = ARRAY_SIZE(wm8971_dapm_widgets),
-	.dapm_routes = wm8971_dapm_routes,
-	.num_dapm_routes = ARRAY_SIZE(wm8971_dapm_routes),
 };
 
 static const struct regmap_config wm8971_regmap = {
@@ -889,6 +1065,8 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
 	if (wm8971 == NULL)
 		return -ENOMEM;
 
+	wm8971->variant = (int)id->driver_data;
+
 	wm8971->regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap);
 	if (IS_ERR(wm8971->regmap))
 		return PTR_ERR(wm8971->regmap);
@@ -896,7 +1074,7 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
 	i2c_set_clientdata(i2c, wm8971);
 
 	ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_wm8971,
-				     &wm8971_dai, 1);
+				     wm8971_dai, ARRAY_SIZE(wm8971_dai));
 
 	return ret;
 }
@@ -908,15 +1086,23 @@ static int wm8971_i2c_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id wm8971_i2c_id[] = {
-	{ "wm8971", 0 },
+	{ "wm8971", VARIANT_WM8971 },
+	{ "wm8973", VARIANT_WM8973 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, wm8971_i2c_id);
 
+static const struct of_device_id wm897x_dt_ids[] = {
+	{ .compatible = "wlf,wm8971" },
+	{ .compatible = "wlf,wm8973" },
+	{ /* sentinel */ }
+};
+
 static struct i2c_driver wm8971_i2c_driver = {
 	.driver = {
 		.name = "wm8971",
 		.owner = THIS_MODULE,
+		.of_match_table = wm897x_dt_ids,
 	},
 	.probe =    wm8971_i2c_probe,
 	.remove =   wm8971_i2c_remove,
@@ -928,3 +1114,5 @@ module_i2c_driver(wm8971_i2c_driver);
 MODULE_DESCRIPTION("ASoC WM8971 driver");
 MODULE_AUTHOR("Lab126");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("*wm8971*");
+MODULE_ALIAS("*wm8973*");
diff --git a/sound/soc/codecs/wm8971.h b/sound/soc/codecs/wm8971.h
index f31c38f..aeab04a 100644
--- a/sound/soc/codecs/wm8971.h
+++ b/sound/soc/codecs/wm8971.h
@@ -22,21 +22,22 @@
 #define WM8971_ADCDAC	0x05
 #define WM8971_IFACE	0x07
 #define WM8971_SRATE	0x08
-#define WM8971_LDAC		0x0a
-#define WM8971_RDAC		0x0b
-#define WM8971_BASS		0x0c
+#define WM8971_LDAC	0x0a
+#define WM8971_RDAC	0x0b
+#define WM8971_BASS	0x0c
 #define WM8971_TREBLE	0x0d
 #define WM8971_RESET	0x0f
-#define WM8971_ALC1		0x11
-#define	WM8971_ALC2		0x12
-#define	WM8971_ALC3		0x13
+#define WM8973_3D       0x10
+#define WM8971_ALC1	0x11
+#define	WM8971_ALC2	0x12
+#define	WM8971_ALC3	0x13
 #define WM8971_NGATE	0x14
-#define WM8971_LADC		0x15
-#define WM8971_RADC		0x16
+#define WM8971_LADC	0x15
+#define WM8971_RADC	0x16
 #define	WM8971_ADCTL1	0x17
 #define	WM8971_ADCTL2	0x18
-#define WM8971_PWR1		0x19
-#define WM8971_PWR2		0x1a
+#define WM8971_PWR1	0x19
+#define WM8971_PWR2	0x1a
 #define	WM8971_ADCTL3	0x1b
 #define WM8971_ADCIN	0x1f
 #define	WM8971_LADCIN	0x20
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 5/9] Using the constraint based on wm8971_set_dai_sysclk
  2014-09-02  3:27 ` [PATCHv3 5/9] Using the constraint based on wm8971_set_dai_sysclk Xavier Hsu
@ 2014-09-02  9:28   ` Charles Keepax
  2014-09-04  9:13     ` [alsa-devel] " Xavier Hsu
  0 siblings, 1 reply; 26+ messages in thread
From: Charles Keepax @ 2014-09-02  9:28 UTC (permalink / raw)
  To: Xavier Hsu; +Cc: Andy Green, alsa-devel, patches, patches

On Tue, Sep 02, 2014 at 11:27:46AM +0800, Xavier Hsu wrote:
> This patch improves WM8971.
> We use the constraint based on the function of
> wm8971_set_dai_sysclk().
> 
> Any comments about improving the patch are welcome.
> Thanks.

Comments like this are probably best put after the --- as they
don't need to appear in the change log.

> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  sound/soc/codecs/wm8971.c |   77 +++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 71 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> index 64ed226..20cfdd3 100755
> --- a/sound/soc/codecs/wm8971.c
> +++ b/sound/soc/codecs/wm8971.c
> @@ -41,6 +41,7 @@ static struct workqueue_struct *wm8971_workq;
>  /* codec private data */
>  struct wm8971_priv {
>  	unsigned int sysclk;
> +	struct snd_pcm_hw_constraint_list *sysclk_constraints;
>  	int playback_fs;
>  	bool deemph;
>  };
> @@ -528,6 +529,53 @@ static int get_coeff(int mclk, int rate)
>  	return -EINVAL;
>  }
>  
> +/* The set of rates we can generate from the above for each SYSCLK */
> +static const unsigned int rates_12288[] = {
> +	8000, 12000, 16000, 24000, 32000, 48000, 96000
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_12288 = {
> +	.count  = ARRAY_SIZE(rates_12288),
> +	.list   = rates_12288,
> +};
> +
> +static const unsigned int rates_112896[] = {
> +	8000, 11025, 22050, 44100, 88200
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_112896 = {
> +	.count  = ARRAY_SIZE(rates_112896),
> +	.list   = rates_112896,
> +};
> +
> +static const unsigned int rates_18432[] = {
> +	8000, 12000, 16000, 24000, 32000, 48000, 96000
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_18432 = {
> +	.count  = ARRAY_SIZE(rates_18432),
> +	.list   = rates_18432,
> +};

This one is identical to the 12288 array, why not combine them
and call them the 48k array?

> +
> +static const unsigned int rates_169344[] = {
> +	8000, 11025, 22050, 44100, 88200
> +};

This one is identical to the 112896 array, why not combine them
and call them the 44k1 array?

> +
> +static struct snd_pcm_hw_constraint_list constraints_169344 = {
> +	.count  = ARRAY_SIZE(rates_169344),
> +	.list   = rates_169344,
> +};
> +
> +static const unsigned int rates_12[] = {
> +	8000, 11025, 12000, 16000, 22050, 2400, 32000, 41100, 48000,

Typo 0 missing                            ^^^^

> +	48000, 88235, 96000,
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_12 = {
> +	.count  = ARRAY_SIZE(rates_12),
> +	.list   = rates_12,
> +};
> +
>  static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>  				 int clk_id, unsigned int freq, int dir)
>  {
> @@ -535,15 +583,32 @@ static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>  	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>  
>  	switch (freq) {
> -	case 11289600:
> -	case 12000000:
>  	case 12288000:
> -	case 16934400:
> +	case 24576000:
> +		wm8971->sysclk_constraints = &constraints_12288;
> +		break;
> +	case 11289600:
> +	case 22579200:
> +		wm8971->sysclk_constraints = &constraints_112896;
> +		break;
>  	case 18432000:
> -		wm8971->sysclk = freq;
> -		return 0;
> +		wm8971->sysclk_constraints = &constraints_18432;
> +		break;
> +	case 16934400:
> +	case 33868800:
> +		wm8971->sysclk_constraints = &constraints_169344;
> +		break;
> +	case 12000000:
> +	case 24000000:
> +		wm8971->sysclk_constraints = &constraints_12;
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
> -	return -EINVAL;

You pick out the constraints here but you never actually set them
with snd_pcm_hw_constraint_list?

> +
> +	wm8971->sysclk = freq;
> +
> +	return 0;
>  }
>  
>  static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
> -- 

Thanks,
Charles

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE
  2014-09-02  3:27 ` [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE Xavier Hsu
@ 2014-09-02  9:33   ` Charles Keepax
  2014-09-02 14:56   ` Lars-Peter Clausen
  1 sibling, 0 replies; 26+ messages in thread
From: Charles Keepax @ 2014-09-02  9:33 UTC (permalink / raw)
  To: Xavier Hsu; +Cc: Andy Green, alsa-devel, patches, patches

On Tue, Sep 02, 2014 at 11:27:43AM +0800, Xavier Hsu wrote:
> This patch improves WM8971.
> We uses SOC_ENUM_SINGLE_DECL macro to
> replace SOC_ENUM_SINGLE macro.
> 
> Any comments about improving the patch are welcome.
> Thanks.
> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  sound/soc/codecs/wm8971.c |  136 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 106 insertions(+), 30 deletions(-)
>  mode change 100644 => 100755 sound/soc/codecs/wm8971.c
> 
> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> old mode 100644
> new mode 100755
> index 064278f..59c4828
> --- a/sound/soc/codecs/wm8971.c
> +++ b/sound/soc/codecs/wm8971.c
> @@ -7,6 +7,10 @@
>   *
>   * Based on wm8753.c by Liam Girdwood
>   *
> + * WM8971 Improve Copyright (C) 2014 Linaro, Ltd
> + * Author: Xavier Hsu <xavier.hsu@linaro.org>
> + *         Andy Green <andy.green@linaro.org>
> + *
>   *  This program is free software; you can redistribute  it and/or modify it
>   *  under  the terms of  the GNU General  Public License as published by the
>   *  Free Software Foundation;  either version 2 of the  License, or (at your
> @@ -36,6 +40,8 @@ static struct workqueue_struct *wm8971_workq;
>  /* codec private data */
>  struct wm8971_priv {
>  	unsigned int sysclk;
> +	int playback_fs;
> +	bool deemph;
>  };
>  
>  /*
> @@ -100,7 +106,6 @@ static const char const *wm8971_alc_func[] = {"Off", "Right",
>  					      "Left", "Stereo"};
>  static const char const *wm8971_ng_type[] = {"Constant PGA Gain",
>  					     "Mute ADC Output"};
> -static const char const *wm8971_deemp[] = {"None", "32kHz", "44.1kHz", "48kHz"};
>  static const char const *wm8971_mono_mux[] = {"Stereo", "Mono (Left)",
>  					      "Mono (Right)", "Digital Mono"};
>  static const char const *wm8971_dac_phase[] = {"Non Inverted", "Inverted"};
> @@ -114,23 +119,91 @@ static const char const *wm8971_rpga_sel[] = {"Line", "Mic", "NC",
>  					      "Differential"};
>  static const char const *wm8971_adcpol[] = {"Normal", "L Invert",
>  					    "R Invert", "L + R Invert"};
> +static const char const *wm8971_deemp[] = {"None", "32kHz",
> +					   "44.1kHz", "48kHz"};
>  
> -static const struct soc_enum wm8971_enum[] = {
> -	SOC_ENUM_SINGLE(WM8971_BASS, 7, 2, wm8971_bass),	/* 0 */
> -	SOC_ENUM_SINGLE(WM8971_BASS, 6, 2, wm8971_bass_filter),
> -	SOC_ENUM_SINGLE(WM8971_TREBLE, 6, 2, wm8971_treble),
> -	SOC_ENUM_SINGLE(WM8971_ALC1, 7, 4, wm8971_alc_func),
> -	SOC_ENUM_SINGLE(WM8971_NGATE, 1, 2, wm8971_ng_type),    /* 4 */
> -	SOC_ENUM_SINGLE(WM8971_ADCDAC, 1, 4, wm8971_deemp),
> -	SOC_ENUM_SINGLE(WM8971_ADCTL1, 4, 4, wm8971_mono_mux),
> -	SOC_ENUM_SINGLE(WM8971_ADCTL1, 1, 2, wm8971_dac_phase),
> -	SOC_ENUM_SINGLE(WM8971_LOUTM1, 0, 5, wm8971_lline_mux), /* 8 */
> -	SOC_ENUM_SINGLE(WM8971_ROUTM1, 0, 5, wm8971_rline_mux),
> -	SOC_ENUM_SINGLE(WM8971_LADCIN, 6, 4, wm8971_lpga_sel),
> -	SOC_ENUM_SINGLE(WM8971_RADCIN, 6, 4, wm8971_rpga_sel),
> -	SOC_ENUM_SINGLE(WM8971_ADCDAC, 5, 4, wm8971_adcpol),    /* 12 */
> -	SOC_ENUM_SINGLE(WM8971_ADCIN, 6, 4, wm8971_mono_mux),
> -};
> +static int wm8971_set_deemph(struct snd_soc_codec *codec)
> +{
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> +	int val = 0, i, best = 0;
> +
> +	/* If we're using deemphasis select the nearest available sample
> +	 * rate.
> +	*/
> +	if (wm8971->deemph) {
> +		best = 1;
> +		for (i = 2; i < ARRAY_SIZE(wm8971_deemp); i++) {
> +			if (abs(wm8971_deemp[i] - wm8971->playback_fs) <
> +				abs(wm8971_deemp[best] -
> +					wm8971->playback_fs))
> +				best = i;
> +		}
> +		val = best << 1;
> +	}
> +
> +	dev_dbg(codec->dev, "Set deemphasis %d (%dHz)\n",
> +		best, wm8971_deemp[best]);
> +
> +	return snd_soc_update_bits(codec, WM8971_ADCDAC, 0x6, val);
> +}
> +
> +static int wm8971_get_deemph(struct snd_kcontrol *kcontrol,
> +			     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> +
> +	ucontrol->value.enumerated.item[0] = wm8971->deemph;
> +
> +	return 0;
> +}
> +
> +static int wm8971_put_deemph(struct snd_kcontrol *kcontrol,
> +			     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> +	int deemph = ucontrol->value.enumerated.item[0];
> +	int ret = 0;
> +
> +	if (deemph > 1)
> +		return -EINVAL;
> +
> +	mutex_lock(&codec->mutex);
> +	if (wm8971->deemph != deemph) {
> +		wm8971->deemph = deemph;
> +		wm8971_set_deemph(codec);
> +
> +		ret = 1;
> +	}
> +	mutex_unlock(&codec->mutex);
> +
> +	return ret;
> +}

This deemph stuff should probably go in a seperate patch it
doesn't really match the commit log of "We uses
SOC_ENUM_SINGLE_DECL macro to replace SOC_ENUM_SINGLE macro."

> +
> +static const SOC_ENUM_SINGLE_DECL(bass_boost, WM8971_BASS, 7, wm8971_bass);
> +static const SOC_ENUM_SINGLE_DECL(bass_filter, WM8971_BASS,
> +				  6, wm8971_bass_filter);
> +static const SOC_ENUM_SINGLE_DECL(treble_cutoff, WM8971_TREBLE,
> +				  6, wm8971_treble);
> +static const SOC_ENUM_SINGLE_DECL(alc_capture_func, WM8971_ALC1,
> +				  7, wm8971_alc_func);
> +static const SOC_ENUM_SINGLE_DECL(alc_capture_ngtype, WM8971_NGATE,
> +				  1, wm8971_ng_type);
> +static const SOC_ENUM_SINGLE_DECL(dac_mono_mix, WM8971_ADCTL1,
> +				  4, wm8971_mono_mux);
> +static const SOC_ENUM_SINGLE_DECL(dac_phase_inv, WM8971_ADCTL1,
> +				  1, wm8971_dac_phase);
> +static const SOC_ENUM_SINGLE_DECL(left_line, WM8971_LOUTM1,
> +				  0, wm8971_lline_mux);
> +static const SOC_ENUM_SINGLE_DECL(right_line, WM8971_ROUTM1,
> +				  0, wm8971_rline_mux);
> +static const SOC_ENUM_SINGLE_DECL(left_pga, WM8971_LADCIN, 6, wm8971_lpga_sel);
> +static const SOC_ENUM_SINGLE_DECL(right_pga, WM8971_RADCIN,
> +				  6, wm8971_rpga_sel);
> +static const SOC_ENUM_SINGLE_DECL(capture_polarity, WM8971_ADCDAC,
> +				  5, wm8971_adcpol);
> +static const SOC_ENUM_SINGLE_DECL(monomux, WM8971_ADCIN, 6, wm8971_mono_mux);
>  
>  static const struct snd_kcontrol_new wm8971_snd_controls[] = {
>  	SOC_DOUBLE_R("Capture Volume", WM8971_LINVOL, WM8971_RINVOL, 0, 63, 0),
> @@ -158,12 +231,12 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
>  	SOC_DOUBLE_R("Speaker Playback Volume", WM8971_LOUT2V,
>  		     WM8971_ROUT2V, 0, 127, 0),
>  
> -	SOC_ENUM("Bass Boost", wm8971_enum[0]),
> -	SOC_ENUM("Bass Filter", wm8971_enum[1]),
> +	SOC_ENUM("Bass Boost", bass_boost),
> +	SOC_ENUM("Bass Filter", bass_filter),
>  	SOC_SINGLE("Bass Volume", WM8971_BASS, 0, 7, 1),
>  
>  	SOC_SINGLE("Treble Volume", WM8971_TREBLE, 0, 7, 0),
> -	SOC_ENUM("Treble Cut-off", wm8971_enum[2]),
> +	SOC_ENUM("Treble Cut-off", treble_cutoff),
>  
>  	SOC_SINGLE("Capture Filter Switch", WM8971_ADCDAC, 0, 1, 1),
>  
> @@ -172,21 +245,22 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
>  
>  	SOC_SINGLE("ALC Capture Target Volume", WM8971_ALC1, 0, 7, 0),
>  	SOC_SINGLE("ALC Capture Max Volume", WM8971_ALC1, 4, 7, 0),
> -	SOC_ENUM("ALC Capture Function", wm8971_enum[3]),
> +	SOC_ENUM("ALC Capture Function", alc_capture_func),
>  	SOC_SINGLE("ALC Capture ZC Switch", WM8971_ALC2, 7, 1, 0),
>  	SOC_SINGLE("ALC Capture Hold Time", WM8971_ALC2, 0, 15, 0),
>  	SOC_SINGLE("ALC Capture Decay Time", WM8971_ALC3, 4, 15, 0),
>  	SOC_SINGLE("ALC Capture Attack Time", WM8971_ALC3, 0, 15, 0),
>  	SOC_SINGLE("ALC Capture NG Threshold", WM8971_NGATE, 3, 31, 0),
> -	SOC_ENUM("ALC Capture NG Type", wm8971_enum[4]),
> +	SOC_ENUM("ALC Capture NG Type", alc_capture_ngtype),
>  	SOC_SINGLE("ALC Capture NG Switch", WM8971_NGATE, 0, 1, 0),
>  
>  	SOC_SINGLE("Capture 6dB Attenuate", WM8971_ADCDAC, 8, 1, 0),
>  	SOC_SINGLE("Playback 6dB Attenuate", WM8971_ADCDAC, 7, 1, 0),
>  
> -	SOC_ENUM("Playback De-emphasis", wm8971_enum[5]),
> -	SOC_ENUM("Playback Function", wm8971_enum[6]),
> -	SOC_ENUM("Playback Phase", wm8971_enum[7]),
> +	SOC_SINGLE_BOOL_EXT("Playback De-emphasis Switch", 0,
> +			    wm8971_get_deemph, wm8971_put_deemph),
> +	SOC_ENUM("Playback Function", dac_mono_mix),
> +	SOC_ENUM("Playback Phase", dac_phase_inv),
>  
>  	SOC_DOUBLE_R("Mic Boost", WM8971_LADCIN, WM8971_RADCIN, 4, 3, 0),
>  };
> @@ -221,23 +295,23 @@ SOC_DAPM_SINGLE("Right Bypass Switch", WM8971_MOUTM2, 7, 1, 0),
>  
>  /* Left Line Mux */
>  static const struct snd_kcontrol_new wm8971_left_line_controls =
> -SOC_DAPM_ENUM("Route", wm8971_enum[8]);
> +SOC_DAPM_ENUM("Route", left_line);
>  
>  /* Right Line Mux */
>  static const struct snd_kcontrol_new wm8971_right_line_controls =
> -SOC_DAPM_ENUM("Route", wm8971_enum[9]);
> +SOC_DAPM_ENUM("Route", right_line);
>  
>  /* Left PGA Mux */
>  static const struct snd_kcontrol_new wm8971_left_pga_controls =
> -SOC_DAPM_ENUM("Route", wm8971_enum[10]);
> +SOC_DAPM_ENUM("Route", left_pga);
>  
>  /* Right PGA Mux */
>  static const struct snd_kcontrol_new wm8971_right_pga_controls =
> -SOC_DAPM_ENUM("Route", wm8971_enum[11]);
> +SOC_DAPM_ENUM("Route", right_pga);
>  
>  /* Mono ADC Mux */
>  static const struct snd_kcontrol_new wm8971_monomux_controls =
> -SOC_DAPM_ENUM("Route", wm8971_enum[13]);
> +SOC_DAPM_ENUM("Route", monomux);
>  
>  static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
>  	SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
> @@ -534,6 +608,8 @@ static int wm8971_pcm_hw_params(struct snd_pcm_substream *substream,
>  		break;
>  	}
>  
> +	wm8971_set_deemph(codec);
> +
>  	/* set iface & srate */
>  	snd_soc_write(codec, WM8971_IFACE, iface);
>  	if (coeff >= 0)
> -- 

Thanks,
Charles

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 3/9] WM8971 uses TLV information
  2014-09-02  3:27 ` [PATCHv3 3/9] WM8971 uses TLV information Xavier Hsu
@ 2014-09-02  9:47   ` Charles Keepax
  0 siblings, 0 replies; 26+ messages in thread
From: Charles Keepax @ 2014-09-02  9:47 UTC (permalink / raw)
  To: Xavier Hsu; +Cc: Andy Green, alsa-devel, patches, patches

On Tue, Sep 02, 2014 at 11:27:44AM +0800, Xavier Hsu wrote:
> This patch improves WM8971.
> We use TLV information.
> 
> Any comments about improving the patch are welcome.
> Thanks.
> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 4/9] Improve wm8971_set_dai_fmt
  2014-09-02  3:27 ` [PATCHv3 4/9] Improve wm8971_set_dai_fmt Xavier Hsu
@ 2014-09-02  9:48   ` Charles Keepax
  0 siblings, 0 replies; 26+ messages in thread
From: Charles Keepax @ 2014-09-02  9:48 UTC (permalink / raw)
  To: Xavier Hsu; +Cc: Andy Green, alsa-devel, patches, patches

On Tue, Sep 02, 2014 at 11:27:45AM +0800, Xavier Hsu wrote:
> This patch improves WM8971.
> We modify the function of wm8971_set_dai_fmt().
> 
> Any comments about improving the patch are welcome.
> Thanks.
> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 6/9] WM8971 uses msleep to replace work queue
  2014-09-02  3:27 ` [PATCHv3 6/9] WM8971 uses msleep to replace work queue Xavier Hsu
@ 2014-09-02 10:00   ` Charles Keepax
  2014-09-05  3:09     ` Xavier Hsu
  0 siblings, 1 reply; 26+ messages in thread
From: Charles Keepax @ 2014-09-02 10:00 UTC (permalink / raw)
  To: Xavier Hsu; +Cc: Andy Green, alsa-devel, patches, patches

On Tue, Sep 02, 2014 at 11:27:47AM +0800, Xavier Hsu wrote:
> This patch improves WM8971.
> We use msleep to replace work queue.
> "As suggested by Mark Brown"
> 
> Any comments about improving the patch are welcome.
> Thanks.
> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  sound/soc/codecs/wm8971.c |   25 ++-----------------------
>  1 file changed, 2 insertions(+), 23 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> index 20cfdd3..68b4f52 100755
> --- a/sound/soc/codecs/wm8971.c
> +++ b/sound/soc/codecs/wm8971.c
> @@ -36,8 +36,6 @@
>  
>  #define	WM8971_REG_COUNT	43
>  
> -static struct workqueue_struct *wm8971_workq;
> -
>  /* codec private data */
>  struct wm8971_priv {
>  	unsigned int sysclk;
> @@ -774,16 +772,6 @@ static struct snd_soc_dai_driver wm8971_dai = {
>  	.ops = &wm8971_dai_ops,
>  };
>  
> -static void wm8971_work(struct work_struct *work)
> -{
> -	struct snd_soc_dapm_context *dapm =
> -		container_of(work, struct snd_soc_dapm_context,
> -			     delayed_work.work);
> -	struct snd_soc_codec *codec = dapm->codec;
> -
> -	wm8971_set_bias_level(codec, codec->dapm.bias_level);
> -}
> -
>  static int wm8971_suspend(struct snd_soc_codec *codec)
>  {
>  	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
> @@ -801,8 +789,7 @@ static int wm8971_resume(struct snd_soc_codec *codec)
>  		reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
>  		snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
>  		codec->dapm.bias_level = SND_SOC_BIAS_ON;
> -		queue_delayed_work(wm8971_workq, &codec->dapm.delayed_work,
> -				   msecs_to_jiffies(1000));
> +		msleep(100);
>  	}
>  
>  	return 0;
> @@ -813,19 +800,13 @@ static int wm8971_probe(struct snd_soc_codec *codec)
>  	int ret = 0;
>  	u16 reg;
>  
> -	INIT_DELAYED_WORK(&codec->dapm.delayed_work, wm8971_work);
> -	wm8971_workq = create_workqueue("wm8971");
> -	if (wm8971_workq == NULL)
> -		return -ENOMEM;
> -
>  	wm8971_reset(codec);
>  
>  	/* charge output caps - set vmid to 5k for quick power up */
>  	reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
>  	snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
>  	codec->dapm.bias_level = SND_SOC_BIAS_STANDBY;
> -	queue_delayed_work(wm8971_workq, &codec->dapm.delayed_work,
> -			   msecs_to_jiffies(1000));
> +	msleep(100);

Does this patch actually work? You have taken out the
queue_delayed_work and replaced it with a delay (which is 10x
shorter than the delayed work version), but you  haven't replaced
the call to wm8971_set_bias_level that used to be in the delayed
work?

>  
>  	/* set the update bits */
>  	snd_soc_update_bits(codec, WM8971_LDAC, 0x0100, 0x0100);
> @@ -846,8 +827,6 @@ static int wm8971_remove(struct snd_soc_codec *codec)
>  {
>  	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
>  
> -	if (wm8971_workq)
> -		destroy_workqueue(wm8971_workq);
>  	return 0;
>  }
>  
> -- 

Thanks,
Charles

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 7/9] WM8971 improves the function of regmap
  2014-09-02  3:27 ` [PATCHv3 7/9] WM8971 improves the function of regmap Xavier Hsu
@ 2014-09-02 11:03   ` Charles Keepax
  2014-09-03 19:19   ` Lars-Peter Clausen
  1 sibling, 0 replies; 26+ messages in thread
From: Charles Keepax @ 2014-09-02 11:03 UTC (permalink / raw)
  To: Xavier Hsu; +Cc: Andy Green, alsa-devel, patches, patches

On Tue, Sep 02, 2014 at 11:27:48AM +0800, Xavier Hsu wrote:
> This patch improves WM8971.
> We modify the function of regmap.
> 
> Any comments about improving the patch are welcome.
> Thanks.

Again move this stuff to after the ---

> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  sound/soc/codecs/wm8971.c |   26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> index 68b4f52..95e7c39 100755
> --- a/sound/soc/codecs/wm8971.c
> +++ b/sound/soc/codecs/wm8971.c
> @@ -38,6 +38,7 @@
>  
>  /* codec private data */
>  struct wm8971_priv {
> +	struct regmap *regmap;
>  	unsigned int sysclk;
>  	struct snd_pcm_hw_constraint_list *sysclk_constraints;
>  	int playback_fs;
> @@ -716,6 +717,7 @@ static int wm8971_mute(struct snd_soc_dai *dai, int mute)
>  static int wm8971_set_bias_level(struct snd_soc_codec *codec,
>  				 enum snd_soc_bias_level level)
>  {
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>  	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
>  
>  	switch (level) {
> @@ -727,7 +729,7 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
>  		break;
>  	case SND_SOC_BIAS_STANDBY:
>  		if (codec->dapm.bias_level == SND_SOC_BIAS_OFF)
> -			snd_soc_cache_sync(codec);
> +			regcache_sync(wm8971->regmap);
>  
>  		/* mute dac and set vmid to 500k, enable VREF */
>  		snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140);
> @@ -774,7 +776,11 @@ static struct snd_soc_dai_driver wm8971_dai = {
>  
>  static int wm8971_suspend(struct snd_soc_codec *codec)
>  {
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> +
>  	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
> +	regcache_mark_dirty(wm8971->regmap);
> +
>  	return 0;
>  }
>  
> @@ -797,9 +803,12 @@ static int wm8971_resume(struct snd_soc_codec *codec)
>  
>  static int wm8971_probe(struct snd_soc_codec *codec)
>  {
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>  	int ret = 0;
>  	u16 reg;
>  
> +	codec->control_data = wm8971->regmap;
> +

I don't think this is necessary anymore? Now you have the
get_regmap callback.

>  	wm8971_reset(codec);
>  
>  	/* charge output caps - set vmid to 5k for quick power up */
> @@ -830,12 +839,20 @@ static int wm8971_remove(struct snd_soc_codec *codec)
>  	return 0;
>  }
>  
> +struct regmap *wm8971_get_regmap(struct device *dev)
> +{
> +	struct wm8971_priv *priv = dev_get_drvdata(dev);
> +
> +	return priv->regmap;
> +}

This function should be marked static.

> +
>  static struct snd_soc_codec_driver soc_codec_dev_wm8971 = {
>  	.probe =	wm8971_probe,
>  	.remove =	wm8971_remove,
>  	.suspend =	wm8971_suspend,
>  	.resume =	wm8971_resume,
>  	.set_bias_level = wm8971_set_bias_level,
> +	.get_regmap =	wm8971_get_regmap,
>  
>  	.controls = wm8971_snd_controls,
>  	.num_controls = ARRAY_SIZE(wm8971_snd_controls),
> @@ -859,7 +876,6 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
>  			    const struct i2c_device_id *id)
>  {
>  	struct wm8971_priv *wm8971;
> -	struct regmap *regmap;
>  	int ret;
>  
>  	wm8971 = devm_kzalloc(&i2c->dev, sizeof(struct wm8971_priv),
> @@ -867,9 +883,9 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
>  	if (wm8971 == NULL)
>  		return -ENOMEM;
>  
> -	regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap);
> -	if (IS_ERR(regmap))
> -		return PTR_ERR(regmap);
> +	wm8971->regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap);
> +	if (IS_ERR(wm8971->regmap))
> +		return PTR_ERR(wm8971->regmap);
>  
>  	i2c_set_clientdata(i2c, wm8971);
>  
> -- 

Thanks,
Charles

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 9/9] ASOC add WM8973 support to WM8971
  2014-09-02  3:27 ` [PATCHv3 9/9] ASOC add WM8973 support to WM8971 Xavier Hsu
@ 2014-09-02 12:38   ` Charles Keepax
  0 siblings, 0 replies; 26+ messages in thread
From: Charles Keepax @ 2014-09-02 12:38 UTC (permalink / raw)
  To: Xavier Hsu; +Cc: Andy Green, alsa-devel, patches, patches

On Tue, Sep 02, 2014 at 11:27:50AM +0800, Xavier Hsu wrote:
> This patch adds support for the wm8973 codec.
> 
> Any comments about improving the patch are welcome.
> Thanks.
> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  Documentation/devicetree/bindings/sound/wm8971.txt |   26 ++
>  sound/soc/codecs/wm8971.c                          |  326 +++++++++++++++-----
>  sound/soc/codecs/wm8971.h                          |   21 +-
>  3 files changed, 294 insertions(+), 79 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/sound/wm8971.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/wm8971.txt b/Documentation/devicetree/bindings/sound/wm8971.txt
> new file mode 100644
> index 0000000..7313dd1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/wm8971.txt
> @@ -0,0 +1,26 @@
> +WM8971 and WM8973 audio CODEC
> +
> +These devices support both I2C and SPI (configured with pin strapping
> +on the board).
> +
> +Required properties:
> +
> +  - compatible : "wlf,wm8971".
> +		 "wlf,wm8973".
> +
> +  - reg : the I2C address of the device for I2C, the chip select
> +	  number for SPI.
> +
> +Optional properties:
> +
> +  - mclk-div : Setting the CLKDIV2 bit for dividing MCLK.
> +    mclk-div = <0> (Default & not divide).
> +    mclk-div = <1> (Divide by 2).
> +
> +Example:
> +
> +codec: wm8973@1a {
> +	compatible = "wlf,wm8973";
> +	reg = <0x1a>;
> +	mclk-div = <1>;
> +};
> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> index 6d34565..2893fc6 100755
> --- a/sound/soc/codecs/wm8971.c
> +++ b/sound/soc/codecs/wm8971.c
> @@ -7,7 +7,7 @@
>   *
>   * Based on wm8753.c by Liam Girdwood
>   *
> - * WM8971 Improve Copyright (C) 2014 Linaro, Ltd
> + * WM8971 Improve & Wm8973 Support Copyright (C) 2014 Linaro, Ltd
>   * Author: Xavier Hsu <xavier.hsu@linaro.org>
>   *         Andy Green <andy.green@linaro.org>
>   *
> @@ -34,15 +34,27 @@
>  
>  #include "wm8971.h"
>  
> +enum variant {
> +	VARIANT_WM8971,
> +	VARIANT_WM8973
> +};
> +
>  #define	WM8971_REG_COUNT	43
>  
>  /* codec private data */
>  struct wm8971_priv {
>  	struct regmap *regmap;
> +	enum variant variant;
>  	unsigned int sysclk;
>  	struct snd_pcm_hw_constraint_list *sysclk_constraints;
>  	int playback_fs;
>  	bool deemph;
> +	int mclk_div;
> +};
> +
> +static const u16 charge[][2] = {
> +	[VARIANT_WM8971] = { 0xfe3e, 0x01c0 },
> +	[VARIANT_WM8973] = { 0x003e, 0x01f0 },
>  };

Feels like this might be better as a structure so you can give
the two numbers a meaningful name.

>  
>  /*
> @@ -98,7 +110,7 @@ static const struct reg_default wm8971_reg_defaults[] = {
>  
>  #define wm8971_reset(c)	snd_soc_write(c, WM8971_RESET, 0)
>  
> -/* WM8971 Controls */
> +/* WM8971 / 3 Controls */
>  static const char const *wm8971_bass[] = {"Linear Control", "Adaptive Boost"};
>  static const char const *wm8971_bass_filter[] = {"130Hz @ 48kHz",
>  						 "200Hz @ 48kHz"};
> @@ -110,19 +122,33 @@ static const char const *wm8971_ng_type[] = {"Constant PGA Gain",
>  static const char const *wm8971_mono_mux[] = {"Stereo", "Mono (Left)",
>  					      "Mono (Right)", "Digital Mono"};
>  static const char const *wm8971_dac_phase[] = {"Non Inverted", "Inverted"};
> -static const char const *wm8971_lline_mux[] = {"Line", "NC", "NC",
> -					       "PGA", "Differential"};
> -static const char const *wm8971_rline_mux[] = {"Line", "Mic", "NC",
> -					       "PGA", "Differential"};
> -static const char const *wm8971_lpga_sel[] = {"Line", "NC", "NC",
> -					      "Differential"};
> -static const char const *wm8971_rpga_sel[] = {"Line", "Mic", "NC",
> -					      "Differential"};
>  static const char const *wm8971_adcpol[] = {"Normal", "L Invert",
>  					    "R Invert", "L + R Invert"};
>  static const char const *wm8971_deemp[] = {"None", "32kHz",
>  					   "44.1kHz", "48kHz"};
>  
> +/* WM8971-only*/
> +static const char const *wm8971_lline_mux[] = {"Line 1", "NC", "NC",
> +					       "PGA", "Differential"};
> +static const char const *wm8971_rline_mux[] = {"Line 1", "Mic", "NC",
> +					       "PGA", "Differential"};
> +static const char const *wm8971_lpga_sel[] = {"Line 1", "NC", "NC",
> +					      "Differential"};
> +static const char const *wm8971_rpga_sel[] = {"Line 1", "Mic", "NC",
> +					      "Differential"};
> +
> +/* WM8973-only*/
> +static const char const *wm8973_line_mux[] = {"Line 1", "Line 2", "Line 3",
> +					      "PGA", "Differential"};
> +static const char const *wm8973_pga_sel[] = {"Line 1", "Line 2", "Line 3",
> +					     "Differential"};
> +static const char const *wm8973_out3[] = {"VREF", "ROUT1 + Vol", "MonoOut",
> +					  "ROUT1"};
> +static const char const *wm8973_diff_sel[] = {"Line 1", "Line 2"};
> +static const char const *wm8973_3d_lc[] = { "200Hz", "500Hz" };
> +static const char const *wm8973_3d_uc[] = { "2.2kHz", "1.5kHz" };
> +static const char const *wm8973_3d_func[] = {"Capture", "Playback"};
> +
>  static int wm8971_set_deemph(struct snd_soc_codec *codec)
>  {
>  	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> @@ -182,6 +208,7 @@ static int wm8971_put_deemph(struct snd_kcontrol *kcontrol,
>  	return ret;
>  }
>  
> +/* WM8971 / 3 Controls */
>  static const SOC_ENUM_SINGLE_DECL(bass_boost, WM8971_BASS, 7, wm8971_bass);
>  static const SOC_ENUM_SINGLE_DECL(bass_filter, WM8971_BASS,
>  				  6, wm8971_bass_filter);
> @@ -195,17 +222,39 @@ static const SOC_ENUM_SINGLE_DECL(dac_mono_mix, WM8971_ADCTL1,
>  				  4, wm8971_mono_mux);
>  static const SOC_ENUM_SINGLE_DECL(dac_phase_inv, WM8971_ADCTL1,
>  				  1, wm8971_dac_phase);
> -static const SOC_ENUM_SINGLE_DECL(left_line, WM8971_LOUTM1,
> -				  0, wm8971_lline_mux);
> -static const SOC_ENUM_SINGLE_DECL(right_line, WM8971_ROUTM1,
> -				  0, wm8971_rline_mux);
> -static const SOC_ENUM_SINGLE_DECL(left_pga, WM8971_LADCIN, 6, wm8971_lpga_sel);
> -static const SOC_ENUM_SINGLE_DECL(right_pga, WM8971_RADCIN,
> -				  6, wm8971_rpga_sel);
>  static const SOC_ENUM_SINGLE_DECL(capture_polarity, WM8971_ADCDAC,
>  				  5, wm8971_adcpol);
>  static const SOC_ENUM_SINGLE_DECL(monomux, WM8971_ADCIN, 6, wm8971_mono_mux);
>  
> +/* WM8971-only */
> +static const SOC_ENUM_SINGLE_DECL(wm8971_left_line, WM8971_LOUTM1,
> +				  0, wm8971_lline_mux);
> +static const SOC_ENUM_SINGLE_DECL(wm8971_right_line, WM8971_ROUTM1,
> +				  0, wm8971_rline_mux);
> +static const SOC_ENUM_SINGLE_DECL(wm8971_left_pga, WM8971_LADCIN,
> +				  6, wm8971_lpga_sel);
> +static const SOC_ENUM_SINGLE_DECL(wm8971_right_pga, WM8971_RADCIN,
> +				  6, wm8971_rpga_sel);
> +
> +/* WM8973-only */
> +static const SOC_ENUM_SINGLE_DECL(wm8973_left_line, WM8971_LOUTM1,
> +				  0, wm8973_line_mux);
> +static const SOC_ENUM_SINGLE_DECL(wm8973_right_line, WM8971_ROUTM1,
> +				  0, wm8973_line_mux);
> +static const SOC_ENUM_SINGLE_DECL(wm8973_left_pga, WM8971_LADCIN,
> +				  6, wm8973_pga_sel);
> +static const SOC_ENUM_SINGLE_DECL(wm8973_right_pga, WM8971_RADCIN,
> +				  6, wm8973_pga_sel);
> +static const SOC_ENUM_SINGLE_DECL(wm8973_out3_control, WM8971_ADCTL2,
> +				  7, wm8973_out3);
> +static const SOC_ENUM_SINGLE_DECL(wm8973_diffmux, WM8971_ADCIN,
> +				  8, wm8973_diff_sel);
> +static const SOC_ENUM_SINGLE_DECL(wm8973_lower_cutoff, WM8973_3D,
> +				  5, wm8973_3d_lc);
> +static const SOC_ENUM_SINGLE_DECL(wm8973_upper_cutoff, WM8973_3D,
> +				  6, wm8973_3d_uc);
> +static const SOC_ENUM_SINGLE_DECL(wm8973_mode, WM8973_3D, 7, wm8973_3d_func);
> +
>  static const DECLARE_TLV_DB_SCALE(in_vol, -1725, 75, 0);
>  static const DECLARE_TLV_DB_SCALE(out_vol, -6700, 91, 0);
>  static const DECLARE_TLV_DB_SCALE(attenuate_6db, -600, 600, 0);
> @@ -292,6 +341,15 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
>  	SOC_SINGLE("Headphone Switch EN", WM8971_ADCTL2, 6, 1, 0),
>  };
>  
> +static const struct snd_kcontrol_new wm8973_snd_controls[] = {
> +	/* 3D Control */
> +	SOC_SINGLE("3D Switch", WM8973_3D, 0, 1, 0),
> +	SOC_SINGLE("3D Volume", WM8973_3D, 1, 15, 0),
> +	SOC_ENUM("3D Lower Cut-off", wm8973_lower_cutoff),
> +	SOC_ENUM("3D Upper Cut-off", wm8973_upper_cutoff),
> +	SOC_ENUM("3D Mode", wm8973_mode),
> +};
> +
>  /*
>   * DAPM Controls
>   */
> @@ -322,25 +380,41 @@ SOC_DAPM_SINGLE("Right Bypass Switch", WM8971_MOUTM2, 7, 1, 0),
>  
>  /* Left Line Mux */
>  static const struct snd_kcontrol_new wm8971_left_line_controls =
> -SOC_DAPM_ENUM("Route", left_line);
> +SOC_DAPM_ENUM("Route", wm8971_left_line);
> +static const struct snd_kcontrol_new wm8973_left_line_controls =
> +SOC_DAPM_ENUM("Route", wm8973_left_line);
>  
>  /* Right Line Mux */
>  static const struct snd_kcontrol_new wm8971_right_line_controls =
> -SOC_DAPM_ENUM("Route", right_line);
> +SOC_DAPM_ENUM("Route", wm8971_right_line);
> +static const struct snd_kcontrol_new wm8973_right_line_controls =
> +SOC_DAPM_ENUM("Route", wm8973_right_line);
>  
>  /* Left PGA Mux */
>  static const struct snd_kcontrol_new wm8971_left_pga_controls =
> -SOC_DAPM_ENUM("Route", left_pga);
> +SOC_DAPM_ENUM("Route", wm8971_left_pga);
> +static const struct snd_kcontrol_new wm8973_left_pga_controls =
> +SOC_DAPM_ENUM("Route", wm8973_left_pga);
>  
>  /* Right PGA Mux */
>  static const struct snd_kcontrol_new wm8971_right_pga_controls =
> -SOC_DAPM_ENUM("Route", right_pga);
> +SOC_DAPM_ENUM("Route", wm8971_right_pga);
> +static const struct snd_kcontrol_new wm8973_right_pga_controls =
> +SOC_DAPM_ENUM("Route", wm8973_right_pga);
> +
> +/* Out 3 Mux */
> +static const struct snd_kcontrol_new wm8973_out3_controls =
> +SOC_DAPM_ENUM("Route", wm8973_out3_control);
> +
> +/* Differential Mux */
> +static const struct snd_kcontrol_new wm8973_diffmux_controls =
> +SOC_DAPM_ENUM("Route", wm8973_diffmux);
>  
>  /* Mono ADC Mux */
>  static const struct snd_kcontrol_new wm8971_monomux_controls =
>  SOC_DAPM_ENUM("Route", monomux);
>  
> -static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
> +static const struct snd_soc_dapm_widget wm897x_dapm_widgets[] = {
>  	SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
>  			   &wm8971_left_mixer_controls[0],
>  			   ARRAY_SIZE(wm8971_left_mixer_controls)),
> @@ -363,15 +437,6 @@ static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
>  	SND_SOC_DAPM_ADC("Right ADC", "Right Capture", WM8971_PWR1, 2, 0),
>  	SND_SOC_DAPM_ADC("Left ADC", "Left Capture", WM8971_PWR1, 3, 0),
>  
> -	SND_SOC_DAPM_MUX("Left PGA Mux", WM8971_PWR1, 5, 0,
> -			 &wm8971_left_pga_controls),
> -	SND_SOC_DAPM_MUX("Right PGA Mux", WM8971_PWR1, 4, 0,
> -			 &wm8971_right_pga_controls),
> -	SND_SOC_DAPM_MUX("Left Line Mux", SND_SOC_NOPM, 0, 0,
> -			 &wm8971_left_line_controls),
> -	SND_SOC_DAPM_MUX("Right Line Mux", SND_SOC_NOPM, 0, 0,
> -			 &wm8971_right_line_controls),
> -
>  	SND_SOC_DAPM_MUX("Left ADC Mux", SND_SOC_NOPM, 0, 0,
>  			 &wm8971_monomux_controls),
>  	SND_SOC_DAPM_MUX("Right ADC Mux", SND_SOC_NOPM, 0, 0,
> @@ -388,6 +453,41 @@ static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
>  	SND_SOC_DAPM_INPUT("MIC"),
>  };
>  
> +static const struct snd_soc_dapm_widget wm8971_dapm_widgets[] = {
> +	SND_SOC_DAPM_MUX("Left Line Mux", SND_SOC_NOPM, 0, 0,
> +			 &wm8971_left_line_controls),
> +	SND_SOC_DAPM_MUX("Right Line Mux", SND_SOC_NOPM, 0, 0,
> +			 &wm8971_right_line_controls),
> +	SND_SOC_DAPM_MUX("Left PGA Mux", WM8971_PWR1, 5, 0,
> +			 &wm8971_left_pga_controls),
> +	SND_SOC_DAPM_MUX("Right PGA Mux", WM8971_PWR1, 4, 0,
> +			 &wm8971_right_pga_controls),
> +};
> +
> +static const struct snd_soc_dapm_widget wm8973_dapm_widgets[] = {
> +	SND_SOC_DAPM_MUX("Left Line Mux", SND_SOC_NOPM, 0, 0,
> +			 &wm8973_left_line_controls),
> +	SND_SOC_DAPM_MUX("Right Line Mux", SND_SOC_NOPM, 0, 0,
> +			 &wm8973_right_line_controls),
> +	SND_SOC_DAPM_MUX("Right PGA Mux", WM8971_PWR1, 4, 0,
> +			 &wm8973_right_pga_controls),
> +	SND_SOC_DAPM_MUX("Left PGA Mux", WM8971_PWR1, 5, 0,
> +			 &wm8973_left_pga_controls),
> +	SND_SOC_DAPM_MUX("Out3 Mux", SND_SOC_NOPM, 0, 0,
> +			 &wm8973_out3_controls),
> +	SND_SOC_DAPM_PGA("Out 3", WM8971_PWR2, 1, 0, NULL, 0),
> +	SND_SOC_DAPM_MUX("Differential Mux", SND_SOC_NOPM, 0, 0,
> +			 &wm8973_diffmux_controls),
> +
> +	SND_SOC_DAPM_OUTPUT("OUT3"),
> +	SND_SOC_DAPM_OUTPUT("VREF"),
> +
> +	SND_SOC_DAPM_INPUT("LINPUT2"),
> +	SND_SOC_DAPM_INPUT("LINPUT3"),
> +	SND_SOC_DAPM_INPUT("RINPUT2"),
> +	SND_SOC_DAPM_INPUT("RINPUT3"),
> +};
> +
>  static const struct snd_soc_dapm_route wm8971_dapm_routes[] = {
>  	/* left mixer */
>  	{"Left Mixer", "Playback Switch", "Left DAC"},
> @@ -428,27 +528,27 @@ static const struct snd_soc_dapm_route wm8971_dapm_routes[] = {
>  	{"MONO1", NULL, "Mono Out"},
>  
>  	/* Left Line Mux */
> -	{"Left Line Mux", "Line", "LINPUT1"},
> +	{"Left Line Mux", "Line 1", "LINPUT1"},
>  	{"Left Line Mux", "PGA", "Left PGA Mux"},
>  	{"Left Line Mux", "Differential", "Differential Mux"},
>  
>  	/* Right Line Mux */
> -	{"Right Line Mux", "Line", "RINPUT1"},
> +	{"Right Line Mux", "Line 1", "RINPUT1"},
>  	{"Right Line Mux", "Mic", "MIC"},
>  	{"Right Line Mux", "PGA", "Right PGA Mux"},
>  	{"Right Line Mux", "Differential", "Differential Mux"},
>  
>  	/* Left PGA Mux */
> -	{"Left PGA Mux", "Line", "LINPUT1"},
> +	{"Left PGA Mux", "Line 1", "LINPUT1"},
>  	{"Left PGA Mux", "Differential", "Differential Mux"},
>  
>  	/* Right PGA Mux */
> -	{"Right PGA Mux", "Line", "RINPUT1"},
> +	{"Right PGA Mux", "Line 1", "RINPUT1"},
>  	{"Right PGA Mux", "Differential", "Differential Mux"},
>  
>  	/* Differential Mux */
> -	{"Differential Mux", "Line", "LINPUT1"},
> -	{"Differential Mux", "Line", "RINPUT1"},
> +	{"Differential Mux", "Line 1", "LINPUT1"},
> +	{"Differential Mux", "Line 1", "RINPUT1"},
>  
>  	/* Left ADC Mux */
>  	{"Left ADC Mux", "Stereo", "Left PGA Mux"},
> @@ -465,6 +565,62 @@ static const struct snd_soc_dapm_route wm8971_dapm_routes[] = {
>  	{"Right ADC", NULL, "Right ADC Mux"},
>  };
>  
> +static const struct snd_soc_dapm_route wm8973_dapm_routes[] = {
> +	{"Out3 Mux", "VREF", "VREF"},
> +	{"Out3 Mux", "ROUT1 + Vol", "ROUT1"},
> +	{"Out3 Mux", "ROUT1", "Right Mixer"},
> +	{"Out3 Mux", "MonoOut", "MONO1"},
> +	{"Out 3", NULL, "Out3 Mux"},
> +	{"OUT3", NULL, "Out 3"},
> +
> +	{"Left Line Mux", "Line 2", "LINPUT2"},
> +	{"Left Line Mux", "Line 3", "LINPUT3"},
> +
> +	{"Right Line Mux", "Line 2", "RINPUT2"},
> +	{"Right Line Mux", "Line 3", "RINPUT3"},
> +
> +	{"Left PGA Mux", "Line 2", "LINPUT2"},
> +	{"Left PGA Mux", "Line 3", "LINPUT3"},
> +
> +	{"Right PGA Mux", "Line 2", "RINPUT2"},
> +	{"Right PGA Mux", "Line 3", "RINPUT3"},
> +
> +	{"Differential Mux", "Line 2", "LINPUT2"},
> +	{"Differential Mux", "Line 2", "RINPUT2"},
> +};
> +
> +static int wm897x_add_controls(struct snd_soc_codec *codec)
> +{
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> +
> +	snd_soc_add_codec_controls(codec, wm8971_snd_controls,
> +				   ARRAY_SIZE(wm8971_snd_controls));

You can still use the entries in snd_soc_codec_driver for the
shared functionality, then just add the additional stuff here.

> +
> +	if (wm8971->variant)
> +		snd_soc_add_codec_controls(codec, wm8973_snd_controls,
> +					   ARRAY_SIZE(wm8973_snd_controls));
> +
> +	snd_soc_dapm_new_controls(dapm, wm897x_dapm_widgets,
> +				  ARRAY_SIZE(wm897x_dapm_widgets));
> +
> +	if (wm8971->variant)
> +		snd_soc_dapm_new_controls(dapm, wm8973_dapm_widgets,
> +					  ARRAY_SIZE(wm8973_dapm_widgets));
> +	else
> +		snd_soc_dapm_new_controls(dapm, wm8971_dapm_widgets,
> +					  ARRAY_SIZE(wm8971_dapm_widgets));
> +
> +	snd_soc_dapm_add_routes(dapm, wm8971_dapm_routes,
> +				ARRAY_SIZE(wm8971_dapm_routes));
> +
> +	if (wm8971->variant)
> +		snd_soc_dapm_add_routes(dapm, wm8973_dapm_routes,
> +					ARRAY_SIZE(wm8973_dapm_routes));
> +
> +	return 0;
> +}
> +
>  struct _coeff_div {
>  	u32 mclk;
>  	u32 rate;
> @@ -724,12 +880,18 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
>  				 enum snd_soc_bias_level level)
>  {
>  	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> -	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
> +	static const u16 mask[][2] = {
> +		[VARIANT_WM8971] = { 0x00c1, 0x0140 },
> +		[VARIANT_WM8973] = { 0x00c2, 0x0141 },
> +	};

Again here perhaps a struct to give a bit more context to the
numbers.

> +	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1)
> +				   & charge[wm8971->variant][0];
>  
>  	switch (level) {
>  	case SND_SOC_BIAS_ON:
>  		/* set vmid to 50k and unmute dac */
> -		snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x00c1);
> +		snd_soc_write(codec, WM8971_PWR1,
> +			      pwr_reg | mask[wm8971->variant][0]);
>  		break;
>  	case SND_SOC_BIAS_PREPARE:
>  		break;
> @@ -738,7 +900,8 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
>  			regcache_sync(wm8971->regmap);
>  
>  		/* mute dac and set vmid to 500k, enable VREF */
> -		snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140);
> +		snd_soc_write(codec, WM8971_PWR1,
> +			      pwr_reg | mask[wm8971->variant][1]);
>  		break;
>  	case SND_SOC_BIAS_OFF:
>  		snd_soc_write(codec, WM8971_PWR1, 0x0001);
> @@ -763,21 +926,28 @@ static const struct snd_soc_dai_ops wm8971_dai_ops = {
>  	.set_sysclk	= wm8971_set_dai_sysclk,
>  };
>  
> -static struct snd_soc_dai_driver wm8971_dai = {
> -	.name = "wm8971-hifi",
> -	.playback = {
> -		.stream_name = "Playback",
> -		.channels_min = 1,
> -		.channels_max = 2,
> -		.rates = WM8971_RATES,
> -		.formats = WM8971_FORMATS,},
> -	.capture = {
> -		.stream_name = "Capture",
> -		.channels_min = 1,
> -		.channels_max = 2,
> -		.rates = WM8971_RATES,
> -		.formats = WM8971_FORMATS,},
> -	.ops = &wm8971_dai_ops,
> +static struct snd_soc_dai_driver wm8971_dai[] = {
> +	{
> +		.name = "wm8971-hifi-playback",
> +		.playback = {
> +			.stream_name = "Playback",
> +			.channels_min = 1,
> +			.channels_max = 2,
> +			.rates = WM8971_RATES,
> +			.formats = WM8971_FORMATS,
> +		},
> +		.ops = &wm8971_dai_ops,
> +	}, {
> +		.name = "wm8971-hifi-capture",
> +		.capture = {
> +			.stream_name = "Capture",
> +			.channels_min = 1,
> +			.channels_max = 2,
> +			.rates = WM8971_RATES,
> +			.formats = WM8971_FORMATS,
> +		},
> +		.ops = &wm8971_dai_ops,
> +	},
>  };
>  
>  static int wm8971_suspend(struct snd_soc_codec *codec)
> @@ -792,14 +962,17 @@ static int wm8971_suspend(struct snd_soc_codec *codec)
>  
>  static int wm8971_resume(struct snd_soc_codec *codec)
>  {
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>  	u16 reg;
>  
>  	wm8971_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
>  
>  	/* charge wm8971 caps */
>  	if (codec->dapm.suspend_bias_level == SND_SOC_BIAS_ON) {
> -		reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
> -		snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
> +		reg = snd_soc_read(codec, WM8971_PWR1)
> +				   & charge[wm8971->variant][0];
> +		snd_soc_write(codec, WM8971_PWR1,
> +			      reg | charge[wm8971->variant][1]);
>  		codec->dapm.bias_level = SND_SOC_BIAS_ON;
>  		msleep(100);
>  	}
> @@ -810,16 +983,25 @@ static int wm8971_resume(struct snd_soc_codec *codec)
>  static int wm8971_probe(struct snd_soc_codec *codec)
>  {
>  	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> +	const u32 *p;
>  	int ret = 0;
>  	u16 reg;
>  
>  	codec->control_data = wm8971->regmap;
>  
>  	wm8971_reset(codec);
> +	if (codec->dev->of_node) {
> +		p = of_get_property(codec->dev->of_node, "mclk-div", NULL);
> +		if (p)
> +			wm8971->mclk_div = be32_to_cpu(*p);
> +	}
> +	/* Master Clock Divide by 2 (0 = not div, 1 = div by 2) */
> +	if (wm8971->mclk_div)
> +		snd_soc_update_bits(codec, WM8971_SRATE, 0x0040, 0x0040);
>  
>  	/* charge output caps - set vmid to 5k for quick power up */
> -	reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
> -	snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
> +	reg = snd_soc_read(codec, WM8971_PWR1) & charge[wm8971->variant][0];
> +	snd_soc_write(codec, WM8971_PWR1, reg | charge[wm8971->variant][1]);
>  	codec->dapm.bias_level = SND_SOC_BIAS_STANDBY;
>  	msleep(100);
>  
> @@ -833,6 +1015,8 @@ static int wm8971_probe(struct snd_soc_codec *codec)
>  	snd_soc_update_bits(codec, WM8971_LINVOL, 0x0100, 0x0100);
>  	snd_soc_update_bits(codec, WM8971_RINVOL, 0x0100, 0x0100);
>  
> +	wm897x_add_controls(codec);
> +
>  	return ret;
>  }
>  
> @@ -841,7 +1025,6 @@ static int wm8971_probe(struct snd_soc_codec *codec)
>  static int wm8971_remove(struct snd_soc_codec *codec)
>  {
>  	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
> -
>  	return 0;
>  }
>  
> @@ -859,13 +1042,6 @@ static struct snd_soc_codec_driver soc_codec_dev_wm8971 = {
>  	.resume =	wm8971_resume,
>  	.set_bias_level = wm8971_set_bias_level,
>  	.get_regmap =	wm8971_get_regmap,
> -
> -	.controls = wm8971_snd_controls,
> -	.num_controls = ARRAY_SIZE(wm8971_snd_controls),
> -	.dapm_widgets = wm8971_dapm_widgets,
> -	.num_dapm_widgets = ARRAY_SIZE(wm8971_dapm_widgets),
> -	.dapm_routes = wm8971_dapm_routes,
> -	.num_dapm_routes = ARRAY_SIZE(wm8971_dapm_routes),
>  };
>  
>  static const struct regmap_config wm8971_regmap = {
> @@ -889,6 +1065,8 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
>  	if (wm8971 == NULL)
>  		return -ENOMEM;
>  
> +	wm8971->variant = (int)id->driver_data;
> +

Does this still work in the device tree case?

>  	wm8971->regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap);
>  	if (IS_ERR(wm8971->regmap))
>  		return PTR_ERR(wm8971->regmap);
> @@ -896,7 +1074,7 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
>  	i2c_set_clientdata(i2c, wm8971);
>  
>  	ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_wm8971,
> -				     &wm8971_dai, 1);
> +				     wm8971_dai, ARRAY_SIZE(wm8971_dai));
>  
>  	return ret;
>  }
> @@ -908,15 +1086,23 @@ static int wm8971_i2c_remove(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id wm8971_i2c_id[] = {
> -	{ "wm8971", 0 },
> +	{ "wm8971", VARIANT_WM8971 },
> +	{ "wm8973", VARIANT_WM8973 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, wm8971_i2c_id);
>  
> +static const struct of_device_id wm897x_dt_ids[] = {
> +	{ .compatible = "wlf,wm8971" },
> +	{ .compatible = "wlf,wm8973" },
> +	{ /* sentinel */ }
> +};
> +
>  static struct i2c_driver wm8971_i2c_driver = {
>  	.driver = {
>  		.name = "wm8971",
>  		.owner = THIS_MODULE,
> +		.of_match_table = wm897x_dt_ids,
>  	},
>  	.probe =    wm8971_i2c_probe,
>  	.remove =   wm8971_i2c_remove,
> @@ -928,3 +1114,5 @@ module_i2c_driver(wm8971_i2c_driver);
>  MODULE_DESCRIPTION("ASoC WM8971 driver");
>  MODULE_AUTHOR("Lab126");
>  MODULE_LICENSE("GPL");
> +MODULE_ALIAS("*wm8971*");
> +MODULE_ALIAS("*wm8973*");
> diff --git a/sound/soc/codecs/wm8971.h b/sound/soc/codecs/wm8971.h
> index f31c38f..aeab04a 100644
> --- a/sound/soc/codecs/wm8971.h
> +++ b/sound/soc/codecs/wm8971.h
> @@ -22,21 +22,22 @@
>  #define WM8971_ADCDAC	0x05
>  #define WM8971_IFACE	0x07
>  #define WM8971_SRATE	0x08
> -#define WM8971_LDAC		0x0a
> -#define WM8971_RDAC		0x0b
> -#define WM8971_BASS		0x0c
> +#define WM8971_LDAC	0x0a
> +#define WM8971_RDAC	0x0b
> +#define WM8971_BASS	0x0c
>  #define WM8971_TREBLE	0x0d
>  #define WM8971_RESET	0x0f
> -#define WM8971_ALC1		0x11
> -#define	WM8971_ALC2		0x12
> -#define	WM8971_ALC3		0x13
> +#define WM8973_3D       0x10
> +#define WM8971_ALC1	0x11
> +#define	WM8971_ALC2	0x12
> +#define	WM8971_ALC3	0x13
>  #define WM8971_NGATE	0x14
> -#define WM8971_LADC		0x15
> -#define WM8971_RADC		0x16
> +#define WM8971_LADC	0x15
> +#define WM8971_RADC	0x16
>  #define	WM8971_ADCTL1	0x17
>  #define	WM8971_ADCTL2	0x18
> -#define WM8971_PWR1		0x19
> -#define WM8971_PWR2		0x1a
> +#define WM8971_PWR1	0x19
> +#define WM8971_PWR2	0x1a
>  #define	WM8971_ADCTL3	0x1b
>  #define WM8971_ADCIN	0x1f
>  #define	WM8971_LADCIN	0x20
> -- 

Thanks,
Charles

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 8/9] WM8971 adds kcontrol functions
  2014-09-02  3:27 ` [PATCHv3 8/9] WM8971 adds kcontrol functions Xavier Hsu
@ 2014-09-02 12:41   ` Charles Keepax
  2014-09-11  3:35     ` Xavier Hsu
  0 siblings, 1 reply; 26+ messages in thread
From: Charles Keepax @ 2014-09-02 12:41 UTC (permalink / raw)
  To: Xavier Hsu; +Cc: Andy Green, alsa-devel, patches, patches

On Tue, Sep 02, 2014 at 11:27:49AM +0800, Xavier Hsu wrote:
> This patch improves WM8971.
> We add kcontrol functions on WM8971.
> 
> Any comments about improving the patch are welcome.
> Thanks.
> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  sound/soc/codecs/wm8971.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> index 95e7c39..6d34565 100755
> --- a/sound/soc/codecs/wm8971.c
> +++ b/sound/soc/codecs/wm8971.c
> @@ -284,6 +284,12 @@ static const struct snd_kcontrol_new wm8971_snd_controls[] = {
>  
>  	SOC_DOUBLE_R_TLV("ADC Volume", WM8971_LADC, WM8971_RADC,
>  			 0, 255, 0, adc_vol),
> +
> +	SOC_SINGLE("Analogue Bias", WM8971_ADCTL1, 6, 3, 0),

Given this relates to the AVDD voltage feels it is very unlikely
to change at runtime. Do we expect it to? Otherwise it might be
better added as a DT entry.

> +	SOC_SINGLE("Right Speaker Playback Invert Switch", WM8971_ADCTL2,
> +		   4, 1, 0),
> +	SOC_SINGLE("Headphone Switch POL", WM8971_ADCTL2, 5, 1, 0),

Again the polarity of the headphone detect is not going to change
at runtime so this should probably be a DT entry.

> +	SOC_SINGLE("Headphone Switch EN", WM8971_ADCTL2, 6, 1, 0),

I am less clear on whether or not this should be a control or a
DT entry, so as long as you are happy this is consistent with the
usage.

>  };
>  
>  /*
> -- 

Thanks,
Charles

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE
  2014-09-02  3:27 ` [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE Xavier Hsu
  2014-09-02  9:33   ` Charles Keepax
@ 2014-09-02 14:56   ` Lars-Peter Clausen
  2014-09-04  3:53     ` Xavier Hsu
  1 sibling, 1 reply; 26+ messages in thread
From: Lars-Peter Clausen @ 2014-09-02 14:56 UTC (permalink / raw)
  To: Xavier Hsu, alsa-devel, patches, patches; +Cc: Andy Green

On 09/02/2014 05:27 AM, Xavier Hsu wrote:
[...]
> +static int wm8971_put_deemph(struct snd_kcontrol *kcontrol,
> +			     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> +	int deemph = ucontrol->value.enumerated.item[0];
> +	int ret = 0;
> +
> +	if (deemph > 1)
> +		return -EINVAL;
> +
> +	mutex_lock(&codec->mutex);

Please use a driver local mutex. The snd_soc_codec mutex is going to be 
removed sooner or later and we shouldn't add new users.

> +	if (wm8971->deemph != deemph) {
> +		wm8971->deemph = deemph;
> +		wm8971_set_deemph(codec);
> +
> +		ret = 1;
> +	}
> +	mutex_unlock(&codec->mutex);
> +
> +	return ret;
> +}

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 7/9] WM8971 improves the function of regmap
  2014-09-02  3:27 ` [PATCHv3 7/9] WM8971 improves the function of regmap Xavier Hsu
  2014-09-02 11:03   ` Charles Keepax
@ 2014-09-03 19:19   ` Lars-Peter Clausen
  2014-09-11  3:21     ` Xavier Hsu
  1 sibling, 1 reply; 26+ messages in thread
From: Lars-Peter Clausen @ 2014-09-03 19:19 UTC (permalink / raw)
  To: Xavier Hsu, alsa-devel, patches, patches; +Cc: Andy Green

On 09/02/2014 05:27 AM, Xavier Hsu wrote:
> This patch improves WM8971.
> We modify the function of regmap.
>
> Any comments about improving the patch are welcome.
> Thanks.
>
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>   sound/soc/codecs/wm8971.c |   26 +++++++++++++++++++++-----
>   1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> index 68b4f52..95e7c39 100755
> --- a/sound/soc/codecs/wm8971.c
> +++ b/sound/soc/codecs/wm8971.c
> @@ -38,6 +38,7 @@
>
>   /* codec private data */
>   struct wm8971_priv {
> +	struct regmap *regmap;
>   	unsigned int sysclk;
>   	struct snd_pcm_hw_constraint_list *sysclk_constraints;
>   	int playback_fs;
> @@ -716,6 +717,7 @@ static int wm8971_mute(struct snd_soc_dai *dai, int mute)
>   static int wm8971_set_bias_level(struct snd_soc_codec *codec,
>   				 enum snd_soc_bias_level level)
>   {
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>   	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
>
>   	switch (level) {
> @@ -727,7 +729,7 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
>   		break;
>   	case SND_SOC_BIAS_STANDBY:
>   		if (codec->dapm.bias_level == SND_SOC_BIAS_OFF)
> -			snd_soc_cache_sync(codec);
> +			regcache_sync(wm8971->regmap);

This is a bugfix and it should be noted in the commit message, what is 
broken, why it is broken and since when it has been broken.

>
>   		/* mute dac and set vmid to 500k, enable VREF */
>   		snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140);
> @@ -774,7 +776,11 @@ static struct snd_soc_dai_driver wm8971_dai = {
>
>   static int wm8971_suspend(struct snd_soc_codec *codec)
>   {
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> +
>   	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
> +	regcache_mark_dirty(wm8971->regmap);

The core already marks the regcache dirty after calling the suspend 
callback, no need to do this from hand.

> +
>   	return 0;
>   }
>
> @@ -797,9 +803,12 @@ static int wm8971_resume(struct snd_soc_codec *codec)
>
>   static int wm8971_probe(struct snd_soc_codec *codec)
>   {
> +	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>   	int ret = 0;
>   	u16 reg;
>
> +	codec->control_data = wm8971->regmap;
> +
>   	wm8971_reset(codec);
>
>   	/* charge output caps - set vmid to 5k for quick power up */
> @@ -830,12 +839,20 @@ static int wm8971_remove(struct snd_soc_codec *codec)
>   	return 0;
>   }
>
> +struct regmap *wm8971_get_regmap(struct device *dev)
> +{
> +	struct wm8971_priv *priv = dev_get_drvdata(dev);
> +
> +	return priv->regmap;
> +}

This is unnecessary, since the regmap has been registered with the device of 
the CODEC the core is able to figure out the correct regmap itself.

[...]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE
  2014-09-02 14:56   ` Lars-Peter Clausen
@ 2014-09-04  3:53     ` Xavier Hsu
  0 siblings, 0 replies; 26+ messages in thread
From: Xavier Hsu @ 2014-09-04  3:53 UTC (permalink / raw)
  To: Lars-Peter Clausen, Charles Keepax
  Cc: Andy Green, alsa-devel, patches, Patch Tracking

Hi Charles and Lars-Peter :
Thanks for your feedback.
I will split into two patch (Modify De-emphasis and Using SOC_ENUM_SINGLE_DECL
macro to replace SOC_ENUM_SINGLE macro) and use local mutex to replace the
snd_soc_codec mutex.

Thanks.

BR,
Xavier


2014-09-02 22:56 GMT+08:00 Lars-Peter Clausen <lars@metafoo.de>:

> On 09/02/2014 05:27 AM, Xavier Hsu wrote:
> [...]
>
>  +static int wm8971_put_deemph(struct snd_kcontrol *kcontrol,
>> +                            struct snd_ctl_elem_value *ucontrol)
>> +{
>> +       struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
>> +       struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>> +       int deemph = ucontrol->value.enumerated.item[0];
>> +       int ret = 0;
>> +
>> +       if (deemph > 1)
>> +               return -EINVAL;
>> +
>> +       mutex_lock(&codec->mutex);
>>
>
> Please use a driver local mutex. The snd_soc_codec mutex is going to be
> removed sooner or later and we shouldn't add new users.
>
>
>  +       if (wm8971->deemph != deemph) {
>> +               wm8971->deemph = deemph;
>> +               wm8971_set_deemph(codec);
>> +
>> +               ret = 1;
>> +       }
>> +       mutex_unlock(&codec->mutex);
>> +
>> +       return ret;
>> +}
>>
>
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [alsa-devel] [PATCHv3 5/9] Using the constraint based on wm8971_set_dai_sysclk
  2014-09-02  9:28   ` Charles Keepax
@ 2014-09-04  9:13     ` Xavier Hsu
  0 siblings, 0 replies; 26+ messages in thread
From: Xavier Hsu @ 2014-09-04  9:13 UTC (permalink / raw)
  To: Charles Keepax; +Cc: Andy Green, alsa-devel, patches, Patch Tracking

Hi Charles :
Thanks for yours feedback.

According your suggestion, I combine the redundant codes and fix some
mistakes.
(rates_12288[] and rates_18432[]      => rates_48000[]
 rates_112896[] and rates_169344[]  => rates_44100[]
 2400 => 24000
)
I also add snd_pcm_hw_constraint_list() to restrict sysclk.

Thanks.

BR,
Xavier


2014-09-02 17:28 GMT+08:00 Charles Keepax <
ckeepax@opensource.wolfsonmicro.com>:

> On Tue, Sep 02, 2014 at 11:27:46AM +0800, Xavier Hsu wrote:
> > This patch improves WM8971.
> > We use the constraint based on the function of
> > wm8971_set_dai_sysclk().
> >
> > Any comments about improving the patch are welcome.
> > Thanks.
>
> Comments like this are probably best put after the --- as they
> don't need to appear in the change log.
>
> >
> > Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> > Signed-off-by: Andy Green <andy.green@linaro.org>
> > ---
> >  sound/soc/codecs/wm8971.c |   77
> +++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 71 insertions(+), 6 deletions(-)
> >
> > diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> > index 64ed226..20cfdd3 100755
> > --- a/sound/soc/codecs/wm8971.c
> > +++ b/sound/soc/codecs/wm8971.c
> > @@ -41,6 +41,7 @@ static struct workqueue_struct *wm8971_workq;
> >  /* codec private data */
> >  struct wm8971_priv {
> >       unsigned int sysclk;
> > +     struct snd_pcm_hw_constraint_list *sysclk_constraints;
> >       int playback_fs;
> >       bool deemph;
> >  };
> > @@ -528,6 +529,53 @@ static int get_coeff(int mclk, int rate)
> >       return -EINVAL;
> >  }
> >
> > +/* The set of rates we can generate from the above for each SYSCLK */
> > +static const unsigned int rates_12288[] = {
> > +     8000, 12000, 16000, 24000, 32000, 48000, 96000
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_12288 = {
> > +     .count  = ARRAY_SIZE(rates_12288),
> > +     .list   = rates_12288,
> > +};
> > +
> > +static const unsigned int rates_112896[] = {
> > +     8000, 11025, 22050, 44100, 88200
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_112896 = {
> > +     .count  = ARRAY_SIZE(rates_112896),
> > +     .list   = rates_112896,
> > +};
> > +
> > +static const unsigned int rates_18432[] = {
> > +     8000, 12000, 16000, 24000, 32000, 48000, 96000
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_18432 = {
> > +     .count  = ARRAY_SIZE(rates_18432),
> > +     .list   = rates_18432,
> > +};
>
> This one is identical to the 12288 array, why not combine them
> and call them the 48k array?
>
> > +
> > +static const unsigned int rates_169344[] = {
> > +     8000, 11025, 22050, 44100, 88200
> > +};
>
> This one is identical to the 112896 array, why not combine them
> and call them the 44k1 array?
>
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_169344 = {
> > +     .count  = ARRAY_SIZE(rates_169344),
> > +     .list   = rates_169344,
> > +};
> > +
> > +static const unsigned int rates_12[] = {
> > +     8000, 11025, 12000, 16000, 22050, 2400, 32000, 41100, 48000,
>
> Typo 0 missing                            ^^^^
>
> > +     48000, 88235, 96000,
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_12 = {
> > +     .count  = ARRAY_SIZE(rates_12),
> > +     .list   = rates_12,
> > +};
> > +
> >  static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> >                                int clk_id, unsigned int freq, int dir)
> >  {
> > @@ -535,15 +583,32 @@ static int wm8971_set_dai_sysclk(struct
> snd_soc_dai *codec_dai,
> >       struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> >
> >       switch (freq) {
> > -     case 11289600:
> > -     case 12000000:
> >       case 12288000:
> > -     case 16934400:
> > +     case 24576000:
> > +             wm8971->sysclk_constraints = &constraints_12288;
> > +             break;
> > +     case 11289600:
> > +     case 22579200:
> > +             wm8971->sysclk_constraints = &constraints_112896;
> > +             break;
> >       case 18432000:
> > -             wm8971->sysclk = freq;
> > -             return 0;
> > +             wm8971->sysclk_constraints = &constraints_18432;
> > +             break;
> > +     case 16934400:
> > +     case 33868800:
> > +             wm8971->sysclk_constraints = &constraints_169344;
> > +             break;
> > +     case 12000000:
> > +     case 24000000:
> > +             wm8971->sysclk_constraints = &constraints_12;
> > +             break;
> > +     default:
> > +             return -EINVAL;
> >       }
> > -     return -EINVAL;
>
> You pick out the constraints here but you never actually set them
> with snd_pcm_hw_constraint_list?
>
> > +
> > +     wm8971->sysclk = freq;
> > +
> > +     return 0;
> >  }
> >
> >  static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
> > --
>
> Thanks,
> Charles
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 6/9] WM8971 uses msleep to replace work queue
  2014-09-02 10:00   ` Charles Keepax
@ 2014-09-05  3:09     ` Xavier Hsu
  0 siblings, 0 replies; 26+ messages in thread
From: Xavier Hsu @ 2014-09-05  3:09 UTC (permalink / raw)
  To: Charles Keepax; +Cc: Andy Green, alsa-devel, patches, Patch Tracking

Hi Charles :
This device driver (we use wm8973) can normally run on our platform.
Anyway, I modify some part of our device driver.
 (msleep(100) => msleep(1000))
 (Before msleep, it will call wm8971_set_bias_level(codec,
codec->dapm.bias_level) first)

BR,
Xavier


2014-09-02 18:00 GMT+08:00 Charles Keepax <
ckeepax@opensource.wolfsonmicro.com>:

> On Tue, Sep 02, 2014 at 11:27:47AM +0800, Xavier Hsu wrote:
> > This patch improves WM8971.
> > We use msleep to replace work queue.
> > "As suggested by Mark Brown"
> >
> > Any comments about improving the patch are welcome.
> > Thanks.
> >
> > Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> > Signed-off-by: Andy Green <andy.green@linaro.org>
> > ---
> >  sound/soc/codecs/wm8971.c |   25 ++-----------------------
> >  1 file changed, 2 insertions(+), 23 deletions(-)
> >
> > diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> > index 20cfdd3..68b4f52 100755
> > --- a/sound/soc/codecs/wm8971.c
> > +++ b/sound/soc/codecs/wm8971.c
> > @@ -36,8 +36,6 @@
> >
> >  #define      WM8971_REG_COUNT        43
> >
> > -static struct workqueue_struct *wm8971_workq;
> > -
> >  /* codec private data */
> >  struct wm8971_priv {
> >       unsigned int sysclk;
> > @@ -774,16 +772,6 @@ static struct snd_soc_dai_driver wm8971_dai = {
> >       .ops = &wm8971_dai_ops,
> >  };
> >
> > -static void wm8971_work(struct work_struct *work)
> > -{
> > -     struct snd_soc_dapm_context *dapm =
> > -             container_of(work, struct snd_soc_dapm_context,
> > -                          delayed_work.work);
> > -     struct snd_soc_codec *codec = dapm->codec;
> > -
> > -     wm8971_set_bias_level(codec, codec->dapm.bias_level);
> > -}
> > -
> >  static int wm8971_suspend(struct snd_soc_codec *codec)
> >  {
> >       wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
> > @@ -801,8 +789,7 @@ static int wm8971_resume(struct snd_soc_codec *codec)
> >               reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
> >               snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
> >               codec->dapm.bias_level = SND_SOC_BIAS_ON;
> > -             queue_delayed_work(wm8971_workq, &codec->dapm.delayed_work,
> > -                                msecs_to_jiffies(1000));
> > +             msleep(100);
> >       }
> >
> >       return 0;
> > @@ -813,19 +800,13 @@ static int wm8971_probe(struct snd_soc_codec
> *codec)
> >       int ret = 0;
> >       u16 reg;
> >
> > -     INIT_DELAYED_WORK(&codec->dapm.delayed_work, wm8971_work);
> > -     wm8971_workq = create_workqueue("wm8971");
> > -     if (wm8971_workq == NULL)
> > -             return -ENOMEM;
> > -
> >       wm8971_reset(codec);
> >
> >       /* charge output caps - set vmid to 5k for quick power up */
> >       reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
> >       snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0);
> >       codec->dapm.bias_level = SND_SOC_BIAS_STANDBY;
> > -     queue_delayed_work(wm8971_workq, &codec->dapm.delayed_work,
> > -                        msecs_to_jiffies(1000));
> > +     msleep(100);
>
> Does this patch actually work? You have taken out the
> queue_delayed_work and replaced it with a delay (which is 10x
> shorter than the delayed work version), but you  haven't replaced
> the call to wm8971_set_bias_level that used to be in the delayed
> work?
>
> >
> >       /* set the update bits */
> >       snd_soc_update_bits(codec, WM8971_LDAC, 0x0100, 0x0100);
> > @@ -846,8 +827,6 @@ static int wm8971_remove(struct snd_soc_codec *codec)
> >  {
> >       wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
> >
> > -     if (wm8971_workq)
> > -             destroy_workqueue(wm8971_workq);
> >       return 0;
> >  }
> >
> > --
>
> Thanks,
> Charles
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 7/9] WM8971 improves the function of regmap
  2014-09-03 19:19   ` Lars-Peter Clausen
@ 2014-09-11  3:21     ` Xavier Hsu
  2014-09-11  7:06       ` Lars-Peter Clausen
  0 siblings, 1 reply; 26+ messages in thread
From: Xavier Hsu @ 2014-09-11  3:21 UTC (permalink / raw)
  To: Lars-Peter Clausen, Charles Keepax
  Cc: Andy Green, alsa-devel, patches, Patch Tracking

Hi Charles and Lars-Peter:
Thanks for your feedback.
I will modify our patch as below.
1. Using (codec->control_data = wm8971->regmap;) or (wm8971_get_regmap) to
register regmap.
2. Modifying our patch the content of commit.

Thanks again.

BR,
Xavier

2014-09-04 3:19 GMT+08:00 Lars-Peter Clausen <lars@metafoo.de>:

> On 09/02/2014 05:27 AM, Xavier Hsu wrote:
>
>> This patch improves WM8971.
>> We modify the function of regmap.
>>
>> Any comments about improving the patch are welcome.
>> Thanks.
>>
>> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
>> Signed-off-by: Andy Green <andy.green@linaro.org>
>> ---
>>   sound/soc/codecs/wm8971.c |   26 +++++++++++++++++++++-----
>>   1 file changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
>> index 68b4f52..95e7c39 100755
>> --- a/sound/soc/codecs/wm8971.c
>> +++ b/sound/soc/codecs/wm8971.c
>> @@ -38,6 +38,7 @@
>>
>>   /* codec private data */
>>   struct wm8971_priv {
>> +       struct regmap *regmap;
>>         unsigned int sysclk;
>>         struct snd_pcm_hw_constraint_list *sysclk_constraints;
>>         int playback_fs;
>> @@ -716,6 +717,7 @@ static int wm8971_mute(struct snd_soc_dai *dai, int
>> mute)
>>   static int wm8971_set_bias_level(struct snd_soc_codec *codec,
>>                                  enum snd_soc_bias_level level)
>>   {
>> +       struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>>         u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
>>
>>         switch (level) {
>> @@ -727,7 +729,7 @@ static int wm8971_set_bias_level(struct snd_soc_codec
>> *codec,
>>                 break;
>>         case SND_SOC_BIAS_STANDBY:
>>                 if (codec->dapm.bias_level == SND_SOC_BIAS_OFF)
>> -                       snd_soc_cache_sync(codec);
>> +                       regcache_sync(wm8971->regmap);
>>
>
> This is a bugfix and it should be noted in the commit message, what is
> broken, why it is broken and since when it has been broken.
>
>
>>                 /* mute dac and set vmid to 500k, enable VREF */
>>                 snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140);
>> @@ -774,7 +776,11 @@ static struct snd_soc_dai_driver wm8971_dai = {
>>
>>   static int wm8971_suspend(struct snd_soc_codec *codec)
>>   {
>> +       struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>> +
>>         wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
>> +       regcache_mark_dirty(wm8971->regmap);
>>
>
> The core already marks the regcache dirty after calling the suspend
> callback, no need to do this from hand.
>
>  +
>>         return 0;
>>   }
>>
>> @@ -797,9 +803,12 @@ static int wm8971_resume(struct snd_soc_codec *codec)
>>
>>   static int wm8971_probe(struct snd_soc_codec *codec)
>>   {
>> +       struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>>         int ret = 0;
>>         u16 reg;
>>
>> +       codec->control_data = wm8971->regmap;
>> +
>>         wm8971_reset(codec);
>>
>>         /* charge output caps - set vmid to 5k for quick power up */
>> @@ -830,12 +839,20 @@ static int wm8971_remove(struct snd_soc_codec
>> *codec)
>>         return 0;
>>   }
>>
>> +struct regmap *wm8971_get_regmap(struct device *dev)
>> +{
>> +       struct wm8971_priv *priv = dev_get_drvdata(dev);
>> +
>> +       return priv->regmap;
>> +}
>>
>
> This is unnecessary, since the regmap has been registered with the device
> of the CODEC the core is able to figure out the correct regmap itself.
>
> [...]
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 8/9] WM8971 adds kcontrol functions
  2014-09-02 12:41   ` Charles Keepax
@ 2014-09-11  3:35     ` Xavier Hsu
  0 siblings, 0 replies; 26+ messages in thread
From: Xavier Hsu @ 2014-09-11  3:35 UTC (permalink / raw)
  To: Charles Keepax; +Cc: Andy Green, alsa-devel, patches, Patch Tracking

Hi Charles :
Thanks for your feedback.
I will set these registers through DT.

BR,
Xavier

2014-09-02 20:41 GMT+08:00 Charles Keepax <
ckeepax@opensource.wolfsonmicro.com>:

> On Tue, Sep 02, 2014 at 11:27:49AM +0800, Xavier Hsu wrote:
> > This patch improves WM8971.
> > We add kcontrol functions on WM8971.
> >
> > Any comments about improving the patch are welcome.
> > Thanks.
> >
> > Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> > Signed-off-by: Andy Green <andy.green@linaro.org>
> > ---
> >  sound/soc/codecs/wm8971.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> > index 95e7c39..6d34565 100755
> > --- a/sound/soc/codecs/wm8971.c
> > +++ b/sound/soc/codecs/wm8971.c
> > @@ -284,6 +284,12 @@ static const struct snd_kcontrol_new
> wm8971_snd_controls[] = {
> >
> >       SOC_DOUBLE_R_TLV("ADC Volume", WM8971_LADC, WM8971_RADC,
> >                        0, 255, 0, adc_vol),
> > +
> > +     SOC_SINGLE("Analogue Bias", WM8971_ADCTL1, 6, 3, 0),
>
> Given this relates to the AVDD voltage feels it is very unlikely
> to change at runtime. Do we expect it to? Otherwise it might be
> better added as a DT entry.
>
> > +     SOC_SINGLE("Right Speaker Playback Invert Switch", WM8971_ADCTL2,
> > +                4, 1, 0),
> > +     SOC_SINGLE("Headphone Switch POL", WM8971_ADCTL2, 5, 1, 0),
>
> Again the polarity of the headphone detect is not going to change
> at runtime so this should probably be a DT entry.
>
> > +     SOC_SINGLE("Headphone Switch EN", WM8971_ADCTL2, 6, 1, 0),
>
> I am less clear on whether or not this should be a control or a
> DT entry, so as long as you are happy this is consistent with the
> usage.
>
> >  };
> >
> >  /*
> > --
>
> Thanks,
> Charles
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 7/9] WM8971 improves the function of regmap
  2014-09-11  3:21     ` Xavier Hsu
@ 2014-09-11  7:06       ` Lars-Peter Clausen
  0 siblings, 0 replies; 26+ messages in thread
From: Lars-Peter Clausen @ 2014-09-11  7:06 UTC (permalink / raw)
  To: Xavier Hsu, Charles Keepax
  Cc: alsa-devel, Andy Green, patches, Patch Tracking

On 09/11/2014 05:21 AM, Xavier Hsu wrote:
> Hi Charles and Lars-Peter:
> Thanks for your feedback.
> I will modify our patch as below.
> 1. Using (codec->control_data = wm8971->regmap;) or (wm8971_get_regmap) to
> register regmap.

You should use neither. The ASoC core is able to get the regmap instance by 
calling dev_get_regmap(), no changes are necessary to the driver.

- Lars

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCHv3 7/9] WM8971 improves the function of regmap
  2014-09-02  3:30 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
@ 2014-09-02  3:30 ` Xavier Hsu
  0 siblings, 0 replies; 26+ messages in thread
From: Xavier Hsu @ 2014-09-02  3:30 UTC (permalink / raw)
  To: alsa-devel, patches, patches; +Cc: Xavier Hsu, ckeepax, lars, Andy Green

This patch improves WM8971.
We modify the function of regmap.

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |   26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 68b4f52..95e7c39 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -38,6 +38,7 @@
 
 /* codec private data */
 struct wm8971_priv {
+	struct regmap *regmap;
 	unsigned int sysclk;
 	struct snd_pcm_hw_constraint_list *sysclk_constraints;
 	int playback_fs;
@@ -716,6 +717,7 @@ static int wm8971_mute(struct snd_soc_dai *dai, int mute)
 static int wm8971_set_bias_level(struct snd_soc_codec *codec,
 				 enum snd_soc_bias_level level)
 {
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
 	u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;
 
 	switch (level) {
@@ -727,7 +729,7 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec,
 		break;
 	case SND_SOC_BIAS_STANDBY:
 		if (codec->dapm.bias_level == SND_SOC_BIAS_OFF)
-			snd_soc_cache_sync(codec);
+			regcache_sync(wm8971->regmap);
 
 		/* mute dac and set vmid to 500k, enable VREF */
 		snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140);
@@ -774,7 +776,11 @@ static struct snd_soc_dai_driver wm8971_dai = {
 
 static int wm8971_suspend(struct snd_soc_codec *codec)
 {
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
+
 	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
+	regcache_mark_dirty(wm8971->regmap);
+
 	return 0;
 }
 
@@ -797,9 +803,12 @@ static int wm8971_resume(struct snd_soc_codec *codec)
 
 static int wm8971_probe(struct snd_soc_codec *codec)
 {
+	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
 	int ret = 0;
 	u16 reg;
 
+	codec->control_data = wm8971->regmap;
+
 	wm8971_reset(codec);
 
 	/* charge output caps - set vmid to 5k for quick power up */
@@ -830,12 +839,20 @@ static int wm8971_remove(struct snd_soc_codec *codec)
 	return 0;
 }
 
+struct regmap *wm8971_get_regmap(struct device *dev)
+{
+	struct wm8971_priv *priv = dev_get_drvdata(dev);
+
+	return priv->regmap;
+}
+
 static struct snd_soc_codec_driver soc_codec_dev_wm8971 = {
 	.probe =	wm8971_probe,
 	.remove =	wm8971_remove,
 	.suspend =	wm8971_suspend,
 	.resume =	wm8971_resume,
 	.set_bias_level = wm8971_set_bias_level,
+	.get_regmap =	wm8971_get_regmap,
 
 	.controls = wm8971_snd_controls,
 	.num_controls = ARRAY_SIZE(wm8971_snd_controls),
@@ -859,7 +876,6 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
 	struct wm8971_priv *wm8971;
-	struct regmap *regmap;
 	int ret;
 
 	wm8971 = devm_kzalloc(&i2c->dev, sizeof(struct wm8971_priv),
@@ -867,9 +883,9 @@ static int wm8971_i2c_probe(struct i2c_client *i2c,
 	if (wm8971 == NULL)
 		return -ENOMEM;
 
-	regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
+	wm8971->regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap);
+	if (IS_ERR(wm8971->regmap))
+		return PTR_ERR(wm8971->regmap);
 
 	i2c_set_clientdata(i2c, wm8971);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2014-09-11  7:06 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-02  3:27 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
2014-09-02  3:27 ` [PATCHv3 2/9] WM8971 uses SOC_ENUM_SINGLE_DECL to replace SOC_ENUM_SINGLE Xavier Hsu
2014-09-02  9:33   ` Charles Keepax
2014-09-02 14:56   ` Lars-Peter Clausen
2014-09-04  3:53     ` Xavier Hsu
2014-09-02  3:27 ` [PATCHv3 3/9] WM8971 uses TLV information Xavier Hsu
2014-09-02  9:47   ` Charles Keepax
2014-09-02  3:27 ` [PATCHv3 4/9] Improve wm8971_set_dai_fmt Xavier Hsu
2014-09-02  9:48   ` Charles Keepax
2014-09-02  3:27 ` [PATCHv3 5/9] Using the constraint based on wm8971_set_dai_sysclk Xavier Hsu
2014-09-02  9:28   ` Charles Keepax
2014-09-04  9:13     ` [alsa-devel] " Xavier Hsu
2014-09-02  3:27 ` [PATCHv3 6/9] WM8971 uses msleep to replace work queue Xavier Hsu
2014-09-02 10:00   ` Charles Keepax
2014-09-05  3:09     ` Xavier Hsu
2014-09-02  3:27 ` [PATCHv3 7/9] WM8971 improves the function of regmap Xavier Hsu
2014-09-02 11:03   ` Charles Keepax
2014-09-03 19:19   ` Lars-Peter Clausen
2014-09-11  3:21     ` Xavier Hsu
2014-09-11  7:06       ` Lars-Peter Clausen
2014-09-02  3:27 ` [PATCHv3 8/9] WM8971 adds kcontrol functions Xavier Hsu
2014-09-02 12:41   ` Charles Keepax
2014-09-11  3:35     ` Xavier Hsu
2014-09-02  3:27 ` [PATCHv3 9/9] ASOC add WM8973 support to WM8971 Xavier Hsu
2014-09-02 12:38   ` Charles Keepax
2014-09-02  3:30 [PATCHv3 1/9] Clean WM8971 through checkpatch Xavier Hsu
2014-09-02  3:30 ` [PATCHv3 7/9] WM8971 improves the function of regmap Xavier Hsu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.