[PATCH 0/3] Handle mailbox clock/power management related issues

[PATCH 0/3] Handle mailbox clock/power management related issues

[Anson Huang]

Current i.MX mailbox driver mainly supports 2 series i.MX SoCs with
different architecture, one is for i.MX8X platforms with SCU inside,
the other is for i.MX6/7/8M series without SCU.

For i.MX8X, 2 types of MU are supported, one is for system IPC, such kind
of MU has no clock/power assignment, they are both controlled by SCU. The
other is for application, such kind of MU has no clock assignment, but have
power domain assignment, consumers need to call mailbox APIs to manage MU
power in runtime;

For i.MX6/7/8M, MU clock or power could be assigned based on different SoCs,
but all the MUs are for application, consumers need to call mailbox APIs to
manage MU clock/power in runtime.

For the power management related issue mentioned above, they are as below:

1. clock should be managed in runtime to make sure MU clock/power can be off
   on i.MX6/7/8M platforms;
2. ONLY system IPC MU needs to have IRQF_NO_SUSPEND flag set, other application
   MU no need to have this flag, since the MU clock/power is OFF in noirq
   suspend phase and if MU interrupt arrives, with IRQF_NO_SUSPEND flag set,
   the MU ISR will try to access MU register and lead to system hang because
   of clock/power disabled;

To distinguish these different MU instances, use MU's clock/power assignment
status to decide whether to save/restore MU context during suspend/resume,
whether to have IRQF_NO_SUSPEND flag set, etc..

patch #1 is identical with https://patchwork.kernel.org/patch/11581215/, the
patch #2/#3 depend on #1, so I resend #1 in this series to make them as a whole
series.

Anson Huang (2):
  mailbox: imx: Add runtime PM callback to handle MU clocks
  mailbox: imx: ONLY IPC MU needs IRQF_NO_SUSPEND flag

Dong Aisheng (1):
  mailbox: imx: Add context save/restore for suspend/resume

 drivers/mailbox/imx-mailbox.c | 72 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 1/3] mailbox: imx: Add context save/restore for suspend/resume

[Anson Huang]

From: Dong Aisheng <aisheng.dong@nxp.com>

For "mem" mode suspend on i.MX8 SoCs, MU settings could be
lost because its power is off, so save/restore is needed
for MU settings during suspend/resume. However, the restore
can ONLY be done when MU settings are actually lost, for the
scenario of settings NOT lost in "freeze" mode suspend, since
there could be still IPC going on multiple CPUs, restoring the
MU settings could overwrite the TIE by mistake and cause system
freeze, so need to make sure ONLY restore the MU settings when
it is powered off, Anson fixes this by checking whether restore
is actually needed when resume.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index bd69ecf..da90a8e 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -67,6 +67,8 @@ struct imx_mu_priv {
 	struct clk		*clk;
 	int			irq;
 
+	u32 xcr;
+
 	bool			side_b;
 };
 
@@ -589,12 +591,45 @@ static const struct of_device_id imx_mu_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, imx_mu_dt_ids);
 
+static int imx_mu_suspend_noirq(struct device *dev)
+{
+	struct imx_mu_priv *priv = dev_get_drvdata(dev);
+
+	priv->xcr = imx_mu_read(priv, priv->dcfg->xCR);
+
+	return 0;
+}
+
+static int imx_mu_resume_noirq(struct device *dev)
+{
+	struct imx_mu_priv *priv = dev_get_drvdata(dev);
+
+	/*
+	 * ONLY restore MU when context lost, the TIE could
+	 * be set during noirq resume as there is MU data
+	 * communication going on, and restore the saved
+	 * value will overwrite the TIE and cause MU data
+	 * send failed, may lead to system freeze. This issue
+	 * is observed by testing freeze mode suspend.
+	 */
+	if (!imx_mu_read(priv, priv->dcfg->xCR))
+		imx_mu_write(priv, priv->xcr, priv->dcfg->xCR);
+
+	return 0;
+}
+
+static const struct dev_pm_ops imx_mu_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_mu_suspend_noirq,
+				      imx_mu_resume_noirq)
+};
+
 static struct platform_driver imx_mu_driver = {
 	.probe		= imx_mu_probe,
 	.remove		= imx_mu_remove,
 	.driver = {
 		.name	= "imx_mu",
 		.of_match_table = imx_mu_dt_ids,
+		.pm = &imx_mu_pm_ops,
 	},
 };
 module_platform_driver(imx_mu_driver);
-- 
2.7.4

[PATCH 2/3] mailbox: imx: Add runtime PM callback to handle MU clocks

[Anson Huang]

Some of i.MX8M SoCs have MU clock, they need to be managed in runtime
to make sure the MU clock can be off in runtime, add runtime PM callback
to handle MU clock.

And on i.MX8MP, the MU clock is combined with power domain and runtime
PM is enabled for the clock driver, during noirq suspend/resume phase,
runtime PM is disabled by device suspend, but the MU context save/restore
needs to enable MU clock for register access, calling clock prepare/enable
will trigger runtime resume failure and lead to system suspend failed.

Actually, the MU context save/restore is ONLY necessary for SCU IPC MU,
other MUs especially on i.MX8MP platforms which have MU clock assigned,
they need to runtime request/free mailbox channel in the consumer driver,
so no need to save/restore MU context for them, hence it can avoid this
issue, so the MU context save/restore is ONLY applied to i.MX platforms
MU instance without clock present.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index da90a8e..080b608 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -536,10 +536,13 @@ static int imx_mu_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto disable_runtime_pm;
 
+	clk_disable_unprepare(priv->clk);
+
 	return 0;
 
 disable_runtime_pm:
 	pm_runtime_disable(dev);
+	clk_disable_unprepare(priv->clk);
 	return ret;
 }
 
@@ -547,7 +550,6 @@ static int imx_mu_remove(struct platform_device *pdev)
 {
 	struct imx_mu_priv *priv = platform_get_drvdata(pdev);
 
-	clk_disable_unprepare(priv->clk);
 	pm_runtime_disable(priv->dev);
 
 	return 0;
@@ -595,7 +597,8 @@ static int imx_mu_suspend_noirq(struct device *dev)
 {
 	struct imx_mu_priv *priv = dev_get_drvdata(dev);
 
-	priv->xcr = imx_mu_read(priv, priv->dcfg->xCR);
+	if (!priv->clk)
+		priv->xcr = imx_mu_read(priv, priv->dcfg->xCR);
 
 	return 0;
 }
@@ -612,15 +615,38 @@ static int imx_mu_resume_noirq(struct device *dev)
 	 * send failed, may lead to system freeze. This issue
 	 * is observed by testing freeze mode suspend.
 	 */
-	if (!imx_mu_read(priv, priv->dcfg->xCR))
+	if (!imx_mu_read(priv, priv->dcfg->xCR) && !priv->clk)
 		imx_mu_write(priv, priv->xcr, priv->dcfg->xCR);
 
 	return 0;
 }
 
+static int imx_mu_runtime_suspend(struct device *dev)
+{
+	struct imx_mu_priv *priv = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static int imx_mu_runtime_resume(struct device *dev)
+{
+	struct imx_mu_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		dev_err(dev, "failed to enable clock\n");
+
+	return ret;
+}
+
 static const struct dev_pm_ops imx_mu_pm_ops = {
 	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_mu_suspend_noirq,
 				      imx_mu_resume_noirq)
+	SET_RUNTIME_PM_OPS(imx_mu_runtime_suspend,
+			   imx_mu_runtime_resume, NULL)
 };
 
 static struct platform_driver imx_mu_driver = {
-- 
2.7.4

[PATCH 3/3] mailbox: imx: ONLY IPC MU needs IRQF_NO_SUSPEND flag

[Anson Huang]

IPC MU has no power domain assigned and there could be IPC during
noirq suspend phase, so IRQF_NO_SUSPEND flag is needed for IPC MU.
However, for other MUs, they have power domain assigned and their
power will be turned off during noirq suspend phase, but with
IRQF_NO_SUSPEND set, their interrupts are NOT disabled even after
their power turned off, it will cause system crash when mailbox
driver trys to handle pending interrupts but the MU power is already
turned off.

So, IRQF_NO_SUSPEND flag should ONLY be added to IPC MU which has
power domain managed by SCU, then all other MUs' pending interrupts
after noirq suspend phase will be handled after system resume.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 080b608..7205b82 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -292,6 +292,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
 {
 	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
 	struct imx_mu_con_priv *cp = chan->con_priv;
+	unsigned long irq_flag = IRQF_SHARED;
 	int ret;
 
 	pm_runtime_get_sync(priv->dev);
@@ -302,8 +303,12 @@ static int imx_mu_startup(struct mbox_chan *chan)
 		return 0;
 	}
 
-	ret = request_irq(priv->irq, imx_mu_isr, IRQF_SHARED |
-			  IRQF_NO_SUSPEND, cp->irq_desc, chan);
+	/* IPC MU should be with IRQF_NO_SUSPEND set */
+	if (!priv->dev->pm_domain)
+		irq_flag |= IRQF_NO_SUSPEND;
+
+	ret = request_irq(priv->irq, imx_mu_isr, irq_flag,
+			  cp->irq_desc, chan);
 	if (ret) {
 		dev_err(priv->dev,
 			"Unable to acquire IRQ %d\n", priv->irq);
-- 
2.7.4