Re: [PATCH v2] media: rockchip: rga: quiesce IRQ before releasing m2m state

[Sven Püschel]

Hi,

On 9/28/26 22:18, Nicolas Dufresne wrote:
> Hi,
>
> Le samedi 04 juillet 2026 à 08:46 +0000, Fan Wu a écrit :
> > rga_remove() releases the m2m state before the IRQ is freed. The IRQ is
> > devm-managed and is only released once rga_remove() returns, so rga_isr()
> > can still run while the m2m device state is being torn down.
> >
> > Store the IRQ number in struct rockchip_rga, unregister the video device
> > first, and free the IRQ before releasing the m2m state so the handler
> > cannot run against the freed state.
> >
> > Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
> Again, I don't know if this is a real issue, but seems fair.

thanks for forwarding. Also stumbled upon the ordering in the remove 
function but kept it, as it looked akward if it mismatched the probe 
cleanup order (which comes from the fact that the video device is 
allocated, then the m2m device and after that the video device is 
registered). Is it possible to init the m2m device first and then 
allocate and register the video_device, thus keeping the same teardown 
order (i'd then adjust this in my multi-core series)? Or is there some 
logic behind this probe ordering?

Btw. the irq variable will probably be dropped with my 
mulit-core/component-devices patchset, as I then have a separate core 
struct and can check if my core has been already bound to something (and 
otherwise ignore IRQs). See [1][2]


Sincerely
    Sven


[1] 
https://lore.kernel.org/linux-media/20260916-spu-rga3multicore-v2-13-23aa2cb74e61@xxxxxxxxxxxxxx/

[2] 
https://lore.kernel.org/linux-media/20260916-spu-rga3multicore-v2-14-23aa2cb74e61@xxxxxxxxxxxxxx/


> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
>
> Picked, ty
>
> > ---
> > Changes in v2:
> > - Rebased onto media-committers/fixes (Linux 7.2-rc1) so the Media CI
> >    valid-ancestor check passes. The rga driver was rewritten upstream
> >    (rga3 support, external IOMMU, clk_bulk, cmdbuf moved to rga_ctx), so
> >    this is a re-port rather than a pure rebase: the resource freed during
> >    removal is now rga->m2m_dev.
> > ---
> >   drivers/media/platform/rockchip/rga/rga.c | 4 +++-
> >   drivers/media/platform/rockchip/rga/rga.h | 1 +
> >   2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> > index b3cb6bf8eb86..fbc99462dce2 100644
> > --- a/drivers/media/platform/rockchip/rga/rga.c
> > +++ b/drivers/media/platform/rockchip/rga/rga.c
> > @@ -797,6 +797,7 @@ static int rga_probe(struct platform_device *pdev)
> >   		ret = irq;
> >   		goto err_put_clk;
> >   	}
> > +	rga->irq = irq;
> >   
> >   	ret = devm_request_irq(rga->dev, irq, rga_isr,
> >   			       rga_has_internal_iommu(rga) ? 0 : IRQF_SHARED,
> > @@ -876,8 +877,9 @@ static void rga_remove(struct platform_device *pdev)
> >   
> >   	v4l2_info(&rga->v4l2_dev, "Removing\n");
> >   
> > -	v4l2_m2m_release(rga->m2m_dev);
> >   	video_unregister_device(rga->vfd);
> > +	devm_free_irq(rga->dev, rga->irq, rga);
> > +	v4l2_m2m_release(rga->m2m_dev);
> >   	v4l2_device_unregister(&rga->v4l2_dev);
> >   
> >   	pm_runtime_disable(rga->dev);
> > diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
> > index bd431534d0d3..7bcdf36c11b4 100644
> > --- a/drivers/media/platform/rockchip/rga/rga.h
> > +++ b/drivers/media/platform/rockchip/rga/rga.h
> > @@ -73,6 +73,7 @@ struct rockchip_rga {
> >   	struct device *dev;
> >   	struct regmap *grf;
> >   	void __iomem *regs;
> > +	int irq;
> >   	struct clk_bulk_data *clks;
> >   	int num_clks;
> >   	struct rockchip_rga_version version;