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);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
