Re: [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove

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

Hi David
  since you have to send a v4 anyway, would you care about also
powering off the device duing remove ?

I have a patch, but since you're already looking at this it doesn't
make much sense to pile another one on top.

Could you squash in the below suggestions ?

On Fri, Mar 27, 2026 at 06:01:29PM +0100, Jacopo Mondi wrote:
> Hi David
>
> On Fri, Mar 27, 2026 at 03:07:06PM +0000, David Carlier wrote:
> > pm_runtime_enable() is called during probe but mali_c55_remove() never
> > calls pm_runtime_disable(), leaving the device's runtime PM state
> > enabled after the driver is unbound.

I would use these as the commit message and the commit title:

    media: mali-c55: Power-off the peripheral in remove()

    The Mali C55 driver doesn't depend on PM. For this reason, if pm_runtime
    is not compiled in it is required to manually power-off the peripheral
    during the driver's remove() handler.

    Also pm_runtime_enable() is called during probe but mali_c55_remove() never
    calls pm_runtime_disable(), leaving the device's runtime PM state enabled
    after the driver is unbound.

    Manually power-off the peripheral in remove() if the peripheral has not
    been suspended using runtime_pm and disable runtime pm.

> >
> > Add the missing pm_runtime_disable() call to the remove path.
>
> The driver doesn't depend on PM, so we need to explicitly power-off
> the peripheral in remove() (and set the pm_runtime status to
> suspended)
>
> >
> > Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> This patch is however correct, I'll add the power-off on top.
>
> > ---
> >  drivers/media/platform/arm/mali-c55/mali-c55-core.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-core.c b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> > index 5cb59c70ffc9..38b11d5ba168 100644
> > --- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> > @@ -859,6 +859,7 @@ static void mali_c55_remove(struct platform_device *pdev)
> >  	struct mali_c55 *mali_c55 = platform_get_drvdata(pdev);
> >
> >  	mali_c55_media_frameworks_deinit(mali_c55);

And this should become:

+       if (!pm_runtime_suspended(&pdev->dev)) {
+               __mali_c55_power_off(mali_c55);
+               pm_runtime_set_suspended(&pdev->dev);
+       }
> > +	pm_runtime_disable(&pdev->dev);

Otherwise, if you want to keep your patch simpler, I'll add a patch
for power-off on top.

Thanks
   j

> >  	kfree(mali_c55->context.registers);
> >  	of_reserved_mem_device_release(&pdev->dev);
> >  }
> > --
> > 2.53.0
> >
> >