[PATCH v2] ASoC: Intel: boards: eve: Fix DMIC records zero
N, Harshapriya
harshapriya.n at intel.com
Thu Aug 13 19:01:04 CEST 2020
Abandoning this patch as it some clock changes that is already fixed in Brent's older patch
(handled it in a different way).
Will make a separate patch for other fixes once the complete solution is ready.
>
> >> > >
> > > This patch adds a dapm route to provide early mclk/sclk for for DMIC
> > > on
> > > SSP0(rt5514) and Headset on SSP1(rt5663).
> > >
> > > The sclk rate for both codecs is different which is taken care of in
> > > the platform_clock_control().The platform has one mclk and one sclk.
> > > Two variables sclk0 and sclk1 are used for the two codecs on
> > > differnet ssp that use different clock rate.
> > >
> > > This change ensures the DMIC PCM port will return valid data
> > >
> > > Signed-off-by: Brent Lu <brent.lu at intel.com>
> > > Signed-off-by: Vamshi Krishna Gopal <vamshi.krishna.gopal at intel.com>
> > > Signed-off-by: Harsha Priya <harshapriya.n at intel.com>
> > Hi Harsha,
> >
> > Isn't it working? I tested it on a eve before upstreaming. Thanks.
> >
> > commit 15747a80207585fe942416025540c0ff34e2aef8
> > Author: Brent Lu <brent.lu at intel.com>
> > Date: Fri Oct 25 17:11:31 2019 +0800
> >
> > ASoC: eve: implement set_bias_level function for rt5514
> >
> > The first DMIC capture always fail (zero sequence data from PCM port)
> > after using DSP hotwording function (i.e. Google assistant).
> >
> > This rt5514 codec requires to control mclk directly in the set_bias_level
> > function. Implement this function in machine driver to control the
> > ssp1_mclk clock explicitly could fix this issue.
> >
> > Signed-off-by: Brent Lu <brent.lu at intel.com>
> > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> > Link: https://lore.kernel.org/r/1571994691-20199-1-git-send-email-
> > brent.lu at intel.com
> > Signed-off-by: Mark Brown <broonie at kernel.org>
> Your patch is necessary and is being used.
> But we found few issues during kernel uprev where we need this current patch
> also to get it working
> >
> > Regards,
> > Brent
> >
> > > ---
> > > v1 -> v2:
> > > - Only one mclk with same rate is used, so changed to using one
> > > variable
> > > - dropping ssp_ prefix from sclk variable names to make them sound right.
> > > - removing a return statment that was not required
> > > - fixed commit message accordingly
> > >
> > > .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 146
> > > ++++++++++++++-------
> > > 1 file changed, 97 insertions(+), 49 deletions(-)
> > >
> > > diff --git a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
> > > b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
> > > index b34cf6c..155f2b4 100644
> > > --- a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
> > > +++ b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c
> > > @@ -54,7 +54,8 @@ struct kbl_codec_private {
> > > struct list_head hdmi_pcm_list;
> > > struct snd_soc_jack kabylake_hdmi[2];
> > > struct clk *mclk;
> > > - struct clk *sclk;
> > > + struct clk *sclk0;
> > > + struct clk *sclk1;
> > > };
> > >
> > > enum {
> > > @@ -77,13 +78,29 @@ static const struct snd_kcontrol_new
> > > kabylake_controls[] = { };
> > >
> > > static int platform_clock_control(struct snd_soc_dapm_widget *w,
> > > - struct snd_kcontrol *k, int event)
> > > + struct snd_kcontrol *k, int event, int ssp_num)
> > > {
> > > struct snd_soc_dapm_context *dapm = w->dapm;
> > > struct snd_soc_card *card = dapm->card;
> > > struct kbl_codec_private *priv = snd_soc_card_get_drvdata(card);
> > > + struct clk *sclk;
> > > + unsigned long sclk_rate;
> > > int ret = 0;
> > >
> > > + switch (ssp_num) {
> > > + case 0:
> > > + sclk = priv->sclk0;
> > > + sclk_rate = 6144000;
> > > + break;
> > > + case 1:
> > > + sclk = priv->sclk1;
> > > + sclk_rate = 3072000;
> > > + break;
> > > + default:
> > > + dev_err(card->dev, "Invalid ssp_num %d\n", ssp_num);
> > > + return -EINVAL;
> > > + }
> > > +
> > > /*
> > > * MCLK/SCLK need to be ON early for a successful synchronization of
> > > * codec internal clock. And the clocks are turned off during @@ -
> > > 91,38 +108,48 @@ static int platform_clock_control(struct
> > > snd_soc_dapm_widget *w,
> > > */
> > > switch (event) {
> > > case SND_SOC_DAPM_PRE_PMU:
> > > - /* Enable MCLK */
> > > ret = clk_set_rate(priv->mclk, 24000000);
> > > if (ret < 0) {
> > > - dev_err(card->dev, "Can't set rate for mclk, err:
> > > %d\n",
> > > - ret);
> > > - return ret;
> > > + dev_err(card->dev, "Can't set rate for mclk for ssp%d,
> > > err: %d\n",
> > > + ssp_num, ret);
> > > + return ret;
> > > }
> > >
> > > - ret = clk_prepare_enable(priv->mclk);
> > > - if (ret < 0) {
> > > - dev_err(card->dev, "Can't enable mclk, err: %d\n",
> > > ret);
> > > - return ret;
> > > + if (!__clk_is_enabled(priv->mclk)) {
> > > + /* Enable MCLK */
> > > + ret = clk_prepare_enable(priv->mclk);
> > > + if (ret < 0) {
> > > + dev_err(card->dev, "Can't enable mclk for
> > > ssp%d, err: %d\n",
> > > + ssp_num, ret);
> > > + return ret;
> > > + }
> > > }
> > >
> > > - /* Enable SCLK */
> > > - ret = clk_set_rate(priv->sclk, 3072000);
> > > + ret = clk_set_rate(sclk, sclk_rate);
> > > if (ret < 0) {
> > > - dev_err(card->dev, "Can't set rate for sclk, err:
> > > %d\n",
> > > - ret);
> > > + dev_err(card->dev, "Can't set rate for sclk for ssp%d,
> > > err: %d\n",
> > > + ssp_num, ret);
> > > clk_disable_unprepare(priv->mclk);
> > > return ret;
> > > }
> > >
> > > - ret = clk_prepare_enable(priv->sclk);
> > > - if (ret < 0) {
> > > - dev_err(card->dev, "Can't enable sclk, err: %d\n",
> > > ret);
> > > - clk_disable_unprepare(priv->mclk);
> > > + if (!__clk_is_enabled(sclk)) {
> > > + /* Enable SCLK */
> > > + ret = clk_prepare_enable(sclk);
> > > + if (ret < 0) {
> > > + dev_err(card->dev, "Can't enable sclk for
> > > ssp%d, err: %d\n",
> > > + ssp_num, ret);
> > > + clk_disable_unprepare(priv->mclk);
> > > + return ret;
> > > + }
> > > }
> > > break;
> > > case SND_SOC_DAPM_POST_PMD:
> > > - clk_disable_unprepare(priv->mclk);
> > > - clk_disable_unprepare(priv->sclk);
> > > + if (__clk_is_enabled(priv->mclk))
> > > + clk_disable_unprepare(priv->mclk);
> > > +
> > > + if (__clk_is_enabled(sclk))
> > > + clk_disable_unprepare(sclk);
> > > break;
> > > default:
> > > return 0;
> > > @@ -131,6 +158,18 @@ static int platform_clock_control(struct
> > > snd_soc_dapm_widget *w,
> > > return 0;
> > > }
> > >
> > > +static int platform_clock_control_ssp0(struct snd_soc_dapm_widget *w,
> > > + struct snd_kcontrol *k, int event) {
> > > + return platform_clock_control(w, k, event, 0); }
> > > +
> > > +static int platform_clock_control_ssp1(struct snd_soc_dapm_widget *w,
> > > + struct snd_kcontrol *k, int event) {
> > > + return platform_clock_control(w, k, event, 1); }
> > > +
> > > static const struct snd_soc_dapm_widget kabylake_widgets[] = {
> > > SND_SOC_DAPM_HP("Headphone Jack", NULL),
> > > SND_SOC_DAPM_MIC("Headset Mic", NULL), @@ -139,15 +178,17
> > @@ static
> > > const struct snd_soc_dapm_widget kabylake_widgets[] = {
> > > SND_SOC_DAPM_MIC("DMIC", NULL),
> > > SND_SOC_DAPM_SPK("HDMI1", NULL),
> > > SND_SOC_DAPM_SPK("HDMI2", NULL),
> > > - SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
> > > - platform_clock_control, SND_SOC_DAPM_PRE_PMU
> > > |
> > > + SND_SOC_DAPM_SUPPLY("Platform Clock SSP0", SND_SOC_NOPM,
> > > 0, 0,
> > > + platform_clock_control_ssp0,
> > > SND_SOC_DAPM_PRE_PMU |
> > > + SND_SOC_DAPM_POST_PMD),
> > > + SND_SOC_DAPM_SUPPLY("Platform Clock SSP1", SND_SOC_NOPM,
> > > 0, 0,
> > > + platform_clock_control_ssp1,
> > > SND_SOC_DAPM_PRE_PMU |
> > > SND_SOC_DAPM_POST_PMD),
> > > -
> > > };
> > >
> > > static const struct snd_soc_dapm_route kabylake_map[] = {
> > > /* Headphones */
> > > - { "Headphone Jack", NULL, "Platform Clock" },
> > > + { "Headphone Jack", NULL, "Platform Clock SSP1" },
> > > { "Headphone Jack", NULL, "HPOL" },
> > > { "Headphone Jack", NULL, "HPOR" },
> > >
> > > @@ -156,7 +197,7 @@ static const struct snd_soc_dapm_route
> > > kabylake_map[] = {
> > > { "Right Spk", NULL, "Right BE_OUT" },
> > >
> > > /* other jacks */
> > > - { "Headset Mic", NULL, "Platform Clock" },
> > > + { "Headset Mic", NULL, "Platform Clock SSP1" },
> > > { "IN1P", NULL, "Headset Mic" },
> > > { "IN1N", NULL, "Headset Mic" },
> > >
> > > @@ -180,6 +221,7 @@ static const struct snd_soc_dapm_route
> > > kabylake_map[] = {
> > > { "ssp0 Rx", NULL, "Right HiFi Capture" },
> > >
> > > /* DMIC */
> > > + { "DMIC", NULL, "Platform Clock SSP0" },
> > > { "DMIC1L", NULL, "DMIC" },
> > > { "DMIC1R", NULL, "DMIC" },
> > > { "DMIC2L", NULL, "DMIC" },
> > > @@ -666,7 +708,7 @@ static int kabylake_set_bias_level(struct
> > > snd_soc_card *card,
> > > if (!component || strcmp(component->name, RT5514_DEV_NAME))
> > > return 0;
> > >
> > > - if (IS_ERR(priv->mclk))
> > > + if (IS_ERR(priv->mclk0))
> > > return 0;
> > >
> > > /*
> > > @@ -757,6 +799,28 @@ static struct snd_soc_card kabylake_audio_card =
> {
> > > .late_probe = kabylake_card_late_probe, };
> > >
> > > +static int kabylake_audio_clk_get(struct device *dev, const char *id,
> > > + struct clk **clk)
> > > +{
> > > + int ret = 0;
> > > +
> > > + if (!clk)
> > > + return -EINVAL;
> > > +
> > > + *clk = devm_clk_get(dev, id);
> > > + if (IS_ERR(*clk)) {
> > > + ret = PTR_ERR(*clk);
> > > + if (ret == -ENOENT) {
> > > + dev_info(dev, "Failed to get %s, defer probe\n", id);
> > > + return -EPROBE_DEFER;
> > > + }
> > > +
> > > + dev_err(dev, "Failed to get %s with err:%d\n", id, ret);
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > static int kabylake_audio_probe(struct platform_device *pdev) {
> > > struct kbl_codec_private *ctx;
> > > @@ -777,33 +841,17 @@ static int kabylake_audio_probe(struct
> > > platform_device *pdev)
> > > dmic_constraints = mach->mach_params.dmic_num == 2 ?
> > > &constraints_dmic_2ch :
> > > &constraints_dmic_channels;
> > >
> > > - ctx->mclk = devm_clk_get(&pdev->dev, "ssp1_mclk");
> > > - if (IS_ERR(ctx->mclk)) {
> > > - ret = PTR_ERR(ctx->mclk);
> > > - if (ret == -ENOENT) {
> > > - dev_info(&pdev->dev,
> > > - "Failed to get ssp1_mclk, defer probe\n");
> > > - return -EPROBE_DEFER;
> > > - }
> > > -
> > > - dev_err(&pdev->dev, "Failed to get ssp1_mclk with
> > > err:%d\n",
> > > - ret);
> > > + ret = kabylake_audio_clk_get(&pdev->dev, "ssp0_sclk", &ctx->sclk0);
> > > + if (ret != 0)
> > > return ret;
> > > - }
> > >
> > > - ctx->sclk = devm_clk_get(&pdev->dev, "ssp1_sclk");
> > > - if (IS_ERR(ctx->sclk)) {
> > > - ret = PTR_ERR(ctx->sclk);
> > > - if (ret == -ENOENT) {
> > > - dev_info(&pdev->dev,
> > > - "Failed to get ssp1_sclk, defer probe\n");
> > > - return -EPROBE_DEFER;
> > > - }
> > > + ret = kabylake_audio_clk_get(&pdev->dev, "ssp1_mclk", &ctx-
> > > >mclk);
> > > + if (ret != 0)
> > > + return ret;
> > >
> > > - dev_err(&pdev->dev, "Failed to get ssp1_sclk with err:%d\n",
> > > - ret);
> > > + ret = kabylake_audio_clk_get(&pdev->dev, "ssp1_sclk", &ctx->sclk1);
> > > + if (ret != 0)
> > > return ret;
> > > - }
> > >
> > > return devm_snd_soc_register_card(&pdev->dev,
> > > &kabylake_audio_card); }
> > > --
> > > 2.7.4
More information about the Alsa-devel
mailing list