> > > @@ -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)) { > > Maybe something like this? if (pll_in == pll_out) clk_id = RT5682S_SCLK_S_MCLK; else { /* 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 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, > > >