The i2sbus PCM code uses pi->active to constrain the sibling stream to
an already prepared duplex format and rate in i2sbus_pcm_open().
That state is set from i2sbus_pcm_prepare(), but the current code only
clears it on close. As a result, the sibling stream can inherit stale
constraints after the prepared state has been torn down, or after a new
prepare attempt fails before completing.
Clear pi->active when hw_params() or hw_free() drops the prepared state,
clear it before starting a new prepare attempt, and set it again only
after prepare succeeds.
Replace the stale FIXME in the duplex constraint comment with
a description of the current driver behavior: i2sbus still programs a
single shared transport configuration for both directions, so mixed
formats are not supported in duplex mode.
Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
Cc: [email protected]
Signed-off-by: Cássio Gabriel <[email protected]>
---
sound/aoa/soundbus/i2sbus/pcm.c | 53 ++++++++++++++++++++++++++++++++---------
1 file changed, 42 insertions(+), 11 deletions(-)
diff --git a/sound/aoa/soundbus/i2sbus/pcm.c b/sound/aoa/soundbus/i2sbus/pcm.c
index 97c807e67d56..47a89da43cff 100644
--- a/sound/aoa/soundbus/i2sbus/pcm.c
+++ b/sound/aoa/soundbus/i2sbus/pcm.c
@@ -165,17 +165,16 @@ static int i2sbus_pcm_open(struct i2sbus_dev *i2sdev, int
in)
* currently in use (if any). */
hw->rate_min = 5512;
hw->rate_max = 192000;
- /* if the other stream is active, then we can only
- * support what it is currently using.
- * FIXME: I lied. This comment is wrong. We can support
- * anything that works with the same serial format, ie.
- * when recording 24 bit sound we can well play 16 bit
- * sound at the same time iff using the same transfer mode.
+ /* If the other stream is already prepared, keep this stream
+ * on the same duplex format and rate.
+ *
+ * i2sbus_pcm_prepare() still programs one shared transport
+ * configuration for both directions, so mixed duplex formats
+ * are not supported here.
*/
if (other->active) {
- /* FIXME: is this guaranteed by the alsa api? */
hw->formats &= pcm_format_to_bits(i2sdev->format);
- /* see above, restrict rates to the one we already have */
+ /* Restrict rates to the one already in use. */
hw->rate_min = i2sdev->rate;
hw->rate_max = i2sdev->rate;
}
@@ -283,6 +282,22 @@ void i2sbus_wait_for_stop_both(struct i2sbus_dev *i2sdev)
}
#endif
+static void i2sbus_pcm_clear_active(struct i2sbus_dev *i2sdev, int in)
+{
+ struct pcm_info *pi;
+
+ guard(mutex)(&i2sdev->lock);
+
+ get_pcm_info(i2sdev, in, &pi, NULL);
+ pi->active = 0;
+}
+
+static inline int i2sbus_hw_params(struct snd_pcm_substream *substream, int in)
+{
+ i2sbus_pcm_clear_active(snd_pcm_substream_chip(substream), in);
+ return 0;
+}
+
static inline int i2sbus_hw_free(struct snd_pcm_substream *substream, int in)
{
struct i2sbus_dev *i2sdev = snd_pcm_substream_chip(substream);
@@ -291,14 +306,25 @@ static inline int i2sbus_hw_free(struct snd_pcm_substream
*substream, int in)
get_pcm_info(i2sdev, in, &pi, NULL);
if (pi->dbdma_ring.stopping)
i2sbus_wait_for_stop(i2sdev, pi);
+ i2sbus_pcm_clear_active(i2sdev, in);
return 0;
}
+static int i2sbus_playback_hw_params(struct snd_pcm_substream *substream)
+{
+ return i2sbus_hw_params(substream, 0);
+}
+
static int i2sbus_playback_hw_free(struct snd_pcm_substream *substream)
{
return i2sbus_hw_free(substream, 0);
}
+static int i2sbus_record_hw_params(struct snd_pcm_substream *substream)
+{
+ return i2sbus_hw_params(substream, 1);
+}
+
static int i2sbus_record_hw_free(struct snd_pcm_substream *substream)
{
return i2sbus_hw_free(substream, 1);
@@ -335,7 +361,7 @@ static int i2sbus_pcm_prepare(struct i2sbus_dev *i2sdev,
int in)
return -EINVAL;
runtime = pi->substream->runtime;
- pi->active = 1;
+ pi->active = 0;
if (other->active &&
((i2sdev->format != runtime->format)
|| (i2sdev->rate != runtime->rate)))
@@ -444,9 +470,11 @@ static int i2sbus_pcm_prepare(struct i2sbus_dev *i2sdev,
int in)
/* early exit if already programmed correctly */
/* not locking these is fine since we touch them only in this function
*/
- if (in_le32(&i2sdev->intfregs->serial_format) == sfr
- && in_le32(&i2sdev->intfregs->data_word_sizes) == dws)
+ if (in_le32(&i2sdev->intfregs->serial_format) == sfr &&
+ in_le32(&i2sdev->intfregs->data_word_sizes) == dws) {
+ pi->active = 1;
return 0;
+ }
/* let's notify the codecs about clocks going away.
* For now we only do mastering on the i2s cell... */
@@ -484,6 +512,7 @@ static int i2sbus_pcm_prepare(struct i2sbus_dev *i2sdev,
int in)
if (cii->codec->switch_clock)
cii->codec->switch_clock(cii, CLOCK_SWITCH_SLAVE);
+ pi->active = 1;
return 0;
}
@@ -728,6 +757,7 @@ static snd_pcm_uframes_t i2sbus_playback_pointer(struct
snd_pcm_substream
static const struct snd_pcm_ops i2sbus_playback_ops = {
.open = i2sbus_playback_open,
.close = i2sbus_playback_close,
+ .hw_params = i2sbus_playback_hw_params,
.hw_free = i2sbus_playback_hw_free,
.prepare = i2sbus_playback_prepare,
.trigger = i2sbus_playback_trigger,
@@ -796,6 +826,7 @@ static snd_pcm_uframes_t i2sbus_record_pointer(struct
snd_pcm_substream
static const struct snd_pcm_ops i2sbus_record_ops = {
.open = i2sbus_record_open,
.close = i2sbus_record_close,
+ .hw_params = i2sbus_record_hw_params,
.hw_free = i2sbus_record_hw_free,
.prepare = i2sbus_record_prepare,
.trigger = i2sbus_record_trigger,
---
base-commit: 46a6512f4a74dd7b18d9a455669c226843fc49ce
change-id: 20260329-aoa-i2sbus-clear-stale-active-4d3b706cc0ef
Best regards,
--
Cássio Gabriel <[email protected]>
