lkml.org 
[lkml]   [2022]   [Oct]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 1/9] ASoC: jz4740-i2s: Handle independent FIFO flush bits
Hi Aidan,

Le sam. 22 oct. 2022 à 20:13:00 +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.
>
> Independent FIFOs can be flushed before the snd_soc_dai_active()
> check because it won't disturb other active streams. This ensures
> that the FIFO we're about to use is always flushed before starting
> up. With shared FIFOs we can't do that because if another substream
> is active, flushing its FIFO would cause underrun errors.
>
> 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>

If you have a Fixes: tag you need to Cc linux-stable as well.
See "option 1" of
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

Also, a cover letter with a description of the changes is good, but you
should still add a changelog per patch (below the --- line so that it
doesn't end up in the commit message). That makes it much easier to
review.

With the Cc line added:
Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
> sound/soc/jz4740/jz4740-i2s.c | 39
> ++++++++++++++++++++++++++++++-----
> 1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/jz4740/jz4740-i2s.c
> b/sound/soc/jz4740/jz4740-i2s.c
> index c4c1e89b47c1..83cb81999c6f 100644
> --- a/sound/soc/jz4740/jz4740-i2s.c
> +++ b/sound/soc/jz4740/jz4740-i2s.c
> @@ -55,7 +55,8 @@
> #define JZ_AIC_CTRL_MONO_TO_STEREO BIT(11)
> #define JZ_AIC_CTRL_SWITCH_ENDIANNESS BIT(10)
> #define JZ_AIC_CTRL_SIGNED_TO_UNSIGNED BIT(9)
> -#define JZ_AIC_CTRL_FLUSH BIT(8)
> +#define JZ_AIC_CTRL_TFLUSH BIT(8)
> +#define JZ_AIC_CTRL_RFLUSH BIT(7)
> #define JZ_AIC_CTRL_ENABLE_ROR_INT BIT(6)
> #define JZ_AIC_CTRL_ENABLE_TUR_INT BIT(5)
> #define JZ_AIC_CTRL_ENABLE_RFS_INT BIT(4)
> @@ -90,6 +91,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 {
> @@ -116,19 +119,44 @@ static inline void jz4740_i2s_write(const
> struct jz4740_i2s *i2s,
> writel(value, i2s->base + reg);
> }
>
> +static inline void jz4740_i2s_set_bits(const struct jz4740_i2s *i2s,
> + unsigned int reg, uint32_t bits)
> +{
> + uint32_t value = jz4740_i2s_read(i2s, reg);
> + value |= bits;
> + jz4740_i2s_write(i2s, reg, value);
> +}
> +
> static int jz4740_i2s_startup(struct snd_pcm_substream *substream,
> struct snd_soc_dai *dai)
> {
> struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> - uint32_t conf, ctrl;
> + uint32_t conf;
> int ret;
>
> + /*
> + * When we can flush FIFOs independently, only flush the FIFO
> + * that is starting up. We can do this when the DAI is active
> + * because it does not disturb other active substreams.
> + */
> + if (!i2s->soc_info->shared_fifo_flush) {
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + jz4740_i2s_set_bits(i2s, JZ_REG_AIC_CTRL, JZ_AIC_CTRL_TFLUSH);
> + else
> + jz4740_i2s_set_bits(i2s, JZ_REG_AIC_CTRL, JZ_AIC_CTRL_RFLUSH);
> + }
> +
> 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, the TFLUSH
> + * bit flushes both FIFOs. Flushing while the DAI is active would
> + * cause FIFO underruns in other active substreams so we have to
> + * guard this behind the snd_soc_dai_active() check.
> + */
> + if (i2s->soc_info->shared_fifo_flush)
> + jz4740_i2s_set_bits(i2s, JZ_REG_AIC_CTRL, JZ_AIC_CTRL_TFLUSH);
>
> ret = clk_prepare_enable(i2s->clk_i2s);
> if (ret)
> @@ -443,6 +471,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.38.1
>


\
 
 \ /
  Last update: 2022-10-22 21:49    [W:0.106 / U:0.956 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site