| Message ID | 1501021659-8597-1-git-send-email-harshapriya.n@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 25, 2017 at 03:27:39PM -0700, Harsha Priya wrote: > This patch adds fixes for the following issues in Kabylake machine driver > > 1. crackling noise in rt5663 headset codec And the fix was..? > 2. enabling 4 slot IV feedback for max98927 speaker amp codec should that be a separate patch? > 3. pop noise while recording from rt5514 dmic codec and this fix was.. Harsha, from the changelog, apart from 2, I am still not clear on the contents of the patch. A patch changelog should document the code changes done, its impact (done here) to help reviewer understand the patch > > Signed-off-by: Harsha Priya <harshapriya.n@intel.com> > --- > .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 28 ++++++++++++---------- > 1 file changed, 15 insertions(+), 13 deletions(-) > > diff --git a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c > index 3fe4a08..4c51965 100644 > --- a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c > +++ b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c > @@ -319,7 +319,9 @@ static int kabylake_rt5663_hw_params(struct snd_pcm_substream *substream, > int ret; > > /* use ASRC for internal clocks, as PLL rate isn't multiple of BCLK */ > - rt5663_sel_asrc_clk_src(codec_dai->codec, RT5663_DA_STEREO_FILTER, 1); > + rt5663_sel_asrc_clk_src(codec_dai->codec, > + RT5663_DA_STEREO_FILTER | RT5663_AD_STEREO_FILTER, > + RT5663_CLK_SEL_I2S1_ASRC); So now the clock source is ARSC right, how will that fix something? > > ret = snd_soc_dai_set_sysclk(codec_dai, > RT5663_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN); > @@ -349,27 +351,27 @@ static int kabylake_ssp0_hw_params(struct snd_pcm_substream *substream, > return ret; > } > > - ret = snd_soc_dai_set_pll(codec_dai, 0, > - RT5514_PLL1_S_BCLK, RT5514_AIF1_BCLK_FREQ, > - RT5514_AIF1_SYSCLK_FREQ); > + ret = snd_soc_dai_set_sysclk(codec_dai, > + RT5514_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN); now you no longer need to set pll? > + > if (ret < 0) { > - dev_err(rtd->dev, "set bclk err: %d\n", ret); > + dev_err(rtd->dev, "set sysclk err: %d\n", ret); > return ret; > } > + } > > - ret = snd_soc_dai_set_sysclk(codec_dai, > - RT5514_SCLK_S_PLL1, RT5514_AIF1_SYSCLK_FREQ, > - SND_SOC_CLOCK_IN); > + if (!strcmp(codec_dai->component->name, MAXIM_DEV0_NAME)) { > + ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x30, 3, 8, 16); > if (ret < 0) { > - dev_err(rtd->dev, "set sclk err: %d\n", ret); > + dev_err(rtd->dev, "DEV0 TDM slot err:%d\n", ret); > return ret; > } > } > - if (!strcmp(codec_dai->component->name, MAXIM_DEV0_NAME) || > - !strcmp(codec_dai->component->name, MAXIM_DEV1_NAME)) { > - ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xF0, 3, 8, 16); > + > + if (!strcmp(codec_dai->component->name, MAXIM_DEV1_NAME)) { > + ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xC0, 3, 8, 16); > if (ret < 0) { > - dev_err(rtd->dev, "set TDM slot err:%d\n", ret); > + dev_err(rtd->dev, "DEV1 TDM slot err:%d\n", ret); > return ret; > } > }
On Tue, Jul 25, 2017 at 03:27:39PM -0700, Harsha Priya wrote: > This patch adds fixes for the following issues in Kabylake machine driver > > 1. crackling noise in rt5663 headset codec > 2. enabling 4 slot IV feedback for max98927 speaker amp codec > 3. pop noise while recording from rt5514 dmic codec This is three different fixes so I'd expect to see three different patches and the changelog doesn't describe what the fixes actually do so it's much harder to review.
> -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Wednesday, July 26, 2017 4:50 AM > To: N, Harshapriya <harshapriya.n@intel.com> > Cc: tiwai@suse.de; Koul, Vinod <vinod.koul@intel.com>; alsa-devel@alsa- > project.org > Subject: Re: [PATCH] ASoC: Intel: Fix Kabylake machine driver for pops and > clicks > > On Tue, Jul 25, 2017 at 03:27:39PM -0700, Harsha Priya wrote: > > This patch adds fixes for the following issues in Kabylake machine > > driver > > > > 1. crackling noise in rt5663 headset codec 2. enabling 4 slot IV > > feedback for max98927 speaker amp codec 3. pop noise while recording > > from rt5514 dmic codec > > This is three different fixes so I'd expect to see three different patches and > the changelog doesn't describe what the fixes actually do so it's much harder > to review. I am sending a patch series with 3 patches in it each fixing one issue. I have tried explaining in detail the fix in the commit message as well.
diff --git a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c index 3fe4a08..4c51965 100644 --- a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c +++ b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c @@ -319,7 +319,9 @@ static int kabylake_rt5663_hw_params(struct snd_pcm_substream *substream, int ret; /* use ASRC for internal clocks, as PLL rate isn't multiple of BCLK */ - rt5663_sel_asrc_clk_src(codec_dai->codec, RT5663_DA_STEREO_FILTER, 1); + rt5663_sel_asrc_clk_src(codec_dai->codec, + RT5663_DA_STEREO_FILTER | RT5663_AD_STEREO_FILTER, + RT5663_CLK_SEL_I2S1_ASRC); ret = snd_soc_dai_set_sysclk(codec_dai, RT5663_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN); @@ -349,27 +351,27 @@ static int kabylake_ssp0_hw_params(struct snd_pcm_substream *substream, return ret; } - ret = snd_soc_dai_set_pll(codec_dai, 0, - RT5514_PLL1_S_BCLK, RT5514_AIF1_BCLK_FREQ, - RT5514_AIF1_SYSCLK_FREQ); + ret = snd_soc_dai_set_sysclk(codec_dai, + RT5514_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN); + if (ret < 0) { - dev_err(rtd->dev, "set bclk err: %d\n", ret); + dev_err(rtd->dev, "set sysclk err: %d\n", ret); return ret; } + } - ret = snd_soc_dai_set_sysclk(codec_dai, - RT5514_SCLK_S_PLL1, RT5514_AIF1_SYSCLK_FREQ, - SND_SOC_CLOCK_IN); + if (!strcmp(codec_dai->component->name, MAXIM_DEV0_NAME)) { + ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x30, 3, 8, 16); if (ret < 0) { - dev_err(rtd->dev, "set sclk err: %d\n", ret); + dev_err(rtd->dev, "DEV0 TDM slot err:%d\n", ret); return ret; } } - if (!strcmp(codec_dai->component->name, MAXIM_DEV0_NAME) || - !strcmp(codec_dai->component->name, MAXIM_DEV1_NAME)) { - ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xF0, 3, 8, 16); + + if (!strcmp(codec_dai->component->name, MAXIM_DEV1_NAME)) { + ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xC0, 3, 8, 16); if (ret < 0) { - dev_err(rtd->dev, "set TDM slot err:%d\n", ret); + dev_err(rtd->dev, "DEV1 TDM slot err:%d\n", ret); return ret; } }
This patch adds fixes for the following issues in Kabylake machine driver 1. crackling noise in rt5663 headset codec 2. enabling 4 slot IV feedback for max98927 speaker amp codec 3. pop noise while recording from rt5514 dmic codec Signed-off-by: Harsha Priya <harshapriya.n@intel.com> --- .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 28 ++++++++++++---------- 1 file changed, 15 insertions(+), 13 deletions(-)