lkml.org 
[lkml]   [2022]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v4 01/11] ASoC: jz4740-i2s: Handle independent FIFO flush bits
Hi Aidan,

Le ven., juil. 8 2022 at 17:02:34 +0100, Aidan MacDonald
<aidanmacdonald.0x0@gmail.com> a écrit :
> On the JZ4740, there is a single bit that flushes (empties) both
> the transmit and receive FIFO. Later SoCs have independent flush
> bits for each FIFO, which allows us to flush the right FIFO when
> starting up a stream.
>
> This also fixes a bug: since we were only setting the JZ4740's
> flush bit, which corresponds to the TX FIFO flush bit on other
> SoCs, other SoCs were not having their RX FIFO flushed at all.
>
> Fixes: 967beb2e8777 ("ASoC: jz4740: Add jz4780 support")
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
> sound/soc/jz4740/jz4740-i2s.c | 33 ++++++++++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/sound/soc/jz4740/jz4740-i2s.c
> b/sound/soc/jz4740/jz4740-i2s.c
> index ecd8df70d39c..576f31f9d734 100644
> --- a/sound/soc/jz4740/jz4740-i2s.c
> +++ b/sound/soc/jz4740/jz4740-i2s.c
> @@ -64,6 +64,9 @@
> #define JZ_AIC_CTRL_ENABLE_PLAYBACK BIT(1)
> #define JZ_AIC_CTRL_ENABLE_CAPTURE BIT(0)
>
> +#define JZ4760_AIC_CTRL_TFLUSH BIT(8)
> +#define JZ4760_AIC_CTRL_RFLUSH BIT(7)

Just rename JZ_AIC_CTRL_FLUSH to JZ_AIC_CTRL_TFLUSH and introduce
JZ_AIC_CTRL_RLUSH.

> +
> #define JZ_AIC_CTRL_OUTPUT_SAMPLE_SIZE_OFFSET 19
> #define JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_OFFSET 16
>
> @@ -90,6 +93,8 @@ enum jz47xx_i2s_version {
> struct i2s_soc_info {
> enum jz47xx_i2s_version version;
> struct snd_soc_dai_driver *dai;
> +
> + bool shared_fifo_flush;
> };
>
> struct jz4740_i2s {
> @@ -124,12 +129,33 @@ static int jz4740_i2s_startup(struct
> snd_pcm_substream *substream,
> uint32_t conf, ctrl;
> int ret;
>
> + /*
> + * When we can flush FIFOs independently, only flush the
> + * FIFO that is starting up.
> + */
> + if (!i2s->soc_info->shared_fifo_flush) {
> + ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL);
> +
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + ctrl |= JZ4760_AIC_CTRL_TFLUSH;
> + else
> + ctrl |= JZ4760_AIC_CTRL_RFLUSH;
> +
> + jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl);
> + }

Wouldn't it be simpler to do one single if/else? And hy is one checked
before the (snd_soc_dai_active(dai)) check, and the other is checked
after?

You could do something like this:

ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL);

if (i2s->soc_info->shared_fifo_flush ||
substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
ctrl |= JZ_AIC_CTRL_TFLUSH;
} else {
ctrl |= JZ_AIC_CTRL_RFLUSH;
}

jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl);

Cheers,
-Paul

> +
> if (snd_soc_dai_active(dai))
> return 0;
>
> - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL);
> - ctrl |= JZ_AIC_CTRL_FLUSH;
> - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl);
> + /*
> + * When there is a shared flush bit for both FIFOs we can
> + * only flush the FIFOs if no other stream has started.
> + */
> + if (i2s->soc_info->shared_fifo_flush) {
> + ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL);
> + ctrl |= JZ_AIC_CTRL_FLUSH;
> + jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl);
> + }
>
> ret = clk_prepare_enable(i2s->clk_i2s);
> if (ret)
> @@ -444,6 +470,7 @@ static struct snd_soc_dai_driver jz4740_i2s_dai =
> {
> static const struct i2s_soc_info jz4740_i2s_soc_info = {
> .version = JZ_I2S_JZ4740,
> .dai = &jz4740_i2s_dai,
> + .shared_fifo_flush = true,
> };
>
> static const struct i2s_soc_info jz4760_i2s_soc_info = {
> --
> 2.35.1
>


\
 
 \ /
  Last update: 2022-07-20 13:47    [W:0.177 / U:1.784 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site