Skip to content

soundwire: Fix bandwidth leak in multi-link aggregation failure #5972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bardliao wants to merge 4 commits into from
Closed

soundwire: Fix bandwidth leak in multi-link aggregation failure #5972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f97990
soundwire: move lane bandwidth into bus params
bardliao Oct 13, 2026
915cdac
soundwire: revert bank if failed after bank switch
bardliao Oct 13, 2026
8ac1813
soundwire: restore bus params after failed stream prepare
bardliao Oct 13, 2026
a90934b
soundwire: stream: Fix bandwidth leak in multi-link aggregation failure
bardliao Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions drivers/soundwire/bus.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions drivers/soundwire/generic_bandwidth_allocation.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,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;
Expand Down Expand Up @@ -269,7 +269,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;
Expand Down Expand Up @@ -509,7 +509,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])) {
Expand All @@ -520,7 +520,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
Expand Down
47 changes: 39 additions & 8 deletions drivers/soundwire/stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1487,19 +1487,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;
int ret, ret1;

/* 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) {
Expand Down Expand Up @@ -1541,16 +1544,44 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
if (ret < 0) {
dev_err(bus->dev, "Prepare port(s) failed ret = %d\n",
ret);
goto restore_params;
goto revert_bank;
}
}

stream->state = SDW_STREAM_PREPARED;

return ret;

revert_bank:
/*
* The bank values are already switched in do_bank_switch(), so we need to switch
* curr_bank and next_bank back to reflect the actual bank.
*/
list_for_each_entry(m_rt, &stream->master_list, stream_node) {
m_rt->params_backup.curr_bank = !m_rt->params_backup.curr_bank;
m_rt->params_backup.next_bank = !m_rt->params_backup.next_bank;
}

restore_params:
memcpy(&bus->params, &params, sizeof(params));
list_for_each_entry(m_rt, &stream->master_list, stream_node) {
/*
* Revert all masters to their snapshotted original parameters.
* Any master that was not modified will safely revert to its own original state.
*/
m_rt->bus->params = m_rt->params_backup;
ret1 = sdw_program_params(m_rt->bus, true);
Comment on lines +1571 to +1572
if (ret1 < 0) {
dev_err(m_rt->bus->dev,
"Program params failed: %d after restoring params\n",
ret1);
/* No need to do bank switch and return the original error code */
return ret;
}
}
Comment on lines +1566 to +1580
ret1 = do_bank_switch(stream);
if (ret1 < 0)
pr_err("%s: do_bank_switch failed: %d after restoring params\n", __func__, ret1);
Comment on lines +1581 to +1583

return ret;
}

Expand Down Expand Up @@ -1802,8 +1833,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 */
Expand Down
4 changes: 2 additions & 2 deletions include/linux/soundwire/sdw.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
};
Expand Down Expand Up @@ -1030,7 +1032,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
* @is_present: indicates whether any peripheral is present on the bus.
*/
struct sdw_bus {
Expand Down Expand Up @@ -1067,7 +1068,6 @@ struct sdw_bus {
struct dentry *debugfs;
#endif
bool multi_link;
unsigned int lane_used_bandwidth[SDW_MAX_LANES];
bool is_present;
};

Expand Down
Loading