[PATCH v2 0/2] media: i2c: vd55g1: preserve stream errors and always attempt stop

[PATCH v2 0/2] media: i2c: vd55g1: preserve stream errors and always attempt stop

[fQwQf]

This series fixes two error paths in the VD55G1 driver. Stream-start
currently replaces the original error with -EINVAL. On stream-off, a
failed exposure read prevents the subsequent stop command from being
issued because the CCI error accumulator remains set.

Preserve the stream-start error and handle the exposure read separately
from the stop sequence. The first patch builds on 5931eed35cb6, which
fixed the earlier false-success return.

Changes in v2:
- Drop both Fixes tags as requested by Benjamin Mugnier.
- No code changes.

Link to v1:
https://lore.kernel.org/all/010601a0f1247437-3d2d5494-0cd2-414c-af01-a7618c8602f3-000000@ap-northeast-1.amazonses.com/

Based on Linux 7.3-rc5; also applies to current media-next
(ee2e9bf41d3e) as checked on 9 October 2026.

Validation:
- ARM64 W=1 driver-object build.
- ASan/UBSan tests of the actual functions: twelve start failure points
  with three error codes, and eight stream-off error combinations.
- Hardware use on a Surface Laptop 7 with downstream VD55G0 support;
  normal IR capture and face authentication have been working.

fQwQf (2):
  media: i2c: vd55g1: preserve the stream-start error
  media: i2c: vd55g1: stop streaming after an exposure read error

 drivers/media/i2c/vd55g1.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
2.53.0

[PATCH v3 0/2] media: i2c: vd55g1: preserve stream errors and always attempt stop

[Jizhou Tong]

This series fixes two error paths in the VD55G1 driver. Stream-start
currently replaces the original error with -EINVAL. On stream-off, a
failed exposure read prevents the subsequent stop command from being
issued because the CCI error accumulator remains set.

Preserve the stream-start error and handle the exposure read separately
from the stop sequence. The first patch builds on 5931eed35cb6, which
fixed the earlier false-success return.

Changes in v3:
- Reference the documented CCI error-accumulator behavior in the
  stream-off comment.
- No functional changes from v2.

Changes in v2:
- Drop both Fixes tags as requested by Benjamin Mugnier.
- No source changes from v1.

v2: https://lore.kernel.org/all/010601a1251f83a6-b487de82-23f8-461b-99c2-dbf401de2158-000000@ap-northeast-1.amazonses.com/

Based on Linux 7.3-rc5. Rechecked against mainline 24a19b953e2a and
media-next d7e7bca71698 on 12 October 2026; both issues remain.

Validation:
- ARM64 W=1 driver-object build.
- ASan/UBSan tests of the actual functions: twelve start failure points
  with three error codes, and eight stream-off error combinations.
- Media CI for v2 passed builds, static analysis, ABI checks, checkpatch
  and virtual-device tests.
- Hardware use on a Surface Laptop 7 with downstream VD55G0 support;
  normal IR capture and face authentication have been working.

Jizhou Tong (2):
  media: i2c: vd55g1: preserve the stream-start error
  media: i2c: vd55g1: stop streaming after an exposure read error

 drivers/media/i2c/vd55g1.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)


base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
2.53.0

[PATCH v3 1/2] media: i2c: vd55g1: preserve the stream-start error

[Jizhou Tong]

The stream-start error path replaces failures from register accesses and
sensor configuration with -EINVAL. This hides the cause of failures such
as an I2C error or a timeout.

Return the saved error after dropping the runtime PM reference. All
paths reaching this label already set ret to the error that triggered
cleanup.

Signed-off-by: Jizhou Tong <fqwqf@fqwqf.xyz>
---
Changes in v3: no code changes from v2.

Fault-injection harness:
https://gist.github.com/fQwQf/60bd12627e06b10a2fc159e545f85dae/7b21a7d3a1b676a0e4cdaba06168ede09a2949e3

 drivers/media/i2c/vd55g1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 6f458f611f63451c97c708e187a313cf7b2aab4c..78e4aebfdd3b24b867ea4d077dc1b676c4a595da 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -1178,7 +1178,7 @@ static int vd55g1_enable_streams(struct v4l2_subdev *sd,
 
 err_rpm_put:
 	pm_runtime_put(sensor->dev);
-	return -EINVAL;
+	return ret;
 }
 
 static int vd55g1_disable_streams(struct v4l2_subdev *sd,
-- 
2.53.0

Re: [PATCH v3 1/2] media: i2c: vd55g1: preserve the stream-start error

[Benjamin Mugnier]

Hi,

Thank you for your patch.

Le 13/10/2026 à 05:10, Jizhou Tong a écrit :
> The stream-start error path replaces failures from register accesses and
> sensor configuration with -EINVAL. This hides the cause of failures such
> as an I2C error or a timeout.
> 
> Return the saved error after dropping the runtime PM reference. All
> paths reaching this label already set ret to the error that triggered
> cleanup.
> 
> Signed-off-by: Jizhou Tong <fqwqf@fqwqf.xyz>

Reviewed-By: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

> ---
> Changes in v3: no code changes from v2.
> 
> Fault-injection harness:
> https://gist.github.com/fQwQf/60bd12627e06b10a2fc159e545f85dae/7b21a7d3a1b676a0e4cdaba06168ede09a2949e3
> 
>  drivers/media/i2c/vd55g1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
> index 6f458f611f63451c97c708e187a313cf7b2aab4c..78e4aebfdd3b24b867ea4d077dc1b676c4a595da 100644
> --- a/drivers/media/i2c/vd55g1.c
> +++ b/drivers/media/i2c/vd55g1.c
> @@ -1178,7 +1178,7 @@ static int vd55g1_enable_streams(struct v4l2_subdev *sd,
>  
>  err_rpm_put:
>  	pm_runtime_put(sensor->dev);
> -	return -EINVAL;
> +	return ret;
>  }
>  
>  static int vd55g1_disable_streams(struct v4l2_subdev *sd,

-- 
Regards,
Benjamin

[PATCH v3 2/2] media: i2c: vd55g1: stop streaming after an exposure read error

[Jizhou Tong]

Stream-off saves exposure settings for the next automatic-exposure cold
start. If that read fails, its error is passed to the subsequent CCI
operations, which skip accesses when the error accumulator is nonzero.
The sensor stop command is therefore never issued, although the runtime
PM reference is released.

Log the exposure snapshot failure separately and clear the accumulator
before stopping the sensor. Treat the optional snapshot failure as a
warning: a successful stop returns success, while a stop failure is
returned to the caller. Completing stream shutdown takes precedence
over saving exposure settings for the next start.

Signed-off-by: Jizhou Tong <fqwqf@fqwqf.xyz>
---
Changes in v3: reference the CCI error-accumulator API in the
stream-off comment. No functional changes from v2.

Fault-injection harness:
https://gist.github.com/fQwQf/60bd12627e06b10a2fc159e545f85dae/7b21a7d3a1b676a0e4cdaba06168ede09a2949e3

 drivers/media/i2c/vd55g1.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 78e4aebfdd3b24b867ea4d077dc1b676c4a595da..e78835f014e65bfc2b3dcc2aaee7b054fb5da390 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -1190,6 +1190,14 @@ static int vd55g1_disable_streams(struct v4l2_subdev *sd,
 
 	/* Retrieve Expo cluster to enable coldstart of AE */
 	ret = vd55g1_read_expo_cluster(sensor);
+	if (ret)
+		dev_warn(sensor->dev, "Failed to save exposure: %d\n", ret);
+
+	/*
+	 * cci_write() skips writes when its error accumulator is set
+	 * (see include/media/v4l2-cci.h). Start a separate stop sequence.
+	 */
+	ret = 0;
 
 	vd55g1_write(sensor, VD55G1_REG_STREAMING, VD55G1_STREAMING_STOP_STREAM,
 		     &ret);
-- 
2.53.0

Re: [PATCH v3 2/2] media: i2c: vd55g1: stop streaming after an exposure read error

[Benjamin Mugnier]

Hi,

Le 13/10/2026 à 05:10, Jizhou Tong a écrit :
> Stream-off saves exposure settings for the next automatic-exposure cold
> start. If that read fails, its error is passed to the subsequent CCI
> operations, which skip accesses when the error accumulator is nonzero.
> The sensor stop command is therefore never issued, although the runtime
> PM reference is released.
> 
> Log the exposure snapshot failure separately and clear the accumulator
> before stopping the sensor. Treat the optional snapshot failure as a
> warning: a successful stop returns success, while a stop failure is
> returned to the caller. Completing stream shutdown takes precedence
> over saving exposure settings for the next start.
> 
> Signed-off-by: Jizhou Tong <fqwqf@fqwqf.xyz>

Reviewed-By: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

> ---
> Changes in v3: reference the CCI error-accumulator API in the
> stream-off comment. No functional changes from v2.
> 
> Fault-injection harness:
> https://gist.github.com/fQwQf/60bd12627e06b10a2fc159e545f85dae/7b21a7d3a1b676a0e4cdaba06168ede09a2949e3
> 
>  drivers/media/i2c/vd55g1.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
> index 78e4aebfdd3b24b867ea4d077dc1b676c4a595da..e78835f014e65bfc2b3dcc2aaee7b054fb5da390 100644
> --- a/drivers/media/i2c/vd55g1.c
> +++ b/drivers/media/i2c/vd55g1.c
> @@ -1190,6 +1190,14 @@ static int vd55g1_disable_streams(struct v4l2_subdev *sd,
>  
>  	/* Retrieve Expo cluster to enable coldstart of AE */
>  	ret = vd55g1_read_expo_cluster(sensor);
> +	if (ret)
> +		dev_warn(sensor->dev, "Failed to save exposure: %d\n", ret);
> +
> +	/*
> +	 * cci_write() skips writes when its error accumulator is set
> +	 * (see include/media/v4l2-cci.h). Start a separate stop sequence.
> +	 */
> +	ret = 0;
>  
>  	vd55g1_write(sensor, VD55G1_REG_STREAMING, VD55G1_STREAMING_STOP_STREAM,
>  		     &ret);

-- 
Regards,
Benjamin