> Subject: Re: [PATCH 1/2] ASoC: rt5682s: Add driver for ALC5682I-VS codec > > On 8/26/2021 4:09 PM, derek.fang@xxxxxxxxxxx wrote: > > + > > +static int is_sys_clk_from_plla(struct snd_soc_dapm_widget *w, > > + struct snd_soc_dapm_widget *sink) > > +{ > > + struct snd_soc_component *component = > snd_soc_dapm_to_component(w->dapm); > > + struct rt5682s_priv *rt5682s = > snd_soc_component_get_drvdata(component); > > + > > + if ((rt5682s->sysclk_src == RT5682S_CLK_SRC_PLL1) || > > + (rt5682s->sysclk_src == RT5682S_CLK_SRC_PLL2 && > > + rt5682s->pll_comb == USE_PLLAB)) > > > What about rt5682s->pll_comb == USE_PLLA? Thanks for review. My design thought of this check function is that pll_comb would be set via snd_soc_dai_set_pll and sysclk_src would be set via snd_soc_dai_set_sysclk. So I want to make sure the two settings are both correct first and then power PLLA/B. Otherwise, not power, although the chance of mistake is rare. ___ +-->[PLLA]-+-------------------> PLL1 out |Clk| | | ___ |Mux|--| +-->|Clk| |___| | |Mux|-->[PLLB]--> PLL2 out +------------->|___| > > > + return 1; > > + > > + return 0; > > +} > > + > > +static int is_sys_clk_from_pllb(struct snd_soc_dapm_widget *w, > > + struct snd_soc_dapm_widget *sink) > > +{ > > + struct snd_soc_component *component = > snd_soc_dapm_to_component(w->dapm); > > + struct rt5682s_priv *rt5682s = > snd_soc_component_get_drvdata(component); > > + > > + if (rt5682s->sysclk_src == RT5682S_CLK_SRC_PLL2) > > > Should we also test rt5682s->pll_comb here? > > > > + return 1; > > + > > + return 0; > > +} > > > ------Please consider the environment before printing this e-mail.