[alsa-devel] [PATCH V3] ASoC: fsl_esai: fix channel swap issue when stream starts

[S.j. Wang]

Ok, will send v4.

Best regards
Wang shengjiu
> 
> 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 at vger.kernel.org>
> > Re> @@ -361,21 +363,13 @@ static int fsl_esai_set_dai_tdm_slot(struct
> > Re> 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 at gmail.com>
> 
> 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);
> >