Re: [PATCH] media: mali-c55: free IRQ when remove while runtime-active

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi Linus

On Fri, Sep 04, 2026 at 12:12:20PM +0200, Linus Walleij wrote:
> On Thu, Sep 3, 2026 at 4:55 AM Li Youhong <dayou5941@163.com> wrote:
>
> > @@ -861,6 +861,8 @@ static void mali_c55_remove(struct platform_device *pdev)
> >
> >         mali_c55_media_frameworks_deinit(mali_c55);
> >         if (!pm_runtime_suspended(&pdev->dev)) {
> > +               if (irq_has_action(mali_c55->irqnum))
> > +                       free_irq(mali_c55->irqnum, &pdev->dev);
> >                 __mali_c55_power_off(mali_c55);
> >                 pm_runtime_set_suspended(&pdev->dev);
>
> This whole runtime suspend/resume looks a bit odd to me.
>
> Normally you keep the IRQ requested perpetually in a driver,
> this looks like some really odd workaround to explicitly deal with
> wakeup capable/non-wakeup capable IRQs.
>
> I tried to see if this was addressed during review of the driver
> and whether there was a reason for doing things like this.
> But there are too many threads on lore.

Indeed it's weird.

I've a branch that I started that tried to fix that, but the issue I
was hitting was due the fact the ISP starts with all ISP unmasked, and
I get very bad races between the power-up routine and the IRQ.

I run out of time and gave up, but if you're interested I can provide
you references to that work

>
> I think the proper fix is to keep the IRQ requested during
> suspend/resume and *not* mark it as wakeup-source; in the
> device tree for the SoC.
>
> Support code is needed in the driver, something along the line
> of this:
>
> suspend():
>                 if (device_may_wakeup(dev))
>                         enable_irq_wake(irq);
>
> resume():
>                 if (device_may_wakeup(dev))
>                         disable_irq_wake(irq);
>
> Do you want to make the patch or should I?

That's something I didn't consider maybe :)

>
> Yours,
> Linus Walleij