[PATCH 3/4] soundwire: stream: allow flagged BPT firmware download while streams are idle

[Syed Saba Kareem]

From: Syed Saba Kareem <Syed.SabaKareem@xxxxxxx>

sdw_master_rt_alloc() rejected a BPT (Bulk Payload Transport) stream
allocation whenever any audio stream was allocated on the bus
(bus->stream_refcount > 0). On a power-off-mode platform, an amplifier
that was left DISABLED across system suspend still holds an allocated
but idle stream runtime, yet it must re-download its firmware over BPT
on resume before that stream can be re-enabled. The blanket refcount
check made the resume-time BPT transfer fail with -EBUSY.

Rather than key this off the audio stream state - which would relax the
policy for every caller and every scenario, not just a controlled
firmware download - let the manager explicitly flag the transfer. Add
bus->bpt_fw_download, which a BPT-capable manager sets around a firmware
download that it knows may coexist with allocated-but-idle audio streams
and for which it guarantees no audio stream is made active on the bus for
the duration.

sdw_master_rt_alloc() now rejects a BPT allocation when:

 - another BPT transfer is already allocated, or
 - an audio stream is actively using the bus (PREPARED/ENABLED), or
 - an audio stream is merely allocated but idle and the manager has not
   set bus->bpt_fw_download.

Add sdw_bus_has_active_stream() for the active-stream test; it returns
true only for streams in the PREPARED or ENABLED state. When
bpt_fw_download is clear (the default, and the only possibility for
managers that do not opt in) the three checks together reproduce the
original strict policy exactly: any allocated stream, active or idle,
still blocks BPT. Only a manager that sets the flag can allocate BPT
alongside idle audio streams, and even then an actively streaming stream
still blocks it.

The audio path is still protected while a flagged download runs:
sdw_program_params() skips master runtimes other than the active BPT
stream while bus->bpt_stream is set, so BPT preparation does not rewrite
the transport/port parameters of idle audio runtimes or deliver BPT bus
parameters to their peripherals via sdw_notify_config(). The bus-wide
SDW_SCP_BUSCLOCK_SCALE programming is intentionally left unfiltered, as
every attached peripheral must track the actual bus clock. The filter
keys off bus->bpt_stream, which managers publish with WRITE_ONCE() only
after raising bpt_stream_refcount and clear before dropping it;
sdw_program_params() reads it with READ_ONCE(). So an audio path that
sees refcount == 0 under bus_lock also sees bpt_stream == NULL and
programs its own parameters instead of being skipped.

BPT and active audio are mutually exclusive on the bus: a flagged
download is only started while all audio streams are idle, and the
manager that sets bpt_fw_download guarantees that no audio stream is
made active (PREPARED/ENABLED) on the bus for the transfer's duration.
Serialising the two is the manager's/codec's responsibility (for
example, the codec completes its firmware download before starting its
own stream). In practice the flagged download runs in the codec's
power-off-mode resume path, before its stream - left DISABLED across
suspend - is re-enabled, and while userspace tasks are still frozen for
system resume, so no userspace PCM operation (prepare, enable or hw_free)
can race the download window. Because of that guarantee, the only
sdw_program_params() passes while bus->bpt_stream is set are the BPT
stream's own prepare/enable/disable/deprepare passes; the filter above
keeps those passes from reprogramming or re-notifying the idle audio
runtimes that remain allocated on the bus. It is not a mechanism for
running audio traffic concurrently with a download.

While at it, make the allocation-time rejection messages state the
actual reason instead of printing the now-misleading stream_refcount.

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/stream.c    | 97 +++++++++++++++++++++++++++++++++--
 include/linux/soundwire/sdw.h | 14 +++++
 2 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index fb5fdafe1999..5162c9cd327f 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -674,9 +674,11 @@ static int sdw_notify_config(struct sdw_master_runtime *m_rt)
 static int sdw_program_params(struct sdw_bus *bus, bool prepare)
 {
 	struct sdw_master_runtime *m_rt;
+	struct sdw_stream_runtime *bpt;
 	struct sdw_slave *slave;
 	int ret = 0;
 	u32 addr1;
+	bool bpt_seen = false;
 
 	/* Check if all Peripherals comply with SDCA */
 	list_for_each_entry(slave, &bus->slaves, node) {
@@ -719,7 +721,21 @@ static int sdw_program_params(struct sdw_bus *bus, bool prepare)
 	}
 
 manager_runtime:
+	/*
+	 * Read bus->bpt_stream once so the whole programming pass uses a
+	 * consistent snapshot. While a BPT transfer owns the bus, only its own
+	 * runtime may be (re)programmed. Any audio runtimes still allocated
+	 * during a flagged resume-time download are idle by contract; skip them
+	 * so BPT preparation does not rewrite their transport/port parameters or
+	 * deliver BPT bus parameters to their Slaves via sdw_notify_config().
+	 */
+	bpt = READ_ONCE(bus->bpt_stream);
 	list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
+		if (bpt) {
+			if (m_rt->stream != bpt)
+				continue;
+			bpt_seen = true;
+		}
 
 		/*
 		 * this loop walks through all master runtimes for a
@@ -756,6 +772,24 @@ static int sdw_program_params(struct sdw_bus *bus, bool prepare)
 		}
 	}
 
+	/*
+	 * bpt_stream is published only while its runtime is on m_rt_list: the
+	 * manager adds the runtime before prepare and clears bpt_stream before
+	 * removing it at teardown, so a non-NULL bpt must always be matched in
+	 * the loop above. If it was not, a BPT teardown left a dangling pointer
+	 * on an error path (or a manager violated the exclusivity contract):
+	 * every audio runtime was filtered out and nothing was programmed.
+	 * Fail loudly rather than return success for Slave registers that were
+	 * never written, which would let the stream state machine advance to
+	 * the bank switch on stale hardware.
+	 */
+	if (bpt && !bpt_seen) {
+		dev_err(bus->dev,
+			"BPT stream set but its runtime is absent; skipped programming\n");
+		WARN_ON_ONCE(1);
+		return -EINVAL;
+	}
+
 	return ret;
 }
 
