[PATCH 0/3] media: hws: fix shared IRQ and video quiesce handling

[PATCH 0/3] media: hws: fix shared IRQ and video quiesce handling

[Ben Hoff]

HWS quiescence currently disables a shared IRQ descriptor and calls
vb2_streamoff() without the queue's state mutex. This can prevent other
devices on the interrupt line from being serviced and race userspace
capture operations during suspend or shutdown.

Remove diagnostic register accesses first, then restrict IRQ quiescence
to the HWS function and serialize monitor and queue transitions. Runtime
readiness checks report an unready core without resetting shared hardware
while capture buffers may still be owned by another channel.

Based on media-next/next at b38d06ad1e13 ("media: ipu6: Fix up missing
IWYU issue"). Apply the three patches in order.

AI assistance: OpenAI Codex using GPT-6 assisted with this submission.
Each patch includes Assisted-by: Codex:GPT-6 to identify the agent and model.

Validation:
- Applied and built every patch boundary as an external module with W=1
  against the prepared x86_64 kernel, with CONFIG_PM_SLEEP=y.
- Strict checkpatch and git diff --check pass.
- Hardware capture and system-sleep tests have not been run for this
  extracted series.

Ben Hoff (3):
  media: hws: remove debug controls and lifecycle tracing
  media: hws: quiesce interrupts without disabling shared IRQ
  media: hws: serialize video quiesce with queue state

 drivers/media/pci/hws/hws.h            |   2 +
 drivers/media/pci/hws/hws_irq.c        |  84 +-----------
 drivers/media/pci/hws/hws_pci.c        | 181 ++++---------------------
 drivers/media/pci/hws/hws_v4l2_ioctl.c |  14 --
 drivers/media/pci/hws/hws_video.c      | 144 ++------------------
 5 files changed, 44 insertions(+), 381 deletions(-)


base-commit: b38d06ad1e13c32970d58039c154f009b81a3368

[PATCH 1/3] media: hws: remove debug controls and lifecycle tracing

[Ben Hoff]

Remove the toggle_debug and dma_window_verify module parameters, register
snapshots, and routine capture and lifecycle messages. These diagnostics
add register reads to interrupt and format paths, including IRQ debug
reads before the suspended-state check.

Keep error reporting and the posted-write flush needed before arming DMA.
This also removes diagnostic MMIO accesses that would otherwise bypass
the suspended-handler guard in the following shared-IRQ fix.

Assisted-by: Codex:GPT-6
Signed-off-by: Ben Hoff <hoff.benjamin.k@gmail.com>
---
 drivers/media/pci/hws/hws_irq.c        |  74 -------------
 drivers/media/pci/hws/hws_pci.c        | 145 +------------------------
 drivers/media/pci/hws/hws_v4l2_ioctl.c |  14 ---
 drivers/media/pci/hws/hws_video.c      | 125 +--------------------
 4 files changed, 4 insertions(+), 354 deletions(-)

diff --git a/drivers/media/pci/hws/hws_irq.c b/drivers/media/pci/hws/hws_irq.c
index eebb4b8a5cd5..787c9e498799 100644
--- a/drivers/media/pci/hws/hws_irq.c
+++ b/drivers/media/pci/hws/hws_irq.c
@@ -16,38 +16,23 @@
 
 #define MAX_INT_LOOPS 100
 
-static bool hws_toggle_debug;
-module_param_named(toggle_debug, hws_toggle_debug, bool, 0644);
-MODULE_PARM_DESC(toggle_debug,
-		 "Read toggle registers in IRQ handler for debug logging");
-
 static int hws_arm_next(struct hws_pcie_dev *hws, u32 ch)
 {
 	struct hws_video *v = &hws->video[ch];
 	unsigned long flags;
 	struct hwsvideo_buffer *buf;
 
-	dev_dbg(&hws->pdev->dev,
-		"arm_next(ch=%u): stop=%d cap=%d queued=%d\n",
-		ch, READ_ONCE(v->stop_requested), READ_ONCE(v->cap_active),
-		!list_empty(&v->capture_queue));
-
 	if (READ_ONCE(hws->suspended)) {
-		dev_dbg(&hws->pdev->dev, "arm_next(ch=%u): suspended\n", ch);
 		return -EBUSY;
 	}
 
 	if (READ_ONCE(v->stop_requested) || !READ_ONCE(v->cap_active)) {
-		dev_dbg(&hws->pdev->dev,
-			"arm_next(ch=%u): stop=%d cap=%d -> cancel\n", ch,
-			v->stop_requested, v->cap_active);
 		return -ECANCELED;
 	}
 
 	spin_lock_irqsave(&v->irq_lock, flags);
 	if (list_empty(&v->capture_queue)) {
 		spin_unlock_irqrestore(&v->irq_lock, flags);
-		dev_dbg(&hws->pdev->dev, "arm_next(ch=%u): queue empty\n", ch);
 		return -EAGAIN;
 	}
 
@@ -57,8 +42,6 @@ static int hws_arm_next(struct hws_pcie_dev *hws, u32 ch)
 		v->queued_count--;
 	v->active = buf;
 	spin_unlock_irqrestore(&v->irq_lock, flags);
-	dev_dbg(&hws->pdev->dev, "arm_next(ch=%u): picked buffer %p\n", ch,
-		buf);
 
 	/* Publish descriptor(s) before doorbell/MMIO kicks. */
 	wmb();
@@ -67,8 +50,6 @@ static int hws_arm_next(struct hws_pcie_dev *hws, u32 ch)
 	if (READ_ONCE(hws->suspended)) {
 		unsigned long f;
 
-		dev_dbg(&hws->pdev->dev,
-			"arm_next(ch=%u): suspended after pick\n", ch);
 		spin_lock_irqsave(&v->irq_lock, f);
 		if (v->active) {
 			list_add(&buf->list, &v->capture_queue);
@@ -88,8 +69,6 @@ static int hws_arm_next(struct hws_pcie_dev *hws, u32 ch)
 			  hws->bar0_base + HWS_REG_DMA_ADDR(ch));
 	}
 
-	dev_dbg(&hws->pdev->dev, "arm_next(ch=%u): programmed buffer %p\n", ch,
-		buf);
 	spin_lock_irqsave(&v->irq_lock, flags);
 	hws_prime_next_locked(v);
 	spin_unlock_irqrestore(&v->irq_lock, flags);
@@ -104,16 +83,8 @@ static void hws_video_handle_vdone(struct hws_video *v)
 	unsigned long flags;
 	bool promoted = false;
 
-	dev_dbg(&hws->pdev->dev,
-		"bh_video(ch=%u): stop=%d cap=%d active=%p\n",
-		ch, READ_ONCE(v->stop_requested), READ_ONCE(v->cap_active),
-		v->active);
-
 	int ret;
 
-	dev_dbg(&hws->pdev->dev,
-		"bh_video(ch=%u): entry stop=%d cap=%d\n", ch,
-		v->stop_requested, v->cap_active);
 	if (READ_ONCE(hws->suspended))
 		return;
 
@@ -149,10 +120,6 @@ static void hws_video_handle_vdone(struct hws_video *v)
 
 		vb2v->sequence = (u32)atomic_inc_return(&v->sequence_number);
 		vb2v->vb2_buf.timestamp = ktime_get_ns();
-		dev_dbg(&hws->pdev->dev,
-			"bh_video(ch=%u): DONE buf=%p seq=%u half_seen=%d toggle=%u\n",
-			ch, done, vb2v->sequence, v->half_seen,
-			v->last_buf_half_toggle);
 
 		if (!promoted)
 			v->active = NULL;	/* channel no longer owns this buffer */
@@ -163,9 +130,6 @@ static void hws_video_handle_vdone(struct hws_video *v)
 		return;
 
 	if (promoted) {
-		dev_dbg(&hws->pdev->dev,
-			"bh_video(ch=%u): promoted pre-armed buffer active=%p\n",
-			ch, v->active);
 		spin_lock_irqsave(&v->irq_lock, flags);
 		hws_prime_next_locked(v);
 		spin_unlock_irqrestore(&v->irq_lock, flags);
@@ -176,13 +140,8 @@ static void hws_video_handle_vdone(struct hws_video *v)
 	/* 2) Immediately arm the next queued buffer (if present) */
 	ret = hws_arm_next(hws, ch);
 	if (ret == -EAGAIN) {
-		dev_dbg(&hws->pdev->dev,
-			"bh_video(ch=%u): no queued buffer to arm\n", ch);
 		return;
 	}
-	dev_dbg(&hws->pdev->dev,
-		"bh_video(ch=%u): armed next buffer, active=%p\n", ch,
-		v->active);
 	/* On success the engine now points at v->active's DMA address */
 }
 
@@ -191,14 +150,6 @@ irqreturn_t hws_irq_handler(int irq, void *info)
 	struct hws_pcie_dev *pdx = info;
 	u32 int_state;
 
-	dev_dbg(&pdx->pdev->dev, "irq: entry\n");
-	if (pdx->bar0_base) {
-		dev_dbg(&pdx->pdev->dev,
-			"irq: INT_EN=0x%08x INT_STATUS=0x%08x\n",
-			readl(pdx->bar0_base + INT_EN_REG_BASE),
-			readl(pdx->bar0_base + HWS_REG_INT_STATUS));
-	}
-
 	/* Fast path: if suspended, quietly ack and exit */
 	if (READ_ONCE(pdx->suspended)) {
 		int_state = readl_relaxed(pdx->bar0_base + HWS_REG_INT_STATUS);
@@ -210,12 +161,8 @@ irqreturn_t hws_irq_handler(int irq, void *info)
 	}
 	int_state = readl_relaxed(pdx->bar0_base + HWS_REG_INT_STATUS);
 	if (!int_state || int_state == 0xFFFFFFFF) {
-		dev_dbg(&pdx->pdev->dev,
-			"irq: spurious or device-gone int_state=0x%08x\n",
-			int_state);
 		return IRQ_NONE;
 	}
-	dev_dbg(&pdx->pdev->dev, "irq: entry INT_STATUS=0x%08x\n", int_state);
 
 	/* Loop until all pending bits are serviced (max 100 iterations) */
 	for (u32 cnt = 0; int_state && cnt < MAX_INT_LOOPS; ++cnt) {
@@ -227,27 +174,9 @@ irqreturn_t hws_irq_handler(int irq, void *info)
 
 			if (READ_ONCE(pdx->video[ch].cap_active) &&
 			    !READ_ONCE(pdx->video[ch].stop_requested)) {
-				if (hws_toggle_debug) {
-					u32 toggle =
-					    readl_relaxed(pdx->bar0_base +
-						  HWS_REG_VBUF_TOGGLE(ch)) & 0x01;
-					WRITE_ONCE(pdx->video[ch].last_buf_half_toggle,
-						   toggle);
-				}
 				dma_rmb();
 				WRITE_ONCE(pdx->video[ch].half_seen, true);
-				dev_dbg(&pdx->pdev->dev,
-					"irq: VDONE ch=%u toggle=%u handling inline (cap=%d)\n",
-					ch,
-					READ_ONCE(pdx->video[ch].last_buf_half_toggle),
-					READ_ONCE(pdx->video[ch].cap_active));
 				hws_video_handle_vdone(&pdx->video[ch]);
-			} else {
-				dev_dbg(&pdx->pdev->dev,
-					"irq: VDONE ch=%u ignored (cap=%d stop=%d)\n",
-					ch,
-					READ_ONCE(pdx->video[ch].cap_active),
-					READ_ONCE(pdx->video[ch].stop_requested));
 			}
 
 			writel(vbit, pdx->bar0_base + HWS_REG_INT_STATUS);
@@ -256,9 +185,6 @@ irqreturn_t hws_irq_handler(int irq, void *info)
 
 		/* Re-read in case new interrupt bits popped while processing */
 		int_state = readl_relaxed(pdx->bar0_base + HWS_REG_INT_STATUS);
-		dev_dbg(&pdx->pdev->dev,
-			"irq: loop cnt=%u new INT_STATUS=0x%08x\n", cnt,
-			int_state);
 		if (cnt + 1 == MAX_INT_LOOPS)
 			dev_warn_ratelimited(&pdx->pdev->dev,
 					     "IRQ storm? status=0x%08x\n",
diff --git a/drivers/media/pci/hws/hws_pci.c b/drivers/media/pci/hws/hws_pci.c
index f06e60dc2ee6..65b32fac6d1a 100644
--- a/drivers/media/pci/hws/hws_pci.c
+++ b/drivers/media/pci/hws/hws_pci.c
@@ -27,11 +27,6 @@
 #define HWS_BUSY_POLL_DELAY_US 10
 #define HWS_BUSY_POLL_TIMEOUT_US 1000000
 
-static unsigned long long hws_elapsed_us(u64 start_ns)
-{
-	return div_u64(ktime_get_mono_fast_ns() - start_ns, 1000);
-}
-
 /* register layout inside HWS_REG_DEVICE_INFO */
 #define DEVINFO_VER GENMASK(15, 8)
 #define DEVINFO_SUBVER GENMASK(23, 16)
@@ -139,38 +134,6 @@ static void hws_configure_hardware_capabilities(struct hws_pcie_dev *hdev)
 
 static void hws_stop_device(struct hws_pcie_dev *hws);
 
-static void hws_log_lifecycle_snapshot(struct hws_pcie_dev *hws,
-				       const char *action,
-				       const char *phase)
-{
-	struct device *dev;
-	u32 int_en, int_status, vcap, sys_status, dec_mode;
-
-	if (!hws || !hws->pdev)
-		return;
-
-	dev = &hws->pdev->dev;
-	if (!hws->bar0_base) {
-		dev_dbg(dev,
-			"lifecycle:%s:%s bar0-unmapped suspended=%d start_run=%d pci_lost=%d irq=%d\n",
-			action, phase, READ_ONCE(hws->suspended), hws->start_run,
-			hws->pci_lost, hws->irq);
-		return;
-	}
-
-	int_en = readl(hws->bar0_base + INT_EN_REG_BASE);
-	int_status = readl(hws->bar0_base + HWS_REG_INT_STATUS);
-	vcap = readl(hws->bar0_base + HWS_REG_VCAP_ENABLE);
-	sys_status = readl(hws->bar0_base + HWS_REG_SYS_STATUS);
-	dec_mode = readl(hws->bar0_base + HWS_REG_DEC_MODE);
-
-	dev_dbg(dev,
-		"lifecycle:%s:%s suspended=%d start_run=%d pci_lost=%d irq=%d INT_EN=0x%08x INT_STATUS=0x%08x VCAP=0x%08x SYS=0x%08x DEC=0x%08x\n",
-		action, phase, READ_ONCE(hws->suspended), hws->start_run,
-		hws->pci_lost, hws->irq, int_en, int_status, vcap,
-		sys_status, dec_mode);
-}
-
 static int read_chip_id(struct hws_pcie_dev *hdev)
 {
 	u32 reg;
@@ -197,11 +160,6 @@ static int read_chip_id(struct hws_pcie_dev *hdev)
 
 	hws_configure_hardware_capabilities(hdev);
 
-	dev_info(&hdev->pdev->dev,
-		 "chip detected: ver=%u subver=%u port=%u yv12=%u\n",
-		 hdev->device_ver, hdev->sub_ver, hdev->port_id,
-		 hdev->support_yv12);
-
 	return 0;
 }
 
@@ -228,7 +186,6 @@ static int main_ks_thread_handle(void *data)
 		schedule_timeout_interruptible(msecs_to_jiffies(1000));
 	}
 
-	dev_dbg(&pdx->pdev->dev, "%s: exiting\n", __func__);
 	return 0;
 }
 
@@ -236,22 +193,14 @@ static void hws_stop_kthread_action(void *data)
 {
 	struct hws_pcie_dev *hws = data;
 	struct task_struct *t;
-	u64 start_ns;
 
 	if (!hws)
 		return;
 
 	t = READ_ONCE(hws->main_task);
 	if (!IS_ERR_OR_NULL(t)) {
-		start_ns = ktime_get_mono_fast_ns();
-		dev_dbg(&hws->pdev->dev,
-			"lifecycle:kthread-stop:begin task=%s[%d]\n",
-			t->comm, t->pid);
 		WRITE_ONCE(hws->main_task, NULL);
 		kthread_stop(t);
-		dev_dbg(&hws->pdev->dev,
-			"lifecycle:kthread-stop:done (%lluus)\n",
-			hws_elapsed_us(start_ns));
 	}
 }
 
@@ -424,8 +373,6 @@ static int hws_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
 		if (ret)
 			return dev_err_probe(&pdev->dev, ret,
 					     "No suitable DMA configuration\n");
-	} else {
-		dev_dbg(&pdev->dev, "Using 64-bit DMA mask\n");
 	}
 
 	/* 3) Apply optional PCIe tuning. */
@@ -436,8 +383,6 @@ static int hws_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
 
 	/* 4) Identify chip & capabilities */
 	read_chip_id(hws);
-	dev_info(&pdev->dev, "Device VID=0x%04x DID=0x%04x\n",
-		 pdev->vendor, pdev->device);
 	hws_init_video_sys(hws, false);
 
 	/* 5) Init channels (video state, locks, vb2, ctrls) */
@@ -462,7 +407,6 @@ static int hws_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
 	irqf = IRQF_SHARED;
 	irq = pdev->irq;
 	hws->irq = irq;
-	dev_info(&pdev->dev, "IRQ mode: legacy INTx (shared), irq=%d\n", irq);
 
 	/* B) Mask the device's global/bridge gate (INT_EN_REG_BASE) */
 	hws_irq_mask_gate(hws);
@@ -485,13 +429,10 @@ static int hws_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
 		ctl_reg |= HWS_CTL_IRQ_ENABLE_BIT;
 		writel(ctl_reg, hws->bar0_base + HWS_REG_CTL);
 		(void)readl(hws->bar0_base + HWS_REG_CTL); /* flush write */
-		dev_info(&pdev->dev, "Global IRQ enable bit set in control register\n");
 	}
 
 	/* F) Open the global gate just like legacy did */
 	hws_irq_unmask_gate(hws);
