From a2dc6f82fd86fa165222f6062e2478fd122f9f1c Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 30 Aug 2019 13:38:39 +0300 Subject: [PATCH] ASoC: ti: davinci-i2s: Move the XSYNCERR workaround to .prepare callback 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 Signed-off-by: Peter Ujfalusi Link: https://lore.kernel.org/r/20190830103841.25128-3-peter.ujfalusi@ti.com Signed-off-by: Mark Brown --- sound/soc/ti/davinci-i2s.c | 82 ++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 48 deletions(-) diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c index 92c1bdc69086..27afdbb9adf3 100644 --- a/sound/soc/ti/davinci-i2s.c +++ b/sound/soc/ti/davinci-i2s.c @@ -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; }