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: <[email protected]>
> 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 <[email protected]>
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);
>