-	dev_info(&pdev->dev, "INT_EN_GATE readback=0x%08x\n",
-		 readl(hws->bar0_base + INT_EN_REG_BASE));
 
 	/* 11) Register V4L2 */
 	ret = hws_video_register(hws);
@@ -515,8 +456,6 @@ static int hws_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
 		goto err_unregister_va; /* reset already stopped the thread */
 	}
 
-	/* 13) Final: show the line is armed */
-	dev_info(&pdev->dev, "irq handler installed on irq=%d\n", irq);
 	return 0;
 
 err_unregister_va:
@@ -558,7 +497,6 @@ static void hws_stop_dsp(struct hws_pcie_dev *hws)
 
 	/* Read the decoder mode/status register */
 	status = readl(hws->bar0_base + HWS_REG_DEC_MODE);
-	dev_dbg(&hws->pdev->dev, "%s: status=0x%08x\n", __func__, status);
 
 	/* If the device looks unplugged/stuck, bail out */
 	if (status == 0xFFFFFFFF)
@@ -593,7 +531,6 @@ static void hws_drain_after_stop(struct hws_pcie_dev *hws)
 {
 	u32 ackmask = 0;
 	unsigned int i;
-	u64 start_ns = ktime_get_mono_fast_ns();
 
 	/* Mask device enables: no new DMA starts. */
 	writel(0x0, hws->bar0_base + HWS_REG_VCAP_ENABLE);
@@ -613,23 +550,17 @@ static void hws_drain_after_stop(struct hws_pcie_dev *hws)
 	/* Ensure no hard IRQ is still running. */
 	if (hws->irq >= 0)
 		synchronize_irq(hws->irq);
-
-	dev_dbg(&hws->pdev->dev, "lifecycle:drain-after-stop:done (%lluus)\n",
-		hws_elapsed_us(start_ns));
 }
 
 static void hws_stop_device(struct hws_pcie_dev *hws)
 {
 	u32 status = readl(hws->bar0_base + HWS_REG_SYS_STATUS);
-	u64 start_ns = ktime_get_mono_fast_ns();
 	bool live = status != 0xFFFFFFFF;
 
-	dev_dbg(&hws->pdev->dev, "%s: status=0x%08x\n", __func__, status);
 	if (!live) {
 		hws->pci_lost = true;
 		goto out;
 	}
-	hws_log_lifecycle_snapshot(hws, "stop-device", "begin");
 
 	/* Make ISR/BH a no-op, then drain engines/IRQ. */
 	hws_publish_stop_flags(hws);
@@ -640,13 +571,6 @@ static void hws_stop_device(struct hws_pcie_dev *hws)
 
 out:
 	hws->start_run = false;
-	if (live)
-		hws_log_lifecycle_snapshot(hws, "stop-device", "end");
-	else
-		dev_dbg(&hws->pdev->dev, "lifecycle:stop-device:device-lost\n");
-	dev_dbg(&hws->pdev->dev, "lifecycle:stop-device:done (%lluus)\n",
-		hws_elapsed_us(start_ns));
-	dev_dbg(&hws->pdev->dev, "%s: complete\n", __func__);
 }
 
 static int hws_quiesce_for_transition(struct hws_pcie_dev *hws,
@@ -654,40 +578,20 @@ static int hws_quiesce_for_transition(struct hws_pcie_dev *hws,
 				      bool stop_thread)
 {
 	struct device *dev = &hws->pdev->dev;
-	u64 start_ns = ktime_get_mono_fast_ns();
-	u64 step_ns;
 	int vret;
 
-	hws_log_lifecycle_snapshot(hws, action, "begin");
-
-	step_ns = ktime_get_mono_fast_ns();
 	hws_block_hotpaths(hws);
-	dev_dbg(dev, "lifecycle:%s:block-hotpaths (%lluus)\n", action,
-		hws_elapsed_us(step_ns));
-	hws_log_lifecycle_snapshot(hws, action, "blocked");
 
 	if (stop_thread) {
-		step_ns = ktime_get_mono_fast_ns();
 		hws_stop_kthread_action(hws);
-		dev_dbg(dev, "lifecycle:%s:stop-kthread (%lluus)\n", action,
-			hws_elapsed_us(step_ns));
 	}
 
-	step_ns = ktime_get_mono_fast_ns();
 	vret = hws_video_quiesce(hws, action);
-	dev_dbg(dev, "lifecycle:%s:video-quiesce ret=%d (%lluus)\n", action,
-		vret, hws_elapsed_us(step_ns));
 	if (vret)
 		dev_warn(dev, "lifecycle:%s video quiesce returned %d\n",
 			 action, vret);
 
-	step_ns = ktime_get_mono_fast_ns();
 	hws_stop_device(hws);
-	dev_dbg(dev, "lifecycle:%s:stop-device (%lluus)\n", action,
-		hws_elapsed_us(step_ns));
-	hws_log_lifecycle_snapshot(hws, action, "end");
-	dev_dbg(dev, "lifecycle:%s:quiesce-done ret=%d (%lluus)\n", action,
-		vret, hws_elapsed_us(start_ns));
 
 	return vret;
 }
@@ -695,15 +599,10 @@ static int hws_quiesce_for_transition(struct hws_pcie_dev *hws,
 static void hws_remove(struct pci_dev *pdev)
 {
 	struct hws_pcie_dev *hws = pci_get_drvdata(pdev);
-	u64 start_ns;
 
 	if (!hws)
 		return;
 
-	start_ns = ktime_get_mono_fast_ns();
-	dev_info(&pdev->dev, "lifecycle:remove begin\n");
-	hws_log_lifecycle_snapshot(hws, "remove", "begin");
-
 	/* Stop the monitor thread before tearing down V4L2/vb2 objects. */
 	hws_block_hotpaths(hws);
 	hws_stop_kthread_action(hws);
@@ -717,9 +616,6 @@ static void hws_remove(struct pci_dev *pdev)
 	/* Release seeded DMA buffers */
 	hws_free_seed_buffers(hws);
 	/* kthread is stopped by the devm action registered in probe. */
-	hws_log_lifecycle_snapshot(hws, "remove", "end");
-	dev_info(&pdev->dev, "lifecycle:remove done (%lluus)\n",
-		 hws_elapsed_us(start_ns));
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -727,22 +623,13 @@ static int hws_pm_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct hws_pcie_dev *hws = pci_get_drvdata(pdev);
-	int vret;
-	u64 start_ns = ktime_get_mono_fast_ns();
-	u64 step_ns;
 
-	dev_info(dev, "lifecycle:pm_suspend begin\n");
-	vret = hws_quiesce_for_transition(hws, "pm_suspend", false);
+	hws_quiesce_for_transition(hws, "pm_suspend", false);
 
-	step_ns = ktime_get_mono_fast_ns();
 	pci_save_state(pdev);
 	pci_clear_master(pdev);
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
-	dev_dbg(dev, "lifecycle:pm_suspend:pci-d3hot (%lluus)\n",
-		hws_elapsed_us(step_ns));
-	dev_info(dev, "lifecycle:pm_suspend done ret=%d (%lluus)\n", vret,
-		 hws_elapsed_us(start_ns));
 
 	return 0;
 }
@@ -752,13 +639,8 @@ static int hws_pm_resume(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct hws_pcie_dev *hws = pci_get_drvdata(pdev);
 	int ret;
-	u64 start_ns = ktime_get_mono_fast_ns();
-	u64 step_ns;
-
-	dev_info(dev, "lifecycle:pm_resume begin\n");
 
 	/* Back to D0 and re-enable the function */
-	step_ns = ktime_get_mono_fast_ns();
 	pci_set_power_state(pdev, PCI_D0);
 
 	ret = pci_enable_device(pdev);
@@ -768,39 +650,25 @@ static int hws_pm_resume(struct device *dev)
 	}
 	pci_restore_state(pdev);
 	pci_set_master(pdev);
-	dev_dbg(dev, "lifecycle:pm_resume:pci-enable (%lluus)\n",
-		hws_elapsed_us(step_ns));
 
 	/* Reapply any PCIe tuning lost across D3 */
 	enable_pcie_relaxed_ordering(pdev);
 
 	/* Reinitialize chip-side capabilities / registers */
-	step_ns = ktime_get_mono_fast_ns();
 	read_chip_id(hws);
 	/* Re-seed BAR remaps/DMA windows and restart the capture core */
 	hws_seed_all_channels(hws);
 	hws_init_video_sys(hws, true);
 	hws_irq_clear_pending(hws);
-	dev_dbg(dev, "lifecycle:pm_resume:chip-reinit (%lluus)\n",
-		hws_elapsed_us(step_ns));
 
 	/* IRQs can be re-enabled now that MMIO is sane */
-	step_ns = ktime_get_mono_fast_ns();
 	if (hws->irq >= 0)
 		enable_irq(hws->irq);
 
 	WRITE_ONCE(hws->suspended, false);
-	dev_dbg(dev, "lifecycle:pm_resume:irq-unsuspend (%lluus)\n",
-		hws_elapsed_us(step_ns));
 
 	/* vb2: nothing mandatory; userspace will STREAMON again when ready */
-	step_ns = ktime_get_mono_fast_ns();
 	hws_video_pm_resume(hws);
-	dev_dbg(dev, "lifecycle:pm_resume:video-resume (%lluus)\n",
-		hws_elapsed_us(step_ns));
-	hws_log_lifecycle_snapshot(hws, "pm_resume", "end");
-	dev_info(dev, "lifecycle:pm_resume done (%lluus)\n",
-		 hws_elapsed_us(start_ns));
 
 	return 0;
 }
@@ -814,22 +682,13 @@ static SIMPLE_DEV_PM_OPS(hws_pm_ops, hws_pm_suspend, hws_pm_resume);
 static void hws_shutdown(struct pci_dev *pdev)
 {
 	struct hws_pcie_dev *hws = pci_get_drvdata(pdev);
-	int vret = 0;
-	u64 start_ns = ktime_get_mono_fast_ns();
-	u64 step_ns;
 
 	if (!hws)
 		return;
 
-	dev_info(&pdev->dev, "lifecycle:pci_shutdown begin\n");
-	vret = hws_quiesce_for_transition(hws, "pci_shutdown", true);
+	hws_quiesce_for_transition(hws, "pci_shutdown", true);
 
-	step_ns = ktime_get_mono_fast_ns();
 	pci_clear_master(pdev);
-	dev_dbg(&pdev->dev, "lifecycle:pci_shutdown:clear-master (%lluus)\n",
-		hws_elapsed_us(step_ns));
-	dev_info(&pdev->dev, "lifecycle:pci_shutdown done ret=%d (%lluus)\n",
-		 vret, hws_elapsed_us(start_ns));
 }
 
 static struct pci_driver hws_pci_driver = {
diff --git a/drivers/media/pci/hws/hws_v4l2_ioctl.c b/drivers/media/pci/hws/hws_v4l2_ioctl.c
index ce396b7225d2..7169fd048e73 100644
--- a/drivers/media/pci/hws/hws_v4l2_ioctl.c
+++ b/drivers/media/pci/hws/hws_v4l2_ioctl.c
@@ -794,10 +794,6 @@ int hws_vidioc_try_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *
 		if (!max_bpl_hw)
 			return -ERANGE;
 		if (bpl > max_bpl_hw) {
-			if (pdev)
-				dev_dbg(&pdev->pdev->dev,
-					"try_fmt: clamp bpl %u -> %zu due to hw buf cap %zu\n",
-					bpl, max_bpl_hw, max_frame);
 			bpl = (u32)max_bpl_hw;
 		}
 	}
@@ -811,11 +807,6 @@ int hws_vidioc_try_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *
 	pix->sizeimage    = (u32)size; /* logical size, not page-aligned */
 
 	hws_set_colorimetry_fmt(pix);
-	if (pdev)
-		dev_dbg(&pdev->pdev->dev,
-			"try_fmt: w=%u h=%u bpl=%u size=%u field=%u\n",
-			pix->width, pix->height, pix->bytesperline,
-			pix->sizeimage, pix->field);
 	return 0;
 }
 
