On Thu, May 12, 2016 at 03:06:12PM +0200, Petr Kulhavy wrote:
>
>
> On 12.05.2016 14:15, Charles Keepax wrote:
> >On Thu, May 12, 2016 at 08:48:54AM +0200, Petr Kulhavy wrote:
> >@@ -693,15 +682,18 @@ static int wm8985_hw_params(struct snd_pcm_substream *substream,
> > struct snd_soc_codec *codec;
> > struct wm8985_priv *wm8985;
> > u16 blen, srate_idx;
> >- unsigned int tmp;
> > int srate_best;
> >+ unsigned int bclk; /* bit clock matching the current params */
> >+ unsigned int sysclk; /* the actual sysclk after post division */
> > codec = dai->codec;
> > wm8985 = snd_soc_codec_get_drvdata(codec);
> >- wm8985->bclk = snd_soc_params_to_bclk(params);
> >- if ((int)wm8985->bclk < 0)
> >- return wm8985->bclk;
> >+ /* always use 256 * fs in order to get best filter quality */
> >+ sysclk = 256 * params_rate(params);
> >Seems quite bold to assume we always want to use 256*fs, I am not
> >super familiar with the part itself but are there good reasons to
> >do that? What if someone wanted to use say a direct MCLK and a
> >lower multiple?
> >
> >The intent presumably here is that set_sysclk is where we set the
> >target SYSCLK and set_pll should set the PLL output then here we
> >probably should work out the correct MCLKDIV to get those.
> As I understand the datasheet the codec is basically designed for 256*fs
> clock:
> DAC and ADC expect it, so do the cutoffs for the digital filters, ALC attack
> and delay times.
> Also all the timing diagrams are specified at 256fs clock.
Ok that seems pretty conclusive that we should always have this
at 256fs.
> >> static int pll_factors(struct pll_div *pll_div, unsigned int target,
> >> unsigned int source)
> >> {
> >>@@ -872,17 +867,23 @@ static int wm8985_set_sysclk(struct snd_soc_dai *dai,
> >> WM8985_CLKSEL_MASK, 0);
> >> snd_soc_update_bits(codec, WM8985_POWER_MANAGEMENT_1,
> >> WM8985_PLLEN_MASK, 0);
> >>+ wm8985->pllout = freq;
> >> break;
> >> case WM8985_CLKSRC_PLL:
> >> snd_soc_update_bits(codec, WM8985_CLOCK_GEN_CONTROL,
> >> WM8985_CLKSEL_MASK, WM8985_CLKSEL);
> >>+ /*
> >>+ * in order to run the PLL within the recommended 90MHz
> >>+ * operating range the wm8985_set_pll() configures the PLL
> >>+ * to output double the required frequency
> >>+ */
> >>+ wm8985->pllout = 2 * freq;
> >Were does this *2 come from? Is this the PLLPRESCALE?
> It comes from the following line in wm8985_set_pll() :
> ret = pll_factors(&pll_div, freq_out * 4 * 2, freq_in);
>
> This formula is taken over from the datasheet (the section Master Clock and
> PLL, calculation of the f2) and is correct.
> It attempts to get the PLL to run at approx. 90MHz where it performs the
> best.
> But since there is only f/4 fixed post-divider after the PLL (see the Figure
> 40 in the wm8758 or Figure 38 in the wm8985 datasheet) the F_pllout is
> effectively freq*2.
Ok that also makes sense.
I will have another quick look over the patch.
Thanks,
Charles
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
