[PATCH 2/4] soundwire: intel_ace2x: order bpt_stream publish/clear against refcount
From: Syed Saba Kareem
Date: Wed Sep 09 2026 - 09:10:21 EST
From: Syed Saba Kareem <Syed.SabaKareem@xxxxxxx>
The BPT (Bulk Payload Transport) stream pointer bus->bpt_stream is read
locklessly by the SoundWire core to tell whether a BPT transfer owns the
bus. For those readers to be safe the pointer and bus->bpt_stream_refcount
must stay consistent: an observer that sees refcount == 0 under bus_lock
must also see bpt_stream == NULL.
Make intel_ace2x maintain that ordering:
- Publish bus->bpt_stream with WRITE_ONCE() only after the master
runtime has been added and bpt_stream_refcount raised, and (on the
open path) before the in-open sdw_prepare_stream().
- Clear it with WRITE_ONCE() before sdw_stream_remove_master() drops the
refcount on the close and error paths. Add a clear_bpt_stream label so
paths that already published the pointer clear it, while the
pre-publish failure paths skip the clear.
- Snapshot the pointer into a local once (READ_ONCE()) so the
open/close/error paths act on a single stable value instead of
repeatedly re-reading the shared field.
This is a no-op under the current policy, where BPT and audio streams are
mutually exclusive, but establishes the ordering the core relies on once
BPT is allowed to run alongside idle audio streams.
Reviewed-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
Reviewed-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@xxxxxxx>
---
drivers/soundwire/intel_ace2x.c | 63 ++++++++++++++++++++++++---------
1 file changed, 46 insertions(+), 17 deletions(-)
diff --git a/drivers/soundwire/intel_ace2x.c b/drivers/soundwire/intel_ace2x.c
index 96940779fdec..65d3630c2822 100644
--- a/drivers/soundwire/intel_ace2x.c
+++ b/drivers/soundwire/intel_ace2x.c
@@ -83,7 +83,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
int len;
int i;
- if (cdns->bus.bpt_stream) {
+ if (READ_ONCE(cdns->bus.bpt_stream)) {
dev_err(cdns->dev, "%s: BPT stream already exists\n", __func__);
return -EAGAIN;
}
@@ -92,8 +92,6 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
if (!stream)
return -ENOMEM;
- cdns->bus.bpt_stream = stream;
-
ret = sdw_slave_bpt_stream_add(slave, stream);
if (ret < 0)
goto release_stream;
@@ -152,9 +150,25 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
goto remove_master;
}
- ret = sdw_prepare_stream(cdns->bus.bpt_stream);
+ /*
+ * Publish bus->bpt_stream now that the BPT master runtime is fully
+ * built and bpt_stream_refcount has been raised. The increment happens
+ * in sdw_slave_bpt_stream_add() above, via sdw_stream_add_slave() ->
+ * sdw_master_rt_alloc(); sdw_stream_add_master() then reuses that same
+ * runtime through sdw_master_rt_find() without incrementing it again.
+ * Publish before sdw_prepare_stream() below: the prepare runs
+ * sdw_program_params(), whose filter skips idle audio runtimes only
+ * while bus->bpt_stream is set, so publishing here (rather than after
+ * the DMA setup) keeps that filter active for the BPT prepare. Ordering
+ * the publish after the refcount is raised keeps the pointer and
+ * refcount consistent for lockless observers, mirroring amd_manager.c
+ * and pairing with the READ_ONCE() in sdw_program_params().
+ */
+ WRITE_ONCE(cdns->bus.bpt_stream, stream);
+
+ ret = sdw_prepare_stream(stream);
if (ret < 0)
- goto remove_master;
+ goto clear_bpt_stream;
command = (msg->flags & SDW_MSG_FLAG_WRITE) ? 0 : 1;
@@ -285,22 +299,30 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
__func__, ret1);
deprepare_stream:
- sdw_deprepare_stream(cdns->bus.bpt_stream);
+ sdw_deprepare_stream(stream);
+
+clear_bpt_stream:
+ /*
+ * Paths that jump here published bus->bpt_stream above; clear it before
+ * sdw_stream_remove_master() drops bpt_stream_refcount so the pointer and
+ * refcount stay consistent for lockless observers. The pre-publish failure
+ * paths jump to remove_master and skip this clear.
+ */
+ WRITE_ONCE(cdns->bus.bpt_stream, NULL);
remove_master:
- ret1 = sdw_stream_remove_master(&cdns->bus, cdns->bus.bpt_stream);
+ ret1 = sdw_stream_remove_master(&cdns->bus, stream);
if (ret1 < 0)
dev_err(cdns->dev, "%s: remove master failed: %d\n",
__func__, ret1);
- ret1 = sdw_stream_remove_slave(slave, cdns->bus.bpt_stream);
+ ret1 = sdw_stream_remove_slave(slave, stream);
if (ret1 < 0)
dev_err(cdns->dev, "%s: remove slave failed: %d\n",
__func__, ret1);
release_stream:
- sdw_release_stream(cdns->bus.bpt_stream);
- cdns->bus.bpt_stream = NULL;
+ sdw_release_stream(stream);
return ret;
}
@@ -309,6 +331,7 @@ static void intel_ace2x_bpt_close_stream(struct sdw_intel *sdw, struct sdw_slave
struct sdw_bpt_msg *msg)
{
struct sdw_cdns *cdns = &sdw->cdns;
+ struct sdw_stream_runtime *stream = READ_ONCE(cdns->bus.bpt_stream);
int ret;
ret = hda_sdw_bpt_close(cdns->dev->parent /* PCI device */, sdw->instance,
@@ -319,23 +342,29 @@ static void intel_ace2x_bpt_close_stream(struct sdw_intel *sdw, struct sdw_slave
dev_err(cdns->dev, "%s: hda_sdw_bpt_close failed: ret %d\n",
__func__, ret);
- ret = sdw_deprepare_stream(cdns->bus.bpt_stream);
+ ret = sdw_deprepare_stream(stream);
if (ret < 0)
dev_err(cdns->dev, "%s: sdw_deprepare_stream failed: ret %d\n",
__func__, ret);
- ret = sdw_stream_remove_master(&cdns->bus, cdns->bus.bpt_stream);
+ /*
+ * Clear bus->bpt_stream before sdw_stream_remove_master() drops
+ * bpt_stream_refcount, so the pointer is never visible while the
+ * refcount reads zero (mirrors the open path and amd_manager.c).
+ */
+ WRITE_ONCE(cdns->bus.bpt_stream, NULL);
+
+ ret = sdw_stream_remove_master(&cdns->bus, stream);
if (ret < 0)
dev_err(cdns->dev, "%s: remove master failed: %d\n",
__func__, ret);
- ret = sdw_stream_remove_slave(slave, cdns->bus.bpt_stream);
+ ret = sdw_stream_remove_slave(slave, stream);
if (ret < 0)
dev_err(cdns->dev, "%s: remove slave failed: %d\n",
__func__, ret);
- sdw_release_stream(cdns->bus.bpt_stream);
- cdns->bus.bpt_stream = NULL;
+ sdw_release_stream(stream);
}
#define INTEL_BPT_MSG_BYTE_MIN 16
@@ -374,7 +403,7 @@ static int intel_ace2x_bpt_send_async(struct sdw_intel *sdw, struct sdw_slave *s
return ret;
}
- ret = sdw_enable_stream(cdns->bus.bpt_stream);
+ ret = sdw_enable_stream(READ_ONCE(cdns->bus.bpt_stream));
if (ret < 0) {
dev_err(cdns->dev, "%s: sdw_stream_enable failed: %d\n",
__func__, ret);
@@ -397,7 +426,7 @@ static int intel_ace2x_bpt_wait(struct sdw_intel *sdw, struct sdw_slave *slave,
if (ret < 0)
dev_err(cdns->dev, "%s: hda_sdw_bpt_wait failed: %d\n", __func__, ret);
- ret = sdw_disable_stream(cdns->bus.bpt_stream);
+ ret = sdw_disable_stream(READ_ONCE(cdns->bus.bpt_stream));
if (ret < 0) {
dev_err(cdns->dev, "%s: sdw_stream_enable failed: %d\n",
__func__, ret);
--
2.43.0