[PATCH v2 0/5] soundwire: Fix bandwidth leak in multi-link aggregation failure

[PATCH v2 0/5] soundwire: Fix bandwidth leak in multi-link aggregation failure

[Bard Liao]

The existing code only copies the params from the first link to the
bus params, but it doesn't copy the bandwidth from the other links.
This can lead to a bandwidth leak when multi-link aggregation fails.
Fix this by saving a snapshot of each bus's parameters before making
any modifications.

Changes in v2:
- Fix lane and rate group handling in the generic bandwidth allocation
that the rollback depends on:
  - Pass the prepare/deprepare context to compute_params().
  - Select the runtime of the stream being prepared for lane allocation
    instead of the first runtime on the bus, which could be an already
    running stream.
  - Include the current stream in the group and port computation when
    it is prepared again from the DEPREPARED state.
- Reset the lane of the stream being prepared to 0 when no multi-lane
is selected, so the lane assignment matches the restored lane_used_bandwidth.

Bard Liao (4):
  soundwire: move lane bandwidth into bus params
  soundwire: Pass prepare context to parameter computation
  soundwire: Select the correct runtime for lane allocation
  soundwire: Include the current stream when preparing again

David Lin (1):
  soundwire: stream: Fix bandwidth leak in multi-link aggregation
    failure

 drivers/soundwire/amd_manager.c               |  3 +-
 drivers/soundwire/bus.h                       |  3 +
 .../soundwire/generic_bandwidth_allocation.c  | 81 ++++++++++++++-----
 drivers/soundwire/qcom.c                      |  3 +-
 drivers/soundwire/stream.c                    | 25 ++++--
 include/linux/soundwire/sdw.h                 | 13 +--
 6 files changed, 92 insertions(+), 36 deletions(-)

-- 
2.43.0

[PATCH v2 1/5] soundwire: move lane bandwidth into bus params

[Bard Liao]

lane_used_bandwidth is used to store the used bandwidth for each lane.
When we restore the bus->params, the lane_used_bandwidth value will not
be restored in current code. Move it to sdw_bus_params to fix the issue.

