From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753615AbdLNQVf (ORCPT ); Thu, 14 Dec 2017 11:21:35 -0500 Received: from mx0b-001ae601.pphosted.com ([67.231.152.168]:45826 "EHLO mx0b-001ae601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753003AbdLNQVd (ORCPT ); Thu, 14 Dec 2017 11:21:33 -0500 Authentication-Results: ppops.net; spf=none smtp.mailfrom=ckeepax@opensource.cirrus.com Date: Thu, 14 Dec 2017 16:19:54 +0000 From: Charles Keepax To: "Chen.Liu" CC: , , , , , , , , Subject: Re: [PATCH v2] ASOC: wm8960: Add multiple MCLK frequency support Message-ID: <20171214161954.wfik3ygadwii3zhm@localhost.localdomain> References: <1513168650-102028-1-git-send-email-chen.liu.opensource@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <1513168650-102028-1-git-send-email-chen.liu.opensource@gmail.com> User-Agent: NeoMutt/20170113 (1.7.2) X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1712140223 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 13, 2017 at 08:37:30PM +0800, Chen.Liu wrote: > From: "Chen.Liu" > diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c > index 997c446..83dd746 100644 > --- a/sound/soc/codecs/wm8960.c > +++ b/sound/soc/codecs/wm8960.c > @@ -1036,28 +1036,38 @@ static bool is_pll_freq_available(unsigned int source, unsigned int target) > * to allow rounding later */ > #define FIXED_PLL_SIZE ((1 << 24) * 10) > > -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 unsupported = 0; > > pr_debug("WM8960 PLL: setting %dHz->%dHz\n", source, target); > > /* Scale up target to PLL operating frequency */ > target *= 4; > > - Ndiv = target / source; > - if (Ndiv < 6) { > - source >>= 1; > - pll_div->pre_div = 1; > + while (1) { > Ndiv = target / source; > - } else > - pll_div->pre_div = 0; > + if (Ndiv < 6) { > + source >>= 1; > + pll_div->pre_div = 1; > + Ndiv = target / source; > + } else > + pll_div->pre_div = 0; > + > + if ((Ndiv < 6) || (Ndiv > 12)) { > + if (unsupported == 1) { > + pr_err("WM8960 PLL: Unsupported N=%d\n", Ndiv); > + return -EINVAL; > + } > + } else > + break; > > - if ((Ndiv < 6) || (Ndiv > 12)) { > - pr_err("WM8960 PLL: Unsupported N=%d\n", Ndiv); > - return -EINVAL; > + target *= 2; > + unsupported += 1; > } > > pll_div->n = Ndiv; > @@ -1077,6 +1087,10 @@ static int pll_factors(unsigned int source, unsigned int target, > > pll_div->k = K; > > + if (unsupported) > + snd_soc_update_bits(codec, WM8960_CLOCK1, 0x6, > + WM8960_SYSCLK_DIV_2); OK so looking over this some more with Mark's comments in mind the thing that troubles me is this. Currently there are two ways you can configure the clocking manually or automatically. The manual way using the set_dai_pll, set_dai_clkdiv and set_dai_sysclk calls. Automatically by calling set_dai_sysclk and set_dai_pll with WM8960_SYSCLK_AUTO and the driver will work it out for you. This option already supports setting the SYSCLK_DIV. I guess my first question would be can you get the result you desire by just using the automatic option? If not, it seems like what we are trying to do here is move the set_dai_clkdiv for SYSCLK_DIV to be automatic in both cases. The problem is that I suspect this handling interfers somewhat with the handling of these bits that is done in wm8960_configure_clocking through wm8960_configure_pll when we are doing things automatically. In which case I think you need to look at how to align these two clocking methods. Thanks, Charles