sound/soc/fsl/fsl_sai.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
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>
---
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..3a5ebf32903f 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) {
+ 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);
+ }
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
--
2.49.0Hi Arun,
kernel test robot noticed the following build warnings:
[auto build test WARNING on broonie-sound/for-next]
[also build test WARNING on linus/master v6.16-rc3 next-20250625]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Arun-Raghavan/ASoC-fsl_sai-Force-a-software-reset-when-starting-in-consumer-mode/20250625-210824
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link: https://lore.kernel.org/r/20250625130648.201331-1-arun%40arunraghavan.net
patch subject: [PATCH] ASoC: fsl_sai: Force a software reset when starting in consumer mode
config: arc-randconfig-002-20250626 (https://download.01.org/0day-ci/archive/20250626/202506260745.Z7PLEr3i-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250626/202506260745.Z7PLEr3i-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506260745.Z7PLEr3i-lkp@intel.com/
All warnings (new ones prefixed by >>):
sound/soc/fsl/fsl_sai.c: In function 'fsl_sai_trigger':
>> sound/soc/fsl/fsl_sai.c:848:21: warning: the comparison will always evaluate as 'true' for the address of 'is_consumer_mode' will never be NULL [-Waddress]
848 | if (sai->is_consumer_mode) {
| ^~~
In file included from sound/soc/fsl/fsl_sai.c:24:
sound/soc/fsl/fsl_sai.h:287:14: note: 'is_consumer_mode' declared here
287 | bool is_consumer_mode[2];
| ^~~~~~~~~~~~~~~~
vim +848 sound/soc/fsl/fsl_sai.c
814
815 static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
816 struct snd_soc_dai *cpu_dai)
817 {
818 struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
819 unsigned int ofs = sai->soc_data->reg_offset;
820
821 bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
822 int adir = tx ? RX : TX;
823 int dir = tx ? TX : RX;
824 u32 xcsr;
825
826 /*
827 * Asynchronous mode: Clear SYNC for both Tx and Rx.
828 * Rx sync with Tx clocks: Clear SYNC for Tx, set it for Rx.
829 * Tx sync with Rx clocks: Clear SYNC for Rx, set it for Tx.
830 */
831 regmap_update_bits(sai->regmap, FSL_SAI_TCR2(ofs), FSL_SAI_CR2_SYNC,
832 sai->synchronous[TX] ? FSL_SAI_CR2_SYNC : 0);
833 regmap_update_bits(sai->regmap, FSL_SAI_RCR2(ofs), FSL_SAI_CR2_SYNC,
834 sai->synchronous[RX] ? FSL_SAI_CR2_SYNC : 0);
835
836 /*
837 * It is recommended that the transmitter is the last enabled
838 * and the first disabled.
839 */
840 switch (cmd) {
841 case SNDRV_PCM_TRIGGER_START:
842 case SNDRV_PCM_TRIGGER_RESUME:
843 case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
844 /*
845 * Force a software reset if we are not the clock provider, as we
846 * might have lost frame sync during xrun recovery.
847 */
> 848 if (sai->is_consumer_mode) {
849 regmap_update_bits(sai->regmap,
850 FSL_SAI_xCSR(tx, ofs), FSL_SAI_CSR_SR,
851 FSL_SAI_CSR_SR);
852 regmap_update_bits(sai->regmap,
853 FSL_SAI_xCSR(tx, ofs), FSL_SAI_CSR_SR,
854 0);
855 }
856 regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
857 FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
858
859 regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
860 FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
861 /*
862 * Enable the opposite direction for synchronous mode
863 * 1. Tx sync with Rx: only set RE for Rx; set TE & RE for Tx
864 * 2. Rx sync with Tx: only set TE for Tx; set RE & TE for Rx
865 *
866 * RM recommends to enable RE after TE for case 1 and to enable
867 * TE after RE for case 2, but we here may not always guarantee
868 * that happens: "arecord 1.wav; aplay 2.wav" in case 1 enables
869 * TE after RE, which is against what RM recommends but should
870 * be safe to do, judging by years of testing results.
871 */
872 if (fsl_sai_dir_is_synced(sai, adir))
873 regmap_update_bits(sai->regmap, FSL_SAI_xCSR((!tx), ofs),
874 FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
875
876 regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
877 FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
878 break;
879 case SNDRV_PCM_TRIGGER_STOP:
880 case SNDRV_PCM_TRIGGER_SUSPEND:
881 case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
882 regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
883 FSL_SAI_CSR_FRDE, 0);
884 regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
885 FSL_SAI_CSR_xIE_MASK, 0);
886
887 /* Check if the opposite FRDE is also disabled */
888 regmap_read(sai->regmap, FSL_SAI_xCSR(!tx, ofs), &xcsr);
889
890 /*
891 * If opposite stream provides clocks for synchronous mode and
892 * it is inactive, disable it before disabling the current one
893 */
894 if (fsl_sai_dir_is_synced(sai, adir) && !(xcsr & FSL_SAI_CSR_FRDE))
895 fsl_sai_config_disable(sai, adir);
896
897 /*
898 * Disable current stream if either of:
899 * 1. current stream doesn't provide clocks for synchronous mode
900 * 2. current stream provides clocks for synchronous mode but no
901 * more stream is active.
902 */
903 if (!fsl_sai_dir_is_synced(sai, dir) || !(xcsr & FSL_SAI_CSR_FRDE))
904 fsl_sai_config_disable(sai, dir);
905
906 break;
907 default:
908 return -EINVAL;
909 }
910
911 return 0;
912 }
913
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
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);
+ }
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
--
2.49.0On Wed, Jun 25, 2025 at 07:56:16PM -0400, Arun Raghavan 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. Please don't send new patches or versions in reply to old threads, it makes it harder to follow what's going on.
On Thu, 26 Jun 2025, at 7:08 AM, Mark Brown wrote: > On Wed, Jun 25, 2025 at 07:56:16PM -0400, Arun Raghavan 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. > > Please don't send new patches or versions in reply to old threads, it > makes it harder to follow what's going on. Ack, sorry about that! -- Arun
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.
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?
Best regards
Shengjiu Wang
> regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
> FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
>
> --
> 2.49.0
>
© 2016 - 2025 Red Hat, Inc.