ASoC: ti: davinci-i2s: Move the XSYNCERR workaround to .prepare callback
authorPeter Ujfalusi <peter.ujfalusi@ti.com>
Fri, 30 Aug 2019 10:38:39 +0000 (13:38 +0300)
committerMark Brown <broonie@kernel.org>
Fri, 30 Aug 2019 11:21:20 +0000 (12:21 +0100)
Currently the driver uses snd_soc_rtdcom_lookup() in it's mcbsp_start
function to try to stop/restart the DMA as the initial XSYNCERR workaround
need to be done before the DMA is armed.

There are couple of things wrong with this:
- the driver crashes with NULL pointer dereference as the
  component->driver->ops is actually NULL
- the driver should not use snd_soc_rtdcom_lookup() in the first place
- Fiddling with DMA is never a good thing

Move the workaround handling to .prepare which is called before the DMA is
armed, so it complies with the requirements.

Reported-by (usage of snd_soc_rtdcom_lookup): Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Link: https://lore.kernel.org/r/20190830103841.25128-3-peter.ujfalusi@ti.com
Signed-off-by: Mark Brown <broonie@kernel.org>
sound/soc/ti/davinci-i2s.c

index 92c1bdc69086090f8d07ee390dd615dee21ec063..27afdbb9adf3674b138f9f4f60054cfb243affc9 100644 (file)
@@ -187,57 +187,9 @@ static void toggle_clock(struct davinci_mcbsp_dev *dev, int playback)
 static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev,
                struct snd_pcm_substream *substream)
 {
-       struct snd_soc_pcm_runtime *rtd = substream->private_data;
-       struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME);
        int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
        u32 spcr;
        u32 mask = playback ? DAVINCI_MCBSP_SPCR_XRST : DAVINCI_MCBSP_SPCR_RRST;
-       spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-       if (spcr & mask) {
-               /* start off disabled */
-               davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG,
-                               spcr & ~mask);
-               toggle_clock(dev, playback);
-       }
-       if (dev->pcr & (DAVINCI_MCBSP_PCR_FSXM | DAVINCI_MCBSP_PCR_FSRM |
-                       DAVINCI_MCBSP_PCR_CLKXM | DAVINCI_MCBSP_PCR_CLKRM)) {
-               /* Start the sample generator */
-               spcr |= DAVINCI_MCBSP_SPCR_GRST;
-               davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
-       }
-
-       if (playback) {
-               /* Stop the DMA to avoid data loss */
-               /* while the transmitter is out of reset to handle XSYNCERR */
-               if (component->driver->ops->trigger) {
-                       int ret = component->driver->ops->trigger(substream,
-                               SNDRV_PCM_TRIGGER_STOP);
-                       if (ret < 0)
-                               printk(KERN_DEBUG "Playback DMA stop failed\n");
-               }
-
-               /* Enable the transmitter */
-               spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-               spcr |= DAVINCI_MCBSP_SPCR_XRST;
-               davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
-
-               /* wait for any unexpected frame sync error to occur */
-               udelay(100);
-
-               /* Disable the transmitter to clear any outstanding XSYNCERR */
-               spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-               spcr &= ~DAVINCI_MCBSP_SPCR_XRST;
-               davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
-               toggle_clock(dev, playback);
-
-               /* Restart the DMA */
-               if (component->driver->ops->trigger) {
-                       int ret = component->driver->ops->trigger(substream,
-                               SNDRV_PCM_TRIGGER_START);
-                       if (ret < 0)
-                               printk(KERN_DEBUG "Playback DMA start failed\n");
-               }
-       }
 
        /* Enable transmitter or receiver */
        spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
@@ -575,7 +527,41 @@ static int davinci_i2s_prepare(struct snd_pcm_substream *substream,
 {
        struct davinci_mcbsp_dev *dev = snd_soc_dai_get_drvdata(dai);
        int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
+       u32 spcr;
+       u32 mask = playback ? DAVINCI_MCBSP_SPCR_XRST : DAVINCI_MCBSP_SPCR_RRST;
+
        davinci_mcbsp_stop(dev, playback);
+
+       spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+       if (spcr & mask) {
+               /* start off disabled */
+               davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG,
+                                       spcr & ~mask);
+               toggle_clock(dev, playback);
+       }
+       if (dev->pcr & (DAVINCI_MCBSP_PCR_FSXM | DAVINCI_MCBSP_PCR_FSRM |
+                       DAVINCI_MCBSP_PCR_CLKXM | DAVINCI_MCBSP_PCR_CLKRM)) {
+               /* Start the sample generator */
+               spcr |= DAVINCI_MCBSP_SPCR_GRST;
+               davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
+       }
+
+       if (playback) {
+               /* Enable the transmitter */
+               spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+               spcr |= DAVINCI_MCBSP_SPCR_XRST;
+               davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
+
+               /* wait for any unexpected frame sync error to occur */
+               udelay(100);
+
+               /* Disable the transmitter to clear any outstanding XSYNCERR */
+               spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+               spcr &= ~DAVINCI_MCBSP_SPCR_XRST;
+               davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
+               toggle_clock(dev, playback);
+       }
+
        return 0;
 }