@@ -862,11 +853,6 @@ int hws_vidioc_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *
 	 * hws_calc_sizeimage(vid, vid->pix.width, vid->pix.height, false);
 	 */
 
-	dev_dbg(&vid->parent->pdev->dev,
-		"s_fmt:   w=%u h=%u bpl=%u size=%u\n",
-		vid->pix.width, vid->pix.height, vid->pix.bytesperline,
-		vid->pix.sizeimage);
-
 	return 0;
 }
 
diff --git a/drivers/media/pci/hws/hws_video.c b/drivers/media/pci/hws/hws_video.c
index dbe0fc2a66b5..624c48a63b3b 100644
--- a/drivers/media/pci/hws/hws_video.c
+++ b/drivers/media/pci/hws/hws_video.c
@@ -44,27 +44,15 @@ static void hws_program_dma_window(struct hws_video *vid, dma_addr_t dma);
 static struct hwsvideo_buffer *
 hws_take_queued_buffer_locked(struct hws_video *vid);
 
-static unsigned long long hws_elapsed_us(u64 start_ns)
-{
-	return div_u64(ktime_get_mono_fast_ns() - start_ns, 1000);
-}
-
 static inline bool list_node_unlinked(const struct list_head *n)
 {
 	return n->next == LIST_POISON1 || n->prev == LIST_POISON2;
 }
 
