Re: [PATCH] ASoC: fsl_ssi: Fix channel swap on playback start

[Nicolin Chen <nicoleotsuka@gmail.com>]

Hi

On Fri, Mar 31, 2017 at 09:59:46PM -0300, Fabio Estevam wrote:

> >> +#define FSLSSI_SSIEN_WORKAROUND (CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE | \
> >> +                              CCSR_SSI_SCR_RE)
> >
> > Enable RE??
> 
> Yes, same idea as in f8fdf5375e2005f2 ("ASoC: fsl-ssi: add SSIEN
> errata work around").
> 
> The idea here was not to restrict the erratum to AC97 mode only.

I understood. Just enabling RE anyway (for I2S) had confused me.

> >> @@ -559,7 +562,8 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable,
> >>                       int i;
> >>                       int max_loop = 100;
> >>                       regmap_update_bits(regs, CCSR_SSI_SCR,
> >> -                                     CCSR_SSI_SCR_SSIEN, CCSR_SSI_SCR_SSIEN);
> >> +                                        FSLSSI_SSIEN_WORKAROUND,
> >> +                                        FSLSSI_SSIEN_WORKAROUND);
> >>                       for (i = 0; i < max_loop; i++) {
> >>                               u32 sfcsr;
> >>                               regmap_read(regs, CCSR_SSI_SFCSR, &sfcsr);
> >
> > If this change is made, the whole "if (enable)" part here seems
> > to be meaningless (or even worse) as it aimed to set TE later
> > than SSIEN so as to offset the delay from DMA TX.
> >
> > Check: https://patchwork.kernel.org/patch/9091051/
> >
> > If this errata is mandatory, we probably should revert that the
> > commit and find other solution/workaround for Arnaud and Caleb.
> 
> Reverting 61fcf10a0ee44763e0 ("ASoC: fsl_ssi: Fix channel slipping in
> Playback at startup") still causes channel swaps on my tests, so
> better not to revert it.

That sounds weird to me. Prior to that commit, the code was setting
SSIEN and TE at the same time if I am not wrong. You could try to
revert it and check the SCR value before/after the last line:
    regmap_update_bits(regs, CCSR_SSI_SCR, vals->scr, vals->scr);