Messages in this thread |  | | Date | Sat, 25 Jul 2020 14:05:20 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH V2 1/1] irqchip: imx-intmux: implement intmux PM |
| |
On Mon, 20 Jul 2020 11:42:37 +0100, Joakim Zhang <qiangqing.zhang@nxp.com> wrote: > > When system suspended, we could explicitly disable clock to save power. > And we need save registers' state since it could be lost after power > off. > > Implement PM which will: > 1) Without CONFIG_PM, clock is always on after probe stage. > 2) With CONFIG_PM, clock is off after probe stage. > 3) Disable clock and save registers' state when do system suspend and > enable clock and restore registers' state while system resume. > 4) Make Power Domain framework be able to shutdown the corresponding > power domain of this device. > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > --- > drivers/irqchip/irq-imx-intmux.c | 70 +++++++++++++++++++++++++++++++- > 1 file changed, 68 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c > index c27577c81126..5971603cc607 100644 > --- a/drivers/irqchip/irq-imx-intmux.c > +++ b/drivers/irqchip/irq-imx-intmux.c > @@ -53,6 +53,7 @@ > #include <linux/of_irq.h> > #include <linux/of_platform.h> > #include <linux/spinlock.h> > +#include <linux/pm_runtime.h> > > #define CHANIER(n) (0x10 + (0x40 * n)) > #define CHANIPR(n) (0x20 + (0x40 * n)) > @@ -60,6 +61,7 @@ > #define CHAN_MAX_NUM 0x8 > > struct intmux_irqchip_data { > + struct irq_chip chip; > int chanidx; > int irq; > struct irq_domain *domain; > @@ -70,6 +72,7 @@ struct intmux_data { > void __iomem *regs; > struct clk *ipg_clk; > int channum; > + u32 *saved_reg; > struct intmux_irqchip_data irqchip_data[]; > }; > > @@ -120,8 +123,10 @@ static struct irq_chip imx_intmux_irq_chip = { > static int imx_intmux_irq_map(struct irq_domain *h, unsigned int irq, > irq_hw_number_t hwirq) > { > - irq_set_chip_data(irq, h->host_data); > - irq_set_chip_and_handler(irq, &imx_intmux_irq_chip, handle_level_irq); > + struct intmux_irqchip_data *data = h->host_data; > + > + irq_set_chip_data(irq, data); > + irq_set_chip_and_handler(irq, &data->chip, handle_level_irq); > > return 0; > } > @@ -232,6 +237,19 @@ 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,
This isn't consistent with the type of data->saved_reg. Consider using sizeof(*data->saved_reg), which is guaranteed to be the right type.
It also begs the question: since this saved_reg array is allocated on a per channel basis, why don't you have a per-channel additional u32 in the intmux_irqchip_data structure instead? This would sidestep this extra allocation altogether.
Thanks,
M.
-- Without deviation from the norm, progress is not possible.
|  |