-static bool dma_window_verify;
-module_param_named(dma_window_verify, dma_window_verify, bool, 0644);
-MODULE_PARM_DESC(dma_window_verify,
-		 "Read back DMA window registers after programming (debug)");
-
 void hws_set_dma_doorbell(struct hws_pcie_dev *hws, unsigned int ch,
 			  dma_addr_t dma, const char *tag)
 {
 	iowrite32(lower_32_bits(dma), hws->bar0_base + HWS_REG_DMA_ADDR(ch));
-	dev_dbg(&hws->pdev->dev, "dma_doorbell ch%u: dma=0x%llx tag=%s\n", ch,
-		(u64)dma, tag ? tag : "");
 }
 
 static void hws_program_dma_window(struct hws_video *vid, dma_addr_t dma)
@@ -111,21 +99,7 @@ static void hws_program_dma_window(struct hws_video *vid, dma_addr_t dma)
 
 	vid->window_valid = true;
 
-	if (dma_window_verify && wrote) {
-		u32 r_hi =
-		    readl(hws->bar0_base + PCI_ADDR_TABLE_BASE + table_off);
-		u32 r_lo =
-		    readl(hws->bar0_base + PCI_ADDR_TABLE_BASE + table_off +
-			  PCIE_BARADDROFSIZE);
-		u32 r_base = readl(hws->bar0_base + HWS_BUF_BASE_OFF(ch));
-		u32 r_half = readl(hws->bar0_base + HWS_HALF_SZ_OFF(ch));
-
-		dev_dbg(&hws->pdev->dev,
-			"ch%u remap verify: hi=0x%08x page_lo=0x%08x exp_page=0x%08x base=0x%08x exp_base=0x%08x half16B=0x%08x exp_half=0x%08x\n",
-			ch, r_hi, r_lo, page_lo, r_base,
-			(ch + 1) * PCIEBAR_AXI_BASE + pci_addr, r_half,
-			vid->pix.half_size / 16);
-	} else if (wrote) {
+	if (wrote) {
 		/* Flush posted writes before arming DMA */
 		readl_relaxed(hws->bar0_base + HWS_HALF_SZ_OFF(ch));
 	}
@@ -172,9 +146,6 @@ void hws_prime_next_locked(struct hws_video *vid)
 	hws_program_dma_for_addr(hws, vid->channel_index, dma);
 	iowrite32(lower_32_bits(dma),
 		  hws->bar0_base + HWS_REG_DMA_ADDR(vid->channel_index));
-	dev_dbg(&hws->pdev->dev,
-		"ch%u pre-armed next buffer %p dma=0x%llx\n",
-		vid->channel_index, next, (u64)dma);
 }
 
 static bool hws_force_no_signal_frame(struct hws_video *v, const char *tag)
