From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 79EAAC433EF for ; Thu, 7 Apr 2022 07:09:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240506AbiDGHK7 (ORCPT ); Thu, 7 Apr 2022 03:10:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52950 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240861AbiDGHK5 (ORCPT ); Thu, 7 Apr 2022 03:10:57 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 369EB17067 for ; Thu, 7 Apr 2022 00:08:58 -0700 (PDT) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ncMGC-00030f-DQ; Thu, 07 Apr 2022 09:08:52 +0200 Received: from sha by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1ncMGB-0004DK-Vx; Thu, 07 Apr 2022 09:08:51 +0200 Date: Thu, 7 Apr 2022 09:08:51 +0200 From: Sascha Hauer To: Shengjiu Wang Cc: alsa-devel@alsa-project.org, Xiubo Li , Fabio Estevam , Sascha Hauer , Vinod Koul , NXP Linux Team , dmaengine@vger.kernel.org Subject: Re: [PATCH v3 15/20] ASoC: fsl_micfil: simplify clock setting Message-ID: <20220407070851.GZ4012@pengutronix.de> References: <20220405075959.2744803-1-s.hauer@pengutronix.de> <20220405075959.2744803-16-s.hauer@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 09:07:10 up 7 days, 19:36, 62 users, load average: 0.14, 0.46, 0.44 User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: dmaengine@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org On Thu, Apr 07, 2022 at 01:09:37PM +0800, Shengjiu Wang wrote: > On Tue, Apr 5, 2022 at 4:00 PM Sascha Hauer <[1]s.hauer@pengutronix.de> > wrote: > > The reference manual has this for calculating the micfil internal clock > divider: > >          MICFIL Clock rate > clkdiv = ----------------- >          8 * OSR * outrate > > (with OSR == Oversampling Rate, outrate == output sample rate) > > The driver first sets the MICFIL Clock rate to (outrate * 1024) and then > calculates back the clkdiv value from the above calculation. > > Simplify this by using a fixed clkdiv value of 8 and set the MICFIL > Clock rate to (outrate * clkdiv * OSR * 8). > > While at it drop disabling the clock before setting its rate. The MICFIL > module is disabled when the rate is changed and it is also resetted > before it is started again, so I doubt it's necessary to disable the > clock. > > Signed-off-by: Sascha Hauer <[2]s.hauer@pengutronix.de> > --- >  sound/soc/fsl/fsl_micfil.c | 45 ++++---------------------------------- >  1 file changed, 4 insertions(+), 41 deletions(-) > > diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c > index 8335646a84d17..fd3b168a38661 100644 > --- a/sound/soc/fsl/fsl_micfil.c > +++ b/sound/soc/fsl/fsl_micfil.c > @@ -111,19 +111,6 @@ static const struct snd_kcontrol_new > fsl_micfil_snd_controls[] = { >                      snd_soc_get_enum_double, snd_soc_put_enum_double), >  }; > > -static inline int get_clk_div(struct fsl_micfil *micfil, > -                             unsigned int rate) > -{ > -       long mclk_rate; > -       int clk_div; > - > -       mclk_rate = clk_get_rate(micfil->mclk); > - > -       clk_div = mclk_rate / (rate * micfil->osr * 8); > - > -       return clk_div; > -} > - >  /* The SRES is a self-negated bit which provides the CPU with the >   * capability to initialize the PDM Interface module through the >   * slave-bus interface. This bit always reads as zero, and this > @@ -147,24 +134,6 @@ static int fsl_micfil_reset(struct device *dev) >         return 0; >  } > > -static int fsl_micfil_set_mclk_rate(struct fsl_micfil *micfil, > -                                   unsigned int freq) > -{ > -       struct device *dev = &micfil->pdev->dev; > -       int ret; > - > -       clk_disable_unprepare(micfil->mclk); > - > -       ret = clk_set_rate(micfil->mclk, freq * 1024); > -       if (ret) > -               dev_warn(dev, "failed to set rate (%u): %d\n", > -                        freq * 1024, ret); > - > -       clk_prepare_enable(micfil->mclk); > - > -       return ret; > -} > - >  static int fsl_micfil_startup(struct snd_pcm_substream *substream, >                               struct snd_soc_dai *dai) >  { > @@ -238,13 +207,12 @@ static int fsl_micfil_trigger(struct > snd_pcm_substream *substream, int cmd, >  static int fsl_set_clock_params(struct device *dev, unsigned int rate) >  { >         struct fsl_micfil *micfil = dev_get_drvdata(dev); > -       int clk_div; > +       int clk_div = 8; >         int ret; > > -       ret = fsl_micfil_set_mclk_rate(micfil, rate); > -       if (ret < 0) > -               dev_err(dev, "failed to set mclk[%lu] to rate %u\n", > -                       clk_get_rate(micfil->mclk), rate); > +       ret = clk_set_rate(micfil->mclk, rate * clk_div * micfil->osr * > 8); > > Please make sure micfil->osr is assigned. Should also be MICFIL_OSR_DEFAULT instead. The micfil->osr field sneaked in during development and I decided against it finally. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 64DBCC433EF for ; Thu, 7 Apr 2022 07:09:55 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id F219F173F; Thu, 7 Apr 2022 09:09:02 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz F219F173F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1649315393; bh=rKImBfnpqvsHeFOIJzB0wwSu/+x9V1MD1lVsK77lX1Q=; h=Date:From:To:Subject:References:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=LNmJi/zD5dDGventVBtxQS1rCZZkvhRMi8gt5vPU8qndhEyOmGD/8cxWqmu1Q+jHB 0fba3woWdDXrU8ErmAkWpFi+qh+5GLkqgmbaFcS6i98BLLiMr//cVYMAYRRYWG/vuR 0Xw+mqJMKwm1UO3a8LZYOjZqzl57yRwxXQIujowU= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 7F17DF80141; Thu, 7 Apr 2022 09:09:02 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 2A2FDF8024C; Thu, 7 Apr 2022 09:09:00 +0200 (CEST) Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 15492F80085 for ; Thu, 7 Apr 2022 09:08:53 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 15492F80085 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ncMGC-00030f-DQ; Thu, 07 Apr 2022 09:08:52 +0200 Received: from sha by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1ncMGB-0004DK-Vx; Thu, 07 Apr 2022 09:08:51 +0200 Date: Thu, 7 Apr 2022 09:08:51 +0200 From: Sascha Hauer To: Shengjiu Wang Subject: Re: [PATCH v3 15/20] ASoC: fsl_micfil: simplify clock setting Message-ID: <20220407070851.GZ4012@pengutronix.de> References: <20220405075959.2744803-1-s.hauer@pengutronix.de> <20220405075959.2744803-16-s.hauer@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 09:07:10 up 7 days, 19:36, 62 users, load average: 0.14, 0.46, 0.44 User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: alsa-devel@alsa-project.org Cc: alsa-devel@alsa-project.org, Xiubo Li , Vinod Koul , NXP Linux Team , Sascha Hauer , dmaengine@vger.kernel.org, Fabio Estevam X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Thu, Apr 07, 2022 at 01:09:37PM +0800, Shengjiu Wang wrote: > On Tue, Apr 5, 2022 at 4:00 PM Sascha Hauer <[1]s.hauer@pengutronix.de> > wrote: > > The reference manual has this for calculating the micfil internal clock > divider: > >          MICFIL Clock rate > clkdiv = ----------------- >          8 * OSR * outrate > > (with OSR == Oversampling Rate, outrate == output sample rate) > > The driver first sets the MICFIL Clock rate to (outrate * 1024) and then > calculates back the clkdiv value from the above calculation. > > Simplify this by using a fixed clkdiv value of 8 and set the MICFIL > Clock rate to (outrate * clkdiv * OSR * 8). > > While at it drop disabling the clock before setting its rate. The MICFIL > module is disabled when the rate is changed and it is also resetted > before it is started again, so I doubt it's necessary to disable the > clock. > > Signed-off-by: Sascha Hauer <[2]s.hauer@pengutronix.de> > --- >  sound/soc/fsl/fsl_micfil.c | 45 ++++---------------------------------- >  1 file changed, 4 insertions(+), 41 deletions(-) > > diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c > index 8335646a84d17..fd3b168a38661 100644 > --- a/sound/soc/fsl/fsl_micfil.c > +++ b/sound/soc/fsl/fsl_micfil.c > @@ -111,19 +111,6 @@ static const struct snd_kcontrol_new > fsl_micfil_snd_controls[] = { >                      snd_soc_get_enum_double, snd_soc_put_enum_double), >  }; > > -static inline int get_clk_div(struct fsl_micfil *micfil, > -                             unsigned int rate) > -{ > -       long mclk_rate; > -       int clk_div; > - > -       mclk_rate = clk_get_rate(micfil->mclk); > - > -       clk_div = mclk_rate / (rate * micfil->osr * 8); > - > -       return clk_div; > -} > - >  /* The SRES is a self-negated bit which provides the CPU with the >   * capability to initialize the PDM Interface module through the >   * slave-bus interface. This bit always reads as zero, and this > @@ -147,24 +134,6 @@ static int fsl_micfil_reset(struct device *dev) >         return 0; >  } > > -static int fsl_micfil_set_mclk_rate(struct fsl_micfil *micfil, > -                                   unsigned int freq) > -{ > -       struct device *dev = &micfil->pdev->dev; > -       int ret; > - > -       clk_disable_unprepare(micfil->mclk); > - > -       ret = clk_set_rate(micfil->mclk, freq * 1024); > -       if (ret) > -               dev_warn(dev, "failed to set rate (%u): %d\n", > -                        freq * 1024, ret); > - > -       clk_prepare_enable(micfil->mclk); > - > -       return ret; > -} > - >  static int fsl_micfil_startup(struct snd_pcm_substream *substream, >                               struct snd_soc_dai *dai) >  { > @@ -238,13 +207,12 @@ static int fsl_micfil_trigger(struct > snd_pcm_substream *substream, int cmd, >  static int fsl_set_clock_params(struct device *dev, unsigned int rate) >  { >         struct fsl_micfil *micfil = dev_get_drvdata(dev); > -       int clk_div; > +       int clk_div = 8; >         int ret; > > -       ret = fsl_micfil_set_mclk_rate(micfil, rate); > -       if (ret < 0) > -               dev_err(dev, "failed to set mclk[%lu] to rate %u\n", > -                       clk_get_rate(micfil->mclk), rate); > +       ret = clk_set_rate(micfil->mclk, rate * clk_div * micfil->osr * > 8); > > Please make sure micfil->osr is assigned. Should also be MICFIL_OSR_DEFAULT instead. The micfil->osr field sneaked in during development and I decided against it finally. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |