[PATCH] slimbus: preserve ports after failed disconnect

[PATCH] slimbus: preserve ports after failed disconnect

[Avi Weiss]

slim_stream_unprepare() ignores errors from slim_disconnect_port() and
frees the port array even when a disconnect transfer fails. The helper
also marks the port disconnected before issuing the transfer, losing
track of which ports still need disconnecting.

Update port state only after a successful transfer. Attempt every
disconnect, return the first error, and retain the port array on
failure so completed ports can be skipped on retry. Free any retained
port array in slim_stream_free().

Fixes: abb9c9b8b51b ("slimbus: stream: add stream support")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
---
 drivers/slimbus/stream.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/slimbus/stream.c b/drivers/slimbus/stream.c
index b4e70a7edd1cc..ab26b243802df 100644
--- a/drivers/slimbus/stream.c
+++ b/drivers/slimbus/stream.c
@@ -150,12 +150,17 @@ static int slim_disconnect_port(struct slim_stream_runtime *stream,
 	struct slim_val_inf msg = {0, 1, NULL, wbuf, NULL};
 	u8 mc = SLIM_MSG_MC_DISCONNECT_PORT;
 	DEFINE_SLIM_LDEST_TXN(txn, mc, 5, stream->dev->laddr, &msg);
+	int ret;
 
 	wbuf[0] = port->id;
+	ret = slim_do_transfer(sdev->ctrl, &txn);
+	if (ret)
+		return ret;
+
 	port->ch.state = SLIM_CH_STATE_DISCONNECTED;
 	port->state = SLIM_PORT_DISCONNECTED;
 
-	return slim_do_transfer(sdev->ctrl, &txn);
+	return 0;
 }
 
 static int slim_deactivate_remove_channel(struct slim_stream_runtime *stream,
@@ -448,13 +453,22 @@ EXPORT_SYMBOL_GPL(slim_stream_disable);
  */
 int slim_stream_unprepare(struct slim_stream_runtime *stream)
 {
-	int i;
+	int ret = 0, err, i;
 
 	if (!stream->ports || !stream->num_ports)
 		return -EINVAL;
 
-	for (i = 0; i < stream->num_ports; i++)
-		slim_disconnect_port(stream, &stream->ports[i]);
+	for (i = 0; i < stream->num_ports; i++) {
+		if (stream->ports[i].state == SLIM_PORT_DISCONNECTED)
+			continue;
+
+		err = slim_disconnect_port(stream, &stream->ports[i]);
+		if (err && !ret)
+			ret = err;
+	}
+
+	if (ret)
+		return ret;
 
 	kfree(stream->ports);
 	stream->ports = NULL;
@@ -484,6 +498,7 @@ int slim_stream_free(struct slim_stream_runtime *stream)
 	list_del(&stream->node);
 	spin_unlock(&sdev->stream_list_lock);
 
+	kfree(stream->ports);
 	kfree(stream->name);
 	kfree(stream);
 

base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
-- 
2.43.0

Re: [PATCH] slimbus: preserve ports after failed disconnect

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] slimbus: retaining ports in slim_stream_unprepare() breaks stream re-prepare
--

commit a86f555a1b6f385fbf7a79c9ae53a6b9032ee169
Author: Avi Weiss <thnkslprpt@gmail.com>

slimbus: preserve ports after failed disconnect

This commit updates slim_disconnect_port() to set port disconnection state
only after a successful transfer, and modifies slim_stream_unprepare() to
propagate errors while retaining the port array on failure so it can be
retried or freed in slim_stream_free().

> diff --git a/drivers/slimbus/stream.c b/drivers/slimbus/stream.c
> index b4e70a7edd1cc..ab26b243802df 100644
> --- a/drivers/slimbus/stream.c
> +++ b/drivers/slimbus/stream.c

[ ... ]

> @@ -448,13 +453,22 @@ EXPORT_SYMBOL_GPL(slim_stream_disable);
>   */
>  int slim_stream_unprepare(struct slim_stream_runtime *stream)
>  {
> -	int i;
> +	int ret = 0, err, i;
>  
>  	if (!stream->ports || !stream->num_ports)
>  		return -EINVAL;
>  
> -	for (i = 0; i < stream->num_ports; i++)
> -		slim_disconnect_port(stream, &stream->ports[i]);
> +	for (i = 0; i < stream->num_ports; i++) {
> +		if (stream->ports[i].state == SLIM_PORT_DISCONNECTED)
> +			continue;
> +
> +		err = slim_disconnect_port(stream, &stream->ports[i]);
> +		if (err && !ret)
> +			ret = err;
> +	}
> +
> +	if (ret)
> +		return ret;

[Severity: Medium]
Can returning early here while retaining stream->ports prevent the stream
from recovering on subsequent trigger cycles and leak stream->ports?

When a SLIM_MSG_MC_DISCONNECT_PORT transaction fails in slim_do_transfer()
(for example with -ETIMEDOUT), slim_stream_unprepare() returns early with
stream->ports still allocated, while any ports processed before the failure
have already been disconnected in hardware and marked SLIM_PORT_DISCONNECTED.

The callers of this API in sound/soc/codecs/wcd9335.c (wcd9335_trigger())
and sound/soc/codecs/wcd934x.c (wcd934x_trigger()) do not retry
slim_stream_unprepare() on failure, and slim_stream_free() has no callers
in the tree:

sound/soc/codecs/wcd9335.c:wcd9335_trigger()
	case SNDRV_PCM_TRIGGER_START:
	case SNDRV_PCM_TRIGGER_RESUME:
	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
		...
		slim_stream_prepare(dai_data->sruntime, cfg);
		slim_stream_enable(dai_data->sruntime);
		break;
	case SNDRV_PCM_TRIGGER_STOP:
	case SNDRV_PCM_TRIGGER_SUSPEND:
	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
		slim_stream_disable(dai_data->sruntime);
		slim_stream_unprepare(dai_data->sruntime);
		break;

When the stream is subsequently restarted or resumed, slim_stream_prepare()
sees the retained rt->ports pointer and rejects the call with -EINVAL:

drivers/slimbus/stream.c:slim_stream_prepare()
	if (rt->ports) {
		dev_err(&rt->dev->dev, "Stream already Prepared\n");
		return -EINVAL;
	}

As a result, slim_connect_port_channel() is skipped and the ports that were
disconnected during the partial unprepare are never reconnected. Immediately
after that, slim_stream_enable() runs and overwrites port->state on those
disconnected ports:

drivers/slimbus/stream.c:slim_stream_enable()
	for (i = 0; i < stream->num_ports; i++) {
		struct slim_port *port = &stream->ports[i];

		slim_activate_channel(stream, port);
		port->state = SLIM_PORT_CONFIGURED;
	}

Furthermore, when wcd9335_slim_set_hw_params() or
wcd934x_slim_set_hw_params() is later called, dai_data->sruntime is
overwritten by a new slim_stream_allocate() call without calling
slim_stream_free(), leaking the retained stream->ports array.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028145508.1111792-2-thnkslprpt@gmail.com?part=1