RE: [PATCH v3] ASoC: amd: add support for rt5682 codec in machine driver

["Shuming [范書銘]" <shumingf@realtek.com>]

> On 3/16/21 8:37 AM, Mukunda,Vijendar wrote:
> >
> >
> > On 15/03/21 9:30 pm, Pierre-Louis Bossart wrote:
> >>
> >>> +static int rt5682_clk_enable(struct snd_pcm_substream *substream) {
> >>> +    int ret;
> >>> +    struct snd_soc_pcm_runtime *rtd =
> >>> +asoc_substream_to_rtd(substream);
> >>> +
> >>> +    /*
> >>> +     * Set wclk to 48000 because the rate constraint of this driver
> >>> +is
> >>> +     * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate
> >>> +that is
> >>> +     * minimum of 64x the LRCLK sample rate." RT5682 is the only
> >>> +clk
> >>> +     * source so for all codecs we have to limit bclk to 64X lrclk.
> >>> +     */
> >>> +    clk_set_rate(rt5682_dai_wclk, 48000);
> >>> +    clk_set_rate(rt5682_dai_bclk, 48000 * 64);
> >>> +    ret = clk_prepare_enable(rt5682_dai_bclk);
> >>> +    if (ret < 0) {
> >>> +        dev_err(rtd->dev, "can't enable master clock %d\n", ret);
> >>> +        return ret;
> >>> +    }
> >>> +    return ret;
> >>> +}
> >>
> >> Out of curiosity, is there a reason why you use clk_prepare_enable()
> >> for the bclk but not for the wclk?These changes were shared by codec
> >> vendor as an initial patch.
> > We should use clk_prepare_enable() for wclk not for bclk.
> > We will update and share the new patch.
> 
> Well the question remains: if you have two clocks and only enable one, why do
> you need to get two clocks.
> 
> Also this patch was modeled after the da7219 case, where the same open
> applies.

The RT5682 could set the wclk and bclk separately.
The bclk could set the different ratios (32/64/128/256).
However, only the wclk of DAI clock has the .prepare callback function to enable the related power.
The next patch should update the clk_prepare_enable() for wclk.

> ------Please consider the environment before printing this e-mail.