Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* AT32AP7000 (AVR32) + WM8510
@ 2008-05-22 16:20 Geoffrey Wossum
  2008-05-22 19:46 ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Geoffrey Wossum @ 2008-05-22 16:20 UTC (permalink / raw)
  To: alsa-devel

Hi all,

I'm attempting to bring the sound up on a board using an Atmel AT32AP7000 
(AVR32) microcontroller with a Wolfson WM8510 mono CODEC.

I'm using the Linux 2.6.24.3.atmel.3 kernel, the latest available for the 
AVR32.  I've patched this kernel with the ASoC subsystem and WM8510 driver 
from git://opensource.wolfsonmicro.com/linux-2.6-asoc, as it appeared when 
v2.6.24 was tagged.  I've also written the platform driver for the AT32, 
which is very close to the AT91 driver, and a machine driver for my specific 
hardware.  I'm letting the WM8510 provide the I2S clocking from its internal 
PLL.

I'm having issues getting the sound to work.  I found what I believe are 
several bugs in the wm8510.c.  I also had to lie about the WM8510 supporting 
2 channels.  Otherwise, the upper ALSA seem to include only 1 channel in the 
data stream, and the WM8510 seems to still expect 2 channels in the I2S 
stream.  A patch is found a the bottom of this email.  With this patch, the 
registers on the WM8510 seem to be getting configured properly (all the clock 
rates and such look correct), but I get no signal at the speaker outputs or 
the mono output, no matter what the mixer settings are.  They just sit VMID 
after the initial power up of the chip.  

Is anyone else using the WM8510?  Any hints or suggestions?

TIA,
---
Geoffrey


Index: linux-2.6.24.3.atmel.3/sound/soc/codecs/wm8510.c
===================================================================
--- linux-2.6.24.3.atmel.3.orig/sound/soc/codecs/wm8510.c
+++ linux-2.6.24.3.atmel.3/sound/soc/codecs/wm8510.c
@@ -186,7 +186,7 @@ SOC_SINGLE("Speaker Playback Volume", WM
 SOC_SINGLE("Speaker Boost", WM8510_OUTPUT, 2, 1, 0),
 
 SOC_SINGLE("Capture Boost(+20dB)", WM8510_ADCBOOST,  8, 1, 0),
-SOC_SINGLE("Mono Playback Switch", WM8510_MONOMIX, 6, 1, 0),
+SOC_SINGLE("Mono Playback Switch", WM8510_MONOMIX, 6, 1, 1),
 };
 
 /* add non dapm controls */
