[alsa-devel] [PATCH v2 0/4] ASoC: davinci-mcasp: Correct start/stop sequences
Hi,
Changes since v1: - Correction for the stop sequence is added - start code cleanup patch added
The startup sequence for TX and RX was not correct, not following the programming sequence from the TRM. This could cause initial channel swap when starting TX. When stopping the stream the AFIFO should be stopped as the last step to avoid unpredictable behavior.
The change has been tested on AM335x and AM437x but it should be valid for all devices.
Regards, Peter --- Peter Ujfalusi (4): ASoC: davinci-mcasp: Correct TX start sequence ASoC: davinci-mcasp: Correct RX start sequence ASoC: davinci-mcasp: When stopping TX/RX stop the AFIFO as the last step ASoC: davinci-mcasp: Move the AFIFO related code under start_tx/rx functions
sound/soc/davinci/davinci-mcasp.c | 95 ++++++++++++++++++--------------------- sound/soc/davinci/davinci-mcasp.h | 6 +++ 2 files changed, 49 insertions(+), 52 deletions(-)
Follow the sequence described in the TRMs when starting TX. This sequence will make sure that we are not facing with initial channel swap caused by no data available in McASP for transmit.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-mcasp.c | 25 +++++++++---------------- sound/soc/davinci/davinci-mcasp.h | 6 ++++++ 2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index a01ae97c90aa..c6f2a7b9b6f6 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -165,31 +165,24 @@ static void mcasp_start_rx(struct davinci_mcasp *mcasp)
static void mcasp_start_tx(struct davinci_mcasp *mcasp) { - u8 offset = 0, i; u32 cnt;
+ /* Start clocks */ mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXHCLKRST); mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXCLKRST); + /* Activate serializer(s) */ mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXSERCLR); - mcasp_set_reg(mcasp, DAVINCI_MCASP_TXBUF_REG, 0);
- mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXSMRST); - mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXFSRST); - mcasp_set_reg(mcasp, DAVINCI_MCASP_TXBUF_REG, 0); - for (i = 0; i < mcasp->num_serializer; i++) { - if (mcasp->serial_dir[i] == TX_MODE) { - offset = i; - break; - } - } - - /* wait for TX ready */ + /* wait for XDATA to be cleared */ cnt = 0; - while (!(mcasp_get_reg(mcasp, DAVINCI_MCASP_XRSRCTL_REG(offset)) & - TXSTATE) && (cnt < 100000)) + while (!(mcasp_get_reg(mcasp, DAVINCI_MCASP_TXSTAT_REG) & + ~XRDATA) && (cnt < 100000)) cnt++;
- mcasp_set_reg(mcasp, DAVINCI_MCASP_TXBUF_REG, 0); + /* Release TX state machine */ + mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXSMRST); + /* Release Frame Sync generator */ + mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXFSRST); }
static void davinci_mcasp_start(struct davinci_mcasp *mcasp, int stream) diff --git a/sound/soc/davinci/davinci-mcasp.h b/sound/soc/davinci/davinci-mcasp.h index 8fed757d6087..53243c22818d 100644 --- a/sound/soc/davinci/davinci-mcasp.h +++ b/sound/soc/davinci/davinci-mcasp.h @@ -253,6 +253,12 @@ #define TXFSRST BIT(12) /* Frame Sync Generator Reset */
/* + * DAVINCI_MCASP_TXSTAT_REG - Transmitter Status Register Bits + * DAVINCI_MCASP_RXSTAT_REG - Receiver Status Register Bits + */ +#define XRDATA BIT(5) /* Transmit/Receive data ready */ + +/* * DAVINCI_MCASP_AMUTE_REG - Mute Control Register Bits */ #define MUTENA(val) (val)
Follow the sequence described in the TRMs when starting RX. Write to RXBUF register was not correct and there is no need to release the RX state machine/Receive frame sync generator twice.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-mcasp.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index c6f2a7b9b6f6..0250353dc6ab 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -136,9 +136,9 @@ static bool mcasp_is_synchronous(struct davinci_mcasp *mcasp)
static void mcasp_start_rx(struct davinci_mcasp *mcasp) { + /* Start clocks */ mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXHCLKRST); mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXCLKRST); - /* * When ASYNC == 0 the transmit and receive sections operate * synchronously from the transmit clock and frame sync. We need to make @@ -149,16 +149,12 @@ static void mcasp_start_rx(struct davinci_mcasp *mcasp) mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXCLKRST); }
+ /* Activate serializer(s) */ mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXSERCLR); - mcasp_set_reg(mcasp, DAVINCI_MCASP_RXBUF_REG, 0); - - mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXSMRST); - mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXFSRST); - mcasp_set_reg(mcasp, DAVINCI_MCASP_RXBUF_REG, 0); - + /* Release RX state machine */ mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXSMRST); + /* Release Frame Sync generator */ mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXFSRST); - if (mcasp_is_synchronous(mcasp)) mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXFSRST); }
The AFIFO should not be stopped (or started for that matter) when McASP is running since it can cause unpredictable issues because we are switching off AFIFO for the direction which was handling the requests from McASP and was generating DMA request toward the system DMA.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-mcasp.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 0250353dc6ab..b1926f9096e5 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -215,6 +215,12 @@ static void mcasp_stop_rx(struct davinci_mcasp *mcasp)
mcasp_set_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, 0); mcasp_set_reg(mcasp, DAVINCI_MCASP_RXSTAT_REG, 0xFFFFFFFF); + + if (mcasp->rxnumevt) { /* disable FIFO */ + u32 reg = mcasp->fifo_base + MCASP_RFIFOCTL_OFFSET; + + mcasp_clr_bits(mcasp, reg, FIFO_ENABLE); + } }
static void mcasp_stop_tx(struct davinci_mcasp *mcasp) @@ -230,27 +236,22 @@ static void mcasp_stop_tx(struct davinci_mcasp *mcasp)
mcasp_set_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, val); mcasp_set_reg(mcasp, DAVINCI_MCASP_TXSTAT_REG, 0xFFFFFFFF); + + if (mcasp->txnumevt) { /* disable FIFO */ + u32 reg = mcasp->fifo_base + MCASP_WFIFOCTL_OFFSET; + + mcasp_clr_bits(mcasp, reg, FIFO_ENABLE); + } }
static void davinci_mcasp_stop(struct davinci_mcasp *mcasp, int stream) { - u32 reg; - mcasp->streams--;
- if (stream == SNDRV_PCM_STREAM_PLAYBACK) { - if (mcasp->txnumevt) { /* disable FIFO */ - reg = mcasp->fifo_base + MCASP_WFIFOCTL_OFFSET; - mcasp_clr_bits(mcasp, reg, FIFO_ENABLE); - } + if (stream == SNDRV_PCM_STREAM_PLAYBACK) mcasp_stop_tx(mcasp); - } else { - if (mcasp->rxnumevt) { /* disable FIFO */ - reg = mcasp->fifo_base + MCASP_RFIFOCTL_OFFSET; - mcasp_clr_bits(mcasp, reg, FIFO_ENABLE); - } + else mcasp_stop_rx(mcasp); - } }
static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
In this way the start code for tx/rx going to be located at the same place.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-mcasp.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index b1926f9096e5..03ad5e5f1ac4 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -136,6 +136,13 @@ static bool mcasp_is_synchronous(struct davinci_mcasp *mcasp)
static void mcasp_start_rx(struct davinci_mcasp *mcasp) { + if (mcasp->rxnumevt) { /* enable FIFO */ + u32 reg = mcasp->fifo_base + MCASP_RFIFOCTL_OFFSET; + + mcasp_clr_bits(mcasp, reg, FIFO_ENABLE); + mcasp_set_bits(mcasp, reg, FIFO_ENABLE); + } + /* Start clocks */ mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXHCLKRST); mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLR_REG, RXCLKRST); @@ -163,6 +170,13 @@ static void mcasp_start_tx(struct davinci_mcasp *mcasp) { u32 cnt;
+ if (mcasp->txnumevt) { /* enable FIFO */ + u32 reg = mcasp->fifo_base + MCASP_WFIFOCTL_OFFSET; + + mcasp_clr_bits(mcasp, reg, FIFO_ENABLE); + mcasp_set_bits(mcasp, reg, FIFO_ENABLE); + } + /* Start clocks */ mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXHCLKRST); mcasp_set_ctl_reg(mcasp, DAVINCI_MCASP_GBLCTLX_REG, TXCLKRST); @@ -183,25 +197,12 @@ static void mcasp_start_tx(struct davinci_mcasp *mcasp)
static void davinci_mcasp_start(struct davinci_mcasp *mcasp, int stream) { - u32 reg; - mcasp->streams++;
- if (stream == SNDRV_PCM_STREAM_PLAYBACK) { - if (mcasp->txnumevt) { /* enable FIFO */ - reg = mcasp->fifo_base + MCASP_WFIFOCTL_OFFSET; - mcasp_clr_bits(mcasp, reg, FIFO_ENABLE); - mcasp_set_bits(mcasp, reg, FIFO_ENABLE); - } + if (stream == SNDRV_PCM_STREAM_PLAYBACK) mcasp_start_tx(mcasp); - } else { - if (mcasp->rxnumevt) { /* enable FIFO */ - reg = mcasp->fifo_base + MCASP_RFIFOCTL_OFFSET; - mcasp_clr_bits(mcasp, reg, FIFO_ENABLE); - mcasp_set_bits(mcasp, reg, FIFO_ENABLE); - } + else mcasp_start_rx(mcasp); - } }
static void mcasp_stop_rx(struct davinci_mcasp *mcasp)
On Fri, Mar 28, 2014 at 11:25:13AM +0200, Peter Ujfalusi wrote:
The startup sequence for TX and RX was not correct, not following the programming sequence from the TRM. This could cause initial channel swap when starting TX. When stopping the stream the AFIFO should be stopped as the last step to avoid unpredictable behavior.
The change has been tested on AM335x and AM437x but it should be valid for all devices.
Applied all, thanks.
participants (2)
-
Mark Brown -
Peter Ujfalusi