[PATCH AUTOSEL 6.19-6.12] media: ipu6: Always close firmware stream

[Sasha Levin <sashal@kernel.org>]

From: Sakari Ailus <sakari.ailus@linux.intel.com>

[ Upstream commit 2b08b7007e55bd1793a58478d3ecea4fd95849a5 ]

Close the firmware stream even when disabling a stream on an upstream
sub-device fails. This allows the firmware to release resources related to
a stream that is stopped in any case.

Suggested-by: Bingbu Cao <bingbu.cao@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of commit: "media: ipu6: Always close firmware stream"

### 1. Commit Message Analysis

The commit message clearly states the intent: close the firmware stream
even when disabling a stream on an upstream sub-device fails. The
rationale is that the firmware needs to release resources related to a
stream that is being stopped regardless of whether the upstream sub-
device disable succeeded or not.

Key indicators:
- "Suggested-by: Bingbu Cao" — an Intel developer familiar with the IPU6
  hardware
- "Reviewed-by" from two reviewers (Bingbu Cao and Mehdi Djait)
- "Tested-by: Mehdi Djait" on actual hardware (Dell XPS 9315)
- Reviewed and merged by Hans Verkuil (V4L2 maintainer)

### 2. Code Change Analysis

The change is very small and surgical — it modifies a single error path
in `ipu6_isys_video_set_streaming()`:

**Before:** When `v4l2_subdev_disable_streams()` fails during stream-off
(`!state`), the function returns immediately with the error,
**skipping** the call to `close_streaming_firmware(av)`.

**After:** When `v4l2_subdev_disable_streams()` fails, the error is
still logged, but execution continues to `close_streaming_firmware(av)`,
ensuring firmware resources are always released.

The diff is just 4 lines changed:
- Removes the `{ return ret; }` block after the error
- Keeps the `dev_err()` logging
- Allows execution to fall through to `close_streaming_firmware(av)`

### 3. Bug Classification: Resource Leak

This is a **resource leak fix**. When `v4l2_subdev_disable_streams()`
fails:
- The stream is being stopped regardless (the user requested stream-off)
- The firmware has allocated resources for this stream
- Without `close_streaming_firmware()`, those firmware resources are
  leaked
- The firmware may not be able to start new streams afterward due to
  leaked resources

This is a classic "error path resource leak" pattern — on failure of one
step during teardown, other cleanup steps are skipped.

### 4. Scope and Risk Assessment

- **Size:** 4 lines changed in 1 file — minimal
- **Risk:** Very low. The change makes teardown more robust. Even if
  `v4l2_subdev_disable_streams()` fails, the stream is being stopped —
  closing the firmware stream is the correct thing to do. The firmware
  needs to know the stream is done.
- **Subsystem:** Intel IPU6 camera driver (media/pci/intel/ipu6/) — used
  on many modern Intel laptops
- **Could this break something?** No — the stream is being stopped in
  any case. Not closing firmware resources is always wrong in this path.

### 5. User Impact

- **Who is affected:** Users of Intel IPU6 cameras (common in modern
  laptops like Dell XPS, many Lenovo/HP models)
- **Symptoms without fix:** After a failed stream-off, firmware
  resources are leaked, potentially preventing future camera usage until
  reboot
- **Severity:** Medium — affects camera functionality, requires reboot
  to recover

### 6. Stability Indicators

- Reviewed by two developers, tested on real hardware
- Author (Sakari Ailus) is a well-known V4L2/media subsystem maintainer
- Merged by Hans Verkuil (another senior media maintainer)

### 7. Dependency Check

The change is self-contained. It only modifies error handling logic
within an existing function. No dependencies on other patches. The IPU6
driver exists in recent stable trees (it was added in the 6.6
timeframe).

### 8. Stable Criteria Evaluation

- **Obviously correct and tested:** Yes — reviewed by 2 people, tested
  on hardware
- **Fixes a real bug:** Yes — resource leak in firmware teardown path
- **Important issue:** Yes — can leave camera non-functional until
  reboot
- **Small and contained:** Yes — 4 lines in 1 file
- **No new features:** Correct — purely a bug fix
- **Applies cleanly:** The change is minimal and should apply cleanly

### Conclusion

This is a straightforward resource leak fix in the IPU6 camera driver.
The fix ensures firmware resources are always released during stream
teardown, even when an intermediate step fails. It's small, obviously
correct, well-reviewed, tested on real hardware, and fixes a real user-
visible problem (camera becoming non-functional after a stream error).
It meets all stable kernel criteria.

**YES**

 drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
index 919b77107cef7..54d861aca0088 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
@@ -1036,11 +1036,10 @@ int ipu6_isys_video_set_streaming(struct ipu6_isys_video *av, int state,
 			sd->name, r_pad->index, stream_mask);
 		ret = v4l2_subdev_disable_streams(sd, r_pad->index,
 						  stream_mask);
-		if (ret) {
+		if (ret)
 			dev_err(dev, "stream off %s failed with %d\n", sd->name,
 				ret);
-			return ret;
-		}
+
 		close_streaming_firmware(av);
 	} else {
 		ret = start_stream_firmware(av, bl);
-- 
2.51.0