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 <[email protected]>
---
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)
+
#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);
+ }
+
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
Hi Aidan,
Le ven., juil. 8 2022 at 17:02:34 +0100, Aidan MacDonald
<[email protected]> 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 <[email protected]>
> ---
> 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
>
Paul Cercueil <[email protected]> writes:
> Hi Aidan,
>
> Le ven., juil. 8 2022 at 17:02:34 +0100, Aidan MacDonald
> <[email protected]> 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 <[email protected]>
>> ---
>> 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.
>
According to the JZ4740 programming manual JZ_AIC_CTRL_FLUSH flushes
both FIFOs, so it's not equivalent JZ4760_AIC_CTRL_TFLUSH. I don't
think it's a good idea to confuse the two, or we'd need comments to
explain why JZ4740 uses TFLUSH but not RFLUSH.
>> +
>> #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?
snd_soc_dai_active() is essentially checking if there's an active
substream. Eg. if no streams are open and you start playback, then
the DAI will be inactive. If you then start capture while playback is
running, the DAI is already active.
With a shared flush bit we can only flush if there are no other active
substreams (because we don't want to disturb the active stream by
flushing the FIFO) so it goes after the snd_soc_dai_active() check.
When the FIFOs can be separately flushed, flushing can be done before
the check because it won't disturb any active substream.
> 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
>>
Hi Aidan,
Le mer., juil. 20 2022 at 15:43:06 +0100, Aidan MacDonald
<[email protected]> a ?crit :
>
> Paul Cercueil <[email protected]> writes:
>
>> Hi Aidan,
>>
>> Le ven., juil. 8 2022 at 17:02:34 +0100, Aidan MacDonald
>> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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.
>>
>
> According to the JZ4740 programming manual JZ_AIC_CTRL_FLUSH flushes
> both FIFOs, so it's not equivalent JZ4760_AIC_CTRL_TFLUSH. I don't
> think it's a good idea to confuse the two, or we'd need comments to
> explain why JZ4740 uses TFLUSH but not RFLUSH.
"shared_fifo_flush" is pretty much self-explanatory though. It then
becomes obvious looking at the code that when this flag is set, TFLUSH
flushes both FIFOs.
If you prefer... you can #define JZ_AIC_CTRL_FLUSH JZ_AIC_CTRL_TFLUSH.
I don't like the JZ4760 prefix, this is in no way specific to the
JZ4760.
>
>>> +
>>> #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?
>
> snd_soc_dai_active() is essentially checking if there's an active
> substream. Eg. if no streams are open and you start playback, then
> the DAI will be inactive. If you then start capture while playback is
> running, the DAI is already active.
>
> With a shared flush bit we can only flush if there are no other active
> substreams (because we don't want to disturb the active stream by
> flushing the FIFO) so it goes after the snd_soc_dai_active() check.
>
> When the FIFOs can be separately flushed, flushing can be done before
> the check because it won't disturb any active substream.
Ok. It makes sense then. Please add some info about this in the commit
message, because it really wasn't obvious to me.
You should maybe factorize the read-modify-write into its own function.
I know this gets eventually modified by [03/11], but this [01/11] is a
bugfix so it will be applied to older kernels, and I'd rather not have
duplicated code there.
Cheers,
-Paul
>
>> 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
>>>
>
Paul Cercueil <[email protected]> writes:
> Hi Aidan,
>
> Le mer., juil. 20 2022 at 15:43:06 +0100, Aidan MacDonald
> <[email protected]> a écrit :
>> Paul Cercueil <[email protected]> writes:
>>
>> According to the JZ4740 programming manual JZ_AIC_CTRL_FLUSH flushes
>> both FIFOs, so it's not equivalent JZ4760_AIC_CTRL_TFLUSH. I don't
>> think it's a good idea to confuse the two, or we'd need comments to
>> explain why JZ4740 uses TFLUSH but not RFLUSH.
>
> "shared_fifo_flush" is pretty much self-explanatory though. It then becomes
> obvious looking at the code that when this flag is set, TFLUSH flushes both
> FIFOs.
>
> If you prefer... you can #define JZ_AIC_CTRL_FLUSH JZ_AIC_CTRL_TFLUSH. I don't
> like the JZ4760 prefix, this is in no way specific to the JZ4760.
>
Makes sense, I'll stick with TFLUSH / RFLUSH only.
>>
>>>> +
>>>> #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?
>> snd_soc_dai_active() is essentially checking if there's an active
>> substream. Eg. if no streams are open and you start playback, then
>> the DAI will be inactive. If you then start capture while playback is
>> running, the DAI is already active.
>> With a shared flush bit we can only flush if there are no other active
>> substreams (because we don't want to disturb the active stream by
>> flushing the FIFO) so it goes after the snd_soc_dai_active() check.
>> When the FIFOs can be separately flushed, flushing can be done before
>> the check because it won't disturb any active substream.
>
> Ok. It makes sense then. Please add some info about this in the commit message,
> because it really wasn't obvious to me.
It wasn't that obvious to me either :)
I've added code comments too since it seems likely to trip people up
if you're only taking a casual glance.
> You should maybe factorize the read-modify-write into its own function. I know
> this gets eventually modified by [03/11], but this [01/11] is a bugfix so it
> will be applied to older kernels, and I'd rather not have duplicated code
> there.
>
> Cheers,
> -Paul
And I've factored out the r-m-w helper as requested.
Regards,
Aidan