Messages in this thread | | | Date | Fri, 17 Jul 2020 09:41:56 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH 1/2] irqchip: imx-intmux: add system PM support |
| |
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.zhang@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...
| |