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

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?

• 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 :)