Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932619AbdCUONp (ORCPT ); Tue, 21 Mar 2017 10:13:45 -0400 Received: from mail-io0-f195.google.com ([209.85.223.195]:34532 "EHLO mail-io0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757087AbdCUONn (ORCPT ); Tue, 21 Mar 2017 10:13:43 -0400 MIME-Version: 1.0 In-Reply-To: <20170321125207.GX6986@localhost.localdomain> References: <1490090976-25877-1-git-send-email-daniel.baluta@nxp.com> <1490090976-25877-3-git-send-email-daniel.baluta@nxp.com> <20170321125207.GX6986@localhost.localdomain> From: Daniel Baluta Date: Tue, 21 Mar 2017 16:05:15 +0200 Message-ID: Subject: Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation To: Charles Keepax Cc: Daniel Baluta , alsa-devel@alsa-project.org, shengjiu.wang@freescale.com, patches@opensource.wolfsonmicro.com, Liam Girdwood , Linux Kernel Mailing List , Mark Brown , viorel.suman@nxp.com, mihai.serban@nxp.com, Takashi Iwai Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1939 Lines: 49 On Tue, Mar 21, 2017 at 2:52 PM, Charles Keepax wrote: > On Tue, Mar 21, 2017 at 12:09:36PM +0200, Daniel Baluta wrote: >> WM8960 derives bit clock from sysclock using BCLKDIV[3:0] of R8 >> clocking register (See WM8960 datasheet, page 71). >> >> There are use cases, like this: >> aplay -Dhw:0,0 -r 48000 -c 1 -f S20_3LE -t raw audio48k20b_3LE1c.pcm >> >> where no BCLKDIV applied to sysclock can give us the exact requested >> bitclk, so driver fails to configure clocking and aplay fails to run. >> >> Fix this by relaxing bitclk computation, so that when no exact value >> can be derived from sysclk pick the closest value greater than >> expected bitclk. >> >> Suggested-by: Charles Keepax >> Signed-off-by: Daniel Baluta >> --- >> Changes since v1: >> * use a marker to check if a match is found >> * didn't removed PLL as Charles suggested because there is >> a special PLL mode which explictly uses PLL. We could start >> a discussion on not using PLL when deriving bitclk, but this >> is to be done in another patch. >> > > Could you elaborate on this a little more am I not sure I follow > 100%? There is a mode which explictly requires the PLL to be used > (WM8960_SYSCLK_PLL) but in that case your wm8960_configure_sysclk > code will not be called so I don't see what is causing that to have > an effect on this patch? My doubt is, what happens if wm8960_configure_clocking is called with wm8960->clk_id = WM8960_SYSCLK_PLL and we remove the PLL as suggested. Is this possible even possible? Anyhow, I noticed that so far wm8960->clk_id is never set to WM8960_SYSCLK_PLL :). So, my proposal is to merge this patch which improves the existing code and deal with PLL fallback in a separate patch later. I am afraid of touching things which I don't understand how they work :D. thanks, Daniel.