Re: [PATCH 1/2] irqchip: imx-intmux: add system PM support

[Marc Zyngier]

On 2020-07-16 20:32, Joakim Zhang wrote:

Add system PM support for intmux interrupt controller.

Care to be a little more descriptive?

Signed-off-by: Joakim Zhang <qiangqing.zh...@nxp.com>
---
 drivers/irqchip/irq-imx-intmux.c | 59 ++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c

index c27577c81126..6095f76c4f0d 100644
--- a/drivers/irqchip/irq-imx-intmux.c
+++ b/drivers/irqchip/irq-imx-intmux.c
@@ -70,6 +70,9 @@ struct intmux_data {
        void __iomem                    *regs;
        struct clk                      *ipg_clk;
        int                             channum;
+#ifdef CONFIG_PM
+       unsigned int                    *saved_reg;

Please use u32 for 32bit HW registers.

+#endif
        struct intmux_irqchip_data      irqchip_data[];
 };

@@ -232,6 +235,15 @@ static int imx_intmux_probe(struct platform_device *pdev)

        data->channum = channum;
        raw_spin_lock_init(&data->lock);

+       if (IS_ENABLED(CONFIG_PM)) {
+               /* save CHANIER register */
+               data->saved_reg = devm_kzalloc(&pdev->dev,
+                                              sizeof(unsigned int) * channum,
+                                              GFP_KERNEL);
+               if (!data->saved_reg)

Which isn't defined when !CONFIG_PM. The compiler is allowed to
bail here, and does (see the two kbuild failures).

+                       return -ENOMEM;
+       }
+
        ret = clk_prepare_enable(data->ipg_clk);
        if (ret) {
                dev_err(&pdev->dev, "failed to enable ipg clk: %d\n", ret);

@@ -293,6 +305,53 @@ static int imx_intmux_remove(struct platform_device *pdev)

        return 0;
 }

+#ifdef CONFIG_PM
+static void imx_intmux_save_regs(struct intmux_data *data)
+{
+       int i;
+
+       for (i = 0; i < data->channum; i++)
+               data->saved_reg[i] = readl_relaxed(data->regs + CHANIER(i));
+}
+
+static void imx_intmux_restore_regs(struct intmux_data *data)
+{
+       int i;
+
+       for (i = 0; i < data->channum; i++)
+               writel_relaxed(data->saved_reg[i], data->regs + CHANIER(i));
+}

Please move these two trivial functions into their respective callers.

+
+static int imx_intmux_suspend(struct device *dev)
+{
+       struct intmux_data *data = dev_get_drvdata(dev);
+
+       imx_intmux_save_regs(data);
+       clk_disable_unprepare(data->ipg_clk);
+
+       return 0;
+}
+
+static int imx_intmux_resume(struct device *dev)
+{
+       struct intmux_data *data = dev_get_drvdata(dev);
+       int ret;
+
+       ret = clk_prepare_enable(data->ipg_clk);
+       if (ret) {
+               dev_err(dev, "failed to enable ipg clk: %d\n", ret);
+               return ret;
+       }
+       imx_intmux_restore_regs(data);
+
+       return 0;
+}
+#endif
+
+static const struct dev_pm_ops imx_intmux_pm_ops = {
+       SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_intmux_suspend, imx_intmux_resume)
+};
+
 static const struct of_device_id imx_intmux_id[] = {
        { .compatible = "fsl,imx-intmux", },
        { /* sentinel */ },

Thanks,

        M.
--
Jazz is not dead. It just smells funny...