Fixes: 366fd59fb894 ("soundwire: generic_bandwidth_allocation: add lane in sdw_group_params")
Cc: stable@vger.kernel.org
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: David Lin <david.lin@intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/generic_bandwidth_allocation.c | 8 ++++----
 drivers/soundwire/stream.c                       | 4 ++--
 include/linux/soundwire/sdw.h                    | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index 30a495abe19e..3d6b9c895e46 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -196,7 +196,7 @@ static void _sdw_compute_port_params(struct sdw_bus *bus,
 
 	/* Run loop for all groups to compute transport parameters */
 	for (l = 0; l < SDW_MAX_LANES; l++) {
-		if (l > 0 && !bus->lane_used_bandwidth[l])
+		if (l > 0 && !bus->params.lane_used_bandwidth[l])
 			continue;
 		/* reset hstop for each lane */
 		hstop = bus->params.col - 1;
@@ -267,7 +267,7 @@ static int sdw_compute_group_params(struct sdw_bus *bus,
 	}
 
 	for (l = 0; l < SDW_MAX_LANES; l++) {
-		if (l > 0 && !bus->lane_used_bandwidth[l])
+		if (l > 0 && !bus->params.lane_used_bandwidth[l])
 			continue;
 		/* reset column_needed for each lane */
 		column_needed = 0;
@@ -507,7 +507,7 @@ static int get_manager_lane(struct sdw_bus *bus, struct sdw_master_runtime *m_rt
 					      m_rt->stream->params.bps;
 		}
 		if (required_bandwidth <=
-		    curr_dr_freq - bus->lane_used_bandwidth[l]) {
+		    curr_dr_freq - bus->params.lane_used_bandwidth[l]) {
 			/* Check if m_lane is connected to all Peripherals */
 			if (!is_lane_connected_to_all_peripherals(m_rt,
 				slave_prop->lane_maps[l])) {
@@ -518,7 +518,7 @@ static int get_manager_lane(struct sdw_bus *bus, struct sdw_master_runtime *m_rt
 			}
 			m_lane = slave_prop->lane_maps[l];
 			dev_dbg(&s_rt->slave->dev, "M lane %d is used\n", m_lane);
-			bus->lane_used_bandwidth[l] += required_bandwidth;
+			bus->params.lane_used_bandwidth[l] += required_bandwidth;
 			/*
 			 * Use non-zero manager lane, subtract the lane 0
 			 * bandwidth that is already calculated
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 31db368ba24b..1d74aa982a4c 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1794,8 +1794,8 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 			bandwidth = m_rt->stream->params.rate * hweight32(p_rt->ch_mask) *
 				    m_rt->stream->params.bps;
 			multi_lane_bandwidth += bandwidth;
-			bus->lane_used_bandwidth[p_rt->lane] -= bandwidth;
-			if (!bus->lane_used_bandwidth[p_rt->lane])
+			bus->params.lane_used_bandwidth[p_rt->lane] -= bandwidth;
+			if (!bus->params.lane_used_bandwidth[p_rt->lane])
 				p_rt->lane = 0;
 		}
 		/* TODO: Update this during Device-Device support */
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f710e5932b4b..fff30bb2d9ed 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -592,6 +592,7 @@ enum sdw_port_prep_ops {
  * @bandwidth: Current bandwidth
  * @col: Active columns
  * @row: Active rows
+ * @lane_used_bandwidth: how much bandwidth in bits per second is used by each lane
  * @s_data_mode: NORMAL, STATIC or PRBS mode for all Slave ports
  * @m_data_mode: NORMAL, STATIC or PRBS mode for all Master ports. The value
  * should be the same to detect transmission issues, but can be different to
@@ -605,6 +606,7 @@ struct sdw_bus_params {
 	unsigned int bandwidth;
 	unsigned int col;
 	unsigned int row;
+	unsigned int lane_used_bandwidth[SDW_MAX_LANES];
 	int s_data_mode;
 	int m_data_mode;
 };
@@ -1028,7 +1030,6 @@ struct sdw_stream_runtime {
  * @multi_link: Store bus property that indicates if multi links
  * are supported. This flag is populated by drivers after reading
  * appropriate firmware (ACPI/DT).
- * @lane_used_bandwidth: how much bandwidth in bits per second is used by each lane
  */
 struct sdw_bus {
 	struct device *dev;
@@ -1063,7 +1064,6 @@ struct sdw_bus {
 	struct dentry *debugfs;
 #endif
 	bool multi_link;
-	unsigned int lane_used_bandwidth[SDW_MAX_LANES];
 };
 
 struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name, enum sdw_stream_type type);
-- 
2.43.0

[PATCH v2 2/5] soundwire: Pass prepare context to parameter computation

[Bard Liao]

The compute_params callback is called from both stream prepare and
deprepare. A followup commit will make it perform different actions
depending on which operation invokes it.

No functional change.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/amd_manager.c                  | 3 ++-
 drivers/soundwire/generic_bandwidth_allocation.c | 4 +++-
 drivers/soundwire/qcom.c                         | 3 ++-
 drivers/soundwire/stream.c                       | 4 ++--
 include/linux/soundwire/sdw.h                    | 9 ++++++---
 5 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index a3316efdf8ac..f40a3af4351f 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -465,7 +465,8 @@ static u32 amd_sdw_read_ping_status(struct sdw_bus *bus)
 	return slave_stat;
 }
 
-static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
+static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
+				  bool prepare)
 {
 	struct amd_sdw_manager *amd_manager = to_amd_sdw(bus);
 	struct sdw_transport_data t_data = {0};
diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index 3d6b9c895e46..1250fe21a3ff 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -657,8 +657,10 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
  *
  * @bus: SDW Bus instance
  * @stream: Soundwire stream
+ * @prepare: True when preparing, false when depreparing
  */
-int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
+int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
+		       bool prepare)
 {
 	int ret;
 
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 6e0c5fb10efa..73d3bdc4afc2 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1215,7 +1215,8 @@ static const struct sdw_master_ops qcom_swrm_ops = {
 	.pre_bank_switch = qcom_swrm_pre_bank_switch,
 };
 
-static int qcom_swrm_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
+static int qcom_swrm_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
+				    bool prepare)
 {
 	struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
 	struct sdw_master_runtime *m_rt;
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 1d74aa982a4c..06f051f03a21 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1502,7 +1502,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
 
 			/* Compute params */
 			if (bus->compute_params) {
-				ret = bus->compute_params(bus, stream);
+				ret = bus->compute_params(bus, stream, true);
 				if (ret < 0) {
 					dev_err(bus->dev, "Compute params failed: %d\n",
 						ret);
@@ -1804,7 +1804,7 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 
 		/* Compute params */
 		if (bus->compute_params) {
-			ret = bus->compute_params(bus, stream);
+			ret = bus->compute_params(bus, stream, false);
 			if (ret < 0) {
 				dev_err(bus->dev, "Compute params failed: %d\n",
 					ret);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index fff30bb2d9ed..0e37b7d5ec48 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -1019,7 +1019,8 @@ struct sdw_stream_runtime {
  * @controller_id: system-unique controller ID. If set to -1, the bus @id will be used.
  * @link_id: Link id number, can be 0 to N, unique for each Controller
  * @id: bus system-wide unique id
- * @compute_params: points to Bus resource management implementation
+ * @compute_params: points to Bus resource management implementation; the prepare
+ * argument is true when preparing a stream and false when depreparing it
  * @assigned: Bitmap for Slave device numbers.
  * Bit set implies used number, bit clear implies unused number.
  * @clk_stop_timeout: Clock stop timeout computed
@@ -1054,7 +1055,8 @@ struct sdw_bus {
 	int controller_id;
 	unsigned int link_id;
 	int id;
-	int (*compute_params)(struct sdw_bus *bus, struct sdw_stream_runtime *stream);
+	int (*compute_params)(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
+			      bool prepare);
 	DECLARE_BITMAP(assigned, SDW_MAX_DEVICES);
 	unsigned int clk_stop_timeout;
 	u32 bank_switch_timeout;
@@ -1069,7 +1071,8 @@ struct sdw_bus {
 struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name, enum sdw_stream_type type);
 void sdw_release_stream(struct sdw_stream_runtime *stream);
 
-int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream);
+int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
+		       bool prepare);
 
 int sdw_stream_add_master(struct sdw_bus *bus,
 			  struct sdw_stream_config *stream_config,
-- 
2.43.0

[PATCH v2 3/5] soundwire: Select the correct runtime for lane allocation

[Bard Liao]

When multiple master runtimes exist on the same bus,
sdw_compute_bus_params() selects the first runtime for which an
additional lane is available. This can select an already running stream
instead of the stream being prepared.

Changing that runtime's port lanes accounts for its bandwidth again.
If preparing the new stream fails, restoring the bus parameters leaves
the active stream's lanes inconsistent with its bandwidth reservation.
Depreparing the active stream can then underflow the lane bandwidth and
leave lane 0 bandwidth reserved.

Pass the current stream and prepare context to sdw_compute_bus_params()
and restrict lane allocation to that stream's runtime during prepare.
Skip lane allocation during deprepare, when its bandwidth reservation
has already been released. Keep clock recomputation and the existing
group and port filtering unchanged.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 .../soundwire/generic_bandwidth_allocation.c  | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index 1250fe21a3ff..e8e68dbe180b 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -536,8 +536,11 @@ static int get_manager_lane(struct sdw_bus *bus, struct sdw_master_runtime *m_rt
  * sdw_compute_bus_params: Compute bus parameters
  *
  * @bus: SDW Bus instance
+ * @stream: SoundWire stream being prepared or deprepared
+ * @prepare: True when preparing, false when depreparing
  */
-static int sdw_compute_bus_params(struct sdw_bus *bus)
+static int sdw_compute_bus_params(struct sdw_bus *bus,
+				  struct sdw_stream_runtime *stream, bool prepare)
 {
 	struct sdw_master_prop *mstr_prop = &bus->prop;
 	struct sdw_slave_prop *slave_prop;
@@ -580,6 +583,14 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
 			break;
 
 		list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
+			/*
+			 * Select only the current stream's runtime. Check prepare
+			 * to avoid computing m_lane during deprepare, when the
+			 * stream's bandwidth reservation has already been released.
+			 */
+			if (m_rt->stream != stream || !prepare)
+				continue;
+
 			/*
 			 * Get the first s_rt that will be used to find the available lane that
 			 * can be used. No need to check all Peripherals because we can't use
@@ -627,8 +638,8 @@ static int sdw_compute_bus_params(struct sdw_bus *bus)
 			}
 		}
 		/*
-		 * Set Manager lanes. Configure the last m_rt in bus->m_rt_list only since
-		 * we don't want to touch other m_rts that are already working.
+		 * Set Manager lanes for the stream being prepared without changing
+		 * other runtimes that are already working.
 		 */
 		list_for_each_entry(m_p_rt, &m_rt->port_list, port_node) {
 			m_p_rt->lane = m_lane;
@@ -665,7 +676,7 @@ int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
 	int ret;
 
 	/* Computes clock frequency, frame shape and frame frequency */
-	ret = sdw_compute_bus_params(bus);
+	ret = sdw_compute_bus_params(bus, stream, prepare);
 	if (ret < 0)
 		return ret;
 
-- 
2.43.0

Re: [PATCH v2 3/5] soundwire: Select the correct runtime for lane allocation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] soundwire: missing Fixes trailer for sdw_compute_bus_params()
  lane fix
--

commit b1efaa73284b1d94c7492050c9026885c088a497
Author: Bard Liao <yung-chuan.liao@linux.intel.com>

> soundwire: Select the correct runtime for lane allocation
>
> When multiple master runtimes exist on the same bus,
> sdw_compute_bus_params() selects the first runtime for which an
> additional lane is available. This can select an already running stream
> instead of the stream being prepared.
>
> Changing that runtime's port lanes accounts for its bandwidth again.
> If preparing the new stream fails, restoring the bus parameters leaves
> the active stream's lanes inconsistent with its bandwidth reservation.
> Depreparing the active stream can then underflow the lane bandwidth and
> leave lane 0 bandwidth reserved.
>
> Pass the current stream and prepare context to sdw_compute_bus_params()
> and restrict lane allocation to that stream's runtime during prepare.
> Skip lane allocation during deprepare, when its bandwidth reservation
> has already been released. Keep clock recomputation and the existing
> group and port filtering unchanged.
>
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>

[Severity: Low]
This fixes the unqualified bus->m_rt_list loop in sdw_compute_bus_params().
Should this include a Fixes tag:

  Fixes: 7a30292ffa2a ("soundwire: generic_bandwidth_allocation: select data lane")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112072105.1296515-1-yung-chuan.liao@linux.intel.com?part=3

[PATCH v2 4/5] soundwire: Include the current stream when preparing again

[Bard Liao]

sdw_prepare_stream() accepts a stream in the DEPREPARED state without
changing its state before parameter computation. However, the generic
allocator excludes DEPREPARED streams from rate groups and port
computation, and only includes a CONFIGURED current stream in group
payload calculations.

Consequently, preparing an existing stream again can reserve bandwidth
without computing the corresponding transport parameters. The filters
need to distinguish preparing that stream again from depreparing it.

Pass the prepare context through the group and port helpers. Include
the current stream during prepare even when it is still DEPREPARED,
while preserving the filtering of all other runtimes and the behavior
during deprepare. This supports preparing again without temporarily
changing stream state.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 .../soundwire/generic_bandwidth_allocation.c  | 36 ++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index e8e68dbe180b..9bacde2c67a1 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -188,7 +188,8 @@ static void sdw_compute_master_ports(struct sdw_master_runtime *m_rt,
 }
 
 static void _sdw_compute_port_params(struct sdw_bus *bus,
-				     struct sdw_group_params *params, int count)
+				     struct sdw_group_params *params, int count,
+				     struct sdw_stream_runtime *stream, bool prepare)
 {
 	struct sdw_master_runtime *m_rt;
 	int port_bo, i, l;
@@ -209,9 +210,12 @@ static void _sdw_compute_port_params(struct sdw_bus *bus,
 				/*
 				 * Only runtimes with CONFIGURED, PREPARED, ENABLED, and DISABLED
 				 * states should be included in the bandwidth calculation.
+				 * Include the current stream during prepare even if it is
+				 * still in the DEPREPARED state.
 				 */
-				if (m_rt->stream->state > SDW_STREAM_DISABLED ||
-				    m_rt->stream->state < SDW_STREAM_CONFIGURED)
+				if (!(prepare && m_rt->stream == stream) &&
+				    (m_rt->stream->state > SDW_STREAM_DISABLED ||
+				     m_rt->stream->state < SDW_STREAM_CONFIGURED))
 					continue;
 				sdw_compute_master_ports(m_rt, &params[i], &port_bo, hstop);
 			}
@@ -224,7 +228,7 @@ static void _sdw_compute_port_params(struct sdw_bus *bus,
 static int sdw_compute_group_params(struct sdw_bus *bus,
 				    struct sdw_stream_runtime *stream,
 				    struct sdw_group_params *params,
-				    struct sdw_group *group)
+				    struct sdw_group *group, bool prepare)
 {
 	struct sdw_master_runtime *m_rt;
 	struct sdw_port_runtime *p_rt;
@@ -242,7 +246,7 @@ static int sdw_compute_group_params(struct sdw_bus *bus,
 	list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
 		if (m_rt->stream == stream) {
 			/* Only runtime during prepare should be added */
-			if (stream->state != SDW_STREAM_CONFIGURED)
+			if (!prepare)
 				continue;
 		} else {
 			/*
@@ -331,7 +335,8 @@ static int sdw_add_element_group_count(struct sdw_group *group,
 }
 
 static int sdw_get_group_count(struct sdw_bus *bus,
-			       struct sdw_group *group)
+			       struct sdw_group *group,
+			       struct sdw_stream_runtime *stream, bool prepare)
 {
 	struct sdw_master_runtime *m_rt;
 	struct sdw_port_runtime *p_rt;
@@ -352,7 +357,12 @@ static int sdw_get_group_count(struct sdw_bus *bus,
 	}
 
 	list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
-		if (m_rt->stream->state == SDW_STREAM_DEPREPARED)
+		/*
+		 * Include the current stream during prepare even if it is still
+		 * in the DEPREPARED state.
+		 */
+		if (!(prepare && m_rt->stream == stream) &&
+		    m_rt->stream->state == SDW_STREAM_DEPREPARED)
 			continue;
 
 		rate = m_rt->stream->params.rate;
@@ -383,14 +393,16 @@ static int sdw_get_group_count(struct sdw_bus *bus,
  *
  * @bus: SDW Bus instance
  * @stream: Soundwire stream
+ * @prepare: True when preparing, false when depreparing
  */
-static int sdw_compute_port_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
+static int sdw_compute_port_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
+				   bool prepare)
 {
 	struct sdw_group_params *params = NULL;
 	struct sdw_group group;
 	int ret;
 
-	ret = sdw_get_group_count(bus, &group);
+	ret = sdw_get_group_count(bus, &group, stream, prepare);
 	if (ret < 0)
 		return ret;
 
@@ -404,11 +416,11 @@ static int sdw_compute_port_params(struct sdw_bus *bus, struct sdw_stream_runtim
 	}
 
 	/* Compute transport parameters for grouped streams */
-	ret = sdw_compute_group_params(bus, stream, params, &group);
+	ret = sdw_compute_group_params(bus, stream, params, &group, prepare);
 	if (ret < 0)
 		goto free_params;
 
-	_sdw_compute_port_params(bus, params, group.count);
+	_sdw_compute_port_params(bus, params, group.count, stream, prepare);
 
 free_params:
 	kfree(params);
@@ -686,7 +698,7 @@ int sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream,
 	}
 
 	/* Compute transport and port params */
-	ret = sdw_compute_port_params(bus, stream);
+	ret = sdw_compute_port_params(bus, stream, prepare);
 	if (ret < 0) {
 		dev_err(bus->dev, "Compute transport params failed: %d\n", ret);
 		return ret;
-- 
2.43.0

[PATCH v2 5/5] soundwire: stream: Fix bandwidth leak in multi-link aggregation failure

[Bard Liao]

From: David Lin <david.lin@intel.com>

When preparing a multi-link aggregation stream, a failure on a subsequent
link during _sdw_prepare_stream() causes the stream preparation to fail.
However, the bandwidth allocated on the preceding successful links is never
released, causing a cumulative bandwidth leak.

Fix this by saving a snapshot of each bus's parameters before making any
modifications. In case of failure, we iterate through the stream's master
list and restore each bus to its original parameter state, ensuring an
atomic rollback and preventing any bandwidth leaks.

The rollback only restores bus->params, while the lane assignments of the
port runtimes are left untouched. Since _sdw_deprepare_stream() resets
p_rt->lane only when the lane's used bandwidth drops to zero, a stream can
also carry a stale lane from a previous prepare or from a failed prepare
that was rolled back. Reset the lane of the stream being prepared to 0 in
sdw_compute_bus_params() when no multi-lane is selected, so that the lane
assignment matches the restored lane_used_bandwidth.

Fixes: 7a30292ffa2a ("soundwire: generic_bandwidth_allocation: select data lane")
Cc: stable@vger.kernel.org
Signed-off-by: David Lin <david.lin@intel.com>
Co-developed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/bus.h                         |  3 +++
 .../soundwire/generic_bandwidth_allocation.c    | 14 ++++++++++++++
 drivers/soundwire/stream.c                      | 17 +++++++++++++----
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 44e4f5193917..476387b70472 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -155,6 +155,8 @@ struct sdw_slave_runtime {
  *
  * @bus: Bus handle
  * @stream: Stream runtime handle
+ * @params_backup: Snapshot of original bus parameters saved before stream
+ * preparation, used for rollback recovery on failure.
  * @direction: Data direction for Master
  * @ch_count: Number of channels handled by the Master for
  * this stream, can be zero.
@@ -166,6 +168,7 @@ struct sdw_slave_runtime {
 struct sdw_master_runtime {
 	struct sdw_bus *bus;
 	struct sdw_stream_runtime *stream;
+	struct sdw_bus_params params_backup;
 	enum sdw_data_direction direction;
 	unsigned int ch_count;
 	struct list_head slave_rt_list;
diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index 9bacde2c67a1..214a1be6c33e 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -656,6 +656,20 @@ static int sdw_compute_bus_params(struct sdw_bus *bus,
 		list_for_each_entry(m_p_rt, &m_rt->port_list, port_node) {
 			m_p_rt->lane = m_lane;
 		}
+	} else if (prepare) {
+		/* Clear stale lane assignments when preparing on lane 0. */
+		list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
+			if (m_rt->stream != stream)
+				continue;
+
+			list_for_each_entry(m_p_rt, &m_rt->port_list, port_node)
+				m_p_rt->lane = 0;
+
+			list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node)
+				list_for_each_entry(s_p_rt, &s_rt->port_list, port_node)
+					s_p_rt->lane = 0;
+			break;
+		}
 	}
 
 	if (!mstr_prop->default_frame_rate || !mstr_prop->default_row)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 06f051f03a21..7c748ba84767 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1479,19 +1479,22 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
 	struct sdw_master_runtime *m_rt;
 	struct sdw_bus *bus;
 	struct sdw_master_prop *prop;
-	struct sdw_bus_params params;
 	int ret;
 
+	/* Pre-pass: snapshot original bus parameters for all masters */
+	list_for_each_entry(m_rt, &stream->master_list, stream_node)
+		m_rt->params_backup = m_rt->bus->params;
+
 	/* Prepare  Master(s) and Slave(s) port(s) associated with stream */
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
 		bus = m_rt->bus;
 		prop = &bus->prop;
-		memcpy(&params, &bus->params, sizeof(params));
 
 		/* TODO: Support Asynchronous mode */
 		if ((prop->max_clk_freq % stream->params.rate) != 0) {
 			dev_err(bus->dev, "Async mode not supported\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto restore_params;
 		}
 
 		if (update_params) {
@@ -1542,7 +1545,13 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
 	return ret;
 
 restore_params:
-	memcpy(&bus->params, &params, sizeof(params));
+	/*
+	 * Revert all masters to their snapshotted original parameters.
+	 * Any master that was not modified will safely revert to its own original state.
+	 */
+	list_for_each_entry(m_rt, &stream->master_list, stream_node)
+		m_rt->bus->params = m_rt->params_backup;
+
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH v2 5/5] soundwire: stream: Fix bandwidth leak in multi-link aggregation failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] soundwire: incomplete port lane and transport rollback in
  _sdw_prepare_stream()
- [High] soundwire: stream: bank state desync in _sdw_prepare_stream()
  rollback
--

commit f77415deadf5285d82da231d58b3652a2ff280ce
Author: David Lin <david.lin@intel.com>

soundwire: stream: Fix bandwidth leak in multi-link aggregation failure

This commit snapshots bus->params into m_rt->params_backup for each master
before stream preparation and restores them on failure, while clearing
port lane assignments in sdw_compute_bus_params() when preparing on lane 0.

> diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
> index 9bacde2c67a11..214a1be6c33e0 100644
> --- a/drivers/soundwire/generic_bandwidth_allocation.c
> +++ b/drivers/soundwire/generic_bandwidth_allocation.c
> @@ -656,6 +656,20 @@ static int sdw_compute_bus_params(struct sdw_bus *bus,
>  		list_for_each_entry(m_p_rt, &m_rt->port_list, port_node) {
>  			m_p_rt->lane = m_lane;
>  		}
> +	} else if (prepare) {
> +		/* Clear stale lane assignments when preparing on lane 0. */
> +		list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
> +			if (m_rt->stream != stream)
> +				continue;
> +
> +			list_for_each_entry(m_p_rt, &m_rt->port_list, port_node)
> +				m_p_rt->lane = 0;
> +
> +			list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node)
> +				list_for_each_entry(s_p_rt, &s_rt->port_list, port_node)
> +					s_p_rt->lane = 0;
> +			break;
> +		}
>  	}

[Severity: High]
Does clearing m_p_rt->lane and s_p_rt->lane only for the stream currently
being prepared leave stale non-zero lanes on a rolled-back stream when
another stream on the same bus is later prepared or deprepared?

When _sdw_prepare_stream() fails on a later link after bus->compute_params()
has selected m_lane > 0 and recomputed port transport parameters on an
earlier link, restore_params reverts bus->params (including
lane_used_bandwidth[] and curr_dr_freq) while the failed stream remains in
SDW_STREAM_CONFIGURED with its non-zero lane assignments.  If a second
stream on that bus is then prepared or deprepared before the failed stream
is re-prepared on lane 0, sdw_compute_group_params() excludes the
SDW_STREAM_CONFIGURED stream from payload_bw and hwidth, but
_sdw_compute_port_params() still includes it:

drivers/soundwire/generic_bandwidth_allocation.c:_sdw_compute_port_params() {
    ...
				if (!(prepare && m_rt->stream == stream) &&
				    (m_rt->stream->state > SDW_STREAM_DISABLED ||
				     m_rt->stream->state < SDW_STREAM_CONFIGURED))
					continue;
				sdw_compute_master_ports(m_rt, &params[i], &port_bo, hstop);
    ...
}

When an active stream uses that same non-zero lane and sample rate,
sdw_compute_master_ports() matches the failed stream's stale p_rt->lane and
advances port_bo without allocated hwidth, shifting active streams' transport
block offsets out of bounds.

In addition, bus->compute_params() on the preceding link has already updated
mstr_prop->default_col and recomputed p_rt->transport_params and
p_rt->port_params across existing streams on that bus; if an already
PREPARED or DISABLED stream on that link is later enabled or disabled,
sdw_program_params(bus, false) programs those un-reverted transport
parameters into hardware without recomputing them for the restored
bus->params.

> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 06f051f03a218..7c748ba847672 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -1479,19 +1479,22 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
>  	struct sdw_master_runtime *m_rt;
>  	struct sdw_bus *bus;
>  	struct sdw_master_prop *prop;
> -	struct sdw_bus_params params;
>  	int ret;
>  
> +	/* Pre-pass: snapshot original bus parameters for all masters */
> +	list_for_each_entry(m_rt, &stream->master_list, stream_node)
> +		m_rt->params_backup = m_rt->bus->params;
> +
>  	/* Prepare  Master(s) and Slave(s) port(s) associated with stream */
>  	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
>  		bus = m_rt->bus;
>  		prop = &bus->prop;
> -		memcpy(&params, &bus->params, sizeof(params));
>  
>  		/* TODO: Support Asynchronous mode */
>  		if ((prop->max_clk_freq % stream->params.rate) != 0) {
>  			dev_err(bus->dev, "Async mode not supported\n");
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto restore_params;
>  		}
>  
>  		if (update_params) {
> @@ -1542,7 +1545,13 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
>  	return ret;
>  
>  restore_params:
> -	memcpy(&bus->params, &params, sizeof(params));
> +	/*
> +	 * Revert all masters to their snapshotted original parameters.
> +	 * Any master that was not modified will safely revert to its own original state.
> +	 */
> +	list_for_each_entry(m_rt, &stream->master_list, stream_node)
> +		m_rt->bus->params = m_rt->params_backup;
> +
>  	return ret;
>  }

[Severity: High]
Can restoring m_rt->params_backup across all masters desynchronize
bus->params.curr_bank and bus->params.next_bank from hardware when
_sdw_prepare_stream() fails during or after do_bank_switch()?

m_rt->params_backup snapshots curr_bank and next_bank before
do_bank_switch() switches the active register bank in hardware and inverts
those fields.  If sdw_prep_deprep_ports(m_rt, true) subsequently fails on
any master (or if do_bank_switch() fails after an earlier master in
stream->master_list has already switched banks), jumping to restore_params
overwrites curr_bank and next_bank on every master with their pre-switch
values and leaves ports prepared on earlier masters in the loop without
depreparing them.  Later stream operations on those buses will then program
banked registers into the currently active hardware bank and attempt to
switch to the bank already in use.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112072105.1296515-1-yung-chuan.liao@linux.intel.com?part=5

RE: [PATCH v2 5/5] soundwire: stream: Fix bandwidth leak in multi-link aggregation failure

[Liao, Bard]

> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Thursday, November 12, 2026 3:37 PM
> To: Bard Liao <yung-chuan.liao@linux.intel.com>; linux-sound@vger.kernel.org
> Subject: Re: [PATCH v2 5/5] soundwire: stream: Fix bandwidth leak in multi-link
> aggregation failure
> 
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [High] soundwire: incomplete port lane and transport rollback in
>   _sdw_prepare_stream()
> - [High] soundwire: stream: bank state desync in _sdw_prepare_stream()
>   rollback
> --
> 
> commit f77415deadf5285d82da231d58b3652a2ff280ce
> Author: David Lin <david.lin@intel.com>
> 
> soundwire: stream: Fix bandwidth leak in multi-link aggregation failure
> 
> This commit snapshots bus->params into m_rt->params_backup for each master
> before stream preparation and restores them on failure, while clearing
> port lane assignments in sdw_compute_bus_params() when preparing on lane 0.
> 
> > diff --git a/drivers/soundwire/generic_bandwidth_allocation.c
> b/drivers/soundwire/generic_bandwidth_allocation.c
> > index 9bacde2c67a11..214a1be6c33e0 100644
> > --- a/drivers/soundwire/generic_bandwidth_allocation.c
> > +++ b/drivers/soundwire/generic_bandwidth_allocation.c
> > @@ -656,6 +656,20 @@ static int sdw_compute_bus_params(struct sdw_bus
> *bus,
> >  		list_for_each_entry(m_p_rt, &m_rt->port_list, port_node) {
> >  			m_p_rt->lane = m_lane;
> >  		}
> > +	} else if (prepare) {
> > +		/* Clear stale lane assignments when preparing on lane 0. */
> > +		list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
> > +			if (m_rt->stream != stream)
> > +				continue;
> > +
> > +			list_for_each_entry(m_p_rt, &m_rt->port_list,
> port_node)
> > +				m_p_rt->lane = 0;
> > +
> > +			list_for_each_entry(s_rt, &m_rt->slave_rt_list,
> m_rt_node)
> > +				list_for_each_entry(s_p_rt, &s_rt->port_list,
> port_node)
> > +					s_p_rt->lane = 0;
> > +			break;
> > +		}
> >  	}
> 
> [Severity: High]
> Does clearing m_p_rt->lane and s_p_rt->lane only for the stream currently
> being prepared leave stale non-zero lanes on a rolled-back stream when
> another stream on the same bus is later prepared or deprepared?
> 
> When _sdw_prepare_stream() fails on a later link after bus->compute_params()
> has selected m_lane > 0 and recomputed port transport parameters on an
> earlier link, restore_params reverts bus->params (including
> lane_used_bandwidth[] and curr_dr_freq) while the failed stream remains in
> SDW_STREAM_CONFIGURED with its non-zero lane assignments.  If a second
> stream on that bus is then prepared or deprepared before the failed stream
> is re-prepared on lane 0, sdw_compute_group_params() excludes the
> SDW_STREAM_CONFIGURED stream from payload_bw and hwidth, but
> _sdw_compute_port_params() still includes it:
> 
> drivers/soundwire/generic_bandwidth_allocation.c:_sdw_compute_port_params
> () {
>     ...
> 				if (!(prepare && m_rt->stream == stream) &&
> 				    (m_rt->stream->state >
> SDW_STREAM_DISABLED ||
> 				     m_rt->stream->state <
> SDW_STREAM_CONFIGURED))
> 					continue;
> 				sdw_compute_master_ports(m_rt, &params[i],
> &port_bo, hstop);
>     ...
> }
> 
> When an active stream uses that same non-zero lane and sample rate,
> sdw_compute_master_ports() matches the failed stream's stale p_rt->lane and
> advances port_bo without allocated hwidth, shifting active streams' transport
> block offsets out of bounds.

I will fix this in v3. The lanes of the stream will be reset to 0 in the
rollback path, so they match the restored lane_used_bandwidth and are no
longer picked up by the port computation of other streams on the same bus.
The reset will be skipped when update_params is false, since the lanes are
still valid in that case.

> 
> In addition, bus->compute_params() on the preceding link has already updated
> mstr_prop->default_col and recomputed p_rt->transport_params and
> p_rt->port_params across existing streams on that bus; if an already
> PREPARED or DISABLED stream on that link is later enabled or disabled,
> sdw_program_params(bus, false) programs those un-reverted transport
> parameters into hardware without recomputing them for the restored
> bus->params.

This is an existing issue and not introduced by this patch.

> 
> > diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> > index 06f051f03a218..7c748ba847672 100644
> > --- a/drivers/soundwire/stream.c
> > +++ b/drivers/soundwire/stream.c
> > @@ -1479,19 +1479,22 @@ static int _sdw_prepare_stream(struct
> sdw_stream_runtime *stream,
> >  	struct sdw_master_runtime *m_rt;
> >  	struct sdw_bus *bus;
> >  	struct sdw_master_prop *prop;
> > -	struct sdw_bus_params params;
> >  	int ret;
> >
> > +	/* Pre-pass: snapshot original bus parameters for all masters */
> > +	list_for_each_entry(m_rt, &stream->master_list, stream_node)
> > +		m_rt->params_backup = m_rt->bus->params;
> > +
> >  	/* Prepare  Master(s) and Slave(s) port(s) associated with stream */
> >  	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
> >  		bus = m_rt->bus;
> >  		prop = &bus->prop;
> > -		memcpy(&params, &bus->params, sizeof(params));
> >
> >  		/* TODO: Support Asynchronous mode */
> >  		if ((prop->max_clk_freq % stream->params.rate) != 0) {
> >  			dev_err(bus->dev, "Async mode not supported\n");
> > -			return -EINVAL;
> > +			ret = -EINVAL;
> > +			goto restore_params;
> >  		}
> >
> >  		if (update_params) {
> > @@ -1542,7 +1545,13 @@ static int _sdw_prepare_stream(struct
> sdw_stream_runtime *stream,
> >  	return ret;
> >
> >  restore_params:
> > -	memcpy(&bus->params, &params, sizeof(params));
> > +	/*
> > +	 * Revert all masters to their snapshotted original parameters.
> > +	 * Any master that was not modified will safely revert to its own original
> state.
> > +	 */
> > +	list_for_each_entry(m_rt, &stream->master_list, stream_node)
> > +		m_rt->bus->params = m_rt->params_backup;
> > +
> >  	return ret;
> >  }
> 
> [Severity: High]
> Can restoring m_rt->params_backup across all masters desynchronize
> bus->params.curr_bank and bus->params.next_bank from hardware when
> _sdw_prepare_stream() fails during or after do_bank_switch()?

I will preserve curr_bank and next_bank when restoring the snapshot in v3,
so they stay consistent with the hardware if the failure happens after the
bank switch.

> 
> m_rt->params_backup snapshots curr_bank and next_bank before
> do_bank_switch() switches the active register bank in hardware and inverts
> those fields.  If sdw_prep_deprep_ports(m_rt, true) subsequently fails on
> any master (or if do_bank_switch() fails after an earlier master in
> stream->master_list has already switched banks), jumping to restore_params
> overwrites curr_bank and next_bank on every master with their pre-switch
> values and leaves ports prepared on earlier masters in the loop without
> depreparing them.  Later stream operations on those buses will then program
> banked registers into the currently active hardware bank and attempt to
> switch to the bank already in use.

The ports that were already prepared on earlier masters are not deprepared
when sdw_prep_deprep_ports() fails. This is the existing behavior of
_sdw_prepare_stream(), which only restores bus->params in the error path,
and it is not introduced by this patch.

> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261112072105.1296515-1-
> yung-chuan.liao@linux.intel.com?part=5