Hi Shengjiu, Two more trivial comments inline. On Tue, Feb 26, 2019 at 03:12:25AM +0000, S.j. Wang wrote: > There is very low possibility ( < 0.1% ) that channel swap happened > in beginning when multi output/input pin is enabled. The issue is > that hardware can't send data to correct pin in the beginning with > the normal enable flow. > > This is hardware issue, but there is no errata, the workaround flow > is that: Each time playback/recording, firstly clear the xSMA/xSMB, > then enable TE/RE, then enable xSMB and xSMA (xSMB must be enabled > before xSMA). Which is to use the xSMA as the trigger start register, > previously the xCR_TE or xCR_RE is the bit for starting. > > Fixes commit 43d24e76b698 ("ASoC: fsl_esai: Add ESAI CPU DAI driver") > Cc: <stable@xxxxxxxxxxxxxxx> > Re> @@ -361,21 +363,13 @@ static int fsl_esai_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask, > esai_priv->slot_width = slot_width; > esai_priv->slots = slots; > + esai_priv->tx_mask = tx_mask; > + esai_priv->rx_mask = rx_mask; Not necessary to have tabs before "="s. > @@ -611,12 +606,23 @@ static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd, > regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx), > tx ? ESAI_xCR_TE_MASK : ESAI_xCR_RE_MASK, > tx ? ESAI_xCR_TE(pins) : ESAI_xCR_RE(pins)); > + mask = tx ? esai_priv->tx_mask : esai_priv->rx_mask; > + Please add a line of brief comments for the reason why we are doing this here, something similar to the commit log, but should have: 1) Clear xSMA/B 2) Set TE/RE before xSMA/B 3)Set xSMB before xSMA. Apparently having this mask settings in the trigger() does not look very common, so I believe a line of comments would help a lot. You may add my Acked-by in your next version: Acked-by: Nicolin Chen <nicoleotsuka@xxxxxxxxx> Thanks > + regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMB(tx), > + ESAI_xSMB_xS_MASK, ESAI_xSMB_xS(mask)); > + regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMA(tx), > + ESAI_xSMA_xS_MASK, ESAI_xSMA_xS(mask)); > + > break; > case SNDRV_PCM_TRIGGER_SUSPEND: > case SNDRV_PCM_TRIGGER_STOP: > case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx), > tx ? ESAI_xCR_TE_MASK : ESAI_xCR_RE_MASK, 0); > + regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMA(tx), > + ESAI_xSMA_xS_MASK, 0); > + regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMB(tx), > + ESAI_xSMB_xS_MASK, 0); > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel