Il 04/02/26 09:54, Tzung-Bi Shih ha scritto:
Prior to commit d935187cfb27 ("remoteproc: mediatek: Break lock
dependency to prepare_lock"), `scp->clk` was prepared and enabled only
when it needs to communicate with the SCP. The commit d935187cfb27
moved the prepare operation to remoteproc's prepare(), keeping the clock
prepared as long as the SCP is running.
The power consumption due to the prolonged clock preparation can be
negligible when the system is running, as SCP is designed to be a very
power efficient processor.
However, the clock remains prepared even when the system enters system
suspend. This prevents the underlying clock controller (and potentially
the parent PLLs) from shutting down, which increases power consumption
and may block the system from entering deep sleep states.
Add suspend and resume callbacks. Unprepare the clock in suspend() if
it was active and re-prepare it in resume() to ensure the clock is
properly disabled during system suspend, while maintaining the "always
prepared" semantics while the system is active.
Fixes: d935187cfb27 ("remoteproc: mediatek: Break lock dependency to
prepare_lock")
Signed-off-by: Tzung-Bi Shih <[email protected]>
Would be great if you could mention that in this driver, at the moment of
writing,
there is no .attach() callback, hence rproc->state can never be RPROC_ATTACHED
(as
in case that would also leave the clock prepared).
In any case, even though I don't exactly *love* how this is actually done, I
don't
really have any better option.
But....
---
drivers/remoteproc/mtk_scp.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 4651311aeb07..6200c687e232 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -1592,12 +1592,40 @@ static const struct of_device_id mtk_scp_of_match[] = {
};
MODULE_DEVICE_TABLE(of, mtk_scp_of_match);
+#ifdef CONFIG_PM_SLEEP
...you don't need this ifdef as when CONFIG_PM_SLEEP is not set the macro
SET_SYSTEM_SLEEP_PM_OPS is stubbed out (check pm.h).
After removing the ifdef, there'll be something to change, specifically:
+static int scp_suspend(struct device *dev)
static int __maybe_unused scp_suspend(struct device *dev)
{
+{
+ struct mtk_scp *scp = dev_get_drvdata(dev);
+ struct rproc *rproc = scp->rproc;
+
+ /* Only unprepare if the SCP is running and holding the clock */
+ if (rproc->state == RPROC_RUNNING)
+ clk_unprepare(scp->clk);
+ return 0;
+}
+
+static int scp_resume(struct device *dev)
static int __maybe_unused scp_resume(struct device *dev)
{
After which,
Reviewed-by: AngeloGioacchino Del Regno
<[email protected]>
Cheers,
Angelo
+{
+ struct mtk_scp *scp = dev_get_drvdata(dev);
+ struct rproc *rproc = scp->rproc;
+
+ if (rproc->state == RPROC_RUNNING)
+ return clk_prepare(scp->clk);
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops scp_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(scp_suspend, scp_resume)
+};
+
static struct platform_driver mtk_scp_driver = {
.probe = scp_probe,
.remove = scp_remove,
.driver = {
.name = "mtk-scp",
.of_match_table = mtk_scp_of_match,
+ .pm = &scp_pm_ops,
},
};