@@ -312,7 +283,6 @@ int hws_video_init_channel(struct hws_pcie_dev *pdev, int ch)
 	atomic_set(&vid->sequence_number, 0);
 	vid->active = NULL;
 
-	/* DMA watchdog removed; retain counters for diagnostics */
 	vid->timeout_count = 0;
 	vid->error_count = 0;
 
@@ -525,9 +495,6 @@ void hws_enable_video_capture(struct hws_pcie_dev *hws, unsigned int chan,
 	(void)readl(hws->bar0_base + HWS_REG_VCAP_ENABLE);
 
 	WRITE_ONCE(hws->video[chan].cap_active, on);
-
-	dev_dbg(&hws->pdev->dev, "vcap %s ch%u (reg=0x%08x)\n",
-		on ? "ON" : "OFF", chan, status);
 }
 
 static void hws_seed_dma_windows(struct hws_pcie_dev *hws)
@@ -660,9 +627,6 @@ int hws_check_card_status(struct hws_pcie_dev *hws)
 
 	/* If RUN/READY bit (bit0) is not set, reinitialize the video core. */
 	if (!(status & BIT(0))) {
-		dev_dbg(&hws->pdev->dev,
-			"SYS_STATUS not ready (0x%08x), reinitializing\n",
-			status);
 		hws_init_video_sys(hws, true);
 	}
 
