[PATCH 0/2] soundwire: roll back failed master registration

[PATCH 0/2] soundwire: roll back failed master registration

[Baek Kyu, Kim]

A failed master registration can leave partial port allocations in the
SoundWire core. The Qualcomm driver also ignores registration errors and
can retain reservations from a failed attempt, so hw_params may report
success with incomplete master registration.

Patch 1 makes core master port allocation atomic and restores the state
changed by failed configuration. Patch 2 depends on patch 1 and restores
the Qualcomm port bitmap before propagating the registration error.
Please apply the patches in this order.

Tested with ARM64 QEMU KUnit through real SoundWire registration APIs,
Qualcomm hw_params and the ASoC wrapper, using synthetic devices and
task-scoped allocation failure injection. The baseline and a return-only
alternative each pass 2 of 8 tests; the complete series passes all 8.
KASAN and lockdep were enabled. Product-only ARM64 LLVM W=1 SoundWire
object builds and strict checkpatch also pass. No physical Qualcomm
SoundWire hardware or audio playback/capture was tested.

Baek Kyu, Kim (2):
  soundwire: stream: roll back failed master registration
  soundwire: qcom: restore port reservations on registration failure

drivers/soundwire/qcom.c   | 26 ++++++++++++++------------
 drivers/soundwire/stream.c | 35 +++++++++++++++++++++++++++++------
 2 files changed, 43 insertions(+), 18 deletions(-)

base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da

[PATCH 1/2] soundwire: stream: roll back failed master registration

[Baek Kyu, Kim]

When slaves have already created a master runtime, a failed master port
allocation leaves earlier allocations on its port list. If the second of
two port allocations fails, a retry sees a nonempty list and succeeds with
only one port and without incrementing m_rt_count.

Allocate master ports on a temporary list and publish them only after all
allocations succeed. On configuration failure, release ports allocated by
this call, balance m_rt_count, and restore the previous channel count and
direction. Preserve an existing slave-first runtime and its slaves, while
freeing a runtime created by the failed call. This lets a retry register
the complete set of ports instead of reusing a partial registration.

Fixes: ac3bc88cc56e ("soundwire: stream: separate alloc and config within sdw_stream_add_xxx()")
Fixes: f3016b891c8c ("soundwire: stream: sdw_stream_add_ functions can be called multiple times")
Signed-off-by: Baek Kyu, Kim <pocj8ur4in@gmail.com>
---
Tested with ARM64 QEMU KUnit using the real SoundWire registration APIs
and synthetic slave capabilities. Register a slave with two ports, fail
the second master port allocation, then disable fault injection and retry.
Before the fix the retry succeeds with one port and m_rt_count == 0;
afterwards it succeeds with two ports and m_rt_count == 1. Configuration
failure and existing/new runtime cases were also checked with KASAN and
lockdep enabled. No physical SoundWire hardware was tested.

 drivers/soundwire/stream.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index f106a877d7b2..8e94dcaf3784 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1107,17 +1107,24 @@ static void sdw_master_port_free(struct sdw_master_runtime *m_rt)
 static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
 				 unsigned int num_ports)
 {
-	struct sdw_port_runtime *p_rt;
+	struct sdw_port_runtime *p_rt, *tmp;
+	LIST_HEAD(port_list);
 	int i;
 
-	/* Iterate for number of ports to perform initialization */
+	/* Publish the ports only after the entire allocation succeeds. */
 	for (i = 0; i < num_ports; i++) {
-		p_rt = sdw_port_alloc(&m_rt->port_list);
+		p_rt = sdw_port_alloc(&port_list);
 		if (!p_rt)
-			return -ENOMEM;
+			goto free_ports;
 	}
 
+	list_splice_tail_init(&port_list, &m_rt->port_list);
 	return 0;
+
+free_ports:
+	list_for_each_entry_safe(p_rt, tmp, &port_list, port_node)
+		sdw_port_free(p_rt);
+	return -ENOMEM;
 }
 
 static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
@@ -2017,6 +2024,9 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 {
 	struct sdw_master_runtime *m_rt;
 	bool alloc_master_rt = false;
+	bool alloc_master_ports = false;
+	unsigned int old_ch_count;
+	enum sdw_data_direction old_direction;
 	int ret;
 
 	mutex_lock(&bus->bus_lock);
@@ -2056,26 +2066,39 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 		alloc_master_rt = true;
 	}
 
+	old_ch_count = m_rt->ch_count;
+	old_direction = m_rt->direction;
+
 	if (!sdw_master_port_allocated(m_rt)) {
 		ret = sdw_master_port_alloc(m_rt, num_ports);
 		if (ret)
 			goto alloc_error;
 
 		stream->m_rt_count++;
+		alloc_master_ports = true;
 	}
 
 	ret = sdw_master_rt_config(m_rt, stream_config);
 	if (ret < 0)
