> > @@ -362,6 +361,9 @@ static int sof_rt5682_hw_params(struct
> snd_pcm_substream *substream,
> > if (sof_rt5682_quirk & SOF_RT5682S_HEADPHONE_CODEC_PRESENT)
> {
> > pll_id = RT5682S_PLL2;
> > clk_id = RT5682S_SCLK_S_PLL2;
> > +
> > + if (pll_in == 24576000)
> > + clk_id = RT5682S_SCLK_S_MCLK;
>
> this case only affects the RT5682s case.
Thanks Pierre, you are right. Per discussed with Shuming, both 5682vs and 5682vd have to affect.
>
> > } else {
> > pll_id = RT5682_PLL1;
> > clk_id = RT5682_SCLK_S_PLL1;
>
> for the RT5682 we keep using the PLL. Is this intentional?
>
> I also wonder if we can avoid the hard-coding. Can we use e.g.
>
> if (pll_in == params_rate(params) * 512)
Agree, no hard-coding.
> ?
>
>
> > @@ -369,11 +371,14 @@ static int sof_rt5682_hw_params(struct
> > snd_pcm_substream *substream,
> >
> > pll_out = params_rate(params) * 512;
> >
> > - /* Configure pll for codec */
> > - ret = snd_soc_dai_set_pll(codec_dai, pll_id, pll_source, pll_in,
> > - pll_out);
> > - if (ret < 0)
> > - dev_err(rtd->dev, "snd_soc_dai_set_pll err = %d\n", ret);
> > + /* when MCLK is 512FS, no need to set PLL configuration additionally.
> */
> > + if (pll_in != 24576000) {
>
> can we use if (pll_in == pll_out) ?
Agree, will modify as if (!(pll_in == pll_out)) {
>
> > + /* Configure pll for codec */
> > + ret = snd_soc_dai_set_pll(codec_dai, pll_id, pll_source, pll_in,
> > + pll_out);
> > + if (ret < 0)
> > + dev_err(rtd->dev, "snd_soc_dai_set_pll err = %d\n",
> ret);
> > + }
> >
> > /* Configure sysclk for codec */
> > ret = snd_soc_dai_set_sysclk(codec_dai, clk_id,
> >
