[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 12 Dec 2017 13:21:04 +0000
From: Charles Keepax <ckeepax@...nsource.cirrus.com>
To: "Chen.Liu" <chen.liu.opensource@...il.com>
CC: <lgirdwood@...il.com>, <broonie@...nel.org>, <perex@...ex.cz>,
<tiwai@...e.com>, <shengjiu.wang@...escale.com>,
<patches@...nsource.wolfsonmicro.com>,
<alsa-devel@...a-project.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ASOC: wm8960: Add multiple MCLK frequency support
On Tue, Dec 12, 2017 at 07:58:56PM +0800, Chen.Liu wrote:
> From: "Chen.Liu" <chen.liu.opensource@...il.com>
Please make sure to CC patches@...nsource.cirrus.com on patches
to the Wolfson CODEC drivers.
>
> Issue:
> MCLK=24MHZ,SYSCLOCK=12.288MHZ.
> When the 'wm8960_set_dai_pll' function is called,the driver will
wm8960_set_pll, not wm8960_set_dai_pll?
> prompted "WM8960 PLL: unsupported N = 4" error message.
> However,the value of PLLN should be 8 based on the table45(
> PLL Frequency Examples) of the wm8960 chip manuanl.
>
> Reason:
> The intergrated PLL can be used to generate SYSCLK for the wm8960.
> The pll_factors function in the wm8974.c file is mainly responsible
Assuming that should be wm8960.c?
> -static int pll_factors(unsigned int source, unsigned int target,
> +static int pll_factors(struct snd_soc_codec *codec,
> + unsigned int source, unsigned int target,
> struct _pll_div *pll_div)
> {
> unsigned long long Kpart;
> unsigned int K, Ndiv, Nmod;
> + int unsupport = 0;
> + u16 reg;
>
> pr_debug("WM8960 PLL: setting %dHz->%dHz\n", source, target);
>
> /* Scale up target to PLL operating frequency */
> target *= 4;
>
> +retry:
> + if (unsupport) {
> + target *= 2;
> + unsupport += 1;
> + }
> Ndiv = target / source;
> if (Ndiv < 6) {
> source >>= 1;
> @@ -1056,8 +1064,12 @@ static int pll_factors(unsigned int source, unsigned int target,
> pll_div->pre_div = 0;
Feels like it would be nice to wrap this Ndiv calculation in a
loop rather than adding the goto into the below if statement.
>
> if ((Ndiv < 6) || (Ndiv > 12)) {
> - pr_err("WM8960 PLL: Unsupported N=%d\n", Ndiv);
> - return -EINVAL;
> + if (unsupport == 2) {
> + pr_err("WM8960 PLL: Unsupported N=%d\n", Ndiv);
> + return -EINVAL;
> + }
> + unsupport = 1;
> + goto retry;
> }
>
> pll_div->n = Ndiv;
> @@ -1077,6 +1089,11 @@ static int pll_factors(unsigned int source, unsigned int target,
>
> pll_div->k = K;
>
> + if (unsupport) {
> + reg = snd_soc_read(codec, WM8960_CLOCK1) | WM8960_SYSCLK_DIV_2;
> + snd_soc_write(codec, WM8960_CLOCK1, reg);
snd_soc_update_bits?
Also there is no code that seems to reset this bit should we
later ask for a configuration that no longer needs it.
Thanks,
Charles
Powered by blists - more mailing lists