Re: [PATCH v3 15/20] ASoC: fsl_micfil: simplify clock setting

[Sascha Hauer <s.hauer@pengutronix.de>]

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 |