[PATCH AUTOSEL 6.17] media: nxp: imx8-isi: Fix streaming cleanup on release

[PATCH AUTOSEL 6.17] media: nxp: imx8-isi: Fix streaming cleanup on release

[Sasha Levin]

From: Richard Leitner <richard.leitner@linux.dev>

[ Upstream commit 47773031a148ad7973b809cc7723cba77eda2b42 ]

The current implementation unconditionally calls
mxc_isi_video_cleanup_streaming() in mxc_isi_video_release(). This can
lead to situations where any release call (like from a simple
"v4l2-ctl -l") may release a currently streaming queue when called on
such a device.

This is reproducible on an i.MX8MP board by streaming from an ISI
capture device using gstreamer:

	gst-launch-1.0 -v v4l2src device=/dev/videoX ! \
	    video/x-raw,format=GRAY8,width=1280,height=800,framerate=1/120 ! \
	    fakesink

While this stream is running, querying the caps of the same device
provokes the error state:

	v4l2-ctl -l -d /dev/videoX

This results in the following trace:

[  155.452152] ------------[ cut here ]------------
[  155.452163] WARNING: CPU: 0 PID: 1708 at drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c:713 mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi]
[  157.004248] Modules linked in: cfg80211 rpmsg_ctrl rpmsg_char rpmsg_tty virtio_rpmsg_bus rpmsg_ns rpmsg_core rfkill nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables mcp251x6
[  157.053499] CPU: 0 UID: 0 PID: 1708 Comm: python3 Not tainted 6.15.4-00114-g1f61ca5cad76 #1 PREEMPT
[  157.064369] Hardware name: imx8mp_board_01 (DT)
[  157.068205] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  157.075169] pc : mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi]
[  157.081195] lr : mxc_isi_pipe_irq_handler+0x38/0x1b0 [imx8_isi]
[  157.087126] sp : ffff800080003ee0
[  157.090438] x29: ffff800080003ee0 x28: ffff0000c3688000 x27: 0000000000000000
[  157.097580] x26: 0000000000000000 x25: ffff0000c1e7ac00 x24: ffff800081b5ad50
[  157.104723] x23: 00000000000000d1 x22: 0000000000000000 x21: ffff0000c25e4000
[  157.111866] x20: 0000000060000200 x19: ffff80007a0608d0 x18: 0000000000000000
[  157.119008] x17: ffff80006a4e3000 x16: ffff800080000000 x15: 0000000000000000
[  157.126146] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[  157.133287] x11: 0000000000000040 x10: ffff0000c01445f0 x9 : ffff80007a053a38
[  157.140425] x8 : ffff0000c04004b8 x7 : 0000000000000000 x6 : 0000000000000000
[  157.147567] x5 : ffff0000c0400490 x4 : ffff80006a4e3000 x3 : ffff0000c25e4000
[  157.154706] x2 : 0000000000000000 x1 : ffff8000825c0014 x0 : 0000000060000200
[  157.161850] Call trace:
[  157.164296]  mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi] (P)
[  157.170319]  __handle_irq_event_percpu+0x58/0x218
[  157.175029]  handle_irq_event+0x54/0xb8
[  157.178867]  handle_fasteoi_irq+0xac/0x248
[  157.182968]  handle_irq_desc+0x48/0x68
[  157.186723]  generic_handle_domain_irq+0x24/0x38
[  157.191346]  gic_handle_irq+0x54/0x120
[  157.195098]  call_on_irq_stack+0x24/0x30
[  157.199027]  do_interrupt_handler+0x88/0x98
[  157.203212]  el0_interrupt+0x44/0xc0
[  157.206792]  __el0_irq_handler_common+0x18/0x28
[  157.211328]  el0t_64_irq_handler+0x10/0x20
[  157.215429]  el0t_64_irq+0x198/0x1a0
[  157.219009] ---[ end trace 0000000000000000 ]---

Address this issue by moving the streaming preparation and cleanup to
the vb2 .prepare_streaming() and .unprepare_streaming() operations. This
also simplifies the driver by allowing direct usage of the
vb2_ioctl_streamon() and vb2_ioctl_streamoff() helpers, and removal of
the manual cleanup from mxc_isi_video_release().

Link: https://lore.kernel.org/r/20250813212451.22140-2-laurent.pinchart@ideasonboard.com
Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Richard Leitner <richard.leitner@linux.dev> # i.MX8MP
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- Fixes a real, user-facing bug: closing any file descriptor on the same
  node could tear down an active stream (e.g., “v4l2-ctl -l”), leading
  to WARNs in the IRQ handler. The commit moves streaming lifetime
  management into vb2 so only the streaming filehandle controls
  teardown.
- Moves pipeline start/stop and discard buffer lifetime from ad-hoc
  ioctl/release paths into vb2’s streaming lifecycle:
  - New prepare path acquires and starts the pipeline, validates format,
    and allocates discard buffers:
    - Acquire/start: drivers/media/platform/nxp/imx8-isi/imx8-isi-
      video.c:949, 957
    - Graph mutex protection (scoped guard):
      drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:948
    - Format validation: drivers/media/platform/nxp/imx8-isi/imx8-isi-
      video.c:962
    - Discard buffers allocation:
      drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:967
    - All in mxc_isi_vb2_prepare_streaming():
      drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:940
  - New unprepare path frees discard buffers, stops the pipeline, and
    releases it:
    - Free/stop/release: drivers/media/platform/nxp/imx8-isi/imx8-isi-
      video.c:1033, 1034, 1035
    - In mxc_isi_vb2_unprepare_streaming():
      drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:1029
- Switches to standard vb2 stream ioctl helpers, aligning with core
  expectations and ensuring (un)prepare hooks are used:
  - .vidioc_streamon/off → vb2 helpers:
    drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:1255, 1256
  - Registers vb2 ops including prepare/unprepare:
    drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:1038
- Removes unconditional teardown from release, preventing non-streaming
  FDs from stopping an active stream:
  - mxc_isi_video_release() now relies on vb2_fop_release() and no
    manual cleanup: drivers/media/platform/nxp/imx8-isi/imx8-isi-
    video.c:1286-1296
- Streaming start/stop remains confined and symmetric:
  - Start: channel init, queue setup, enable:
    drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:981-1011
  - Stop: pipe disable, channel put, return buffers:
    drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:1019-1027

Why this fits stable backport criteria
- Important bugfix with clear user impact (streams can be disrupted by
  unrelated handle close; WARN in IRQ handler).
- Contained to the imx8-isi driver; no architectural changes to core
  subsystems.
- Uses established vb2 mechanisms to correctly bind resource lifetime to
  the streaming filehandle, minimizing regression risk.
- Simplifies code by deferring to vb2 helpers and lifecycle callbacks.

Compatibility notes for stable maintainers
- Requires vb2 .prepare_streaming/.unprepare_streaming support (present
  in modern stable series such as 6.1.y and 6.6.y).
- If targeting older stables lacking these vb2 ops, an equivalent fix
  must avoid unconditional release-time cleanup and keep pipeline
  (un)prepare tied to STREAMON/STREAMOFF (i.e., adapt without the new
  callbacks).
- The scoped_guard pattern
  (drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c:948) can be
  replaced with explicit mutex_lock/unlock for older trees if needed.

Overall, this is a focused, low-risk fix for a real streaming lifecycle
bug and is suitable for stable backporting.

 .../platform/nxp/imx8-isi/imx8-isi-video.c    | 156 +++++++-----------
 1 file changed, 58 insertions(+), 98 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
index 8654150728a86..042b554d2775a 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
@@ -937,6 +937,49 @@ static void mxc_isi_video_init_channel(struct mxc_isi_video *video)
 	mxc_isi_channel_set_output_format(pipe, video->fmtinfo, &video->pix);
 }
 
+static int mxc_isi_vb2_prepare_streaming(struct vb2_queue *q)
+{
+	struct mxc_isi_video *video = vb2_get_drv_priv(q);
+	struct media_device *mdev = &video->pipe->isi->media_dev;
+	struct media_pipeline *pipe;
+	int ret;
+
+	/* Get a pipeline for the video node and start it. */
+	scoped_guard(mutex, &mdev->graph_mutex) {
+		ret = mxc_isi_pipe_acquire(video->pipe,
+					   &mxc_isi_video_frame_write_done);
+		if (ret)
+			return ret;
+
+		pipe = media_entity_pipeline(&video->vdev.entity)
+		     ? : &video->pipe->pipe;
+
+		ret = __video_device_pipeline_start(&video->vdev, pipe);
+		if (ret)
+			goto err_release;
+	}
+
+	/* Verify that the video format matches the output of the subdev. */
+	ret = mxc_isi_video_validate_format(video);
+	if (ret)
+		goto err_stop;
+
+	/* Allocate buffers for discard operation. */
+	ret = mxc_isi_video_alloc_discard_buffers(video);
+	if (ret)
+		goto err_stop;
+
+	video->is_streaming = true;
+
+	return 0;
+
+err_stop:
+	video_device_pipeline_stop(&video->vdev);
+err_release:
+	mxc_isi_pipe_release(video->pipe);
+	return ret;
+}
+
 static int mxc_isi_vb2_start_streaming(struct vb2_queue *q, unsigned int count)
 {
 	struct mxc_isi_video *video = vb2_get_drv_priv(q);
@@ -985,13 +1028,26 @@ static void mxc_isi_vb2_stop_streaming(struct vb2_queue *q)
 	mxc_isi_video_return_buffers(video, VB2_BUF_STATE_ERROR);
 }
 
