RE: [PATCH v2] media: nuvoton: npcm-video: quiesce VCD IRQ before teardown

[KFLIN@xxxxxxxxxxx]

Hi Hans, Fan,

I've tested this patch on our platform and did not observe any issues.
Thanks for the patch.

Tested-by: Marvin Lin <kflin@xxxxxxxxxxx>

Regards,
Marvin

> -----Original Message-----
> From: Hans Verkuil <hverkuil+cisco@xxxxxxxxxx>
> Sent: Tuesday, July 28, 2026 8:12 PM
> To: Fan Wu <fanwu01@xxxxxxxxxx>; CS20 KWLiu <KWLIU@xxxxxxxxxxx>;
> CS20 KFLin <KFLIN@xxxxxxxxxxx>
> Cc: hverkuil@xxxxxxxxxx; mchehab@xxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx;
> openbmc@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2] media: nuvoton: npcm-video: quiesce VCD IRQ before
> teardown
>
> CAUTION - External Email: Do not click links or open attachments unless you
> acknowledge the sender and content.
>
>
> Can Nuvoton test this patch? I feel happier if this is given a quick test.
>
> Regards,
>
>         Hans
>
> On 16/07/2026 12:15, Fan Wu wrote:
> > The VCD IRQ is devm-requested, but npcm_video_remove() frees the video
> > object before devres releases that IRQ.  The threaded handler
> > dereferences
> > video->vcd_regmap before checking VIDEO_STREAMING, so an interrupt in
> > video->that
> > interval can access freed memory.
> >
> > Request the IRQ with IRQF_NO_AUTOEN.  Enable it after starting capture
> > and setting VIDEO_STREAMING, and disable it first in stop_streaming().
> > disable_irq() waits for an in-flight threaded handler to finish, after
> > which stop_streaming() can mask and reset the VCD without a handler
> > re-enabling it.
> >
> > Use vb2_video_unregister_device() during remove.  It releases the vb2
> > queue and calls stop_streaming() for an active stream, ensuring that
> > the IRQ is disabled before the video object is freed.  Do not release
> > the queue separately.
> >
> > If streaming is never started, IRQF_NO_AUTOEN keeps the IRQ disabled
> > until devres releases it.
> >
> > This issue was found by an in-house static analysis tool.
> >
> > Fixes: 46c15a4ff1f4 ("media: nuvoton: Add driver for NPCM video
> > capture and encoding engine")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Assisted-by: Codex:gpt-5.6
> > Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
> > ---
> > Changes since v1:
> > - Follow Hans Verkuil's suggestion to tie IRQ enablement to the streaming
> >   lifecycle (IRQF_NO_AUTOEN + enable_irq/disable_irq) and to use
> >   vb2_video_unregister_device() for teardown.
> >
> > Compile-tested only; I do not have NPCM hardware, so runtime testing
> > by the Nuvoton maintainers would be appreciated.
> > ---
> >  drivers/media/platform/nuvoton/npcm-video.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/nuvoton/npcm-video.c
> > b/drivers/media/platform/nuvoton/npcm-video.c
> > index 52505af35c08..c28d9d7edd83 100644
> > --- a/drivers/media/platform/nuvoton/npcm-video.c
> > +++ b/drivers/media/platform/nuvoton/npcm-video.c
> > @@ -120,6 +120,7 @@ struct npcm_video {
> >
> >       struct list_head buffers;
> >       struct mutex buffer_lock; /* buffer list lock */
> > +     int irq;
> >       unsigned long flags;
> >       unsigned int sequence;
> >
> > @@ -1486,6 +1487,7 @@ static int npcm_video_start_streaming(struct
> vb2_queue *q, unsigned int count)
> >       }
> >
> >       set_bit(VIDEO_STREAMING, &video->flags);
> > +     enable_irq(video->irq);
> >       return 0;
> >  }
> >
> > @@ -1494,6 +1496,7 @@ static void npcm_video_stop_streaming(struct
> vb2_queue *q)
> >       struct npcm_video *video = vb2_get_drv_priv(q);
> >       struct regmap *vcd = video->vcd_regmap;
> >
> > +     disable_irq(video->irq);
> >       clear_bit(VIDEO_STREAMING, &video->flags);
> >       regmap_write(vcd, VCD_INTE, 0);
> >       regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR); @@ -1707,9 +1710,10
> > @@ static int npcm_video_init(struct npcm_video *video)
> >               dev_err(dev, "Failed to find VCD IRQ\n");
> >               return -ENODEV;
> >       }
> > +     video->irq = irq;
> >
> >       rc = devm_request_threaded_irq(dev, irq, NULL, npcm_video_irq,
> > -                                    IRQF_ONESHOT, DEVICE_NAME, video);
> > +                                    IRQF_ONESHOT | IRQF_NO_AUTOEN,
> > + DEVICE_NAME, video);
> >       if (rc < 0) {
> >               dev_err(dev, "Failed to request IRQ %d\n", irq);
> >               return rc;
> > @@ -1807,8 +1811,7 @@ static void npcm_video_remove(struct
> platform_device *pdev)
> >       struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
> >       struct npcm_video *video = to_npcm_video(v4l2_dev);
> >
> > -     video_unregister_device(&video->vdev);
> > -     vb2_queue_release(&video->queue);
> > +     vb2_video_unregister_device(&video->vdev);
> >       v4l2_ctrl_handler_free(&video->ctrl_handler);
> >       v4l2_device_unregister(v4l2_dev);
> >       if (video->ece.enable)

________________________________
________________________________
 The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.