-		goto unlock;
+		goto config_error;
 
 	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
 	if (ret)
-		goto unlock;
+		goto config_error;
 
 	ret = sdw_master_port_config(m_rt, port_config);
+	if (ret)
+		goto config_error;
 
 	goto unlock;
 
+config_error:
+	m_rt->ch_count = old_ch_count;
+	m_rt->direction = old_direction;
+	if (alloc_master_ports) {
+		sdw_master_port_free(m_rt);
+		stream->m_rt_count--;
+	}
 alloc_error:
 	/*
 	 * we only cleanup what was allocated in this routine

[PATCH 2/2] soundwire: qcom: restore port reservations on registration failure

[Baek Kyu, Kim]

qcom_swrm_stream_alloc_ports() ignores sdw_stream_add_master() errors and
returns success after master registration fails. Reservations made before
registration, or before discovering that all ports are busy, also remain
set in the controller bitmap.

For example, failing the second of two master port allocations returns
-ENOMEM from the core, but hw_params reports success and retains the new
port reservations. The stream is marked as configured despite incomplete
master registration.

Save the bitmap under port_lock and restore it on either failure, then
return the original error. This preserves earlier reservations and allows
a failed stream to retry. Remove the hw_params error cleanup that walks
master runtime ports: that list does not reliably describe reservations
made by the failed attempt. The allocation helper now restores its own
reservations before returning.

This patch depends on patch 1/2, which rolls back partial master port
allocation and failed stream configuration in the SoundWire core.

Fixes: 02efb49aa805 ("soundwire: qcom: add support for SoundWire controller")
Fixes: 5c68b66d4d7e ("soundwire: qcom: drop unneeded qcom_swrm_stream_alloc_ports() cleanup")
Signed-off-by: Baek Kyu, Kim <pocj8ur4in@gmail.com>
---
Tested with ARM64 QEMU KUnit using synthetic devices. Failing the second
master port allocation made hw_params return success and retain
reservations before the fix. With both patches, it returns -ENOMEM,
restores the original bitmap, and allows a successful retry. No physical
Qualcomm audio hardware was tested.

 drivers/soundwire/qcom.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 55678a30cd4a..586bdd17171a 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1241,7 +1241,8 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 	struct sdw_port_runtime *p_rt;
 	struct sdw_slave *slave;
 	unsigned long *port_mask;
-	int maxport, pn, nports = 0;
+	int maxport, pn, nports = 0, ret;
+	unsigned long saved_port_mask;
 	unsigned int m_port;
 	struct sdw_port_config *pconfig __free(kfree) = kzalloc_objs(*pconfig,
 								     ctrl->nports);
@@ -1260,6 +1261,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 	sconfig.bps = 1;
 
 	guard(mutex)(&ctrl->port_lock);
+	saved_port_mask = ctrl->port_mask;
 
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
 		/*
@@ -1286,7 +1288,8 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 
 				if (pn >= maxport) {
 					dev_err(ctrl->dev, "All ports busy\n");
-					return -EBUSY;
+					ret = -EBUSY;
+					goto restore_mask;
 				}
 				set_bit(pn, port_mask);
 				pconfig[nports].num = pn;
@@ -1296,10 +1299,14 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 		}
 	}
 
-	sdw_stream_add_master(&ctrl->bus, &sconfig, pconfig,
-			      nports, stream);
+	ret = sdw_stream_add_master(&ctrl->bus, &sconfig, pconfig,
+				    nports, stream);
+	if (!ret)
+		return 0;
 
-	return 0;
+restore_mask:
+	ctrl->port_mask = saved_port_mask;
+	return ret;
 }
 
 static int qcom_swrm_hw_params(struct snd_pcm_substream *substream,
@@ -1308,14 +1315,9 @@ static int qcom_swrm_hw_params(struct snd_pcm_substream *substream,
 {
 	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev);
 	struct sdw_stream_runtime *sruntime = ctrl->sruntime[dai->id];
-	int ret;
 
-	ret = qcom_swrm_stream_alloc_ports(ctrl, sruntime, params,
-					   substream->stream);
-	if (ret)
-		qcom_swrm_stream_free_ports(ctrl, sruntime);
-
-	return ret;
+	return qcom_swrm_stream_alloc_ports(ctrl, sruntime, params,
+					    substream->stream);
 }
 
 static int qcom_swrm_hw_free(struct snd_pcm_substream *substream,