@@ -743,18 +707,13 @@ static bool hws_read_active_state(struct hws_pcie_dev *pdx, unsigned int ch,
 static void handle_hwv2_path(struct hws_pcie_dev *hws, unsigned int ch)
 {
 	struct hws_video *vid;
-	u32 reg, in_fps, cur_out_res, want_out_res;
+	u32 reg, cur_out_res, want_out_res;
 
 	if (!hws || !hws->bar0_base || ch >= hws->max_channels)
 		return;
 
 	vid = &hws->video[ch];
 
-	/* 1) Input frame rate (read-only; log or export via debugfs if wanted) */
-	in_fps = readl(hws->bar0_base + HWS_REG_FRAME_RATE(ch));
-	/* dev_dbg(&hws->pdev->dev, "ch%u input fps=%u\n", ch, in_fps); */
-	(void)in_fps;
-
 	/* 2) Output resolution programming.
 	 * For now, mirror the current format to OUT_RES.
 	 */
@@ -1104,11 +1063,6 @@ static void hws_buffer_queue(struct vb2_buffer *vb)
 	struct hws_pcie_dev *hws = vid->parent;
 	unsigned long flags;
 
-	dev_dbg(&hws->pdev->dev,
-		"buffer_queue(ch=%u): vb=%p sizeimage=%u q_active=%d\n",
-		vid->channel_index, vb, vid->pix.sizeimage,
-		READ_ONCE(vid->cap_active));
-
 	/* Initialize buffer slot */
 	buf->slot = 0;
 
@@ -1120,9 +1074,6 @@ static void hws_buffer_queue(struct vb2_buffer *vb)
 	if (READ_ONCE(vid->cap_active) && !vid->active) {
 		dma_addr_t dma_addr;
 
-		dev_dbg(&hws->pdev->dev,
-			"buffer_queue(ch=%u): priming first vb=%p\n",
-			vid->channel_index, &buf->vb.vb2_buf);
 		list_del_init(&buf->list);
 		vid->queued_count--;
 		vid->active = buf;
@@ -1151,9 +1102,6 @@ static int hws_start_streaming(struct vb2_queue *q, unsigned int count)
 	unsigned long flags;
 	int ret;
 
-	dev_dbg(&hws->pdev->dev, "start_streaming: ch=%u count=%u\n",
-		v->channel_index, count);
-
 	ret = hws_check_card_status(hws);
 	if (ret) {
 		struct hwsvideo_buffer *b, *tmp;
@@ -1201,9 +1149,6 @@ static int hws_start_streaming(struct vb2_queue *q, unsigned int count)
 		v->queued_count--;
 		v->active = to_program;
 		prog_vb2 = &to_program->vb.vb2_buf;
-		dev_dbg(&hws->pdev->dev,
-			"start_streaming: ch=%u took buffer %p\n",
-			v->channel_index, to_program);
 	}
 	spin_unlock_irqrestore(&v->irq_lock, flags);
 
@@ -1219,10 +1164,6 @@ static int hws_start_streaming(struct vb2_queue *q, unsigned int count)
 			iowrite32(lower_32_bits(dma_addr),
 				  hws->bar0_base +
 				  HWS_REG_DMA_ADDR(v->channel_index));
-			dev_dbg(&hws->pdev->dev,
-				"start_streaming: ch=%u programmed buffer %p dma=0x%08x\n",
-				v->channel_index, to_program,
-				lower_32_bits(dma_addr));
 			(void)readl(hws->bar0_base + HWS_REG_INT_STATUS);
 		}
 
@@ -1235,47 +1176,11 @@ static int hws_start_streaming(struct vb2_queue *q, unsigned int count)
 			hws_prime_next_locked(v);
 			spin_unlock_irqrestore(&v->irq_lock, pf);
 		}
-	} else {
-		dev_dbg(&hws->pdev->dev,
-			"start_streaming: ch=%u no buffer yet (will arm on QBUF)\n",
-			v->channel_index);
 	}
 
 	return 0;
 }
 
-static void hws_log_video_state(struct hws_video *v, const char *action,
-				const char *phase)
-{
-	struct hws_pcie_dev *hws = v->parent;
-	unsigned long flags;
-	unsigned int queued = 0;
-	unsigned int tracked = 0;
-	unsigned int seq = 0;
-	struct hwsvideo_buffer *b;
-	bool streaming = vb2_is_streaming(&v->buffer_queue);
-	bool cap_active;
-	bool stop_requested;
-	struct hwsvideo_buffer *active;
-	struct hwsvideo_buffer *next_prepared;
-
-	spin_lock_irqsave(&v->irq_lock, flags);
-	list_for_each_entry(b, &v->capture_queue, list)
-		queued++;
-	cap_active = READ_ONCE(v->cap_active);
-	stop_requested = READ_ONCE(v->stop_requested);
-	active = v->active;
-	next_prepared = v->next_prepared;
-	tracked = v->queued_count;
-	seq = (u32)atomic_read(&v->sequence_number);
-	spin_unlock_irqrestore(&v->irq_lock, flags);
-
-	dev_dbg(&hws->pdev->dev,
-		"video:%s:%s ch=%u streaming=%d cap=%d stop=%d active=%p next=%p queued=%u tracked=%u seq=%u\n",
-		action, phase, v->channel_index, streaming, cap_active,
-		stop_requested, active, next_prepared, queued, tracked, seq);
-}
-
 static void hws_stop_streaming(struct vb2_queue *q)
 {
 	struct hws_video *v = q->drv_priv;
@@ -1283,10 +1188,6 @@ static void hws_stop_streaming(struct vb2_queue *q)
 	unsigned long flags;
 	struct hwsvideo_buffer *b, *tmp;
 	LIST_HEAD(done);
-	unsigned int done_cnt = 0;
-	u64 start_ns = ktime_get_mono_fast_ns();
-
-	hws_log_video_state(v, "streamoff", "begin");
 
 	/* 1) Quiesce SW/HW first */
 	lockdep_assert_held(&v->state_lock);
@@ -1307,12 +1208,7 @@ static void hws_stop_streaming(struct vb2_queue *q)
 		/* Unlink from 'done' before completing */
 		list_del_init(&b->list);
 		vb2_buffer_done(&b->vb.vb2_buf, VB2_BUF_STATE_ERROR);
-		done_cnt++;
 	}
