Skip to contentSkip to conversation

ASoc: wm8731: add 32bit mode and normal mode with 12MHz XTAL

PATCH In review Matt Flax 3 participants 3 messages Jun 9, 2016

Subsystems (3)
lore
Share & tools
lore

Matt Flax

author
#0 lore raw

PATCH 1/1 ASoc: wm8731: add 32bit mode and normal mode with 12MHz XTAL

This patch adds the 32 bit capability and updates the WM8731_IFACE as required.

This patch also allows the machine to set normal mode when using a 12 MHz crystal. In USB_MODE the 12 MHz crystal signal is output on the BCLK pin. In NORMAL_MODE the bit clock is output on the BCLK pin. The previously ignored direction variable input to the wm8731_set_dai_sysclk function is used to indicate whether crystal or bit clock is output on the BCLK codec pin.
I have ensured that this does not effect the db1200.c machine driver (the only other driver to use a 12 MHz crystal in USB_MODE). It also does not effect the other machine drivers which use the wm8731 codec as they don't use USB_MODE nor 12 MHz crystals.

---
 sound/soc/codecs/wm8731.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
sound/soc/codecs/wm8731.c +8 -3
index 4bcf5f8..b155cf6 100644
@@ -5,6 +5,7 @@
  * Copyright 2006-12 Wolfson Microelectronics, plc
  *
  * Author: Richard Purdie <[email protected]>
+ * Author: Matt Flax <[email protected]> 12Mhz XTAL normal mode and 32bit mode.
  *
  * Based on wm8753.c by Liam Girdwood
  *
@@ -53,6 +54,7 @@ struct wm8731_priv {
 	int sysclk_type;
 	int playback_fs;
 	bool deemph;
+	bool bclk_12_mhz;
  	struct mutex lock;
 };
