| Message ID | 1527056028-12552-1-git-send-email-mac.chiang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23 May 2018 07:14, Mac Chiang wrote: > From: Mac Chiang <mac.chiang@intel.com> > > unexpected error: PLL input clock 0 below valid range > > backport from broonie's tree > commit fd0f237572ad > ("ASoC: Intel: bxt: Move codec sysclk config to codec_init function") > > The MCLK for DA7219 does not change in this platform, but is > currently being configured everytime as part of the platform_clock > event handler for DAPM. The upshot of this is that we have > unnecessary calls to this function, and it also means that if > a stream hasn't yet been started, DA7219 driver does not have the > correct MCLK rates programmed and so the HP detection feature does > not operate as expected. > > This patch rectifies this issue by moving the sysclk call to > codec_init function so it's only called once at initialisation. > > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> > Acked-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com> > Singed-off-by: Mac Chiang <mac.chiang@intel.com> > --- > sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c > b/sound/soc/intel/boards/kbl_da7219_max98357a.c > index 60e739f..88fbc6a 100644 > --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c > +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c > @@ -65,14 +65,6 @@ static int platform_clock_control(struct > snd_soc_dapm_widget *w, > return -EIO; > } > > - /* Configure sysclk for codec */ > - ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000, > - SND_SOC_CLOCK_IN); Code here is setting the MCLK rate for the codec as being 24.576MHz but... > - if (ret) { > - dev_err(card->dev, "can't set codec sysclk configuration\n"); > - return ret; > - } > - > if (SND_SOC_DAPM_EVENT_OFF(event)) { > ret = snd_soc_dai_set_pll(codec_dai, 0, > DA7219_SYSCLK_MCLK, 0, 0); > @@ -169,9 +161,18 @@ static int kabylake_da7219_codec_init(struct > snd_soc_pcm_runtime *rtd) > { > struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card); > struct snd_soc_component *component = rtd->codec_dai->component; > + struct snd_soc_dai *codec_dai = rtd->codec_dai; > struct snd_soc_jack *jack; > int ret; > > + /* Configure sysclk for codec */ > + ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 19200000, > + SND_SOC_CLOCK_IN); ...here you're setting MCLK rate to be 19.2MHz. Is that correct? > + if (ret) { > + dev_err(rtd->dev, "can't set codec sysclk configuration\n"); > + return ret; > + } > + > /* > * Headset buttons map to the google Reference headset. > * These can be configured by userspace. > -- > 2.7.4 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 05/24/2018 04:45 AM, Adam Thomson wrote: > On 23 May 2018 07:14, Mac Chiang wrote: > >> From: Mac Chiang <mac.chiang@intel.com> >> >> unexpected error: PLL input clock 0 below valid range >> >> backport from broonie's tree >> commit fd0f237572ad >> ("ASoC: Intel: bxt: Move codec sysclk config to codec_init function") >> >> The MCLK for DA7219 does not change in this platform, but is >> currently being configured everytime as part of the platform_clock >> event handler for DAPM. The upshot of this is that we have >> unnecessary calls to this function, and it also means that if >> a stream hasn't yet been started, DA7219 driver does not have the >> correct MCLK rates programmed and so the HP detection feature does >> not operate as expected. >> >> This patch rectifies this issue by moving the sysclk call to >> codec_init function so it's only called once at initialisation. >> >> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> >> Acked-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com> >> Singed-off-by: Mac Chiang <mac.chiang@intel.com> Mac, I provided you internal feedback that your commit message was misleading. This is not a 'backport', the intent here is to fix a problem on KBL using the same solution already merged for ApolloLake with commit fd0f237572ad ("ASoC: Intel: bxt: Move codec sysclk config to codec_init function"). You also want to be careful when including someone else's Signed-off-by and check for typos, if you run checkpatch.pl --strict you'd see this: WARNING: 'everytime' may be misspelled - perhaps 'every time'? #14: currently being configured everytime as part of the platform_clock WARNING: Non-standard signature: Singed-off-by: #26: Singed-off-by: Mac Chiang <mac.chiang@intel.com> CHECK: Alignment should match open parenthesis #57: FILE: sound/soc/intel/boards/kbl_da7219_max98357a.c:170: + ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 19200000, + SND_SOC_CLOCK_IN); total: 0 errors, 2 warnings, 1 checks, 32 lines checked In addition, both Adam and I mentioned that the clocks are different between the two platforms, so this patch is incorrect and cannot be applied as is, see below. >> --- >> sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- >> 1 file changed, 9 insertions(+), 8 deletions(-) >> >> diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c >> b/sound/soc/intel/boards/kbl_da7219_max98357a.c >> index 60e739f..88fbc6a 100644 >> --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c >> +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c >> @@ -65,14 +65,6 @@ static int platform_clock_control(struct >> snd_soc_dapm_widget *w, >> return -EIO; >> } >> >> - /* Configure sysclk for codec */ >> - ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000, >> - SND_SOC_CLOCK_IN); > Code here is setting the MCLK rate for the codec as being 24.576MHz but... > >> - if (ret) { >> - dev_err(card->dev, "can't set codec sysclk configuration\n"); >> - return ret; >> - } >> - >> if (SND_SOC_DAPM_EVENT_OFF(event)) { >> ret = snd_soc_dai_set_pll(codec_dai, 0, >> DA7219_SYSCLK_MCLK, 0, 0); >> @@ -169,9 +161,18 @@ static int kabylake_da7219_codec_init(struct >> snd_soc_pcm_runtime *rtd) >> { >> struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card); >> struct snd_soc_component *component = rtd->codec_dai->component; >> + struct snd_soc_dai *codec_dai = rtd->codec_dai; >> struct snd_soc_jack *jack; >> int ret; >> >> + /* Configure sysclk for codec */ >> + ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 19200000, >> + SND_SOC_CLOCK_IN); > ...here you're setting MCLK rate to be 19.2MHz. Is that correct? That is clearly wrong. Likely a copy/paste from bxt_da7219_max98357a.c instead of moving the code within the same file. > >> + if (ret) { >> + dev_err(rtd->dev, "can't set codec sysclk configuration\n"); >> + return ret; >> + } >> + >> /* >> * Headset buttons map to the google Reference headset. >> * These can be configured by userspace. >> -- >> 2.7.4 >> >> _______________________________________________ >> Alsa-devel mailing list >> Alsa-devel@alsa-project.org >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c index 60e739f..88fbc6a 100644 --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c @@ -65,14 +65,6 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w, return -EIO; } - /* Configure sysclk for codec */ - ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000, - SND_SOC_CLOCK_IN); - if (ret) { - dev_err(card->dev, "can't set codec sysclk configuration\n"); - return ret; - } - if (SND_SOC_DAPM_EVENT_OFF(event)) { ret = snd_soc_dai_set_pll(codec_dai, 0, DA7219_SYSCLK_MCLK, 0, 0); @@ -169,9 +161,18 @@ static int kabylake_da7219_codec_init(struct snd_soc_pcm_runtime *rtd) { struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card); struct snd_soc_component *component = rtd->codec_dai->component; + struct snd_soc_dai *codec_dai = rtd->codec_dai; struct snd_soc_jack *jack; int ret; + /* Configure sysclk for codec */ + ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 19200000, + SND_SOC_CLOCK_IN); + if (ret) { + dev_err(rtd->dev, "can't set codec sysclk configuration\n"); + return ret; + } + /* * Headset buttons map to the google Reference headset. * These can be configured by userspace.