-	dev_dbg(&hws->pdev->dev,
-		"video:streamoff:done ch=%u completed=%u (%lluus)\n",
-		v->channel_index, done_cnt, hws_elapsed_us(start_ns));
-	hws_log_video_state(v, "streamoff", "end");
 }
 
 static const struct vb2_ops hwspcie_video_qops = {
@@ -1447,42 +1343,25 @@ void hws_video_unregister(struct hws_pcie_dev *dev)
 int hws_video_quiesce(struct hws_pcie_dev *hws, const char *reason)
 {
 	int i, ret = 0;
-	u64 start_ns = ktime_get_mono_fast_ns();
 
-	dev_dbg(&hws->pdev->dev, "video:%s:begin channels=%u\n", reason,
-		hws->cur_max_video_ch);
 	for (i = 0; i < hws->cur_max_video_ch; i++) {
 		struct hws_video *vid = &hws->video[i];
 		struct vb2_queue *q = &vid->buffer_queue;
-		u64 ch_start_ns = ktime_get_mono_fast_ns();
 		bool streaming;
 
 		if (!q || !q->ops) {
-			dev_dbg(&hws->pdev->dev,
-				"video:%s:ch=%d skipped queue-unavailable\n",
-				reason, i);
 			continue;
 		}
 
 		streaming = vb2_is_streaming(q);
-		hws_log_video_state(vid, reason, "channel");
 		if (streaming) {
 			/* Stop via vb2, which runs .stop_streaming. */
 			int r = vb2_streamoff(q, q->type);
 
-			dev_dbg(&hws->pdev->dev,
-				"video:%s:ch=%d streamoff ret=%d (%lluus)\n",
-				reason, i, r, hws_elapsed_us(ch_start_ns));
 			if (r && !ret)
 				ret = r;
-		} else {
-			dev_dbg(&hws->pdev->dev,
-				"video:%s:ch=%d idle (%lluus)\n",
-				reason, i, hws_elapsed_us(ch_start_ns));
 		}
 	}
-	dev_dbg(&hws->pdev->dev, "video:%s:done ret=%d (%lluus)\n", reason,
-		ret, hws_elapsed_us(start_ns));
 	return ret;
 }
 

[PATCH 2/3] media: hws: quiesce interrupts without disabling shared IRQ

[Ben Hoff]

HWS requests its legacy interrupt with IRQF_SHARED, but suspend, shutdown,
and removal call disable_irq() on the shared descriptor. This prevents
other devices on the line from being serviced, and removal leaves the
IRQ disable unbalanced.

Mask the HWS interrupt gate, flush the write, and synchronize the handler.
Publish suspended state before draining so callbacks caused by a peer on
the shared line return without accessing HWS registers after suspend.

Keep capture-core initialization from opening the interrupt gate. Resume
restores the core and clears pending causes before publishing live state
and unmasking the device-local gate.

Fixes: ba07fd2f5742 ("media: pci: add AVMatrix HWS capture driver")
Assisted-by: Codex:GPT-6
Signed-off-by: Ben Hoff <hoff.benjamin.k@gmail.com>
---
 drivers/media/pci/hws/hws_irq.c   | 12 +++---------
 drivers/media/pci/hws/hws_pci.c   | 27 +++++++++++++++++----------
 drivers/media/pci/hws/hws_video.c |  8 ++------
 3 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/media/pci/hws/hws_irq.c b/drivers/media/pci/hws/hws_irq.c
index 787c9e498799..8d883663617b 100644
--- a/drivers/media/pci/hws/hws_irq.c
+++ b/drivers/media/pci/hws/hws_irq.c
@@ -150,15 +150,9 @@ irqreturn_t hws_irq_handler(int irq, void *info)
 	struct hws_pcie_dev *pdx = info;
 	u32 int_state;
 
-	/* Fast path: if suspended, quietly ack and exit */
-	if (READ_ONCE(pdx->suspended)) {
-		int_state = readl_relaxed(pdx->bar0_base + HWS_REG_INT_STATUS);
-		if (int_state) {
-			writel(int_state, pdx->bar0_base + HWS_REG_INT_STATUS);
-			(void)readl_relaxed(pdx->bar0_base + HWS_REG_INT_STATUS);
-		}
-		return int_state ? IRQ_HANDLED : IRQ_NONE;
-	}
+	if (!pdx || READ_ONCE(pdx->suspended) || !pdx->bar0_base)
+		return IRQ_NONE;
+
 	int_state = readl_relaxed(pdx->bar0_base + HWS_REG_INT_STATUS);
 	if (!int_state || int_state == 0xFFFFFFFF) {
 		return IRQ_NONE;
diff --git a/drivers/media/pci/hws/hws_pci.c b/drivers/media/pci/hws/hws_pci.c
index 65b32fac6d1a..c9397b13392a 100644
--- a/drivers/media/pci/hws/hws_pci.c
+++ b/drivers/media/pci/hws/hws_pci.c
@@ -325,14 +325,21 @@ static void hws_irq_clear_pending(struct hws_pcie_dev *hws)
 static void hws_block_hotpaths(struct hws_pcie_dev *hws)
 {
 	WRITE_ONCE(hws->suspended, true);
-	if (hws->irq >= 0)
-		disable_irq(hws->irq);
+	/* Publish the stop state before a racing handler can enter MMIO. */
+	smp_mb();
 
-	if (!hws->bar0_base)
-		return;
+	if (hws->bar0_base)
+		hws_irq_mask_gate(hws);
 
-	hws_irq_mask_gate(hws);
-	hws_irq_clear_pending(hws);
+	/*
+	 * Do not disable the shared descriptor. Wait for any invocation of this
+	 * handler that raced with the device-local gate instead.
+	 */
+	if (hws->irq >= 0)
+		synchronize_irq(hws->irq);
+
+	if (hws->bar0_base)
+		hws_irq_clear_pending(hws);
 }
 
 static int hws_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
@@ -661,11 +668,11 @@ static int hws_pm_resume(struct device *dev)
 	hws_init_video_sys(hws, true);
 	hws_irq_clear_pending(hws);
 
-	/* IRQs can be re-enabled now that MMIO is sane */
-	if (hws->irq >= 0)
-		enable_irq(hws->irq);
-
+	/* Make our handler live before reopening only this device's IRQ gate. */
 	WRITE_ONCE(hws->suspended, false);
+	/* Publish the live state before the device can raise another interrupt. */
+	smp_mb();
+	hws_irq_unmask_gate(hws);
 
 	/* vb2: nothing mandatory; userspace will STREAMON again when ready */
 	hws_video_pm_resume(hws);
diff --git a/drivers/media/pci/hws/hws_video.c b/drivers/media/pci/hws/hws_video.c
index 624c48a63b3b..bdbce09ec3e6 100644
--- a/drivers/media/pci/hws/hws_video.c
+++ b/drivers/media/pci/hws/hws_video.c
@@ -565,7 +565,7 @@ static void hws_ack_all_irqs(struct hws_pcie_dev *hws)
 	}
 }
 
