From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754803AbdLLNVz (ORCPT ); Tue, 12 Dec 2017 08:21:55 -0500 Received: from mx0a-001ae601.pphosted.com ([67.231.149.25]:38272 "EHLO mx0b-001ae601.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754742AbdLLNVx (ORCPT ); Tue, 12 Dec 2017 08:21:53 -0500 Authentication-Results: ppops.net; spf=none smtp.mailfrom=ckeepax@opensource.cirrus.com Date: Tue, 12 Dec 2017 13:21:04 +0000 From: Charles Keepax To: "Chen.Liu" CC: , , , , , , , Subject: Re: [PATCH] ASOC: wm8960: Add multiple MCLK frequency support Message-ID: <20171212132104.vnocewljituntwin@localhost.localdomain> References: <1513079936-163297-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: <1513079936-163297-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=1011 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-1712120193 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 12, 2017 at 07:58:56PM +0800, Chen.Liu wrote: > From: "Chen.Liu" Please make sure to CC patches@opensource.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