Re: [PATCH v2] ASoC: fsl-asoc-card: Remove fsl_asoc_card_set_bias_level function

On Sat, Aug 1, 2020 at 4:01 PM Nicolin Chen nicoleotsuka@gmail.com wrote:

Hi,

Having two nits and one question, inline:

On Thu, Jul 30, 2020 at 05:47:02PM +0800, Shengjiu Wang wrote:

...

@@ -182,6 +180,69 @@ static int fsl_asoc_card_hw_params(struct snd_pcm_substream *substream, cpu_priv->slot_width); if (ret && ret != -ENOTSUPP) { dev_err(dev, "failed to set TDM slot for cpu dai\n");

•                 goto out;
  

•         }
  

• }
  

• /* Specific configuration for PLL */
  

• if (codec_priv->pll_id && codec_priv->fll_id) {
  

•         if (priv->sample_format == SNDRV_PCM_FORMAT_S24_LE)
  

•                 pll_out = priv->sample_rate * 384;
  

•         else
  

•                 pll_out = priv->sample_rate * 256;
  

•         ret = snd_soc_dai_set_pll(asoc_rtd_to_codec(rtd, 0),
  

•                                   codec_priv->pll_id,
  

•                                   codec_priv->mclk_id,
  

•                                   codec_priv->mclk_freq, pll_out);
  

•         if (ret) {
  

•                 dev_err(dev, "failed to start FLL: %d\n", ret);
  

•                 goto out;
  

•         }
  

•         ret = snd_soc_dai_set_sysclk(asoc_rtd_to_codec(rtd, 0),
  

•                                      codec_priv->fll_id,
  

•                                      pll_out, SND_SOC_CLOCK_IN);
  

Just came into my mind: do we need some protection here to prevent PLL/SYSCLK reconfiguration if TX/RX end up with different values?

Sorry, not really catching your point. could you please elaborate? Why do TX/RX end up with different values?

best regards wang shengiu

...

• return 0;
  

+out:

• priv->streams &= ~BIT(substream->stream);
  

• return ret;
  

Rather than "out:" which doesn't explicitly indicate an error-out, "fail:" would be better, following what we used in probe().

...

+static int fsl_asoc_card_hw_free(struct snd_pcm_substream *substream) +{

• struct snd_soc_pcm_runtime *rtd = substream->private_data;
  

• struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(rtd->card);
  

• struct codec_priv *codec_priv = &priv->codec_priv;
  

• struct device *dev = rtd->card->dev;
  

• int ret;
  

• priv->streams &= ~BIT(substream->stream);
  

...

• if (!priv->streams && codec_priv->pll_id &&
  

•     codec_priv->fll_id) {
  

This now can fit into single line :)