@@ -209,14 +209,14 @@ static int wm8510_add_controls(struct sn
 static const struct snd_kcontrol_new wm8510_speaker_mixer_controls[] = {
 SOC_DAPM_SINGLE("Line Bypass Switch", WM8510_SPKMIX, 1, 1, 0),
 SOC_DAPM_SINGLE("Aux Playback Switch", WM8510_SPKMIX, 5, 1, 0),
-SOC_DAPM_SINGLE("PCM Playback Switch", WM8510_SPKMIX, 0, 1, 1),
+SOC_DAPM_SINGLE("PCM Playback Switch", WM8510_SPKMIX, 0, 1, 0),
 };
 
 /* Mono Output Mixer */
 static const struct snd_kcontrol_new wm8510_mono_mixer_controls[] = {
 SOC_DAPM_SINGLE("Line Bypass Switch", WM8510_MONOMIX, 1, 1, 0),
 SOC_DAPM_SINGLE("Aux Playback Switch", WM8510_MONOMIX, 2, 1, 0),
-SOC_DAPM_SINGLE("PCM Playback Switch", WM8510_MONOMIX, 0, 1, 1),
+SOC_DAPM_SINGLE("PCM Playback Switch", WM8510_MONOMIX, 0, 1, 0),
 };
 
 /* AUX Input boost vol */
@@ -251,7 +251,7 @@ SND_SOC_DAPM_MIXER("Mono Mixer", WM8510_
 	&wm8510_mono_mixer_controls[0],
 	ARRAY_SIZE(wm8510_mono_mixer_controls)),
 SND_SOC_DAPM_DAC("DAC", "HiFi Playback", WM8510_POWER3, 0, 0),
-SND_SOC_DAPM_ADC("ADC", "HiFi Capture", WM8510_POWER3, 0, 0),
+SND_SOC_DAPM_ADC("ADC", "HiFi Capture", WM8510_POWER2, 0, 0),
 SND_SOC_DAPM_PGA("Aux Input", WM8510_POWER1, 6, 0, NULL, 0),
 SND_SOC_DAPM_PGA("SpkN Out", WM8510_POWER3, 5, 0, NULL, 0),
 SND_SOC_DAPM_PGA("SpkP Out", WM8510_POWER3, 6, 0, NULL, 0),
@@ -393,8 +393,8 @@ static int wm8510_set_dai_pll(struct snd
 
 	wm8510_write(codec, WM8510_PLLN, (pll_div.pre_div << 4) | pll_div.n);
 	wm8510_write(codec, WM8510_PLLK1, pll_div.k >> 18);
-	wm8510_write(codec, WM8510_PLLK1, (pll_div.k >> 9) && 0x1ff);
-	wm8510_write(codec, WM8510_PLLK1, pll_div.k && 0x1ff);
+	wm8510_write(codec, WM8510_PLLK2, (pll_div.k >> 9) & 0x1ff);
+	wm8510_write(codec, WM8510_PLLK3, pll_div.k & 0x1ff);
 	reg = wm8510_read_reg_cache(codec, WM8510_POWER1);
 	wm8510_write(codec, WM8510_POWER1, reg | 0x020);
 	return 0;
@@ -412,23 +412,23 @@ static int wm8510_set_dai_clkdiv(struct 
 
 	switch (div_id) {
 	case WM8510_OPCLKDIV:
-		reg = wm8510_read_reg_cache(codec, WM8510_GPIO & 0x1cf);
+                reg = wm8510_read_reg_cache(codec, WM8510_GPIO) & 0x1cf;
 		wm8510_write(codec, WM8510_GPIO, reg | div);
 		break;
 	case WM8510_MCLKDIV:
-		reg = wm8510_read_reg_cache(codec, WM8510_CLOCK & 0x1f);
+		reg = wm8510_read_reg_cache(codec, WM8510_CLOCK) & 0x1f;
 		wm8510_write(codec, WM8510_CLOCK, reg | div);
 		break;
 	case WM8510_ADCCLK:
-		reg = wm8510_read_reg_cache(codec, WM8510_ADC & 0x1f7);
+		reg = wm8510_read_reg_cache(codec, WM8510_ADC) & 0x1f7;
 		wm8510_write(codec, WM8510_ADC, reg | div);
 		break;
 	case WM8510_DACCLK:
-		reg = wm8510_read_reg_cache(codec, WM8510_DAC & 0x1f7);
+		reg = wm8510_read_reg_cache(codec, WM8510_DAC) & 0x1f7;
 		wm8510_write(codec, WM8510_DAC, reg | div);
 		break;
 	case WM8510_BCLKDIV:
-		reg = wm8510_read_reg_cache(codec, WM8510_CLOCK & 0x1e3);
+		reg = wm8510_read_reg_cache(codec, WM8510_CLOCK) & 0x1e3;
 		wm8510_write(codec, WM8510_CLOCK, reg | div);
 		break;
 	default:
@@ -597,14 +597,14 @@ struct snd_soc_codec_dai wm8510_dai = {
 	.name = "WM8510 HiFi",
 	.playback = {
 		.stream_name = "Playback",
-		.channels_min = 1,
-		.channels_max = 1,
+		.channels_min = 2,
+		.channels_max = 2,
 		.rates = WM8510_RATES,
 		.formats = WM8510_FORMATS,},
 	.capture = {
 		.stream_name = "Capture",
-		.channels_min = 1,
-		.channels_max = 1,
+		.channels_min = 2,
+		.channels_max = 2,
 		.rates = WM8510_RATES,
 		.formats = WM8510_FORMATS,},
 	.ops = {

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-22 16:20 AT32AP7000 (AVR32) + WM8510 Geoffrey Wossum
@ 2008-05-22 19:46 ` Mark Brown
  2008-05-22 19:57   ` Geoffrey Wossum
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2008-05-22 19:46 UTC (permalink / raw)
  To: Geoffrey Wossum; +Cc: alsa-devel

On Thu, May 22, 2008 at 11:20:08AM -0500, Geoffrey Wossum wrote:

> stream.  A patch is found a the bottom of this email.  With this patch, the 
> registers on the WM8510 seem to be getting configured properly (all the clock 
> rates and such look correct), but I get no signal at the speaker outputs or 

Almost of this looks good, thanks for spotting these!  I've applied the
bits I was able to check, see below for one issue.  Note that when
submitting changes it's always best to supply a Signed-off-by, though
for changes like this it's probably OK not to.

> the mono output, no matter what the mixer settings are.  They just sit VMID 
> after the initial power up of the chip.  

Check that all the required DAPM widgets are powered up by looking in
/sys/bus/platform/devices/soc-audio/dapm_widget during playback - if
they aren't then enabling DAPM_DEBUG in sound/soc/soc-dapm.c will make
DAPM display information about the decisions it's making.  If you can't
see anything there could you please supply a register dump from during
playback (availible in /sys/bus/platform/devices/soc-audio/codec_reg)?

I'm out of the office at the minute so can't test on actual hardware
myself right now.

> @@ -597,14 +597,14 @@ struct snd_soc_codec_dai wm8510_dai = {
>  	.name = "WM8510 HiFi",
>  	.playback = {
>  		.stream_name = "Playback",
> -		.channels_min = 1,
> -		.channels_max = 1,
> +		.channels_min = 2,
> +		.channels_max = 2,
>  		.rates = WM8510_RATES,

I'll need to check this out in more detail - the WM8510 is a mono codec
so it can only play one channel.

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-22 19:46 ` Mark Brown
@ 2008-05-22 19:57   ` Geoffrey Wossum
  2008-05-22 22:48     ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Geoffrey Wossum @ 2008-05-22 19:57 UTC (permalink / raw)
  To: alsa-devel

On Thursday 22 May 2008 02:46:21 pm Mark Brown wrote:
> On Thu, May 22, 2008 at 11:20:08AM -0500, Geoffrey Wossum wrote:

> Check that all the required DAPM widgets are powered up by looking in
> /sys/bus/platform/devices/soc-audio/dapm_widget during playback - if
> they aren't then enabling DAPM_DEBUG in sound/soc/soc-dapm.c will make
> DAPM display information about the decisions it's making.  If you can't
> see anything there could you please supply a register dump from during
> playback (availible in /sys/bus/platform/devices/soc-audio/codec_reg)?

Here's the DAPM widgets when I'm trying to play a file with aplay:
Ext Spk: On
Int Mic: Off
Mic Bias: Off
Boost Mixer: Off
Mic Boost: Off
Aux Boost: Off
Mic PGA: Off
Mono Out: Off
SpkP Out: On
SpkN Out: On
Aux Input: Off
ADC: Off
DAC: On
Mono Mixer: Off
Speaker Mixer: On
PM State: D0

Looks like it should work to me.  Note that I do hear a pop from the speaker 
the first time I try to play a file.  Here's a dump of the registers while 
I'm trying to play a file with aplay.  I'm combing through them right now to 
see if I can find anything that looks off.

 WM8510 registers
 0:    0
 1:  1ff
 2:  1ff
 3:  1ff
 4:   10
 5:    0
 6:  151
 7:    0
 8:    0
 9:    0
 a:    0
 b:   ff
 c:    0
 d:    0
 e:  100
 f:   ff
10:    0
11:    0
12:  12c
13:   2c
14:   2c
15:   2c
16:   2c
17:    0
18:   32
19:    0
1a:    0
1b:    0
1c:    0
1d:    0
1e:    0
1f:    0
20:   38
21:    b
22:   32
23:    0
24:    7
25:   2f
26:   e7
27:  10a
28:    0
29:    0
2a:    0
2b:    0
2c:    3
2d:   10
2e:    0
2f:    0
30:    0
31:    2
32:    1
33:    0
34:    0
35:    0
36:   39
37:    0
38:    0
-snip-
Bunch of bogus register reads equal to ffffffff after here.

Wish I had known about this register dump earlier today.  I added code to do 
the same thing with debugfs.

> > @@ -597,14 +597,14 @@ struct snd_soc_codec_dai wm8510_dai = {
> >  	.name = "WM8510 HiFi",
> >  	.playback = {
> >  		.stream_name = "Playback",
> > -		.channels_min = 1,
> > -		.channels_max = 1,
> > +		.channels_min = 2,
> > +		.channels_max = 2,
> >  		.rates = WM8510_RATES,
>
> I'll need to check this out in more detail - the WM8510 is a mono codec
> so it can only play one channel.

Not sure about this either, since I can't get it to play anything yet.  But it 
looks from the WM8510 data sheet that even though the WM8510 is mono, it 
expects a stereo I2S stream, and just ignores one of the channels based on 
the value of DACLRSWAP register bit.

Thanks,
---
Geoffrey

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-22 19:57   ` Geoffrey Wossum
@ 2008-05-22 22:48     ` Mark Brown
  2008-05-22 23:11       ` Geoffrey Wossum
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2008-05-22 22:48 UTC (permalink / raw)
  To: Geoffrey Wossum; +Cc: alsa-devel

On Thu, May 22, 2008 at 02:57:41PM -0500, Geoffrey Wossum wrote:

> Looks like it should work to me.  Note that I do hear a pop from the speaker 
> the first time I try to play a file.  Here's a dump of the registers while 
> I'm trying to play a file with aplay.  I'm combing through them right now to 
> see if I can find anything that looks off.

Yes, it looks like the output paths are OK from the DAPM point of view.

> Wish I had known about this register dump earlier today.  I added code to do 
> the same thing with debugfs.

I'll have a look at it at some point, but not right now :) .  I should
also have asked you if you could post your machine driver - if you'd
rather post it off-list that'd be fine.

> > I'll need to check this out in more detail - the WM8510 is a mono codec
> > so it can only play one channel.

> Not sure about this either, since I can't get it to play anything yet.  But it 
> looks from the WM8510 data sheet that even though the WM8510 is mono, it 
> expects a stereo I2S stream, and just ignores one of the channels based on 
> the value of DACLRSWAP register bit.

Indeed, the main thing I need to check is to confirm what is normally
done with mono channels - in any case, the codec should at least make
some noise providing the rates are correct and there's stereo data.

I take it the digital audio looks fine with a scope - clocks at the
correct rate, sensible looking data and so on?

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-22 22:48     ` Mark Brown
@ 2008-05-22 23:11       ` Geoffrey Wossum
  2008-05-23 22:56         ` Geoffrey Wossum
  0 siblings, 1 reply; 10+ messages in thread
From: Geoffrey Wossum @ 2008-05-22 23:11 UTC (permalink / raw)
  To: alsa-devel

On Thursday 22 May 2008 05:48:41 pm Mark Brown wrote:
> On Thu, May 22, 2008 at 02:57:41PM -0500, Geoffrey Wossum wrote:
> > Looks like it should work to me.  Note that I do hear a pop from the
> > speaker the first time I try to play a file.  Here's a dump of the
> > registers while I'm trying to play a file with aplay.  I'm combing
> > through them right now to see if I can find anything that looks off.

> I'll have a look at it at some point, but not right now :) .  I should
> also have asked you if you could post your machine driver - if you'd
> rather post it off-list that'd be fine.

I'll have to do that tomorrow, I'm on my way out the door.

> I take it the digital audio looks fine with a scope - clocks at the
> correct rate, sensible looking data and so on?

The DACDAT signal looks pretty good.  However, I did notice something very 
odd.  The BCLK is dead on for whatever bitrate I'm doing.  However, the FRAME 
always runs at 44.1 kHz, no matter what I do.  From my understanding of I2S 
(which could be wrong), I would expect FRAME to run at the sample rate I'm 
playing back.  So if I'm playing back at 11025, I would expect BCLK to run at 
352 kHz and FRAME to run at 11 kHz.  I see BCLK running at 352 kHz, check.  
But frame is running at 44.1 kHz.  This doesn't seem right.  Am I missing 
something here?

---
Geoffrey

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-22 23:11       ` Geoffrey Wossum
@ 2008-05-23 22:56         ` Geoffrey Wossum
  2008-05-26  9:45           ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Geoffrey Wossum @ 2008-05-23 22:56 UTC (permalink / raw)
  To: alsa-devel

On Thursday 22 May 2008 06:11:31 pm Geoffrey Wossum wrote:
> On Thursday 22 May 2008 05:48:41 pm Mark Brown wrote:
> > On Thu, May 22, 2008 at 02:57:41PM -0500, Geoffrey Wossum wrote:
> > > Looks like it should work to me.  Note that I do hear a pop from the
> > > speaker the first time I try to play a file.  Here's a dump of the
> > > registers while I'm trying to play a file with aplay.  I'm combing
> > > through them right now to see if I can find anything that looks off.
> >
> > I'll have a look at it at some point, but not right now :) .  I should
> > also have asked you if you could post your machine driver - if you'd
> > rather post it off-list that'd be fine.

Ok, I got it working.  It can now play "I'm sorry, Dave.  I'm afraid I can't 
do that."  This should be the audio equivalent of "Hello world" :)

First, I changed it so the AVR32 provided the BCLK and FRAME.  I could never 
get the WM8510 to generate what looked like proper FRAMES.  I did continue 
using the WM8510's PLL to generate the DAC clock.  With the AVR32 providing 
the clocks, I at least got some noise out of it.

After a lot of head scratching and analyzing register settings and what not, I 
realized that I had based my AVR32 platform drivers on the AT91.  The AT91 is 
little endian, the AVR32 is big endian.  Changing all the SNDRV_PCM_FMTBIT_ 
settings to be BE instead of LE changed the noise into the files I was trying 
to play.

Finally, I really had to lie in the wm8510 code and say that it supported a 
minimum of 2 channels, and a maximum of 2 channels.  This makes sense from 
the data sheet, since it still wants a left and a right channel of data, and 
just throws one away.

A few questions:

I had to change the WM8510's format to say that it wanted big endian.  Should 
the CODEC (and platform and machine) be setup just to say that it uses 
SNDRV_PCM_FMTBIT_S16 instead of SNDRV_PCM_FMTBIT_S16_LE/BE?  Looks from 
sound/pcm.h this should just do the Right Thing.

Any idea why I couldn't get the WM8510 to generate the FRAME signal properly?  
Like I said before, it generated a 44.1 kHz frame no matter the BCLK divider 
or the word length were set to.

Thanks for the help!  As soon as I get the code cleaned up some, I'll 
contribute the AVR32 platform driver code.

---
Geoffrey

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-23 22:56         ` Geoffrey Wossum
@ 2008-05-26  9:45           ` Mark Brown
  2008-05-27 15:16             ` Geoffrey Wossum
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2008-05-26  9:45 UTC (permalink / raw)
  To: Geoffrey Wossum; +Cc: alsa-devel

On Fri, May 23, 2008 at 05:56:00PM -0500, Geoffrey Wossum wrote:

> First, I changed it so the AVR32 provided the BCLK and FRAME.  I could never 
> get the WM8510 to generate what looked like proper FRAMES.  I did continue 
> using the WM8510's PLL to generate the DAC clock.  With the AVR32 providing 
> the clocks, I at least got some noise out of it.

The frame clock rate generated by the WM8510 will be 1/256 of the system
clock, which should be configured to 256 times the sample rate using the
PLL and dividers.

> Finally, I really had to lie in the wm8510 code and say that it supported a 
> minimum of 2 channels, and a maximum of 2 channels.  This makes sense from 
> the data sheet, since it still wants a left and a right channel of data, and 
> just throws one away.

Yes, that's right.  I've applied your changes to the driver - thanks!

> I had to change the WM8510's format to say that it wanted big endian.  Should 
> the CODEC (and platform and machine) be setup just to say that it uses 
> SNDRV_PCM_FMTBIT_S16 instead of SNDRV_PCM_FMTBIT_S16_LE/BE?  Looks from 
> sound/pcm.h this should just do the Right Thing.

Yes, it does look reasonable - I'll investigate.

> Like I said before, it generated a 44.1 kHz frame no matter the BCLK divider 
> or the word length were set to.

The frame clock is independent of both those options - I assume you are
using a static system clock?

> Thanks for the help!  As soon as I get the code cleaned up some, I'll 
> contribute the AVR32 platform driver code.

That would be excellent.

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-26  9:45           ` Mark Brown
@ 2008-05-27 15:16             ` Geoffrey Wossum
  2008-05-27 18:45               ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Geoffrey Wossum @ 2008-05-27 15:16 UTC (permalink / raw)
  To: alsa-devel

On Monday 26 May 2008 04:45:24 am Mark Brown wrote:
> On Fri, May 23, 2008 at 05:56:00PM -0500, Geoffrey Wossum wrote:

> The frame clock rate generated by the WM8510 will be 1/256 of the system
> clock, which should be configured to 256 times the sample rate using the
> PLL and dividers.

Ah, so FRAME is not affected by BCLKDIV.  I was misled by figure 31 on page 54 
of the WM8510 datasheet, which gives the impression that BCLKDIV affects 
FRAME as well as BCLK.  I might give the WM8510 a try as the master again, 
since I can get the sample rates closer with it.


> > I had to change the WM8510's format to say that it wanted big endian. 
> > Should the CODEC (and platform and machine) be setup just to say that it
> > uses SNDRV_PCM_FMTBIT_S16 instead of SNDRV_PCM_FMTBIT_S16_LE/BE?  Looks
> > from sound/pcm.h this should just do the Right Thing.
>
> Yes, it does look reasonable - I'll investigate.

I've tried it out, and it seems to do exactly the right thing.  Only problem 
is that there is no auto-endian constant for SNDRV_PCM_FMTBIT_S24_3LE/3BE.  I 
don't know enough to know if this is an oversight or a technical issue.

It seems like the other CODEC drivers will eventually need to use the 
auto-endian constants, especially if big endian microcontrollers like the 
AVR32 catch on.

I'm attaching my final patch to sound/soc/codecs/wm8510.c (again, this is 
against a 2.6.24 kernel).  I noticed that there seems to be a misprint in the 
WM8510 datasheet.  Register 0x32 (SPKMIX) seems to have a different default 
value than what is in the datasheet.  Also, the driver never configured CLKSEL 
to run the CODEC from PLL.  I changed wm8510_set_dai_pll() to automatically 
set CLKSEL when the PLL is started, and to clear CLKSEL when the PLL is 
turned off.  It doesn't seem like anyone would ever configure the PLL if they 
didn't want to use the PLL.

Thanks!
---
Geoffrey

Index: linux-2.6.24.3.atmel.3/sound/soc/codecs/wm8510.c
===================================================================
--- linux-2.6.24.3.atmel.3.orig/sound/soc/codecs/wm8510.c
+++ linux-2.6.24.3.atmel.3/sound/soc/codecs/wm8510.c
@@ -71,7 +71,7 @@ static const u16 wm8510_reg[WM8510_CACHE
     0x0008, 0x000c, 0x0093, 0x00e9,
     0x0000, 0x0000, 0x0000, 0x0000,
     0x0003, 0x0010, 0x0000, 0x0000,
-    0x0000, 0x0002, 0x0000, 0x0000,
+    0x0000, 0x0002, 0x0001, 0x0000,
     0x0000, 0x0000, 0x0039, 0x0000,
     0x0000,
 };
@@ -186,7 +186,7 @@ SOC_SINGLE("Speaker Playback Volume", WM
 SOC_SINGLE("Speaker Boost", WM8510_OUTPUT, 2, 1, 0),
 
 SOC_SINGLE("Capture Boost(+20dB)", WM8510_ADCBOOST,  8, 1, 0),
-SOC_SINGLE("Mono Playback Switch", WM8510_MONOMIX, 6, 1, 0),
+SOC_SINGLE("Mono Playback Switch", WM8510_MONOMIX, 6, 1, 1),
 };
 
 /* add non dapm controls */
@@ -209,14 +209,14 @@ static int wm8510_add_controls(struct sn
 static const struct snd_kcontrol_new wm8510_speaker_mixer_controls[] = {
 SOC_DAPM_SINGLE("Line Bypass Switch", WM8510_SPKMIX, 1, 1, 0),
 SOC_DAPM_SINGLE("Aux Playback Switch", WM8510_SPKMIX, 5, 1, 0),
-SOC_DAPM_SINGLE("PCM Playback Switch", WM8510_SPKMIX, 0, 1, 1),
+SOC_DAPM_SINGLE("PCM Playback Switch", WM8510_SPKMIX, 0, 1, 0),
 };
 
 /* Mono Output Mixer */
 static const struct snd_kcontrol_new wm8510_mono_mixer_controls[] = {
 SOC_DAPM_SINGLE("Line Bypass Switch", WM8510_MONOMIX, 1, 1, 0),
 SOC_DAPM_SINGLE("Aux Playback Switch", WM8510_MONOMIX, 2, 1, 0),
-SOC_DAPM_SINGLE("PCM Playback Switch", WM8510_MONOMIX, 0, 1, 1),
+SOC_DAPM_SINGLE("PCM Playback Switch", WM8510_MONOMIX, 0, 1, 0),
 };
 
 /* AUX Input boost vol */
@@ -251,7 +251,7 @@ SND_SOC_DAPM_MIXER("Mono Mixer", WM8510_
 	&wm8510_mono_mixer_controls[0],
 	ARRAY_SIZE(wm8510_mono_mixer_controls)),
 SND_SOC_DAPM_DAC("DAC", "HiFi Playback", WM8510_POWER3, 0, 0),
-SND_SOC_DAPM_ADC("ADC", "HiFi Capture", WM8510_POWER3, 0, 0),
+SND_SOC_DAPM_ADC("ADC", "HiFi Capture", WM8510_POWER2, 0, 0),
 SND_SOC_DAPM_PGA("Aux Input", WM8510_POWER1, 6, 0, NULL, 0),
 SND_SOC_DAPM_PGA("SpkN Out", WM8510_POWER3, 5, 0, NULL, 0),
 SND_SOC_DAPM_PGA("SpkP Out", WM8510_POWER3, 6, 0, NULL, 0),
@@ -384,6 +384,11 @@ static int wm8510_set_dai_pll(struct snd
 	u16 reg;
 
 	if (freq_in == 0 || freq_out == 0) {
+                /* Clock CODEC directly from MCLK */
+                reg = wm8510_read_reg_cache(codec, WM8510_CLOCK);
+                wm8510_write(codec, WM8510_CLOCK, reg & 0x0ff);
+
+                /* Turn off PLL */
 		reg = wm8510_read_reg_cache(codec, WM8510_POWER1);
 		wm8510_write(codec, WM8510_POWER1, reg & 0x1df);
 		return 0;
@@ -391,12 +396,17 @@ static int wm8510_set_dai_pll(struct snd
 
 	pll_factors(freq_out*8, freq_in);
 
+        /* Set PLL parameters, enable PLL */
 	wm8510_write(codec, WM8510_PLLN, (pll_div.pre_div << 4) | pll_div.n);
 	wm8510_write(codec, WM8510_PLLK1, pll_div.k >> 18);
-	wm8510_write(codec, WM8510_PLLK1, (pll_div.k >> 9) && 0x1ff);
-	wm8510_write(codec, WM8510_PLLK1, pll_div.k && 0x1ff);
+	wm8510_write(codec, WM8510_PLLK2, (pll_div.k >> 9) & 0x1ff);
+	wm8510_write(codec, WM8510_PLLK3, pll_div.k & 0x1ff);
 	reg = wm8510_read_reg_cache(codec, WM8510_POWER1);
 	wm8510_write(codec, WM8510_POWER1, reg | 0x020);
+
+        /* Run CODEC from PLL instead of MCLK */
+        reg = wm8510_read_reg_cache(codec, WM8510_CLOCK);
+        wm8510_write(codec, WM8510_CLOCK, reg | 0x100);
 	return 0;
 
 }
@@ -412,23 +422,23 @@ static int wm8510_set_dai_clkdiv(struct 
 
 	switch (div_id) {
 	case WM8510_OPCLKDIV:
-		reg = wm8510_read_reg_cache(codec, WM8510_GPIO & 0x1cf);
+                reg = wm8510_read_reg_cache(codec, WM8510_GPIO) & 0x1cf;
 		wm8510_write(codec, WM8510_GPIO, reg | div);
 		break;
 	case WM8510_MCLKDIV:
-		reg = wm8510_read_reg_cache(codec, WM8510_CLOCK & 0x1f);
+		reg = wm8510_read_reg_cache(codec, WM8510_CLOCK) & 0x1f;
 		wm8510_write(codec, WM8510_CLOCK, reg | div);
 		break;
 	case WM8510_ADCCLK:
-		reg = wm8510_read_reg_cache(codec, WM8510_ADC & 0x1f7);
+		reg = wm8510_read_reg_cache(codec, WM8510_ADC) & 0x1f7;
 		wm8510_write(codec, WM8510_ADC, reg | div);
 		break;
 	case WM8510_DACCLK:
-		reg = wm8510_read_reg_cache(codec, WM8510_DAC & 0x1f7);
+		reg = wm8510_read_reg_cache(codec, WM8510_DAC) & 0x1f7;
 		wm8510_write(codec, WM8510_DAC, reg | div);
 		break;
 	case WM8510_BCLKDIV:
-		reg = wm8510_read_reg_cache(codec, WM8510_CLOCK & 0x1e3);
+		reg = wm8510_read_reg_cache(codec, WM8510_CLOCK) & 0x1e3;
 		wm8510_write(codec, WM8510_CLOCK, reg | div);
 		break;
 	default:
@@ -506,15 +516,15 @@ static int wm8510_pcm_hw_params(struct s
 
 	/* bit size */
 	switch (params_format(params)) {
-	case SNDRV_PCM_FORMAT_S16_LE:
+	case SNDRV_PCM_FORMAT_S16:
 		break;
 	case SNDRV_PCM_FORMAT_S20_3LE:
 		iface |= 0x0020;
 		break;
-	case SNDRV_PCM_FORMAT_S24_LE:
+	case SNDRV_PCM_FORMAT_S24:
 		iface |= 0x0040;
 		break;
-	case SNDRV_PCM_FORMAT_S32_LE:
+	case SNDRV_PCM_FORMAT_S32:
 		iface |= 0x0060;
 		break;
 	}
@@ -573,7 +583,9 @@ static int wm8510_dapm_event(struct snd_
 		break;
 	case SNDRV_CTL_POWER_D3hot: /* Off, with power */
 		/* everything off except vref/vmid, dac mute, inactive */
-
+                wm8510_write(codec, WM8510_POWER1, 0x00f);
+		wm8510_write(codec, WM8510_POWER2, 0x0);
+		wm8510_write(codec, WM8510_POWER3, 0x0);
 		break;
 	case SNDRV_CTL_POWER_D3cold: /* Off, without power */
 		/* everything off, dac mute, inactive */
@@ -590,21 +602,21 @@ static int wm8510_dapm_event(struct snd_
 		SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 |\
 		SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000)
 
-#define WM8510_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
-	SNDRV_PCM_FMTBIT_S24_LE)
+#define WM8510_FORMATS (SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_S20_3LE |\
+	SNDRV_PCM_FMTBIT_S24)
 
 struct snd_soc_codec_dai wm8510_dai = {
 	.name = "WM8510 HiFi",
 	.playback = {
 		.stream_name = "Playback",
-		.channels_min = 1,
-		.channels_max = 1,
+		.channels_min = 2,
+		.channels_max = 2,
 		.rates = WM8510_RATES,
 		.formats = WM8510_FORMATS,},
 	.capture = {
 		.stream_name = "Capture",
-		.channels_min = 1,
-		.channels_max = 1,
+		.channels_min = 2,
+		.channels_max = 2,
 		.rates = WM8510_RATES,
 		.formats = WM8510_FORMATS,},
 	.ops = {

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-27 15:16             ` Geoffrey Wossum
@ 2008-05-27 18:45               ` Mark Brown
  2008-05-27 22:26                 ` Geoffrey Wossum
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2008-05-27 18:45 UTC (permalink / raw)
  To: Geoffrey Wossum; +Cc: alsa-devel

On Tue, May 27, 2008 at 10:16:56AM -0500, Geoffrey Wossum wrote:

> FRAME as well as BCLK.  I might give the WM8510 a try as the master again, 
> since I can get the sample rates closer with it.

Let me know how that goes, please.

> It seems like the other CODEC drivers will eventually need to use the 
> auto-endian constants, especially if big endian microcontrollers like the 
> AVR32 catch on.

If anything offering both big endian and little endian seems best - that
will allow the host controller to offer whatever memory formats it feels
like offering.

> I'm attaching my final patch to sound/soc/codecs/wm8510.c (again, this is 
> against a 2.6.24 kernel).  I noticed that there seems to be a misprint in the 

Thanks - as I said previously it's much better if you can include a
Signed-off-by line in patch submissions in order to allow them to be
submitted to the mainline kernel, though in the case of fairly obvious
register access fixes like these it's generally not an obstacle.

> WM8510 datasheet.  Register 0x32 (SPKMIX) seems to have a different default 

> value than what is in the datasheet.  Also, the driver never configured CLKSEL 
I'll take a look.

> to run the CODEC from PLL.  I changed wm8510_set_dai_pll() to automatically 
> set CLKSEL when the PLL is started, and to clear CLKSEL when the PLL is 
> turned off.  It doesn't seem like anyone would ever configure the PLL if they 
> didn't want to use the PLL.

Yup, and that's exactly what most of our codec drivers do.  I'll apply
this fix to the driver - thanks!

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

* Re: AT32AP7000 (AVR32) + WM8510
  2008-05-27 18:45               ` Mark Brown
@ 2008-05-27 22:26                 ` Geoffrey Wossum
  0 siblings, 0 replies; 10+ messages in thread
From: Geoffrey Wossum @ 2008-05-27 22:26 UTC (permalink / raw)
  To: alsa-devel

On Tuesday 27 May 2008 01:45:28 pm Mark Brown wrote:
> On Tue, May 27, 2008 at 10:16:56AM -0500, Geoffrey Wossum wrote:
> > FRAME as well as BCLK.  I might give the WM8510 a try as the master
> > again, since I can get the sample rates closer with it.
>
> Let me know how that goes, please.

Works great, now that I know the secret.  Pitch is correct now, and sound 
quality is improved, especially at rates that the AT32AP7000 couldn't 
accurately generate.

> Thanks - as I said previously it's much better if you can include a
> Signed-off-by line in patch submissions in order to allow them to be
> submitted to the mainline kernel, though in the case of fairly obvious
> register access fixes like these it's generally not an obstacle.

I'll try to remember that :)


I'm not doing any input / microphone stuff right now, but the input signal 
path seems incorrect.  I get the following error message everytime I start 
the wm8510 driver:
asoc: no dapm match for Boost Mixer --> Capture Switch --> Capture Boost

Looking at the signal routing in wm8510.c, there seems to also be issues with 
the source and destination of the route being reversed.  

---
Geoffrey

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

end of thread, other threads:[~2008-05-27 22:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-22 16:20 AT32AP7000 (AVR32) + WM8510 Geoffrey Wossum
2008-05-22 19:46 ` Mark Brown
2008-05-22 19:57   ` Geoffrey Wossum
2008-05-22 22:48     ` Mark Brown
2008-05-22 23:11       ` Geoffrey Wossum
2008-05-23 22:56         ` Geoffrey Wossum
2008-05-26  9:45           ` Mark Brown
2008-05-27 15:16             ` Geoffrey Wossum
2008-05-27 18:45               ` Mark Brown
2008-05-27 22:26                 ` Geoffrey Wossum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox