Re: [PATCH] media: qcom: camss: stop started subdevices when stream-on fails

[Jizhou Tong]

Hi Loic, Qianhui,

Thanks for the pointer. I have read v2 2/2 [1]; it covers the ordinary
start-failure cleanup addressed by my patch, and sharing the traversal
with stream-off makes sense.

There is one remaining difference: camss_video_subdevs_set_stream()
returns immediately on a disable error. If A and B start successfully,
C fails to start, and stopping A then fails, B is never stopped. Normal
stream-off also stops walking at that first error.

I reproduced this with the actual helper and injected subdevice errors.
The increment below keeps the first stop error and continues cleanup;
start still returns immediately, and video_start_streaming() retains the
original start error if cleanup also fails. It applies on top of your
v2 2/2. With this covered, my earlier standalone patch can be superseded.

The helper and the actual video start/stop functions pass 416 ASan/UBSan
cases covering partial starts, multiple stop failures, ignored states
and pipeline/format errors. With the unmodified helper, the stop-error
case leaves B enabled. The ARM64 W=1 camss-video.o build passes. These are
software tests; I have not replaced the running camera driver to test
the combined change on hardware.

Would you fold this into your next revision? I have left the multiple
source-link question to the existing discussion with Bryan.

[1]
https://lore.kernel.org/all/20261031041154.308030-3-xiaqinghuan@xxxxxxxxxxxxxxxx/

Signed-off-by: Jizhou Tong <fqwqf@xxxxxxxxx>
---
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 9a633ca066..31d608d9a0 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -268,12 +268,16 @@ static int video_prepare_streaming(struct vb2_queue *q)
  * subdevice without a streaming operation reports -ENOIOCTLCMD, which is
  * ignored here to match the previous .s_stream() call sites.
  *
+ * Start stops at the first error. Stop continues through the pipeline even if
+ * a subdevice fails, so one failure does not prevent cleanup of the others.
+ *
  * Return: 0 on success, or the error code of the first subdevice that failed.
  */
 static int camss_video_subdevs_set_stream(struct camss_video *video,
 					  bool enable)
 {
 	struct media_entity *entity = &video->vdev.entity;
+	int first_error = 0;
 
 	while (1) {
 		struct media_pad *pad;
@@ -296,13 +300,17 @@ static int camss_video_subdevs_set_stream(struct camss_video *video,
 			ret = v4l2_subdev_disable_streams(subdev, pad->index,
 							  BIT_ULL(0));
 
-		if (ret && ret != -EALREADY && ret != -ENOIOCTLCMD)
-			return ret;
+		if (ret && ret != -EALREADY && ret != -ENOIOCTLCMD) {
+			if (enable)
+				return ret;
+			if (!first_error)
+				first_error = ret;
+		}
 
 		entity = pad->entity;
 	}
 
-	return 0;
+	return first_error;
 }
 
 static int video_start_streaming(struct vb2_queue *q, unsigned int count)