-static void hws_open_irq_fabric(struct hws_pcie_dev *hws)
+static void hws_configure_irq_fabric(struct hws_pcie_dev *hws)
 {
 	/* Route all sources to vector 0. */
 	writel(0x00000000, hws->bar0_base + PCIE_INT_DEC_REG_BASE);
@@ -574,10 +574,6 @@ static void hws_open_irq_fabric(struct hws_pcie_dev *hws)
 	/* Enable the PCIe bridge. */
 	writel(0x00000001, hws->bar0_base + PCIEBR_EN_REG_BASE);
 	(void)readl(hws->bar0_base + PCIEBR_EN_REG_BASE);
-
-	/* Open the global/bridge gate (legacy 0x3FFFF) */
-	writel(HWS_INT_EN_MASK, hws->bar0_base + INT_EN_REG_BASE);
-	(void)readl(hws->bar0_base + INT_EN_REG_BASE);
 }
 
 void hws_init_video_sys(struct hws_pcie_dev *hws, bool enable)
@@ -604,7 +600,7 @@ void hws_init_video_sys(struct hws_pcie_dev *hws, bool enable)
 	writel(0x80FFFFFF, hws->bar0_base + HWS_REG_DEC_MODE);
 	writel(0x13, hws->bar0_base + HWS_REG_DEC_MODE);
 	hws_ack_all_irqs(hws);
-	hws_open_irq_fabric(hws);
+	hws_configure_irq_fabric(hws);
 	/* 6) record that we're now running */
 	hws->start_run = true;
 }

[PATCH 3/3] media: hws: serialize video quiesce with queue state

[Ben Hoff]

Suspend and shutdown call vb2_streamoff() without taking the state mutex
used by the video device and its vb2 queue. This can race userspace queue
operations and violates the locking requirement in hws_stop_streaming().

Hold the channel state mutex around the streaming check and streamoff.
Serialize monitor passes with lifecycle quiescence, and recheck suspended
state after acquiring the monitor mutex so a delayed pass cannot enter
hardware access after teardown has drained it.

Reject readiness checks once suspension begins. If the core is not ready,
return an error instead of resetting shared hardware while another
channel may still own capture buffers.

Fixes: ba07fd2f5742 ("media: pci: add AVMatrix HWS capture driver")
Assisted-by: Codex:GPT-6
Signed-off-by: Ben Hoff <hoff.benjamin.k@gmail.com>
---
 drivers/media/pci/hws/hws.h       |  2 ++
 drivers/media/pci/hws/hws_pci.c   | 11 +++++++++--
 drivers/media/pci/hws/hws_video.c | 11 +++++++++--
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/hws/hws.h b/drivers/media/pci/hws/hws.h
index d87d52674b69..01a6b00dcca6 100644
--- a/drivers/media/pci/hws/hws.h
+++ b/drivers/media/pci/hws/hws.h
@@ -8,6 +8,7 @@
 #include <linux/kthread.h>
 #include <linux/pci.h>
 #include <linux/list.h>
+#include <linux/mutex.h>
 #include <linux/spinlock.h>
 #include <linux/sizes.h>
 #include <linux/atomic.h>
@@ -161,6 +162,7 @@ struct hws_pcie_dev {
 
 	/* Kernel thread */
 	struct task_struct *main_task;
+	struct mutex monitor_lock; /* serializes monitor and lifecycle changes */
 	struct hws_scratch_dma scratch_vid[MAX_VID_CHANNELS];
 
 	bool suspended;
diff --git a/drivers/media/pci/hws/hws_pci.c b/drivers/media/pci/hws/hws_pci.c
index c9397b13392a..7fdb1087d247 100644
--- a/drivers/media/pci/hws/hws_pci.c
+++ b/drivers/media/pci/hws/hws_pci.c
@@ -177,8 +177,10 @@ static int main_ks_thread_handle(void *data)
 			continue;
 		}
 
-		/* avoid MMIO when suspended (guarded above) */
-		check_video_format(pdx);
+		mutex_lock(&pdx->monitor_lock);
+		if (!READ_ONCE(pdx->suspended))
+			check_video_format(pdx);
+		mutex_unlock(&pdx->monitor_lock);
 
 		try_to_freeze(); /* cooperate with freezer each loop */
 
@@ -338,6 +340,10 @@ static void hws_block_hotpaths(struct hws_pcie_dev *hws)
 	if (hws->irq >= 0)
 		synchronize_irq(hws->irq);
 
+	/* Wait for a monitor pass that started before suspended was set. */
+	mutex_lock(&hws->monitor_lock);
+	mutex_unlock(&hws->monitor_lock);
+
 	if (hws->bar0_base)
 		hws_irq_clear_pending(hws);
 }
@@ -357,6 +363,7 @@ static int hws_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
 	hws->pdev = pdev;
 	hws->irq = -1;
 	hws->suspended = false;
+	mutex_init(&hws->monitor_lock);
 	pci_set_drvdata(pdev, hws);
 
 	/* 1) Enable device + bus mastering (managed) */
diff --git a/drivers/media/pci/hws/hws_video.c b/drivers/media/pci/hws/hws_video.c
index bdbce09ec3e6..8e029b71b5b5 100644
--- a/drivers/media/pci/hws/hws_video.c
+++ b/drivers/media/pci/hws/hws_video.c
@@ -611,6 +611,8 @@ int hws_check_card_status(struct hws_pcie_dev *hws)
 
 	if (!hws || !hws->bar0_base)
 		return -ENODEV;
+	if (READ_ONCE(hws->suspended))
+		return -EBUSY;
 
 	status = readl(hws->bar0_base + HWS_REG_SYS_STATUS);
 
@@ -621,9 +623,12 @@ int hws_check_card_status(struct hws_pcie_dev *hws)
 		return -ENODEV;
 	}
 
-	/* If RUN/READY bit (bit0) is not set, reinitialize the video core. */
+	/* Runtime reset would invalidate every active channel's DMA ownership. */
 	if (!(status & BIT(0))) {
-		hws_init_video_sys(hws, true);
+		dev_warn_ratelimited(&hws->pdev->dev,
+				     "SYS_STATUS not ready (0x%08x); runtime core reset refused\n",
+				     status);
+		return -EIO;
 	}
 
 	return 0;
@@ -1349,6 +1354,7 @@ int hws_video_quiesce(struct hws_pcie_dev *hws, const char *reason)
 			continue;
 		}
 
+		mutex_lock(&vid->state_lock);
 		streaming = vb2_is_streaming(q);
 		if (streaming) {
 			/* Stop via vb2, which runs .stop_streaming. */
@@ -1357,6 +1363,7 @@ int hws_video_quiesce(struct hws_pcie_dev *hws, const char *reason)
 			if (r && !ret)
 				ret = r;
 		}
+		mutex_unlock(&vid->state_lock);
 	}
 	return ret;
 }