Re: [PATCH 2/3] media: mali-c55: Keep IRQ requested during suspend
From: Dan Scally
Date: Wed Oct 07 2026 - 07:11:52 EST
Hi Linus
On 29/09/2026 13:02, Linus Walleij wrote:
The interrupt is currently freed on every runtime suspend and requested
again on runtime resume. Apart from tying interrupt ownership to the
power state rather than to the driver lifetime, this leaves remove to
guess whether an action is installed for the IRQ.
Request the interrupt once during probe and free it during remove.
Disable and synchronize the IRQ before powering the ISP off, and enable
it only after a successful power-on.
Hold a runtime PM reference during probe until the IRQ is installed,
and stop runtime PM and drain the IRQ before unregistering the media
entities during remove.
If firmware marks the ISP as a wakeup source, initialize device wakeup
and enable IRQ wake during system suspend. Disable it again before
resuming the device.
I think that adding this could be a separate commit to fixing the irq handling...and perhaps that should be the case given the Fixes tag?
This configures the interrupt controller wake path.
Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@xxxxxxxxxx>
---
.../media/platform/arm/mali-c55/mali-c55-core.c | 84 ++++++++++++++++------
1 file changed, 64 insertions(+), 20 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 f28e9f4354ac..07267b79801b 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
@@ -17,6 +17,8 @@
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/pm_wakeup.h>
+#include <linux/property.h>
#include <linux/reset.h>
#include <linux/slab.h>
#include <linux/string.h>
@@ -675,8 +677,7 @@ static int __maybe_unused mali_c55_runtime_suspend(struct device *dev)
{
struct mali_c55 *mali_c55 = dev_get_drvdata(dev);
- if (irq_has_action(mali_c55->irqnum))
- free_irq(mali_c55->irqnum, dev);
+ disable_irq(mali_c55->irqnum);
__mali_c55_power_off(mali_c55);
return 0;
@@ -745,25 +746,41 @@ static int __maybe_unused mali_c55_runtime_resume(struct device *dev)
if (ret)
return ret;
- /*
- * The driver needs to transfer large amounts of register settings to
- * the ISP each frame, using either a DMA transfer or memcpy. We use a
- * threaded IRQ to avoid disabling interrupts the entire time that's
- * happening.
- */
- ret = request_threaded_irq(mali_c55->irqnum, NULL, mali_c55_isr,
- IRQF_ONESHOT, dev_driver_string(dev), dev);
- if (ret) {
- __mali_c55_power_off(mali_c55);
- dev_err(dev, "failed to request irq\n");
+ enable_irq(mali_c55->irqnum);
+
+ return 0;
+}
+
+static int __maybe_unused mali_c55_suspend(struct device *dev)
+{
+ struct mali_c55 *mali_c55 = dev_get_drvdata(dev);
+ int ret;
+
+ if (device_may_wakeup(dev)) {
+ ret = enable_irq_wake(mali_c55->irqnum);
+ if (ret)
+ return ret;
}
+ ret = pm_runtime_force_suspend(dev);
+ if (ret && device_may_wakeup(dev))
+ disable_irq_wake(mali_c55->irqnum);
+
return ret;
}
+static int __maybe_unused mali_c55_resume(struct device *dev)
+{
+ struct mali_c55 *mali_c55 = dev_get_drvdata(dev);
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(mali_c55->irqnum);
+
+ return pm_runtime_force_resume(dev);
+}
+
static const struct dev_pm_ops mali_c55_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
- pm_runtime_force_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(mali_c55_suspend, mali_c55_resume)
SET_RUNTIME_PM_OPS(mali_c55_runtime_suspend, mali_c55_runtime_resume,
NULL)
};
@@ -825,27 +842,53 @@ static int mali_c55_probe(struct platform_device *pdev)
pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
+ pm_runtime_get_noresume(dev);
pm_runtime_enable(&pdev->dev);
ret = mali_c55_media_frameworks_init(mali_c55);
if (ret)
goto err_pm_runtime_disable;
- pm_runtime_idle(&pdev->dev);
-
mali_c55->irqnum = platform_get_irq(pdev, 0);
if (mali_c55->irqnum < 0) {
ret = mali_c55->irqnum;
goto err_deinit_media_frameworks;
}
+ /*
+ * The driver needs to transfer large amounts of register settings to
+ * the ISP each frame, using either a DMA transfer or memcpy. We use a
+ * threaded IRQ to avoid disabling interrupts the entire time that's
+ * happening.
+ */
+ ret = request_threaded_irq(mali_c55->irqnum, NULL, mali_c55_isr,
+ IRQF_ONESHOT, dev_driver_string(dev), dev);
+ if (ret) {
+ dev_err(dev, "failed to request irq\n");
+ goto err_deinit_media_frameworks;
+ }
+
+ if (device_property_read_bool(dev, "wakeup-source")) {
+ ret = devm_device_init_wakeup(dev);
+ if (ret) {
+ ret = dev_err_probe(dev, ret,
+ "failed to initialize wakeup\n");
+ goto err_free_irq;
+ }
+ }
+
+ pm_runtime_put_autosuspend(dev);
+
return 0;
+err_free_irq:
+ free_irq(mali_c55->irqnum, dev);
err_deinit_media_frameworks:
mali_c55_media_frameworks_deinit(mali_c55);
err_pm_runtime_disable:
- pm_runtime_set_suspended(&pdev->dev);
pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(dev);
+ pm_runtime_set_suspended(&pdev->dev);
I would say that this re-ordering of the pm_runtime_set_suspended() and pm_runtime_disable() calls probably ought to be in a separate commit too, since it's a distinct change that should be backported to fix stable branches.
Thanks
Dan
kfree(mali_c55->context.registers);
err_power_off:
__mali_c55_power_off(mali_c55);
@@ -859,12 +902,13 @@ static void mali_c55_remove(struct platform_device *pdev)
{
struct mali_c55 *mali_c55 = platform_get_drvdata(pdev);
+ pm_runtime_disable(&pdev->dev);
+ free_irq(mali_c55->irqnum, &pdev->dev);
mali_c55_media_frameworks_deinit(mali_c55);
- if (!pm_runtime_suspended(&pdev->dev)) {
+ if (!pm_runtime_status_suspended(&pdev->dev)) {
__mali_c55_power_off(mali_c55);
pm_runtime_set_suspended(&pdev->dev);
}
- pm_runtime_disable(&pdev->dev);
kfree(mali_c55->context.registers);
of_reserved_mem_device_release(&pdev->dev);
}