[alsa-devel] [PATCH 0/2] ASoC: tlv320dac33 fixes
Hello,
Revisiting the start/stop sequence used by the tlv320dac33 codec in order to avoid race conditions, which can occur, when the codec is in FIFO modes. It has been found that OMAP3 McBSP needs the BCLK continuously running on the serial interface when it is configured to be slave. DAC33 can be configured in a way, that it keeps the BCLK ticking, and only enable the FS when it is needed. Option for the platform data to select between continuous BCLK or non continuous BCLK.
--- Peter Ujfalusi (2): ASoC: tlv320dac33: Start/stop sequence change ASoC: tlv320dac33: Add option for keeping the BCLK running
include/sound/tlv320dac33-plat.h | 1 + sound/soc/codecs/tlv320dac33.c | 38 ++++++++++++++------------------------ 2 files changed, 15 insertions(+), 24 deletions(-)
To avoid race condition especially in FIFO modes the sequence for enabling and disabling the codec need to be changed.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/tlv320dac33.c | 25 +++---------------------- 1 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c index f9f367d..e845c4b 100644 --- a/sound/soc/codecs/tlv320dac33.c +++ b/sound/soc/codecs/tlv320dac33.c @@ -310,7 +310,8 @@ static inline void dac33_soft_power(struct snd_soc_codec *codec, int power) if (power) reg |= DAC33_PDNALLB; else - reg &= ~DAC33_PDNALLB; + reg &= ~(DAC33_PDNALLB | DAC33_OSCPDNB | + DAC33_DACRPDNB | DAC33_DACLPDNB); dac33_write(codec, DAC33_PWR_CTRL, reg); }
@@ -634,26 +635,6 @@ static irqreturn_t dac33_interrupt_handler(int irq, void *dev) return IRQ_HANDLED; }
-static void dac33_shutdown(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_device *socdev = rtd->socdev; - struct snd_soc_codec *codec = socdev->card->codec; - struct tlv320dac33_priv *dac33 = codec->private_data; - unsigned int pwr_ctrl; - - /* Stop pending workqueue */ - if (dac33->fifo_mode) - cancel_work_sync(&dac33->work); - - mutex_lock(&dac33->mutex); - pwr_ctrl = dac33_read_reg_cache(codec, DAC33_PWR_CTRL); - pwr_ctrl &= ~(DAC33_OSCPDNB | DAC33_DACRPDNB | DAC33_DACLPDNB); - dac33_write(codec, DAC33_PWR_CTRL, pwr_ctrl); - mutex_unlock(&dac33->mutex); -} - static void dac33_oscwait(struct snd_soc_codec *codec) { int timeout = 20; @@ -751,6 +732,7 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) }
mutex_lock(&dac33->mutex); + dac33_soft_power(codec, 0); dac33_soft_power(codec, 1);
reg_tmp = dac33_read_reg_cache(codec, DAC33_INT_OSC_CTRL); @@ -1185,7 +1167,6 @@ EXPORT_SYMBOL_GPL(soc_codec_dev_tlv320dac33); #define DAC33_FORMATS SNDRV_PCM_FMTBIT_S16_LE
static struct snd_soc_dai_ops dac33_dai_ops = { - .shutdown = dac33_shutdown, .hw_params = dac33_hw_params, .prepare = dac33_pcm_prepare, .trigger = dac33_pcm_trigger,
On Thu, 2010-03-11 at 16:26 +0200, Peter Ujfalusi wrote:
To avoid race condition especially in FIFO modes the sequence for enabling and disabling the codec need to be changed.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Platform data option for the codec to keep the BCLK clock continuously running in FIFO modes (codec master).
OMAP3 McBSP when in slave mode needs continuous BCLK running on the serial bus in order to operate correctly.
Since in FIFO mode the DAC33 can also shut down the BCLK clock and enable it only when it is needed, let the platforms decide if the CPU side needs the BCLK running or not.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- include/sound/tlv320dac33-plat.h | 1 + sound/soc/codecs/tlv320dac33.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/sound/tlv320dac33-plat.h b/include/sound/tlv320dac33-plat.h index ac06652..3f428d5 100644 --- a/include/sound/tlv320dac33-plat.h +++ b/include/sound/tlv320dac33-plat.h @@ -15,6 +15,7 @@
struct tlv320dac33_platform_data { int power_gpio; + int keep_bclk; /* Keep the BCLK running in FIFO modes */ u8 burst_bclkdiv; };
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c index e845c4b..a6f1927 100644 --- a/sound/soc/codecs/tlv320dac33.c +++ b/sound/soc/codecs/tlv320dac33.c @@ -93,6 +93,8 @@ struct tlv320dac33_priv { unsigned int nsample; /* burst read amount from host */ u8 burst_bclkdiv; /* BCLK divider value in burst mode */
+ int keep_bclk; /* Keep the BCLK continuously running + * in FIFO modes */ enum dac33_state state; };
@@ -803,7 +805,10 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) */ fifoctrl_a &= ~DAC33_FBYPAS; fifoctrl_a &= ~DAC33_FAUTO; - aictrl_b &= ~DAC33_BCLKON; + if (dac33->keep_bclk) + aictrl_b |= DAC33_BCLKON; + else + aictrl_b &= ~DAC33_BCLKON; break; case DAC33_FIFO_MODE7: /* @@ -814,7 +819,10 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) */ fifoctrl_a &= ~DAC33_FBYPAS; fifoctrl_a |= DAC33_FAUTO; - aictrl_b &= ~DAC33_BCLKON; + if (dac33->keep_bclk) + aictrl_b |= DAC33_BCLKON; + else + aictrl_b &= ~DAC33_BCLKON; break; default: /* @@ -1234,6 +1242,7 @@ static int __devinit dac33_i2c_probe(struct i2c_client *client,
dac33->power_gpio = pdata->power_gpio; dac33->burst_bclkdiv = pdata->burst_bclkdiv; + dac33->keep_bclk = pdata->keep_bclk; dac33->irq = client->irq; dac33->nsample = NSAMPLE_MAX; /* Disable FIFO use by default */
On Thu, 2010-03-11 at 16:26 +0200, Peter Ujfalusi wrote:
Platform data option for the codec to keep the BCLK clock continuously running in FIFO modes (codec master).
OMAP3 McBSP when in slave mode needs continuous BCLK running on the serial bus in order to operate correctly.
Since in FIFO mode the DAC33 can also shut down the BCLK clock and enable it only when it is needed, let the platforms decide if the CPU side needs the BCLK running or not.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Thu, 11 Mar 2010 16:26:20 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
It has been found that OMAP3 McBSP needs the BCLK continuously running on the serial interface when it is configured to be slave.
Just curious: what would happen if the BCLK is cut while the McBSP is operating? I'm thinking are there also other similar problems, e.g. if the rate is not correct.
On Thursday 11 March 2010 16:46:28 ext Jarkko Nikula wrote:
On Thu, 11 Mar 2010 16:26:20 +0200
Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
It has been found that OMAP3 McBSP needs the BCLK continuously running on the serial interface when it is configured to be slave.
Just curious: what would happen if the BCLK is cut while the McBSP is operating?
The symptom is that we can not access to McBSP register address space causing kernel panic, which can only be fixed by rebooting the device. The burst driven BCLK causes some internal state machine to stuck, leaving the given McBSP port dead from the outside. Other ports operate after this event. Obviously this only bites in McBSP slave mode, and with codec like DAC33 which have burst mode, in other cases the BCLK is always running (or McBSP is master).
I'm thinking are there also other similar problems, e.g. if the rate is not correct.
Hmmm, could be possible, but I did not experienced with such a problem.
On Thu, 11 Mar 2010 16:55:12 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
Just curious: what would happen if the BCLK is cut while the McBSP is operating?
The symptom is that we can not access to McBSP register address space causing kernel panic, which can only be fixed by rebooting the device. The burst driven BCLK causes some internal state machine to stuck, leaving the given McBSP port dead from the outside. Other ports operate after this event. Obviously this only bites in McBSP slave mode, and with codec like DAC33 which have burst mode, in other cases the BCLK is always running (or McBSP is master).
I'm thinking are there also other similar problems, e.g. if the rate is not correct.
Hmmm, could be possible, but I did not experienced with such a problem.
Thanks for sharing this info.
Sounds exactly similar problem what I encountered once with the OMAP2420 and EAC. I didn't debug that problem any further then but there also some register accesses (not all) caused kernel panic if the external clock was missing or if the rate wasn't correct.
On Thu, Mar 11, 2010 at 04:26:20PM +0200, Peter Ujfalusi wrote:
Revisiting the start/stop sequence used by the tlv320dac33 codec in order to avoid race conditions, which can occur, when the codec is in FIFO modes.
Applied, thanks.
It has been found that OMAP3 McBSP needs the BCLK continuously running on the serial interface when it is configured to be slave. DAC33 can be configured in a way, that it keeps the BCLK ticking, and only enable the FS when it is needed.
I suspect there's some cause and effect going on in the CODEC having this feature :/
participants (4)
-
Jarkko Nikula -
Liam Girdwood -
Mark Brown -
Peter Ujfalusi