Re: [PATCH v2] ASoC: fsl_sai: Force a software reset when starting in consumer mode

["Arun Raghavan" <arun@arunraghavan.net>]

On Thu, 26 Jun 2025, at 2:55 AM, Shengjiu Wang wrote:
> On Thu, Jun 26, 2025 at 7:58 AM Arun Raghavan <arun@arunraghavan.net> wrote:
>>
>> From: Arun Raghavan <arun@asymptotic.io>
>>
>> In a setup with an external clock provider, when running the receiver
>> (arecord) and triggering an xrun with xrun_injection, we see a channel
>> swap/offset. This happens sometimes when running only the receiver, but
>> occurs reliably if a transmitter (aplay) is also concurrently running.
>>
>> The theory is that SAI seems to lose track of frame sync during the
>> trigger stop -> trigger start cycle that occurs during an xrun. Doing
>> just a FIFO reset in this case does not suffice, and only a software
>> reset seems to get it back on track.
>>
>> Signed-off-by: Arun Raghavan <arun@asymptotic.io>
>> Reported-by: Pieterjan Camerlynck <p.camerlynck@televic.com>
>> ---
>>
>> v2:
>> - Address build warning from kernel test robot
>>
>>  sound/soc/fsl/fsl_sai.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
>> index af1a168d35e3..d158352c7640 100644
>> --- a/sound/soc/fsl/fsl_sai.c
>> +++ b/sound/soc/fsl/fsl_sai.c
>> @@ -841,6 +841,18 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
>>         case SNDRV_PCM_TRIGGER_START:
>>         case SNDRV_PCM_TRIGGER_RESUME:
>>         case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>> +               /*
>> +                * Force a software reset if we are not the clock provider, as we
>> +                * might have lost frame sync during xrun recovery.
>> +                */
>> +               if (sai->is_consumer_mode[tx]) {
>> +                       regmap_update_bits(sai->regmap,
>> +                                       FSL_SAI_xCSR(tx, ofs), FSL_SAI_CSR_SR,
>> +                                       FSL_SAI_CSR_SR);
>> +                       regmap_update_bits(sai->regmap,
>> +                                       FSL_SAI_xCSR(tx, ofs), FSL_SAI_CSR_SR,
>> +                                       0);
>> +               }
>
> Which platform are you using?  and please add chip info in your commit
> message.

It's an imx8mm. I'll add that to the commit message.

> This change can be moved to fsl_sai_config_disable(). that is:
>
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -798,18 +798,16 @@ static void fsl_sai_config_disable(struct
> fsl_sai *sai, int dir)
>                            FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
>
>         /*
> -        * For sai master mode, after several open/close sai,
> +        * For sai master/slave mode, after several open/close sai,
>          * there will be no frame clock, and can't recover
>          * anymore. Add software reset to fix this issue.
>          * This is a hardware bug, and will be fix in the
>          * next sai version.
>          */
> -       if (!sai->is_consumer_mode[tx]) {
> -               /* Software Reset */
> -               regmap_write(sai->regmap, FSL_SAI_xCSR(tx, ofs),
> FSL_SAI_CSR_SR);
> -               /* Clear SR bit to finish the reset */
> -               regmap_write(sai->regmap, FSL_SAI_xCSR(tx, ofs), 0);
> -       }
> +       /* Software Reset */
> +       regmap_write(sai->regmap, FSL_SAI_xCSR(tx, ofs), FSL_SAI_CSR_SR);
> +       /* Clear SR bit to finish the reset */
> +       regmap_write(sai->regmap, FSL_SAI_xCSR(tx, ofs), 0);
>  }
>
> Could you please try the above change to also work for your case?

Ah, I should've spotted that! Yes, doing it this way works just fine. Updated patch incoming.

Thank you,
Arun