@@ -1234,6 +1268,33 @@ static struct sdw_master_runtime
 	return NULL;
 }
 
+/*
+ * sdw_bus_has_active_stream() - check for an audio stream actively using the bus
+ *
+ * Returns true if any master runtime on @bus has a stream in the PREPARED or
+ * ENABLED state, i.e. one that is reserving or moving data over the bus. BPT
+ * and active audio are mutually exclusive, so a BPT transfer must not be
+ * started while this returns true. Allocated-but-idle streams
+ * (ALLOCATED/CONFIGURED/DISABLED/DEPREPARED) are not reported here; whether
+ * they permit BPT is decided by the caller and gated on the
+ * bus->bpt_fw_download resume flag.
+ *
+ * Must be called with bus_lock held.
+ */
+static bool sdw_bus_has_active_stream(struct sdw_bus *bus)
+{
+	struct sdw_master_runtime *m_rt;
+
+	list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
+		if (m_rt->stream &&
+		    (m_rt->stream->state == SDW_STREAM_PREPARED ||
+		     m_rt->stream->state == SDW_STREAM_ENABLED))
+			return true;
+	}
+
+	return false;
+}
+
 /**
  * sdw_master_rt_alloc() - Allocates a Master runtime handle
  *
@@ -1250,9 +1311,39 @@ static struct sdw_master_runtime
 	struct list_head *insert_after;
 
 	if (stream->type == SDW_STREAM_BPT) {
-		if (bus->stream_refcount > 0 || bus->bpt_stream_refcount > 0) {
-			dev_err(bus->dev, "%s: %d/%d audio/BPT stream already allocated\n",
-				__func__, bus->stream_refcount, bus->bpt_stream_refcount);
+		/*
+		 * BPT and audio are mutually exclusive on the bus: BPT needs
+		 * exclusive bandwidth, so it must never run while another BPT
+		 * transfer is allocated or while an audio stream is actively using
+		 * the bus (PREPARED/ENABLED).
+		 *
+		 * The one exception is resume-time firmware download, flagged by the
+		 * manager via bus->bpt_fw_download: on a power-off-mode platform the
+		 * codec loses power across system suspend and must re-download its
+		 * firmware over BPT before its stream (left DISABLED across suspend)
+		 * can be re-enabled. In that window the manager guarantees no audio
+		 * stream is made active, so there is no concurrent audio and no
+		 * bandwidth to share; only then may BPT proceed while an idle
+		 * allocated stream exists. Without the flag such a stream blocks BPT.
+		 * This is not a mechanism for running audio concurrently with a
+		 * download.
+		 */
+		if (bus->bpt_stream_refcount > 0) {
+			dev_err(bus->dev,
+				"%s: BPT rejected: another BPT transfer active\n",
+				__func__);
+			return ERR_PTR(-EBUSY);
+		}
+		if (sdw_bus_has_active_stream(bus)) {
+			dev_err(bus->dev,
+				"%s: BPT rejected: audio stream active\n",
+				__func__);
+			return ERR_PTR(-EBUSY);
+		}
+		if (bus->stream_refcount > 0 && !READ_ONCE(bus->bpt_fw_download)) {
+			dev_err(bus->dev,
+				"%s: BPT rejected: audio stream allocated\n",
+				__func__);
 			return ERR_PTR(-EBUSY);
 		}
 	} else {
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f710e5932b4b..9a3904f42078 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -1005,6 +1005,19 @@ struct sdw_stream_runtime {
  * @bpt_stream_refcount: number of BTP streams currently using this bus (should
  * be zero or one, multiple streams per link is not supported).
  * @bpt_stream: pointer stored to handle BTP streams.
+ * @bpt_fw_download: set by a BPT-capable manager to flag a resume-time firmware
+ * download (BPT/BRA). BPT and active audio are mutually exclusive on the bus;
+ * this flag marks the one narrow exception -- a power-off-mode resume where the
+ * codec must re-download firmware over BPT before its stream (left DISABLED
+ * across suspend) is re-enabled. The manager guarantees no audio stream is made
+ * active on the bus for the duration, so sdw_master_rt_alloc() permits the BPT
+ * allocation even when idle audio streams are still allocated; an actively
+ * streaming audio stream (PREPARED/ENABLED) still blocks BPT. It is not a
+ * mechanism for running audio concurrently with a download. Written with
+ * WRITE_ONCE() by the manager before it enters the stream allocation path and
+ * cleared (also WRITE_ONCE()) after the transfer; read with READ_ONCE() in
+ * sdw_master_rt_alloc() under bus_lock. Single-BPT exclusivity
+ * (bpt_stream_refcount) means no concurrent writer races the lock-protected read.
  * @ops: Master callback ops
  * @port_ops: Master port callback ops
  * @prop: Master properties
@@ -1045,6 +1058,7 @@ struct sdw_bus {
 	int stream_refcount;
 	int bpt_stream_refcount;
 	struct sdw_stream_runtime *bpt_stream;
+	bool bpt_fw_download;
 	const struct sdw_master_ops *ops;
 	const struct sdw_master_port_ops *port_ops;
 	struct sdw_master_prop prop;
-- 
2.43.0