@@ -192,7 +194,7 @@ static const struct snd_kcontrol_new wm8731_input_mux_controls =
 SOC_DAPM_ENUM("Input Select", wm8731_insel_enum);
  static const struct snd_soc_dapm_widget wm8731_dapm_widgets[] = {
-SND_SOC_DAPM_SUPPLY("ACTIVE",WM8731_ACTIVE, 0, 0, NULL, 0),
+SND_SOC_DAPM_SUPPLY("ACTIVE", WM8731_ACTIVE, 0, 0, NULL, 0),
 SND_SOC_DAPM_SUPPLY("OSC", WM8731_PWR, 5, 1, NULL, 0),
 SND_SOC_DAPM_MIXER("Output Mixer", WM8731_PWR, 4, 1,
 	&wm8731_output_mixer_controls[0],
@@ -341,8 +343,11 @@ static int wm8731_hw_params(struct snd_pcm_substream *substream,
 	struct wm8731_priv *wm8731 = snd_soc_codec_get_drvdata(codec);
 	u16 iface = snd_soc_read(codec, WM8731_IFACE) & 0xfff3;
 	int i = get_coeff(wm8731->sysclk, params_rate(params));
-	u16 srate = (coeff_div[i].sr << 2) |
-		(coeff_div[i].bosr << 1) | coeff_div[i].usb;
+	u16 srate = (coeff_div[i].sr << 2) | (coeff_div[i].bosr << 1);
+
+	/* this determines whether to output bit or crystal clk */
+	if (wm8731->bclk_12_mhz)
+		srate |= coeff_div[i].usb;
  	wm8731->playback_fs = params_rate(params);
 @@ -358,6 +363,9 @@ static int wm8731_hw_params(struct 

snd_pcm_substream *substream,
case 24:
iface |= 0x0008;
break;
+ case 32:
+ iface |= 0x000c;
+ break;
}
wm8731_set_deemph(codec);
@@ -417,6 +425,11 @@ static int wm8731_set_dai_sysclk(struct snd_soc_dai *codec_dai,
wm8731->sysclk = freq;
+ if (dir == SND_SOC_CLOCK_IN) /* output the 12 MHz clock */
+ wm8731->bclk_12_mhz = 1;
+ else /* SND_SOC_CLOCK_OUT, output the bit clock */
+ wm8731->bclk_12_mhz = 0;
+
snd_soc_dapm_sync(dapm);
return 0;
@@ -541,7 +554,7 @@ static int wm8731_startup(struct snd_pcm_substream *substream,
#define WM8731_RATES SNDRV_PCM_RATE_8000_96000
#define WM8731_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |
SNDRV_PCM_FMTBIT_S20_3LE |\
- SNDRV_PCM_FMTBIT_S24_LE)
+ SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
static const struct snd_soc_dai_ops wm8731_dai_ops = {
.startup = wm8731_startup,

Signature · 1 line
2.5.0
#1 lore raw
Quoted 13 lines · Matt Flax

This patch adds the 32 bit capability and updates the WM8731_IFACE as required.

This patch also allows the machine to set normal mode when using a 12 MHz crystal. In USB_MODE the 12 MHz crystal signal is output on the BCLK pin. In NORMAL_MODE the bit clock is output on the BCLK pin. The previously ignored direction variable input to the wm8731_set_dai_sysclk function is used to indicate whether crystal or bit clock is output on the BCLK codec pin.
I have ensured that this does not effect the db1200.c machine driver (the only other driver to use a 12 MHz crystal in USB_MODE). It also does not effect the other machine drivers which use the wm8731 codec as they don't use USB_MODE nor 12 MHz crystals.

Probably best to split these into two patches, I know they are both simple changes but they appear unrelated and if they arn't you probably need to explain why they need to go in a single patch.

Quoted 15 lines
---
 sound/soc/codecs/wm8731.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
sound/soc/codecs/wm8731.c +1 -0
index 4bcf5f8..b155cf6 100644
@@ -5,6 +5,7 @@
  * Copyright 2006-12 Wolfson Microelectronics, plc
  *
  * Author: Richard Purdie <[email protected]>
+ * Author: Matt Flax <[email protected]> 12Mhz XTAL normal mode and 32bit
Quoted 1 line

mode.

Are you sure you mail setup is good here? A few points in the patch it looks like lines have been line wrapped. Git won't apply the patches if they have been tampered with by your email client. A good way to test this is to email the patch to yourself first and then try to apply it yourself before you send it to the list.

Quoted 14 lines

*
* Based on wm8753.c by Liam Girdwood
*

sound/soc/codecs/wm8731.c +1 -0
@@ -53,6 +54,7 @@ struct wm8731_priv {
 	int sysclk_type;
 	int playback_fs;
 	bool deemph;
+	bool bclk_12_mhz;
  	struct mutex lock;
 };
@@ -192,7 +194,7 @@ static const struct snd_kcontrol_new

wm8731_input_mux_controls =
SOC_DAPM_ENUM("Input Select", wm8731_insel_enum);
static const struct snd_soc_dapm_widget wm8731_dapm_widgets[] = {

sound/soc/codecs/wm8731.c static const struct snd_kcontrol_new wm8731_input_mux_controls = · line 196

Quoted 2 lines
sound/soc/codecs/wm8731.c +1 -1
-SND_SOC_DAPM_SUPPLY("ACTIVE",WM8731_ACTIVE, 0, 0, NULL, 0),
+SND_SOC_DAPM_SUPPLY("ACTIVE", WM8731_ACTIVE, 0, 0, NULL, 0),

Again probably better to put this into a seperate change it has nothing to do with the rest of the patch.

Quoted 47 lines

SND_SOC_DAPM_SUPPLY("OSC", WM8731_PWR, 5, 1, NULL, 0), SND_SOC_DAPM_MIXER("Output Mixer", WM8731_PWR, 4, 1,

sound/soc/codecs/wm8731.c +5 -2
 	&wm8731_output_mixer_controls[0],
@@ -341,8 +343,11 @@ static int wm8731_hw_params(struct snd_pcm_substream
 *substream,
 	struct wm8731_priv *wm8731 = snd_soc_codec_get_drvdata(codec);
 	u16 iface = snd_soc_read(codec, WM8731_IFACE) & 0xfff3;
 	int i = get_coeff(wm8731->sysclk, params_rate(params));
-	u16 srate = (coeff_div[i].sr << 2) |
-		(coeff_div[i].bosr << 1) | coeff_div[i].usb;
+	u16 srate = (coeff_div[i].sr << 2) | (coeff_div[i].bosr << 1);
+
+	/* this determines whether to output bit or crystal clk */
+	if (wm8731->bclk_12_mhz)
+		srate |= coeff_div[i].usb;
  	wm8731->playback_fs = params_rate(params);
 @@ -358,6 +363,9 @@ static int wm8731_hw_params(struct snd_pcm_substream

*substream,

sound/soc/codecs/wm8731.c +8 -1
 	case 24:
 		iface |= 0x0008;
 		break;
+	case 32:
+		iface |= 0x000c;
+		break;
 	}
  	wm8731_set_deemph(codec);
@@ -417,6 +425,11 @@ static int wm8731_set_dai_sysclk(struct snd_soc_dai
 *codec_dai,
  	wm8731->sysclk = freq;
 +	if (dir == SND_SOC_CLOCK_IN) /* output the 12 MHz clock */
+		wm8731->bclk_12_mhz = 1;
+	else /* SND_SOC_CLOCK_OUT, output the bit clock */
+		wm8731->bclk_12_mhz = 0;
+
 	snd_soc_dapm_sync(dapm);
  	return 0;
@@ -541,7 +554,7 @@ static int wm8731_startup(struct snd_pcm_substream
 *substream,
 #define WM8731_RATES SNDRV_PCM_RATE_8000_96000
  #define WM8731_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE
 |\
-	SNDRV_PCM_FMTBIT_S24_LE)
+	SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
  static const struct snd_soc_dai_ops wm8731_dai_ops = {
 	.startup	= wm8731_startup,
Signature · 1 line
2.5.0

Thanks,
Charles

maintainer
#2 lore raw
Quoted 11 lines · Charles Keepax
Quoted 4 lines · Matt Flax

I have ensured that this does not effect the db1200.c machine driver (the only other driver to use a 12 MHz crystal in USB_MODE). It also does not effect the other machine drivers which use the wm8731 codec as they don't use USB_MODE nor 12 MHz crystals.

Probably best to split these into two patches, I know they are both simple changes but they appear unrelated and if they arn't you probably need to explain why they need to go in a single patch.

Yes.