On Wed, Oct 23, 2019 at 03:29:49PM +0800, Shengjiu Wang wrote:

xrun may happen at the end of stream, the trigger->fsl_esai_trigger_stop maybe called in the middle of fsl_esai_hw_reset, this may cause esai in wrong state after stop, and there may be endless xrun interrupt.

What about fsl_esai_trigger_start? It touches ESAI_xFCR_xFEN bit that is being checked in the beginning of fsl_esai_hw_reset.

Could the scenario below be possible also?

1) ESAI TX starts 2) Xrun happens to TX 3) Starting fsl_esai_hw_reset (enabled[TX] = true; enabled[RX] = false) 4) ESAI RX starts 5) Finishing fsl_esai_hw_reset (enabled[RX] is still false)

Thanks Nicolin

So Add spin lock to lock these two function.

Fixes: 7ccafa2b3879 ("ASoC: fsl_esai: recover the channel swap after xrun") Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com

---

sound/soc/fsl/fsl_esai.c | 10 ++++++++++ 1 file changed, 10 insertions(+)

diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c index 37b14c48b537..6a797648b66d 100644 --- a/sound/soc/fsl/fsl_esai.c +++ b/sound/soc/fsl/fsl_esai.c @@ -33,6 +33,7 @@

• @fsysclk: system clock source to derive HCK, SCK and FS
• @spbaclk: SPBA clock (optional, depending on SoC design)
• @task: tasklet to handle the reset operation

• • @lock: spin lock to handle reset and stop behavior
  • @fifo_depth: depth of tx/rx FIFO
  • @slot_width: width of each DAI slot
  • @slots: number of slots

@@ -56,6 +57,7 @@ struct fsl_esai { struct clk *fsysclk; struct clk *spbaclk; struct tasklet_struct task;

• spinlock_t lock; /* Protect reset and stop */ u32 fifo_depth; u32 slot_width; u32 slots;

@@ -676,8 +678,10 @@ static void fsl_esai_hw_reset(unsigned long arg) { struct fsl_esai *esai_priv = (struct fsl_esai *)arg; bool tx = true, rx = false, enabled[2];

• unsigned long lock_flags; u32 tfcr, rfcr;

• spin_lock_irqsave(&esai_priv->lock, lock_flags); /* Save the registers */ regmap_read(esai_priv->regmap, REG_ESAI_TFCR, &tfcr); regmap_read(esai_priv->regmap, REG_ESAI_RFCR, &rfcr);

@@ -715,6 +719,8 @@ static void fsl_esai_hw_reset(unsigned long arg) fsl_esai_trigger_start(esai_priv, tx); if (enabled[rx]) fsl_esai_trigger_start(esai_priv, rx);

• spin_unlock_irqrestore(&esai_priv->lock, lock_flags);

}

static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd, @@ -722,6 +728,7 @@ static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd, { struct fsl_esai *esai_priv = snd_soc_dai_get_drvdata(dai); bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;

• unsigned long lock_flags;

  esai_priv->channels[tx] = substream->runtime->channels;

@@ -734,7 +741,9 @@ static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd, case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_PAUSE_PUSH:

• spin_lock_irqsave(&esai_priv->lock, lock_flags);
  

  fsl_esai_trigger_stop(esai_priv, tx);

• spin_unlock_irqrestore(&esai_priv->lock, lock_flags);
  

  break; default: return -EINVAL;

@@ -1002,6 +1011,7 @@ static int fsl_esai_probe(struct platform_device *pdev)

dev_set_drvdata(&pdev->dev, esai_priv);

• spin_lock_init(&esai_priv->lock); ret = fsl_esai_hw_init(esai_priv); if (ret) return ret;

-- 2.21.0