+static void mxc_isi_vb2_unprepare_streaming(struct vb2_queue *q)
+{
+	struct mxc_isi_video *video = vb2_get_drv_priv(q);
+
+	mxc_isi_video_free_discard_buffers(video);
+	video_device_pipeline_stop(&video->vdev);
+	mxc_isi_pipe_release(video->pipe);
+
+	video->is_streaming = false;
+}
+
 static const struct vb2_ops mxc_isi_vb2_qops = {
 	.queue_setup		= mxc_isi_vb2_queue_setup,
 	.buf_init		= mxc_isi_vb2_buffer_init,
 	.buf_prepare		= mxc_isi_vb2_buffer_prepare,
 	.buf_queue		= mxc_isi_vb2_buffer_queue,
+	.prepare_streaming	= mxc_isi_vb2_prepare_streaming,
 	.start_streaming	= mxc_isi_vb2_start_streaming,
 	.stop_streaming		= mxc_isi_vb2_stop_streaming,
+	.unprepare_streaming	= mxc_isi_vb2_unprepare_streaming,
 };
 
 /* -----------------------------------------------------------------------------
@@ -1145,97 +1201,6 @@ static int mxc_isi_video_s_fmt(struct file *file, void *priv,
 	return 0;
 }
 
-static int mxc_isi_video_streamon(struct file *file, void *priv,
-				  enum v4l2_buf_type type)
-{
-	struct mxc_isi_video *video = video_drvdata(file);
-	struct media_device *mdev = &video->pipe->isi->media_dev;
-	struct media_pipeline *pipe;
-	int ret;
-
-	if (vb2_queue_is_busy(&video->vb2_q, file))
-		return -EBUSY;
-
-	/*
-	 * Get a pipeline for the video node and start it. This must be done
-	 * here and not in the queue .start_streaming() handler, so that
-	 * pipeline start errors can be reported from VIDIOC_STREAMON and not
-	 * delayed until subsequent VIDIOC_QBUF calls.
-	 */
-	mutex_lock(&mdev->graph_mutex);
-
-	ret = mxc_isi_pipe_acquire(video->pipe, &mxc_isi_video_frame_write_done);
-	if (ret) {
-		mutex_unlock(&mdev->graph_mutex);
-		return ret;
-	}
-
-	pipe = media_entity_pipeline(&video->vdev.entity) ? : &video->pipe->pipe;
-
-	ret = __video_device_pipeline_start(&video->vdev, pipe);
-	if (ret) {
-		mutex_unlock(&mdev->graph_mutex);
-		goto err_release;
-	}
-
-	mutex_unlock(&mdev->graph_mutex);
-
-	/* Verify that the video format matches the output of the subdev. */
-	ret = mxc_isi_video_validate_format(video);
-	if (ret)
-		goto err_stop;
-
-	/* Allocate buffers for discard operation. */
-	ret = mxc_isi_video_alloc_discard_buffers(video);
-	if (ret)
-		goto err_stop;
-
-	ret = vb2_streamon(&video->vb2_q, type);
-	if (ret)
-		goto err_free;
-
-	video->is_streaming = true;
-
-	return 0;
-
-err_free:
-	mxc_isi_video_free_discard_buffers(video);
-err_stop:
-	video_device_pipeline_stop(&video->vdev);
-err_release:
-	mxc_isi_pipe_release(video->pipe);
-	return ret;
-}
-
-static void mxc_isi_video_cleanup_streaming(struct mxc_isi_video *video)
-{
-	lockdep_assert_held(&video->lock);
-
-	if (!video->is_streaming)
-		return;
-
-	mxc_isi_video_free_discard_buffers(video);
-	video_device_pipeline_stop(&video->vdev);
-	mxc_isi_pipe_release(video->pipe);
-
-	video->is_streaming = false;
-}
-
-static int mxc_isi_video_streamoff(struct file *file, void *priv,
-				   enum v4l2_buf_type type)
-{
-	struct mxc_isi_video *video = video_drvdata(file);
-	int ret;
-
-	ret = vb2_ioctl_streamoff(file, priv, type);
-	if (ret)
-		return ret;
-
-	mxc_isi_video_cleanup_streaming(video);
-
-	return 0;
-}
-
 static int mxc_isi_video_enum_framesizes(struct file *file, void *priv,
 					 struct v4l2_frmsizeenum *fsize)
 {
@@ -1291,9 +1256,8 @@ static const struct v4l2_ioctl_ops mxc_isi_video_ioctl_ops = {
 	.vidioc_expbuf			= vb2_ioctl_expbuf,
 	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
 	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
-
-	.vidioc_streamon		= mxc_isi_video_streamon,
-	.vidioc_streamoff		= mxc_isi_video_streamoff,
+	.vidioc_streamon		= vb2_ioctl_streamon,
+	.vidioc_streamoff		= vb2_ioctl_streamoff,
 
 	.vidioc_enum_framesizes		= mxc_isi_video_enum_framesizes,
 
@@ -1332,10 +1296,6 @@ static int mxc_isi_video_release(struct file *file)
 	if (ret)
 		dev_err(video->pipe->isi->dev, "%s fail\n", __func__);
 
-	mutex_lock(&video->lock);
-	mxc_isi_video_cleanup_streaming(video);
-	mutex_unlock(&video->lock);
-
 	pm_runtime_put(video->pipe->isi->dev);
 	return ret;
 }
-- 
2.51.0

[PATCH AUTOSEL 6.17] drm/xe: improve dma-resv handling for backup object

[Sasha Levin]

From: Matthew Auld <matthew.auld@intel.com>

[ Upstream commit edb1745fc618ba8ef63a45ce3ae60de1bdf29231 ]

Since the dma-resv is shared we don't need to reserve and add a fence
slot fence twice, plus no need to loop through the dependencies.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Link: https://lore.kernel.org/r/20250829164715.720735-2-matthew.auld@intel.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

Explanation

- What it fixes
  - Removes redundant dma-resv operations when a backup BO shares the
    same reservation object as the original BO, preventing the same
    fence from being reserved/added twice to the same `dma_resv`.
  - Avoids scanning the same dependency set twice when source and
    destination BOs share the same `dma_resv`.

- Why the change is correct
  - The backup object is created to share the parent’s reservation
    object, so a single reserve/add is sufficient:
    - The backup BO is initialized with the parent’s resv:
      `drivers/gpu/drm/xe/xe_bo.c:1309` (`xe_bo_init_locked(...,
      bo->ttm.base.resv, ...)`), ensuring `bo->ttm.base.resv ==
      backup->ttm.base.resv`.
    - The patch adds an explicit invariant check to document and enforce
      this: `drivers/gpu/drm/xe/xe_bo.c:1225` (`xe_assert(xe,
      bo->ttm.base.resv == backup->ttm.base.resv)`).
  - With shared `dma_resv`, adding the same fence twice is at best
    redundant (wasting fence slots and memory) and at worst error-prone.
    Reserving fence slots only once and adding the fence once is the
    correct behavior.

- Specific code changes and effects
  - Evict path (GPU migration copy case):
    - Before: reserves and adds fence on both `bo->ttm.base.resv` and
      `backup->ttm.base.resv`.
    - After: reserves and adds exactly once, guarded by the shared-resv
      assertion.
    - See single reserve and add: `drivers/gpu/drm/xe/xe_bo.c:1226`
      (reserve) and `drivers/gpu/drm/xe/xe_bo.c:1237` (add fence). This
      is the core fix; the removed second reserve/add on the backup is
      the redundant part eliminated.
  - Restore path (migration copy back):
    - Same simplification: reserve once, add once on the shared
      `dma_resv`.
    - See single reserve and add: `drivers/gpu/drm/xe/xe_bo.c:1375`
      (reserve) and `drivers/gpu/drm/xe/xe_bo.c:1387` (add fence).
  - Dependency handling in migrate:
    - Before: added deps for both src and dst based only on `src_bo !=
      dst_bo`.
    - After: only add dst deps if the resv objects differ, avoiding
      double-walking the same `dma_resv`.
    - See updated condition: `drivers/gpu/drm/xe/xe_migrate.c:932`
      (`src_bo->ttm.base.resv != dst_bo->ttm.base.resv`).

- User-visible impact without the patch
  - Duplicate `dma_resv_add_fence()` calls on the same reservation
    object can:
    - Consume extra shared-fence slots and memory.
    - Inflate dependency lists, causing unnecessary scheduler waits and
      overhead.
    - Increase failure likelihood of `dma_resv_reserve_fences()` under
      memory pressure.
  - These paths are exercised during suspend/resume flows of pinned VRAM
    BOs (evict/restore), so reliability and performance in power
    transitions can be affected.

- Scope and risk
  - Small, focused changes localized to the Intel Xe driver
    migration/evict/restore paths:
    - Files: `drivers/gpu/drm/xe/xe_bo.c`,
      `drivers/gpu/drm/xe/xe_migrate.c`.
  - No API changes or architectural refactors; logic strictly reduces
    redundant operations.
  - The `xe_assert` acts as a safety net to catch unexpected non-shared
    `resv` usage; normal runtime behavior is unchanged when the
    invariant holds.
  - The CPU copy fallback paths are untouched.

- Stable backport considerations
  - This is a clear correctness and robustness fix, not a feature.
  - Low regression risk if the stable branch also creates the backup BO
    with the parent’s `dma_resv` (as shown by the use of
    `xe_bo_init_locked(..., bo->ttm.base.resv, ...)` in
    `drivers/gpu/drm/xe/xe_bo.c:1309`).
  - If a stable branch diverges and the backup BO does not share the
    resv, this patch would need adjustment (i.e., keep dual reserve/add
    in that case). The added `xe_assert` helps surface such mismatches
    during testing.

Conclusion: This commit fixes a real bug (duplicate fence reserve/add
and duplicate dependency scanning on a shared `dma_resv`) with a
minimal, well-scoped change. It aligns with stable rules (important
bugfix, low risk, contained), so it should be backported.

 drivers/gpu/drm/xe/xe_bo.c      | 13 +------------
 drivers/gpu/drm/xe/xe_migrate.c |  2 +-
 2 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index d07e23eb1a54d..5a61441d68af5 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -1242,14 +1242,11 @@ int xe_bo_evict_pinned(struct xe_bo *bo)
 		else
 			migrate = mem_type_to_migrate(xe, bo->ttm.resource->mem_type);
 
+		xe_assert(xe, bo->ttm.base.resv == backup->ttm.base.resv);
 		ret = dma_resv_reserve_fences(bo->ttm.base.resv, 1);
 		if (ret)
 			goto out_backup;
 
-		ret = dma_resv_reserve_fences(backup->ttm.base.resv, 1);
-		if (ret)
-			goto out_backup;
-
 		fence = xe_migrate_copy(migrate, bo, backup, bo->ttm.resource,
 					backup->ttm.resource, false);
 		if (IS_ERR(fence)) {
@@ -1259,8 +1256,6 @@ int xe_bo_evict_pinned(struct xe_bo *bo)
 
 		dma_resv_add_fence(bo->ttm.base.resv, fence,
 				   DMA_RESV_USAGE_KERNEL);
-		dma_resv_add_fence(backup->ttm.base.resv, fence,
-				   DMA_RESV_USAGE_KERNEL);
 		dma_fence_put(fence);
 	} else {
 		ret = xe_bo_vmap(backup);
@@ -1338,10 +1333,6 @@ int xe_bo_restore_pinned(struct xe_bo *bo)
 		if (ret)
 			goto out_unlock_bo;
 
-		ret = dma_resv_reserve_fences(backup->ttm.base.resv, 1);
-		if (ret)
-			goto out_unlock_bo;
-
 		fence = xe_migrate_copy(migrate, backup, bo,
 					backup->ttm.resource, bo->ttm.resource,
 					false);
@@ -1352,8 +1343,6 @@ int xe_bo_restore_pinned(struct xe_bo *bo)
 
 		dma_resv_add_fence(bo->ttm.base.resv, fence,
 				   DMA_RESV_USAGE_KERNEL);
-		dma_resv_add_fence(backup->ttm.base.resv, fence,
-				   DMA_RESV_USAGE_KERNEL);
 		dma_fence_put(fence);
 	} else {
 		ret = xe_bo_vmap(backup);
diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
index 2a627ed64b8f8..ba9b8590eccb2 100644
--- a/drivers/gpu/drm/xe/xe_migrate.c
+++ b/drivers/gpu/drm/xe/xe_migrate.c
@@ -901,7 +901,7 @@ struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
 		if (!fence) {
 			err = xe_sched_job_add_deps(job, src_bo->ttm.base.resv,
 						    DMA_RESV_USAGE_BOOKKEEP);
-			if (!err && src_bo != dst_bo)
+			if (!err && src_bo->ttm.base.resv != dst_bo->ttm.base.resv)
 				err = xe_sched_job_add_deps(job, dst_bo->ttm.base.resv,
 							    DMA_RESV_USAGE_BOOKKEEP);
 			if (err)
-- 
2.51.0

[PATCH AUTOSEL 6.17-5.15] media: adv7180: Only validate format in querystd

[Sasha Levin]

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

[ Upstream commit 91c5d7c849273d14bc4bae1b92666bdb5409294a ]

The .querystd callback should not program the device with the detected
standard, it should only report the standard to user-space. User-space
may then use .s_std to set the standard, if it wants to use it.

All that is required of .querystd is to setup the auto detection of
standards and report its findings.

While at it add some documentation on why this can't happen while
streaming and improve the error handling using a scoped guard.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- What it fixes
  - Removes unintended device reprogramming in `.querystd`: Previously,
    after enabling autodetection and sampling, the code reprogrammed the
    decoder using the configured `curr_norm` instead of leaving hardware
    untouched. If userspace had never called `.s_std`, `curr_norm`
    defaults to NTSC (drivers/media/i2c/adv7180.c:1463), so a simple
    `.querystd` could force NTSC regardless of the detected standard.
    The new code stops doing that and only reports the detected
    standard.
  - Aligns with V4L2 semantics: `.querystd` should detect and report,
    not change the active standard. Userspace can call `.s_std` to set
    it.

- Key code changes
  - adv7180_querystd only sets autodetect, waits, and reports the
    detected standard:
    - Sets autodetect: drivers/media/i2c/adv7180.c:388
    - Returns detection result directly: drivers/media/i2c/adv7180.c:388
    - Adds clear rationale comment about not running during streaming
      since it touches VID_SEL: drivers/media/i2c/adv7180.c:388
  - Removes the reprogramming step via
    `v4l2_std_to_adv7180(state->curr_norm)` and the second
    `adv7180_set_video_standard(...)` (these were in the old body and
    are now gone), eliminating side effects of `.querystd`.
  - Improves error handling and robustness by using a scoped guard for
    the mutex (auto-unlock on all paths) and by returning the result of
    `__adv7180_status()` instead of ignoring it
    (drivers/media/i2c/adv7180.c:388).

- Why it’s safe and minimal
  - Localized change: confined to `adv7180_querystd` only
    (drivers/media/i2c/adv7180.c:388).
  - No ABI or architectural changes; just corrects behavior to be read-
    only.
  - Streaming safety preserved: returns `-EBUSY` while streaming to
    avoid touching VID_SEL mid-capture
    (drivers/media/i2c/adv7180.c:388).
  - Consistent control flow: `.s_std` now only validates and stores the
    intended standard in `curr_norm` (drivers/media/i2c/adv7180.c:463),
    and actual programming is done at stream start via
    `adv7180_program_std()` (drivers/media/i2c/adv7180.c:449) called by
    initialization/streaming code paths. Leaving the device in
    autodetect after `.querystd` does not affect users because streaming
    is off (enforced by `-EBUSY`) and streaming will reprogram from
    `curr_norm` anyway.

- User-visible impact addressed
  - Prevents `.querystd` from changing hardware state (e.g., enforcing
    NTSC because `curr_norm` defaults to NTSC at probe:
    drivers/media/i2c/adv7180.c:1463), which could break subsequent
    expectations if userspace hasn’t explicitly called `.s_std`.

- Backport considerations
  - The `guard(mutex)` scoped guard may not exist in older stable
    series. That’s trivial to adapt to explicit
    `mutex_lock_interruptible()`/`mutex_unlock()` with identical
    behavior. No other dependencies or refactors are required.

- Subsystem and risk profile
  - Touches a single V4L2 i2c decoder driver; small, self-contained
    change with clear behavioral bugfix and minimal regression risk.
  - Follows stable rules: bugfix, no new features, no architectural
    churn, and limited scope.

Given these points, this is a good candidate for stable backporting.

 drivers/media/i2c/adv7180.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 8100fe6b0f1d4..5accf3020e076 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -357,32 +357,27 @@ static inline struct adv7180_state *to_state(struct v4l2_subdev *sd)
 static int adv7180_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
 {
 	struct adv7180_state *state = to_state(sd);
-	int err = mutex_lock_interruptible(&state->mutex);
-	if (err)
-		return err;
-
-	if (state->streaming) {
-		err = -EBUSY;
-		goto unlock;
-	}
+	int ret;
 
-	err = adv7180_set_video_standard(state,
-			ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
-	if (err)
-		goto unlock;
+	guard(mutex)(&state->mutex);
 
-	msleep(100);
-	__adv7180_status(state, NULL, std);
+	/*
+	 * We can't sample the standard if the device is streaming as that would
+	 * interfere with the capture session as the VID_SEL reg is touched.
+	 */
+	if (state->streaming)
+		return -EBUSY;
 
-	err = v4l2_std_to_adv7180(state->curr_norm);
-	if (err < 0)
-		goto unlock;
+	/* Set the standard to autodetect PAL B/G/H/I/D, NTSC J or SECAM */
+	ret = adv7180_set_video_standard(state,
+					 ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
+	if (ret)
+		return ret;
 
-	err = adv7180_set_video_standard(state, err);
+	/* Allow some time for the autodetection to run. */
+	msleep(100);
 
-unlock:
-	mutex_unlock(&state->mutex);
-	return err;
+	return __adv7180_status(state, NULL, std);
 }
 
 static int adv7180_s_routing(struct v4l2_subdev *sd, u32 input,
-- 
2.51.0

[PATCH AUTOSEL 6.17-6.6] media: ov08x40: Fix the horizontal flip control

[Sasha Levin]

From: Hao Yao <hao.yao@intel.com>

[ Upstream commit c7df6f339af94689fdc433887f9fbb480bf8a4ed ]

The datasheet of ov08x40 doesn't match the hardware behavior.
0x3821[2] == 1 is the original state and 0 the horizontal flip enabled.

Signed-off-by: Hao Yao <hao.yao@intel.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Tested-by: Hans de Goede <hansg@kernel.org> # ThinkPad X1 Carbon Gen 12 & Gen 13
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@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:

YES

- What it fixes: The horizontal flip (HFLIP) control polarity was wrong
  for the OV08X40 sensor. Hardware enables H-mirror when bit 2 of
  register 0x3821 is cleared, but the driver treated setting the bit as
  “flip on”. The fix inverts this polarity so V4L2_CID_HFLIP now matches
  actual hardware behavior.
- Precise change: In `ov08x40_set_ctrl_hflip()`, the write toggling
  `OV08X40_REG_MIRROR` bit 2 is flipped:
  - Before: `ctrl_val ? val | BIT(2) : val & ~BIT(2)`
  - After: `ctrl_val ? val & ~BIT(2) : val | BIT(2)`
  - Location: drivers/media/i2c/ov08x40.c:1651
- Scope and containment:
  - Only one line in a single driver file is changed:
    `drivers/media/i2c/ov08x40.c`.
  - The function reads the current register value first and only changes
    bit 2, preserving other bits (drivers/media/i2c/ov08x40.c:1646,
    1651).
  - Vertical flip handling remains unchanged and continues to set bit 2
    of 0x3820 when enabled (drivers/media/i2c/ov08x40.c:1666), showing
    the change is isolated to HFLIP.
  - The HFLIP control is wired through the standard control path
    (`ov08x40_set_ctrl()` case V4L2_CID_HFLIP →
    `ov08x40_set_ctrl_hflip()`, drivers/media/i2c/ov08x40.c:1735) and
    HFLIP is created as a standard V4L2 control
    (drivers/media/i2c/ov08x40.c:2153).
- User impact: Without this fix, user-space sees inverted behavior for
  HFLIP (enabling the control does not produce the expected mirror),
  which is a clear functional bug affecting image orientation and any
  applications relying on correct orientation metadata/control.
- Risk assessment:
  - Minimal risk: a single-bit polarity correction in a register write,
    no ABI/API change, no architectural changes, and constrained to the
    ov08x40 driver.
  - Safe behavior: other bits are preserved; change is applied only when
    the device is powered/streaming via existing PM checks.
  - Real-world validation: Reviewed and Tested-by are present, including
    testing on ThinkPad X1 Carbon Gen 12/13, which reduces regression
    risk.

This is a small, targeted, and user-visible bugfix that aligns with
stable backporting rules and should be backported.

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

diff --git a/drivers/media/i2c/ov08x40.c b/drivers/media/i2c/ov08x40.c
index e0094305ca2ab..90887fc54fb0e 100644
--- a/drivers/media/i2c/ov08x40.c
+++ b/drivers/media/i2c/ov08x40.c
@@ -1648,7 +1648,7 @@ static int ov08x40_set_ctrl_hflip(struct ov08x40 *ov08x, u32 ctrl_val)
 
 	return ov08x40_write_reg(ov08x, OV08X40_REG_MIRROR,
 				 OV08X40_REG_VALUE_08BIT,
-				 ctrl_val ? val | BIT(2) : val & ~BIT(2));
+				 ctrl_val ? val & ~BIT(2) : val | BIT(2));
 }
 
 static int ov08x40_set_ctrl_vflip(struct ov08x40 *ov08x, u32 ctrl_val)
-- 
2.51.0

[PATCH AUTOSEL 6.17-5.15] media: em28xx: add special case for legacy gpiolib interface

[Sasha Levin]

From: Arnd Bergmann <arnd@arndb.de>

[ Upstream commit d5d299e7e7f6b4ead31383d4abffca34e4296df0 ]

The em28xx driver uses the old-style gpio_request_one() interface to
switch the lna on the PCTV 290E card.

This interface is becoming optional and should no longer be called by
portable drivers. As I could not figure out an obvious replacement,
select the new GPIOLIB_LEGACY symbol as a workaround.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- What it fixes: Prevents build breakage when `GPIOLIB=y` but the legacy
  GPIO consumer API is disabled. `gpio_request_one()` is only declared
  when `CONFIG_GPIOLIB_LEGACY` is enabled (see
  `include/linux/gpio.h:88`), so compiling code guarded only by
  `CONFIG_GPIOLIB` fails if legacy support is off.

- Code changes (small and contained):
  - Guards updated to match the actual availability of
    `gpio_request_one()`:
    - `drivers/media/usb/em28xx/em28xx-dvb.c:730` switches `#ifdef
      CONFIG_GPIOLIB` to `#ifdef CONFIG_GPIOLIB_LEGACY` in
      `em28xx_pctv_290e_set_lna()`, so the legacy-only API is used only
      when the legacy interface is present.
    - `drivers/media/usb/em28xx/em28xx-dvb.c:1708` does the same in
      `em28xx_dvb_init()` where the LNA is enabled on init for the PCTV
      290E.
  - Kconfig ensures the legacy API is pulled in when this driver
    configuration requires it:
    - `drivers/media/usb/em28xx/Kconfig:71` adds `select GPIOLIB_LEGACY
      if GPIOLIB && DVB_CXD2820R`.

- Behavior and scope:
  - If `GPIOLIB_LEGACY` is available, functionality is unchanged: the
    driver still toggles the LNA via `gpio_request_one()` and frees it.
  - If `GPIOLIB=y` but `GPIOLIB_LEGACY=n`, the code now cleanly compiles
    and falls back to a warning and no-op in
    `em28xx_pctv_290e_set_lna()` (see `drivers/media/usb/em28xx/em28xx-
    dvb.c:750`), avoiding a build error.
  - The Kconfig `select` line actively keeps legacy enabled for this
    combo, preserving LNA control where it mattered before.

- Risk assessment:
  - No architectural changes; purely Kconfig and preprocessor guards.
  - Touches only the em28xx media USB driver and its Kconfig.
  - Aligns with the tree-wide pattern where `gpio_request_one()` and
    friends are guarded by `CONFIG_GPIOLIB_LEGACY` (e.g.,
    `include/linux/gpio.h:88`, multiple existing users throughout the
    tree).
  - Minimal regression risk; at worst, adds the tiny legacy gpiolib-
    legacy code when selected by Kconfig.

- Stable criteria fit:
  - Fixes a real user-facing problem (build failure in valid configs).
  - Small, targeted, and low risk.
  - No new features; purely compatibility/build fix.

Note: Backport is relevant for stable series where
`CONFIG_GPIOLIB_LEGACY` exists and can be disabled. Older stable series
lacking this symbol won’t need (or may not accept) the Kconfig/guard
changes.

 drivers/media/usb/em28xx/Kconfig      | 1 +
 drivers/media/usb/em28xx/em28xx-dvb.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/em28xx/Kconfig b/drivers/media/usb/em28xx/Kconfig
index cb61fd6cc6c61..3122d4bdfc596 100644
--- a/drivers/media/usb/em28xx/Kconfig
+++ b/drivers/media/usb/em28xx/Kconfig
@@ -68,6 +68,7 @@ config VIDEO_EM28XX_DVB
 	select MEDIA_TUNER_XC5000 if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_MT2060 if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_MXL692 if MEDIA_SUBDRV_AUTOSELECT
+	select GPIOLIB_LEGACY if GPIOLIB && DVB_CXD2820R
 	help
 	  This adds support for DVB cards based on the
 	  Empiatech em28xx chips.
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 9fce59979e3bd..b94f5c70ab750 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -727,7 +727,7 @@ static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe)
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	struct em28xx_i2c_bus *i2c_bus = fe->dvb->priv;
 	struct em28xx *dev = i2c_bus->dev;
-#ifdef CONFIG_GPIOLIB
+#ifdef CONFIG_GPIOLIB_LEGACY
 	struct em28xx_dvb *dvb = dev->dvb;
 	int ret;
 	unsigned long flags;
@@ -1705,7 +1705,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
 				goto out_free;
 			}
 
-#ifdef CONFIG_GPIOLIB
+#ifdef CONFIG_GPIOLIB_LEGACY
 			/* enable LNA for DVB-T, DVB-T2 and DVB-C */
 			result = gpio_request_one(dvb->lna_gpio,
 						  GPIOF_OUT_INIT_LOW, NULL);
-- 
2.51.0

Re: [PATCH AUTOSEL 6.17-5.15] media: em28xx: add special case for legacy gpiolib interface

[Arnd Bergmann]

On Sat, Oct 25, 2025, at 17:55, Sasha Levin wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> [ Upstream commit d5d299e7e7f6b4ead31383d4abffca34e4296df0 ]
>
> The em28xx driver uses the old-style gpio_request_one() interface to
> switch the lna on the PCTV 290E card.
>
> This interface is becoming optional and should no longer be called by
> portable drivers. As I could not figure out an obvious replacement,
> select the new GPIOLIB_LEGACY symbol as a workaround.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>
> LLM Generated explanations, may be completely bogus:
>
> YES
>
> - What it fixes: Prevents build breakage when `GPIOLIB=y` but the legacy
>   GPIO consumer API is disabled. `gpio_request_one()` is only declared
>   when `CONFIG_GPIOLIB_LEGACY` is enabled (see
>   `include/linux/gpio.h:88`), so compiling code guarded only by
>   `CONFIG_GPIOLIB` fails if legacy support is off.

It's not needed for stable and has no effect in 6.17. This is
only a preparation for a later change.

         Arnd

Re: [PATCH AUTOSEL 6.17-5.15] media: em28xx: add special case for legacy gpiolib interface

[Sasha Levin]

On Mon, Oct 27, 2025 at 10:24:47AM +0100, Arnd Bergmann wrote:
>On Sat, Oct 25, 2025, at 17:55, Sasha Levin wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> [ Upstream commit d5d299e7e7f6b4ead31383d4abffca34e4296df0 ]
>>
>> The em28xx driver uses the old-style gpio_request_one() interface to
>> switch the lna on the PCTV 290E card.
>>
>> This interface is becoming optional and should no longer be called by
>> portable drivers. As I could not figure out an obvious replacement,
>> select the new GPIOLIB_LEGACY symbol as a workaround.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>>
>> LLM Generated explanations, may be completely bogus:
>>
>> YES
>>
>> - What it fixes: Prevents build breakage when `GPIOLIB=y` but the legacy
>>   GPIO consumer API is disabled. `gpio_request_one()` is only declared
>>   when `CONFIG_GPIOLIB_LEGACY` is enabled (see
>>   `include/linux/gpio.h:88`), so compiling code guarded only by
>>   `CONFIG_GPIOLIB` fails if legacy support is off.
>
>It's not needed for stable and has no effect in 6.17. This is
>only a preparation for a later change.

Dropped, thanks!

-- 
Thanks,
Sasha

[PATCH AUTOSEL 6.17-5.15] media: adv7180: Do not write format to device in set_fmt

[Sasha Levin]

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

[ Upstream commit 46c1e7814d1c3310ef23c01ed1a582ef0c8ab1d2 ]

The .set_fmt callback should not write the new format directly do the
device, it should only store it and have it applied by .s_stream.

The .s_stream callback already calls adv7180_set_field_mode() so it's
safe to remove programming of the device and just store the format and
have .s_stream apply it.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.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:

YES

- Fixes incorrect behavior: The change enforces the V4L2 subdev rule
  that .set_fmt should not program hardware but only update state, with
  hardware programming deferred to .s_stream. In the new code, .set_fmt
  (driver’s `adv7180_set_pad_format`) only stores the requested field
  and does not touch the device, eliminating unintended runtime side
  effects during format negotiation. See
  `drivers/media/i2c/adv7180.c:784-793` where it now only assigns
  `state->field` for ACTIVE formats and no longer toggles power or
  programs registers.

- Safe because .s_stream already applies the format: The .s_stream path
  powers the device off, configures it, then powers it on, and
  explicitly applies the field mode. `adv7180_s_stream` calls
  `init_device` when enabling, which calls `adv7180_set_field_mode`.
  See:
  - `drivers/media/i2c/adv7180.c:927-955` (.s_stream powers off, calls
    `init_device`, powers on),
  - `drivers/media/i2c/adv7180.c:844-859` (`init_device` calls
    `adv7180_program_std` then `adv7180_set_field_mode` at
    `drivers/media/i2c/adv7180.c:854`).
  This guarantees the stored `state->field` is applied at the correct
time.

- Eliminates disruptive side effects: Previously, .set_fmt would power-
  cycle the decoder and program field mode immediately (via
  `adv7180_set_power(state, false)`, `adv7180_set_field_mode(state)`,
  `adv7180_set_power(state, true)`), which could:
  - Disrupt ongoing or prepared streaming sessions by unexpectedly
    toggling power during format negotiation.
  - Violate the expected V4L2 subdev semantics where format negotiation
    should be side-effect-free.
  The new behavior prevents these issues by staging only the state
change and deferring hardware ops to .s_stream.

- Consistency and correctness: The stored field influences both
  negotiated format reporting and hardware setup when streaming starts:
  - Format reporting uses `state->field` to compute height and field
    (see `drivers/media/i2c/adv7180.c:700-709` and `759-761`).
  - When powering on with CSI-2, `adv7180_set_power` also considers
    `state->field` (e.g., sets CSI register 0x1D if progressive output
    is used), ensuring consistent device programming once the stream
    starts (see `drivers/media/i2c/adv7180.c:202-219`).

- Contained change, minimal risk:
  - Single-file, localized change to adv7180 driver.
  - No new features or ABI changes; aligns with established subdev
    patterns.
  - No architectural shifts; only removes premature device programming
    from .set_fmt.
  - The driver already centralizes device programming within the
    streaming lifecycle, so behavior remains correct while avoiding
    premature side effects.

- Stable suitability:
  - Addresses a real behavioral bug (programming hardware during
    .set_fmt) that can lead to video glitches or races during
    negotiation.
  - Small, targeted patch that reduces side effects and adheres to V4L2
    best practices.
  - Low regression risk provided the target stable trees have the same
    pattern where .s_stream calls into `init_device` →
    `adv7180_set_field_mode` (as in this tree). In this codebase, that
    condition is satisfied.

Given these points, this is a good candidate for backporting to stable
media trees.

 drivers/media/i2c/adv7180.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 84600fa75ae8a..8100fe6b0f1d4 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -812,14 +812,7 @@ static int adv7180_set_pad_format(struct v4l2_subdev *sd,
 	ret = adv7180_mbus_fmt(sd,  &format->format);
 
 	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
-		if (state->field != format->format.field) {
-			guard(mutex)(&state->mutex);
-
-			state->field = format->format.field;
-			adv7180_set_power(state, false);
-			adv7180_set_field_mode(state);
-			adv7180_set_power(state, true);
-		}
+		state->field = format->format.field;
 	} else {
 		framefmt = v4l2_subdev_state_get_format(sd_state, 0);
 		*framefmt = format->format;
-- 
2.51.0

[PATCH AUTOSEL 6.17] media: imx-mipi-csis: Only set clock rate when specified in DT

[Sasha Levin]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

[ Upstream commit 65673c6e33cf46f220cc5774166b373b3c087739 ]

The imx-mipi-csis driver sets the rate of the wrap clock to the value
specified in the device tree's "clock-frequency" property, and defaults
to 166 MHz otherwise. This is a historical mistake, as clock rate
selection should have been left to the assigned-clock-rates property.

Honouring the clock-frequency property can't be removed without breaking
backwards compatibility, and the corresponding code isn't very
intrusive. The 166 MHz default, on the other hand, prevents
configuration of the clock rate through assigned-clock-rates, as the
driver immediately overwrites the rate. This behaviour is confusing and
has cost debugging time.

There is little value in a 166 MHz default. All mainline device tree
sources that enable the CSIS specify a clock-frequency explicitly, and
the default wrap clock configuration on supported platforms is at least
as high as 166 MHz. Drop the default, and only set the clock rate
manually when the clock-frequency property is specified.

Link: https://lore.kernel.org/r/20250822002734.23516-10-laurent.pinchart@ideasonboard.com
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Frank Li <Frank.Li@nxp.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:

YES

Why this is a bug fix
- The driver unconditionally forced the wrap clock to 166 MHz when the
  DT lacked a clock-frequency property, which overrides DT-assigned
  clock rates. That breaks the expected DT model where `assigned-clock-
  rates` controls rates, leading to misconfiguration and hard-to-debug
  behavior.
- This change stops overriding the clock unless the DT explicitly
  requests it, restoring correct DT semantics.

What changed (code references)
- Set rate only when explicitly requested:
  - `drivers/media/platform/nxp/imx-mipi-csis.c:744` now guards
    `clk_set_rate()` with `if (csis->clk_frequency) { ... }`, meaning
    the driver only sets the rate when the DT provided `clock-
    frequency`.
- Drop the 166 MHz fallback:
  - `drivers/media/platform/nxp/imx-mipi-csis.c:1483` now reads `clock-
    frequency` without assigning a default if the property is absent,
    removing the prior implicit 166 MHz default.
- The removal of the default macro and fallback behavior eliminates the
  unconditional override while preserving backward compatibility for DTs
  that do specify `clock-frequency`.

Why it matters (user impact)
- Systems using `assigned-clock-rates` in DT were previously ignored by
  the driver due to the unconditional 166 MHz set, causing unexpected
  clock rates and potential functional issues.
- With this patch, DT-provided assigned rates take effect unless a
  legacy DT explicitly uses `clock-frequency`, which is retained for
  compatibility.

Risk and compatibility
- Scope is small and contained to one driver; no core or architectural
  changes.
- Backward compatibility is preserved for legacy DTs that specify
  `clock-frequency` (the driver still sets the rate in that case).
- For DTs without `clock-frequency`, the driver no longer forces 166 MHz
  and leaves the rate to the clock framework/DT assignments. The commit
  rationale notes that all mainline DTs enabling CSIS already specify
  `clock-frequency`, and default platform wrap clock configurations are
  at least as high as 166 MHz, reducing regression risk.
- The only functional behavior change is the removal of an incorrect
  default that masked DT configuration.

Stable criteria assessment
- Fixes a real misbehavior that affects users (DT `assigned-clock-rates`
  ignored).
- Minimal, well-contained change in a single driver file.
- No new features or architectural changes.
- Low regression risk with explicit consideration for legacy DT
  compatibility.
- No explicit “Cc: stable” or “Fixes” tag, but technically aligns with
  stable policy as a correctness fix that removes a problematic default
  override.

Conclusion
- This is a clear, low-risk bug fix that restores proper DT semantics
  and prevents the driver from clobbering assigned clock rates. It
  should be backported to stable.

 drivers/media/platform/nxp/imx-mipi-csis.c | 23 +++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 2beb5f43c2c01..cea017a2b14ec 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -228,8 +228,6 @@
 #define MIPI_CSIS_PKTDATA_EVEN			0x3000
 #define MIPI_CSIS_PKTDATA_SIZE			SZ_4K
 
-#define DEFAULT_SCLK_CSIS_FREQ			166000000UL
-
 struct mipi_csis_event {
 	bool debug;
 	u32 mask;
@@ -704,12 +702,17 @@ static int mipi_csis_clk_get(struct mipi_csis_device *csis)
 	if (ret < 0)
 		return ret;
 
-	/* Set clock rate */
-	ret = clk_set_rate(csis->clks[MIPI_CSIS_CLK_WRAP].clk,
-			   csis->clk_frequency);
-	if (ret < 0)
-		dev_err(csis->dev, "set rate=%d failed: %d\n",
-			csis->clk_frequency, ret);
+	if (csis->clk_frequency) {
+		/*
+		 * Set the clock rate. This is deprecated, for backward
+		 * compatibility with old device trees.
+		 */
+		ret = clk_set_rate(csis->clks[MIPI_CSIS_CLK_WRAP].clk,
+				   csis->clk_frequency);
+		if (ret < 0)
+			dev_err(csis->dev, "set rate=%d failed: %d\n",
+				csis->clk_frequency, ret);
+	}
 
 	return ret;
 }
@@ -1413,9 +1416,7 @@ static int mipi_csis_parse_dt(struct mipi_csis_device *csis)
 {
 	struct device_node *node = csis->dev->of_node;
 
-	if (of_property_read_u32(node, "clock-frequency",
-				 &csis->clk_frequency))
-		csis->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
+	of_property_read_u32(node, "clock-frequency", &csis->clk_frequency);
 
 	return 0;
 }
-- 
2.51.0

[PATCH AUTOSEL 6.17] media: qcom: camss: csiphy-3ph: Add CSIPHY 2ph DPHY v2.0.1 init sequence

[Sasha Levin]

From: Loic Poulain <loic.poulain@oss.qualcomm.com>

[ Upstream commit ce63fbdf849f52584d9b5d9a4cc23cbc88746c30 ]

This is the CSI PHY version found in QCS2290/QCM2290 SoCs.
The table is extracted from downstream camera driver.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Bryan O'Donoghue <bod@kernel.org>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- User impact and scope
  - Fixes non-functional camera PHY initialization on QCS2290/QCM2290 by
    adding the correct D-PHY v2.0.1 register init sequence. Without
    this, CSIPHY programming is incomplete and CSI2 links can fail to
    come up on this SoC.
  - Change is tightly scoped to the Qualcomm CAMSS CSIPHY 3-phase 1.0
    driver and only activates for `CAMSS_2290`.

- What the change does
  - Adds a SoC-specific init table used by the Gen2 programming path:
    - New `lane_regs_qcm2290` table programs the 14nm 2PH v2.0.1 D-PHY
      sequence, including per-lane settle count override points:
      - `drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c:403`
        through `drivers/media/platform/qcom/camss/camss-
        csiphy-3ph-1-0.c:483`
      - Entries like `0x0008`, `0x0208`, `0x0408`, `0x0608`, `0x0708`
        are tagged `CSIPHY_SETTLE_CNT_LOWER_BYTE`, which
        `csiphy_gen2_config_lanes()` will replace with the runtime-
        computed settle count (see
        `drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c:702`).
  - Ensures the Gen2 path is selected for this SoC:
    - Adds `CAMSS_2290` to the Gen2 detection so that
      `csiphy_lanes_enable()` chooses `csiphy_gen2_config_lanes()` over
      the generic Gen1 sequence:
      - Gen2 check updated in `csiphy_is_gen2()` (commit diff shows new
        case for `CAMSS_2290`), which is used at
        `drivers/media/platform/qcom/camss/camss-
        csiphy-3ph-1-0.c:957-960`.
  - Hooks the new table for 2290 during init:
    - `csiphy_init()` selects `lane_regs_qcm2290` and its size when
      `camss->res->version == CAMSS_2290`:
      - `drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c:998`
        to `drivers/media/platform/qcom/camss/camss-
        csiphy-3ph-1-0.c:1001`.
  - Identifies the SoC variant in the CAMSS version enum:
    - Adds `CAMSS_2290` to `enum camss_version`:
      - `drivers/media/platform/qcom/camss/camss.h:81`.

- Context in the subsystem
  - QCM2290 CAMSS resources already bind the CSIPHY instance to this
    driver and versioned resources set `.hw_ops = &csiphy_ops_3ph_1_0`
    for this SoC (e.g., `drivers/media/platform/qcom/camss/camss.c:518`
    for `csiphy_res_2290` and
    `drivers/media/platform/qcom/camss/camss.c:4330` for `.version =
    CAMSS_2290`), so this change fills the missing CSIPHY programming
    piece required for link bring-up on 2290.
  - The Gen2 write path (`csiphy_gen2_config_lanes`) consumes the new
    table and applies settle count correctly at the tagged offsets
    (`drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c:702`).

- Stable backport criteria
  - Bug fix: Yes. It corrects CSIPHY initialization for QCS/QCM2290,
    enabling a non-working camera interface on supported hardware.
  - Small and contained: Yes. Adds one SoC-specific data table and two
    switch cases; no broad refactors or ABI changes.
  - Side effects: Minimal. Only affects `CAMSS_2290` via explicit
    version checks; other SoCs are untouched.
  - Architectural changes: None; follows existing pattern used for
    SM8250/SM8550/etc.
  - Critical subsystems: Media platform driver (CAMSS), not core kernel;
    limited blast radius.
  - Stable tags: None in message, but the change clearly fits “important
    bugfix, minimal risk”.

- Risk assessment
  - Limited to 2290; if values were wrong, the only impact would be on
    camera bring-up on that SoC.
  - Enum addition is internal to the driver family; not user-visible
    ABI.
  - The settle count path remains computed dynamically and is properly
    injected into the register sequence, matching existing Gen2
    implementations.

Given this is a targeted fix that enables camera functionality on
QCS2290/QCM2290 by providing the correct PHY init sequence with low
regression risk, it is a good candidate for stable backport.

 .../qcom/camss/camss-csiphy-3ph-1-0.c         | 89 +++++++++++++++++++
 drivers/media/platform/qcom/camss/camss.h     |  1 +
 2 files changed, 90 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
index 88c0ba495c327..a128a42f1303d 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
@@ -319,6 +319,90 @@ csiphy_lane_regs lane_regs_sm8250[] = {
 	{0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
 };
 
+/* 14nm 2PH v 2.0.1 2p5Gbps 4 lane DPHY mode */
+static const struct
+csiphy_lane_regs lane_regs_qcm2290[] = {
+	{0x0030, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x002c, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0034, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0028, 0x04, 0x00, CSIPHY_DNP_PARAMS},
+	{0x003c, 0xb8, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x001c, 0x0a, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0000, 0xd7, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0004, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0020, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0008, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{0x000c, 0xff, 0x00, CSIPHY_DNP_PARAMS},
+	{0x0010, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0038, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0060, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0064, 0x3f, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+	{0x0730, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x072c, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0734, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0728, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x073c, 0xb8, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x071c, 0x0a, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0700, 0xc0, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0704, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0720, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0708, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{0x070c, 0xff, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0710, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0738, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0760, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0764, 0x3f, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+	{0x0230, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x022c, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0234, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0228, 0x04, 0x00, CSIPHY_DNP_PARAMS},
+	{0x023c, 0xb8, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x021c, 0x0a, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0200, 0xd7, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0204, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0220, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0208, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{0x020c, 0xff, 0x00, CSIPHY_DNP_PARAMS},
+	{0x0210, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0238, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0260, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0264, 0x3f, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+	{0x0430, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x042c, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0434, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0428, 0x04, 0x00, CSIPHY_DNP_PARAMS},
+	{0x043c, 0xb8, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x041c, 0x0a, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0400, 0xd7, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0404, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0420, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0408, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{0x040C, 0xff, 0x00, CSIPHY_DNP_PARAMS},
+	{0x0410, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0438, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0460, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0464, 0x3f, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+	{0x0630, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x062c, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0634, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0628, 0x04, 0x00, CSIPHY_DNP_PARAMS},
+	{0x063c, 0xb8, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x061c, 0x0a, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0600, 0xd7, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0604, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0620, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0608, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{0x060C, 0xff, 0x00, CSIPHY_DNP_PARAMS},
+	{0x0610, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0638, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0660, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0664, 0x3f, 0x00, CSIPHY_DEFAULT_PARAMS},
+};
+
 /* GEN2 2.1.2 2PH DPHY mode */
 static const struct
 csiphy_lane_regs lane_regs_sm8550[] = {
@@ -744,6 +828,7 @@ static bool csiphy_is_gen2(u32 version)
 	bool ret = false;
 
 	switch (version) {
+	case CAMSS_2290:
 	case CAMSS_7280:
 	case CAMSS_8250:
 	case CAMSS_8280XP:
@@ -829,6 +914,10 @@ static int csiphy_init(struct csiphy_device *csiphy)
 		regs->lane_regs = &lane_regs_sdm845[0];
 		regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
 		break;
+	case CAMSS_2290:
+		regs->lane_regs = &lane_regs_qcm2290[0];
+		regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290);
+		break;
 	case CAMSS_7280:
 	case CAMSS_8250:
 		regs->lane_regs = &lane_regs_sm8250[0];
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 63c0afee154a0..377707d91ff2f 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -78,6 +78,7 @@ enum pm_domain {
 
 enum camss_version {
 	CAMSS_660,
+	CAMSS_2290,
 	CAMSS_7280,
 	CAMSS_8x16,
 	CAMSS_8x53,
-- 
2.51.0

[PATCH AUTOSEL 6.17-5.4] media: redrat3: use int type to store negative error codes

[Sasha Levin]

From: Qianfeng Rong <rongqianfeng@vivo.com>

[ Upstream commit ecba852dc9f4993f4f894ea1f352564560e19a3e ]

Change "ret" from u8 to int type in redrat3_enable_detector() to store
negative error codes or zero returned by redrat3_send_cmd() and
usb_submit_urb() - this better aligns with the coding standards and
maintains code consistency.

No effect on runtime.

Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- What it fixes
  - Prevents truncation of negative error codes in
    `redrat3_enable_detector()`. Previously `ret` was `u8`, so failures
    from `usb_submit_urb()` (negative errno) would wrap to a positive
    byte (e.g., `-EPIPE` → `224`), causing the caller to treat failures
    as success.
  - The caller checks only for negative returns: `retval =
    redrat3_enable_detector(rr3); if (retval < 0) ...` in
    `drivers/media/rc/redrat3.c:1114-1116`. With a `u8` `ret`, errors
    would be lost, the probe would continue, and the device could end up
    non-functional (URBs not running) while the driver reports success.

- Evidence in code
  - `redrat3_send_cmd()` returns negative errno or non-negative status;
    it’s already `int`: `drivers/media/rc/redrat3.c:394-419`.
  - The patch changes `ret` to `int` in the detector enable path:
    `drivers/media/rc/redrat3.c:425`.
  - URB submissions return negative errno on error; these are assigned
    to and returned via `ret`: `drivers/media/rc/redrat3.c:439-443`,
    `drivers/media/rc/redrat3.c:445-449`. With `ret` as `u8`, a negative
    error like `-EPIPE` becomes a large positive and bypasses the `< 0`
    check at the call site (`drivers/media/rc/redrat3.c:1114-1116`).

- Scope and risk
  - Minimal, localized change (1 line, one function, single driver).
  - No API/ABI change; no behavior change on success paths; only
    corrects error propagation.
  - Aligns with kernel conventions where error codes are negative
    `int`s; the rest of this driver already uses `int ret` broadly
    (e.g., `drivers/media/rc/redrat3.c:503`,
    `drivers/media/rc/redrat3.c:657`, `drivers/media/rc/redrat3.c:691`,
    etc.), improving consistency.

- Stable backport criteria
  - Fixes a real, user-visible bug (driver may “succeed” initialization
    while URBs failed, leading to non-working IR receive).
  - Small and contained; no architectural change; regression risk is
    very low.
  - Not a feature addition; strictly a correctness fix for error
    handling.

Given the clear correctness improvement, minimal risk, and user impact
on failure paths, this commit is a good candidate for stable backport.

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

diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index d89a4cfe3c895..a49173f54a4d0 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -422,7 +422,7 @@ static int redrat3_send_cmd(int cmd, struct redrat3_dev *rr3)
 static int redrat3_enable_detector(struct redrat3_dev *rr3)
 {
 	struct device *dev = rr3->dev;
-	u8 ret;
+	int ret;
 
 	ret = redrat3_send_cmd(RR3_RC_DET_ENABLE, rr3);
 	if (ret != 0)
-- 
2.51.0

[PATCH AUTOSEL 6.17-5.4] media: pci: ivtv: Don't create fake v4l2_fh

[Sasha Levin]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

[ Upstream commit cc6e8d1ccea792d8550428e0831e3a35b0ccfddc ]

The ivtv driver has a structure named ivtv_open_id that models an open
file handle for the device. It embeds a v4l2_fh instance for file
handles that correspond to a V4L2 video device, and stores a pointer to
that v4l2_fh in struct ivtv_stream to identify which open file handle
owns a particular stream.

In addition to video devices, streams can be owned by ALSA PCM devices.
Those devices do not make use of the v4l2_fh instance for obvious
reasons, but the snd_ivtv_pcm_capture_open() function still initializes
a "fake" v4l2_fh for the sole purpose of using it as an open file handle
identifier. The v4l2_fh is not properly destroyed when the ALSA PCM
device is closed, leading to possible resource leaks.

Fortunately, the v4l2_fh instance pointed to by ivtv_stream is not
accessed, only the pointer value is used for comparison. Replace it with
a pointer to the ivtv_open_id structure that embeds the v4l2_fh, and
don't initialize the v4l2_fh for ALSA PCM devices.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.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:

YES

Why this is a good stable backport
- Fixes a real bug: ALSA PCM open in ivtv initialized a fake v4l2_fh
  that was never properly destroyed on success or on “already streaming”
  early return, causing a kernel memory leak across repeated open/close
  cycles.
- Small, contained change: confines to ivtv driver internals (media:
  pci: ivtv), with no API/ABI changes to V4L2/ALSA frameworks.
- Behavior-preserving: the driver used the v4l2_fh pointer solely as an
  opaque owner identifier (pointer equality/non-NULL), never
  dereferencing it. Switching to an ivtv_open_id pointer preserves
  semantics without allocating any v4l2_fh resources.
- Low regression risk: only owner-tracking logic and ALSA open path are
  touched; no architectural changes; no critical core subsystems
  affected.
- User impact: prevents kernel memory leakage reachable by regular users
  via the ALSA PCM device node.

What the patch changes (with code evidence)
- Stop creating a fake v4l2_fh for ALSA PCM open:
  - Removes `v4l2_fh_init(&item.fh, &s->vdev)` in ALSA open, avoiding
    allocation that was never exited in the success/streaming path.
  - Evidence: drivers/media/pci/ivtv/ivtv-alsa-pcm.c:151
- Switch stream owner tracking from v4l2_fh* to ivtv_open_id*:
  - Struct change: `struct ivtv_stream` replaces `struct v4l2_fh *fh;`
    with `struct ivtv_open_id *id;`
  - Evidence: drivers/media/pci/ivtv/ivtv-driver.h:334
  - Adds forward decl for `struct ivtv_open_id;` so the pointer type can
    be used earlier in the header (part of the change).
- Update owner comparisons and checks accordingly:
  - ivtv_claim_stream now compares `s->id == id` instead of `s->fh ==
    &id->fh`, assigns `s->id = id`, and handles VBI special-case the
    same way.
  - Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:42, 46–51, 59
  - ivtv_release_stream clears `s->id = NULL` (was `s->fh = NULL`) and
    uses `s_vbi->id` for “still claimed” checks.
  - Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:97, 129
  - ivtv_read initialization check uses `s->id == NULL` instead of
    `s->fh == NULL`.
  - Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:362
  - ivtv_stop_capture VBI-internal use case sets `s->id = NULL` instead
    of `s->fh = NULL`.
  - Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:834
  - ivtv_v4l2_close compares `s->id != id` instead of `s->fh != &id->fh`
    to detect ownership.
  - Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:918
  - ivtv-irq VBI data handling checks `s->id == NULL` (was `s->fh ==
    NULL`) to decide whether to free buffers if no owner, and uses
    `s->id` for wakeups.
  - Evidence: drivers/media/pci/ivtv/ivtv-irq.c:301–312 (check), 333–334
    (wake_up)
- Remove the now-unneeded v4l2_fh exit on the ALSA open fail path:
  - Since we no longer init a fake fh, there’s nothing to exit on
    -EBUSY.
  - Evidence: drivers/media/pci/ivtv/ivtv-alsa-pcm.c:151 (removal
    implies corresponding exit removal)

Why this fixes the leak
- Before: `snd_ivtv_pcm_capture_open()` created `item.fh` via
  `v4l2_fh_init` and then:
  - On successful claim and subsequent returns (including the “already
    streaming” fast path), there was no matching `v4l2_fh_exit`, leaking
    internal resources of `v4l2_fh`. See initialization at
    drivers/media/pci/ivtv/ivtv-alsa-pcm.c:151 and early return in the
    streaming case just after claim.
- After: There is no `v4l2_fh_init` for ALSA opens; the driver tracks
  ownership with a raw `ivtv_open_id *` pointer whose address is used
  only as a token (never dereferenced), eliminating the need for any
  initialization or teardown on the ALSA path.

Risk and side effects
- Preserved semantics: s->fh was never dereferenced; it was used only
  for pointer equality and non-NULL checks. Replacing with s->id keeps
  those semantics while removing the artificial fh lifecycle.
- Internal-only: Changes are contained within ivtv; no external APIs
  affected. Struct layout changed only internally to the driver.
- Concurrency/logic continuity: The VBI special-case claim/release and
  wakeup conditions are updated consistently to use s->id; read path and
  IRQ paths keep identical logic, just different pointer type.

Stable criteria check
- Important bugfix: addresses a user-visible kernel memory leak.
- Minimal, localized change: touches only ivtv, no cross-subsystem
  churn.
- No new features or architectural changes.
- Low regression risk, with clear correctness rationale.
- No explicit “Cc: stable” tag in the message, but it meets the stable
  rules for critical bug fixes with minimal risk.

Backport notes
- Ensure all s->fh uses in ivtv are converted to s->id. In the current
  tree, these occur at:
  - drivers/media/pci/ivtv/ivtv-fileops.c:42, 46–51, 59, 97, 129, 362,
    834, 918
  - drivers/media/pci/ivtv/ivtv-irq.c:308, 333
  - drivers/media/pci/ivtv/ivtv-driver.h:334 (field)
- No other ivtv files refer to s->fh; remaining fh usage is via id->fh
  for V4L2 event/poll mechanisms and is unchanged.
- The forward declaration `struct ivtv_open_id;` must be added before
  `struct ivtv_stream` in ivtv-driver.h to avoid compile errors.

Conclusion
- This commit fixes a resource leak in a safe, contained way without
  changing behavior, and is suitable for backporting to stable kernel
  trees.

 drivers/media/pci/ivtv/ivtv-alsa-pcm.c |  2 --
 drivers/media/pci/ivtv/ivtv-driver.h   |  3 ++-
 drivers/media/pci/ivtv/ivtv-fileops.c  | 18 +++++++++---------
 drivers/media/pci/ivtv/ivtv-irq.c      |  4 ++--
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
index 8f346d7da9c8d..269a799ec046c 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
@@ -148,14 +148,12 @@ static int snd_ivtv_pcm_capture_open(struct snd_pcm_substream *substream)
 
 	s = &itv->streams[IVTV_ENC_STREAM_TYPE_PCM];
 
-	v4l2_fh_init(&item.fh, &s->vdev);
 	item.itv = itv;
 	item.type = s->type;
 
 	/* See if the stream is available */
 	if (ivtv_claim_stream(&item, item.type)) {
 		/* No, it's already in use */
-		v4l2_fh_exit(&item.fh);
 		snd_ivtv_unlock(itvsc);
 		return -EBUSY;
 	}
diff --git a/drivers/media/pci/ivtv/ivtv-driver.h b/drivers/media/pci/ivtv/ivtv-driver.h
index a6ffa99e16bc6..83818048f7fe4 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h
@@ -322,6 +322,7 @@ struct ivtv_queue {
 };
 
 struct ivtv;				/* forward reference */
+struct ivtv_open_id;
 
 struct ivtv_stream {
 	/* These first four fields are always set, even if the stream
@@ -331,7 +332,7 @@ struct ivtv_stream {
 	const char *name;		/* name of the stream */
 	int type;			/* stream type */
 
-	struct v4l2_fh *fh;		/* pointer to the streaming filehandle */
+	struct ivtv_open_id *id;	/* pointer to the streaming ivtv_open_id */
 	spinlock_t qlock;		/* locks access to the queues */
 	unsigned long s_flags;		/* status flags, see above */
 	int dma;			/* can be PCI_DMA_TODEVICE, PCI_DMA_FROMDEVICE or PCI_DMA_NONE */
diff --git a/drivers/media/pci/ivtv/ivtv-fileops.c b/drivers/media/pci/ivtv/ivtv-fileops.c
index cfa28d0355863..1ac8d691df5cd 100644
--- a/drivers/media/pci/ivtv/ivtv-fileops.c
+++ b/drivers/media/pci/ivtv/ivtv-fileops.c
@@ -39,16 +39,16 @@ int ivtv_claim_stream(struct ivtv_open_id *id, int type)
 
 	if (test_and_set_bit(IVTV_F_S_CLAIMED, &s->s_flags)) {
 		/* someone already claimed this stream */
-		if (s->fh == &id->fh) {
+		if (s->id == id) {
 			/* yes, this file descriptor did. So that's OK. */
 			return 0;
 		}
-		if (s->fh == NULL && (type == IVTV_DEC_STREAM_TYPE_VBI ||
+		if (s->id == NULL && (type == IVTV_DEC_STREAM_TYPE_VBI ||
 					 type == IVTV_ENC_STREAM_TYPE_VBI)) {
 			/* VBI is handled already internally, now also assign
 			   the file descriptor to this stream for external
 			   reading of the stream. */
-			s->fh = &id->fh;
+			s->id = id;
 			IVTV_DEBUG_INFO("Start Read VBI\n");
 			return 0;
 		}
@@ -56,7 +56,7 @@ int ivtv_claim_stream(struct ivtv_open_id *id, int type)
 		IVTV_DEBUG_INFO("Stream %d is busy\n", type);
 		return -EBUSY;
 	}
-	s->fh = &id->fh;
+	s->id = id;
 	if (type == IVTV_DEC_STREAM_TYPE_VBI) {
 		/* Enable reinsertion interrupt */
 		ivtv_clear_irq_mask(itv, IVTV_IRQ_DEC_VBI_RE_INSERT);
@@ -94,7 +94,7 @@ void ivtv_release_stream(struct ivtv_stream *s)
 	struct ivtv *itv = s->itv;
 	struct ivtv_stream *s_vbi;
 
-	s->fh = NULL;
+	s->id = NULL;
 	if ((s->type == IVTV_DEC_STREAM_TYPE_VBI || s->type == IVTV_ENC_STREAM_TYPE_VBI) &&
 		test_bit(IVTV_F_S_INTERNAL_USE, &s->s_flags)) {
 		/* this stream is still in use internally */
@@ -126,7 +126,7 @@ void ivtv_release_stream(struct ivtv_stream *s)
 		/* was already cleared */
 		return;
 	}
-	if (s_vbi->fh) {
+	if (s_vbi->id) {
 		/* VBI stream still claimed by a file descriptor */
 		return;
 	}
@@ -359,7 +359,7 @@ static ssize_t ivtv_read(struct ivtv_stream *s, char __user *ubuf, size_t tot_co
 	size_t tot_written = 0;
 	int single_frame = 0;
 
-	if (atomic_read(&itv->capturing) == 0 && s->fh == NULL) {
+	if (atomic_read(&itv->capturing) == 0 && s->id == NULL) {
 		/* shouldn't happen */
 		IVTV_DEBUG_WARN("Stream %s not initialized before read\n", s->name);
 		return -EIO;
@@ -831,7 +831,7 @@ void ivtv_stop_capture(struct ivtv_open_id *id, int gop_end)
 		     id->type == IVTV_ENC_STREAM_TYPE_VBI) &&
 		    test_bit(IVTV_F_S_INTERNAL_USE, &s->s_flags)) {
 			/* Also used internally, don't stop capturing */
-			s->fh = NULL;
+			s->id = NULL;
 		}
 		else {
 			ivtv_stop_v4l2_encode_stream(s, gop_end);
@@ -915,7 +915,7 @@ int ivtv_v4l2_close(struct file *filp)
 	v4l2_fh_exit(fh);
 
 	/* Easy case first: this stream was never claimed by us */
-	if (s->fh != &id->fh)
+	if (s->id != id)
 		goto close_done;
 
 	/* 'Unclaim' this stream */
diff --git a/drivers/media/pci/ivtv/ivtv-irq.c b/drivers/media/pci/ivtv/ivtv-irq.c
index 4d63daa01eed2..078d9cd77c710 100644
--- a/drivers/media/pci/ivtv/ivtv-irq.c
+++ b/drivers/media/pci/ivtv/ivtv-irq.c
@@ -305,7 +305,7 @@ static void dma_post(struct ivtv_stream *s)
 			ivtv_process_vbi_data(itv, buf, 0, s->type);
 			s->q_dma.bytesused += buf->bytesused;
 		}
-		if (s->fh == NULL) {
+		if (s->id == NULL) {
 			ivtv_queue_move(s, &s->q_dma, NULL, &s->q_free, 0);
 			return;
 		}
@@ -330,7 +330,7 @@ static void dma_post(struct ivtv_stream *s)
 		set_bit(IVTV_F_I_HAVE_WORK, &itv->i_flags);
 	}
 
-	if (s->fh)
+	if (s->id)
 		wake_up(&s->waitq);
 }
 
-- 
2.51.0

[PATCH AUTOSEL 6.17-5.4] media: imon: make send_packet() more robust

[Sasha Levin]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit eecd203ada43a4693ce6fdd3a58ae10c7819252c ]

syzbot is reporting that imon has three problems which result in
hung tasks due to forever holding device lock [1].

First problem is that when usb_rx_callback_intf0() once got -EPROTO error
after ictx->dev_present_intf0 became true, usb_rx_callback_intf0()
resubmits urb after printk(), and resubmitted urb causes
usb_rx_callback_intf0() to again get -EPROTO error. This results in
printk() flooding (RCU stalls).

Alan Stern commented [2] that

  In theory it's okay to resubmit _if_ the driver has a robust
  error-recovery scheme (such as giving up after some fixed limit on the
  number of errors or after some fixed time has elapsed, perhaps with a
  time delay to prevent a flood of errors).  Most drivers don't bother to
  do this; they simply give up right away.  This makes them more
  vulnerable to short-term noise interference during USB transfers, but in
  reality such interference is quite rare.  There's nothing really wrong
  with giving up right away.

but imon has a poor error-recovery scheme which just retries forever;
this behavior should be fixed.

Since I'm not sure whether it is safe for imon users to give up upon any
error code, this patch takes care of only union of error codes chosen from
modules in drivers/media/rc/ directory which handle -EPROTO error (i.e.
ir_toy, mceusb and igorplugusb).

Second problem is that when usb_rx_callback_intf0() once got -EPROTO error
before ictx->dev_present_intf0 becomes true, usb_rx_callback_intf0() always
resubmits urb due to commit 8791d63af0cf ("[media] imon: don't wedge
hardware after early callbacks"). Move the ictx->dev_present_intf0 test
introduced by commit 6f6b90c9231a ("[media] imon: don't parse scancodes
until intf configured") to immediately before imon_incoming_packet(), or
the first problem explained above happens without printk() flooding (i.e.
hung task).

Third problem is that when usb_rx_callback_intf0() is not called for some
reason (e.g. flaky hardware; the reproducer for this problem sometimes
prevents usb_rx_callback_intf0() from being called),
wait_for_completion_interruptible() in send_packet() never returns (i.e.
hung task). As a workaround for such situation, change send_packet() to
wait for completion with timeout of 10 seconds.

Link: https://syzkaller.appspot.com/bug?extid=592e2ab8775dbe0bf09a [1]
Link: https://lkml.kernel.org/r/d6da6709-d799-4be3-a695-850bddd6eb24@rowland.harvard.edu [2]
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

This is a targeted robustness fix for the SoundGraph iMON IR/display USB
driver that addresses real-world hangs and RCU stalls reported by
syzbot, with small, contained changes limited to
`drivers/media/rc/imon.c`. It fits stable backport rules: fixes
important bugs, avoids features/architecture changes, and aligns error
handling with peer RC drivers.

Key fixes and rationale
- Stop infinite RX error loops (-EPROTO flood):
  - Current code resubmits the URB on all errors, which can loop
    endlessly on persistent USB errors such as -EPROTO, spamming logs
    and causing RCU stalls.
  - The patch handles specific transient/bus/pipe errors by logging and
    not resubmitting, breaking the error loop:
    - Adds early-return without resubmit on `-ECONNRESET`, `-EILSEQ`,
      `-EPROTO`, `-EPIPE` in both callbacks.
    - This mirrors other rc drivers’ behavior (see below).
  - Code reference (pre-change): `drivers/media/rc/imon.c:1757` shows an
    unconditional “resubmit on error” path via `goto out` and
    fallthrough to `usb_submit_urb()`, with “ignored” logging in
    `default:` (e.g. `drivers/media/rc/imon.c:1765` onward for intf0;
    similar for intf1 at `drivers/media/rc/imon.c:1806`).
- Fix early-callback behavior before interface is configured:
  - Previously, if an early callback happened before
    `dev_present_intf{0,1}` was set, the code always resubmitted (due to
    the pre-switch `if (!dev_present) goto out;`), which can contribute
    to error loops without even processing packets.
  - The patch moves the `dev_present_intf{0,1}` check into the `case 0:`
    (success) branch so that only valid data is processed, while error
    handling goes through the new non-resubmit paths. This prevents
    unnecessary requeueing during pre-configured phases and avoids
    wedging/hung tasks without printk flood.
  - Code reference (pre-change): the pre-switch gating at
    `drivers/media/rc/imon.c:1757-1764` (intf0) and
    `drivers/media/rc/imon.c:1798-1805` (intf1) causes unconditional
    resubmit on any pre-configured callback.
- Prevent indefinite TX wait (hung tasks):
  - Currently `send_packet()` waits indefinitely for the TX completion
    and only wakes on interrupt, hanging if the TX callback never
    arrives (e.g., flaky hardware).
  - The patch changes the wait to
    `wait_for_completion_interruptible_timeout()` with a 10s timeout,
    kills the URB on timeout, and sets a sensible error status
    (-ETIMEDOUT on timeout, or the negative retval on signal), then
    reports failure.
  - Code reference (pre-change): `drivers/media/rc/imon.c:653-659` uses
    `wait_for_completion_interruptible(&ictx->tx.finished)` with no
    timeout. The patch replaces this with a 10*HZ timeout and sets
    `ictx->tx.status` appropriately after `usb_kill_urb()`.

Consistency with other rc drivers
- This driver’s new error handling aligns with other `drivers/media/rc/`
  drivers which do not resubmit on transient/bus errors:
  - `mceusb`: avoids resubmit for `-ECONNRESET`, `-ENOENT`, `-EILSEQ`,
    `-EPROTO`, `-ESHUTDOWN` in RX (drivers/media/rc/mceusb.c:1360).
  - `igorplugusb`: returns without resubmit on `-EPROTO`, `-ECONNRESET`,
    `-ENOENT`, `-ESHUTDOWN` (drivers/media/rc/igorplugusb.c:100).
  - `ir_toy`: returns/unlinks on `-ECONNRESET`, `-ENOENT`, `-ESHUTDOWN`,
    `-EPROTO`, `-EPIPE` (drivers/media/rc/ir_toy.c:212).

Scope and risk assessment
- Small, self-contained changes in a single driver file; no ABI/API
  changes; no architectural shifts.
- Fixes reproducible hangs and RCU stalls (syzbot report), clearly
  improving stability.
- Behavior trade-off (not resubmitting on certain USB errors) matches
  established patterns in similar drivers, reducing regression risk.
- Interaction with earlier imon commits:
  - 6f6b90c9231a “[media] imon: don't parse scancodes until intf
    configured” introduced early-return gating using `dev_present_*`.
  - 8791d63af0cf “[media] imon: don't wedge hardware after early
    callbacks” ensured resubmit even before configuration.
  - This patch refines both behaviors: only resubmit when it makes sense
    (successful RX or non-specified errors), and bound TX wait time.

Why this belongs in stable
- Fixes real bugs affecting users (hung tasks, potential RCU
  stall/flooded logs).
- Minimal and surgical per-driver changes; no new features.
- Matches stable policy and conventions across related drivers.
- Signed by subsystem maintainers (Sean Young, Hans Verkuil), indicating
  maintainership approval.

Conclusion
- Backporting this commit will prevent hangs and error floods in `imon`
  with low risk and improve robustness without altering kernel-wide
  behavior.

 drivers/media/rc/imon.c | 61 +++++++++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index cf3e6e43c0c7e..8668d53c0d426 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -650,12 +650,15 @@ static int send_packet(struct imon_context *ictx)
 		smp_rmb(); /* ensure later readers know we're not busy */
 		pr_err_ratelimited("error submitting urb(%d)\n", retval);
 	} else {
-		/* Wait for transmission to complete (or abort) */
-		retval = wait_for_completion_interruptible(
-				&ictx->tx.finished);
-		if (retval) {
+		/* Wait for transmission to complete (or abort or timeout) */
+		retval = wait_for_completion_interruptible_timeout(&ictx->tx.finished, 10 * HZ);
+		if (retval <= 0) {
 			usb_kill_urb(ictx->tx_urb);
 			pr_err_ratelimited("task interrupted\n");
+			if (retval < 0)
+				ictx->tx.status = retval;
+			else
+				ictx->tx.status = -ETIMEDOUT;
 		}
 
 		ictx->tx.busy = false;
@@ -1754,14 +1757,6 @@ static void usb_rx_callback_intf0(struct urb *urb)
 	if (!ictx)
 		return;
 
-	/*
-	 * if we get a callback before we're done configuring the hardware, we
-	 * can't yet process the data, as there's nowhere to send it, but we
-	 * still need to submit a new rx URB to avoid wedging the hardware
-	 */
-	if (!ictx->dev_present_intf0)
-		goto out;
-
 	switch (urb->status) {
 	case -ENOENT:		/* usbcore unlink successful! */
 		return;
@@ -1770,16 +1765,29 @@ static void usb_rx_callback_intf0(struct urb *urb)
 		break;
 
 	case 0:
-		imon_incoming_packet(ictx, urb, intfnum);
+		/*
+		 * if we get a callback before we're done configuring the hardware, we
+		 * can't yet process the data, as there's nowhere to send it, but we
+		 * still need to submit a new rx URB to avoid wedging the hardware
+		 */
+		if (ictx->dev_present_intf0)
+			imon_incoming_packet(ictx, urb, intfnum);
 		break;
 
+	case -ECONNRESET:
+	case -EILSEQ:
+	case -EPROTO:
+	case -EPIPE:
+		dev_warn(ictx->dev, "imon %s: status(%d)\n",
+			 __func__, urb->status);
+		return;
+
 	default:
 		dev_warn(ictx->dev, "imon %s: status(%d): ignored\n",
 			 __func__, urb->status);
 		break;
 	}
 
-out:
 	usb_submit_urb(ictx->rx_urb_intf0, GFP_ATOMIC);
 }
 
@@ -1795,14 +1803,6 @@ static void usb_rx_callback_intf1(struct urb *urb)
 	if (!ictx)
 		return;
 
-	/*
-	 * if we get a callback before we're done configuring the hardware, we
-	 * can't yet process the data, as there's nowhere to send it, but we
-	 * still need to submit a new rx URB to avoid wedging the hardware
-	 */
-	if (!ictx->dev_present_intf1)
-		goto out;
-
 	switch (urb->status) {
 	case -ENOENT:		/* usbcore unlink successful! */
 		return;
@@ -1811,16 +1811,29 @@ static void usb_rx_callback_intf1(struct urb *urb)
 		break;
 
 	case 0:
-		imon_incoming_packet(ictx, urb, intfnum);
+		/*
+		 * if we get a callback before we're done configuring the hardware, we
+		 * can't yet process the data, as there's nowhere to send it, but we
+		 * still need to submit a new rx URB to avoid wedging the hardware
+		 */
+		if (ictx->dev_present_intf1)
+			imon_incoming_packet(ictx, urb, intfnum);
 		break;
 
+	case -ECONNRESET:
+	case -EILSEQ:
+	case -EPROTO:
+	case -EPIPE:
+		dev_warn(ictx->dev, "imon %s: status(%d)\n",
+			 __func__, urb->status);
+		return;
+
 	default:
 		dev_warn(ictx->dev, "imon %s: status(%d): ignored\n",
 			 __func__, urb->status);
 		break;
 	}
 
-out:
 	usb_submit_urb(ictx->rx_urb_intf1, GFP_ATOMIC);
 }
 
-- 
2.51.0

[PATCH AUTOSEL 6.17-5.15] media: adv7180: Add missing lock in suspend callback

[Sasha Levin]

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

[ Upstream commit 878c496ac5080f94a93a9216a8f70cfd67ace8c9 ]

The adv7180_set_power() utilizes adv7180_write() which in turn requires
the state mutex to be held, take it before calling adv7180_set_power()
to avoid tripping a lockdep_assert_held().

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- Bug fixed: adv7180_set_power() calls adv7180_write(), which asserts
  the device mutex must be held. See the assertion in
  `drivers/media/i2c/adv7180.c:246`
  (`lockdep_assert_held(&state->mutex)`) and the write from
  adv7180_set_power() in `drivers/media/i2c/adv7180.c:286`. Without
  holding the mutex, callers can trip lockdep and risk racy register
  writes.
- Missing lock covered in suspend: The change adds
  `guard(mutex)(&state->mutex);` before powering down in the PM suspend
  callback, ensuring the lockdep requirement is satisfied when calling
  adv7180_set_power(false). In this tree that guard is present at
  `drivers/media/i2c/adv7180.c:1555`, matching the intent of the commit
  you’re evaluating.
- Missing lock covered in resume: Similarly, the resume path takes the
  mutex before calling adv7180_set_power() to restore power state. In
  this tree that guard is present at `drivers/media/i2c/adv7180.c:1566`.
  This avoids unprotected register writes during resume, when
  concurrency is common.
- Missing lock in set_pad_format when changing field: The diff also adds
  the mutex guard around the active-format case where `state->field`
  changes and the code power-cycles the device and calls
  adv7180_set_field_mode(). Since that sequence invokes
  adv7180_set_power(), it must hold the mutex to satisfy the lockdep
  assertion. Protecting this block is consistent with the rest of the
  driver, which already guards other adv7180_set_power() call sites
  (e.g., `drivers/media/i2c/adv7180.c:933`,
  `drivers/media/i2c/adv7180.c:939`, `drivers/media/i2c/adv7180.c:948`).
- Scope and risk: The change is small, localized to the adv7180 driver’s
  state mutex usage, and does not alter interfaces or architecture. It
  enforces an existing locking contract rather than introducing new
  behavior.
- User impact: Without this fix, users can hit lockdep warnings and
  potential races during suspend/resume or when changing the field mode
  through pad format. With the fix, register accesses are serialized as
  intended, preventing subtle resume/configuration issues.
- Stable criteria: It’s a clear bug fix, minimal and self-contained,
  with low regression risk in a single driver. While the commit message
  doesn’t include a Stable tag, it meets stable backport rules
  (correctness fix, no new features).

Note: Older stable trees that lack the `guard(mutex)` helper will need a
trivial adaptation to explicit
`mutex_lock(&state->mutex)`/`mutex_unlock(&state->mutex)` at the same
points.

 drivers/media/i2c/adv7180.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 5d90b8ab9b6df..84600fa75ae8a 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -813,6 +813,8 @@ static int adv7180_set_pad_format(struct v4l2_subdev *sd,
 
 	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
 		if (state->field != format->format.field) {
+			guard(mutex)(&state->mutex);
+
 			state->field = format->format.field;
 			adv7180_set_power(state, false);
 			adv7180_set_field_mode(state);
@@ -1549,6 +1551,8 @@ static int adv7180_suspend(struct device *dev)
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct adv7180_state *state = to_state(sd);
 
+	guard(mutex)(&state->mutex);
+
 	return adv7180_set_power(state, false);
 }
 
@@ -1562,6 +1566,8 @@ static int adv7180_resume(struct device *dev)
 	if (ret < 0)
 		return ret;
 
+	guard(mutex)(&state->mutex);
+
 	ret = adv7180_set_power(state, state->powered);
 	if (ret)
 		return ret;
-- 
2.51.0

[PATCH AUTOSEL 6.17-6.1] media: verisilicon: Explicitly disable selection api ioctls for decoders

[Sasha Levin]

From: Paul Kocialkowski <paulk@sys-base.io>

[ Upstream commit 73d50aa92f28ee8414fbfde011974fce970b82cc ]

Call the dedicated v4l2_disable_ioctl helper instead of manually
checking whether the current context is an encoder for the selection
api ioctls.

Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.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:

YES

**Why This Fix Matters**
- Correctly hides unsupported ioctls on decoder nodes: Previously, the
  driver exposed `VIDIOC_G_SELECTION`/`VIDIOC_S_SELECTION` to decoders
  but rejected them at runtime with `-EINVAL`. This incorrectly
  advertised capability and confused userspace and core heuristics. With
  this change, those ioctls are explicitly disabled and return
  `-ENOTTY`, which is the correct “not supported” error and matches V4L2
  expectations (drivers/media/v4l2-core/v4l2-ioctl.c:3073, 3111).
- Avoids misleading legacy crop exposure: V4L2 core auto-enables legacy
  crop ioctls if selection is available. Disabling selection for
  decoders prevents the core from enabling `VIDIOC_G_CROP/CROPCAP` on
  decoder nodes (drivers/media/v4l2-core/v4l2-dev.c:657,
  drivers/media/v4l2-core/v4l2-dev.c:659,
  drivers/media/v4l2-core/v4l2-dev.c:662,
  drivers/media/v4l2-core/v4l2-dev.c:663,
  drivers/media/v4l2-core/v4l2-dev.c:664,
  drivers/media/v4l2-core/v4l2-dev.c:665). This fixes a user-visible API
  correctness issue.

**Change Details**
- Disables selection ioctls for decoder device nodes using the standard
  helper, before registration:
  - `v4l2_disable_ioctl(vfd, VIDIOC_G_SELECTION);`
  - `v4l2_disable_ioctl(vfd, VIDIOC_S_SELECTION);`
  - Location: drivers/media/platform/verisilicon/hantro_drv.c:918,
    drivers/media/platform/verisilicon/hantro_drv.c:919
  - Called before `video_register_device`, as required
    (drivers/media/platform/verisilicon/hantro_drv.c:924).
- Simplifies selection handlers to only enforce buffer type, removing
  runtime checks on context role:
  - Dropped `!ctx->is_encoder` checks; now only `sel->type !=
    V4L2_BUF_TYPE_VIDEO_OUTPUT` is validated.
  - `vidioc_g_selection`:
    drivers/media/platform/verisilicon/hantro_v4l2.c:666–667
  - `vidioc_s_selection`:
    drivers/media/platform/verisilicon/hantro_v4l2.c:698–699
  - Effect: No functional change for encoders (where `ctx->is_encoder`
    is always true), and decoders won’t reach these handlers since the
    ioctls are disabled.

**Risk and Side Effects**
- Behavior change is limited to decoders for selection ioctls: return
  code changes from `-EINVAL` to `-ENOTTY` via core gating
  (`is_valid_ioctl()` fails, `ret` remains `-ENOTTY`,
  drivers/media/v4l2-core/v4l2-ioctl.c:3073, 3111–3113). This is the
  correct semantics for “unsupported ioctl” and improves userspace
  detection.
- No architectural changes; confined to the Verisilicon Hantro driver.
  Encoder behavior is unchanged.
- Very small, contained patch; unlikely to introduce regressions. Aligns
  with common media driver practice of disabling non-applicable ioctls
  for a given node.

**Stable Backport Fit**
- Fixes a user-visible API bug (misadvertised capability and wrong
  errno) with minimal, localized changes.
- No new features or interfaces; follows stable rules for correctness
  fixes.
- Touches a non-core subsystem (media, platform driver), minimizing
  cross-subsystem risk.

Given the above, this commit is a good candidate for stable backporting.

 drivers/media/platform/verisilicon/hantro_drv.c  | 2 ++
 drivers/media/platform/verisilicon/hantro_v4l2.c | 6 ++----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index fa972effd4a2c..9d5e50fedae1f 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -917,6 +917,8 @@ static int hantro_add_func(struct hantro_dev *vpu, unsigned int funcid)
 		vpu->decoder = func;
 		v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD);
 		v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD);
+		v4l2_disable_ioctl(vfd, VIDIOC_G_SELECTION);
+		v4l2_disable_ioctl(vfd, VIDIOC_S_SELECTION);
 	}
 
 	video_set_drvdata(vfd, vpu);
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index 7c3515cf7d64a..4598f9b4bd21c 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -663,8 +663,7 @@ static int vidioc_g_selection(struct file *file, void *priv,
 	struct hantro_ctx *ctx = fh_to_ctx(priv);
 
 	/* Crop only supported on source. */
-	if (!ctx->is_encoder ||
-	    sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+	if (sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
 	switch (sel->target) {
@@ -696,8 +695,7 @@ static int vidioc_s_selection(struct file *file, void *priv,
 	struct vb2_queue *vq;
 
 	/* Crop only supported on source. */
-	if (!ctx->is_encoder ||
-	    sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+	if (sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
 	/* Change not allowed if the queue is streaming. */
-- 
2.51.0

[PATCH AUTOSEL 6.17-6.12] media: pci: mgb4: Fix timings comparison in VIDIOC_S_DV_TIMINGS

[Sasha Levin]

From: Martin Tůma <martin.tuma@digiteqautomotive.com>

[ Upstream commit 0750649b528ff18d1d68aecb45b34ec22d5ab778 ]

Compare the whole v4l2_bt_timings struct, not just the width/height when
setting new timings. Timings with the same resolution and different
pixelclock can now be properly set.

Signed-off-by: Martin Tůma <martin.tuma@digiteqautomotive.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:

YES

- What it fixes: Prevents a real functional bug where setting DV timings
  with the same width/height but different pixelclock (or other BT
  fields) was treated as “no change” and ignored. This blocked valid
  timing changes (e.g., framerate changes at the same resolution).
- Code change: Replaces a narrow equality check on resolution with a
  full DV timings comparison:
  - New check: `v4l2_match_dv_timings(timings, &vindev->timings, 0,
    false)` in `drivers/media/pci/mgb4/mgb4_vin.c:613`.
  - Old behavior (implicit from the diff): only compared `width` and
    `height`, causing false “match” for differing
    pixelclock/porches/polarities/etc.
- Correct behavior when busy: With the fix, if the queue is streaming
  and the requested timings differ in any BT field,
  `vidioc_s_dv_timings` returns `-EBUSY` instead of silently returning 0
  while not applying the change (see `vb2_is_busy` branch right after
  the match check in `drivers/media/pci/mgb4/mgb4_vin.c:615`).
- Scope and risk: Minimal and contained (one-line logic change in a
  single driver). No API/ABI change, no architectural impact, only
  affects `VIDIOC_S_DV_TIMINGS` behavior in the MGB4 capture driver.
- Uses a proven helper: `v4l2_match_dv_timings` is the standard V4L2
  helper that compares the full `v4l2_bt_timings` including
  width/height, interlaced, polarities, pixelclock (with tolerance),
  porches, vsync, flags, and interlaced-specific fields; see
  implementation at `drivers/media/v4l2-core/v4l2-dv-timings.c:267`.
  This pattern is used across other drivers.
- User impact: Enables setting legitimate timings that share resolution
  but differ in pixelclock (and other BT parameters). Previously such
  requests were incorrectly treated as no-ops.
- Stable criteria fit:
  - Important bugfix affecting real use (DV timings changes ignored).
  - Small, localized change with low regression risk.
  - No new features or interface changes.
  - Touches only a non-core driver
    (`drivers/media/pci/mgb4/mgb4_vin.c`).
- Backport note: Apply to stable kernels that include the MGB4 driver;
  the helper `v4l2_match_dv_timings` is long-standing in V4L2 and does
  not introduce dependencies.

Overall, this is a low-risk, clear bug fix that improves correctness and
user experience when changing DV timings; it should be backported.

 drivers/media/pci/mgb4/mgb4_vin.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/pci/mgb4/mgb4_vin.c b/drivers/media/pci/mgb4/mgb4_vin.c
index 989e93f67f75b..42c327bc50e10 100644
--- a/drivers/media/pci/mgb4/mgb4_vin.c
+++ b/drivers/media/pci/mgb4/mgb4_vin.c
@@ -610,8 +610,7 @@ static int vidioc_s_dv_timings(struct file *file, void *fh,
 	    timings->bt.height < video_timings_cap.bt.min_height ||
 	    timings->bt.height > video_timings_cap.bt.max_height)
 		return -EINVAL;
-	if (timings->bt.width == vindev->timings.bt.width &&
-	    timings->bt.height == vindev->timings.bt.height)
+	if (v4l2_match_dv_timings(timings, &vindev->timings, 0, false))
 		return 0;
 	if (vb2_is_busy(&vindev->queue))
 		return -EBUSY;
-- 
2.51.0

[PATCH AUTOSEL 6.17-6.12] media: ipu6: isys: Set embedded data type correctly for metadata formats

[Sasha Levin]

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

[ Upstream commit f5a2826cd50c6fd1af803812d1d910a64ae8e0a1 ]

The IPU6 ISYS driver supported metadata formats but was missing correct
embedded data type in the receiver configuration. Add it now.

Signed-off-by: Sakari Ailus <sakari.ailus@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:

## **BACKPORT RECOMMENDATION: YES**

After conducting an exhaustive analysis of this commit, I strongly
recommend backporting it to stable kernel trees (v6.10+).

---

## **DETAILED ANALYSIS**

### **The Bug Identified**

This commit fixes a **genuine implementation bug** in the IPU6 ISYS
driver's metadata format support:

1. **Incomplete Feature Implementation**: Commit `d3bd039cd2a00`
   (January 31, 2024) added "line-based metadata capture support" to the
   IPU6 driver in v6.10. However, the implementation was incomplete:
   - The function `ipu6_isys_mbus_code_to_bpp()` at lines 19-52
     correctly handles metadata formats (META_8, META_10, META_12,
     META_16, META_24)
   - The function `ipu6_isys_mbus_code_to_mipi()` at lines 54-89 **does
     not** handle metadata formats

2. **Consequences of the Bug**:
   - When metadata formats are used, `ipu6_isys_mbus_code_to_mipi()`
     falls through to the default case (line 84-87)
   - This triggers `WARN_ON(1)` causing kernel warning messages in dmesg
   - Returns 0x3f (an invalid MIPI data type) instead of the correct
     `MIPI_CSI2_DT_EMBEDDED_8B` (0x12)
   - The invalid data type gets propagated to firmware at
     `drivers/media/pci/intel/ipu6/ipu6-isys-video.c:477` where
     `input_pin->dt = av->dt`
   - Result: **Metadata capture doesn't work correctly** and hardware is
     misconfigured

3. **Evidence of the Bug**:
   - The driver advertises support for metadata formats in CSI2 receiver
     (`ipu6-isys-csi2.c:45-49`)
   - Maps metadata formats to V4L2 pixel formats (`ipu6-isys-
     video.c:88-95`)
   - But fails to provide correct MIPI data type conversion for these
     formats

### **The Fix Evaluation**

**Technical Correctness:**
- Adds 6 case statements for `MEDIA_BUS_FMT_META_*` formats
- Returns `MIPI_CSI2_DT_EMBEDDED_8B` (0x12), which is the **correct MIPI
  CSI-2 data type** per the MIPI CSI-2 specification
  (`include/media/mipi-csi2.h:21`)
- Aligns with standard V4L2/media subsystem conventions for embedded
  data

**Code Changes Analysis:**
```c
// Added lines 85-90:
case MEDIA_BUS_FMT_META_8:
case MEDIA_BUS_FMT_META_10:
case MEDIA_BUS_FMT_META_12:
case MEDIA_BUS_FMT_META_16:
case MEDIA_BUS_FMT_META_24:
    return MIPI_CSI2_DT_EMBEDDED_8B;
```

**Risk Assessment: VERY LOW**
1. **Minimal Scope**: Only 6 lines added to a switch statement
2. **No Regression Risk**: Only affects formats that were **completely
   broken** before (triggering WARN_ON and returning invalid data type)
3. **Self-Contained**: No dependencies, no side effects on existing
   working formats (RGB, YUV, Bayer patterns)
4. **Trivial to Verify**: Basic functional testing would immediately
   confirm correctness

### **Stable Tree Rules Compliance**

Evaluating against Documentation/process/stable-kernel-rules.rst:

✅ **Fixes an important bug**: Metadata capture is advertised but doesn't
work
✅ **Small and obviously correct**: 6 lines, trivial logic
✅ **No complex dependencies**: Self-contained change
✅ **Not theoretical**: Real bug with observable symptoms (WARN_ON,
hardware misconfiguration)
✅ **Affects stable kernels**: Bug present since v6.10

### **Impact Assessment**

**Who is Affected:**
- Users of Intel IPU6 hardware (Tiger Lake, Alder Lake, and newer
  platforms)
- Applications requiring metadata capture (e.g., camera statistics,
  embedded data from sensors)
- Professional camera software and development tools

**Why Low User Reports:**
- **Specialized Feature**: Metadata capture is a niche feature used
  mainly by professional camera applications
- **Recent Addition**: Feature added in v6.10 (2024), relatively new
- **Hardware Availability**: IPU6 hardware is in newer Intel platforms
- **Professional Users**: May have workarounds or simply disabled
  metadata capture

**Note**: Absence of user bug reports does **NOT** mean the bug should
be ignored. The driver claims to support a feature that is fundamentally
broken.

### **Research Findings**

Using the search-specialist agent, I found:
- **No specific user bug reports** for this WARN_ON issue
- **No CVE or security implications**
- **No related fixes or reverts** in the codebase
- The code clearly shows the bug exists and the fix is correct

### **Comparison with Similar Commits**

This commit is similar to other stable-worthy commits that:
- Fix incomplete driver implementations
- Have minimal code changes
- Fix bugs in specialized features
- Prevent kernel warnings (WARN_ON)
- Correct hardware configuration issues

---

## **FINAL RECOMMENDATION**

**YES - This commit SHOULD be backported to stable kernel trees
(v6.10+)**

**Justification:**
1. Fixes a **real, verifiable bug** with observable symptoms
2. Extremely **low risk** - only affects broken functionality
3. **Simple and contained** - 6 lines in a switch statement
4. Provides **correct hardware configuration** per MIPI CSI-2
   specification
5. Eliminates **kernel warnings** (WARN_ON)
6. Completes the metadata format support that was partially implemented
7. No known side effects or regression risks

The fix is so simple, correct, and low-risk that there is no compelling
reason NOT to backport it. Even if metadata capture is a specialized
feature with few current users, having broken functionality in the
kernel is unacceptable when the fix is this straightforward.

 drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
index 0a06de5c739c7..463a0adf9e131 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
@@ -81,6 +81,12 @@ unsigned int ipu6_isys_mbus_code_to_mipi(u32 code)
 	case MEDIA_BUS_FMT_SGRBG8_1X8:
 	case MEDIA_BUS_FMT_SRGGB8_1X8:
 		return MIPI_CSI2_DT_RAW8;
+	case MEDIA_BUS_FMT_META_8:
+	case MEDIA_BUS_FMT_META_10:
+	case MEDIA_BUS_FMT_META_12:
+	case MEDIA_BUS_FMT_META_16:
+	case MEDIA_BUS_FMT_META_24:
+		return MIPI_CSI2_DT_EMBEDDED_8B;
 	default:
 		/* return unavailable MIPI data type - 0x3f */
 		WARN_ON(1);
-- 
2.51.0

[PATCH AUTOSEL 6.17-6.1] media: i2c: og01a1b: Specify monochrome media bus format instead of Bayer

[Sasha Levin]

From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

[ Upstream commit bfbd5aa5347fbd11ade188b316b800bfb27d9e22 ]

The OmniVision OG01A1B image sensor is a monochrome sensor, it supports
8-bit and 10-bit RAW output formats only.

That said the planar greyscale Y8/Y10 media formats are more appropriate
for the sensor instead of the originally and arbitrary selected SGRBG one,
since there is no red, green or blue color components.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Signed-off-by: Sakari Ailus <sakari.ailus@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:

YES

- Problem fixed: The sensor is monochrome, but the driver advertised a
  Bayer RAW10 bus code. This is incorrect and can cause wrong pipeline
  negotiation and processing (e.g., debayering on a non-CFA stream).
- Precise fix: Three small, contained changes switch the media bus code
  from Bayer to monochrome:
  - Default pad format: `fmt->code = MEDIA_BUS_FMT_Y10_1X10;` in
    `og01a1b_update_pad_format` (drivers/media/i2c/og01a1b.c:685).
  - Advertised code enumeration: `code->code = MEDIA_BUS_FMT_Y10_1X10;`
    in `og01a1b_enum_mbus_code` (drivers/media/i2c/og01a1b.c:827).
  - Frame-size enumeration input check: `fse->code !=
    MEDIA_BUS_FMT_Y10_1X10` (drivers/media/i2c/og01a1b.c:839).
- Scope and risk: The changes are limited to format reporting in this
  sensor subdev; no register programming, timing, or streaming logic is
  touched. The on-wire MIPI CSI-2 data type remains RAW10; this is a
  representational fix, not a hardware/protocol change.
- User impact: Correctly reporting monochrome Y10 avoids erroneous color
  processing and enables proper link validation with components that
  expect grayscale formats. Many bridge/ISP drivers already support
  `MEDIA_BUS_FMT_Y10_1X10`, minimizing regression risk.
- Compatibility: The driver’s single supported mode is 10-bit, and the
  code already reflects 10-bit depth, so Y10 is consistent with current
  capabilities. There’s no API/ABI change visible to V4L2 capture nodes;
  this is subdev pad-format metadata.
- Stable criteria alignment:
  - Important bug fix (wrong format reported for a monochrome sensor).
  - Small, surgical change (three lines).
  - No architectural or cross-subsystem churn.
  - Low regression risk; aligns with established handling of monochrome
    sensors.
  - Confined to one i2c/media driver file
    (`drivers/media/i2c/og01a1b.c`).

Given the correctness improvement, minimal scope, and low risk, this is
a good candidate for stable backport.

 drivers/media/i2c/og01a1b.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/og01a1b.c b/drivers/media/i2c/og01a1b.c
index 78d5d406e4b72..b7d0b677975d5 100644
--- a/drivers/media/i2c/og01a1b.c
+++ b/drivers/media/i2c/og01a1b.c
@@ -682,7 +682,7 @@ static void og01a1b_update_pad_format(const struct og01a1b_mode *mode,
 {
 	fmt->width = mode->width;
 	fmt->height = mode->height;
-	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+	fmt->code = MEDIA_BUS_FMT_Y10_1X10;
 	fmt->field = V4L2_FIELD_NONE;
 }
 
@@ -828,7 +828,7 @@ static int og01a1b_enum_mbus_code(struct v4l2_subdev *sd,
 	if (code->index > 0)
 		return -EINVAL;
 
-	code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+	code->code = MEDIA_BUS_FMT_Y10_1X10;
 
 	return 0;
 }
@@ -840,7 +840,7 @@ static int og01a1b_enum_frame_size(struct v4l2_subdev *sd,
 	if (fse->index >= ARRAY_SIZE(supported_modes))
 		return -EINVAL;
 
-	if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10)
+	if (fse->code != MEDIA_BUS_FMT_Y10_1X10)
 		return -EINVAL;
 
 	fse->min_width = supported_modes[fse->index].width;
-- 
2.51.0

[PATCH AUTOSEL 6.17-6.1] media: amphion: Delete v4l2_fh synchronously in .release()

[Sasha Levin]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

[ Upstream commit 19fb9c5b815f70eb90d5b545f65b83bc9c490ecd ]

The v4l2_fh initialized and added in vpu_v4l2_open() is delete and
cleaned up when the last reference to the vpu_inst is released. This may
happen later than at vpu_v4l2_close() time.

Not deleting and cleaning up the v4l2_fh when closing the file handle to
the video device is not ideal, as the v4l2_fh will still be present in
the video device's fh_list, and will store a copy of events queued to
the video device. There may also be other side effects of keeping alive
an object that represents an open file handle after the file handle is
closed.

The v4l2_fh instance is embedded in the vpu_inst structure, and is
accessed in two different ways:

- in vpu_notify_eos() and vpu_notify_source_change(), to queue V4L2
  events to the file handle ; and

- through the driver to access the v4l2_fh.m2m_ctx pointer.

The v4l2_fh.m2m_ctx pointer is not touched by v4l2_fh_del() and
v4l2_fh_exit(). It is set to NULL by the driver when closing the file
handle, in vpu_v4l2_close().

The vpu_notify_eos() and vpu_notify_source_change() functions are called
in vpu_set_last_buffer_dequeued() and vdec_handle_resolution_change()
respectively, only if the v4l2_fh.m2m_ctx pointer is not NULL. There is
therefore a guarantee that no new event will be queued to the v4l2_fh
after vpu_v4l2_close() destroys the m2m_ctx.

The vpu_notify_eos() function is also called from vpu_vb2_buf_finish(),
which is guaranteed to be called for all queued buffers when
vpu_v4l2_close() calls v4l2_m2m_ctx_release(), and will not be called
later.

It is therefore safe to assume that the driver will not touch the
v4l2_fh, except to check the m2m_ctx pointer, after vpu_v4l2_close()
destroys the m2m_ctx. We can safely delete and cleanup the v4l2_fh
synchronously in vpu_v4l2_close().

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Ming Qian <ming.qian@oss.nxp.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:

YES

- What it fixes
  - Today the driver deletes the `v4l2_fh` only when the last reference
    to the instance is put, not when the file is closed. In current
    code, `v4l2_fh_del()` and `v4l2_fh_exit()` are called from the
    instance release path `vpu_v4l2_release()`
    (drivers/media/platform/amphion/vpu_v4l2.c:712-713), not from the
    file `.release()` (close) path
    (drivers/media/platform/amphion/vpu_v4l2.c:786-804).
  - That means after a user closes the device file, the `v4l2_fh` object
    still remains on the `video_device`’s `fh_list` and can accumulate
    device events until the last kref is dropped. This is undesirable
    (stale file-handle state, latent memory consumption and confusing
    semantics), and it also diverges from how most V4L2 mem2mem drivers
    behave and from the documented expectation that `v4l2_fh_del/exit`
    are called in the v4l2 file `.release()`.

- What changes and why it’s safe
  - The patch moves `v4l2_fh_del()`/`v4l2_fh_exit()` out of the instance
    release path and into the file `.release()` (`vpu_v4l2_close()`),
    and also adds them to the open error path:
    - Remove from instance release: currently called in
      `vpu_v4l2_release()` at
      drivers/media/platform/amphion/vpu_v4l2.c:712-713.
    - Add to close path: after the driver releases the mem2mem context
      in `vpu_v4l2_close()`
      (drivers/media/platform/amphion/vpu_v4l2.c:793-799), it will now
      call `v4l2_fh_del()`/`v4l2_fh_exit()` and only then proceed to
      unregister and put the instance.
    - Add to the open error label: currently the `error:` path lacks
      `v4l2_fh_del/exit`
      (drivers/media/platform/amphion/vpu_v4l2.c:781-783); the patch
      adds them there to avoid leaving an fh briefly on the device list
      after a failed open.
  - Safety argument (from code):
    - After close, `vpu_v4l2_close()` already destroys the mem2mem
      context (`v4l2_m2m_ctx_release`) before anything else of interest
      (drivers/media/platform/amphion/vpu_v4l2.c:793-799). This is
      critical: it ensures the driver no longer queues new events to the
      `v4l2_fh`.
    - Calls that queue events check `m2m_ctx` first:
      - `vpu_set_last_buffer_dequeued()` returns if `inst->fh.m2m_ctx ==
        NULL` (drivers/media/platform/amphion/vpu_v4l2.c:110).
      - The decoder’s resolution-change path
        (`vdec_handle_resolution_change()`) also returns early if
        `inst->fh.m2m_ctx == NULL`
        (drivers/media/platform/amphion/vdec.c:357-366) before calling
        `vpu_notify_source_change()`.
      - `vpu_vb2_buf_finish()` may call `vpu_notify_eos(inst)`, but
        buffer-finish callbacks are guaranteed to flush during
        `v4l2_m2m_ctx_release()` and not after it returns, so there are
        no post-close event queues to an already exited `fh`.
    - With `m2m_ctx` destroyed first, no code path will call
      `v4l2_event_queue_fh()` after `v4l2_fh_exit()` sets `fh->vdev =
      NULL`. This avoids the risk of dereferencing a NULL `fh->vdev` in
      the core event code (see
      drivers/media/v4l2-core/v4l2-event.c:173-179 and
      drivers/media/v4l2-core/v4l2-fh.c:87-114).

- Why this is a good stable backport
  - Bug fix that affects users: prevents stale `fh` objects from staying
    on the device’s `fh_list` after close, which can accumulate events
    and resources and misrepresent the state of “open” file handles.
  - Small and contained: only changes
    `drivers/media/platform/amphion/vpu_v4l2.c`, moving two calls and
    adding them to an error path. No API or architectural changes.
  - Aligns with V4L2 expectations and common driver practice: many V4L2
    mem2mem drivers delete and exit the `v4l2_fh` in their file
    `.release()`; the V4L2 API documentation for `v4l2_fh_del/exit`
    indicates they should be called in the `.release()` handler (see
    include/media/v4l2-fh.h).
  - Low regression risk: the mem2mem context is released at close time
    already (drivers/media/platform/amphion/vpu_v4l2.c:793-799), and all
    event-queuing paths are guarded by `m2m_ctx != NULL`, ensuring no
    events are queued after `fh` is deleted/exited.
  - Extra robustness: adding `v4l2_fh_del/exit` to the open error path
    ensures no transient fhs linger on the device list if open fails
    after `v4l2_fh_add`.

- Preconditions for backporting
  - Ensure the target stable branch matches the current behavior where
    `vpu_v4l2_close()` releases `inst->fh.m2m_ctx`
    (drivers/media/platform/amphion/vpu_v4l2.c:793-799). Earlier
    versions temporarily released `m2m_ctx` in the instance release
    path; this patch’s safety relies on doing it in `.close()`. If the
    branch still releases `m2m_ctx` in the instance release routine,
    this patch should be combined with or preceded by the change that
    moves `m2m_ctx` release into `.close()`.

Given the above, this change is a clear, minimal correctness fix with
low risk and should be backported.

 drivers/media/platform/amphion/vpu_v4l2.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/amphion/vpu_v4l2.c b/drivers/media/platform/amphion/vpu_v4l2.c
index 74668fa362e24..1c3740baf6942 100644
--- a/drivers/media/platform/amphion/vpu_v4l2.c
+++ b/drivers/media/platform/amphion/vpu_v4l2.c
@@ -718,8 +718,6 @@ static int vpu_v4l2_release(struct vpu_inst *inst)
 
 	v4l2_ctrl_handler_free(&inst->ctrl_handler);
 	mutex_destroy(&inst->lock);
-	v4l2_fh_del(&inst->fh);
-	v4l2_fh_exit(&inst->fh);
 
 	call_void_vop(inst, cleanup);
 
@@ -788,6 +786,8 @@ int vpu_v4l2_open(struct file *file, struct vpu_inst *inst)
 
 	return 0;
 error:
+	v4l2_fh_del(&inst->fh);
+	v4l2_fh_exit(&inst->fh);
 	vpu_inst_put(inst);
 	return ret;
 }
@@ -807,6 +807,9 @@ int vpu_v4l2_close(struct file *file)
 	call_void_vop(inst, release);
 	vpu_inst_unlock(inst);
 
+	v4l2_fh_del(&inst->fh);
+	v4l2_fh_exit(&inst->fh);
+
 	vpu_inst_unregister(inst);
 	vpu_inst_put(inst);
 
-- 
2.51.0