Re: [alsa-devel] [PATCH 1/4 v2] ASoC: add a WM8978 codec driver

[Guennadi Liakhovetski <g.liakhovetski@gmx.de>]

On Sat, 23 Jan 2010, Mark Brown wrote:

> On Fri, Jan 22, 2010 at 05:27:53PM +0100, Guennadi Liakhovetski wrote:

[snip]

> > +	if (freq_in = 0 || freq_out = 0) {
> > +		/* Clock CODEC directly from MCLK */
> > +		reg = snd_soc_read(codec, WM8978_CLOCKING);
> > +		snd_soc_write(codec, WM8978_CLOCKING, reg & ~0x100);
> > +
> > +		/* Turn off PLL */
> > +		reg = snd_soc_read(codec, WM8978_POWER_MANAGEMENT_1);
> > +		snd_soc_write(codec, WM8978_POWER_MANAGEMENT_1, reg & ~0x20);
> > +		return 0;
> > +	}
> 
> Note that ideally the PLL updates will handle the case where the PLL is
> already on and power down the PLL while reconfiguring it, though it's
> not essential.
> 
> > +	opclk_div = ((snd_soc_read(codec, WM8978_GPIO_CONTROL) & 0x30) >> 4) + 1;
> 
> > +	wm8978->f_pllout = freq_out * opclk_div;
> 
> Hrm.  I fear that there's a bit of confusion here - the PLL output is
> used separately as the source for both OPCLK and the system clock for
> the CODEC.  This means that the PLL output frequency that we need to
> refer to later on is that before the OPCLK divide.
> 
> Looking further down the driver f_pllout is also being used as the
> system master clock frequency, meaning that the driver will only work
> when the PLL is in use.  Normally what would happen with this stuff is
> that the system clock used by hw_params() will be set using the
> set_sysclk() API call, which can tell the driver what the clock rate is
> and also select between multiple clock sources.  You'd end up saying
> something like:
> 
> 	snd_soc_dai_set_pll(dai, 0, 0, 12000000, 256 * params_rate(params));
> 	snd_soc_dai_set_sysclk(dai, WM8978_PLL, 256 * params_rate(params), 0);
> 
> in the machine driver (note that this also means that the switchover to
> use of the PLL as the clock would then be managed via set_sysclk() not
> set_pll()).

Ok, my new concept is the following:

1. wm8978 needs an input clock for its operation, and it needs to know its 
   frequency. The only way to supply this information to the driver is to 
   use

	snd_soc_dai_set_pll(dai, 0, 0, f_in, f_out);

   where if f_in = 0, input clock is off and the driver has nothing 
   better to do but to switch its own clocking off too. f_out is the 
   frequency, that the user (board) is asking wm8978 to provide on its 
   OPCLK (GPIO1) output. If f_out = 0, this means, the board does not 
   need that output _and_ it is asking the codec to switch PLL off. 
   Otherwise PLL is immediately configured and switched on.

2. having configured codec's PLL the board can select, which clock the 
   codec shall be using as a source for its system clock - PLL or codec 
   input clock directly. The board uses

	snd_soc_dai_set_sysclk(dai, WM8978_PLL, f_sysclk, 0);

   or

	snd_soc_dai_set_sysclk(dai, WM8978_MCLK, f_sysclk, 0);

   for this. Notice, that f_sysclk is ignored, because in wm8978 it is 
   fixed at 256 * fs, and therefore it has to be configured in 
   .hw_params().

3. in .hw_params() we configure the MCLK divider, based on the system 
   clock frequency and baudrate, and set up the selected clock source.

This allows for the most flexible clock configuration, one can even 
configure to use PLL for output clock and input clock directly for the 
system clock, even if such a configuration doesn't make much sense.

I'll be submitting an updated patch shortly, please, let me know if there 
are any problems with this concept.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/