* [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release()
@ 2026-03-27 15:07 David Carlier
2026-03-27 15:07 ` [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove David Carlier
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: David Carlier @ 2026-03-27 15:07 UTC (permalink / raw)
To: Daniel Scally, Jacopo Mondi, Mauro Carvalho Chehab,
Nayden Kanchev, Hans Verkuil
Cc: linux-media, David Carlier
mali_c55_probe() calls of_reserved_mem_device_init() to associate
reserved memory regions with the device. This function allocates a
struct rmem_assigned_device and adds it to a global linked list, which
must be explicitly released via of_reserved_mem_device_release() — there
is no devm variant of this API.
However, neither the probe error paths nor mali_c55_remove() called
of_reserved_mem_device_release(). Any probe failure after the
of_reserved_mem_device_init() call, as well as every normal device
removal, leaked the reserved memory association on the global list.
Fix this by adding an err_release_mem label at the end of the probe
error chain and calling of_reserved_mem_device_release() in
mali_c55_remove(). The remove teardown order is also corrected to call
mali_c55_media_frameworks_deinit() before kfree(), mirroring the probe
init order in reverse.
Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
drivers/media/platform/arm/mali-c55/mali-c55-core.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
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 c1a562cd214e..5cb59c70ffc9 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
@@ -806,8 +806,10 @@ static int mali_c55_probe(struct platform_device *pdev)
vb2_dma_contig_set_max_seg_size(dev, UINT_MAX);
ret = __mali_c55_power_on(mali_c55);
- if (ret)
- return dev_err_probe(dev, ret, "failed to power on\n");
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to power on\n");
+ goto err_release_mem;
+ }
ret = mali_c55_check_hwcfg(mali_c55);
if (ret)
@@ -846,6 +848,8 @@ static int mali_c55_probe(struct platform_device *pdev)
kfree(mali_c55->context.registers);
err_power_off:
__mali_c55_power_off(mali_c55);
+err_release_mem:
+ of_reserved_mem_device_release(dev);
return ret;
}
@@ -854,8 +858,9 @@ static void mali_c55_remove(struct platform_device *pdev)
{
struct mali_c55 *mali_c55 = platform_get_drvdata(pdev);
- kfree(mali_c55->context.registers);
mali_c55_media_frameworks_deinit(mali_c55);
+ kfree(mali_c55->context.registers);
+ of_reserved_mem_device_release(&pdev->dev);
}
static const struct of_device_id mali_c55_of_match[] = {
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove
2026-03-27 15:07 [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release() David Carlier
@ 2026-03-27 15:07 ` David Carlier
2026-03-27 17:01 ` Jacopo Mondi
2026-03-27 15:07 ` [PATCH v3 3/3] media: mali-c55: fix probe error path skipping pm_runtime_disable() David Carlier
2026-03-27 16:58 ` [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release() Jacopo Mondi
2 siblings, 1 reply; 10+ messages in thread
From: David Carlier @ 2026-03-27 15:07 UTC (permalink / raw)
To: Daniel Scally, Jacopo Mondi, Mauro Carvalho Chehab,
Nayden Kanchev, Hans Verkuil
Cc: linux-media, David Carlier
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.
Add the missing pm_runtime_disable() call to the remove path.
Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
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);
+ pm_runtime_disable(&pdev->dev);
kfree(mali_c55->context.registers);
of_reserved_mem_device_release(&pdev->dev);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] media: mali-c55: fix probe error path skipping pm_runtime_disable()
2026-03-27 15:07 [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release() David Carlier
2026-03-27 15:07 ` [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove David Carlier
@ 2026-03-27 15:07 ` David Carlier
2026-03-27 17:04 ` Jacopo Mondi
2026-03-27 16:58 ` [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release() Jacopo Mondi
2 siblings, 1 reply; 10+ messages in thread
From: David Carlier @ 2026-03-27 15:07 UTC (permalink / raw)
To: Daniel Scally, Jacopo Mondi, Mauro Carvalho Chehab,
Nayden Kanchev, Hans Verkuil
Cc: linux-media, David Carlier
When mali_c55_media_frameworks_init() fails, the goto target jumps to
err_free_context_registers, skipping pm_runtime_disable() despite
pm_runtime having already been enabled earlier in the function.
Fix this by adding an err_pm_runtime_disable label and redirecting the
frameworks init failure to it, so pm_runtime is properly unwound on
that error path.
Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
drivers/media/platform/arm/mali-c55/mali-c55-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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 38b11d5ba168..f998f914b355 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
@@ -828,7 +828,7 @@ static int mali_c55_probe(struct platform_device *pdev)
ret = mali_c55_media_frameworks_init(mali_c55);
if (ret)
- goto err_free_context_registers;
+ goto err_pm_runtime_disable;
pm_runtime_idle(&pdev->dev);
@@ -843,8 +843,8 @@ static int mali_c55_probe(struct platform_device *pdev)
err_deinit_media_frameworks:
mali_c55_media_frameworks_deinit(mali_c55);
+err_pm_runtime_disable:
pm_runtime_disable(&pdev->dev);
-err_free_context_registers:
kfree(mali_c55->context.registers);
err_power_off:
__mali_c55_power_off(mali_c55);
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release()
2026-03-27 15:07 [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release() David Carlier
2026-03-27 15:07 ` [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove David Carlier
2026-03-27 15:07 ` [PATCH v3 3/3] media: mali-c55: fix probe error path skipping pm_runtime_disable() David Carlier
@ 2026-03-27 16:58 ` Jacopo Mondi
2026-03-27 17:05 ` David CARLIER
2 siblings, 1 reply; 10+ messages in thread
From: Jacopo Mondi @ 2026-03-27 16:58 UTC (permalink / raw)
To: David Carlier
Cc: Daniel Scally, Jacopo Mondi, Mauro Carvalho Chehab,
Nayden Kanchev, Hans Verkuil, linux-media
Hi David
On Fri, Mar 27, 2026 at 03:07:05PM +0000, David Carlier wrote:
> mali_c55_probe() calls of_reserved_mem_device_init() to associate
> reserved memory regions with the device. This function allocates a
> struct rmem_assigned_device and adds it to a global linked list, which
> must be explicitly released via of_reserved_mem_device_release() — there
> is no devm variant of this API.
>
> However, neither the probe error paths nor mali_c55_remove() called
> of_reserved_mem_device_release(). Any probe failure after the
> of_reserved_mem_device_init() call, as well as every normal device
> removal, leaked the reserved memory association on the global list.
>
> Fix this by adding an err_release_mem label at the end of the probe
> error chain and calling of_reserved_mem_device_release() in
> mali_c55_remove(). The remove teardown order is also corrected to call
> mali_c55_media_frameworks_deinit() before kfree(), mirroring the probe
> init order in reverse.
>
> 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>
With your permission, I'll take these three patches in a follow-up
series that fixes the module loading/unloading.
I could also specify it as pre-requisite to b4, I'll see what's easier
Thanks
j
> ---
> drivers/media/platform/arm/mali-c55/mali-c55-core.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> 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 c1a562cd214e..5cb59c70ffc9 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> @@ -806,8 +806,10 @@ static int mali_c55_probe(struct platform_device *pdev)
> vb2_dma_contig_set_max_seg_size(dev, UINT_MAX);
>
> ret = __mali_c55_power_on(mali_c55);
> - if (ret)
> - return dev_err_probe(dev, ret, "failed to power on\n");
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to power on\n");
> + goto err_release_mem;
> + }
>
> ret = mali_c55_check_hwcfg(mali_c55);
> if (ret)
> @@ -846,6 +848,8 @@ static int mali_c55_probe(struct platform_device *pdev)
> kfree(mali_c55->context.registers);
> err_power_off:
> __mali_c55_power_off(mali_c55);
> +err_release_mem:
> + of_reserved_mem_device_release(dev);
>
> return ret;
> }
> @@ -854,8 +858,9 @@ static void mali_c55_remove(struct platform_device *pdev)
> {
> struct mali_c55 *mali_c55 = platform_get_drvdata(pdev);
>
> - kfree(mali_c55->context.registers);
> mali_c55_media_frameworks_deinit(mali_c55);
> + kfree(mali_c55->context.registers);
> + of_reserved_mem_device_release(&pdev->dev);
> }
>
> static const struct of_device_id mali_c55_of_match[] = {
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove
2026-03-27 15:07 ` [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove David Carlier
@ 2026-03-27 17:01 ` Jacopo Mondi
2026-03-28 14:58 ` Jacopo Mondi
0 siblings, 1 reply; 10+ messages in thread
From: Jacopo Mondi @ 2026-03-27 17:01 UTC (permalink / raw)
To: David Carlier
Cc: Daniel Scally, Jacopo Mondi, Mauro Carvalho Chehab,
Nayden Kanchev, Hans Verkuil, linux-media
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.
>
> 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);
> + pm_runtime_disable(&pdev->dev);
> kfree(mali_c55->context.registers);
> of_reserved_mem_device_release(&pdev->dev);
> }
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] media: mali-c55: fix probe error path skipping pm_runtime_disable()
2026-03-27 15:07 ` [PATCH v3 3/3] media: mali-c55: fix probe error path skipping pm_runtime_disable() David Carlier
@ 2026-03-27 17:04 ` Jacopo Mondi
2026-03-27 17:10 ` David CARLIER
0 siblings, 1 reply; 10+ messages in thread
From: Jacopo Mondi @ 2026-03-27 17:04 UTC (permalink / raw)
To: David Carlier
Cc: Daniel Scally, Jacopo Mondi, Mauro Carvalho Chehab,
Nayden Kanchev, Hans Verkuil, linux-media
Hi David
On Fri, Mar 27, 2026 at 03:07:07PM +0000, David Carlier wrote:
> When mali_c55_media_frameworks_init() fails, the goto target jumps to
> err_free_context_registers, skipping pm_runtime_disable() despite
> pm_runtime having already been enabled earlier in the function.
>
> Fix this by adding an err_pm_runtime_disable label and redirecting the
> frameworks init failure to it, so pm_runtime is properly unwound on
> that error path.
>
> Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> drivers/media/platform/arm/mali-c55/mali-c55-core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> 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 38b11d5ba168..f998f914b355 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> @@ -828,7 +828,7 @@ static int mali_c55_probe(struct platform_device *pdev)
>
> ret = mali_c55_media_frameworks_init(mali_c55);
> if (ret)
> - goto err_free_context_registers;
> + goto err_pm_runtime_disable;
>
> pm_runtime_idle(&pdev->dev);
>
> @@ -843,8 +843,8 @@ static int mali_c55_probe(struct platform_device *pdev)
>
> err_deinit_media_frameworks:
> mali_c55_media_frameworks_deinit(mali_c55);
> +err_pm_runtime_disable:
Is it necessary to
pm_runtime_set_suspended(&pdev->dev);
?
> pm_runtime_disable(&pdev->dev);
> -err_free_context_registers:
> kfree(mali_c55->context.registers);
> err_power_off:
> __mali_c55_power_off(mali_c55);
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release()
2026-03-27 16:58 ` [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release() Jacopo Mondi
@ 2026-03-27 17:05 ` David CARLIER
0 siblings, 0 replies; 10+ messages in thread
From: David CARLIER @ 2026-03-27 17:05 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Daniel Scally, Mauro Carvalho Chehab, Nayden Kanchev,
Hans Verkuil, linux-media
On Fri, 27 Mar 2026 at 16:58, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi David
>
> On Fri, Mar 27, 2026 at 03:07:05PM +0000, David Carlier wrote:
> > mali_c55_probe() calls of_reserved_mem_device_init() to associate
> > reserved memory regions with the device. This function allocates a
> > struct rmem_assigned_device and adds it to a global linked list, which
> > must be explicitly released via of_reserved_mem_device_release() — there
> > is no devm variant of this API.
> >
> > However, neither the probe error paths nor mali_c55_remove() called
> > of_reserved_mem_device_release(). Any probe failure after the
> > of_reserved_mem_device_init() call, as well as every normal device
> > removal, leaked the reserved memory association on the global list.
> >
> > Fix this by adding an err_release_mem label at the end of the probe
> > error chain and calling of_reserved_mem_device_release() in
> > mali_c55_remove(). The remove teardown order is also corrected to call
> > mali_c55_media_frameworks_deinit() before kfree(), mirroring the probe
> > init order in reverse.
> >
> > 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>
>
> With your permission, I'll take these three patches in a follow-up
> series that fixes the module loading/unloading.
Yes please go ahead ! Cheers,
>
> I could also specify it as pre-requisite to b4, I'll see what's easier
>
> Thanks
> j
>
> > ---
> > drivers/media/platform/arm/mali-c55/mali-c55-core.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > 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 c1a562cd214e..5cb59c70ffc9 100644
> > --- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> > @@ -806,8 +806,10 @@ static int mali_c55_probe(struct platform_device *pdev)
> > vb2_dma_contig_set_max_seg_size(dev, UINT_MAX);
> >
> > ret = __mali_c55_power_on(mali_c55);
> > - if (ret)
> > - return dev_err_probe(dev, ret, "failed to power on\n");
> > + if (ret) {
> > + dev_err_probe(dev, ret, "failed to power on\n");
> > + goto err_release_mem;
> > + }
> >
> > ret = mali_c55_check_hwcfg(mali_c55);
> > if (ret)
> > @@ -846,6 +848,8 @@ static int mali_c55_probe(struct platform_device *pdev)
> > kfree(mali_c55->context.registers);
> > err_power_off:
> > __mali_c55_power_off(mali_c55);
> > +err_release_mem:
> > + of_reserved_mem_device_release(dev);
> >
> > return ret;
> > }
> > @@ -854,8 +858,9 @@ static void mali_c55_remove(struct platform_device *pdev)
> > {
> > struct mali_c55 *mali_c55 = platform_get_drvdata(pdev);
> >
> > - kfree(mali_c55->context.registers);
> > mali_c55_media_frameworks_deinit(mali_c55);
> > + kfree(mali_c55->context.registers);
> > + of_reserved_mem_device_release(&pdev->dev);
> > }
> >
> > static const struct of_device_id mali_c55_of_match[] = {
> > --
> > 2.53.0
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] media: mali-c55: fix probe error path skipping pm_runtime_disable()
2026-03-27 17:04 ` Jacopo Mondi
@ 2026-03-27 17:10 ` David CARLIER
0 siblings, 0 replies; 10+ messages in thread
From: David CARLIER @ 2026-03-27 17:10 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Daniel Scally, Mauro Carvalho Chehab, Nayden Kanchev,
Hans Verkuil, linux-media
On Fri, 27 Mar 2026 at 17:04, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi David
>
> On Fri, Mar 27, 2026 at 03:07:07PM +0000, David Carlier wrote:
> > When mali_c55_media_frameworks_init() fails, the goto target jumps to
> > err_free_context_registers, skipping pm_runtime_disable() despite
> > pm_runtime having already been enabled earlier in the function.
> >
> > Fix this by adding an err_pm_runtime_disable label and redirecting the
> > frameworks init failure to it, so pm_runtime is properly unwound on
> > that error path.
> >
> > Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> > drivers/media/platform/arm/mali-c55/mali-c55-core.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > 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 38b11d5ba168..f998f914b355 100644
> > --- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> > @@ -828,7 +828,7 @@ static int mali_c55_probe(struct platform_device *pdev)
> >
> > ret = mali_c55_media_frameworks_init(mali_c55);
> > if (ret)
> > - goto err_free_context_registers;
> > + goto err_pm_runtime_disable;
> >
> > pm_runtime_idle(&pdev->dev);
> >
> > @@ -843,8 +843,8 @@ static int mali_c55_probe(struct platform_device *pdev)
> >
> > err_deinit_media_frameworks:
> > mali_c55_media_frameworks_deinit(mali_c55);
> > +err_pm_runtime_disable:
>
> Is it necessary to
>
> pm_runtime_set_suspended(&pdev->dev);
>
> ?
>
> > pm_runtime_disable(&pdev->dev);
> > -err_free_context_registers:
> > kfree(mali_c55->context.registers);
> > err_power_off:
> > __mali_c55_power_off(mali_c55);
> > --
Hi,
Good point — yes, pm_runtime_set_active() is called earlier in
probe, so the error unwind should also call pm_runtime_set_suspended()
to undo it and decrement the
parent's usage count. Same applies to the remove path.
I'll send a v4 with that added to both patches 2 and 3.
Thanks for the review,
David
> > 2.53.0
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove
2026-03-27 17:01 ` Jacopo Mondi
@ 2026-03-28 14:58 ` Jacopo Mondi
2026-03-28 15:09 ` David CARLIER
0 siblings, 1 reply; 10+ messages in thread
From: Jacopo Mondi @ 2026-03-28 14:58 UTC (permalink / raw)
To: Jacopo Mondi
Cc: David Carlier, Daniel Scally, Mauro Carvalho Chehab,
Nayden Kanchev, Hans Verkuil, linux-media
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
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove
2026-03-28 14:58 ` Jacopo Mondi
@ 2026-03-28 15:09 ` David CARLIER
0 siblings, 0 replies; 10+ messages in thread
From: David CARLIER @ 2026-03-28 15:09 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Daniel Scally, Mauro Carvalho Chehab, Nayden Kanchev,
Hans Verkuil, linux-media
Hi sorry I missed your previous messages ok looking into it. Cheers.
On Sat, 28 Mar 2026 at 14:58, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> 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
> > >
> > >
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-03-28 15:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-27 15:07 [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release() David Carlier
2026-03-27 15:07 ` [PATCH v3 2/3] media: mali-c55: add missing pm_runtime_disable() in remove David Carlier
2026-03-27 17:01 ` Jacopo Mondi
2026-03-28 14:58 ` Jacopo Mondi
2026-03-28 15:09 ` David CARLIER
2026-03-27 15:07 ` [PATCH v3 3/3] media: mali-c55: fix probe error path skipping pm_runtime_disable() David Carlier
2026-03-27 17:04 ` Jacopo Mondi
2026-03-27 17:10 ` David CARLIER
2026-03-27 16:58 ` [PATCH v3 1/3] media: mali-c55: add missing of_reserved_mem_device_release() Jacopo Mondi
2026-03-27 17:05 ` David CARLIER
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox